workout: fix iCloud sync stomping the Watch-logged streak, remove Undo (KC-67)
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
|
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
|
fix still have no real `createdAt` and fall through to the anchor/inferred
|
||||||
heuristics in both widgets.
|
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 }
|
else { return }
|
||||||
|
|
||||||
for key in changedKeys {
|
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)
|
UserDefaults.kisani.set(val, forKey: key)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -663,20 +663,6 @@ class WorkoutViewModel: ObservableObject {
|
|||||||
save()
|
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)
|
// 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()) } },
|
onFinish: { withAnimation(KisaniSpring.snappy) { vm.markWorkoutDone(on: Date()) } },
|
||||||
onMarkAllSets: { withAnimation(KisaniSpring.snappy) { vm.setAllSetsDone(true) } },
|
onMarkAllSets: { withAnimation(KisaniSpring.snappy) { vm.setAllSetsDone(true) } },
|
||||||
onClearAllSets: { withAnimation(KisaniSpring.snappy) { vm.setAllSetsDone(false) } },
|
onClearAllSets: { withAnimation(KisaniSpring.snappy) { vm.setAllSetsDone(false) } },
|
||||||
onNotCompleted: { withAnimation(KisaniSpring.snappy) { vm.markWorkoutMissed(on: Date()) } },
|
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()) }
|
|
||||||
}
|
|
||||||
}
|
|
||||||
)
|
)
|
||||||
.listRowBackground(Color.clear)
|
.listRowBackground(Color.clear)
|
||||||
.listRowSeparator(.hidden)
|
.listRowSeparator(.hidden)
|
||||||
@@ -604,7 +598,6 @@ struct DotProgressCard: View {
|
|||||||
let onMarkAllSets: () -> Void
|
let onMarkAllSets: () -> Void
|
||||||
let onClearAllSets: () -> Void
|
let onClearAllSets: () -> Void
|
||||||
let onNotCompleted: () -> Void
|
let onNotCompleted: () -> Void
|
||||||
let onUndo: () -> Void
|
|
||||||
|
|
||||||
/// Every quick action that changes today's state is confirmed first — a
|
/// Every quick action that changes today's state is confirmed first — a
|
||||||
/// stray tap here shouldn't silently rewrite the day.
|
/// stray tap here shouldn't silently rewrite the day.
|
||||||
@@ -648,9 +641,10 @@ struct DotProgressCard: View {
|
|||||||
DotGridProgress(progress: total > 0 ? progress : 0, cs: cs)
|
DotGridProgress(progress: total > 0 ? progress : 0, cs: cs)
|
||||||
}
|
}
|
||||||
|
|
||||||
if isDoneToday || isMissedToday {
|
// Watch/Health is the source of truth once today is logged — no
|
||||||
undoRow
|
// in-card way to undo that from here; the set checklist above is
|
||||||
} else {
|
// only for confirming sets, not for contesting the logged day.
|
||||||
|
if !(isDoneToday || isMissedToday) {
|
||||||
actionRow
|
actionRow
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -740,7 +734,7 @@ struct DotProgressCard: View {
|
|||||||
private var actionRow: some View {
|
private var actionRow: some View {
|
||||||
VStack(spacing: 8) {
|
VStack(spacing: 8) {
|
||||||
Button {
|
Button {
|
||||||
if done == total && total > 0 { onFinish() } else { pendingAction = .finish }
|
pendingAction = .finish
|
||||||
} label: {
|
} label: {
|
||||||
Label(done == total && total > 0 ? "Finish Workout" : "Finish Anyway",
|
Label(done == total && total > 0 ? "Finish Workout" : "Finish Anyway",
|
||||||
systemImage: "checkmark.circle.fill")
|
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
|
// MARK: - Workout Stats
|
||||||
|
|||||||
Reference in New Issue
Block a user