Jul 27 - Update code for scheduled tasks 3

This commit is contained in:
Nguyen Ngo
2026-07-27 15:56:19 -04:00
parent ad91a92ff1
commit 4132373fee
6 changed files with 91 additions and 41 deletions
+5 -2
View File
@@ -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. **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. **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. | | 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. | | 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. | | 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. | | 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()`. |
--- ---
@@ -38,6 +38,27 @@ final class LocalScheduledInspection {
var isOverdue: Bool var isOverdue: Bool
var notes: String? 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. /// Manager-authored instructions for this occurrence, normalised.
/// ///
/// The wire field, the server model attribute and the DB column are all /// 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.dueDateString = api.nextDueDate ?? ""
self.isOverdue = api.isOverdue self.isOverdue = api.isOverdue
self.notes = api.notes self.notes = api.notes
self.fulfilledLocally = false
self.updatedAt = Date() self.updatedAt = Date()
} }
@@ -98,6 +120,10 @@ final class LocalScheduledInspection {
self.dueDateString = api.nextDueDate ?? "" self.dueDateString = api.nextDueDate ?? ""
self.isOverdue = api.isOverdue self.isOverdue = api.isOverdue
self.notes = api.notes 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() self.updatedAt = Date()
} }
} }
+30 -32
View File
@@ -1,44 +1,42 @@
JQC iOS — scheduled inspection fulfilment fix JQC iOS — recurring schedules keep showing the old due date
============================================= ===========================================================
Repo: jqc_ios_app Branch target: main Repo: jqc_ios_app iOS ONLY — no server change, no migration.
All paths are relative to the repo root (the folder containing JanitorialQC.xcodeproj).
DROP-IN (overwrite): OVERWRITE (paths relative to the folder containing JanitorialQC.xcodeproj):
JanitorialQC/Models/LocalScheduledInspection.swift
JanitorialQC/Views/Dashboard/ExecuteInspectionView.swift JanitorialQC/Views/Dashboard/ExecuteInspectionView.swift
JanitorialQC/Views/Dashboard/ScheduledInspectionsView.swift JanitorialQC/Views/Dashboard/ScheduledInspectionsView.swift
JanitorialQC/Views/Dashboard/MyInspectionsView.swift JanitorialQC/Views/Dashboard/MyInspectionsView.swift
JanitorialQC/Views/Dashboard/DashboardView.swift
JanitorialQC/CLAUDE.md JanitorialQC/CLAUDE.md
NO new files. NO files removed. NO Xcode project edits No new files, none removed, no .xcodeproj edit.
(the target uses PBXFileSystemSynchronizedRootGroup — new/changed
files under JanitorialQC/ are picked up automatically).
NO server changes. NO SwiftData schema change, so no migration SwiftData: `fulfilledLocally: Bool = false` is a new stored property, but it is
and no need to delete the app from the iPad. non-optional WITH an inline default -> lightweight migration. Existing rows read
as false. NO need to delete the app from the iPad.
BUILD BUILD
1. git pull / copy files in Product > Clean Build Folder (Shift-Cmd-K), then run.
2. Xcode → Product → Clean Build Folder (Shift-Cmd-K)
3. Build & run on the iPad
VERIFY VERIFY
A. Scheduled row path 1. DAILY schedule due today. Complete + submit on the iPad.
Dashboard → SCHEDULED → tap row → Start → complete → Submit - SCHEDULED card clears immediately
→ SCHEDULED card clears immediately - within one sync cycle (<=60s) the row REAPPEARS showing TOMORROW
→ web: Inspections → Scheduled → one-time schedule shows "Inactive" Cross-check the server agrees:
B. "+" path (this is the case that was broken) SELECT id, frequency, next_due_date, last_completed_at
Dashboard/My Inspections → "+" → pick the SAME facility + template FROM scheduled_inspections WHERE id = <id>;
as a schedule due today or earlier → complete → Submit 2. WEEKLY Mon/Wed/Fri: complete on Monday -> row returns showing Wednesday.
→ same result as A 3. MONTHLY day-of-month: complete -> row returns showing next month.
C. Offline 4. ONE-TIME: complete -> row clears and STAYS gone (server deactivates it).
Airplane mode → run A → SCHEDULED card clears at once This is the case that always worked and must not regress.
→ re-enable network → schedule goes Inactive on the web 5. Offline: airplane mode, complete a daily schedule
D. Regression -> card clears at once, row stays hidden while offline
Re-inspection (History → Re-inspect) must NOT deactivate any schedule -> re-enable network -> row returns with tomorrow's date
Ad-hoc inspection for a facility whose schedule is due NEXT month 6. Failed submit: if the inspection never syncs, the row comes back with its
must NOT deactivate it ORIGINAL date (still genuinely due) rather than staying hidden.
SERVER-SIDE CHECK IF STEP 1 STILL FAILS
sudo journalctl -u janitorial_qc --since "10 min ago" | grep "API INSPECTIONS" Check which side is wrong before changing the app again:
expect: API INSPECTIONS | schedule fulfilled | schedule=N | inspection=M | next=deactivated - 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"
@@ -770,6 +770,7 @@ struct ExecuteInspectionView: View {
$0.templateServerId == tid && $0.templateServerId == tid &&
$0.facilityServerId == fid && $0.facilityServerId == fid &&
($0.inspectorId == nil || $0.inspectorId == uid) && ($0.inspectorId == nil || $0.inspectorId == uid) &&
!$0.fulfilledLocally && // already satisfied, awaiting sync
!$0.dueDateString.isEmpty && !$0.dueDateString.isEmpty &&
$0.dueDateString <= today // ISO strings sort chronologically $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, guard let schedId = inspection.scheduledInspectionServerId,
let sched = all.first(where: { $0.serverId == schedId }) let sched = all.first(where: { $0.serverId == schedId })
else { return } else { return }
context.delete(sched) sched.fulfilledLocally = true
} }
/// "yyyy-MM-dd", matching `LocalScheduledInspection.dueDateString`. /// "yyyy-MM-dd", matching `LocalScheduledInspection.dueDateString`.
@@ -19,6 +19,11 @@ struct MyInspectionsView: View {
@Query(sort: \LocalScheduledInspection.dueDateString, order: .forward) @Query(sort: \LocalScheduledInspection.dueDateString, order: .forward)
private var scheduledAll: [LocalScheduledInspection] private var scheduledAll: [LocalScheduledInspection]
/// Rows still awaiting action see ScheduledInspectionsCard.visible.
private var scheduledVisible: [LocalScheduledInspection] {
scheduledAll.filter { !$0.fulfilledLocally }
}
@Environment(\.modelContext) private var context @Environment(\.modelContext) private var context
@State private var showNewInspection = false @State private var showNewInspection = false
@@ -30,7 +35,7 @@ struct MyInspectionsView: View {
var body: some View { var body: some View {
Group { Group {
if inspections.isEmpty && scheduledAll.isEmpty { if inspections.isEmpty && scheduledVisible.isEmpty {
ContentUnavailableView( ContentUnavailableView(
"No Inspections", "No Inspections",
systemImage: "checklist", systemImage: "checklist",
@@ -39,9 +44,9 @@ struct MyInspectionsView: View {
} else { } else {
List { List {
// Scheduled assignments (phase36) self-hides when empty. // Scheduled assignments (phase36) self-hides when empty.
if !scheduledAll.isEmpty { if !scheduledVisible.isEmpty {
Section("Scheduled") { Section("Scheduled") {
ForEach(scheduledAll) { s in ForEach(scheduledVisible) { s in
Button { scheduledStartTarget = ScheduledStartTarget(s) } label: { Button { scheduledStartTarget = ScheduledStartTarget(s) } label: {
ScheduledRow(schedule: s) ScheduledRow(schedule: s)
} }
@@ -136,6 +136,14 @@ struct ScheduledInspectionsCard: View {
@Query(sort: \LocalScheduledInspection.dueDateString, order: .forward) @Query(sort: \LocalScheduledInspection.dueDateString, order: .forward)
private var scheduled: [LocalScheduledInspection] 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 /// Tap handler. The `.fullScreenCover` deliberately lives in the PARENT
/// (`DashboardStatsView`, on its always-present ScrollView) rather than here: /// (`DashboardStatsView`, on its always-present ScrollView) rather than here:
/// this card self-hides, and submitting the last scheduled inspection removes /// this card self-hides, and submitting the last scheduled inspection removes
@@ -146,14 +154,14 @@ struct ScheduledInspectionsCard: View {
let onStart: (ScheduledStartTarget) -> Void let onStart: (ScheduledStartTarget) -> Void
var body: some View { var body: some View {
if !scheduled.isEmpty { if !visible.isEmpty {
VStack(alignment: .leading, spacing: 10) { VStack(alignment: .leading, spacing: 10) {
Text("SCHEDULED") Text("SCHEDULED")
.font(.caption.bold()) .font(.caption.bold())
.foregroundStyle(.secondary) .foregroundStyle(.secondary)
.tracking(1) .tracking(1)
ForEach(scheduled) { s in ForEach(visible) { s in
Button { onStart(ScheduledStartTarget(s)) } label: { Button { onStart(ScheduledStartTarget(s)) } label: {
ScheduledRow(schedule: s) ScheduledRow(schedule: s)
.padding(12) .padding(12)