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

feat: adds a new method for requesting exercise permissions #167

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

ugurakin1
Copy link
Contributor

@ugurakin1 ugurakin1 commented Oct 9, 2024

Changes

  1. Adds a new method for requesting route data of a particular exercise session record following the guide from Android docs.
  2. Modifies requestPermission to accept an optional flag that indicates requested permissions should include write-route permission. Removed the unused providerPackageName arg from the method as well.
  3. Updates the example app to include a button for inserting a random exercise session with routes and another button that requests the route of the given exercise id.

Issues

#34

@@ -34,12 +34,21 @@ class HealthConnectModule internal constructor(context: ReactApplicationContext)
@ReactMethod
override fun requestPermission(
permissions: ReadableArray,
providerPackageName: String,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wasn't used so replaced it.

recordType: 'ExerciseSession',
},
],
true
Copy link
Contributor Author

@ugurakin1 ugurakin1 Oct 9, 2024

Choose a reason for hiding this comment

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

Added true here, hard to see the diff

@ugurakin1 ugurakin1 marked this pull request as ready for review October 9, 2024 15:57
@ugurakin1
Copy link
Contributor Author

Should solve #86, #127, #128 @matinzd. Whipped up in a rush, happy to address comments.

@@ -97,16 +97,20 @@ export function openHealthConnectDataManagement(
*/
export function requestPermission(
permissions: Permission[],
providerPackageName = DEFAULT_PROVIDER_PACKAGE_NAME
includeRoute?: boolean
Copy link
Contributor Author

@ugurakin1 ugurakin1 Oct 9, 2024

Choose a reason for hiding this comment

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

Omitting this in arg in JS seems to cause issues atm:

Exception in native call
com.facebook.react.bridge.NativeArgumentsParseException: Attempt to invoke virtual method 'boolean java.lang.Boolean.booleanValue()' on a null object reference (constructing arguments for HealthConnect.requestPermission at argument index 1)

Comment on lines +84 to +91
val exerciseRoute = HealthConnectPermissionDelegate.launchExerciseRouteAccessRequestDialog(recordId)
if (exerciseRoute != null) {
promise.resolve(ReactExerciseSessionRecord.parseExerciseRoute(exerciseRoute))
}
else{
promise.rejectWithException(ExerciseRouteAccessDenied())
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm getting some flaky results with exercises, everything returns permissions denied as in exerciseRoute is null and as is there is no way to know for certain if that's because there is no data or something is wrong with my patch/permissions etc. Maybe we should fetch the exercise record here and switch on ExerciseRouteResult to determine whether to request permission or reject with "NoDataException". Could go hand in hand with this.

@ugurakin1
Copy link
Contributor Author

Will look into this further this weekend

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.

1 participant