diff --git a/CLAUDE.md b/CLAUDE.md index 8ba9c60..8bb635f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -2,7 +2,7 @@ > **Audience:** AI assistants and developers working on this codebase. > **Purpose:** Authoritative reference for architecture, conventions, gotchas, and decisions. -> **Last reviewed:** May 2026 (Phase B complete — iPad offline inspection app; Critical/Notable hardening pass) +> **Last reviewed:** May 2026 (Phase C complete — iPad notification polling, assigned-issue sync, issue photo display, follow-up notifications) --- @@ -44,7 +44,7 @@ - **Reports** — on-demand PDF/CSV scorecards and scheduled email digests - **Audit trail** — immutable log of every create/update/delete action - **Mobile API** — JWT-authenticated REST layer for the iPad native app -- **iPad native app** — SwiftUI + SwiftData offline-first inspection tool (Phase A + B complete) +- **iPad native app** — SwiftUI + SwiftData offline-first inspection tool (Phase A + B + C complete) The application is actively deployed in production and maintained by a single developer/administrator. @@ -248,10 +248,13 @@ issues: id, inspection_id (nullable), area_id, severity (low/medium/high/critica ### Notification / NotificationPreference ``` -notifications: id, user_id, title, body, link, is_read, created_at, issue_id, event_type +notifications: id, user_id, title, body, link, is_read, created_at, issue_id, + inspection_id, event_type VARCHAR(50) NULL, digest_pending notification_preferences: id, user_id, event_type, email_enabled, digest_mode, digest_frequency ``` +**`event_type`:** Added in phase17. Stored by `notify()` and returned by `GET /api/v1/notifications` so the iPad can categorise alerts. `NULL` for notifications created before the migration. + ### NotificationMatrix ``` @@ -328,9 +331,10 @@ api_device_tokens: id, user_id, device_id, apns_token, device_name, app_version | `api_auth` | `/api/v1` | `/auth/login`, `/auth/refresh`, `/auth/logout`, `/auth/me`, `/devices/register` | | `api_facilities` | `/api/v1` | `/facilities`, `/facilities//areas` | | `api_templates` | `/api/v1` | `/templates`, `/templates/` | -| `api_inspections` | `/api/v1` | `POST /inspections`, `PATCH /inspections/` | -| `api_issues` | `/api/v1` | `POST /issues` | +| `api_inspections` | `/api/v1` | `GET /inspections`, `POST /inspections`, `PATCH /inspections/` | +| `api_issues` | `/api/v1` | `GET /issues`, `POST /issues`, `GET /issues/`, `PATCH /issues//status` | | `api_photos` | `/api/v1` | `POST /photos/upload` | +| `api_notifications` | `/api/v1` | `GET /notifications`, `PATCH /notifications/mark-read` | --- @@ -349,7 +353,7 @@ api_device_tokens: id, user_id, device_id, apns_token, device_name, app_version All WTForms classes. `AreaForm.area_type` includes `floor`. `UserForm` excludes `customer` role. ### `notifications.py` -`notify()`, `notify_by_matrix()`, `notify_customers_for_facility()` — all email sent in background thread. +`notify()`, `notify_by_matrix()`, `notify_customers_for_facility()` — all email sent in background thread. `notify()` stores `event_type` on the `Notification` record (phase17+) so the mobile API can return it to the iPad for categorisation. `flag_followup` route calls `notify()` for the original inspector so they receive a follow-up request notification on the iPad. ### `sla.py` `sla_status(issue)` → `'ok'` | `'at_risk'` | `'breached'` | `None` (resolved). @@ -359,7 +363,7 @@ ReportLab-based. 12-column grid must be preserved — never collapse in PDF view --- -## 9. Mobile API (Phase 7 / Phase A / Phase B) +## 9. Mobile API (Phase 7 / Phase A / Phase B / Phase C) ### CSRF Exemption Pattern — Critical @@ -367,22 +371,24 @@ ReportLab-based. 12-column grid must be preserved — never collapse in PDF view ```python from app.api import register_api, api_bp -from app.api.auth import bp as _api_auth_bp -from app.api.facilities import bp as _api_facilities_bp -from app.api.templates import bp as _api_templates_bp -from app.api.inspections import bp as _api_inspections_bp -from app.api.issues import bp as _api_issues_bp -from app.api.photos import bp as _api_photos_bp +from app.api.auth import bp as _api_auth_bp +from app.api.facilities import bp as _api_facilities_bp +from app.api.templates import bp as _api_templates_bp +from app.api.inspections import bp as _api_inspections_bp +from app.api.issues import bp as _api_issues_bp +from app.api.photos import bp as _api_photos_bp +from app.api.notifications import bp as _api_notifications_bp csrf.exempt(_api_auth_bp) csrf.exempt(_api_facilities_bp) csrf.exempt(_api_templates_bp) csrf.exempt(_api_inspections_bp) csrf.exempt(_api_issues_bp) csrf.exempt(_api_photos_bp) +csrf.exempt(_api_notifications_bp) register_api(app) ``` -**Every new Phase C+ blueprint must add its own `csrf.exempt()` line here before `register_api(app)`.** Failing to do so produces a `"The CSRF token is missing."` error on all POST requests to that blueprint. +**Every new Phase D+ blueprint must add its own `csrf.exempt()` line here before `register_api(app)`.** Failing to do so produces a `"The CSRF token is missing."` error on all POST requests to that blueprint. ### Auth Flow 1. `POST /api/v1/auth/login` → access token (60 min JWT) + refresh token (30 day opaque hex) @@ -416,6 +422,27 @@ Customer role is blocked from template endpoints (`_ALLOWED_ROLES` check). Facil | `POST /api/v1/issues` | jwt_required | Create issue; idempotent via `mobile_local_id` | | `POST /api/v1/photos/upload` | jwt_required | Multipart photo upload; returns `server_path` | +### Phase C Endpoints + +| Endpoint | Auth | Description | +|---|---|---| +| `GET /api/v1/inspections` | jwt_required | Inspector's own inspection history (paginated) | +| `GET /api/v1/issues` | jwt_required | Issues assigned to current user (inspectors); all non-resolved (admin/director/PM) | +| `GET /api/v1/issues/` | jwt_required | Single issue detail — inspectors scoped to assigned only | +| `PATCH /api/v1/issues//status` | jwt_required | Update issue status — inspectors scoped to assigned only | +| `GET /api/v1/notifications` | jwt_required | Unread notifications for current user; accepts `?since=` | +| `PATCH /api/v1/notifications/mark-read` | jwt_required | Mark list of notification IDs as read | + +### Issue API Scope Rules + +- **Inspector:** `GET /issues` returns only `assigned_to == current_user.id`. `GET /issues/` and `PATCH /issues//status` both enforce the same restriction. +- **Admin / Director / Project Manager:** `GET /issues` returns all non-resolved issues (default) or filtered by `?status=`. +- `_issue_payload()` returns: `id`, `status`, `severity`, `description`, `assigned_to`, `facility_id`, `facility_name`, `reported_at`, `resolved_at`, `mobile_local_id`, `photo_path`, `result_photos`. + +### Notification API — OperationalError Safety + +`GET /api/v1/notifications` wraps the ORM query in `try/except sqlalchemy.exc.OperationalError`. If the `event_type` column does not yet exist (phase17 migration not run), it falls back to a raw-SQL query that omits the column and returns `"event_type": null`. This keeps the endpoint functional before and after the migration. + ### Idempotency Pattern All Phase B write endpoints accept `mobile_local_id` (UUID string from device). On receipt: @@ -502,6 +529,9 @@ Inspector action → SwiftData write (always succeeds) → SyncQueue entry - **Sequential reference data fetch:** `pullReferenceData()` uses sequential `await` (not `async let`) to avoid Swift 6 actor-isolation warnings on `Decodable` structs. - **Photo-before-inspection ordering:** `processPhotoQueue` runs before `processInspectionQueue`. An inspection is only submitted after all its `pendingPhotos` have `uploadStatus == "uploaded"` or `"failed"`. - **Retry limit:** 5 retries per item before marking `syncStatus = "failed"`. +- **`pullAssignedIssues()`:** Fetches `GET /api/v1/issues` and upserts into SwiftData keyed by `serverId`. Records pulled from server carry `syncStatus = "synced"` and `inspectionLocalId = ""` so `processIssueQueue` never re-submits them. **Deletion pass runs after upsert** — records with `syncStatus == "synced"` AND `inspectionLocalId == ""` whose `serverId` is absent from the server response are deleted. This removes issues that were reassigned to another inspector. The empty-response case is not short-circuited, so unassignment is always handled. +- **Notification polling:** `pollNotifications()` is called at the end of every `triggerSync()` and also on a 60-second `Task.sleep` loop started by `startPollTask()`. Uses `lastNotificationFetch` as a cursor (`?since=` param) so only new notifications are fetched. Marks fetched IDs read on server after local delivery. +- **`Task.sleep` not `Timer.scheduledTimer`:** `Timer.scheduledTimer` requires `RunLoop.main` to be ticking; inside a Swift Concurrency `Task { @MainActor }` block `RunLoop.current` is not `RunLoop.main` and the timer fires never. Always use `Task.sleep` for periodic work in SyncManager. ### APIClient Key Behaviours @@ -620,7 +650,8 @@ phase1_projects_roles → phase6_features → phase7_mobile_api → phase8_notif → phase9_user_full_name → phase10_customer_password_setup → phase11_director_role → phase12_performance_indexes → phase_b_mobile_local_id → phase13_issue_facility → phase14_facility_created_at - → phase15_audit_log_indexes → phase16_notifications_columns ← HEAD + → phase15_audit_log_indexes → phase16_notifications_columns + → phase17_notification_event_type ← HEAD ``` ### phase_b_mobile_local_id @@ -647,6 +678,10 @@ Uses `INFORMATION_SCHEMA.STATISTICS` existence checks — safe to re-run. Ensures `digest_pending TINYINT NOT NULL DEFAULT 0` and `inspection_id INT NULL FK` exist on the `notifications` table. Both columns are defined in the model but were absent from any prior migration because the `notifications` table predates the chain. Uses `INFORMATION_SCHEMA` existence checks — safe to re-run. +### phase17_notification_event_type + +Adds `event_type VARCHAR(50) NULL` to the `notifications` table. Required for `GET /api/v1/notifications` to return event type to the iPad so it can categorise alerts. The API endpoint catches `OperationalError` and falls back to raw SQL before this migration runs. Uses `INFORMATION_SCHEMA` existence check — safe to re-run. + ### MySQL ENUM Change Protocol (3 steps — always follow) ```sql -- 1. Expand @@ -774,6 +809,11 @@ timeout = 30 | 33 | **f-string fallback strings must use double-quotes inside single-quoted f-strings** | Python 3.11 raises `SyntaxError` on nested same-delimiter quotes; use `"\u2014"` not `'—'` inside `f'...'` | | 34 | **`computeScore` field ID must be cast explicitly: `String` → `as? String`, `Int` → `as? Int` then `String(n)`** | `Optional.map` on `Any?` returns `Optional(value)` not `value`; the old guard-let produced `"Optional(5)"` as the lookup key, so all integer-ID field scores were silently 0 | | 35 | **Strip `local://` photo paths from `formData` before `submitInspection`** | A failed photo upload leaves `"local://..."` in formData; `JSONSerialization` drops non-serialisable values silently, which is worse than an empty string on the server | +| 36 | **`notify()` must always receive `event_type`** | Without it the mobile API returns `null` for event type and the iPad cannot categorise the alert banner | +| 37 | **`flag_followup` calls `notify()` for the original inspector** | Without this, the inspector receives no follow-up request notification on any channel | +| 38 | **`GET /api/v1/notifications` catches `OperationalError`** | If phase17 migration hasn't run, the ORM query fails at the SQL layer because `event_type` is in the SELECT but not in the DB; `getattr()` does NOT protect against this — only a try/except does | +| 39 | **Issue API scope: inspectors see only their assigned issues** | `GET /issues`, `GET /issues/`, `PATCH /issues//status` all enforce `assigned_to == current_user.id` for the inspector role | +| 40 | **`_issue_payload()` must return `photo_path` and `result_photos`** | iPad `pullAssignedIssues` stores these in `photoServerPaths` for display via `AsyncImage`; omitting them means server-created issues show no photos | ---