workout: fix iCloud sync stomping the Watch-logged streak, remove Undo (KC-67)
Some checks failed
CI / build-and-test (push) Has been cancelled
Some checks failed
CI / build-and-test (push) Has been cancelled
Root cause: CloudSyncManager.kvStoreChanged blindly overwrote any externally-changed iCloud key, including workoutDates - a lifetime tally that every local write path (HealthKit sync, manual finish) carefully dedups/unions. A stale iCloud snapshot racing a fresh Watch-sync write could silently replace the correct array with an older, smaller one. Now unions instead of overwriting for that key specifically. Also: removed the Undo action/row from DotProgressCard entirely (Watch/ Health is the source of truth once a day is logged), and restored the confirmation dialog on Finish Workout so every state-changing action confirms first. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -2348,3 +2348,59 @@ Not build-verified (no iOS runtime here). Self-reviewed: balanced braces/
|
||||
parens confirmed per-file. Same caveat as KC-65: tasks created before that
|
||||
fix still have no real `createdAt` and fall through to the anchor/inferred
|
||||
heuristics in both widgets.
|
||||
|
||||
## KC-67 — Watch/Health workout streak got stomped by a stale iCloud sync
|
||||
|
||||
Status: Implemented (not build-verified — no iOS runtime here)
|
||||
Reported by: User — streak reset to 3 after a workout was already logged
|
||||
by the Watch, plus a request for Finish/Clear-All confirmations and to
|
||||
remove Undo
|
||||
Area: Workout (`DotProgressCard`, `WorkoutViewModel`, `CloudSyncManager`)
|
||||
|
||||
### Root cause
|
||||
`workoutDates` is a lifetime tally (KC-40 — `Set(workoutDates).count`,
|
||||
append-only by design) and every local write path already respects that:
|
||||
`logWorkoutCompleted` dedups by key before appending, `syncFromHealthKit`
|
||||
only unions HealthKit's dates in. But `CloudSyncManager.kvStoreChanged` —
|
||||
the handler for "iCloud KV store changed externally" — did a **blind
|
||||
overwrite**: `UserDefaults.kisani.set(val, forKey: key)` for every changed
|
||||
key, no merge. Since `save()` pushes `workoutDates` to iCloud on every
|
||||
write (including the Watch/HealthKit sync path), a stale iCloud snapshot
|
||||
arriving after a fresh local write (races are inherent to `NSUbiquitous
|
||||
KeyValueStore` — it's eventually consistent) would silently replace the
|
||||
correct, larger local array with an older, smaller one — exactly matching
|
||||
"the workout was already logged by the Watch, then tapping Finish reset
|
||||
the streak."
|
||||
|
||||
### Change
|
||||
- `CloudSyncManager.kvStoreChanged`: for keys matching
|
||||
`kisani.workout.completedDates.*`, union the incoming iCloud value with
|
||||
whatever's already local instead of overwriting — an external change can
|
||||
now only ever add days, never erase ones this device already knows
|
||||
about. Every other synced key keeps the existing overwrite behavior
|
||||
(unaffected — this tally is the only append-only-by-design value among
|
||||
them).
|
||||
- `DotProgressCard`: removed the `onUndo` action, its row, and the
|
||||
`isDoneToday`/`isMissedToday` branch that showed it — the done/missed
|
||||
state now shows the status header + progress dots with no action row
|
||||
underneath at all (per "remove the Undo action from the logged workout
|
||||
state"). Watch/Health is the source of truth once a day is logged;
|
||||
there's no in-card way to contest that.
|
||||
- Removed `onUndo:` at the `DotProgressCard(...)` call site in
|
||||
`WorkoutView.swift`, and deleted `WorkoutViewModel.unmarkWorkoutDone`/
|
||||
`unmarkWorkoutMissed` (now dead — Undo was their only caller).
|
||||
- Re-added the confirmation dialog on "Finish Workout" (previously
|
||||
reverted in KC-63's follow-up at the user's request; re-requested here
|
||||
alongside the streak fix) — every state-changing action on the card
|
||||
(Finish, Check All, Clear All, Didn't Train) now confirms first.
|
||||
|
||||
Files: `KisaniCal/Managers/CloudSyncManager.swift`,
|
||||
`KisaniCal/Views/WorkoutView.swift`, `KisaniCal/Models/ExerciseModels.swift`.
|
||||
|
||||
### Note
|
||||
Not build-verified (no iOS runtime here). Self-reviewed: balanced braces/
|
||||
parens confirmed per-file. Structurally, the action row (with the Finish
|
||||
button) is only ever rendered when `isDoneToday`/`isMissedToday` are both
|
||||
false, so a watch-logged day already hides Finish entirely rather than
|
||||
needing a runtime guard against double-logging — the real gap was the
|
||||
CloudSync race above, not the button itself.
|
||||
|
||||
@@ -84,7 +84,17 @@ final class CloudSyncManager {
|
||||
else { return }
|
||||
|
||||
for key in changedKeys {
|
||||
if let val = kv.object(forKey: key) {
|
||||
guard let val = kv.object(forKey: key) else { continue }
|
||||
if key.hasPrefix("kisani.workout.completedDates."), let incoming = val as? [String] {
|
||||
// Workout dates are an append-only lifetime tally, and the Watch/
|
||||
// HealthKit sync writes here independently of this device — a
|
||||
// blind overwrite can race a fresh local write with a stale
|
||||
// iCloud snapshot and silently drop already-logged days. Union
|
||||
// instead, so an external change can only ever add days, never
|
||||
// erase ones this device already knows about.
|
||||
let existing = Set(UserDefaults.kisani.stringArray(forKey: key) ?? [])
|
||||
UserDefaults.kisani.set(Array(existing.union(incoming)), forKey: key)
|
||||
} else {
|
||||
UserDefaults.kisani.set(val, forKey: key)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -663,20 +663,6 @@ class WorkoutViewModel: ObservableObject {
|
||||
save()
|
||||
}
|
||||
|
||||
/// Undo a "done" mark for a specific day — back to pending. Does NOT mark it
|
||||
/// missed (that's a separate, explicit choice). Logged set data is kept.
|
||||
func unmarkWorkoutDone(on date: Date) {
|
||||
let key = iso.string(from: date)
|
||||
workoutDates.removeAll { $0 == key }
|
||||
save()
|
||||
}
|
||||
|
||||
/// Undo a "missed" mark — back to pending, without claiming it was done.
|
||||
func unmarkWorkoutMissed(on date: Date) {
|
||||
let key = iso.string(from: date)
|
||||
missedDates.removeAll { $0 == key }
|
||||
save()
|
||||
}
|
||||
|
||||
// MARK: - Compensation (recover a missed workout on a rest day)
|
||||
|
||||
|
||||
@@ -146,13 +146,7 @@ struct WorkoutView: View {
|
||||
onFinish: { withAnimation(KisaniSpring.snappy) { vm.markWorkoutDone(on: Date()) } },
|
||||
onMarkAllSets: { withAnimation(KisaniSpring.snappy) { vm.setAllSetsDone(true) } },
|
||||
onClearAllSets: { withAnimation(KisaniSpring.snappy) { vm.setAllSetsDone(false) } },
|
||||
onNotCompleted: { withAnimation(KisaniSpring.snappy) { vm.markWorkoutMissed(on: Date()) } },
|
||||
onUndo: {
|
||||
withAnimation(KisaniSpring.snappy) {
|
||||
if vm.isWorkoutMissed(on: Date()) { vm.unmarkWorkoutMissed(on: Date()) }
|
||||
else { vm.unmarkWorkoutDone(on: Date()) }
|
||||
}
|
||||
}
|
||||
onNotCompleted: { withAnimation(KisaniSpring.snappy) { vm.markWorkoutMissed(on: Date()) } }
|
||||
)
|
||||
.listRowBackground(Color.clear)
|
||||
.listRowSeparator(.hidden)
|
||||
@@ -604,7 +598,6 @@ struct DotProgressCard: View {
|
||||
let onMarkAllSets: () -> Void
|
||||
let onClearAllSets: () -> Void
|
||||
let onNotCompleted: () -> Void
|
||||
let onUndo: () -> Void
|
||||
|
||||
/// Every quick action that changes today's state is confirmed first — a
|
||||
/// stray tap here shouldn't silently rewrite the day.
|
||||
@@ -648,9 +641,10 @@ struct DotProgressCard: View {
|
||||
DotGridProgress(progress: total > 0 ? progress : 0, cs: cs)
|
||||
}
|
||||
|
||||
if isDoneToday || isMissedToday {
|
||||
undoRow
|
||||
} else {
|
||||
// Watch/Health is the source of truth once today is logged — no
|
||||
// in-card way to undo that from here; the set checklist above is
|
||||
// only for confirming sets, not for contesting the logged day.
|
||||
if !(isDoneToday || isMissedToday) {
|
||||
actionRow
|
||||
}
|
||||
}
|
||||
@@ -740,7 +734,7 @@ struct DotProgressCard: View {
|
||||
private var actionRow: some View {
|
||||
VStack(spacing: 8) {
|
||||
Button {
|
||||
if done == total && total > 0 { onFinish() } else { pendingAction = .finish }
|
||||
pendingAction = .finish
|
||||
} label: {
|
||||
Label(done == total && total > 0 ? "Finish Workout" : "Finish Anyway",
|
||||
systemImage: "checkmark.circle.fill")
|
||||
@@ -789,16 +783,6 @@ struct DotProgressCard: View {
|
||||
}
|
||||
}
|
||||
|
||||
private var undoRow: some View {
|
||||
HStack {
|
||||
Spacer()
|
||||
Button(action: onUndo) {
|
||||
Label("Undo", systemImage: "arrow.uturn.backward")
|
||||
.font(AppFonts.sans(11, weight: .semibold))
|
||||
}
|
||||
.foregroundColor(AppColors.accent)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// MARK: - Workout Stats
|
||||
|
||||
Reference in New Issue
Block a user