Skip to content

Migrate some Apple Health daily data providers to V2.#613

Open
greinard wants to merge 1 commit into
mainfrom
v2-migration
Open

Migrate some Apple Health daily data providers to V2.#613
greinard wants to merge 1 commit into
mainfrom
v2-migration

Conversation

@greinard

Copy link
Copy Markdown
Collaborator

Overview

This branch migrates a number of Apple Health daily data providers to use the V2 API. I also added a few new ones. To the extent possible, I used the V2 aggregate API.

New Providers:

  • Apple Health Blood Glucose (V2 type: Blood Glucose, aggregate: avg)
  • Apple Health Min Blood Glucose (V2 type: Blood Glucose, aggregate: min)
  • Apple Health Max Blood Glucose (V2 type: Blood Glucose, aggregate: max)
  • Apple Health Min Heart Rate (V2 type: Hourly Minimum Heart Rate, aggregate: min)
  • Health Connect Blood Glucose (V2 type: blood-glucose, aggregate: avg)
  • Health Connect Min Blood Glucose (V2 type: blood-glucose, aggregate: min)
  • Health Connect Max Blood Glucose (V2 type: blood-glucose, aggregate: max)
  • Combined Blood Glucose - Uses the Apple Health Blood Glucose and Health Connect Blood Glucose data providers, prioritizing the value from Apple Health when both are present for a given day.

Updated Providers:

  • Apple Health Max Heart Rate (V2 type: Hourly Maximum Heart Rate, aggregate: max)
  • Apple Health Heart Rate Range - Uses values from the V2 min and max heart rate providers to compute the range for each day.
  • Apple Health Mindful Minutes (V2 type: Mindful Sessions, filter: not SilverCloud CBT)
  • Apple Health Resting Heart Rate (V2 type: Resting Heart Rate, aggregate: avg)
  • Apple Health Sleep Time (V2 type: Sleep Analysis, values: ['AsleepCore', 'AsleepREM', 'AsleepDeep', 'Asleep'])
  • Apple Health Core Sleep Time (V2 type: Sleep Analysis, value: AsleepCore)
  • Apple Health REM Sleep Time (V2 type: Sleep Analysis, value: AsleepREM)
  • Apple Health Deep Sleep Time (V2 type: Sleep Analysis, value: AsleepDeep)
  • Apple Health In Bed Time (V2 type: Sleep Analysis, value: InBed)
  • Apple Health Steps (V2 type: Hourly Steps, aggregate: sum)
  • Apple Health Steps While Wearing Device (V2 type: Hourly Steps, aggregate: sum)
  • Combined Mindful Minutes - Updated the criteria for using the V2 Apple Health Mindful Minutes provider.
  • Combined Resting Heart Rate - Updated the criteria for using the V2 Apple Health Resting Heart Rate provider.
  • Combined Sleep Time - Updated the criteria for using the V2 Apple Health Sleep Time provider.
  • Combined Steps - Updated the criteria for using the V2 Apple Health Steps provider.

Security

  • I have ensured no secure credentials or sensitive information remain in code, metadata, comments, etc.
    • Please verify that you double checked that .storybook/preview.js does not contain your participant access key details.
    • There are no temporary testing changes committed such as API base URLs, access tokens, print/log statements, etc.
  • These changes do not introduce any security risks, or any such risks have been properly mitigated.

No new security risk. Just migrating to use the delegated V2 device data API, which is already accessible to participants.

Testing

  • This change can be adequately tested using the MDH Storybook.
  • This change requires additional testing in the MDH iOS/Android/Web apps. (Create a pre-release tag/build and test in a ViewBuilder PR.)

This change requires a VB PR to fully test.

  • Create a test view with a daily data chart for each of the affected daily data providers.
  • Configure the view as a tab in QA and visit it with a test participant.
  • Share device data with MDH as needed to verify the charts continue to render that data correctly.

Documentation

Consider whether there are any documentation impacts and check one of the following boxes:

  • I have added relevant Storybook updates to this PR.
  • If this feature requires a developer doc update, I have tagged @CareEvolution/api-docs.
  • This change does not impact documentation or Storybook.

@aws-amplify-us-east-1

Copy link
Copy Markdown

This pull request is automatically being deployed by Amplify Hosting (learn more).

Access this pull request here: https://pr-613.d1xp2kmk6zrv44.amplifyapp.com

@greinard greinard requested review from ajmenca, chrisnowak and zwkohn May 26, 2026 17:29

@lynnfaraday lynnfaraday left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry it took so long.
Just a few little things and suggestions.

I'm not sure how to check if all of the device type names are correct, so it would be good to have someone more familiar with device data (Vlad maybe?) glance through as well.

Comment thread locales/de.json
"average-stress-level": "Durchschnittliche Stressstufe",
"awake-time": "Wachzeit",
"back": "Zurück",
"blood-glucose": "Blutzucker",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change needs to go through the localization hub. Otherwise they're going to get out of sync and cause merge conflicts later. Happy to do that if you like, or you can try yourself. Instructions on slab.

@@ -0,0 +1,82 @@
import { describe, expect, it } from '@jest/globals';
import { sampleEndDate, sampleStartDate } from '../../../fixtures/daily-data-providers';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the setupXYX functions in the fixture, but having the actual sample data separated from the tests makes them pretty hard to follow. And since most of them are just empty dates or really simple object hashes like { "SomeDate": 100 }, I'm not sure it's worth the added obfuscation. Not a huge deal, just an observation.

if (maxHeartRate && minHeartRate) {
result[dayKey] = maxHeartRate - minHeartRate;
return Object.entries(minResult).reduce((result, [dayKey, minHeartRate]) => {
if (maxResult[dayKey] > minHeartRate) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works, but would it be better to have an explicit check that maxResult actually has an entry for that day?

import { querySleepMinutesV2 } from './common-sleep-v2';
import { DailyDataQueryResult } from '../query-daily-data';

export function totalSleepMinutes(startDate: Date, endDate: Date): Promise<DailyDataQueryResult> {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is going to be a major version bump, right? Since we're fundamentally changing the way things like inBedTime work and effectively deprecating the V1 queries...

import { DailyDataQueryResult } from '../query-daily-data';
import { queryAggregateDailyData } from './daily-data/daily-data-aggregate';

const MMOL_TO_MGDL_SCALE_FACTOR = 18;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A comment for context would be nice on this scale constant.

Ditto in the other two files.

setupDailyData('AppleHealth', 'RestingHeartRate', sampleStartDate, sampleEndDate, startDateFunctionEvaluator, sampleDailyData);
setupAverageValueResult(sampleDailyData, sampleResult);
it('Should query for aggregate daily data and return the average values keyed by date.', async () => {
setupAggregateDailyData('AppleHealth', 'Resting Heart Rate', sampleStartDate, sampleEndDate, 'avg', sampleResult);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Throughout this are data type names like "Resting Heart Rate". We have what seems to be a mapping for this in the tables like RESTING_HEART_RATE_SOURCES. It seems like it would be more clear if we used that, so instead of a literal string we have something like RESTING_HEART_RATE_SOURCES[AppleHealth].

This would apply both to the tests and to the providers themselves. So something like:

return queryAggregateDailyData('AppleHealth', 'Hourly Steps', startDate, endDate, 'sum');

would become:

return queryAggregateDailyData('AppleHealth', STEPS_SOURCES[AppleHealth], startDate, endDate, 'sum');

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants