Skip to content
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

Separate the implementation for clinical records #104

Merged
merged 17 commits into from
Oct 18, 2019

Conversation

rossmartin
Copy link
Collaborator

This PR has a few things, most notably including -

  • adds queryClinicalSampleType, queryForClinicalRecordsFromSource, and queryForClinicalRecordsWithFHIRResourceType.
  • resolves iOS app store rejection due to HKClinicalRecord #103.
  • the readme was updated with info about reading clinical data.
  • the demo was updated with usage and info on the new functions.
  • the deprecated API calls on source were fixed.

@awatson1978
Copy link
Collaborator

Cursory read through looks like clean straight-forward code. +1 to merge this in.

@awatson1978
Copy link
Collaborator

awatson1978 commented Dec 1, 2018

Well... I managed to get the plugin installed from rossmartin repo using the following in our .meteor/cordova-plugins file:

com.telerik.plugins.healthkit@https://github.com/rossmartin/HealthKit.git#5174fde920092c94fe3dd3dead64700d293d9235

Plugin compiles and launches on my iPad, with HealthKit and HealthRecord entitlements activated. :)

Going to give the new queries a try!

@awatson1978
Copy link
Collaborator

Getting the following when trying to use this PR:

Terminating app due to uncaught exception 'NSInvalidArgumentException', 
reason: 'Authorization to share the following types is disallowed: 
HKClinicalTypeIdentifierProcedureRecord, HKClinicalTypeIdentifierVitalSignRecord, HKClinicalTypeIdentifierMedicationRecord, HKClinicalTypeIdentifierAllergyRecord, HKClinicalTypeIdentifierImmunizationRecord, HKClinicalTypeIdentifierLabResultRecord, HKClinicalTypeIdentifierConditionRecord'

Filed a separate issue #106 regarding it. Am currently trying to debug.

@awatson1978
Copy link
Collaborator

awatson1978 commented Dec 2, 2018

May want to update documentation with NSHealthClinicalHealthRecordsShareUsageDescription as a third key that needs to be set in .plist.

@awatson1978
Copy link
Collaborator

awatson1978 commented Dec 2, 2018

It works!!!!!!!!!

screen shot 2018-12-02 at 10 54 44 am

Have some clean up to do, but THANK YOU THANK YOU THANK YOU!!!!

@awatson1978
Copy link
Collaborator

Hmmm.... continuing to work on this, and queryClinicalSampleType doesn't seem to be working. Keep getting the following error:

Uncaught Error: TypeError: undefined is not a function (near '...window.plugins.healthkit.queryClinicalSampleType...'):320:http://localhost:12224/__cordova/packages/symptomatic_continuity-of-care.js?hash=3b9de071618721a5e300bca132c618933c7def8f

Here's the code we're using:

      window.plugins.healthkit.queryClinicalSampleType(
        {
          'startDate': new Date(new Date().getTime() - 365 * 24 * 60 * 60 * 1000), // 365 days ago
          'endDate': new Date(), // now
          'sampleType': 'HKClinicalTypeIdentifierVitalSignRecord'
        },
        function(results){
          console.log('HKClinicalTypeIdentifierVitalSignRecord.ok', JSON.stringify(results))
        },
        function(){console.log('HKFHIRResourceTypeObservation.nok')}
      );

@awatson1978
Copy link
Collaborator

Yeah, we've been able to use queryForClinicalRecordsWithFHIRResourceType to get at just about all the resources, but haven't been able to get queryClinicalSampleType to work.

In case anybody is interest, this is the authorization scope code we had to use:

      window.plugins.healthkit.requestAuthorization(
          {
            readTypes: [
              'HKClinicalTypeIdentifierAllergyRecord',
              'HKClinicalTypeIdentifierConditionRecord',
              'HKClinicalTypeIdentifierImmunizationRecord',
              'HKClinicalTypeIdentifierLabResultRecord',
              'HKClinicalTypeIdentifierMedicationRecord',
              'HKClinicalTypeIdentifierProcedureRecord',
              'HKClinicalTypeIdentifierVitalSignRecord',

              'HKFHIRResourceTypeAllergyIntolerance',
              'HKFHIRResourceTypeImmunization',
              'HKFHIRResourceTypeMedicationDispense',
              'HKFHIRResourceTypeMedicationOrder',
              'HKFHIRResourceTypeMedicationStatement',
              'HKFHIRResourceTypeObservation',
              'HKFHIRResourceTypeProcedure'
          ],
            writeTypes: [
              'HKQuantityTypeIdentifierWeight'
            ]
          },
          function(){console.log('requestAuthorization.ok')},
          function(){console.log('requestAuthorization.nok')}
      );

