workout: restore confirmed Undo for manually-logged days only (KC-68)
KC-67 removed Undo entirely on the reasoning that Watch/Health is the source of truth once a day is logged - correct for a Watch-detected day, but it also blocked undoing a day logged manually via the in-app Finish button, leaving no way back (hit while testing KC-67 itself). Added manualWorkoutDates to track which completions came from the app's own Finish button vs HealthKit sync, and only offer Undo (now confirmed via a dialog) for those. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -2404,3 +2404,54 @@ button) is only ever rendered when `isDoneToday`/`isMissedToday` are both
|
|||||||
false, so a watch-logged day already hides Finish entirely rather than
|
false, so a watch-logged day already hides Finish entirely rather than
|
||||||
needing a runtime guard against double-logging — the real gap was the
|
needing a runtime guard against double-logging — the real gap was the
|
||||||
CloudSync race above, not the button itself.
|
CloudSync race above, not the button itself.
|
||||||
|
|
||||||
|
## KC-68 — Bring back Undo, but only for manually-logged days
|
||||||
|
|
||||||
|
Status: Implemented (not build-verified — no iOS runtime here)
|
||||||
|
Reported by: User — tested Finish Workout to confirm KC-67 and got stuck
|
||||||
|
with no way back since KC-67 removed Undo entirely
|
||||||
|
Area: Workout (`DotProgressCard`, `WorkoutViewModel`)
|
||||||
|
|
||||||
|
### Description
|
||||||
|
KC-67 removed Undo outright on the reasoning "Watch/Health is the source of
|
||||||
|
truth once a day is logged." That's correct for a Watch/Health-detected
|
||||||
|
day, but it also blocked undoing a day the user logged manually through
|
||||||
|
the in-app Finish button (e.g. while testing) — there was no way back
|
||||||
|
short of waiting for a new calendar day. User wants Undo back, but only
|
||||||
|
when the day wasn't Watch/Health-sourced, and confirmed first.
|
||||||
|
|
||||||
|
### Change
|
||||||
|
- `workoutDates` (the lifetime tally) didn't previously track *how* a day
|
||||||
|
was logged. Added `manualWorkoutDates: [String]` (own storage key
|
||||||
|
`kisani.workout.manualDates.<uid>`, loaded/saved/pushed to iCloud
|
||||||
|
alongside the other workout keys) — the subset of `workoutDates` logged
|
||||||
|
via the in-app Finish button.
|
||||||
|
- `logWorkoutCompleted` (only ever called from the manual `markWorkoutDone`
|
||||||
|
path, never from `syncFromHealthKit`) now also records the date into
|
||||||
|
`manualWorkoutDates`. If the day is already in `workoutDates` (e.g.
|
||||||
|
Watch/Health got there first), the existing dedup guard makes this whole
|
||||||
|
function a no-op — so a Watch-logged day can never retroactively become
|
||||||
|
"manual."
|
||||||
|
- Added `WorkoutViewModel.isManuallyLogged(on:)` and restored
|
||||||
|
`unmarkWorkoutDone(on:)` (removes from both `workoutDates` and
|
||||||
|
`manualWorkoutDates`).
|
||||||
|
- `DotProgressCard` gained `isManuallyLogged`/`onUndo` params back. Undo is
|
||||||
|
now shown only when `isDoneToday && isManuallyLogged` (a Watch/Health day
|
||||||
|
still shows nothing extra below the dots, per KC-67), and tapping it
|
||||||
|
goes through a new `.undo` case in the existing `confirmationDialog`
|
||||||
|
("Undo today's logged workout? ... won't affect anything recorded by
|
||||||
|
Health or your Watch") rather than acting immediately — the bare Undo
|
||||||
|
button KC-67 removed had no confirmation at all.
|
||||||
|
|
||||||
|
Files: `KisaniCal/Models/ExerciseModels.swift`,
|
||||||
|
`KisaniCal/Views/WorkoutView.swift`.
|
||||||
|
|
||||||
|
### Note
|
||||||
|
Not build-verified (no iOS runtime here). Self-reviewed: balanced braces/
|
||||||
|
parens confirmed per-file. Deliberately did NOT extend the KC-67 iCloud
|
||||||
|
union-merge fix to the new `manualDates` key — unlike the lifetime tally,
|
||||||
|
this set is meant to shrink when the user undoes, and merging it
|
||||||
|
across-devices-only-grows would make an undo silently un-do itself on
|
||||||
|
another device. Worst case for this key without that protection is a rare
|
||||||
|
multi-device timing quirk in whether Undo is offered, not a data-integrity
|
||||||
|
bug like KC-67's.
|
||||||
|
|||||||
@@ -233,6 +233,10 @@ class WorkoutViewModel: ObservableObject {
|
|||||||
@Published var activeProgramId: UUID
|
@Published var activeProgramId: UUID
|
||||||
@Published var activeSections: [WorkoutSection]
|
@Published var activeSections: [WorkoutSection]
|
||||||
@Published var workoutDates: [String] = []
|
@Published var workoutDates: [String] = []
|
||||||
|
/// Subset of `workoutDates` logged via the in-app Finish button rather than
|
||||||
|
/// detected from Apple Health/Watch — only these are eligible for Undo, since
|
||||||
|
/// Watch/Health is the source of truth once it has recorded a day itself.
|
||||||
|
@Published var manualWorkoutDates: [String] = []
|
||||||
@Published var missedDates: [String] = [] // days marked "didn't do it"
|
@Published var missedDates: [String] = [] // days marked "didn't do it"
|
||||||
@Published var compensations: [String: String] = [:] // rest-day "yyyy-MM-dd" → programId
|
@Published var compensations: [String: String] = [:] // rest-day "yyyy-MM-dd" → programId
|
||||||
@Published var askCompensate: MissedDay? = nil // triggers the compensation sheet
|
@Published var askCompensate: MissedDay? = nil // triggers the compensation sheet
|
||||||
@@ -324,6 +328,7 @@ class WorkoutViewModel: ObservableObject {
|
|||||||
private let kSchedule: String
|
private let kSchedule: String
|
||||||
private let kActivePid: String
|
private let kActivePid: String
|
||||||
private let kWorkoutDates: String
|
private let kWorkoutDates: String
|
||||||
|
private let kManualWorkoutDates: String
|
||||||
private let kLastActiveDay: String
|
private let kLastActiveDay: String
|
||||||
private let kWorkoutHistory: String
|
private let kWorkoutHistory: String
|
||||||
private let kMissedDates: String
|
private let kMissedDates: String
|
||||||
@@ -335,6 +340,7 @@ class WorkoutViewModel: ObservableObject {
|
|||||||
self.kSchedule = "kisani.workout.schedule.\(uid)"
|
self.kSchedule = "kisani.workout.schedule.\(uid)"
|
||||||
self.kActivePid = "kisani.workout.activePid.\(uid)"
|
self.kActivePid = "kisani.workout.activePid.\(uid)"
|
||||||
self.kWorkoutDates = "kisani.workout.completedDates.\(uid)"
|
self.kWorkoutDates = "kisani.workout.completedDates.\(uid)"
|
||||||
|
self.kManualWorkoutDates = "kisani.workout.manualDates.\(uid)"
|
||||||
self.kLastActiveDay = "kisani.workout.lastActiveDay.\(uid)"
|
self.kLastActiveDay = "kisani.workout.lastActiveDay.\(uid)"
|
||||||
self.kWorkoutHistory = "kisani.workout.history.\(uid)"
|
self.kWorkoutHistory = "kisani.workout.history.\(uid)"
|
||||||
self.kMissedDates = "kisani.workout.missedDates.\(uid)"
|
self.kMissedDates = "kisani.workout.missedDates.\(uid)"
|
||||||
@@ -346,6 +352,7 @@ class WorkoutViewModel: ObservableObject {
|
|||||||
CloudSyncManager.shared.restoreIfMissing(key: "kisani.workout.schedule.\(uid)")
|
CloudSyncManager.shared.restoreIfMissing(key: "kisani.workout.schedule.\(uid)")
|
||||||
CloudSyncManager.shared.restoreIfMissing(key: "kisani.workout.activePid.\(uid)")
|
CloudSyncManager.shared.restoreIfMissing(key: "kisani.workout.activePid.\(uid)")
|
||||||
CloudSyncManager.shared.restoreIfMissing(key: "kisani.workout.completedDates.\(uid)")
|
CloudSyncManager.shared.restoreIfMissing(key: "kisani.workout.completedDates.\(uid)")
|
||||||
|
CloudSyncManager.shared.restoreIfMissing(key: "kisani.workout.manualDates.\(uid)")
|
||||||
CloudSyncManager.shared.restoreIfMissing(key: "kisani.workout.history.\(uid)")
|
CloudSyncManager.shared.restoreIfMissing(key: "kisani.workout.history.\(uid)")
|
||||||
CloudSyncManager.shared.restoreIfMissing(key: "kisani.workout.missedDates.\(uid)")
|
CloudSyncManager.shared.restoreIfMissing(key: "kisani.workout.missedDates.\(uid)")
|
||||||
CloudSyncManager.shared.restoreIfMissing(key: "kisani.workout.compensations.\(uid)")
|
CloudSyncManager.shared.restoreIfMissing(key: "kisani.workout.compensations.\(uid)")
|
||||||
@@ -412,6 +419,7 @@ class WorkoutViewModel: ObservableObject {
|
|||||||
activeSections = blank.sections
|
activeSections = blank.sections
|
||||||
}
|
}
|
||||||
workoutDates = UserDefaults.kisani.stringArray(forKey: kWorkoutDates) ?? []
|
workoutDates = UserDefaults.kisani.stringArray(forKey: kWorkoutDates) ?? []
|
||||||
|
manualWorkoutDates = UserDefaults.kisani.stringArray(forKey: kManualWorkoutDates) ?? []
|
||||||
missedDates = UserDefaults.kisani.stringArray(forKey: kMissedDates) ?? []
|
missedDates = UserDefaults.kisani.stringArray(forKey: kMissedDates) ?? []
|
||||||
compensations = (UserDefaults.kisani.dictionary(forKey: kCompensations) as? [String: String]) ?? [:]
|
compensations = (UserDefaults.kisani.dictionary(forKey: kCompensations) as? [String: String]) ?? [:]
|
||||||
if let hData = UserDefaults.kisani.data(forKey: kWorkoutHistory),
|
if let hData = UserDefaults.kisani.data(forKey: kWorkoutHistory),
|
||||||
@@ -430,6 +438,7 @@ class WorkoutViewModel: ObservableObject {
|
|||||||
UserDefaults.kisani.set(schedDict, forKey: kSchedule)
|
UserDefaults.kisani.set(schedDict, forKey: kSchedule)
|
||||||
UserDefaults.kisani.set(activeProgramId.uuidString, forKey: kActivePid)
|
UserDefaults.kisani.set(activeProgramId.uuidString, forKey: kActivePid)
|
||||||
UserDefaults.kisani.set(workoutDates, forKey: kWorkoutDates)
|
UserDefaults.kisani.set(workoutDates, forKey: kWorkoutDates)
|
||||||
|
UserDefaults.kisani.set(manualWorkoutDates, forKey: kManualWorkoutDates)
|
||||||
UserDefaults.kisani.set(missedDates, forKey: kMissedDates)
|
UserDefaults.kisani.set(missedDates, forKey: kMissedDates)
|
||||||
UserDefaults.kisani.set(compensations, forKey: kCompensations)
|
UserDefaults.kisani.set(compensations, forKey: kCompensations)
|
||||||
CloudSyncManager.shared.push(value: missedDates, forKey: kMissedDates)
|
CloudSyncManager.shared.push(value: missedDates, forKey: kMissedDates)
|
||||||
@@ -441,6 +450,7 @@ class WorkoutViewModel: ObservableObject {
|
|||||||
CloudSyncManager.shared.push(value: schedDict, forKey: kSchedule)
|
CloudSyncManager.shared.push(value: schedDict, forKey: kSchedule)
|
||||||
CloudSyncManager.shared.push(value: activeProgramId.uuidString, forKey: kActivePid)
|
CloudSyncManager.shared.push(value: activeProgramId.uuidString, forKey: kActivePid)
|
||||||
CloudSyncManager.shared.push(value: workoutDates, forKey: kWorkoutDates)
|
CloudSyncManager.shared.push(value: workoutDates, forKey: kWorkoutDates)
|
||||||
|
CloudSyncManager.shared.push(value: manualWorkoutDates, forKey: kManualWorkoutDates)
|
||||||
}
|
}
|
||||||
|
|
||||||
// MARK: - Daily Reset
|
// MARK: - Daily Reset
|
||||||
@@ -563,6 +573,10 @@ class WorkoutViewModel: ObservableObject {
|
|||||||
snapshotDay(key) // record today's session in history
|
snapshotDay(key) // record today's session in history
|
||||||
guard !workoutDates.contains(key) else { return }
|
guard !workoutDates.contains(key) else { return }
|
||||||
workoutDates.append(key)
|
workoutDates.append(key)
|
||||||
|
// Only called from the manual Finish path (never from the HealthKit sync
|
||||||
|
// below), so a day only lands here — undo-eligible — when the app itself
|
||||||
|
// is the one claiming it, not Watch/Health.
|
||||||
|
if !manualWorkoutDates.contains(key) { manualWorkoutDates.append(key) }
|
||||||
save()
|
save()
|
||||||
let start = sessionStartDate ?? date.addingTimeInterval(-3600)
|
let start = sessionStartDate ?? date.addingTimeInterval(-3600)
|
||||||
Task { await HealthKitManager.shared.saveWorkout(start: start, end: date) }
|
Task { await HealthKitManager.shared.saveWorkout(start: start, end: date) }
|
||||||
@@ -633,6 +647,13 @@ class WorkoutViewModel: ObservableObject {
|
|||||||
workoutDates.contains(iso.string(from: date))
|
workoutDates.contains(iso.string(from: date))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// True only if THIS date's completion came from the in-app Finish button —
|
||||||
|
/// the one case where Undo is offered. A Watch/Health-detected day is never
|
||||||
|
/// undo-eligible, since that record lives outside the app.
|
||||||
|
func isManuallyLogged(on date: Date) -> Bool {
|
||||||
|
manualWorkoutDates.contains(iso.string(from: date))
|
||||||
|
}
|
||||||
|
|
||||||
// MARK: - Per-date status (Pending / Done / Missed / Compensation)
|
// MARK: - Per-date status (Pending / Done / Missed / Compensation)
|
||||||
|
|
||||||
enum WorkoutDayStatus { case none, pending, done, missed }
|
enum WorkoutDayStatus { case none, pending, done, missed }
|
||||||
@@ -655,6 +676,16 @@ class WorkoutViewModel: ObservableObject {
|
|||||||
logWorkoutCompleted(on: date) // adds to workoutDates, snapshots history, saves
|
logWorkoutCompleted(on: date) // adds to workoutDates, snapshots history, saves
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Undo a manually-logged "done" mark — back to pending. Only ever offered
|
||||||
|
/// for days in `manualWorkoutDates`; a Watch/Health-sourced day has no undo
|
||||||
|
/// path here (that record isn't this app's to remove).
|
||||||
|
func unmarkWorkoutDone(on date: Date) {
|
||||||
|
let key = iso.string(from: date)
|
||||||
|
workoutDates.removeAll { $0 == key }
|
||||||
|
manualWorkoutDates.removeAll { $0 == key }
|
||||||
|
save()
|
||||||
|
}
|
||||||
|
|
||||||
/// Mark one specific day's workout missed (undoes a done mark for that day only).
|
/// Mark one specific day's workout missed (undoes a done mark for that day only).
|
||||||
func markWorkoutMissed(on date: Date) {
|
func markWorkoutMissed(on date: Date) {
|
||||||
let key = iso.string(from: date)
|
let key = iso.string(from: date)
|
||||||
|
|||||||
@@ -146,7 +146,9 @@ 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()) } },
|
||||||
|
isManuallyLogged: vm.isManuallyLogged(on: Date()),
|
||||||
|
onUndo: { withAnimation(KisaniSpring.snappy) { vm.unmarkWorkoutDone(on: Date()) } }
|
||||||
)
|
)
|
||||||
.listRowBackground(Color.clear)
|
.listRowBackground(Color.clear)
|
||||||
.listRowSeparator(.hidden)
|
.listRowSeparator(.hidden)
|
||||||
@@ -598,11 +600,15 @@ struct DotProgressCard: View {
|
|||||||
let onMarkAllSets: () -> Void
|
let onMarkAllSets: () -> Void
|
||||||
let onClearAllSets: () -> Void
|
let onClearAllSets: () -> Void
|
||||||
let onNotCompleted: () -> Void
|
let onNotCompleted: () -> Void
|
||||||
|
/// Whether today's "done" mark came from this app's own Finish button rather
|
||||||
|
/// than Watch/Health — only manually-logged days offer an Undo (see body).
|
||||||
|
let isManuallyLogged: Bool
|
||||||
|
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.
|
||||||
private enum PendingAction: Identifiable {
|
private enum PendingAction: Identifiable {
|
||||||
case finish, checkAll, clearAll, notCompleted
|
case finish, checkAll, clearAll, notCompleted, undo
|
||||||
var id: Self { self }
|
var id: Self { self }
|
||||||
|
|
||||||
var confirmLabel: String {
|
var confirmLabel: String {
|
||||||
@@ -611,9 +617,10 @@ struct DotProgressCard: View {
|
|||||||
case .checkAll: return "Check All"
|
case .checkAll: return "Check All"
|
||||||
case .clearAll: return "Clear All"
|
case .clearAll: return "Clear All"
|
||||||
case .notCompleted: return "Mark Not Completed"
|
case .notCompleted: return "Mark Not Completed"
|
||||||
|
case .undo: return "Undo"
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
var isDestructive: Bool { self == .clearAll || self == .notCompleted }
|
var isDestructive: Bool { self == .clearAll || self == .notCompleted || self == .undo }
|
||||||
}
|
}
|
||||||
@State private var pendingAction: PendingAction?
|
@State private var pendingAction: PendingAction?
|
||||||
|
|
||||||
@@ -641,11 +648,14 @@ struct DotProgressCard: View {
|
|||||||
DotGridProgress(progress: total > 0 ? progress : 0, cs: cs)
|
DotGridProgress(progress: total > 0 ? progress : 0, cs: cs)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Watch/Health is the source of truth once today is logged — no
|
// Watch/Health is the source of truth once it has logged a day —
|
||||||
// in-card way to undo that from here; the set checklist above is
|
// no in-card way to undo that. A manually-logged day (this app's
|
||||||
// only for confirming sets, not for contesting the logged day.
|
// own Finish button) is the one case Undo is still offered for,
|
||||||
|
// confirmed first like every other state change here.
|
||||||
if !(isDoneToday || isMissedToday) {
|
if !(isDoneToday || isMissedToday) {
|
||||||
actionRow
|
actionRow
|
||||||
|
} else if isDoneToday && isManuallyLogged {
|
||||||
|
manualUndoRow
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
.padding(14)
|
.padding(14)
|
||||||
@@ -671,6 +681,7 @@ struct DotProgressCard: View {
|
|||||||
case .checkAll: return "Check every set as done?"
|
case .checkAll: return "Check every set as done?"
|
||||||
case .clearAll: return "Clear all logged sets?"
|
case .clearAll: return "Clear all logged sets?"
|
||||||
case .notCompleted: return "Mark today as not completed?"
|
case .notCompleted: return "Mark today as not completed?"
|
||||||
|
case .undo: return "Undo today's logged workout?"
|
||||||
case nil: return ""
|
case nil: return ""
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -687,6 +698,8 @@ struct DotProgressCard: View {
|
|||||||
return "This un-checks all \(total) sets. It won't undo today's logged workout."
|
return "This un-checks all \(total) sets. It won't undo today's logged workout."
|
||||||
case .notCompleted:
|
case .notCompleted:
|
||||||
return isDoneToday ? "This replaces today's logged workout with \"not completed.\"" : "You can undo this afterward."
|
return isDoneToday ? "This replaces today's logged workout with \"not completed.\"" : "You can undo this afterward."
|
||||||
|
case .undo:
|
||||||
|
return "This removes today's manually logged workout and goes back to pending. It won't affect anything recorded by Health or your Watch."
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -696,6 +709,7 @@ struct DotProgressCard: View {
|
|||||||
case .checkAll: onMarkAllSets()
|
case .checkAll: onMarkAllSets()
|
||||||
case .clearAll: onClearAllSets()
|
case .clearAll: onClearAllSets()
|
||||||
case .notCompleted: onNotCompleted()
|
case .notCompleted: onNotCompleted()
|
||||||
|
case .undo: onUndo()
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -783,6 +797,18 @@ struct DotProgressCard: View {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Only shown for a manually-logged day — tapping this always confirms first
|
||||||
|
/// (see `PendingAction.undo`), so a stray tap can't silently un-log a workout.
|
||||||
|
private var manualUndoRow: some View {
|
||||||
|
HStack {
|
||||||
|
Spacer()
|
||||||
|
Button { pendingAction = .undo } label: {
|
||||||
|
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