From 4132373fee3012cd5379ce8db9fd77886dcfc612 Mon Sep 17 00:00:00 2001 From: Nguyen Ngo Date: Mon, 27 Jul 2026 15:56:19 -0400 Subject: [PATCH] Jul 27 - Update code for scheduled tasks 3 --- JanitorialQC/CLAUDE.md | 7 ++- .../Models/LocalScheduledInspection.swift | 26 ++++++++ JanitorialQC/PLACEMENT_MAP.txt | 62 +++++++++---------- .../Dashboard/ExecuteInspectionView.swift | 14 ++++- .../Views/Dashboard/MyInspectionsView.swift | 11 +++- .../Dashboard/ScheduledInspectionsView.swift | 12 +++- 6 files changed, 91 insertions(+), 41 deletions(-) diff --git a/JanitorialQC/CLAUDE.md b/JanitorialQC/CLAUDE.md index 937faec..e8b0f37 100644 --- a/JanitorialQC/CLAUDE.md +++ b/JanitorialQC/CLAUDE.md @@ -391,7 +391,9 @@ No SyncManager change was needed: `pullScheduledInspections()` already runs afte **Second fix (July 2026) — the `+` path and the vanishing cover.** The above only covers inspections *started from a Scheduled row*. The iPad's `+` button reaches the identical form with the facility and template hand-picked, and that path leaves `scheduledInspectionServerId` nil — the server stores `scheduled_inspection_id = NULL`, never calls `_fulfill_schedule()`, and the schedule stays **Active** on the web. On the web this cannot happen: `scheduled_inspections.start` is the only way in. `ExecuteInspectionView.resolveAndFulfillSchedule()` (called from `submitInspection()`, before `context.save()`) restores parity — see rule 69 for the match criteria and why the cached row is deleted rather than rolled forward. -Deleting that row at submit time exposed a second problem: `ScheduledInspectionsCard` self-hid the moment its `@Query` emptied, tearing down the `.fullScreenCover` it owned — with the inspector's form inside it. Cover ownership therefore moved to the parents (`DashboardStatsView`'s `ScrollView`, `MyInspectionsView`'s `Group`) and the cover item became the value type `ScheduledStartTarget`. See rule 68. +**Correction (July 2026).** That first cut *deleted* the cached row at submit. It shipped, and recurring schedules then stopped picking up their new due date — see rule 71. `resolveAndFulfillSchedule()` now sets `fulfilledLocally` instead, and both scheduled lists filter on it. The cover-ownership work below still stands: `pullScheduledInspections()` continues to delete rows for one-time schedules, so consumers must still hold value snapshots rather than the model. + +Flagging that row at submit time exposed a second problem: `ScheduledInspectionsCard` self-hid the moment its `@Query` emptied, tearing down the `.fullScreenCover` it owned — with the inspector's form inside it. Cover ownership therefore moved to the parents (`DashboardStatsView`'s `ScrollView`, `MyInspectionsView`'s `Group`) and the cover item became the value type `ScheduledStartTarget`. See rule 68. **Instructions (July 2026).** The manager-authored text on a schedule is labelled **"Instructions"** everywhere the user sees it, but remains `notes` on the wire, in `APIScheduledInspection`, and in `LocalScheduledInspection`. `LocalScheduledInspection.instructions` is a *computed* accessor over `notes` that trims and nils-out blank text — no stored property, so no schema change and no migration. Three surfaces: a one-line preview on `ScheduledRow`, a full Section at the top of `StartInspectionView` (passed in as `preFillScheduleInstructions` via `ScheduledStartTarget.instructions`), and a collapsible banner above the form in `ExecuteInspectionView` (looked up from SwiftData, so it also works on the draft-resume path where no parameter is threaded). See rule 70. @@ -693,8 +695,9 @@ Deletes `LocalIssue` where `serverId != nil`. Preserves `serverId == nil` record | 66 | **Modal (fullScreenCover root) views need an explicit Close/Cancel; pushed views get the nav back button for free** | `ExecuteInspectionView` takes `isModallyPresented` and shows a leading `Close` only when true (draft-resume from the dashboard is the root of its `NavigationStack`, no back button). All inspection-start flows now use `.fullScreenCover` for a consistent full-screen form: draft-resume → `ExecuteInspectionView(isModallyPresented: true)`; scheduled/new/re-inspection → `StartInspectionView` (its own `.cancellationAction` Cancel). `.onDisappear`/auto-save preserves work, so Close is always safe. | | 67 | **Render server photos through `ServerConfig.mediaURL(absolute:path:)`, never by hand-building `current + "/static/" + path`** | After the R2 migration the server returns absolute display URLs (presigned R2, or absolute-static on the local backend): `photo_urls`/`result_photo_urls` on issues (`APIAssignedIssue`/`APIIssueDetail` → `LocalIssue.photoServerUrls`/`resultPhotoServerUrls`, parallel to the path arrays), and `form_media` `{fieldId: url}` on inspection detail (`APIInspectionSummary.mediaURLByPath`, injected into the read-only grid via the `\.mediaURLByPath` environment for `PhotoThumbnailView`). The resolver prefers the absolute URL and falls back to `/static/` for older servers. Presigned URLs expire (24h) — always render from the freshest pull/detail fetch; don't persist a URL and reuse it days later. | | 68 | **A `.fullScreenCover` owner must outlive the rows that trigger it; carry a value snapshot, not the `@Model` object** | `ScheduledInspectionsCard` self-hides on `scheduled.isEmpty`, and submitting the last scheduled inspection empties that `@Query` **while the cover is still on screen** — the card disappears and takes its cover (and the inspector's form) with it. Fix: the card is presentational and reports taps via `onStart`; `DashboardStatsView` owns the cover on its always-present `ScrollView`, `MyInspectionsView` on its `Group`. The cover item is `ScheduledStartTarget` (three plain `Int`s), never `LocalScheduledInspection` — reading a deleted `PersistentModel` traps. | -| 69 | **`ExecuteInspectionView.resolveAndFulfillSchedule()` links the submission to its schedule and invalidates the cached row — it never computes the next due date** | Two entry points reach the identical form (Scheduled row → prefilled; `+` → hand-picked), but only the first sets `scheduledInspectionServerId`, so a `+`-started inspection lands with `scheduled_inspection_id = NULL` and the schedule stays **Active** on the web. The fallback matches an active `LocalScheduledInspection` on facility + template, gated to `dueDateString <= today`, assignee `nil`-or-self, earliest due first, and skipped for re-inspections. Roll-forward stays server-side: the local model has no phase43 recurrence detail (weekdays / month_mode / day_of_month / nth_week / nth_weekday), so the row is **deleted**, and `pullScheduledInspections()` re-inserts it with the authoritative `next_due_date` — which also self-heals a submission that never lands. | +| 69 | **`ExecuteInspectionView.resolveAndFulfillSchedule()` links the submission to its schedule and invalidates the cached row — it never computes the next due date** | Two entry points reach the identical form (Scheduled row → prefilled; `+` → hand-picked), but only the first sets `scheduledInspectionServerId`, so a `+`-started inspection lands with `scheduled_inspection_id = NULL` and the schedule stays **Active** on the web. The fallback matches an active `LocalScheduledInspection` on facility + template, gated to `dueDateString <= today`, assignee `nil`-or-self, earliest due first, and skipped for re-inspections. Roll-forward stays server-side: the local model has no phase43 recurrence detail (weekdays / month_mode / day_of_month / nth_week / nth_weekday), so the row is **flagged `fulfilledLocally`, never deleted** (see rule 71), and `pullScheduledInspections()` writes the authoritative `next_due_date` via `update(from:)` — which also self-heals a submission that never lands. | | 70 | **Snapshot schedule instructions into `@State` in `onAppear` — never read them from SwiftData during `body`** | `ExecuteInspectionView` shows the schedule's instructions above the form, but `resolveAndFulfillSchedule()` **deletes** that `LocalScheduledInspection` the instant Submit is tapped and the view stays up for another 2.5 s showing the success banner. A computed lookup would re-read a deleted `PersistentModel` in that window and trap. `loadScheduleInstructions()` copies the `String` once, at appear. Same reasoning as `ScheduledStartTarget` (rule 68): once the fulfilment path can delete a cached row mid-flow, every consumer must hold a value, not the model. | +| 71 | **Never delete a cached row the server is going to send again — flag it** | The first cut of `resolveAndFulfillSchedule()` deleted the `LocalScheduledInspection` at submit. One-time schedules were fine (the server deactivates them and never returns them again), but every **recurring** schedule is returned again on its next occurrence, so each completion became delete-then-reinsert against an `@Attribute(.unique)` serverId — and the reinserted row did not reliably carry the rolled-forward date. A daily schedule kept showing today's date after being completed. Fix: `fulfilledLocally: Bool = false` hides the row locally; `init(from:)`/`update(from:)` clear it, so the pull remains the only thing that ever writes a cached schedule's dates. Deletion of schedule rows now happens in exactly one place — the "server no longer returns it" branch of `pullScheduledInspections()`. | --- diff --git a/JanitorialQC/Models/LocalScheduledInspection.swift b/JanitorialQC/Models/LocalScheduledInspection.swift index b6b5b50..38f6d39 100644 --- a/JanitorialQC/Models/LocalScheduledInspection.swift +++ b/JanitorialQC/Models/LocalScheduledInspection.swift @@ -38,6 +38,27 @@ final class LocalScheduledInspection { var isOverdue: Bool var notes: String? + /// Set on device the moment an inspection fulfilling this schedule is + /// submitted, so the SCHEDULED lists hide the row immediately — online or + /// offline — without waiting for the round trip. + /// + /// Purely a display flag. The schedule lifecycle stays server-driven: the + /// next successful pull calls `update(from:)`, which clears this and writes + /// the authoritative `next_due_date`. If the submission never lands, the + /// server still reports the schedule as due and the row simply comes back — + /// self-healing. + /// + /// This exists instead of deleting the row. A recurring schedule is + /// returned by the server *again* after it rolls forward, so deleting meant + /// delete-then-reinsert against an `@Attribute(.unique)` key on every + /// completion, and the reinserted row did not reliably carry the new due + /// date. Flagging keeps `update(from:)` — the path that has always worked — + /// as the only way a cached row's dates ever change. + /// + /// Non-optional with an inline default, so SwiftData migrates lightweight + /// (CLAUDE.md rule 12): existing rows read as `false`, no app reinstall. + var fulfilledLocally: Bool = false + /// Manager-authored instructions for this occurrence, normalised. /// /// The wire field, the server model attribute and the DB column are all @@ -84,6 +105,7 @@ final class LocalScheduledInspection { self.dueDateString = api.nextDueDate ?? "" self.isOverdue = api.isOverdue self.notes = api.notes + self.fulfilledLocally = false self.updatedAt = Date() } @@ -98,6 +120,10 @@ final class LocalScheduledInspection { self.dueDateString = api.nextDueDate ?? "" self.isOverdue = api.isOverdue self.notes = api.notes + // The server is authoritative. Being returned by the pull at all means + // this schedule is live again — for a recurring one, on its NEXT + // occurrence — so any local "just did it" flag is stale by definition. + self.fulfilledLocally = false self.updatedAt = Date() } } diff --git a/JanitorialQC/PLACEMENT_MAP.txt b/JanitorialQC/PLACEMENT_MAP.txt index c4c6b3b..a0188de 100644 --- a/JanitorialQC/PLACEMENT_MAP.txt +++ b/JanitorialQC/PLACEMENT_MAP.txt @@ -1,44 +1,42 @@ -JQC iOS — scheduled inspection fulfilment fix -============================================= -Repo: jqc_ios_app Branch target: main -All paths are relative to the repo root (the folder containing JanitorialQC.xcodeproj). +JQC iOS — recurring schedules keep showing the old due date +=========================================================== +Repo: jqc_ios_app iOS ONLY — no server change, no migration. -DROP-IN (overwrite): +OVERWRITE (paths relative to the folder containing JanitorialQC.xcodeproj): + JanitorialQC/Models/LocalScheduledInspection.swift JanitorialQC/Views/Dashboard/ExecuteInspectionView.swift JanitorialQC/Views/Dashboard/ScheduledInspectionsView.swift JanitorialQC/Views/Dashboard/MyInspectionsView.swift - JanitorialQC/Views/Dashboard/DashboardView.swift JanitorialQC/CLAUDE.md -NO new files. NO files removed. NO Xcode project edits -(the target uses PBXFileSystemSynchronizedRootGroup — new/changed -files under JanitorialQC/ are picked up automatically). +No new files, none removed, no .xcodeproj edit. -NO server changes. NO SwiftData schema change, so no migration -and no need to delete the app from the iPad. +SwiftData: `fulfilledLocally: Bool = false` is a new stored property, but it is +non-optional WITH an inline default -> lightweight migration. Existing rows read +as false. NO need to delete the app from the iPad. BUILD - 1. git pull / copy files in - 2. Xcode → Product → Clean Build Folder (Shift-Cmd-K) - 3. Build & run on the iPad + Product > Clean Build Folder (Shift-Cmd-K), then run. VERIFY - A. Scheduled row path - Dashboard → SCHEDULED → tap row → Start → complete → Submit - → SCHEDULED card clears immediately - → web: Inspections → Scheduled → one-time schedule shows "Inactive" - B. "+" path (this is the case that was broken) - Dashboard/My Inspections → "+" → pick the SAME facility + template - as a schedule due today or earlier → complete → Submit - → same result as A - C. Offline - Airplane mode → run A → SCHEDULED card clears at once - → re-enable network → schedule goes Inactive on the web - D. Regression - Re-inspection (History → Re-inspect) must NOT deactivate any schedule - Ad-hoc inspection for a facility whose schedule is due NEXT month - must NOT deactivate it + 1. DAILY schedule due today. Complete + submit on the iPad. + - SCHEDULED card clears immediately + - within one sync cycle (<=60s) the row REAPPEARS showing TOMORROW + Cross-check the server agrees: + SELECT id, frequency, next_due_date, last_completed_at + FROM scheduled_inspections WHERE id = ; + 2. WEEKLY Mon/Wed/Fri: complete on Monday -> row returns showing Wednesday. + 3. MONTHLY day-of-month: complete -> row returns showing next month. + 4. ONE-TIME: complete -> row clears and STAYS gone (server deactivates it). + This is the case that always worked and must not regress. + 5. Offline: airplane mode, complete a daily schedule + -> card clears at once, row stays hidden while offline + -> re-enable network -> row returns with tomorrow's date + 6. Failed submit: if the inspection never syncs, the row comes back with its + ORIGINAL date (still genuinely due) rather than staying hidden. -SERVER-SIDE CHECK - sudo journalctl -u janitorial_qc --since "10 min ago" | grep "API INSPECTIONS" - expect: API INSPECTIONS | schedule fulfilled | schedule=N | inspection=M | next=deactivated +IF STEP 1 STILL FAILS + Check which side is wrong before changing the app again: + - server shows tomorrow, iPad shows today -> client, reopen this fix + - server still shows today -> fulfill() never ran; check + sudo journalctl -u janitorial_qc | grep "schedule fulfilled" diff --git a/JanitorialQC/Views/Dashboard/ExecuteInspectionView.swift b/JanitorialQC/Views/Dashboard/ExecuteInspectionView.swift index 79de01a..2d2dd7a 100644 --- a/JanitorialQC/Views/Dashboard/ExecuteInspectionView.swift +++ b/JanitorialQC/Views/Dashboard/ExecuteInspectionView.swift @@ -770,6 +770,7 @@ struct ExecuteInspectionView: View { $0.templateServerId == tid && $0.facilityServerId == fid && ($0.inspectorId == nil || $0.inspectorId == uid) && + !$0.fulfilledLocally && // already satisfied, awaiting sync !$0.dueDateString.isEmpty && $0.dueDateString <= today // ISO strings sort chronologically } @@ -781,12 +782,21 @@ struct ExecuteInspectionView: View { } } - // Clear the cached row for whichever schedule this submission fulfils. + // Hide the row until the server confirms what happened to it. + // + // NOT a delete. A recurring schedule comes back from the server on its + // next occurrence, so deleting turned every completion into a + // delete-then-reinsert against the `@Attribute(.unique)` serverId, and + // the reinserted row did not reliably pick up the new due date — a + // daily schedule kept showing today's date after being completed. + // One-time schedules masked it, because the server stops returning them + // and they are never reinserted. Flagging leaves `update(from:)` as the + // single path that ever writes a cached schedule's dates. guard let schedId = inspection.scheduledInspectionServerId, let sched = all.first(where: { $0.serverId == schedId }) else { return } - context.delete(sched) + sched.fulfilledLocally = true } /// "yyyy-MM-dd", matching `LocalScheduledInspection.dueDateString`. diff --git a/JanitorialQC/Views/Dashboard/MyInspectionsView.swift b/JanitorialQC/Views/Dashboard/MyInspectionsView.swift index 5be07ba..548dde4 100644 --- a/JanitorialQC/Views/Dashboard/MyInspectionsView.swift +++ b/JanitorialQC/Views/Dashboard/MyInspectionsView.swift @@ -19,6 +19,11 @@ struct MyInspectionsView: View { @Query(sort: \LocalScheduledInspection.dueDateString, order: .forward) private var scheduledAll: [LocalScheduledInspection] + /// Rows still awaiting action — see ScheduledInspectionsCard.visible. + private var scheduledVisible: [LocalScheduledInspection] { + scheduledAll.filter { !$0.fulfilledLocally } + } + @Environment(\.modelContext) private var context @State private var showNewInspection = false @@ -30,7 +35,7 @@ struct MyInspectionsView: View { var body: some View { Group { - if inspections.isEmpty && scheduledAll.isEmpty { + if inspections.isEmpty && scheduledVisible.isEmpty { ContentUnavailableView( "No Inspections", systemImage: "checklist", @@ -39,9 +44,9 @@ struct MyInspectionsView: View { } else { List { // Scheduled assignments (phase36) — self-hides when empty. - if !scheduledAll.isEmpty { + if !scheduledVisible.isEmpty { Section("Scheduled") { - ForEach(scheduledAll) { s in + ForEach(scheduledVisible) { s in Button { scheduledStartTarget = ScheduledStartTarget(s) } label: { ScheduledRow(schedule: s) } diff --git a/JanitorialQC/Views/Dashboard/ScheduledInspectionsView.swift b/JanitorialQC/Views/Dashboard/ScheduledInspectionsView.swift index 3090e20..e7c38de 100644 --- a/JanitorialQC/Views/Dashboard/ScheduledInspectionsView.swift +++ b/JanitorialQC/Views/Dashboard/ScheduledInspectionsView.swift @@ -136,6 +136,14 @@ struct ScheduledInspectionsCard: View { @Query(sort: \LocalScheduledInspection.dueDateString, order: .forward) private var scheduled: [LocalScheduledInspection] + /// Rows still awaiting action. Filtered in Swift rather than in the @Query + /// predicate, per CLAUDE.md rule 3. `fulfilledLocally` is set at submit and + /// cleared by the next pull, so a completed schedule leaves the card at + /// once and reappears only when the server says it is due again. + private var visible: [LocalScheduledInspection] { + scheduled.filter { !$0.fulfilledLocally } + } + /// Tap handler. The `.fullScreenCover` deliberately lives in the PARENT /// (`DashboardStatsView`, on its always-present ScrollView) rather than here: /// this card self-hides, and submitting the last scheduled inspection removes @@ -146,14 +154,14 @@ struct ScheduledInspectionsCard: View { let onStart: (ScheduledStartTarget) -> Void var body: some View { - if !scheduled.isEmpty { + if !visible.isEmpty { VStack(alignment: .leading, spacing: 10) { Text("SCHEDULED") .font(.caption.bold()) .foregroundStyle(.secondary) .tracking(1) - ForEach(scheduled) { s in + ForEach(visible) { s in Button { onStart(ScheduledStartTarget(s)) } label: { ScheduledRow(schedule: s) .padding(12)