Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 47 additions & 4 deletions Strand/Data/CaffeineLog.swift
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,19 @@ public struct CaffeineIntake: Codable, Equatable, Identifiable, Sendable {
public let at: Date
/// Amount in mg, if the user gave one. nil = "logged it, didn't say how much" — we never invent a number.
public let mg: Double?
/// The Apple Health sample this came from, or nil when the user logged it here (#949).
///
/// Optional so old stored JSON — which has no such field — still decodes, with every existing intake
/// correctly reading back as hand-logged. It carries the HealthKit sample UUID, which is what lets a
/// re-import recognise a drink it already knows rather than logging a second copy of it.
public let externalId: String?

public init(id: UUID = UUID(), at: Date, mg: Double? = nil) {
self.id = id; self.at = at; self.mg = mg
/// True when this came from Apple Health rather than from a tap in NOOP. Imported intakes are not
/// editable here — the app that logged the drink owns it.
public var isImported: Bool { externalId != nil }

public init(id: UUID = UUID(), at: Date, mg: Double? = nil, externalId: String? = nil) {
self.id = id; self.at = at; self.mg = mg; self.externalId = externalId
}
}

Expand Down Expand Up @@ -178,6 +188,14 @@ public struct CaffeineActiveEstimate: Equatable, Sendable {
@MainActor
public final class CaffeineLogStore: ObservableObject {

/// The process-wide store (#949).
///
/// The card used to own its own instance, which was fine while every write came from the card
/// itself. The Apple Health import writes from outside it, and two instances over one UserDefaults
/// key would mean the card kept publishing its stale in-memory array until it was rebuilt — the
/// imported intakes would be on disk and invisible. One instance, one source of truth.
public static let shared = CaffeineLogStore()

/// Logged intakes, newest first. Persisted as JSON under one UserDefaults key.
@Published public private(set) var intakes: [CaffeineIntake] { didSet { save() } }

Expand Down Expand Up @@ -210,14 +228,39 @@ public final class CaffeineLogStore: ObservableObject {
intakes = ([CaffeineIntake(at: date, mg: cleanMg)] + intakes).sorted { $0.at > $1.at }
}

/// Replace every IMPORTED intake with `imported`, leaving hand-logged ones untouched (#949).
///
/// Wholesale replacement rather than a merge, for the same reason the imported hydration row is
/// replaced rather than added to: the health store hands back the full window each time, so writing
/// it whole makes a re-import idempotent, and a drink deleted in the source app disappears here on
/// the next sync instead of being stranded. Retention pruning still happens on load.
public func replaceImported(_ imported: [CaffeineIntake]) {
let manual = intakes.filter { !$0.isImported }
let next = (manual + imported).sorted { $0.at > $1.at }
// Skip the write when nothing actually changed — `intakes` has a didSet that persists, and this
// runs on every sync, so an unconditional assignment would rewrite the JSON (and republish to
// every observing view) even on the overwhelmingly common no-op sync.
if next != intakes { intakes = next }
}

/// Remove a logged intake (the user mis-logged / wants to clear it).
///
/// Imported intakes are deliberately NOT removable: the next sync re-reads the same window from
/// Apple Health and would bring it straight back, so honouring the tap would be a lie. The UI does
/// not offer the control; this guard means a caller that tries anyway gets a no-op rather than an
/// entry that silently reappears.
public func remove(_ id: UUID) {
guard let hit = intakes.first(where: { $0.id == id }), !hit.isImported else { return }
intakes = intakes.filter { $0.id != id }
}

/// Clear every logged intake.
/// Clear every HAND-LOGGED intake.
///
/// Imported ones survive for the same reason `remove` refuses them (#949) — clearing them here would
/// only last until the next sync re-read the window, so the list would silently repopulate and the
/// button would look broken.
public func clearAll() {
intakes = []
intakes = intakes.filter { $0.isImported }
}

/// The current "still active" estimate, computed from the logged intakes at `now()`.
Expand Down
30 changes: 20 additions & 10 deletions Strand/Screens/CaffeineLogCard.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@ import StrandDesign
/// unknown (we never invent mg), the active hint covers the dose-unknown case in words, and the copy
/// states it's an estimate from what was logged.
struct CaffeineLogCard: View {
/// Single-user state owned here (UserDefaults-backed), so hosting needs no app-level injection.
@StateObject private var store = CaffeineLogStore()
/// The shared UserDefaults-backed store (#949). Shared rather than owned here so the Apple Health
/// import and this card write through the same instance — see `CaffeineLogStore.shared`.
@ObservedObject private var store = CaffeineLogStore.shared

/// Drives a live recompute of the estimate while the card is on screen (the decay is time-based).
@State private var tick = Date()
Expand Down Expand Up @@ -278,15 +279,24 @@ struct CaffeineLogCard: View {
.font(StrandFont.body)
.foregroundStyle(StrandPalette.textPrimary)
Spacer()
Button {
store.remove(intake.id)
} label: {
Image(systemName: "minus.circle.fill")
.font(StrandFont.body)
.foregroundStyle(StrandPalette.statusCritical)
// No remove control on an imported intake (#949): the next sync re-reads the same window
// from Apple Health and would bring it straight back, so offering the button would be
// offering something NOOP cannot honour. Remove it where it was logged.
if intake.isImported {
Text("Apple Health")
.font(StrandFont.caption)
.foregroundStyle(StrandPalette.textTertiary)
} else {
Button {
store.remove(intake.id)
} label: {
Image(systemName: "minus.circle.fill")
.font(StrandFont.body)
.foregroundStyle(StrandPalette.statusCritical)
}
.buttonStyle(.plain)
.accessibilityLabel("Remove caffeine intake at \(Self.timeFormatter.string(from: intake.at))")
}
.buttonStyle(.plain)
.accessibilityLabel("Remove caffeine intake at \(Self.timeFormatter.string(from: intake.at))")
}
}
}
Expand Down
227 changes: 227 additions & 0 deletions StrandTests/CaffeineImportTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,227 @@
import XCTest
@testable import Strand

/// #949 — caffeine imported from Apple Health sits in the same log as hand-entered intakes, and these
/// pin the rules that keep the two from corrupting each other.
///
/// The shape of the problem is the same one the imported hydration row solves, but harder: hydration is
/// a single day total that can simply be replaced, whereas caffeine is a LIST of individual events. So
/// imported intakes carry the HealthKit sample UUID in `externalId`, and a sync replaces the imported
/// subset wholesale — that is what makes a re-import idempotent instead of logging a second copy of
/// every coffee, and what lets a drink deleted in the source app disappear here too.
///
/// Headless: a private UserDefaults suite per test, no HealthKit, no UI.
final class CaffeineImportTests: XCTestCase {

@MainActor
private func store(_ now: Date = Date()) -> CaffeineLogStore {
let suite = UserDefaults(suiteName: "caffeine.import.test.\(UUID().uuidString)")!
suite.removePersistentDomain(forName: suite.description)
return CaffeineLogStore(defaults: suite, now: { now })
}

private func imported(_ at: Date, mg: Double, id: String) -> CaffeineIntake {
CaffeineIntake(at: at, mg: mg, externalId: id)
}

/// A fixed stand-in for a HealthKit sample uuid.
private let sampleUUID = UUID(uuidString: "11111111-2222-3333-4444-555555555555")!

/// Builds an intake exactly the way `HealthKitBridge.collectCaffeine` does — the sample's own uuid
/// as BOTH the identity and the external marker. Kept in one place so a drift between this and the
/// bridge is a one-line fix rather than a silently passing test.
private func fromSample(uuid: UUID, at: Date, mg: Double) -> CaffeineIntake {
CaffeineIntake(id: uuid, at: at, mg: mg, externalId: uuid.uuidString)
}

// MARK: - the model

@MainActor
func testAHandLoggedIntakeIsNotImported() {
XCTAssertFalse(CaffeineIntake(at: Date(), mg: 80).isImported)
}

@MainActor
func testAnIntakeWithAnExternalIdIsImported() {
XCTAssertTrue(imported(Date(), mg: 80, id: "abc").isImported)
}

/// Stored JSON written before this change has no `externalId` field at all. It must still decode, and
/// every existing intake must read back as HAND-LOGGED — not as an import that the next sync would
/// then feel free to delete.
@MainActor
func testOldJsonWithoutExternalIdDecodesAsHandLogged() throws {
let json = #"[{"id":"\#(UUID().uuidString)","at":760000000,"mg":95}]"#
let decoded = try JSONDecoder().decode([CaffeineIntake].self, from: Data(json.utf8))
XCTAssertEqual(decoded.count, 1)
XCTAssertNil(decoded[0].externalId)
XCTAssertFalse(decoded[0].isImported)
}

// MARK: - replaceImported

@MainActor
func testImportedIntakesAreAdded() {
let now = Date()
let s = store(now)
s.replaceImported([imported(now.addingTimeInterval(-3600), mg: 95, id: "a")])
XCTAssertEqual(s.intakes.count, 1)
XCTAssertEqual(s.intakes[0].mg, 95)
}

/// The core idempotency claim: syncing the same Health window twice leaves ONE intake, not two.
///
/// Built from a fresh `CaffeineIntake` each time rather than reusing one value, because that is what
/// the bridge actually does — it rebuilds every intake from the HealthKit samples on each sync. An
/// earlier version of this test reused a single value and would have passed while the real path was
/// broken.
@MainActor
func testReimportingTheSameSampleDoesNotDuplicateIt() {
let now = Date()
let s = store(now)
let at = now.addingTimeInterval(-3600)
s.replaceImported([fromSample(uuid: sampleUUID, at: at, mg: 95)])
s.replaceImported([fromSample(uuid: sampleUUID, at: at, mg: 95)])
XCTAssertEqual(s.intakes.count, 1)
}

/// The identity of an imported intake must come from the SAMPLE, not be freshly minted per sync.
///
/// This is the bug the test above could not see: `CaffeineIntake.id` defaults to `UUID()`, so
/// building one per sync gave the same coffee a new identity every time — `replaceImported`'s
/// "nothing changed" guard never matched (a pointless rewrite and republish on every sync) and
/// `ForEach` rebuilt the whole logged list. Two intakes built from one sample must be EQUAL.
@MainActor
func testTwoIntakesBuiltFromTheSameSampleAreEqual() {
let at = Date().addingTimeInterval(-3600)
XCTAssertEqual(fromSample(uuid: sampleUUID, at: at, mg: 95),
fromSample(uuid: sampleUUID, at: at, mg: 95))
}

/// ...and the id is the sample's, so SwiftUI sees a stable row across syncs.
@MainActor
func testImportedIntakeIdIsTheSampleId() {
let intake = fromSample(uuid: sampleUUID, at: Date(), mg: 95)
XCTAssertEqual(intake.id, sampleUUID)
XCTAssertEqual(intake.externalId, sampleUUID.uuidString)
}

/// Re-importing an unchanged window must leave the stored array untouched, identities included.
@MainActor
func testReimportingAnUnchangedWindowKeepsTheSameIds() {
let now = Date()
let s = store(now)
let at = now.addingTimeInterval(-3600)
s.replaceImported([fromSample(uuid: sampleUUID, at: at, mg: 95)])
let before = s.intakes.map(\.id)
s.replaceImported([fromSample(uuid: sampleUUID, at: at, mg: 95)])
XCTAssertEqual(before, s.intakes.map(\.id))
}

/// Hand-logged intakes are never touched by a sync.
@MainActor
func testImportingLeavesHandLoggedIntakesAlone() {
let now = Date()
let s = store(now)
s.log(at: now.addingTimeInterval(-1800), mg: 60)
s.replaceImported([imported(now.addingTimeInterval(-3600), mg: 95, id: "a")])
XCTAssertEqual(s.intakes.count, 2)
XCTAssertEqual(s.intakes.filter { !$0.isImported }.count, 1)
XCTAssertEqual(s.intakes.filter { $0.isImported }.count, 1)
}

/// Deleting the coffee in the app that logged it removes it here on the next sync, rather than
/// leaving it stranded in NOOP forever.
@MainActor
func testAnIntakeThatVanishesFromHealthIsDropped() {
let now = Date()
let s = store(now)
s.replaceImported([imported(now.addingTimeInterval(-3600), mg: 95, id: "a"),
imported(now.addingTimeInterval(-7200), mg: 80, id: "b")])
XCTAssertEqual(s.intakes.count, 2)
s.replaceImported([imported(now.addingTimeInterval(-3600), mg: 95, id: "a")])
XCTAssertEqual(s.intakes.count, 1)
XCTAssertEqual(s.intakes[0].externalId, "a")
}

/// An empty Health window clears the imported set but must not touch what the user typed in.
@MainActor
func testAnEmptyImportClearsOnlyTheImportedOnes() {
let now = Date()
let s = store(now)
s.log(at: now.addingTimeInterval(-1800), mg: 60)
s.replaceImported([imported(now.addingTimeInterval(-3600), mg: 95, id: "a")])
s.replaceImported([])
XCTAssertEqual(s.intakes.count, 1)
XCTAssertFalse(s.intakes[0].isImported)
}

@MainActor
func testIntakesStayNewestFirstAfterAnImport() {
let now = Date()
let s = store(now)
s.log(at: now.addingTimeInterval(-7200), mg: 60)
s.replaceImported([imported(now.addingTimeInterval(-1800), mg: 95, id: "a")])
XCTAssertEqual(s.intakes.map(\.at), s.intakes.map(\.at).sorted(by: >))
}

// MARK: - imported intakes are not the user's to edit

/// Honouring this would be a lie: the next sync re-reads the same window and brings it back.
@MainActor
func testRemoveRefusesAnImportedIntake() {
let now = Date()
let s = store(now)
s.replaceImported([imported(now.addingTimeInterval(-3600), mg: 95, id: "a")])
let id = s.intakes[0].id
s.remove(id)
XCTAssertEqual(s.intakes.count, 1, "an imported intake must survive a remove")
}

@MainActor
func testRemoveStillWorksOnAHandLoggedIntake() {
let now = Date()
let s = store(now)
s.log(at: now.addingTimeInterval(-1800), mg: 60)
s.remove(s.intakes[0].id)
XCTAssertTrue(s.intakes.isEmpty)
}

/// "Clear" clears what the user owns. Wiping the imported ones would only last until the next sync.
@MainActor
func testClearAllKeepsImportedAndClearsHandLogged() {
let now = Date()
let s = store(now)
s.log(at: now.addingTimeInterval(-1800), mg: 60)
s.replaceImported([imported(now.addingTimeInterval(-3600), mg: 95, id: "a")])
s.clearAll()
XCTAssertEqual(s.intakes.count, 1)
XCTAssertTrue(s.intakes[0].isImported)
}

// MARK: - the estimate consumes imported intakes like any other

@MainActor
func testImportedIntakesFeedTheActiveEstimate() {
let now = Date()
let s = store(now)
s.replaceImported([imported(now.addingTimeInterval(-3600), mg: 100, id: "a")])
let est = s.estimate()
XCTAssertTrue(est.hasActive)
XCTAssertNotNil(est.totalRemainingMg, "an imported sample always carries a dose")
}

/// Persistence round-trip: a fresh store over the same defaults sees the imported intake, and still
/// knows it was imported.
@MainActor
func testImportedIntakesSurviveAReload() throws {
let now = Date()
let suite = UserDefaults(suiteName: "caffeine.import.test.\(UUID().uuidString)")!
let a = CaffeineLogStore(defaults: suite, now: { now })
a.replaceImported([imported(now.addingTimeInterval(-3600), mg: 95, id: "sample-1")])
let b = CaffeineLogStore(defaults: suite, now: { now })
XCTAssertEqual(b.intakes.count, 1)
XCTAssertEqual(b.intakes[0].externalId, "sample-1")
XCTAssertTrue(b.intakes[0].isImported)
}
}
Loading