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

Merged
merged 15 commits into from
Nov 10, 2024

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

example/src/App.tsx Outdated Show resolved Hide resolved
@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.

src/index.tsx Outdated Show resolved Hide resolved
@ugurakin1
Copy link
Contributor Author

Will look into this further this weekend

): Promise<Permission[]> {
return HealthConnect.requestPermission(permissions, providerPackageName);
return HealthConnect.requestPermission(permissions);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Initially replaced this with another arg, then removed the arg in favor of a special permission obj (see src/types/index.ts). It wasn't doing anything so didn't add it again but can undo the changes for the sake of not breaking anyone's code.

Copy link
Owner

Choose a reason for hiding this comment

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

This needs to be fixed later by myself. By default most people use health connect to get the data, but it should be possible to customize this.

@matinzd matinzd self-assigned this Oct 28, 2024
@ugurakin1
Copy link
Contributor Author

@matinzd Any chance you could take a look at this? I patched our own app and syncing worked as expected. It also touches a few other things that we don't use necessarily so a review would be much appreciated 🙏

@ugurakin1
Copy link
Contributor Author

When do you think we can get to this @matinzd ? If you're busy with other things not a problem but an eta would be super helpful to decide using a patch vs waiting for a proper release and upgrading.

@matinzd
Copy link
Owner

matinzd commented Oct 30, 2024

Hey @ugurakin1

Can you please rebase your branch?

@ugurakin1
Copy link
Contributor Author

Done @matinzd

@ugurakin1
Copy link
Contributor Author

Hey @matinzd , any updates on this/eta?

Also, did you want me to just update so there aren't any conflicts or did you actually want me to rebase instead?

@matinzd
Copy link
Owner

matinzd commented Nov 1, 2024

Hey!

I am gonna try to release it this weekend. It doesn't matter, I don't want to see any conflicts.

@ugurakin1
Copy link
Contributor Author

Let me know if you need anything from me @matinzd, happy to land a hand 👍

@ugurakin1
Copy link
Contributor Author

Hey @matinzd , any chance we can take another look this weekend?

@matinzd matinzd merged commit 3cbe841 into matinzd:main Nov 10, 2024
3 checks passed
@ugurakin1 ugurakin1 deleted the feat/exercise-route-permissions branch November 10, 2024 14:49
@ugurakin1
Copy link
Contributor Author

Massive thanks @matinzd 🙏

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