@rossmartin
Copy link
Collaborator Author

@awatson1978 I believe the reason queryClinicalSampleType is not working for you is because when you installed it via meteor you are using an old commit hash.

The hash you will want to use is from the latest commit 2db3a359fd12f90ec9c0817d4b7b1bc9b8ea458b (this commit is included in the PR).

It would look like this -

com.telerik.plugins.healthkit@https://github.com/rossmartin/HealthKit.git#2db3a359fd12f90ec9c0817d4b7b1bc9b8ea458b

I think some things are being lost because this PR hasn't been merged yet and the readme here on this repo is not up to date with mine. The readme on my repo talks about the CLINICAL_READ_PERMISSION that will end up being used for the NSHealthClinicalHealthRecordsShareUsageDescription. In your case with meteor this variable may not be getting added to the app's config.xml - I had to use an after-plugin-install hook and made it append a config-file element to the app's config.xml (see here for the gory details if you want).

Thanks for testing this out and posting your findings. Let me know how it goes after you update the plugin to use that latest hash.

@rossmartin
Copy link
Collaborator Author

@awatson1978 Also you will probably need this in your meteor mobile-config.js. This is needed for the cordova hook to run and set the CLINICAL_READ_PERMISSION(the value of this variable sets NSHealthClinicalHealthRecordsShareUsageDescription).

App.configurePlugin('com.telerik.plugins.healthkit', {
  CLINICAL_READ_PERMISSION: 'App needs read access',
  HEALTH_READ_PERMISSION: 'App needs read access',
  HEALTH_WRITE_PERMISSION: 'App needs write access'
});

The cordova hook needs to run to add the HealthKitClinicalRecords.m and HealthKitClinicalRecords.h files. This was done to solve #103

@awatson1978
Copy link
Collaborator

awatson1978 commented Dec 4, 2018

Hmmm... okay, using the latest commit, and things have broken again.

2018-12-03 23:57:56.730361-0600 Symptomatic[45653:3069036] 
*** Terminating app due to uncaught exception 'NSInvalidArgumentException', 
reason: 'Authorization to read the following types is disallowed: 
HKClinicalTypeIdentifierMedicationRecord, HKClinicalTypeIdentifierLabResultRecord, HKClinicalTypeIdentifierProcedureRecord, HKClinicalTypeIdentifierVitalSignRecord, HKClinicalTypeIdentifierImmunizationRecord, HKClinicalTypeIdentifierConditionRecord, HKClinicalTypeIdentifierAllergyRecord'

@rossmartin
Copy link
Collaborator Author

rossmartin commented Dec 4, 2018

@awatson1978 is the NSHealthClinicalHealthRecordsShareUsageDescription in the info.plist set?

@awatson1978
Copy link
Collaborator

Okay.... I've traced all my code line-by-line after pulling in commit 2db3a359fd12f90ec9c0817d4b7b1bc9b8ea458b. And the plugin is isn't returning any records.

Here's the code I'm using:

      window.plugins.healthkit.requestAuthorization(
          {
            readTypes: [
              'HKClinicalTypeIdentifierAllergyRecord'
          ],
            writeTypes: []
          },
          function(msg){console.log('requestAuthorization.ok', msg)},
          function(msg){console.log('requestAuthorization.nok', msg)}
      );

      console.log('-------------------------------------------')
      console.log('queryClinicalSampleType')
      window.plugins.healthkit.queryClinicalSampleType(
        {
          'startDate': new Date(new Date().getTime() - 365 * 24 * 60 * 60 * 1000) , // 365 days ago
          'endDate': new Date(), // now
          sampleType: 'HKClinicalTypeIdentifierAllergyRecord'
          // fhirResourceType: 'HKFHIRResourceTypeAllergyIntolerance'
        },
        function(results){
          console.log('HKFHIRResourceTypeAllergyIntolerance.ok', JSON.stringify(results))
          results.forEach(function(record){
            if(get(record, 'FHIRResource.data.resourceType') === "AllergyIntolerance"){
              AllergyIntolerances._collection.insert(get(record, 'FHIRResource.data'));
            }
          })
        },
      function(msg){console.log('HKFHIRResourceTypeAllergyIntolerance.nok', msg)}
      );

      console.log('-------------------------------------------')
      console.log('queryForClinicalRecordsWithFHIRResourceType')
      window.plugins.healthkit.queryForClinicalRecordsWithFHIRResourceType(
        {
          fhirResourceType: 'HKFHIRResourceTypeAllergyIntolerance',
          sampleType: 'HKClinicalTypeIdentifierAllergyRecord'
        },
        function(results){
          console.log('HKFHIRResourceTypeAllergyIntolerance.ok', JSON.stringify(results))
          results.forEach(function(record){
            if(get(record, 'FHIRResource.data.resourceType') === "AllergyIntolerance"){
              AllergyIntolerances._collection.insert(get(record, 'FHIRResource.data'));
            }
          })
        },
      function(msg){console.log('HKFHIRResourceTypeAllergyIntolerance.nok', msg)}
      );

      console.log('-------------------------------------------')
      console.log('queryForClinicalRecordsFromSource')
      window.plugins.healthkit.queryForClinicalRecordsFromSource(
        {
          fhirResourceType: 'HKFHIRResourceTypeAllergyIntolerance',
          sampleType: 'HKClinicalTypeIdentifierAllergyRecord'
        },
        function(results){
          console.log('HKFHIRResourceTypeAllergyIntolerance.ok', JSON.stringify(results))
          results.forEach(function(record){
            if(get(record, 'FHIRResource.data.resourceType') === "AllergyIntolerance"){
              AllergyIntolerances._collection.insert(get(record, 'FHIRResource.data'));
            }
          })
        },
      function(msg){console.log('HKFHIRResourceTypeAllergyIntolerance.nok', msg)}
      );

Here's the debugging log:
screen shot 2018-12-04 at 2 05 03 pm

You can see that NSHealthClinicalHealthRecordsShareUsageDescription and other keys are set.

Originally, I had put HKCharacteristicType* values into the readTypes array, which caused problems with requestAuthorization. That's what started the line-by-line debugging. But eventually got that all sorted out.

However... queryForClinicalRecordsWithFHIRResourceType is neither returning a result nor erroring out. So that's a regression from commit 5174fde920092c94fe3dd3dead64700d293d9235.

Also, with queryClinicalSampleType we're receiving an error on startDate needing to be a Javascript Date Object.

@awatson1978
Copy link
Collaborator

tl;dr - Commit 2db3a359fd12f90ec9c0817d4b7b1bc9b8ea458b seems to have a regression, and we can successfully authenticate, but we're not returning any records.

@rossmartin
Copy link
Collaborator Author

@awatson1978 Does your Xcode project have HealthKitClinicalRecords.h and HealthKitClinicalRecords.m in the Plugins folder?

I think whats happening is that those files aren't being added to your Xcode project because of the way meteor handles installing cordova plugins. There is an after-plugin-install hook used with this plugin that had to be done to solve #103.

@awatson1978
Copy link
Collaborator

Ah, yeah, looks like the after-plugin-install hook isn't being run. At least, the files aren't in the Plugins folder. Hrmm.....

@awatson1978
Copy link
Collaborator

awatson1978 commented Dec 4, 2018

Okay, doing a quick bit of looking around, and here's a relevant thread from an MDG core developer on developing after-plugin-install hooks:

https://forums.meteor.com/t/cordova-how-to-apply-platform-hook-with-meteor/17692/10

Think we could maybe try refactoring the contents of scripts/after-plugin-install.js into plugin.xml? Or the launching? It seems the contents of the hook can maybe stay where they are.

https://github.com/meteor/cordova-plugin-meteor-webapp/blob/master/plugin.xml#L24-L31

@rossmartin
Copy link
Collaborator Author

I think it will be easiest to manually add the HealthKitClinicalRecords.h and HealthKitClinicalRecords.m to the plugins folder in your Xcode project. I'll respond with steps in just a few.

@awatson1978
Copy link
Collaborator

Hmmm....

Well, the good news is that adding those files in, and everything is working again! :)

@rossmartin
Copy link
Collaborator Author

rossmartin commented Dec 4, 2018

@awatson1978 I'm simply trying to help. I don't know why you would downvote a comment that is providing useful information in your situation.

There is a problem with the hook that I created if the xcode dependency is not installed which I need to fix. It may just simply come down to requiring it to be installed manually before installing the plugin (if you are installing it with the CLINICAL_READ_PERMISSION set of course).

I'm not sure that this matters because I don't know if the hook is being ran when the cordova plugin is added by meteor. I'm going to try it with a meteor cordova project and see.

Another alternative is to simply create a separate cordova plugin to access the clinical health data. I think that is a sound option at this point. Dealing with solving #103 was a mess and this functionality might be best as a separate plugin.

@awatson1978
Copy link
Collaborator

awatson1978 commented Dec 4, 2018

Well, mostly I had the landlord at the door interrupting with plumping and electrical issues, and I just clicked an emoticon to summarize a half-dozen thoughts that I didn't have time to write out. Downvote was just a knee-jerk response to adding a manual step to the build pipeline, is all.

Thank you for all the work. And I'll be happy to continue verifying/validating/debugging/documenting the solution until the PR gets merged in. Would like to see a solution that supports all the major build pipelines though.

That being said, we may need to simply submit a follow-up PR to address the meteor build pipeline.
There might be some things we can do by substituting and/or adding thebefore_prepare hook in addition to theafter_plugin_install hook. There's also a cordova-build-override that I'm looking into. Sorta suboptimal, but better than where we were before, and perhaps a manageable solution. But maybe needs to be in its own PR.

@awatson1978
Copy link
Collaborator

Well... managed to get all the FHIR resources loaded into the app in both the Simulator and when the app is installed directly on the device via XCode; but the app crashes when trying to load the FHIR resources when the app has been installed via TestFlight.

It's the same code as was run on the Phone from XCode, in that we didn't run the Meteor build pipeline or adjust the Cordova package in anyway. Initial thought is that we're losing the NSHealthClinicalHealthRecordsShareUsageDescription value during the Archiving and Distribution/Validation process?

idk. Need to think on it....

@rossmartin
Copy link
Collaborator Author

@awatson1978 According to this comment at #100 (comment) it can be fixed by editing the entitlements-release.

@rossmartin
Copy link
Collaborator Author

rossmartin commented Dec 6, 2018

@awatson1978 I updated the after-plugin-install hook to work with meteor in the recent commit 994bf6b

If you set the CLINICAL_READ_PERMISSION in your meteor mobile-config.js the HealthKitClinicalRecords.h and HealthKitClinicalRecords.m files will get automatically added to your Xcode project.

Example mobile-config.js

App.configurePlugin('com.telerik.plugins.healthkit', {
  HEALTH_READ_PERMISSION: 'App needs read access',
  HEALTH_WRITE_PERMISSION: 'App needs write access',
  CLINICAL_READ_PERMISSION: 'App needs read access'
});

The hook will also add this to your cordova projects config.xml and it will handle setting the NSHealthClinicalHealthRecordsShareUsageDescription. You shouldn't have to set that manually in the info.plist anymore from a fresh install.

<config-file parent="NSHealthClinicalHealthRecordsShareUsageDescription" target="*-Info.plist">
    <string>App needs read access</string>
</config-file>

Let me know how that works, thanks.

@awatson1978
Copy link
Collaborator

Hmmm… I haven’t been having quite so much luck using App.configurePlugin. However, it was a pretty great suggestion, which led to the following, which seems to be working really well:




App.appendToConfig(`<config-file parent="NSHealthClinicalHealthRecordsShareUsageDescription" target="*-Info.plist">
<string>App needs read access</string>
</config-file>`)

That's been the only thing so far to get NSHealthClinicalHealthRecordsShareUsageDescription automatically registered in the config.xml. 

Also, the code for detecting whether CLINICAL_READ_PERMISSION exists in mobile-config.js doesn’t entirely seem to be 100% quite yet? I see what you’re trying to do there, but am still needing to create the HealthKitClinicalRecords.h/m files by hand. 



But I was able to get pull up and display HealthRecords after distributing via TestFlight. Needed to add the following Entitlements-Release.plist file:

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
    <key>application-identifier</key>
    <string>$(AppIdentifierPrefix)$(CFBundleIdentifier)</string>
    <key>beta-reports-active</key>
    <true/>
</dict>
</plist>

The Entitlements-Release.plist file needed to be in the same directory as theBridging-Header.h. The Entitlements-Debug file has a custom UI in XCode, and is filtered out of the project files. Had to find it on disk to add the Release version.

