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

Get workout routes for workout UUID #79

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

karolinaoparczyk
Copy link
Contributor

Issue: #78

Status

READY

Migrations

NO

Description

Getting workout routes with time interval Predicate starting and ending at the time of the workout doesn't explicitly return routes of this workout. We get some (or no) routes without any information in them to which workout they belong to.

Trying to get routes by providng the workout UUID returns the desired routes for each workout.

I've considered extending the current Predicate object and workoutRouteQuery function to take UUID as an additional argument, but it caused a lot of issues:

  • It would be hard to do it in the way that projects which are currently using the package wouldn't be affected
  • Passing UUID to the Predicate should mean that start and end date shouldn't be provided and vice versa (needed migration)
  • I've started with creating a new Predicate for workoutRouteQuery, that would either take start and end time or UUID. I've changed the map to return Map<String, dynamic> and swift code to take [String, Any] arguments map, but trying to parse arguments in workoutRouteQuery failed when any of the parameters was null. Using Map<String, String> and fallbacking to an empty string when any of the parameters was null didn't help, as parsing failed when trying to parse timestamps (and although it could be checked and avoided, I think it's a pretty nasty way of doing it)

Looking from another angle, when I passed the UUID to workoutRouteQuery, I couldn't get it to work using

HKQuery.predicateForObject(with: uuid)

or any other variations of this method I've found when reserching the topic.

The solution I've seen the most is getting routes by providing a predicate from the workout itself. This is tricky in the current codebase, because I'd need concurrency to get the workout asynchronously, then get its routes and return them. This is available in iOS 13 and forcing projects already using workoutRouteQuery to update the iOS version is a very bad idea.

All of these reasons led me to my final decision: creating a new seperate function for getting routes by providing the workout UUID: workoutRouteForUUIDQuery with new UUIDPredicate.

This way, previous versions won't be affected and we'd get a new feature, available from iOS 13.

Related PRs

Not applicable.

Deploy Notes

Not applicable.

Steps to Test or Reproduce

git pull --prune
git checkout get-workout-routes-for-workout-uuid

Run example app - there is a new text button workoutRouteForUUIDQuery that gets workout routes for a provided UUID in workoutRouteForUUIDQuery method. I've added this to test whether the native connection works. Bear in mind that this package does not provide a way to save workout routes and it's tricky to provide any test data on iOS simulator, so this method will most likely return an empty list.

I've also noticed that although we pass the workout UUID when saving it, it gets its own UUID later on, so setting a valid UUID in saveWorkout method and then trying to get it in workoutRouteForUUIDQuery won't work, as the UUID will be changed.

Impacted Areas in Application

  • New method workoutRouteForUUIDQuery and new UUIDPredicate
  • Example app now targets iOS 13

List general components of the application that this PR will affect:

  • HealthKitReporter (new method)
  • Example app

@kvs-coder
Copy link
Owner

hi @karolinaoparczyk

please sorry for my long absence, I have just merged you PR #77

@karolinaoparczyk
Copy link
Contributor Author

hi @kvs-coder :)
Thanks for merging #77, but it needs kvs-coder/HealthKitReporter#28 merged too to work :)

I've merged this PR with the new changes, have you got a chance to look at this new feature?

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