Skip to content

Conversation

Strime
Copy link

@Strime Strime commented Jun 4, 2025

Description

This PR fixes an issue where the health plugin unconditionally fetches distance data for workout requests, even when distance information is not needed or requested.

Problem

  • The plugin always requests DistanceRecord when fetching workout data (line ~729 in HealthPlugin.kt)
  • This forces apps to include location permissions even when distance data is not needed
  • Apps without location permissions fail with SecurityException when requesting workout data
  • This contradicts the documentation which states location permissions are only needed "if the distance of a workout is requested"

Solution

Modified the workout data fetching logic to only request distance data when HealthDataType.DISTANCE is explicitly included in the requested data types.

Changes

  • Updated getData method in HealthPlugin.kt
  • Added conditional check for distance data type before requesting DistanceRecord
  • Maintains backward compatibility for apps that do request distance data

Testing

  • ✅ Workout data retrieval without distance works without location permissions
  • ✅ Workout data retrieval with distance still works with location permissions
  • ✅ No breaking changes for existing implementations

Related Issues

Fixes #1173

@Strime Strime changed the title [Health] Distance data is always fetched for workouts regardless of need [health] Fix: Only fetch distance data for workouts when explicitly requested Jun 4, 2025
@iarata
Copy link
Contributor

iarata commented Jun 5, 2025

We are testing the refactored Android version for Health (See PR #1211 - Branch health13/refactor-kotlin). Could you modify your code to match the refactored version?

@Strime Strime changed the base branch from master to health-13 June 5, 2025 04:58
@Strime
Copy link
Author

Strime commented Jun 5, 2025

Thank you for the feedback! I've updated this PR to work with the refactored Android architecture from branch health13/refactor-kotlin.

Updated architecture alignment:

  • Moved permission checking logic to HealthPermissionChecker.kt (new utility class)
  • Integrated the distance/calories permission checks into HealthDataReader.kt's handleWorkoutData method

Implementation details:

  • Added isLocationPermissionGranted(), isHealthDistancePermissionGranted(), and isHealthTotalCaloriesBurnedPermissionGranted() methods
  • Updated the workout data fetching logic to check permissions before attempting to read distance and calories data

The core fix remains the same (checking permissions before requesting distance data), but it's now properly integrated into the refactored class structure.

Ready for review with the refactored codebase! 🚀

@iarata iarata changed the base branch from health-13 to health13/refactor-kotlin June 5, 2025 12:46
@iarata iarata changed the base branch from health13/refactor-kotlin to health13/small-bugs June 22, 2025 16:28
@iarata iarata merged commit fb9adeb into carp-dk:health13/small-bugs Jun 22, 2025
1 check passed
@iarata
Copy link
Contributor

iarata commented Jun 24, 2025

I checked your implementation, the distance is fetched when the location permission is not given and when the location permission is given the distance is not being fetched @Strime

@shliama
Copy link

shliama commented Jun 29, 2025

@iarata how so? HealthPermissionChecker methods such as isLocationPermissionGranted, isHealthDistancePermissionGranted and isHealthTotalCaloriesBurnedPermissionGranted are correct. And the changes inside HealthDataReader are also simple if-statements that look absolutely valid. How do you get the opposite results?

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.

[health] Distance data is always fetched for workouts regardless of need

3 participants