Fix permissions check on Resume GPS trip - #98955
Conversation
|
@codex please review |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
|
@dukenv0307 could you review as this is part of the GPS project? |
|
Sure |
|
@dukenv0307 Could you unassign me from here and the issue as well? |
|
ping me when ready |
|
on it now |
|
Code looks good, I'll test it now |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2026-08-20.at.17.01.41.movAndroid: mWeb ChromeiOS: HybridAppScreen.Recording.2026-08-20.at.17.12.01.moviOS: mWeb SafariMacOS: Chrome / Safari |
|
LGTM 👍 |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚧 mountiny has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/mountiny in version: 9.4.58-0 🚀
|
No help site changes requiredI reviewed this PR against the help site articles under Why: This is an internal bug fix. The Resume button on a stopped GPS trip previously called What I checkedThe GPS distance-tracking flow is already documented, and both articles accurately describe the Track distance → GPS → Start / Resume / Stop flow and that GPS requires the device's location services:
Neither article documents the buggy behavior being fixed (permissions were silently skipped on Resume), so nothing in the published docs becomes inaccurate or needs correcting. The fix simply makes the app behave the way the docs already imply. Since no article content changes, I did not open a draft help site PR. If you'd like me to add an explicit note about the permission prompt appearing on Resume (e.g., that you may be re-asked to allow location access), let me know and I can draft that. |
Explanation of Change
The "Resume" button on a stopped GPS trip called
resumeGpsTripdirectly, skipping the location services and background permissions checks that "Start" performs. It now callscheckSettingsAndPermissions, andBackgroundLocationPermissionsFlow'sonGrantresolves toresumeGpsTripwhen the trip is stopped, otherwisestartGpsTrip.Fixed Issues
$ #98954
PROPOSAL: N/A
Tests
Offline tests
N/A
QA Steps
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Screen.Recording.2026-08-19.at.09.44.43.mov
iOS: Native
Screen.Recording.2026-08-19.at.09.31.01.mov