Query Property Wrappers, HealthChart, and Other Refactoring #27
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Remove `CollectSamples`. The same functionality can be achieved using a for loop in the configuration builder. - Rework the HealthKit auth handling. We still cannot access the read auth request responses, but there's also no point in somehow trying to keep track of which type's we've already requested access to, so we now just keep track of _whether_ we've already requested access (which is the same thing). - Add an API for requesting write access - Rework the `HealthKitSampleType` to be more lean and generic (and to better support the individual types) - Add a couple more sample types
…rs until their respective sample types have been authorized
in the previous approach, it would happen that if you were observing let's say 5 different sample types using eg `CollectSample`, it would "enable" all of these in a for loop, and each would then request authorization to its respective sample type. but since they kinda all were starting their queries at about the same time, you'd get a situation where only the first one to run would be able to present the HealthKit data access request sheet, and all other ones would fail to request access.
The new approach works as follows: we try to combine all Health data we need to access into a single auth request, and we no longer automatically request access as part of e.g. starting a background CollectSample observer. instead, we give the user the responsibility to call HealthKit.askForAuthorization at some point. the (automatic) background queries will start once the permission request has completed (regardless of whether we actually were given access; we can't see that).
This is better in several ways:
- it avoids the issue of all background queries except the first failing to request access
- it allows the app control over when exactly the HealthKit access request should happen. (Eg: you probably want to embed this into an onboarding flow, instead of just having it happen randomly at some point, without the user being informed in advance what;s about to happen)
The simplest case (assuming no onboarding) would be that the app would simply have a `.task { try? await healthKit.askForAuthorization() }` somewhere in the root view. this would only result in the auth sheet being presented once, or when the access requirements change. otherwise, you can call this function without there being any side effects.
this sadly seems to be necessary to work around swiftlang/swift#78734 and swiftlang/swift#78735
PSchmiedmayer
approved these changes
Jan 19, 2025
There was a problem hiding this comment.
Great job with the PR and additions here @lukaskollmer!
I added some comments throughout the PR. Thank you for resolving all the remaining TODOs in this PR or we start to transform some of them into GitHub issues to be resolved later on in subsequent PRs.
We should also adjust the README (similar to other Spezi READMEs) to reflect the functionality & HealthChart examples. We should also copy the same content to the DocC setup & ensure that it also is having all the updated references and examples in place.
This will be a huge step forward for SpeziHealthKit and will bring it closer to a 1.0, thank you! 🚀
Sources/SpeziHealthKit/HealthKitDeliverySetting/HealthKitDeliverySetting.swift
Outdated
Show resolved
Hide resolved
Sources/SpeziHealthKit/Queries/HealthKitCharacteristicsQuery.swift
Outdated
Show resolved
Hide resolved
lukaskollmer
commented
Jan 19, 2025
PSchmiedmayer
approved these changes
Jan 20, 2025
1 task
1 task
1 task
even though the Health data store isn't available on macOS, HealthKit as a framework still is.
PSchmiedmayer
approved these changes
Jan 28, 2025
PSchmiedmayer
changed the title
PSchmiedmayer
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
query property wrappers, HealthChart, other refactoring
♻️ Current situation & Problem
The HealthKit module currently only provides access to HealthKit data via long-running anchor queries that deliver information about new/deleted objects to the app's Standard.
It does not provide any facilities for querying for past samples, or accessing health data from SwiftUI. This PR attempts to address these issues.
Furthermore, the HealthKit module is lacking an API allowing spezi users to integrate custom HealthKit permission requests into the module's permission handling (i.e., you currently can only request HealthKit access for some specific sample type by actively defining a long-running observer for that sample type).
Furthermore, this PR attempts to implement a
HealthChartview, which can display various types of queried HealthKit data as a chart.resolves #8
requires StanfordSpezi/SpeziFoundation#19
requires StanfordBDHG/XCTestExtensions#28
⚙️ Release Notes
HealthKitQueryproperty wrapperHealthKitStatisticsQueryproperty wrapperHealthKitCharacteristicQueryproperty wrapperHealthChartviewHealthKitconfiguration API to allow users to specify sample types the system should request read and/or write access toCollectSamples. The same functionality can be achieved using aforloop creating individualCollectSampleinstances.📚 Documentation
All added new and changed existing APIs are documented. The DocC structure was reworked and some key aspects of SpeziHealthKit now have dedicated article/extension pages.
✅ Testing
The new and changed APIs are tested using both "normal" unit tests and UI tests. Existing tests were kept and adapted where possible
📝 Code of Conduct & Contributing Guidelines
By submitting creating this pull request, you agree to follow our Code of Conduct and Contributing Guidelines: