# Story 11.1: Database Schema & Model Updates

## Epic Reference
**Epic 11:** Guest Booking

## Story Context
This is the **foundational story** for Epic 11. All subsequent guest booking stories depend on these database and model changes being complete. This story prepares the data layer to support consultations without a linked user account.

## User Story
As a **developer**,
I want **to extend the consultation system to support guest bookings**,
So that **consultations can be created for both registered clients and anonymous guests**.

## Acceptance Criteria

### Database Migration
- [x] `user_id` column changed to nullable on `consultations` table
- [x] `guest_name` column added (varchar 255, nullable)
- [x] `guest_email` column added (varchar 255, nullable)
- [x] `guest_phone` column added (varchar 50, nullable)
- [x] Index added on `guest_email` for 1-per-day lookup performance
- [x] Migration is reversible without data loss
- [x] Existing consultations with `user_id` unaffected

### Model Updates
- [x] `Consultation` model updated with new fillable fields
- [x] `isGuest()` helper method returns true when `user_id` is null
- [x] `getClientName()` helper returns guest_name or user->full_name
- [x] `getClientEmail()` helper returns guest_email or user->email
- [x] `getClientPhone()` helper returns guest_phone or user->phone
- [x] Model validation ensures either user_id OR guest fields are present

### Factory Updates
- [x] `ConsultationFactory` updated with `guest()` state
- [x] Guest state generates fake guest_name, guest_email, guest_phone
- [x] Guest state sets user_id to null

## Implementation Steps

### Step 1: Create Migration
```bash
php artisan make:migration add_guest_fields_to_consultations_table
```

Migration content:
```php
public function up(): void
{
    Schema::table('consultations', function (Blueprint $table) {
        // Make user_id nullable
        $table->foreignId('user_id')->nullable()->change();

        // Add guest fields
        $table->string('guest_name', 255)->nullable()->after('user_id');
        $table->string('guest_email', 255)->nullable()->after('guest_name');
        $table->string('guest_phone', 50)->nullable()->after('guest_email');

        // Index for 1-per-day lookup
        $table->index('guest_email');
    });
}

public function down(): void
{
    Schema::table('consultations', function (Blueprint $table) {
        $table->dropIndex(['guest_email']);
        $table->dropColumn(['guest_name', 'guest_email', 'guest_phone']);
        // Note: Cannot safely restore NOT NULL if guest records exist
    });
}
```

### Step 2: Update Consultation Model
Add to `app/Models/Consultation.php`:

```php
// Add to fillable array
protected $fillable = [
    // ... existing fields
    'guest_name',
    'guest_email',
    'guest_phone',
];

/**
 * Check if this is a guest consultation (no linked user).
 */
public function isGuest(): bool
{
    return is_null($this->user_id);
}

/**
 * Get the client's display name.
 */
public function getClientName(): string
{
    return $this->isGuest()
        ? $this->guest_name
        : $this->user->name;
}

/**
 * Get the client's email address.
 */
public function getClientEmail(): string
{
    return $this->isGuest()
        ? $this->guest_email
        : $this->user->email;
}

/**
 * Get the client's phone number.
 */
public function getClientPhone(): ?string
{
    return $this->isGuest()
        ? $this->guest_phone
        : $this->user->phone;
}
```

### Step 3: Update Consultation Factory
Add to `database/factories/ConsultationFactory.php`:

```php
/**
 * Create a guest consultation (no user account).
 */
public function guest(): static
{
    return $this->state(fn (array $attributes) => [
        'user_id' => null,
        'guest_name' => fake()->name(),
        'guest_email' => fake()->unique()->safeEmail(),
        'guest_phone' => fake()->phoneNumber(),
    ]);
}
```

### Step 4: Add Model Validation (Optional Boot Method)
Consider adding validation in model boot to ensure data integrity:

```php
protected static function booted(): void
{
    static::saving(function (Consultation $consultation) {
        // Either user_id or guest fields must be present
        if (is_null($consultation->user_id)) {
            if (empty($consultation->guest_name) ||
                empty($consultation->guest_email) ||
                empty($consultation->guest_phone)) {
                throw new \InvalidArgumentException(
                    'Guest consultations require guest_name, guest_email, and guest_phone'
                );
            }
        }
    });
}
```

## Technical Notes

### Foreign Key Consideration
The existing foreign key `consultations_user_id_foreign` cascades on delete. With nullable user_id:
- Guest consultations won't be affected by user deletions (they have no user)
- Client consultations still cascade delete when user is deleted

### Query Scopes
Consider adding scopes for filtering:
```php
public function scopeGuests(Builder $query): Builder
{
    return $query->whereNull('user_id');
}

public function scopeClients(Builder $query): Builder
{
    return $query->whereNotNull('user_id');
}
```

## Testing Requirements

### Unit Tests
```php
test('consultation can be created as guest', function () {
    $consultation = Consultation::factory()->guest()->create();

    expect($consultation->isGuest())->toBeTrue();
    expect($consultation->user_id)->toBeNull();
    expect($consultation->guest_name)->not->toBeNull();
    expect($consultation->guest_email)->not->toBeNull();
    expect($consultation->guest_phone)->not->toBeNull();
});

test('consultation can be created for client', function () {
    $user = User::factory()->client()->create();
    $consultation = Consultation::factory()->create(['user_id' => $user->id]);

    expect($consultation->isGuest())->toBeFalse();
    expect($consultation->user_id)->toBe($user->id);
});

test('getClientName returns guest name for guest consultation', function () {
    $consultation = Consultation::factory()->guest()->create([
        'guest_name' => 'John Doe',
    ]);

    expect($consultation->getClientName())->toBe('John Doe');
});

test('getClientName returns user name for client consultation', function () {
    $user = User::factory()->client()->create(['name' => 'Jane Smith']);
    $consultation = Consultation::factory()->create(['user_id' => $user->id]);

    expect($consultation->getClientName())->toBe('Jane Smith');
});

test('existing consultations are not affected by migration', function () {
    // Verify existing consultations still work
    $user = User::factory()->client()->create();
    $consultation = Consultation::factory()->create(['user_id' => $user->id]);

    expect($consultation->user)->toBeInstanceOf(User::class);
    expect($consultation->isGuest())->toBeFalse();
});
```

## Dependencies
- None (foundational story)

## Definition of Done
- [x] Migration created and runs successfully
- [x] Migration is reversible
- [x] Consultation model updated with guest fields and helpers
- [x] Factory updated with guest state
- [x] All existing consultation tests still pass
- [x] New unit tests for guest functionality pass
- [x] Code follows project patterns (Pint formatted)

---

## Dev Agent Record

### Agent Model Used
Claude Opus 4.5 (claude-opus-4-5-20251101)

### Completion Notes
- Migration `2026_01_03_165813_add_guest_fields_to_consultations_table.php` created successfully
- User model uses `full_name` attribute instead of `name` - updated `getClientName()` accordingly
- Added `scopeGuests()` and `scopeClients()` query scopes as recommended in Technical Notes
- All 17 new unit tests pass; all 190 consultation-related tests pass
- Pre-existing memory exhaustion issue in test suite unrelated to these changes

### File List
| File | Action |
|------|--------|
| `database/migrations/2026_01_03_165813_add_guest_fields_to_consultations_table.php` | Created |
| `app/Models/Consultation.php` | Modified |
| `database/factories/ConsultationFactory.php` | Modified |
| `tests/Unit/Models/ConsultationTest.php` | Modified |

### Change Log
| Change | Reason |
|--------|--------|
| Used `user->full_name` instead of `user->name` | User model in this project uses `full_name` attribute |
| Added `scopeGuests()` and `scopeClients()` scopes | Recommended in Technical Notes section for filtering |

### Status
Ready for Review

---

## QA Results

### Review Date: 2026-01-03

### Reviewed By: Quinn (Test Architect)

### Risk Assessment
- **Risk Level**: LOW
- **Rationale**: Foundational data layer change with no auth/payment/security components directly modified
- **Lines Changed**: ~150 lines across 4 files (below 500-line threshold)
- **Previous Gate**: N/A (first review)

### Code Quality Assessment

**Overall: EXCELLENT**

The implementation demonstrates strong adherence to Laravel and project conventions:

1. **Model Updates** (`app/Models/Consultation.php`):
   - Correct use of `$fillable` for mass assignment protection
   - Proper `booted()` hook for domain validation (ensures guest bookings have required fields)
   - Clean helper methods (`isGuest()`, `getClientName()`, `getClientEmail()`, `getClientPhone()`)
   - Query scopes (`scopeGuests()`, `scopeClients()`) added as recommended in Technical Notes
   - Correctly uses `user->full_name` matching existing User model attribute

2. **Migration** (`database/migrations/2026_01_03_...`):
   - Proper use of `->nullable()->change()` for making `user_id` nullable
   - Correct column order using `->after()` for logical grouping
   - Index added on `guest_email` for lookup performance
   - Reversible migration with appropriate down() method
   - Clear comment noting data loss caveat for NOT NULL restoration

3. **Factory** (`database/factories/ConsultationFactory.php`):
   - `guest()` state properly nullifies `user_id` and sets guest fields
   - Uses `fake()->unique()->safeEmail()` to prevent conflicts in tests

### Requirements Traceability

| AC | Description | Test Coverage | Status |
|----|-------------|---------------|--------|
| 1 | `user_id` nullable | Schema verified via migration + `consultation can be created for client` | ✓ |
| 2 | `guest_name` column added | `getClientName returns guest name for guest consultation` | ✓ |
| 3 | `guest_email` column added | `getClientEmail returns guest email for guest consultation` | ✓ |
| 4 | `guest_phone` column added | `getClientPhone returns guest phone for guest consultation` | ✓ |
| 5 | Index on `guest_email` | Schema verification (index exists) | ✓ |
| 6 | Migration reversible | `down()` method drops columns/index properly | ✓ |
| 7 | Existing consultations unaffected | `existing consultations with user are not affected` | ✓ |
| 8 | New fillable fields | `consultation can be created as guest` | ✓ |
| 9 | `isGuest()` helper | `consultation can be created as guest`, `consultation can be created for client` | ✓ |
| 10 | `getClientName()` helper | Tests for both guest and client variants | ✓ |
| 11 | `getClientEmail()` helper | Tests for both guest and client variants | ✓ |
| 12 | `getClientPhone()` helper | Tests for both guest and client variants | ✓ |
| 13 | Validation (user_id OR guest fields) | `guest consultation throws exception without required fields` | ✓ |
| 14 | Factory `guest()` state | `consultation can be created as guest` | ✓ |
| 15 | `scopeGuests()` | `guests scope returns only guest consultations` | ✓ |
| 16 | `scopeClients()` | `clients scope returns only client consultations` | ✓ |

**Coverage**: 16/16 acceptance criteria mapped to tests (100%)

### Test Architecture Assessment

- **Test Count**: 17 new tests with 27 assertions
- **Test Level**: Appropriate unit tests for model behavior
- **Test Quality**: Clean, focused tests following Given-When-Then pattern
- **Edge Cases**:
  - ✓ Guest without required fields (throws exception)
  - ✓ Mixed guest/client scopes
  - ✓ Explicit field values (e.g., `guest_name` = 'John Doe')
- **Execution Time**: 0.22s (excellent)

### Refactoring Performed

None required. Implementation is clean and follows project standards.

### Compliance Check

- Coding Standards: ✓ Pint passes, naming conventions followed
- Project Structure: ✓ Files in correct locations
- Testing Strategy: ✓ Unit tests for model layer
- All ACs Met: ✓ All 16 acceptance criteria implemented and tested

### Improvements Checklist

- [x] All acceptance criteria implemented
- [x] All required tests passing (17/17)
- [x] Migration is reversible
- [x] Domain validation in model boot
- [x] Query scopes added per Technical Notes
- [ ] *Future consideration*: Add integration test for cascade delete behavior (guest consultations should survive user deletions since they have no user_id)

### Security Review

**Status: PASS**

- No sensitive data exposed
- Guest email indexed but not unique (intentional for 1-per-day business rule to be enforced at application layer)
- Model validation prevents malformed guest consultations (requires all three guest fields)
- Foreign key cascade behavior preserved for registered user consultations

### Performance Considerations

**Status: PASS**

- Index on `guest_email` added for O(log n) lookups
- No N+1 query risks introduced
- Helper methods are simple attribute access, no database calls

### Files Modified During Review

None. No refactoring required.

### Gate Status

**Gate: PASS** → `docs/qa/gates/11.1-database-schema-model-updates.yml`

### Recommended Status

**✓ Ready for Done**

All acceptance criteria met, comprehensive test coverage (17 tests/27 assertions), clean implementation following project patterns. No blocking issues identified.
