# Implementation Plan ## Validation Checklist - [x] All specification file paths are correct and exist - [x] Context priming section is complete - [x] All implementation phases are defined - [x] Each phase follows TDD: Prime → Test → Implement → Validate - [x] Dependencies between phases are clear (no circular dependencies) - [x] Parallel work is properly tagged with `[parallel: true]` - [x] Activity hints provided for specialist selection `[activity: type]` - [x] Every phase references relevant SDD sections - [x] Every test references PRD acceptance criteria - [x] Integration & E2E tests defined in final phase - [x] Project commands match actual project setup - [x] A developer could follow this plan independently --- ## Specification Compliance Guidelines ### How to Ensure Specification Adherence 1. **Before Each Phase**: Complete the Pre-Implementation Specification Gate 2. **During Implementation**: Reference specific SDD sections in each task 3. **After Each Task**: Run Specification Compliance checks 4. **Phase Completion**: Verify all specification requirements are met ### Deviation Protocol If implementation cannot follow specification exactly: 1. Document the deviation and reason 2. Get approval before proceeding 3. Update SDD if the deviation is an improvement 4. Never deviate without documentation ## Metadata Reference - `[parallel: true]` - Tasks that can run concurrently - `[component: component-name]` - For multi-component features - `[ref: document/section; lines: 1, 2-3]` - Links to specifications, patterns, or interfaces and (if applicable) line(s) - `[activity: type]` - Activity hint for specialist agent selection --- ## Context Priming *GATE: You MUST fully read all files mentioned in this section before starting any implementation.* **Specification**: - `docs/specs/012-fivestars-modernization/product-requirements.md` - Product Requirements - `docs/specs/012-fivestars-modernization/solution-design.md` - Solution Design **Key Design Decisions** (from SDD Architecture Decisions): - **ADR-1**: Single FiveStarsApiClient shared between main app and FSRunner - **ADR-2**: Service Layer Architecture - Extract business logic to Services/, keep Controllers thin - **ADR-3**: Environment Variables for Credentials - All API credentials via $_ENV - **ADR-4**: Retry with Exponential Backoff - 3 retries with 1s, 2s, 4s delays **Implementation Context**: - Commands to run (all from `userfrosting/` directory): - Unit tests: `./test.sh --testsuite unit` - Integration tests: `./test.sh --testsuite integration` - Coverage: `./test.sh --coverage` - PHPStan: `cd userfrosting && ./vendor/bin/phpstan analyse src/BuyerKiosk/FiveStars/` - Patterns to follow: - `docs/patterns/psr4-autoloading.md` - PSR-4 namespace conventions - Chat system services: `userfrosting/src/BuyerKiosk/Chat/Services/` - Modern service pattern - Critical files to understand: - `userfrosting/src/BuyerKiosk/FiveStars/API.php` - Main app client (uses env vars) - `FSRunner/class/API.php` - FSRunner client (HARDCODED CREDENTIALS - DELETE) - `userfrosting/src/BuyerKiosk/FiveStars/Controllers/StoreController.php` - Business logic to extract - `userfrosting/src/BuyerKiosk/FiveStars/Controllers/RewardController.php` - Reward logic with caching **Environment Variables Required**: ```bash # Production FS_API_URL=https://api.fivestars.com/api/unified/ FS_API_KEY= FS_API_SECRET= # Development FS_API_URL_DEV=https://api.partnersandbox.fivestars.com/api/unified/ FS_API_KEY_DEV= FS_API_SECRET_DEV= # Shared REDIS_URL= LOG_DIR=/path/to/logs ``` --- ## Implementation Phases - [ ] **T1 Foundation: Exception Framework & Unified API Client** - [ ] T1.1 Prime Context - [ ] T1.1.1 Read Chat system service patterns `[ref: userfrosting/src/BuyerKiosk/Chat/Services/ChatBillingService.php]` - [ ] T1.1.2 Read existing FiveStars API implementation `[ref: userfrosting/src/BuyerKiosk/FiveStars/API.php]` - [ ] T1.1.3 Read error handling pattern from SDD `[ref: solution-design.md; lines: 917-941]` - [ ] T1.1.4 Read FiveStarsApiClient specification `[ref: solution-design.md; lines: 388-682]` - [ ] T1.2 Write Tests for Exception Framework `[activity: test-implementation]` - [ ] T1.2.1 Create `tests/Unit/FiveStars/ExceptionsTest.php` `[ref: SDD; lines: 917-941]` - [ ] T1.2.2 Test FiveStarsApiException with HTTP code preservation `[ref: PRD; lines: 186-200 edge case: API returns invalid JSON]` - [ ] T1.2.3 Test FiveStarsConfigException with clear error messages `[ref: PRD; lines: 93-94 acceptance: Missing credentials produce clear error]` - [ ] T1.2.4 Test FiveStarsValidationException for input validation `[ref: SDD; lines: 920-926]` - [ ] T1.3 Implement Exception Framework `[activity: backend-implementation]` - [ ] T1.3.1 Create `src/BuyerKiosk/FiveStars/Exceptions/FiveStarsException.php` (abstract base) - [ ] T1.3.2 Create `src/BuyerKiosk/FiveStars/Exceptions/FiveStarsApiException.php` - [ ] T1.3.3 Create `src/BuyerKiosk/FiveStars/Exceptions/FiveStarsConfigException.php` - [ ] T1.3.4 Create `src/BuyerKiosk/FiveStars/Exceptions/FiveStarsValidationException.php` - [ ] T1.4 Write Tests for FiveStarsApiClient `[activity: test-implementation]` - [ ] T1.4.1 Create `tests/Mocks/FiveStarsApiMock.php` for HTTP mocking - [ ] T1.4.2 Create `tests/Unit/FiveStars/Services/FiveStarsApiClientTest.php` - [ ] T1.4.3 Test successful API call without retry `[ref: PRD; lines: 186-200]` - [ ] T1.4.4 Test retry on 5xx transient errors (1s, 2s, 4s backoff) `[ref: PRD; lines: 123-130 Feature 5; SDD; lines: 945-959]` - [ ] T1.4.5 Test NO retry on 4xx permanent errors `[ref: PRD; line: 128; SDD; lines: 961-972]` - [ ] T1.4.6 Test max retries exhausted throws exception `[ref: PRD; lines: 197-198 edge case]` - [ ] T1.4.7 Test createFromEnvironment loads credentials for production `[ref: PRD; lines: 92-93; SDD; lines: 587-603]` - [ ] T1.4.8 Test createFromEnvironment throws on missing env vars `[ref: PRD; lines: 93-94 acceptance criteria]` - [ ] T1.4.9 Test phoneHash uses SHA1 algorithm `[ref: PRD; line: 193 business rule]` - [ ] T1.4.10 Test connection timeout triggers retry `[ref: PRD; lines: 132-137 Feature 6]` - [ ] T1.4.11 Test request timeout (30s) triggers retry `[ref: PRD; lines: 134-135 acceptance]` - [ ] T1.4.12 Test createFromEnvironment selects DEV credentials when isDev=true `[ref: SDD; lines: 588-592]` - [ ] T1.4.13 Test createFromEnvironment selects PROD credentials when isDev=false `[ref: SDD; lines: 593-596]` - [ ] T1.4.14 Test logging on retry attempts (verify no secrets logged) `[ref: PRD; line: 129 acceptance: All retry attempts are logged]` - [ ] T1.4.15 Test logging on API success (verify no secrets logged) `[ref: PRD; line: 192 business rule: All API calls MUST be logged]` - [ ] T1.4.16 Test logging on API failure (verify no secrets logged) - [ ] T1.5 Implement FiveStarsApiClient `[activity: backend-implementation]` - [ ] T1.5.1 Create `src/BuyerKiosk/FiveStars/Services/FiveStarsApiClient.php` `[ref: SDD; lines: 548-682]` - [ ] T1.5.2 Implement constructor with validation - [ ] T1.5.3 Implement `createFromEnvironment()` factory method with dev/prod selection - [ ] T1.5.4 Implement `call()` method with retry logic - [ ] T1.5.5 Implement `phoneHash()` method (SHA1) - [ ] T1.5.6 Implement timeout configuration (10s connect, 30s request) - [ ] T1.5.7 Implement logging with KLogger (LOG_DIR env var) - ensure no secrets in logs - [ ] T1.6 Validate Phase 1 `[activity: quality-assurance]` - [ ] T1.6.1 Run PHPStan: `cd userfrosting && ./vendor/bin/phpstan analyse src/BuyerKiosk/FiveStars/Exceptions/ src/BuyerKiosk/FiveStars/Services/FiveStarsApiClient.php` - [ ] T1.6.2 Run unit tests: `./test.sh --testsuite unit --filter FiveStars` - [ ] T1.6.3 Verify new/modified FiveStars classes have PHPDoc on public methods - [ ] T1.6.4 Verify new/modified FiveStars classes have type hints on parameters - [ ] **T2 Services Layer: Business Logic Extraction** `[ref: SDD; lines: 308-340]` - [ ] T2.1 PointsService `[parallel: true]` `[component: points-service]` - [ ] T2.1.1 Prime Context - [ ] Read StoreController points logic `[ref: userfrosting/src/BuyerKiosk/FiveStars/Controllers/StoreController.php]` - [ ] Read PointsService specification `[ref: SDD; lines: 445-476]` - [ ] T2.1.2 Write Tests `[activity: test-implementation]` - [ ] Create `tests/Unit/FiveStars/Services/PointsServiceTest.php` - [ ] Test calculatePointsForDay with tax included `[ref: SDD; lines: 814-838 algorithm]` - [ ] Test calculatePointsForDay with tax excluded `[ref: SDD; lines: 820-822]` - [ ] Test calculatePointsForDay with negative sales (CEIL behavior) `[ref: SDD; lines: 826-827]` - [ ] Test queuePointsForPosting creates fsOutgoing records `[ref: SDD; line: 471]` - [ ] Test getPointsReportByDateRange returns all fields `[ref: PRD Feature 4; lines: 111-120]` - [ ] Test database queries use prepared statements (security) `[ref: PRD; lines: 87-98 Feature 1 security]` - [ ] Test timezone handling uses store timezone `[ref: SDD; line: 1023 gotcha]` - [ ] Test logging on success/failure (no secrets leaked) `[ref: PRD; line: 192]` - [ ] T2.1.3 Implement `[activity: backend-implementation]` - [ ] Create `src/BuyerKiosk/FiveStars/Services/PointsService.php` - [ ] Implement constructor with full DI - [ ] Implement `createWithDefaults()` factory method - [ ] Extract `calculatePointsForDay()` from StoreController::getPointsForDay() - [ ] Extract `queuePointsForPosting()` from StoreController::getPostsForDay() - [ ] Extract `getPointsReportByDateRange()` from StoreController - [ ] Add KLogger for observability - [ ] T2.1.4 Validate (Automated Tests) - [ ] Create `tests/Unit/FiveStars/Services/PointsServiceValidationTest.php` - [ ] Test prepared statement usage (no SQL injection) - [ ] Test timezone conversion correctness - [ ] Run: `./test.sh --testsuite unit --filter PointsService` - [ ] T2.2 RewardsService `[parallel: true]` `[component: rewards-service]` - [ ] T2.2.1 Prime Context - [ ] Read RewardController reward logic `[ref: userfrosting/src/BuyerKiosk/FiveStars/Controllers/RewardController.php]` - [ ] Read RewardsService specification `[ref: SDD; lines: 478-514]` - [ ] T2.2.2 Write Tests `[activity: test-implementation]` - [ ] Create `tests/Unit/FiveStars/Services/RewardsServiceTest.php` - [ ] Test getAllRewards from cache (RedisMock) `[ref: SDD; lines: 502-507]` - [ ] Test getAllRewards from API on cache miss `[ref: SDD; lines: 505-507]` - [ ] Test getAllRewards sets 2-hour TTL `[ref: SDD; line: 507; PRD; line: 189]` - [ ] Test getPointsAndRewardsByPhone returns member info `[ref: SDD; line: 510]` - [ ] Test redeemRewardByPhone removes duplicate UIDs `[ref: SDD; line: 513]` - [ ] Test addMember creates new account `[ref: SDD; line: 514]` - [ ] Test getAllRewards handles API failure gracefully (returns cached or empty) `[ref: PRD; lines: 196-199 edge cases]` - [ ] Test getAllRewards handles Redis failure gracefully (falls back to API) `[ref: PRD; line: 197 edge case: Network timeout]` - [ ] Test redeemRewardByPhone handles API failure (logs error, throws) `[ref: PRD; line: 198]` - [ ] Test logging on API calls (no secrets leaked) `[ref: PRD; line: 192]` - [ ] T2.2.3 Implement `[activity: backend-implementation]` - [ ] Create `src/BuyerKiosk/FiveStars/Services/RewardsService.php` - [ ] Implement constructor with full DI (Store, PDO, ApiClient, Redis, Logger) - [ ] Implement `createWithDefaults()` factory method - [ ] Extract `getAllRewards()` with cache-first pattern - [ ] Extract `getAllPromotions()` (24hr cache) - [ ] Extract `getPointsAndRewardsByPhone()` - [ ] Extract `redeemRewardByPhone()` - [ ] Extract `addMember()` - [ ] Add KLogger for observability - [ ] T2.2.4 Validate (Automated Tests) - [ ] Create `tests/Unit/FiveStars/Services/RewardsServiceValidationTest.php` - [ ] Test Redis key pattern matches `{typeNum}_rewards` exactly - [ ] Test cache TTL: 2hr for rewards, 24hr for promotions - [ ] Run: `./test.sh --testsuite unit --filter RewardsService` - [ ] T2.3 SalesIngestionService `[parallel: true]` `[component: sales-service]` - [ ] T2.3.1 Prime Context - [ ] Read StoreController sales logic `[ref: userfrosting/src/BuyerKiosk/FiveStars/Controllers/StoreController.php]` - [ ] T2.3.2 Write Tests `[activity: test-implementation]` - [ ] Create `tests/Unit/FiveStars/Services/SalesIngestionServiceTest.php` - [ ] Test processSalesData inserts sale records `[ref: SDD; lines: 745-750]` - [ ] Test processSalesData skips null phone numbers ("9999999999") `[ref: SDD; line: 748]` - [ ] Test processSalesData returns count metrics `[ref: SDD; line: 749]` - [ ] Test processSalesData validates JSON payload `[ref: PRD; line: 197 edge case: API returns invalid JSON]` - [ ] Test checkSalesDayHash prevents duplicates `[ref: SDD; lines: 745-750]` - [ ] Test timezone handling uses store timezone `[ref: SDD; line: 1023]` - [ ] Test JSON errors produce FiveStarsValidationException `[ref: SDD; lines: 920-926]` - [ ] Test logging on ingestion (no secrets leaked) `[ref: PRD; line: 192]` - [ ] T2.3.3 Implement `[activity: backend-implementation]` - [ ] Create `src/BuyerKiosk/FiveStars/Services/SalesIngestionService.php` - [ ] Implement constructor with DI - [ ] Implement `createWithDefaults()` factory method - [ ] Extract `processSalesData()` from StoreController::receiveSalesData() - [ ] Extract `checkSalesDayHash()` and `insertSalesDayHash()` - [ ] Add KLogger for observability - [ ] T2.3.4 Validate (Automated Tests) - [ ] Create `tests/Unit/FiveStars/Services/SalesIngestionServiceValidationTest.php` - [ ] Test timezone conversion correctness - [ ] Test JSON error messages are clear and actionable - [ ] Run: `./test.sh --testsuite unit --filter SalesIngestionService` - [ ] T2.4 Services Integration Testing `[activity: test-implementation]` - [ ] T2.4.1 Create `tests/Integration/FiveStars/ServicesIntegrationTest.php` - [ ] T2.4.2 Test cross-service data flow `[ref: SDD; lines: 745-796 runtime view]` - [ ] T2.4.3 Test all services can be instantiated with factory methods - [ ] T2.4.4 Test existing fsOutgoing/dailySalesData records are processed correctly by new services (data integrity) - [ ] **T3 Controller Refactoring: Thin Controllers** `[ref: SDD; lines: 254-301]` - [ ] T3.1 StoreController Refactoring `[component: store-controller]` - [ ] T3.1.1 Prime Context - [ ] Review current StoreController implementation - [ ] Map methods to service delegation targets - [ ] T3.1.2 Write Tests `[activity: test-implementation]` - [ ] Create `tests/Unit/FiveStars/Controllers/StoreControllerTest.php` - [ ] Test getPointsForDay delegates to PointsService `[ref: SDD; lines: 254-261]` - [ ] Test getPostsForDay delegates to PointsService - [ ] Test getPointsReportByDateRange delegates to PointsService - [ ] Test receiveSalesData delegates to SalesIngestionService - [ ] T3.1.3 Implement `[activity: backend-implementation]` - [ ] Modify `Controllers/StoreController.php` - [ ] Add service property and injection - [ ] Replace direct logic with service delegation - [ ] Remove direct API class instantiation - [ ] Preserve public method signatures (backward compatibility) - [ ] T3.1.4 Validate (Automated Tests) - [ ] Create `tests/Integration/FiveStars/Controllers/StoreControllerBackwardCompatTest.php` - [ ] Test all existing response formats unchanged `[ref: SDD; lines: 378-384]` - [ ] Test HTTP status codes match original behavior - [ ] Run: `./test.sh --testsuite integration --filter StoreController` - [ ] T3.2 RewardController Refactoring `[component: reward-controller]` - [ ] T3.2.1 Prime Context - [ ] Review current RewardController implementation - [ ] Map methods to service delegation targets - [ ] T3.2.2 Write Tests `[activity: test-implementation]` - [ ] Create `tests/Unit/FiveStars/Controllers/RewardControllerTest.php` - [ ] Test getAllRewards delegates to RewardsService `[ref: SDD; lines: 254-261]` - [ ] Test getPointsAndRewardsByPhone delegates to RewardsService - [ ] Test redeemRewardByPhone delegates to RewardsService - [ ] Test addMember delegates to RewardsService - [ ] T3.2.3 Implement `[activity: backend-implementation]` - [ ] Modify `Controllers/RewardController.php` - [ ] Add RewardsService property and injection - [ ] Replace direct logic with service delegation - [ ] Remove direct API and Predis instantiation - [ ] Preserve public method signatures - [ ] T3.2.4 Validate (Automated Tests) - [ ] Create `tests/Integration/FiveStars/Controllers/RewardControllerBackwardCompatTest.php` - [ ] Test all existing response formats unchanged `[ref: SDD; lines: 378-384]` - [ ] Run: `./test.sh --testsuite integration --filter RewardController` - [ ] T3.3 Backward Compatibility Validation `[activity: quality-assurance]` - [ ] T3.3.1 Create `tests/Integration/FiveStars/ApiBackwardCompatibilityTest.php` - [ ] T3.3.2 Test rewards endpoint response format unchanged `[ref: SDD; lines: 378-384]` - [ ] T3.3.3 Test points report response format unchanged - [ ] T3.3.4 Test redeem reward error codes match original - [ ] **T4 FSRunner Integration: Shared Client** `[ref: SDD; lines: 328-336]` - [ ] T4.1 Update FSRunner to Use Shared Client `[component: fsrunner]` - [ ] T4.1.1 Prime Context - [ ] Read FSRunner daemon structure `[ref: FSRunner/FSRunner.php]` - [ ] Read FiveStarsGlobalController `[ref: FSRunner/controller/FiveStarsGlobalController.php]` - [ ] CRITICAL: Read hardcoded credentials in `FSRunner/class/API.php` (lines 7-13) - [ ] T4.1.2 Modify FSRunner Bootstrap `[activity: backend-implementation]` - [ ] Ensure Composer autoloader is loaded - [ ] Make FiveStarsApiClient available - [ ] T4.1.3 Refactor FiveStarsGlobalController `[activity: backend-implementation]` - [ ] Replace `new API($dev)` with FiveStarsApiClient - [ ] Add FiveStarsApiClient to constructor - [ ] Update `postPointsToFiveStarsAPI()` method - [ ] T4.1.4 Delete Old FSRunner API Class `[activity: cleanup]` - [ ] DELETE `FSRunner/class/API.php` (contains HARDCODED CREDENTIALS) - [ ] Document credential rotation requirement - [ ] T4.1.5 Validate - [ ] FSRunner starts without hardcoded credentials - [ ] Queue processing works with new client - [ ] T4.1.6 Rollback Plan `[activity: documentation]` - [ ] Document how to revert FSRunner to old API class if issues arise - [ ] Keep old `FSRunner/class/API.php` in git history (not deleted from repo history) - [ ] Create environment variable `USE_LEGACY_FIVESTARS_CLIENT=true` fallback option - [ ] T4.2 FSRunner Integration Tests `[activity: test-implementation]` - [ ] T4.2.1 Create `tests/Integration/FiveStars/FSRunnerIntegrationTest.php` - [ ] T4.2.2 Test FSRunner posts points using unified FiveStarsApiClient `[ref: SDD; lines: 328-336]` - [ ] T4.2.3 Test FSRunner respects retry logic from unified client `[ref: PRD; lines: 123-130]` - [ ] T4.2.4 Test FSRunner handles API errors correctly - [ ] T4.2.5 Test FSRunner uses correct environment (dev vs prod) based on Store.getDev() `[ref: SDD; line: 1022 gotcha]` - [ ] T4.2.6 Test repeated queue processing is idempotent (no duplicate posts) - [ ] T4.2.7 Test fsOutgoing record updates correctly on success/failure - [ ] **T5 Security Fixes: Protected Endpoints & Credential Removal** `[ref: PRD; lines: 105-110]` - [ ] T5.1 Protect Reports Endpoint `[component: security]` - [ ] T5.1.1 Prime Context - [ ] Read current routes `[ref: userfrosting/routes/groups/fivestars.php; lines: 5-22]` - [ ] Review validateAPIKey usage in other endpoints - [ ] Audit all FiveStars routes for authentication status - [ ] T5.1.2 Write Security Tests `[activity: test-implementation]` - [ ] Create `tests/Unit/FiveStars/Security/ReportsEndpointSecurityTest.php` - [ ] Test endpoint returns 401 without API key `[ref: PRD; lines: 107-108]` - [ ] Test endpoint returns 401 with invalid key - [ ] Test endpoint returns 200 with valid key `[ref: PRD; line: 109]` - [ ] T5.1.3 Implement `[activity: backend-implementation]` - [ ] Modify `routes/groups/fivestars.php` - [ ] Add validateAPIKey() call to `/reports/points/getByDateRange` - [ ] Return clear 401 error message - [ ] T5.1.4 Validate `[activity: security-review]` - [ ] Verify unauthorized requests blocked - [ ] Verify valid requests succeed - [ ] T5.1.5 Document Endpoint Security Status `[activity: documentation]` - [ ] Create audit of all FiveStars routes with authentication status - [ ] Note: Security gate "All endpoints authenticated" applies to reports endpoint only (other endpoints already secured) - [ ] T5.2 Credential Removal & Documentation `[component: security]` - [ ] T5.2.1 Verify All Credentials Externalized `[activity: security-review]` - [ ] Audit: No API credentials in source code - [ ] Audit: `userfrosting/src/BuyerKiosk/FiveStars/API.php` uses env vars (then DELETE) - [ ] Audit: `FSRunner/class/API.php` DELETED (was hardcoded) - [ ] T5.2.2 Delete Old API Classes `[activity: cleanup]` - [ ] DELETE `userfrosting/src/BuyerKiosk/FiveStars/API.php` - [ ] Verify no remaining references to old API class - [ ] T5.2.3 Document Credential Rotation Process `[activity: documentation]` - [ ] Create rotation procedure document - [ ] Document environment variable setup - [ ] CRITICAL: Old credentials must be rotated after deployment - [ ] **T6 Integration & End-to-End Validation** - [ ] T6.1 End-to-End Testing `[activity: test-implementation]` - [ ] T6.1.1 Create `tests/Integration/FiveStars/PointsFlowE2ETest.php` - [ ] Test complete points posting flow: Sales → Points → Queue → API `[ref: SDD; lines: 745-796]` - [ ] Test retry on transient API failure `[ref: PRD; lines: 123-130]` - [ ] Test points report accuracy with multiple statuses - [ ] Test existing fsOutgoing data is processed correctly (data integrity) - [ ] T6.1.2 Create `tests/Integration/FiveStars/RewardsFlowE2ETest.php` - [ ] Test complete redeem flow: Member lookup → Reward filter → Redemption `[ref: SDD; lines: 478-514]` - [ ] Test Redis caching behavior - [ ] Test duplicate redemption prevention - [ ] T6.1.3 Create `tests/Integration/FiveStars/FSRunnerE2ETest.php` - [ ] Test queue-to-posting flow `[ref: SDD; lines: 755-796]` - [ ] Test error recording in fsOutgoing - [ ] Test FSRunner uses unified FiveStarsApiClient with correct env config - [ ] Test repeated processing is idempotent - [ ] Test existing queue data processed correctly after migration - [ ] T6.2 Coverage & Quality Validation `[activity: quality-assurance]` - [ ] T6.2.1 Measure test coverage: `./test.sh --coverage` - [ ] Target: 80%+ code coverage for FiveStars module `[ref: PRD; line: 118]` - [ ] T6.2.2 Run static analysis: `cd userfrosting && ./vendor/bin/phpstan analyse src/BuyerKiosk/FiveStars/` - [ ] Target: No PHPStan errors - [ ] T6.2.3 Run full test suite: `./test.sh` - [ ] All unit tests pass - [ ] All integration tests pass - [ ] T6.3 Specification Compliance Verification `[activity: business-acceptance]` - [ ] T6.3.1 Verify Must Have Features (PRD Features 1-4) - [ ] Feature 1: Secure Credential Management `[ref: PRD; lines: 89-98]` - [ ] No API credentials in source code - [ ] All credentials from environment variables - [ ] Missing credentials produce clear error - [ ] Feature 2: Unified API Client `[ref: PRD; lines: 99-104]` - [ ] Single FiveStarsApiClient used by main app and FSRunner - [ ] Dev/production detection via config - [ ] Feature 3: Protected Reports Endpoint `[ref: PRD; lines: 105-110]` - [ ] Reports endpoint requires API key - [ ] Unauthorized returns 401 - [ ] Feature 4: Comprehensive Test Coverage `[ref: PRD; lines: 111-120]` - [ ] Unit tests for Points calculation - [ ] Unit tests for Reward redemption - [ ] 80%+ coverage for FiveStars module - [ ] T6.3.2 Verify Should Have Features (PRD Features 5-7) - [ ] Feature 5: Retry Logic `[ref: PRD; lines: 123-130]` - [ ] Feature 6: Request Timeouts `[ref: PRD; lines: 132-137]` - [ ] Feature 7: Modern Code Structure `[ref: PRD; lines: 139-146]` - [ ] T6.4 Performance Validation `[activity: performance-testing]` - [ ] T6.4.1 Create `tests/Performance/FiveStars/PerformanceTest.php` - [ ] Test API calls complete in <30s (measure with microtime) `[ref: SDD; lines: 998-999]` - [ ] Test reports return in <5s for 90-day range (measure with microtime) - [ ] T6.4.2 Create performance baseline script - [ ] Script to run N API calls and log timing - [ ] Output: avg, p50, p95, p99 response times - [ ] T6.4.3 Document monitoring approach for 99%+ success rate - [ ] Log analysis query for fsOutgoing success/failure ratio - [ ] Alert threshold: <99% success over 24hr window - [ ] T6.5 Documentation & Deployment `[activity: documentation]` - [ ] T6.5.1 Update API documentation - [ ] T6.5.2 Create deployment checklist - [ ] Pre-deployment: All tests passing, coverage verified - [ ] Deployment: Update environment variables with NEW credentials - [ ] Post-deployment: Rotate/revoke OLD FiveStars credentials - [ ] T6.5.3 Create rollback procedure - [ ] How to revert to old API classes if critical issues found - [ ] Environment variable toggle: `USE_LEGACY_FIVESTARS_CLIENT` - [ ] Git commands to restore deleted files if needed - [ ] T6.5.4 Verify all PRD requirements implemented `[ref: PRD; lines: 87-170]` - [ ] T6.5.5 Verify implementation follows SDD design `[ref: SDD; lines: 254-340]` --- ## Dependency Matrix ``` Phase 1: Foundation (Exceptions & API Client) ├─ T1.1-T1.3 Exception Framework (foundation) ├─ T1.4-T1.5 FiveStarsApiClient (depends on exceptions) └─ T1.6 Validation Phase 2: Services (depends on Phase 1) ├─ T2.1 PointsService [parallel: true] ├─ T2.2 RewardsService [parallel: true] ├─ T2.3 SalesIngestionService [parallel: true] └─ T2.4 Integration Tests Phase 3: Controller Refactoring (depends on Phase 2) ├─ T3.1 StoreController (depends on T2.1, T2.3) ├─ T3.2 RewardController (depends on T2.2) └─ T3.3 Backward Compatibility Tests Phase 4: FSRunner Integration (depends on Phase 1) ├─ T4.1 Update FSRunner (depends on T1.5) └─ T4.2 Integration Tests Phase 5: Security Fixes (depends on Phase 3, 4) ├─ T5.1 Protect Reports Endpoint └─ T5.2 Credential Removal Phase 6: Final Validation (depends on all phases) ├─ T6.1 E2E Testing ├─ T6.2 Coverage & Quality ├─ T6.3 Specification Compliance ├─ T6.4 Performance Tests └─ T6.5 Documentation & Deployment ``` --- ## Files Summary **Files to CREATE:** | File | Phase | Purpose | |------|-------|---------| | `src/BuyerKiosk/FiveStars/Exceptions/FiveStarsException.php` | T1.3 | Abstract base exception | | `src/BuyerKiosk/FiveStars/Exceptions/FiveStarsApiException.php` | T1.3 | API errors | | `src/BuyerKiosk/FiveStars/Exceptions/FiveStarsConfigException.php` | T1.3 | Configuration errors | | `src/BuyerKiosk/FiveStars/Exceptions/FiveStarsValidationException.php` | T1.3 | Input validation errors | | `src/BuyerKiosk/FiveStars/Services/FiveStarsApiClient.php` | T1.5 | Unified API client | | `src/BuyerKiosk/FiveStars/Services/PointsService.php` | T2.1 | Points business logic | | `src/BuyerKiosk/FiveStars/Services/RewardsService.php` | T2.2 | Rewards business logic | | `src/BuyerKiosk/FiveStars/Services/SalesIngestionService.php` | T2.3 | Sales processing | | `tests/Mocks/FiveStarsApiMock.php` | T1.4 | HTTP mocking for tests | | `tests/Unit/FiveStars/ExceptionsTest.php` | T1.2 | Exception tests | | `tests/Unit/FiveStars/Services/FiveStarsApiClientTest.php` | T1.4 | API client tests | | `tests/Unit/FiveStars/Services/PointsServiceTest.php` | T2.1 | Points service tests | | `tests/Unit/FiveStars/Services/RewardsServiceTest.php` | T2.2 | Rewards service tests | | `tests/Unit/FiveStars/Services/SalesIngestionServiceTest.php` | T2.3 | Sales service tests | | `tests/Performance/FiveStars/PerformanceTest.php` | T6.4 | Performance validation | **Files to MODIFY:** > **Note**: SDD mentions `Controllers/BaseController.php` modification. Service injection is performed directly in concrete controllers, making base class changes optional. | File | Phase | Changes | |------|-------|---------| | `Controllers/StoreController.php` | T3.1 | Delegate to services | | `Controllers/RewardController.php` | T3.2 | Delegate to services | | `routes/groups/fivestars.php` | T5.1 | Add API key validation | | `FSRunner/FSRunner.php` | T4.1 | Load autoloader | | `FSRunner/controller/FiveStarsGlobalController.php` | T4.1 | Use shared client | **Files to DELETE:** | File | Phase | Reason | |------|-------|--------| | `src/BuyerKiosk/FiveStars/API.php` | T5.2 | Replaced by FiveStarsApiClient | | `FSRunner/class/API.php` | T4.1 | CRITICAL: Contains hardcoded credentials | --- ## Success Criteria **Quality Gates:** - [ ] 80%+ test coverage for FiveStars module - [ ] PHPStan analysis passes with no errors - [ ] All unit tests pass - [ ] All integration tests pass - [ ] All E2E tests pass **Security Gates:** - [ ] No hardcoded credentials in source code - [ ] Reports endpoint authenticated (other FiveStars endpoints already secured) - [ ] Old credentials rotated after deployment - [ ] No secrets logged in any log output **Backward Compatibility:** - [ ] All API response formats unchanged - [ ] All existing endpoint URLs unchanged - [ ] All HTTP status codes match original - [ ] Existing fsOutgoing/dailySalesData records processed correctly **Performance:** - [ ] API calls complete in <30s (measured via automated tests) - [ ] Reports return in <5s for 90-day range (measured via automated tests) - [ ] 99%+ API success rate maintained (measured via log analysis) **Rollback Readiness:** - [ ] Rollback procedure documented - [ ] Legacy client fallback option available via environment variable - [ ] Git history preserves deleted files for emergency restoration