# 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 ### Known SDD Deviation **SDD specifies `src/BuyerKiosk/Feature/SystemAlerts/`** but the codebase convention uses flat namespaces under `src/BuyerKiosk/` (e.g., `Support/`, `Chat/`, `Workbook/`). This plan uses **`src/BuyerKiosk/SystemAlerts/`** to follow the established convention. Update SDD directory map accordingly. ## Metadata Reference - `[parallel: true]` - Tasks that can run concurrently - `[component: component-name]` - For multi-component features - `[ref: document/section; lines: X-Y]` - Links to specifications - `[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/040-system-alerts/product-requirements.md` - Product Requirements (8 Must Have features, 2 Should Have, 2 Could Have, 11 edge cases) - `docs/specs/040-system-alerts/solution-design.md` - Solution Design (6 ADRs, full API spec, DB schema, sequence diagrams) **Key Design Decisions** (from SDD ADRs): - **ADR-1**: All 3 tables in central DB `kiosk_buykiosk` (not per-store) - **ADR-2**: Dedicated Ably channels `alerts:global` and `alerts:{typeNum}` (not existing store channels) - **ADR-3**: Unified `NotificationManager` replaces direct toast rendering; chat delegates rendering but keeps its own logic - **ADR-4**: No separate analytics table — JOINs on `systemAlertAcknowledgments` are sufficient - **ADR-5**: Syncfusion RichTextEditor for alert body (init after modal shown) - **ADR-6**: Single `alerts:global` channel for all-stores alerts (not N publishes) **Implementation Context**: - **Commands**: - Run tests: `./test.sh --testsuite unit` - Targeted tests: `cd userfrosting && ./vendor/bin/phpunit --filter "SystemAlert"` - Run migrations: `php userfrosting/conductor run` - CSS build: `php userfrosting/conductor build-css --minify` - Static analysis: `cd userfrosting && ./vendor/bin/phpstan analyse` - **Patterns to Follow**: - Controller pattern: `userfrosting/src/BuyerKiosk/Chat/Controllers/ChatAdminController.php` (constructor injection, JSON responses) - Ably publisher: `userfrosting/src/BuyerKiosk/Chat/Events/ChatAblyPublisher.php` (publish pattern, error handling) - Rate limiting: `userfrosting/src/BuyerKiosk/Core/AblyPublishThrottle.php` (Redis-based, 40 msg/sec) - Route groups: `userfrosting/routes/support/api.php` (checkAccessAndReturnStoreObject pattern) - Migration JSON: `userfrosting/migrations/input/20260225_004_quote_requests.json` (create_table + check_query) - Permission migration: `userfrosting/migrations/input/20260224_039_004_workbook_manage_tasks_permission.json` - Admin page template: `userfrosting/templates/themes/default/admin/chat/usage.html` (head/footer includes, meta tags) - Chat notifications: `public_html/js/workspace/modules/chat/chat-notifications.js` (toast rendering, event system, sound, browser notifications) - Workspace header: `userfrosting/templates/themes/default/workspace/layouts/workspace-head.html` (header-actions div for bell icon) - **Interfaces to Implement**: - Admin API: 5 endpoints (List, Create, Update, Toggle, Ack Dashboard) `[ref: SDD; lines: 452-521]` - Staff API: 3 endpoints (Pending, Acknowledge, History) `[ref: SDD; lines: 525-547]` - Ably events: 3 event types (created, updated, deactivated) `[ref: SDD; lines: 597-636]` - Client modules: NotificationManager, BannerRenderer, ToastRenderer, AlertBellDropdown, AlertAblySync `[ref: SDD; lines: 386-402]` --- ## Risks & Mitigations | Risk | Impact | Mitigation | Phase | |------|--------|------------|-------| | Ably token auth missing `alerts:*` capability | Clients can't subscribe to alert channels | Update token generation in StaffChatApiController to include alert channel capabilities | T3 | | Syncfusion RTE init in hidden modal | `Cannot read properties of null` errors | Init RTE on `shown.bs.modal`, destroy on `hidden.bs.modal` | T5 | | Chat toast migration breaks existing behavior | Users lose chat notifications | Document baseline behaviors pre-migration, comprehensive regression test checklist | T7 | | Ably rate limit exhaustion (40 msg/sec shared) | Alert publish delays or drops | Low alert volume expected; AblyPublishThrottle handles gracefully; DB is source of truth | T2 | | MultiSelect init in hidden container | Null reference errors | Init only when modal tab is visible, destroy + recreate on each open | T5 | | PDO named param reuse in complex queries | `HY093: Invalid parameter number` silent failure | Use unique param names `:typeNum1`, `:typeNum2` and bind both | T2 | | Rich text content XSS | Stored XSS in alert body | Server-side HTML sanitization (strip script, iframe, event handlers); CTA URL validation | T2, T8 | | Multi-tab dedup race condition | Duplicate acknowledgment API calls | localStorage event listener + UNIQUE constraint on (alertId, userId) for idempotency | T4 | --- ## Phase Definition of Done (DoD) Checklist Each phase is complete when ALL of the following are satisfied: - [ ] All phase tasks marked complete - [ ] All tests pass (`./test.sh --testsuite unit` for backend phases) - [ ] PHPStan passes for new PHP classes (`cd userfrosting && ./vendor/bin/phpstan analyse src/BuyerKiosk/SystemAlerts/`) - [ ] CSS build succeeds for frontend phases (`php userfrosting/conductor build-css --minify`) - [ ] No console errors on affected pages - [ ] Code follows existing codebase patterns (controller pattern, naming conventions, etc.) - [ ] SDD compliance: every component matches its specification section - [ ] Manual smoke test of phase deliverables passes --- ## Implementation Phases ### Phase Dependencies ``` T1 (Database + Migrations) → T2 (Backend Services) → T3 (Backend API) ↓ T4 (Frontend NotificationManager) ← depends on → T3 (API available) T5 (Admin UI) ← depends on → T3 (Admin API available) T4 (NotificationManager) → T6 (Ably Integration + Bell) T4 (NotificationManager) → T7 (Chat Migration) T1-T7 → T8 (Integration & E2E Validation) ``` --- - [x] **T1 Phase 1: Database Foundation (Migrations + Permissions)** _(COMPLETED 2026-02-27)_ *Delivers: 3 new tables in central DB + permission hooks for admin access* - [x] T1.1 Prime Context - [x] T1.1.1 Read SDD data storage schema for all 3 tables (`systemAlerts`, `systemAlertStoreTargets`, `systemAlertAcknowledgments`) `[ref: SDD; lines: 408-448]` - [x] T1.1.2 Read SDD index definitions and constraints `[ref: SDD; lines: 424-448]` - [x] T1.1.3 Review existing migration patterns (quote_requests for table creation, support_portal for permissions) `[ref: userfrosting/migrations/input/20260225_004_quote_requests.json]` - [x] T1.1.4 Review permission group IDs (1=User, 2=SiteAdmin, 7=Manager, 8=Staff, 9=SuperAdmin) `[ref: userfrosting/migrations/input/20260224_039_004_workbook_manage_tasks_permission.json]` - [x] T1.2 Implement Migrations `[activity: data-architecture]` - [x] T1.2.1 Create migration file `userfrosting/migrations/input/040_001_system_alerts_tables.json` with 3 table creation operations: - `systemAlerts` table with all columns, ENUMs, indexes per SDD `[ref: SDD; lines: 409-426]` - `systemAlertStoreTargets` table with FK to systemAlerts, unique constraint `[ref: SDD; lines: 428-436]` - `systemAlertAcknowledgments` table with FK, unique constraint on (alertId, userId) `[ref: SDD; lines: 439-448]` - Each operation includes `check_query` using `information_schema.tables` pattern - All tables use `ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci` - [x] T1.2.2 ~~Create migration file `userfrosting/migrations/input/040_002_system_alerts_permissions.json`~~ — NOT NEEDED: `uri_bkadmin` already exists in codebase (used by Billing controllers and sidebar menu) - [x] T1.3 Validate `[activity: run-tests]` - [x] T1.3.1 Run migrations: `php userfrosting/conductor run` and verify all 3 tables created `[activity: run-tests]` - [x] T1.3.2 Verify table schemas match SDD column definitions (check column types, nullability, defaults, indexes) `[activity: review-code]` - [x] T1.3.3 Verify foreign key constraints work (insert test row, attempt invalid FK) `[activity: run-tests]` - [x] T1.3.4 Verify permission hooks exist in `uf_authorize_group` `[activity: review-code]` **Phase T1 Review Summary (Codex Review 2026-02-27)** _Codex Findings:_ - **Critical (fixed):** `createdBy`/`updatedBy`/`userId` were signed `int` but `users.id` is `int unsigned` — type mismatch would block future FK additions. Fixed via `040_002_system_alerts_column_fixes.json`. - **Important (fixed):** Redundant `idx_alert_user` index alongside `uq_alert_user` on acknowledgments table — unnecessary write overhead. Dropped via same patch migration. - **Nice-to-have (skipped):** `check_query` only verifies table existence, not full schema — known migration system limitation. - **Nice-to-have (skipped):** No FK from `createdBy`/`userId` to `users.id` — cross-database FKs (`kiosk_buykiosk` → `kiosk_users`) not supported in MySQL. App-level enforcement planned for T2. - **Nice-to-have (deferred):** `ctaUrl` app-level validation — already planned for T2 service layer. _Changes Made:_ - Updated `040_001_system_alerts_tables.json` to use `int unsigned` for `createdBy`, `updatedBy`, `userId` (for fresh deployments) - Created `040_002_system_alerts_column_fixes.json` with 2 ALTER TABLE operations (for dev env where tables already existed) - Removed redundant `idx_alert_user` index from acknowledgments table --- - [x] **T2 Phase 2: Backend Domain Layer (Models + Repositories + Services)** *(Completed 2026-02-27)* *Delivers: PHP classes for alert CRUD, acknowledgment tracking, and Ably publishing* - [x] T2.1 Entity Models `[component: backend-models]` *(Must complete before T2.2 and T2.3 — repos and publisher depend on model classes)* - [x] T2.1.1 Prime: Read SDD application data models `[ref: SDD; lines: 549-593]` - [x] T2.1.2 Write Tests: Unit tests for `SystemAlert` entity `[activity: write-unit-tests]` - Test `isExpired()` with NULL expiry (not expired), future expiry (not expired), past expiry (expired) `[ref: PRD Feature 6; line: 132]` - Test `isTargetedTo(typeNum)` for `targetType='all'` and `targetType='specific'` `[ref: PRD Feature 2; lines: 92-93]` - Test `toArray()` serialization includes all fields `[ref: SDD; lines: 551-573]` - Test `toAblyPayload()` returns minimal payload (alertId, title, body, severity, displayType, ctaLabel, ctaUrl, createdAt) `[ref: SDD; line: 608]` - [x] T2.1.3 Implement: Create `userfrosting/src/BuyerKiosk/SystemAlerts/Models/SystemAlert.php` `[activity: domain-modeling]` - Namespace: `BuyerKiosk\SystemAlerts\Models` - Constructor accepts associative array (hydration from DB row) - All properties with getters - `isExpired()`, `isTargetedTo()`, `toArray()`, `toAblyPayload()` methods - [x] T2.1.4 Implement: Create `userfrosting/src/BuyerKiosk/SystemAlerts/Models/AlertAcknowledgment.php` `[activity: domain-modeling]` - Simple DTO: id, alertId, userId, dismissedAt - [x] T2.1.5 Implement: Create `userfrosting/src/BuyerKiosk/SystemAlerts/Models/PendingAlert.php` `[activity: domain-modeling]` - DTO for staff API responses `[ref: SDD; lines: 583-593]` - Fields: id, title, body, severity, displayType, ctaLabel, ctaUrl, createdAt - `toArray()` method for JSON serialization - Static `fromRow(array $row): PendingAlert` factory method - [x] T2.1.6 Validate: Run unit tests for models `[activity: run-tests]` - [x] T2.2 Repositories `[parallel: true]` `[component: backend-repos]` - [x] T2.2.1 Prime: Read SDD repository interfaces and query algorithms `[ref: SDD; lines: 1008-1069]` - [x] T2.2.2 Write Tests: Unit tests for `SystemAlertRepository` `[activity: write-unit-tests]` - Test `create()` inserts alert + store targets (if specific) `[ref: SDD; lines: 900-914]` - Test `update()` updates alert fields `[ref: SDD; lines: 497-501]` - Test `findById()` returns SystemAlert with joined targetStores - Test `findActive()` filters by isActive=1 and non-expired - Test `findPendingForUser()` implements targeting resolution algorithm `[ref: SDD; lines: 1008-1038]` - Test `toggleActive()` flips isActive flag `[ref: SDD; lines: 503-507]` - Test `listForAdmin()` with filters (status, severity, store, date range, sort) `[ref: SDD; lines: 456-476]` - [x] T2.2.3 Write Tests: Unit tests for `AcknowledgmentRepository` `[activity: write-unit-tests]` - Test `create()` inserts acknowledgment record - Test `create()` with duplicate (alertId, userId) is idempotent `[ref: SDD; line: 1002]` - Test `deleteAllForAlert()` removes all acks for an alertId `[ref: PRD Feature 5; line: 122]` - Test `getStatsForAlert()` returns targeted/acknowledged/pending counts `[ref: SDD; lines: 1041-1069]` - Test `getAcknowledgedUsers()` returns user details with timestamps - Test `getPendingUsers()` returns users who haven't acknowledged - Test `hasUserAcknowledged()` returns bool - [x] T2.2.4 Implement: Create `userfrosting/src/BuyerKiosk/SystemAlerts/Repositories/SystemAlertRepository.php` `[activity: data-architecture]` - Constructor: `PDO $centralDb` (kiosk_buykiosk connection) - All methods use PDO prepared statements - `findPendingForUser(int $userId, array $userStoreTypeNums)` implements targeting resolution `[ref: SDD; lines: 1008-1038]` - Remember: PDO named params cannot be reused — use `:typeNum1`, `:typeNum2` pattern - [x] T2.2.5 Implement: Create `userfrosting/src/BuyerKiosk/SystemAlerts/Repositories/AcknowledgmentRepository.php` `[activity: data-architecture]` - Constructor: `PDO $centralDb`, `PDO $usersDb` (kiosk_users for JOIN to users table) - `create()` catches duplicate key exception for idempotency - `getStatsForAlert()` implements dashboard algorithm `[ref: SDD; lines: 1041-1069]` - [x] T2.2.6 Validate: Run repository unit tests `[activity: run-tests]` - [x] T2.3 Ably Publisher `[parallel: true]` `[component: backend-ably]` - [x] T2.3.1 Prime: Read ChatAblyPublisher pattern and AblyPublishThrottle `[ref: userfrosting/src/BuyerKiosk/Chat/Events/ChatAblyPublisher.php]` `[ref: userfrosting/src/BuyerKiosk/Core/AblyPublishThrottle.php]` - [x] T2.3.2 Prime: Read SDD Ably integration spec `[ref: SDD; lines: 597-636]` - [x] T2.3.3 Write Tests: Unit tests for `AlertAblyPublisher` `[activity: write-unit-tests]` - Test `publishCreated()` publishes to `alerts:global` when targetType='all' `[ref: SDD; lines: 909-911]` - Test `publishCreated()` publishes to each `alerts:{typeNum}` when targetType='specific' `[ref: SDD; lines: 912-914]` - Test `publishUpdated()` publishes `alert:updated` event `[ref: SDD; line: 605]` - Test `publishDeactivated()` publishes `alert:deactivated` with minimal payload (alertId only) `[ref: SDD; line: 609]` - Test rate limiting via AblyPublishThrottle (mock Redis) - Test graceful failure when Ably unavailable (no exception thrown) `[ref: SDD; line: 1000]` - [x] T2.3.4 Implement: Create `userfrosting/src/BuyerKiosk/SystemAlerts/Events/AlertAblyPublisher.php` `[activity: backend-implementation]` - Follow ChatAblyPublisher pattern: constructor takes ABLY_KEY, creates AblyRest client - Inject AblyPublishThrottle for rate limiting - `publishCreated(SystemAlert $alert)`: publish to global or per-store channels - `publishUpdated(SystemAlert $alert)`: same channel logic as created - `publishDeactivated(int $alertId, SystemAlert $alert)`: publish alertId-only payload - All methods are void, wrapped in try-catch with error_log - [x] T2.3.5 Validate: Run publisher unit tests `[activity: run-tests]` - [x] T2.4 Alert Service `[component: backend-service]` - [x] T2.4.1 Prime: Read SDD runtime flows and business rules `[ref: SDD; lines: 875-993]` - [x] T2.4.2 Write Tests: Unit tests for `SystemAlertService` `[activity: write-unit-tests]` - Test `createAlert()` validates input, creates record, publishes via Ably `[ref: SDD Scenario 1; lines: 1242-1258]` - Test `createAlert()` with targetType='specific' creates store target records `[ref: SDD; lines: 903-905]` - Test `updateAlert()` updates record, deletes all acknowledgments, re-publishes `[ref: SDD Scenario 4; lines: 1287-1295]` `[ref: PRD Feature 5; line: 122]` - Test `toggleActive()` flips isActive, publishes deactivated event `[ref: PRD Feature 6; lines: 131-133]` - Test `getPendingForUser()` calls repository with correct params `[ref: SDD; lines: 1008-1038]` - Test `acknowledgeAlert()` creates ack record `[ref: PRD Feature 5; lines: 118-121]` - Test `getAlertHistory()` returns alerts with ack status for user `[ref: SDD; lines: 539-547]` - Test `getAcknowledgmentDashboard()` returns full stats `[ref: SDD; lines: 509-521]` - Test input validation: title max 255, required fields, CTA URL validation (https or relative only, no javascript/data/ftp) `[ref: SDD; lines: 1197]` - Test HTML sanitization of body field (strip script, iframe, event handlers) `[ref: SDD; line: 1196]` - [x] T2.4.3 Implement: Create `userfrosting/src/BuyerKiosk/SystemAlerts/Services/SystemAlertService.php` `[activity: backend-implementation]` - Constructor: `SystemAlertRepository`, `AcknowledgmentRepository`, `AlertAblyPublisher` - `createAlert(array $data, int $userId): SystemAlert` - `updateAlert(int $alertId, array $data, int $userId): SystemAlert` - `toggleActive(int $alertId, int $userId): SystemAlert` - `getPendingForUser(int $userId): array` - `acknowledgeAlert(int $alertId, int $userId): void` - `getAlertHistory(int $userId, int $limit): array` - `getAcknowledgmentDashboard(int $alertId): array` - `listForAdmin(array $filters): array` - Private: `validateInput()`, `sanitizeHtml()`, `validateCtaUrl()` - [x] T2.4.4 Decision: Ack logic kept in SystemAlertService (no separate AcknowledgmentService needed) `[activity: backend-implementation]` - [x] T2.4.5 Validate: Run all backend unit tests `[activity: run-tests]` - Command: `cd userfrosting && ./vendor/bin/phpunit --filter "SystemAlert"` - Result: 90 tests, 425 assertions, all passing - [x] T2.4.6 Validate: Run PHPStan analysis on new classes `[activity: lint-code]` - Command: `cd userfrosting && ./vendor/bin/phpstan analyse src/BuyerKiosk/SystemAlerts/` - Result: 0 errors **T2 Phase Review (Codex, 2026-02-27)** Findings and resolutions: | # | Finding | Severity | Resolution | |---|---------|----------|------------| | 1 | SQL injection via `sortBy`/`sortDir` in `listForAdmin()` | Critical | Fixed: added `ALLOWED_SORT_COLUMNS` whitelist and sortDir sanitization | | 2 | Target-wipe on update when `targetStores` sent without `targetType` | Critical | Fixed: service passes `_existingTargetType` to repo; repo falls back to it | | 3 | CTA URL allows `http://` but SDD says HTTPS-only | Important | Fixed: removed `http` from `ALLOWED_URL_SCHEMES`, updated test | | 4 | `getAlertHistory()` missing `dismissedAt` and uses `hasUserAcknowledged` | Important | Fixed: added `getDismissedAt()` to AcknowledgmentRepository, updated service | | 5 | Cross-DB query in `getTotalTargetedUsers()` via central connection | Important | Fixed: injected `$usersDb` into `SystemAlertRepository` constructor | | 6 | HTML sanitization missing `vbscript:` and `file:` in body links | Nice-to-have | Fixed: consolidated regex to block javascript/data/vbscript/file protocols | | 7 | Missing `listForAdmin()` test | Important | Deferred to T3 (controller integration tests will exercise this path) | Post-review validation: **97 tests, 449 assertions, PHPStan 0 errors** --- - [x] **T3 Phase 3: Backend API Layer (Controllers + Routes)** *(Completed 2026-02-27)* *Delivers: Admin API (5 endpoints) + Staff API (3 endpoints) with permission gating* - [x] T3.1 Admin Controller + Routes `[parallel: true]` `[component: backend-admin-api]` - [x] T3.1.1 Prime: Read SDD admin endpoint specs `[ref: SDD; lines: 452-521]` - [x] T3.1.2 Prime: Read route group pattern `[ref: userfrosting/routes/support/api.php]` - [x] T3.1.3 Prime: Read controller pattern `[ref: userfrosting/src/BuyerKiosk/Chat/Controllers/ChatAdminController.php]` - [x] T3.1.4 Write Tests: Unit tests for `SystemAlertsAdminController` `[activity: write-unit-tests]` - Test `listAlerts()` returns paginated alerts with ack progress and summary stats `[ref: SDD; lines: 456-476]` - Test `createAlert()` validates input and returns created alert `[ref: SDD; lines: 478-493]` - Test `updateAlert()` returns updated alert with ack reset side effect `[ref: SDD; lines: 495-501]` - Test `toggleAlert()` returns toggled alert `[ref: SDD; lines: 503-507]` - Test `getAcknowledgments()` returns dashboard stats `[ref: SDD; lines: 509-521]` - Test 403 when user lacks `uri_bkadmin` permission `[ref: PRD Feature 1; line: 81]` — handled at route level - Test 400 for invalid input (missing title, invalid severity, bad CTA URL) `[ref: SDD; lines: 997-998]` - Test 404 for non-existent alertId `[ref: SDD; line: 999]` - [x] T3.1.5 Implement: Create `userfrosting/src/BuyerKiosk/SystemAlerts/Controllers/SystemAlertsAdminController.php` `[activity: api-development]` - Constructor: `$app`, injects `SystemAlertService` (DI for testability) - JSON responses via `$this->jsonResponse()` / `$this->jsonError()` helpers - Methods: `listAlerts()`, `createAlert()`, `updateAlert($alertId)`, `toggleAlert($alertId)`, `getAcknowledgments($alertId)` - [x] T3.1.6 Implement: Create `userfrosting/routes/system-alerts/admin.php` `[activity: api-development]` - Admin page route: `GET /admin/system-alerts/` (renders admin page template) - API routes grouped under `/api/system-alerts/admin/`: - `GET /api/system-alerts/admin/alerts` → listAlerts - `POST /api/system-alerts/admin/alerts` → createAlert - `PUT /api/system-alerts/admin/alerts/:alertId` → updateAlert - `PUT /api/system-alerts/admin/alerts/:alertId/toggle` → toggleAlert - `GET /api/system-alerts/admin/alerts/:alertId/acknowledgments` → getAcknowledgments - Permission check: `uri_bkadmin` via `$app->user->checkAccess('uri_bkadmin')` at route level - `buildSystemAlertService()` helper constructs full dependency chain - [x] T3.1.7 Validate: Run admin controller tests `[activity: run-tests]` - Result: 13 tests passing - [x] T3.2 Staff Controller + Routes `[parallel: true]` `[component: backend-staff-api]` - [x] T3.2.1 Prime: Read SDD staff endpoint specs `[ref: SDD; lines: 525-547]` - [x] T3.2.2 Write Tests: Unit tests for `SystemAlertsStaffController` `[activity: write-unit-tests]` - Test `getPending()` returns unacknowledged alerts for user's stores `[ref: SDD; lines: 527-533]` - Test `acknowledge()` records acknowledgment and returns success `[ref: SDD; lines: 535-537]` - Test `getHistory()` returns alert history with ack status `[ref: SDD; lines: 539-547]` - Test user with no store assignments gets empty pending list - Test duplicate acknowledgment returns success (idempotent) `[ref: SDD; line: 1002]` - [x] T3.2.3 Implement: Create `userfrosting/src/BuyerKiosk/SystemAlerts/Controllers/SystemAlertsStaffController.php` `[activity: api-development]` - Constructor: `$app`, `SystemAlertService`, `array $userStoreTypeNums` - Gets userId from `$app->user->id` - Store assignments passed in from route builder (queried from `userStoreAssignments`) - Methods: `getPending()`, `acknowledge($alertId)`, `getHistory()` - [x] T3.2.4 Implement: Create `userfrosting/routes/system-alerts/api.php` `[activity: api-development]` - Staff API routes (session auth required, no specific permission — all logged-in users): - `GET /api/system-alerts/pending` → getPending - `POST /api/system-alerts/:alertId/acknowledge` → acknowledge - `GET /api/system-alerts/history` → getHistory - `getUserStoreTypeNums()` helper queries active store assignments - `buildStaffController()` wires up full dependency chain - [x] T3.2.5 Validate: Run staff controller tests `[activity: run-tests]` - Result: 9 tests passing - [x] T3.3 Ably Token Auth Update `[component: backend-ably-auth]` `[activity: backend-implementation]` - [x] T3.3.1 Modify `userfrosting/src/BuyerKiosk/StaffChat/Controllers/StaffChatApiController.php` - Added `alerts:global` with `['subscribe']` capability for ALL users - Queries `kiosk_users.userStoreAssignments` for ALL assigned typeNums - Adds `alerts:{typeNum}` with `['subscribe']` for each assigned store - Wrapped in try-catch — falls back to global-only on error - [x] T3.3.2 ~~Write Tests~~ — Deferred: StaffChatApiController has complex constructor dependencies making unit testing of getAblyToken impractical. Alert capabilities tested via integration in T8. - [x] T3.4 Register Routes `[component: backend-routes]` - [x] T3.4.1 Register both route files in `public_html/index.php` - `include("../userfrosting/routes/system-alerts/admin.php");` - `include("../userfrosting/routes/system-alerts/api.php");` - [x] T3.4.2 Validate: PHPStan passes on all SystemAlerts classes — 0 errors - [x] T3.4.3 Validate: Run full unit test suite `[activity: run-tests]` - Result: 119 SystemAlert tests, 547 assertions, all passing - Full suite: 8085 tests, only pre-existing failures (WhiteboardManager, KPI, TaskComment) **T3 Phase Summary (2026-02-27)** _Files Created:_ - `userfrosting/src/BuyerKiosk/SystemAlerts/Controllers/SystemAlertsAdminController.php` — Admin API (5 endpoints) - `userfrosting/src/BuyerKiosk/SystemAlerts/Controllers/SystemAlertsStaffController.php` — Staff API (3 endpoints) - `userfrosting/routes/system-alerts/admin.php` — Admin page + API routes with CSRF - `userfrosting/routes/system-alerts/api.php` — Staff API routes - `userfrosting/tests/Unit/SystemAlerts/Controllers/SystemAlertsAdminControllerTest.php` — 13 tests - `userfrosting/tests/Unit/SystemAlerts/Controllers/SystemAlertsStaffControllerTest.php` — 9 tests _Files Modified:_ - `userfrosting/src/BuyerKiosk/StaffChat/Controllers/StaffChatApiController.php` — Ably token alert capabilities - `public_html/index.php` — Route registration _Deferred:_ - T3.3.2 Ably token unit tests deferred to T8 integration testing (constructor DI complexity) **T3 Phase Review (Codex, 2026-02-27)** _Codex Findings:_ | # | Finding | Severity | Resolution | |---|---------|----------|------------| | 1 | Admin controller missing `jsonResponse`/`jsonError` | Critical | **False positive** — methods exist at lines 238-264; Codex missed them | | 2 | `->conditions()` not supported in Slim 2 | High | **Incorrect** — `->conditions()` is standard Slim 2, used throughout codebase (e.g., `support/api.php` lines 29, 44, 89) | | 3 | Missing CSRF on POST/PUT endpoints | High (Security) | **Fixed**: Added `checkSystemAlertsCsrf()` helper using `NoCSRF` pattern from `premium.php`. Applied to admin create/update/toggle and staff acknowledge | | 4 | Admin API routes missing auth check | Medium | **Fixed**: Added `checkSystemAlertsAdminAccess()` helper with `isset($app->user)` + `uri_bkadmin` check | | 5 | `sortBy`/`sortDir` injection risk | Medium | **Already handled** — `SystemAlertRepository` has `ALLOWED_SORT_COLUMNS` whitelist (fixed in T2 review) | | 6 | Dependency builder duplication | Low | **Accepted** — `function_exists('buildSystemAlertService')` guard ensures no conflict; minor duplication acceptable for route isolation | _Changes Made:_ - Added `checkSystemAlertsAdminAccess()` auth guard to all admin API routes - Added `checkSystemAlertsCsrf()` CSRF validation to all mutating routes (POST/PUT) - CSRF token generation uses `\NoCSRF::generate('csrf_token')` on admin page (not raw `$_SESSION`) - Staff acknowledge endpoint also CSRF-protected (via `function_exists` guard for loading order) Post-review validation: **119 tests, 547 assertions, PHPStan 0 errors** --- - [x] **T4 Phase 4: Frontend Notification Framework (NotificationManager + Renderers)** ✅ COMPLETE *Delivers: Unified notification rendering engine with queue management, banner renderer, and toast renderer* ### T4 Review Summary - **Files created**: NotificationManager.js (763 lines), BannerRenderer.js (249 lines), ToastRenderer.js (287 lines), notifications.css (380+ lines) - **CSS linked** in workspace-head.html with cache-busting version parameter - **Live browser validation**: All 3 modules load without errors, banner renders above header pushing content down, toasts stack bottom-right with correct severity colors - **Severity colors**: blue (info), amber (warning), rose (critical), purple (chat) — all using design tokens - **Accessibility**: role="alert", aria-live, aria-labels, Escape key dismiss, focus-visible outlines, tabindex - **Multi-tab sync**: localStorage-based acknowledged IDs with storage event listener - **CSS build**: `conductor build-css --minify` passes (334.91 KB / 500 KB limit) - **No new console errors** — all errors on workspace page are pre-existing (Ably close, Chrome extension) - **Admin page CSS**: Will be included directly in admin system-alerts template (T5), not via conductor bundle - [x] T4.1 Prime Context - [x] T4.1.1 Read SDD NotificationManager queue algorithm `[ref: SDD; lines: 646-714]` - [x] T4.1.2 Read SDD BannerRenderer positioning spec `[ref: SDD; lines: 1189-1192, 1232]` - [x] T4.1.3 Read SDD ToastRenderer positioning spec `[ref: SDD; lines: 1192, 1232]` - [x] T4.1.4 Read SDD multi-tab dedup via localStorage `[ref: SDD; lines: 1141-1147]` - [x] T4.1.5 Read existing chat-notifications.js event system `[ref: public_html/js/workspace/modules/chat/chat-notifications.js]` - [x] T4.2 Implement: Create NotificationManager `[component: frontend-core]` `[activity: frontend-implementation]` - [x] T4.2.1 Create `public_html/js/workspace/modules/notifications/NotificationManager.js` - IIFE module pattern (matching existing codebase style) - Private state: `_visibleItems[]`, `_queue[]`, `_acknowledgedIds` Set - Constants: `MAX_VISIBLE = 3`, `PRIORITY = { critical: 3, warning: 2, info: 1, chat: 0 }` - Public API: - `init()` — load acknowledged IDs from localStorage - `enqueue(notification)` — add to priority queue, deduplicate, process queue `[ref: SDD; lines: 658-685]` - `dismiss(notificationId, alertId)` — remove from visible, send ack API call, localStorage sync, process queue `[ref: SDD; lines: 696-712]` - `remove(notificationId)` — remove from visible/queue without ack (for deactivated alerts) - `getQueueSize()` — return pending count - `getPendingCount()` — return unacknowledged system alert count (for bell badge) - Rendering delegation: calls BannerRenderer or ToastRenderer based on `displayType` - Acknowledgment API call: `POST /api/system-alerts/{alertId}/acknowledge` with CSRF token - Multi-tab sync: `localStorage.setItem('alert-dismissed-' + alertId, Date.now())` + `window.addEventListener('storage', ...)` `[ref: SDD; lines: 1141-1147]` - Dedup: skip enqueue if alertId already in `_acknowledgedIds` or already visible/queued `[ref: SDD; lines: 659-663]` - [x] T4.3 Implement: Create BannerRenderer `[component: frontend-banner]` `[activity: frontend-implementation]` - [x] T4.3.1 Create `public_html/js/workspace/modules/notifications/BannerRenderer.js` - Renders banner above page header (pushes content down, no overlay) `[ref: PRD Feature 3; lines: 100-105]` - Severity color-coding: blue (info), yellow (warning), red (critical) `[ref: PRD Feature 3; line: 101]` - Shows: title, rich text body, optional CTA button, dismiss button `[ref: PRD Feature 3; line: 102]` - z-index: 1031 (above header's 1030) `[ref: SDD; line: 1232]` - Slide-in animation on show, slide-out on dismiss - CTA button: `` tag with sanitized href, opens in new tab - Dismiss button fires callback to NotificationManager - [x] T4.4 Implement: Create ToastRenderer `[component: frontend-toast]` `[activity: frontend-implementation]` - [x] T4.4.1 Create `public_html/js/workspace/modules/notifications/ToastRenderer.js` - Fixed position: bottom-right (`bottom: 80px; right: 20px`) matching chat toast location `[ref: SDD; line: 1232]` `[ref: PRD Feature 4; line: 110]` - Severity color-coding (same as banner) - Shows: severity indicator, title, message preview, optional CTA, close button `[ref: PRD Feature 4; line: 111]` - All toasts persist until manually closed (no auto-dismiss) `[ref: PRD Feature 4; line: 112]` - Stack vertically with 10px gap `[ref: PRD Feature 4; line: 115]` - z-index: 1090 `[ref: SDD; line: 1232]` - Toast for chat type: distinct color/icon, onClick callback for thread opening `[ref: PRD Feature 8; line: 150]` - Close button fires callback to NotificationManager - [x] T4.5 Implement: Create notification CSS `[component: frontend-styles]` `[activity: frontend-implementation]` - [x] T4.5.1 Create `public_html/css/workspace/notifications.css` - Banner styles: full-width, severity colors, animations, dismiss button, CTA button - Toast styles: card-like, severity colors, stacking, fixed position, dismiss button - Use CSS variables from `tokens.css` where applicable `[ref: public_html/css/admin/tokens.css]` - Responsive: banners and toasts must work on mobile viewports - [x] T4.6 Accessibility `[component: frontend-a11y]` `[activity: frontend-implementation]` - [x] T4.6.1 Banner: Add `role="alert"` and `aria-live="assertive"` for critical, `aria-live="polite"` for info/warning - [x] T4.6.2 Banner + Toast dismiss buttons: Add `aria-label="Dismiss alert: {title}"` and ensure focusable (`