05/16 Fix bugs 3

This commit is contained in:
Nguyen Ngo
2026-05-16 14:56:11 -04:00
parent 9c7aa72ff3
commit f436ac2ef2
3 changed files with 93 additions and 45 deletions
+45 -2
View File
@@ -288,14 +288,44 @@ struct APIInspectionSummary: Decodable, Identifiable, Hashable, Sendable {
let inspectionDate: String? let inspectionDate: String?
let completedAt: String? let completedAt: String?
let mobileLocalId: 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 // Follow-up / re-inspection
let followUpRequired: Bool let followUpRequired: Bool
let followUpNote: String? let followUpNote: String?
let parentInspectionId: Int? 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? { var inspectionDateParsed: Date? {
guard let str = inspectionDate else { return nil } 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 { nonisolated init(from decoder: any Decoder) throws {
@@ -312,6 +342,9 @@ struct APIInspectionSummary: Decodable, Identifiable, Hashable, Sendable {
inspectionDate = try? c.decode(String.self, forKey: .inspectionDate) inspectionDate = try? c.decode(String.self, forKey: .inspectionDate)
completedAt = try? c.decode(String.self, forKey: .completedAt) completedAt = try? c.decode(String.self, forKey: .completedAt)
mobileLocalId = try? c.decode(String.self, forKey: .mobileLocalId) 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 followUpRequired = (try? c.decode(Bool.self, forKey: .followUpRequired)) ?? false
followUpNote = try? c.decode(String.self, forKey: .followUpNote) followUpNote = try? c.decode(String.self, forKey: .followUpNote)
parentInspectionId = try? c.decode(Int.self, forKey: .parentInspectionId) parentInspectionId = try? c.decode(Int.self, forKey: .parentInspectionId)
@@ -319,9 +352,19 @@ struct APIInspectionSummary: Decodable, Identifiable, Hashable, Sendable {
private enum CodingKeys: String, CodingKey { private enum CodingKeys: String, CodingKey {
case id, templateId, templateName, facilityId, facilityName case id, templateId, templateName, facilityId, facilityName
case areaId, areaName, status, overallScore case areaId, areaName, status, overallScore
case inspectionDate, completedAt, mobileLocalId case inspectionDate, completedAt, mobileLocalId, inspectorNotes
case formData, formSchema
case followUpRequired, followUpNote, parentInspectionId 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 { struct InspectionHistoryResponseData: Decodable, Sendable {
+11 -6
View File
@@ -2,7 +2,7 @@
> **Audience:** AI assistants and developers working on the JanitorialQC iPad app. > **Audience:** AI assistants and developers working on the JanitorialQC iPad app.
> **Purpose:** Authoritative reference for architecture, conventions, constraints, and decisions. > **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. > **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. - `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`. - **`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]`. - **`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 ### Fetch pattern — CRITICAL for Xcode 26
@@ -346,15 +347,15 @@ request(endpoint, method, body, retrying) async throws -> T
| Method | Endpoint | Notes | | Method | Endpoint | Notes |
|---|---|---| |---|---|---|
| `request<T>` | Any | Generic GET/POST | | `request<T>` | Any | Generic GET/POST; `retrying: Bool` prevents double-refresh loop |
| `post<T>` | Any | POST convenience | | `post<T>` | 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 | | `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 | | `submitIssue` | `POST /api/v1/issues` | Sends `inspection_id` only if `inspection.serverId` is non-nil |
| `fetchInspectionHistory` | `GET /api/v1/inspections` | Paginated, returns `InspectionHistoryResponseData` | | `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/<id>` | Fetches current status for detail view | | `fetchIssueDetail` | `GET /api/v1/issues/<id>` | Fetches current status for detail view |
| `updateIssueStatus` | `PATCH /api/v1/issues/<id>/status` | Inspector updates status on assigned issues | | `updateIssueStatus` | `PATCH /api/v1/issues/<id>/status` | Inspector updates status on assigned/reported issues |
| `fetchNotifications` | `GET /api/v1/notifications` | Accepts optional `since: Date`; returns `[APINotification]` | | `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 | | `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 | | 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 | | 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 | | 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. 3. Chained optional patterns `(try? fetch(...))?.filter { }` are split into two `let` statements.
4. `triggerSync()` guards on `isAuthenticated` to prevent 401 cascades during startup. 4. `triggerSync()` guards on `isAuthenticated` to prevent 401 cascades during startup.
5. Notification polling uses `Task.sleep` not `Timer.scheduledTimer` (RunLoop dependency). 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. **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.
@@ -211,31 +211,21 @@ struct HistoryDetailView: View {
@Environment(\.modelContext) private var context @Environment(\.modelContext) private var context
@State private var showReInspect = false @State private var showReInspect = false
// Look up the local copy by mobileLocalId present only for this-device submissions // Local SwiftData copy used only for follow-up sync-back.
private var localCopy: LocalInspection? { // Form data and schema come from the server response directly so
guard let lid = inspection.mobileLocalId else { return nil } // History works even after app reinstall or on a different device.
return try? context.fetch( @State private var localCopy: LocalInspection? = nil
FetchDescriptor<LocalInspection>(
predicate: #Predicate { $0.localId == lid }
)
).first
}
// Fetch the template schema so we can render the form grid private var formSchema: [[String: Any]] { inspection.formSchema }
private var localTemplate: LocalTemplate? { private var savedValues: [String: String] { inspection.formValues }
guard let copy = localCopy else { return nil }
let id = copy.templateServerId
return try? context.fetch(
FetchDescriptor<LocalTemplate>(predicate: #Predicate { $0.serverId == id })
).first
}
private var formSchema: [[String: Any]] { localTemplate?.formSchema ?? [] } /// Load the local SwiftData copy once on appear (for follow-up sync only).
/// Fetch-all + filter in Swift #Predicate with captured String is banned
// Convert saved form data to [String: String] for the grid renderer /// under Xcode 26 SWIFT_DEFAULT_ACTOR_ISOLATION = MainActor (CLAUDE.md rules 3, 25).
private var savedValues: [String: String] { private func loadLocalData() {
guard let copy = localCopy else { return [:] } guard let lid = inspection.mobileLocalId else { return }
return copy.formData.compactMapValues { "\($0)" } let all = (try? context.fetch(FetchDescriptor<LocalInspection>())) ?? []
localCopy = all.first { $0.localId == lid }
} }
var body: some View { var body: some View {
@@ -290,6 +280,23 @@ struct HistoryDetailView: View {
issuesCard(copy.localIssues) 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 // Form responses
if !formSchema.isEmpty { if !formSchema.isEmpty {
VStack(alignment: .leading, spacing: 8) { VStack(alignment: .leading, spacing: 8) {
@@ -297,20 +304,12 @@ struct HistoryDetailView: View {
.font(.headline) .font(.headline)
.padding(.horizontal, 24) .padding(.horizontal, 24)
// Read-only form grid reuses GridFormView with disabled inputs
ReadOnlyGridFormView( ReadOnlyGridFormView(
schema: formSchema, schema: formSchema,
formValues: savedValues formValues: savedValues
) )
.padding(.horizontal, 24) .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) .padding(.vertical, 16)
@@ -319,6 +318,7 @@ struct HistoryDetailView: View {
.navigationTitle(inspection.templateName) .navigationTitle(inspection.templateName)
.navigationBarTitleDisplayMode(.inline) .navigationBarTitleDisplayMode(.inline)
.onAppear { .onAppear {
loadLocalData()
syncFollowUpToLocalCopy() syncFollowUpToLocalCopy()
} }
.sheet(isPresented: $showReInspect) { .sheet(isPresented: $showReInspect) {