-
Notifications
You must be signed in to change notification settings - Fork 0
Manual-entry audit trail + on-device 24h location summary #64
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
02216ac
3b9b384
b43ea52
39a3591
50a2cdf
cf29cd7
7ee60ea
5c9c868
b2a75c6
ebefc39
14b1f1a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -50,6 +50,16 @@ public final class CoreLocationSource: NSObject, LocationSource { | |
| /// thus permanently strand — the first. | ||
| private var pendingPermissionContinuations: [CheckedContinuation<Void, Error>] = [] | ||
|
|
||
| /// Waiters for an in-flight `requestCurrentLocation()`. Overlapping callers | ||
| /// coalesce onto the next delivered fix (or the shared timeout / failure); | ||
| /// only the first triggers `requestLocation()`. Every waiter is resumed | ||
| /// together, so a second caller can't strand the first. | ||
| private var pendingLocationContinuations: [CheckedContinuation<LocationSample?, Never>] = [] | ||
|
|
||
| /// How long to wait for a one-shot fix before giving up and recording no | ||
| /// captured location. Kept short so a manual entry's Save isn't held up. | ||
| private static let currentLocationTimeout: Duration = .seconds(10) | ||
|
|
||
| override public init() { | ||
| // The "create stream, capture its continuation" two-step is | ||
| // the idiomatic Swift `AsyncStream` initializer pattern when | ||
|
|
@@ -76,6 +86,42 @@ public final class CoreLocationSource: NSObject, LocationSource { | |
| manager.stopMonitoringVisits() | ||
| } | ||
|
|
||
| public func requestCurrentLocation() async -> LocationSample? { | ||
| // Best-effort: without a granted status `requestLocation()` just fails, | ||
| // so short-circuit to "no fix" rather than starting a doomed request. | ||
| switch manager.authorizationStatus { | ||
| case .authorizedAlways, .authorizedWhenInUse: | ||
| break | ||
| case .denied, .restricted, .notDetermined: | ||
| return nil | ||
| @unknown default: | ||
| return nil | ||
| } | ||
|
|
||
| return await withCheckedContinuation { continuation in | ||
| pendingLocationContinuations.append(continuation) | ||
| guard pendingLocationContinuations.count == 1 else { return } | ||
| manager.requestLocation() | ||
| // Bound the wait so a Save never hangs on a slow/absent fix. | ||
| Task { @MainActor [weak self] in | ||
| try? await Task.sleep(for: Self.currentLocationTimeout) | ||
|
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this mean we always wait for 10s? This seems non-optimal; can we instead base this on the GPS resolution we have?
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
No — 10s is only an upper bound. |
||
| self?.resolvePendingLocation(nil) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// Resume (and clear) every coalesced one-shot location waiter with the same | ||
| /// result. Cleared before resuming so a fix delivered after the timeout (or | ||
| /// vice-versa) is a no-op rather than a double-resume. | ||
| private func resolvePendingLocation(_ sample: LocationSample?) { | ||
| guard !pendingLocationContinuations.isEmpty else { return } | ||
| let waiters = pendingLocationContinuations | ||
| pendingLocationContinuations.removeAll() | ||
| for waiter in waiters { | ||
| waiter.resume(returning: sample) | ||
| } | ||
| } | ||
|
|
||
| public func currentAuthorization() async -> LocationAuthorizationStatus { | ||
| Self.map(manager.authorizationStatus) | ||
| } | ||
|
|
@@ -179,6 +225,7 @@ extension CoreLocationSource: CLLocationManagerDelegate { | |
| _: CLLocationManager, | ||
| didUpdateLocations locations: [CLLocation], | ||
| ) { | ||
| var latest: LocationSample? | ||
| for location in locations { | ||
| let sample = LocationSample( | ||
| timestamp: location.timestamp, | ||
|
|
@@ -190,6 +237,26 @@ extension CoreLocationSource: CLLocationManagerDelegate { | |
| source: .gpsSignificantChange, | ||
| ) | ||
| sampleContinuation.yield(sample) | ||
| latest = sample | ||
| } | ||
| // A one-shot `requestCurrentLocation()` is delivered here too; satisfy | ||
| // any pending waiter with the freshest fix in this batch. | ||
| if let latest { | ||
| Task { @MainActor [weak self] in | ||
| self?.resolvePendingLocation(latest) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| public nonisolated func locationManager( | ||
| _: CLLocationManager, | ||
| didFailWithError _: any Error, | ||
| ) { | ||
| // `requestLocation()` reports failures here. Resolve any one-shot waiter | ||
| // with "no fix" (best-effort audit capture) rather than leaving it to | ||
| // wait out the full timeout. | ||
| Task { @MainActor [weak self] in | ||
| self?.resolvePendingLocation(nil) | ||
| } | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -40,6 +40,16 @@ public protocol LocationSource: AnyObject, Sendable { | |
| func start() async | ||
| func stop() async | ||
|
|
||
| /// Best-effort one-shot GPS fix for "where is the device *right now*". | ||
| /// | ||
| /// Unlike the passive `sampleStream` (Visits + significant-change, which can | ||
| /// be minutes stale), this actively asks for a fresh fix — used to stamp a | ||
| /// manual entry's audit trail with where it was made. Returns `nil` rather | ||
| /// than throwing when a fix can't be obtained (permission not granted, | ||
| /// timeout, or a location error): the capture is audit metadata, so an | ||
| /// absent fix is recorded honestly instead of blocking the entry. | ||
| func requestCurrentLocation() async -> LocationSample? | ||
|
|
||
| /// The current authorization status, read on demand. | ||
| func currentAuthorization() async -> LocationAuthorizationStatus | ||
|
|
||
|
|
@@ -72,6 +82,9 @@ public final class ScriptedLocationSource: LocationSource, @unchecked Sendable { | |
|
|
||
| private let lock = NSLock() | ||
| private var _status: LocationAuthorizationStatus | ||
| /// What the next `requestCurrentLocation()` returns. Defaults to `nil` (no | ||
| /// fix available) so tests opt in to a captured location explicitly. | ||
| private var _nextRequestedLocation: LocationSample? | ||
|
|
||
| /// - Parameters: | ||
| /// - permissionResult: what the next call to `requestPermission()` | ||
|
|
@@ -94,6 +107,16 @@ public final class ScriptedLocationSource: LocationSource, @unchecked Sendable { | |
| public func start() async {} | ||
| public func stop() async {} | ||
|
|
||
| public func requestCurrentLocation() async -> LocationSample? { | ||
| lock.withLock { _nextRequestedLocation } | ||
| } | ||
|
|
||
| /// Set the fix the next `requestCurrentLocation()` will return (or `nil` to | ||
| /// simulate no fix). Mirrors how `emit(_:)` scripts the passive stream. | ||
| public func setNextRequestedLocation(_ sample: LocationSample?) { | ||
|
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this only for tests? If not, wondering if we should restructure a bit so the "passive" location is different from the otherwise observed location...
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yes — |
||
| lock.withLock { _nextRequestedLocation = sample } | ||
| } | ||
|
|
||
| public func currentAuthorization() async -> LocationAuthorizationStatus { | ||
| lock.withLock { _status } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| import Foundation | ||
|
|
||
| /// Audit metadata attached to a user-made manual day entry (a backfill or an | ||
| /// authoritative override), retained so a residency/day-count audit can later | ||
| /// answer *when* the correction was made, *why* (the note), and *where the | ||
| /// device physically was* at the time. | ||
| /// | ||
| /// This describes the *act of making the entry*, not the day it corrects — the | ||
| /// day and its regions live on `DayPresence`. `note` and `location` are each | ||
| /// independently optional: the user may leave the reason blank, and a GPS fix | ||
| /// may be unobtainable (permission/timeout), in which case the audit still | ||
| /// honestly records `recordedAt` with a `nil` `location` rather than a faked | ||
| /// coordinate. | ||
| public struct ManualEntryAudit: Hashable, Sendable, Codable { | ||
| /// When the manual entry was made (not the day it applies to). | ||
| public let recordedAt: Date | ||
| /// The user's free-text reason for the entry. `nil` (or empty) when none | ||
| /// was given. | ||
| public let note: String? | ||
| /// Where the device was when the entry was made, when a fix was available. | ||
| public let location: CapturedLocation? | ||
|
|
||
| public init(recordedAt: Date, note: String?, location: CapturedLocation?) { | ||
| self.recordedAt = recordedAt | ||
| self.note = note | ||
| self.location = location | ||
| } | ||
| } | ||
|
|
||
| /// A GPS fix captured at a single moment — the "where were you when you made | ||
| /// this entry" half of `ManualEntryAudit`. Kept as a plain value type so the | ||
| /// model layer stays CoreLocation-free (see `Coordinate`). | ||
| public struct CapturedLocation: Hashable, Sendable, Codable { | ||
| public let coordinate: Coordinate | ||
| public let horizontalAccuracy: Double | ||
| /// The timestamp reported by the location fix itself, which can differ | ||
| /// slightly from `ManualEntryAudit.recordedAt`. | ||
| public let timestamp: Date | ||
|
|
||
| public init(coordinate: Coordinate, horizontalAccuracy: Double, timestamp: Date) { | ||
| self.coordinate = coordinate | ||
| self.horizontalAccuracy = horizontalAccuracy | ||
| self.timestamp = timestamp | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Confirm we show a waiting / countdown UI for this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch — we did not. Both
DayRelabelViewandManualDayEntryViewonly disabled Save during the write. Fixed in 14b1f1a: while a save is in flight we now show a "Capturing location…" status row (spinner + label) and swap the Save button for a spinner, so the up-to-a-few-seconds GPS capture has visible feedback. No literal countdown — a spinner + label reads better than a ticking timer.DayRelabelViewalso now models the in-flight write as an enum (saving vs. resetting) so the location status only shows for the GPS-capturing save, not the quick reset.