From f436ac2ef2a95c58d4f79a42083a0d14a3ce1e82 Mon Sep 17 00:00:00 2001 From: Nguyen Ngo Date: Sat, 16 May 2026 14:56:11 -0400 Subject: [PATCH] 05/16 Fix bugs 3 --- JanitorialQC/API/APIModels.swift | 59 +++++++++++++++--- JanitorialQC/CLAUDE.md | 17 +++-- .../Inspection/InspectionHistoryView.swift | 62 +++++++++---------- 3 files changed, 93 insertions(+), 45 deletions(-) diff --git a/JanitorialQC/API/APIModels.swift b/JanitorialQC/API/APIModels.swift index 8264558..bf2eb5f 100644 --- a/JanitorialQC/API/APIModels.swift +++ b/JanitorialQC/API/APIModels.swift @@ -288,30 +288,63 @@ struct APIInspectionSummary: Decodable, Identifiable, Hashable, Sendable { let inspectionDate: String? let completedAt: String? let mobileLocalId: String? + let inspectorNotes: String + // Form responses and schema — included in every history response so the + // detail view works without a local SwiftData copy (e.g. after reinstall). + let formDataRaw: [String: JSONValue] + let formSchemaRaw: [[String: JSONValue]] // ── Follow-up / re-inspection ───────────────────────────────────────── let followUpRequired: Bool let followUpNote: String? let parentInspectionId: Int? + /// Form field values as [fieldId: stringValue] for the grid renderer. + var formValues: [String: String] { + var result: [String: String] = [:] + for (k, v) in formDataRaw { + switch v { + case .string(let s): result[k] = s + case .int(let n): result[k] = String(n) + case .double(let d): result[k] = String(d) + case .bool(let b): result[k] = b ? "true" : "false" + case .array(let a): result[k] = a.map { "\($0.anyValue)" }.joined(separator: ", ") + case .null: result[k] = "" + case .object: result[k] = "" + } + } + return result + } + + /// Form schema as [[String: Any]] for ReadOnlyGridFormView. + var formSchema: [[String: Any]] { + formSchemaRaw.map { dict in dict.mapValues { $0.anyValue } } + } + var inspectionDateParsed: Date? { guard let str = inspectionDate else { return nil } - return ISO8601DateFormatter().date(from: str) + // Server sends "yyyy-MM-dd'T'HH:mm:ss" with no timezone suffix. + // ISO8601DateFormatter() requires a timezone by default and returns nil + // for timezone-less strings — use the shared DateFormatter instead. + return SyncManager.isoFormatter.date(from: str) } nonisolated init(from decoder: any Decoder) throws { let c = try decoder.container(keyedBy: CodingKeys.self) - id = try c.decode(Int.self, forKey: .id) - templateId = try c.decode(Int.self, forKey: .templateId) - templateName = try c.decode(String.self, forKey: .templateName) - facilityId = try c.decode(Int.self, forKey: .facilityId) - facilityName = try c.decode(String.self, forKey: .facilityName) + id = try c.decode(Int.self, forKey: .id) + templateId = try c.decode(Int.self, forKey: .templateId) + templateName = try c.decode(String.self, forKey: .templateName) + facilityId = try c.decode(Int.self, forKey: .facilityId) + facilityName = try c.decode(String.self, forKey: .facilityName) areaId = try? c.decode(Int.self, forKey: .areaId) areaName = try? c.decode(String.self, forKey: .areaName) - status = try c.decode(String.self, forKey: .status) + status = try c.decode(String.self, forKey: .status) overallScore = try? c.decode(Double.self, forKey: .overallScore) inspectionDate = try? c.decode(String.self, forKey: .inspectionDate) completedAt = try? c.decode(String.self, forKey: .completedAt) mobileLocalId = try? c.decode(String.self, forKey: .mobileLocalId) + inspectorNotes = (try? c.decode(String.self, forKey: .inspectorNotes)) ?? "" + formDataRaw = (try? c.decode([String: JSONValue].self, forKey: .formData)) ?? [:] + formSchemaRaw = (try? c.decode([[String: JSONValue]].self, forKey: .formSchema)) ?? [] followUpRequired = (try? c.decode(Bool.self, forKey: .followUpRequired)) ?? false followUpNote = try? c.decode(String.self, forKey: .followUpNote) parentInspectionId = try? c.decode(Int.self, forKey: .parentInspectionId) @@ -319,9 +352,19 @@ struct APIInspectionSummary: Decodable, Identifiable, Hashable, Sendable { private enum CodingKeys: String, CodingKey { case id, templateId, templateName, facilityId, facilityName case areaId, areaName, status, overallScore - case inspectionDate, completedAt, mobileLocalId + case inspectionDate, completedAt, mobileLocalId, inspectorNotes + case formData, formSchema case followUpRequired, followUpNote, parentInspectionId } + + // Explicit Hashable — formDataRaw/formSchemaRaw contain JSONValue which + // has no Hashable conformance; identity is determined by server id alone. + static func == (lhs: APIInspectionSummary, rhs: APIInspectionSummary) -> Bool { + lhs.id == rhs.id + } + func hash(into hasher: inout Hasher) { + hasher.combine(id) + } } struct InspectionHistoryResponseData: Decodable, Sendable { diff --git a/JanitorialQC/CLAUDE.md b/JanitorialQC/CLAUDE.md index c06d21d..361a89f 100644 --- a/JanitorialQC/CLAUDE.md +++ b/JanitorialQC/CLAUDE.md @@ -2,7 +2,7 @@ > **Audience:** AI assistants and developers working on the JanitorialQC iPad app. > **Purpose:** Authoritative reference for architecture, conventions, constraints, and decisions. -> **Last reviewed:** May 2026 (Phase C complete — notification polling, assigned-issue sync, issue photo display, FlagIssueView confirmation banner, re-inspection pre-fill fix) +> **Last reviewed:** May 2026 (Phase 18 complete — static DateFormatter, uploadPhoto retry guard, explicit LocalIssue relationship inverse, APIUser.fullName/displayName, build fixes: SyncManager.shared restored, _RefreshEnvelope nonisolated init) > **Companion:** See the web backend's `CLAUDE.md` for API contract, server-side rules, and migration chain. --- @@ -293,6 +293,7 @@ The `isAuthenticated` guard is critical. `NWPathMonitor` fires immediately on co - `resetNotificationPoller()` cancels task + clears `lastNotificationFetch`. Call on logout. - **`Timer.scheduledTimer` is banned for periodic work in SyncManager.** Timer requires `RunLoop.main` to be ticking; inside a Swift Concurrency `Task { @MainActor }` block `RunLoop.current` ≠ `RunLoop.main` — the timer fires silently never. Always use `Task.sleep`. - **`UNUserNotificationCenterDelegate` is required for foreground delivery.** Without it, iOS silently drops local notifications when the app is active. `NotificationDelegate.shared` is set as `UNUserNotificationCenter.current().delegate` in `JanitorialQCApp.init()`. Its `willPresent` returns `[.banner, .sound]`. +- **`Self.isoFormatter` for all date parsing.** `SyncManager.isoFormatter` is a `nonisolated static let DateFormatter` with `locale = Locale(identifier: "en_US_POSIX")` and `dateFormat = "yyyy-MM-dd'T'HH:mm:ss"`. Used by both `pollNotifications()` and `pullAssignedIssues()`. **Never allocate a `DateFormatter` per call or per loop iteration** — it is expensive. The `en_US_POSIX` locale is mandatory for fixed-format parsing; without it, the system locale can reinterpret the format string unpredictably. ### Fetch pattern — CRITICAL for Xcode 26 @@ -346,15 +347,15 @@ request(endpoint, method, body, retrying) async throws -> T | Method | Endpoint | Notes | |---|---|---| -| `request` | Any | Generic GET/POST | +| `request` | Any | Generic GET/POST; `retrying: Bool` prevents double-refresh loop | | `post` | Any | POST convenience | -| `uploadPhoto` | `POST /api/v1/photos/upload` | Multipart form-data, manual boundary | +| `uploadPhoto` | `POST /api/v1/photos/upload` | Multipart form-data, manual boundary; `retrying: Bool = false` matches `request()` retry pattern | | `submitInspection` | `POST /api/v1/inspections` | Sanitises `local://` photo paths before send | | `submitIssue` | `POST /api/v1/issues` | Sends `inspection_id` only if `inspection.serverId` is non-nil | | `fetchInspectionHistory` | `GET /api/v1/inspections` | Paginated, returns `InspectionHistoryResponseData` | -| `fetchAssignedIssues` | `GET /api/v1/issues` | Returns issues assigned to current user | +| `fetchAssignedIssues` | `GET /api/v1/issues` | Returns issues assigned to OR reported by current user | | `fetchIssueDetail` | `GET /api/v1/issues/` | Fetches current status for detail view | -| `updateIssueStatus` | `PATCH /api/v1/issues//status` | Inspector updates status on assigned issues | +| `updateIssueStatus` | `PATCH /api/v1/issues//status` | Inspector updates status on assigned/reported issues | | `fetchNotifications` | `GET /api/v1/notifications` | Accepts optional `since: Date`; returns `[APINotification]` | | `markNotificationsRead` | `PATCH /api/v1/notifications/mark-read` | Marks list of IDs read on server | @@ -666,6 +667,10 @@ The app registers a `BGProcessingTask` with identifier `com.jqc.sync`. | 32 | **`StartInspectionView.onChange(of: selectedProjectId)` guards against resetting pre-filled facility** | Check `facilityBelongsToContract` before clearing `selectedFacilityId`; the onChange fires during `applyPreFill()` before `selectedFacilityId` is applied, wiping it if unchecked | | 33 | **Issue photos are stored in `photoServerPaths` after `pullAssignedIssues`** | `photo_path` and `result_photos` from `_issue_payload` are merged into a single `[String]` and stored in `local.photoServerPaths` on insert/update | | 34 | **`IssueDetailView` shows both `photoLocalPaths` and `photoServerPaths`** | Local paths use `UIImage(contentsOfFile:)`; server paths use `AsyncImage` with `Constants.baseURL` prefix. Both sections are independent | +| 35 | **`SyncManager.isoFormatter` is the only date formatter — never allocate per-call** | `DateFormatter` init is expensive. The static `nonisolated` formatter with `en_US_POSIX` locale handles all `yyyy-MM-dd'T'HH:mm:ss` parsing. Adding a new `DateFormatter` anywhere in SyncManager is wrong. | +| 36 | **`uploadPhoto(retrying:)` — pass `retrying: true` on recursive retry** | Matches `request()` pattern. Without it, a 401 on the retry triggers a second token refresh instead of throwing `notAuthenticated`. | +| 37 | **`LocalIssue.inspection` must declare explicit `@Relationship` inverse** | `@Relationship(deleteRule: .nullify, inverse: \LocalInspection.localIssues)` — without it SwiftData infers the inverse implicitly, which can produce migration warnings and incorrect cascade behaviour under some Xcode 26 versions. `deleteRule` is `.nullify` not `.cascade` because cascade is already declared on `LocalInspection.localIssues`. | +| 38 | **`APIUser.displayName` uses `fullName` when non-empty, falls back to `username`** | Server `_user_payload` sends `full_name`; iOS decodes as `fullName: String` (empty string when unset). `displayName` computed property: `fullName.isEmpty ? username : fullName`. Mirrors `User.display_name` server-side. | --- @@ -689,7 +694,7 @@ Xcode 26 sets this build setting when creating new projects with "approachable c 3. Chained optional patterns `(try? fetch(...))?.filter { }` are split into two `let` statements. 4. `triggerSync()` guards on `isAuthenticated` to prevent 401 cascades during startup. 5. Notification polling uses `Task.sleep` not `Timer.scheduledTimer` (RunLoop dependency). -6. All API model structs declare `nonisolated init(from decoder: any Decoder)` explicitly. +6. All API model structs declare `nonisolated init(from decoder: any Decoder)` explicitly — including file-scope `private` structs like `_RefreshEnvelope` in `APIClient.swift`. A `private` struct defined in a file containing an `actor` or `@MainActor` type can have its `Decodable` conformance tainted with `@MainActor`, producing "cannot be used in actor-isolated context" errors in Swift 6 mode. The fix is always an explicit `nonisolated init(from:)` with a matching `CodingKeys` enum. **Do NOT remove `SWIFT_DEFAULT_ACTOR_ISOLATION = MainActor` from build settings** unless you fully audit every file for the resulting isolation changes. The patterns above are the correct workarounds. diff --git a/JanitorialQC/Views/Inspection/InspectionHistoryView.swift b/JanitorialQC/Views/Inspection/InspectionHistoryView.swift index 6cda5db..a67452c 100644 --- a/JanitorialQC/Views/Inspection/InspectionHistoryView.swift +++ b/JanitorialQC/Views/Inspection/InspectionHistoryView.swift @@ -211,31 +211,21 @@ struct HistoryDetailView: View { @Environment(\.modelContext) private var context @State private var showReInspect = false - // Look up the local copy by mobileLocalId — present only for this-device submissions - private var localCopy: LocalInspection? { - guard let lid = inspection.mobileLocalId else { return nil } - return try? context.fetch( - FetchDescriptor( - predicate: #Predicate { $0.localId == lid } - ) - ).first - } + // Local SwiftData copy — used only for follow-up sync-back. + // Form data and schema come from the server response directly so + // History works even after app reinstall or on a different device. + @State private var localCopy: LocalInspection? = nil - // Fetch the template schema so we can render the form grid - private var localTemplate: LocalTemplate? { - guard let copy = localCopy else { return nil } - let id = copy.templateServerId - return try? context.fetch( - FetchDescriptor(predicate: #Predicate { $0.serverId == id }) - ).first - } + private var formSchema: [[String: Any]] { inspection.formSchema } + private var savedValues: [String: String] { inspection.formValues } - private var formSchema: [[String: Any]] { localTemplate?.formSchema ?? [] } - - // Convert saved form data to [String: String] for the grid renderer - private var savedValues: [String: String] { - guard let copy = localCopy else { return [:] } - return copy.formData.compactMapValues { "\($0)" } + /// Load the local SwiftData copy once on appear (for follow-up sync only). + /// Fetch-all + filter in Swift — #Predicate with captured String is banned + /// under Xcode 26 SWIFT_DEFAULT_ACTOR_ISOLATION = MainActor (CLAUDE.md rules 3, 25). + private func loadLocalData() { + guard let lid = inspection.mobileLocalId else { return } + let all = (try? context.fetch(FetchDescriptor())) ?? [] + localCopy = all.first { $0.localId == lid } } var body: some View { @@ -290,6 +280,23 @@ struct HistoryDetailView: View { issuesCard(copy.localIssues) } + // ── Inspector notes ──────────────────────────────────────── + if !inspection.inspectorNotes.isEmpty { + VStack(alignment: .leading, spacing: 6) { + Text("Inspector Notes") + .font(.headline) + .padding(.horizontal, 24) + Text(inspection.inspectorNotes) + .font(.callout) + .foregroundStyle(.primary) + .padding(16) + .frame(maxWidth: .infinity, alignment: .leading) + .background(Color(.secondarySystemBackground)) + .clipShape(RoundedRectangle(cornerRadius: 12)) + .padding(.horizontal, 24) + } + } + // ── Form responses ───────────────────────────────────────── if !formSchema.isEmpty { VStack(alignment: .leading, spacing: 8) { @@ -297,20 +304,12 @@ struct HistoryDetailView: View { .font(.headline) .padding(.horizontal, 24) - // Read-only form grid — reuses GridFormView with disabled inputs ReadOnlyGridFormView( schema: formSchema, formValues: savedValues ) .padding(.horizontal, 24) } - } else if localCopy != nil { - // Template schema no longer cached locally - infoRow( - icon: "doc.text", - text: "Form schema not available offline. Sync to view full responses." - ) - .padding(.horizontal, 24) } } .padding(.vertical, 16) @@ -319,6 +318,7 @@ struct HistoryDetailView: View { .navigationTitle(inspection.templateName) .navigationBarTitleDisplayMode(.inline) .onAppear { + loadLocalData() syncFollowUpToLocalCopy() } .sheet(isPresented: $showReInspect) {