Aug 27 - Fixed inspection lost photos recovery
This commit is contained in:
Nguyen Ngo
@@ -24,12 +24,14 @@ class SyncManager: ObservableObject {
|
||||
@Published var pendingCount = 0
|
||||
/// Dashboard KPI stats fetched from the server. Nil until first successful fetch.
|
||||
@Published var dashboardStats: APIDashboardStats?
|
||||
/// Count of notifications received since last resetNotificationPoller().
|
||||
/// Incremented on each poll that returns new items; reset to 0 on logout.
|
||||
/// Number of `LocalNotification` rows with `isRead == false`.
|
||||
///
|
||||
/// Derived from the store rather than counted as items arrive: read state
|
||||
/// is now real and can change from either end (a tap here, Mark All, or the
|
||||
/// same account reading on the web), so an incrementing tally would drift.
|
||||
/// Refreshed by `refreshUnreadNotificationCount()` after every poll and
|
||||
/// every read action.
|
||||
@Published var unreadNotificationCount = 0
|
||||
/// The most recent batch of notifications (up to 50) for the in-app inbox.
|
||||
/// Replaced entirely on each successful poll; empty until first fetch.
|
||||
@Published var recentNotifications: [APINotification] = []
|
||||
|
||||
// ── Dependencies ──────────────────────────────────────────────────────
|
||||
|
||||
@@ -142,11 +144,17 @@ class SyncManager: ObservableObject {
|
||||
}
|
||||
|
||||
/// Called on logout so the next login starts a clean fetch.
|
||||
///
|
||||
/// Resets the CURSOR and stops the task; it does not touch the stored
|
||||
/// inbox. The same inspector signing back in should still find their
|
||||
/// notifications and their read state, exactly as they find their cached
|
||||
/// facilities and issues — a different inspector is handled by the identity
|
||||
/// purge instead (rule 88). `unreadNotificationCount` is recomputed from
|
||||
/// the store rather than zeroed, so the badge stays truthful.
|
||||
func resetNotificationPoller() {
|
||||
lastNotificationFetch = nil
|
||||
unreadNotificationCount = 0
|
||||
recentNotifications = []
|
||||
lastNotificationFetch = nil
|
||||
stopPollTask()
|
||||
refreshUnreadNotificationCount()
|
||||
}
|
||||
|
||||
/// Called when the app enters the background (scenePhase == .background).
|
||||
@@ -164,10 +172,10 @@ class SyncManager: ObservableObject {
|
||||
Task { await triggerSync() }
|
||||
}
|
||||
|
||||
/// Call when the user opens the NotificationsView to clear the badge.
|
||||
func markNotificationsViewed() {
|
||||
unreadNotificationCount = 0
|
||||
}
|
||||
// NOTE: `markNotificationsViewed()` is gone. It zeroed the badge merely
|
||||
// because the inbox had been OPENED, which is incompatible with showing
|
||||
// real read state — the badge would read 0 while every row still rendered
|
||||
// as unread. Reading is now an explicit act: tap a row, or Mark All Read.
|
||||
|
||||
// ── Notification polling ──────────────────────────────────────────────
|
||||
|
||||
@@ -177,25 +185,39 @@ class SyncManager: ObservableObject {
|
||||
let notifications = try await APIClient.shared.fetchNotifications(since: lastNotificationFetch)
|
||||
guard !notifications.isEmpty else { return }
|
||||
|
||||
// Deliver a local notification for each new item
|
||||
for n in notifications {
|
||||
deliverLocalNotification(n)
|
||||
// Upsert into the local inbox. The endpoint only ever returns
|
||||
// UNREAD rows and never sends the flag, so a row disappearing from
|
||||
// the response says nothing — it may have been read on the web, or
|
||||
// simply be older than the cursor. Rows are therefore never deleted
|
||||
// here; `pruneReadNotifications()` handles retention instead.
|
||||
if let context = modelContext {
|
||||
let existing = (try? context.fetch(FetchDescriptor<LocalNotification>())) ?? []
|
||||
var byServerId: [Int: LocalNotification] = [:]
|
||||
for row in existing { byServerId[row.serverId] = row }
|
||||
|
||||
for api in notifications {
|
||||
if let row = byServerId[api.id] {
|
||||
row.update(from: api)
|
||||
} else {
|
||||
context.insert(LocalNotification(from: api))
|
||||
// Banner ONLY for genuinely new rows. Previously every
|
||||
// polled item was delivered, so a cold launch (cursor
|
||||
// nil → the server returns all unread) re-banner'd the
|
||||
// inspector's whole backlog on every app start.
|
||||
deliverLocalNotification(api)
|
||||
}
|
||||
}
|
||||
try? context.save()
|
||||
refreshUnreadNotificationCount()
|
||||
}
|
||||
|
||||
// Update in-app inbox state.
|
||||
// Prepend new notifications and cap at 50 — avoids allocating two
|
||||
// arrays and concatenating them on every poll (the old pattern
|
||||
// `notifications + recentNotifications.prefix(50 - count)` always
|
||||
// created a new array even when notifications.count >= 50).
|
||||
recentNotifications.insert(contentsOf: notifications, at: 0)
|
||||
if recentNotifications.count > 50 { recentNotifications = Array(recentNotifications.prefix(50)) }
|
||||
unreadNotificationCount += notifications.count
|
||||
|
||||
// Update the cursor to the newest notification's timestamp so the
|
||||
// next poll only fetches newer items — do NOT mark notifications as
|
||||
// read on the server. Read state is a deliberate user action managed
|
||||
// via the web app; marking read here would cause the web badge count
|
||||
// to always show zero when the iPad has polled before the user checks.
|
||||
// Advance the cursor so the next poll only fetches newer items.
|
||||
//
|
||||
// Still no implicit mark-read: the server's read state changes only
|
||||
// on a deliberate user action — a tap or Mark All, which route
|
||||
// through markNotificationRead/markAllNotificationsRead. Marking on
|
||||
// poll would zero the user's WEB badge simply because the iPad was
|
||||
// switched on (rule 92).
|
||||
let dates = notifications.compactMap { Self.isoFormatter.date(from: $0.createdAt) }
|
||||
if let newest = dates.max() {
|
||||
lastNotificationFetch = newest
|
||||
@@ -208,6 +230,86 @@ class SyncManager: ObservableObject {
|
||||
}
|
||||
}
|
||||
|
||||
// ── Notification read state ───────────────────────────────────────────
|
||||
|
||||
/// Recount unread rows and publish. Cheap: one fetch, no relationships.
|
||||
func refreshUnreadNotificationCount() {
|
||||
guard let context = modelContext else { return }
|
||||
let all = (try? context.fetch(FetchDescriptor<LocalNotification>())) ?? []
|
||||
unreadNotificationCount = all.filter { !$0.isRead }.count
|
||||
}
|
||||
|
||||
/// Mark one notification read — optimistically local, then pushed.
|
||||
///
|
||||
/// Local first so the inbox responds instantly and works offline; the
|
||||
/// server call is best-effort and `readSyncPending` keeps the debt until it
|
||||
/// lands (the same shape as every other write in this app).
|
||||
func markNotificationRead(_ notification: LocalNotification) async {
|
||||
guard !notification.isRead else { return }
|
||||
notification.markRead()
|
||||
try? modelContext?.save()
|
||||
refreshUnreadNotificationCount()
|
||||
await pushNotificationReadState()
|
||||
}
|
||||
|
||||
/// Mark every unread notification read.
|
||||
func markAllNotificationsRead() async {
|
||||
guard let context = modelContext else { return }
|
||||
let all = (try? context.fetch(FetchDescriptor<LocalNotification>())) ?? []
|
||||
let unread = all.filter { !$0.isRead }
|
||||
guard !unread.isEmpty else { return }
|
||||
for row in unread { row.markRead() }
|
||||
try? context.save()
|
||||
refreshUnreadNotificationCount()
|
||||
await pushNotificationReadState()
|
||||
}
|
||||
|
||||
/// Push any locally-read notifications the server does not know about yet.
|
||||
///
|
||||
/// Runs on every sync as well as immediately after a read action, so a
|
||||
/// notification opened in airplane mode still clears the web badge once the
|
||||
/// iPad reconnects. `markNotificationsRead` was dead code before this.
|
||||
func pushNotificationReadState() async {
|
||||
guard isOnline, AuthManager.shared.isAuthenticated,
|
||||
let context = modelContext
|
||||
else { return }
|
||||
|
||||
let all = (try? context.fetch(FetchDescriptor<LocalNotification>())) ?? []
|
||||
let pending = all.filter { $0.readSyncPending }
|
||||
guard !pending.isEmpty else { return }
|
||||
|
||||
do {
|
||||
try await APIClient.shared.markNotificationsRead(ids: pending.map { $0.serverId })
|
||||
for row in pending { row.readSyncPending = false }
|
||||
try? context.save()
|
||||
} catch {
|
||||
// Left pending — retried on the next sync. The row already reads as
|
||||
// read locally, which is what the inspector asked for.
|
||||
}
|
||||
}
|
||||
|
||||
/// Drop read notifications older than the retention window.
|
||||
///
|
||||
/// Needed because nothing else ever deletes a row: the poll endpoint cannot
|
||||
/// tell us a notification is gone (it only returns unread), so without this
|
||||
/// the inbox would grow without bound. Unread rows are never pruned however
|
||||
/// old — an unread alert is outstanding work.
|
||||
private static let notificationRetention: TimeInterval = 30 * 24 * 3600 // 30 days
|
||||
|
||||
private func pruneReadNotifications(context: ModelContext) {
|
||||
let cutoff = Date().addingTimeInterval(-Self.notificationRetention)
|
||||
let all = (try? context.fetch(FetchDescriptor<LocalNotification>())) ?? []
|
||||
var removed = 0
|
||||
for row in all where row.isRead && !row.readSyncPending && row.createdAt < cutoff {
|
||||
context.delete(row)
|
||||
removed += 1
|
||||
}
|
||||
if removed > 0 {
|
||||
try? context.save()
|
||||
print("[JQC] Sync | pruneReadNotifications | removed \(removed) row(s)")
|
||||
}
|
||||
}
|
||||
|
||||
// ── Local notification delivery ───────────────────────────────────────
|
||||
|
||||
private func deliverLocalNotification(_ n: APINotification) {
|
||||
@@ -284,6 +386,9 @@ class SyncManager: ObservableObject {
|
||||
// for the 60-second timer — ensures the inspector sees assignments
|
||||
// and follow-up requests as soon as the app goes online.
|
||||
await pollNotifications()
|
||||
// Drain read state marked while offline, then trim the inbox.
|
||||
await pushNotificationReadState()
|
||||
pruneReadNotifications(context: context)
|
||||
|
||||
// Fetch dashboard KPIs — best-effort, non-fatal on failure.
|
||||
await fetchDashboardStats()
|
||||
@@ -311,6 +416,41 @@ class SyncManager: ObservableObject {
|
||||
// string literals against PendingPhoto.uploadStatus reliably
|
||||
// when the predicate type is inferred across model boundaries.
|
||||
guard let allPhotos = try? context.fetch(FetchDescriptor<PendingPhoto>()) else { return }
|
||||
|
||||
// ── Heal stale container paths FIRST ──────────────────────────────
|
||||
//
|
||||
// Stored paths are absolute and embed the app-container UUID, which iOS
|
||||
// reassigns on every app update, reinstall and restore. The files
|
||||
// survive in Documents/; the paths do not. `uploadPhoto` then fails on
|
||||
// `FileManager.contents(atPath:)` and can NEVER succeed however often it
|
||||
// is retried, because the path names a container that no longer exists.
|
||||
//
|
||||
// This is what stranded inspection #887's nine photos: reported as
|
||||
// `upload: pending`, 0 rows failed, every file present on disk under the
|
||||
// CURRENT container. Retrying was futile; re-resolving is all that was
|
||||
// ever needed. See Utils/PhotoStore.swift and rule 91.
|
||||
//
|
||||
// Runs before the "pending" filter on purpose, so a row already given up
|
||||
// on is REVIVED rather than left stranded — otherwise a single app
|
||||
// update permanently costs an inspection its evidence.
|
||||
var healed = 0
|
||||
for photo in allPhotos where photo.uploadStatus != "uploaded" {
|
||||
guard let live = PhotoStore.resolve(photo.localFilePath),
|
||||
live != photo.localFilePath
|
||||
else { continue }
|
||||
photo.localFilePath = live
|
||||
// Previous failures were about a path that no longer applies, so
|
||||
// they are not evidence about this one — reset the retry budget.
|
||||
photo.uploadRetryCount = 0
|
||||
photo.lastUploadError = nil
|
||||
if photo.uploadStatus == "failed" { photo.uploadStatus = "pending" }
|
||||
healed += 1
|
||||
}
|
||||
if healed > 0 {
|
||||
try? context.save()
|
||||
print("[JQC] Sync | processPhotoQueue | re-resolved \(healed) stale photo path(s)")
|
||||
}
|
||||
|
||||
let pending = allPhotos
|
||||
.filter { $0.uploadStatus == "pending" }
|
||||
.sorted { $0.createdAt < $1.createdAt }
|
||||
@@ -351,6 +491,18 @@ class SyncManager: ObservableObject {
|
||||
var uploadedPaths: [String: String] = [:] // localFilePath -> serverPath
|
||||
|
||||
for photo in toUpload {
|
||||
// Paths were healed above, so an unresolvable one here means the
|
||||
// file is genuinely gone from every container. Fail fast rather
|
||||
// than burning five attempts and five sync cycles on it.
|
||||
guard PhotoStore.resolve(photo.localFilePath) != nil else {
|
||||
photo.uploadStatus = "failed"
|
||||
photo.uploadRetryCount = Self.maxPhotoUploadAttempts
|
||||
photo.lastUploadError = "File no longer on this device: "
|
||||
+ PhotoStore.filename(of: photo.localFilePath)
|
||||
try? context.save()
|
||||
continue
|
||||
}
|
||||
|
||||
do {
|
||||
// Capture metadata was recorded at the shutter, not now — the
|
||||
// sync may run hours after an offline capture, and the server
|
||||
@@ -546,15 +698,31 @@ class SyncManager: ObservableObject {
|
||||
.filter { $0.status == "completed" && $0.syncStatus == "pending" }
|
||||
.sorted { $0.createdAt < $1.createdAt }
|
||||
|
||||
// Photos are matched by entityLocalId rather than navigated to via the
|
||||
// relationship — see the photosReady guard below for why.
|
||||
let allPhotos = (try? context.fetch(FetchDescriptor<PendingPhoto>())) ?? []
|
||||
|
||||
for inspection in pending {
|
||||
// "failed" is only reachable after maxPhotoUploadAttempts, so this
|
||||
// now means "uploaded, or genuinely unrecoverable" rather than
|
||||
// means "uploaded, or genuinely unrecoverable" rather than
|
||||
// "uploaded, or hit one network error". A still-retrying photo
|
||||
// keeps its row "pending" and holds the inspection back — which is
|
||||
// the point: submitting first is what blanked the field for good,
|
||||
// since APIClient.submitInspection rewrites a surviving local://
|
||||
// value to "" and the inspection is then marked synced forever.
|
||||
let photosReady = inspection.pendingPhotos.allSatisfy {
|
||||
//
|
||||
// Matched by entityLocalId, NOT via `inspection.pendingPhotos`.
|
||||
// That relationship declares no explicit inverse (rule 37, which
|
||||
// LocalIssue follows and PendingPhoto does not), so it is not a
|
||||
// trustworthy source of truth here — and an EMPTY array makes
|
||||
// `allSatisfy` vacuously true, which silently converts this guard
|
||||
// into no guard at all and submits the inspection with every photo
|
||||
// still pending. processPhotoQueue and processIssueQueue already
|
||||
// query globally; this is now consistent with them.
|
||||
let ownPhotos = allPhotos.filter {
|
||||
$0.entityType == "inspection" && $0.entityLocalId == inspection.localId
|
||||
}
|
||||
let photosReady = ownPhotos.allSatisfy {
|
||||
$0.uploadStatus == "uploaded" || $0.uploadStatus == "failed"
|
||||
}
|
||||
guard photosReady else { continue }
|
||||
@@ -854,14 +1022,22 @@ class SyncManager: ObservableObject {
|
||||
guard last == nil || Date().timeIntervalSince(last!) >= Self.cleanupInterval else { return }
|
||||
UserDefaults.standard.set(Date(), forKey: Self.lastCleanupKey)
|
||||
|
||||
// ── Phase 1: collect referenced paths on @MainActor (SwiftData fetches) ──
|
||||
// These are fast in-memory operations — always runs on the main actor.
|
||||
var referencedPaths = Set<String>()
|
||||
// ── Phase 1: collect referenced FILENAMES on @MainActor ───────────────
|
||||
// Filenames, not paths. Stored paths are absolute and embed the app
|
||||
// container UUID, which iOS changes on every app update — so after an
|
||||
// update every reference would fail to match its own file on disk and
|
||||
// this sweep would delete the lot, including photos still awaiting
|
||||
// upload. Filenames are per-save UUIDs and survive the move (PhotoStore).
|
||||
var referencedNames = Set<String>()
|
||||
func reference(_ storedPath: String) {
|
||||
let name = PhotoStore.filename(of: storedPath)
|
||||
if !name.isEmpty { referencedNames.insert(name) }
|
||||
}
|
||||
|
||||
// PendingPhoto — not yet uploaded
|
||||
if let pendingPhotos = try? context.fetch(FetchDescriptor<PendingPhoto>()) {
|
||||
for p in pendingPhotos where p.uploadStatus != "uploaded" {
|
||||
referencedPaths.insert(p.localFilePath)
|
||||
reference(p.localFilePath)
|
||||
}
|
||||
}
|
||||
// LocalInspection — EVERY field still holding the local:// sentinel,
|
||||
@@ -877,7 +1053,7 @@ class SyncManager: ObservableObject {
|
||||
for insp in inspections {
|
||||
for val in insp.formData.values {
|
||||
if let s = val as? String, s.hasPrefix("local://") {
|
||||
referencedPaths.insert(String(s.dropFirst("local://".count)))
|
||||
reference(String(s.dropFirst("local://".count)))
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -888,14 +1064,14 @@ class SyncManager: ObservableObject {
|
||||
// does not have.
|
||||
if let issues = try? context.fetch(FetchDescriptor<LocalIssue>()) {
|
||||
for issue in issues {
|
||||
for path in issue.photoLocalPaths { referencedPaths.insert(path) }
|
||||
for path in issue.photoLocalPaths { reference(path) }
|
||||
}
|
||||
}
|
||||
|
||||
// ── Phase 2: FileManager enumeration + deletion on a background thread ──
|
||||
// Directory enumeration and file removal are I/O-bound and can stutter
|
||||
// the main thread when JQCPhotos/ contains hundreds of files. Dispatching
|
||||
// here is safe because `referencedPaths` is a value type (Set<String>)
|
||||
// here is safe because `referencedNames` is a value type (Set<String>)
|
||||
// captured by copy — no shared mutable state crosses the boundary.
|
||||
let minAge = Self.cleanupMinFileAge
|
||||
Task.detached(priority: .utility) {
|
||||
@@ -921,7 +1097,8 @@ class SyncManager: ObservableObject {
|
||||
let cutoff = Date().addingTimeInterval(-minAge)
|
||||
var deletedCount = 0
|
||||
for fileURL in diskFiles {
|
||||
if referencedPaths.contains(fileURL.path) { continue }
|
||||
// Matched by filename — see the Phase 1 comment.
|
||||
if referencedNames.contains(fileURL.lastPathComponent) { continue }
|
||||
// Age floor — never touch a file young enough to belong to a
|
||||
// capture flow that has not yet written its record.
|
||||
let modified = (try? fileURL.resourceValues(
|
||||
@@ -1278,6 +1455,9 @@ class SyncManager: ObservableObject {
|
||||
deleteAll(LocalTemplate.self, from: context)
|
||||
deleteAll(LocalScheduledInspection.self, from: context)
|
||||
deleteAll(LocalFollowUpRequest.self, from: context)
|
||||
// Notifications are addressed to one user — the most personal thing in
|
||||
// the store, and the clearest thing another inspector must never see.
|
||||
deleteAll(LocalNotification.self, from: context)
|
||||
|
||||
// ── Issues ────────────────────────────────────────────────────────
|
||||
// All of them, unconditionally. LocalIssue carries no author field, so
|
||||
@@ -1302,7 +1482,7 @@ class SyncManager: ObservableObject {
|
||||
// Remove the JPEGs too — cleanupOrphanedPhotos would otherwise
|
||||
// wait out its 7-day age floor holding another user's evidence.
|
||||
for photo in insp.pendingPhotos {
|
||||
try? FileManager.default.removeItem(atPath: photo.localFilePath)
|
||||
PhotoStore.remove(at: photo.localFilePath)
|
||||
}
|
||||
context.delete(insp) // cascades pendingPhotos + localIssues
|
||||
}
|
||||
@@ -1317,7 +1497,7 @@ class SyncManager: ObservableObject {
|
||||
let stillOwned = photo.entityType == "inspection"
|
||||
&& keptInspectionIds.contains(photo.entityLocalId)
|
||||
if !stillOwned {
|
||||
try? FileManager.default.removeItem(atPath: photo.localFilePath)
|
||||
PhotoStore.remove(at: photo.localFilePath)
|
||||
context.delete(photo)
|
||||
}
|
||||
}
|
||||
@@ -1331,7 +1511,6 @@ class SyncManager: ObservableObject {
|
||||
dashboardStats = nil
|
||||
lastNotificationFetch = nil
|
||||
unreadNotificationCount = 0
|
||||
recentNotifications = []
|
||||
syncError = nil
|
||||
lastSyncAt = nil
|
||||
updatePendingCount(context: context)
|
||||
|
||||
Reference in New Issue
Block a user