@rossmartin
Copy link
Collaborator Author

@awatson1978 are you using the latest commit hash when installing with meteor?

It should be this -

com.telerik.plugins.healthkit@https://github.com/rossmartin/HealthKit.git#994bf6b1696ae504793067f7107c4baa6923a8ad

@awatson1978
Copy link
Collaborator

awatson1978 commented Dec 6, 2018

Yes, and I made sure to clear out the caches and build folders, so not using a cached version by accident.

Are you also using Meteor and have actually confirmed the App.configurePlugin() works? I've never actually gotten it to work (although it definately led to the 
App.appendToConfig() which does appear to).

@rossmartin
Copy link
Collaborator Author

rossmartin commented Dec 6, 2018

@awatson1978 I have experience with meteor and this plugin install works on an old project and a new scratch project I made.

Can you try -

  1. Remove the cordova plugin (meteor remove cordova:com.telerik.plugins.healthkit)
  2. Do a meteor build after ^
  3. Install com.telerik.plugins.healthkit@https://github.com/rossmartin/HealthKit.git#994bf6b1696ae504793067f7107c4baa6923a8ad
  4. Do another meteor build.

I've also seen in some cases with meteor plugins where you have to delete the .meteor/local/cordova-build folder and do another build to get plugins updated (this will delete your android/xcode project)

@awatson1978
Copy link
Collaborator

awatson1978 commented Dec 6, 2018

Yeah, deleting the .meteor/local/cordova-build folder is exactly what I was doing. And latest build is already using com.telerik.plugins.healthkit@https://github.com/rossmartin/HealthKit.git#994bf6b1696ae504793067f7107c4baa6923a8ad.

But I'll give it another try this evening. Have to run out for the evening, but thank you for all the help! Getting the app into TestFlight was a milestone!

(Now just need to clean everything up, debug memmory leaks, automate pipeline, etc. etc. Interesting idea to do a rebuild from a scratch project though.)

@laken
Copy link

laken commented Apr 1, 2019

We need this PR also - what's the current status? We could also lend some cycles if there's more work to be done. Thx.

igoris and others added 3 commits April 2, 2019 12:41
Automatically enable HealthKit and Health Records capabilities in Xcode
@awatson1978
Copy link
Collaborator

@laken - Status is that we're waiting for more confirmed implementations and that we're still seeking out bugs. If you'd like to lend some cycles, write up a report of your experiences installing this branch/PR. If it goes without any problems, then tell us, so that EddyVerbruggen knows that this PR doesn't break anything. If it does break something, help us reproduce the error so the bug can be fixed.

@laken
Copy link

laken commented Apr 2, 2019

@awatson1978 Great, I've passed this along to our dev team. I think @igoris has already contributed a patch.

@chrisweight
Copy link

Any progress on this being accepted? I'll be needing to use this PR shortly, so happy to report back any findings.

@awatson1978
Copy link
Collaborator

awatson1978 commented Jul 10, 2019

I'm happy to report that Apple just approved our app for release into the App Store. As part of the review, they asked 'How does this app use the Clinical HealthRecords API?' and I pointed them to this pull request (among other things). So, Apple has presumably reviewed this Cordova plugin PR, and approved at least one app with the rossmartin:master fork. Huge shoutout and thank you to @rossmartin, by the way.

@awatson1978
Copy link
Collaborator

Any movement on this @EddyVerbruggen? Community sponsored solution, no conflicts with base branch, two separate community members (both healthcare professionals) have reviewed and independently verified the solution works, and other community members asking for the merge to pulled in.

We're running into hiccups referencing the plugin by commit hash as we close out and open new projects. Would love to see this in the master branch Anything else we can do to help get this PR accepted?

@EddyVerbruggen
Copy link
Owner

Hi @awatson1978! Thanks for testing this, much appreciated.

This PR might have slipped my attention because I was never tagged after the review, but to avoid this in the future I've added you and the awesome @rossmartin as collaborators to the repo.

Please go ahead and merge this in. I can also add you as an owner to the npm package if you like.

@awatson1978 awatson1978 merged commit 1d9847c into EddyVerbruggen:master Oct 18, 2019
@awatson1978
Copy link
Collaborator

Big shoutout and THANK YOU to @rossmartin!!!

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

Successfully merging this pull request may close these issues.

iOS app store rejection due to HKClinicalRecord
6 participants