# Phase 2: Breaking Changes from Phase 1 Review Fixes

## ProcessingResult Signature Change

The `ProcessingResult` class signature has changed to align with SDD specifications.

### Old Signature (Phase 1 initial)
```php
ProcessingResult::success(
    string $providerMessageId,
    string $displayStatus,      // ❌ Changed
    bool $billingUpdated,
    bool $chatMessageUpdated,
    bool $ablyPublished,
    bool $costJobDispatched
);

ProcessingResult::failure(string $errorMessage);  // ❌ Changed
```

### New Signature (Phase 1 Review Fixes)
```php
ProcessingResult::success(
    string $providerMessageId,
    MappedStatus $mappedStatus,  // ✅ New - full mapped status object
    ?string $typeNum,            // ✅ New - store identifier (null if unmatched)
    bool $billingUpdated,
    bool $chatMessageUpdated,
    bool $ablyPublished,
    bool $costJobDispatched
);

ProcessingResult::failure(
    string $providerMessageId,   // ✅ New parameter
    string $errorMessage
);
```

### What Changed

1. **success()**: Now accepts `MappedStatus` object instead of string `displayStatus`
2. **success()**: Now accepts `typeNum` (store identifier) as parameter
3. **failure()**: Now requires `providerMessageId` as first parameter
4. **New getters**: `getMappedStatus()` and `getTypeNum()`
5. **Convenience getter**: `getDisplayStatus()` still works (reads from mappedStatus)

### Required Updates in Phase 2

**File:** `userfrosting/src/BuyerKiosk/SMS/Webhooks/DeliveryStatusProcessor.php` (to be created)

When creating this class in Phase 2, use the NEW signature:

```php
// ✅ CORRECT - Use new signature
$mapped = DeliveryStatusMapper::mapTwilioStatus($rawStatus);

return ProcessingResult::success(
    $providerMessageId,
    $mapped,                    // Pass MappedStatus object
    $record['typeNum'] ?? null, // Pass typeNum
    true,
    true,
    true,
    false
);

// For failures:
return ProcessingResult::failure($providerMessageId, "Error message");
```

```php
// ❌ WRONG - Old signature (Phase 1 initial)
return ProcessingResult::success(
    $providerMessageId,
    'delivered',  // ❌ String displayStatus - no longer accepted
    true,
    true,
    true,
    false
);

// ❌ WRONG - Old failure signature
return ProcessingResult::failure("Error message");  // ❌ Missing providerMessageId
```

### Migration Guide

If you already started Phase 2 implementation with old signature:

1. **Import MappedStatus**: Add `use BuyerKiosk\SMS\Webhooks\MappedStatus;`
2. **Get mapped status**: `$mapped = DeliveryStatusMapper::mapTwilioStatus($rawStatus);`
3. **Replace displayStatus with mapped**: Change second parameter from string to MappedStatus
4. **Add typeNum parameter**: Pass `$record['typeNum'] ?? null` as third parameter
5. **Update failure calls**: Add providerMessageId as first parameter

### Testing Impact

All existing ProcessingResultTest tests have been updated. Phase 2 tests should follow the new pattern:

```php
public function testProcessDeliverySuccess(): void
{
    // Arrange
    $mappedStatus = new MappedStatus('delivered', 'delivered', true);

    // Act
    $result = ProcessingResult::success(
        'SM123',
        $mappedStatus,
        'ou00',
        true,
        true,
        true,
        false
    );

    // Assert
    $this->assertEquals('delivered', $result->getDisplayStatus());
    $this->assertEquals('ou00', $result->getTypeNum());
}
```

### Rationale

This change aligns with SDD specification in `solution-design.md` line 779-788:

```
ENTITY: ProcessingResult (NEW)
  FIELDS:
    success: boolean
    providerMessageId: string
    mappedStatus: MappedStatus      ← Full object, not just displayStatus
    typeNum: ?string                ← Store identifier
    chatMessageUpdated: boolean
    ablyPublished: boolean
    costJobDispatched: boolean
    errorMessage: ?string
```

The change provides:
- **More context**: Full MappedStatus object includes rawStatus and isFinal
- **Better traceability**: typeNum shows which store the message belongs to
- **Failure context**: providerMessageId in failures aids debugging