Revert hasAcceptedTerms on travel terms failure and cleanup provisioning state#88969
Revert hasAcceptedTerms on travel terms failure and cleanup provisioning state#88969stitesExpensify merged 2 commits intomainfrom
Conversation
When AcceptSpotnanaTerms fails, the optimistic hasAcceptedTerms=true was not being reverted. This caused BookTravelButton to skip the terms screen on retry, since it saw hasAcceptedTerms as true despite provisioning never completing. Also call cleanupTravelProvisioningSession on generic API failure so stale TRAVEL_PROVISIONING errors don't persist across retries. Co-authored-by: Shubham Agrawal <shubham1206agra@users.noreply.github.com>
|
|
||
| // Handle general API failure | ||
| if (response?.jsonCode !== 200) { | ||
| cleanupTravelProvisioningSession(); |
There was a problem hiding this comment.
@MelvinBot Please revert this change. I don't think we should do a cleanup on every API failure.
There was a problem hiding this comment.
Reverted — removed the cleanupTravelProvisioningSession() call from the generic API failure path. The hasAcceptedTerms revert in Travel.ts is still in place.
|
The failing check ( Failure: Evidence: This PR only modifies |
Co-authored-by: shubham1206agra <58412969+shubham1206agra@users.noreply.github.com> Co-authored-by: Shubham Agrawal <shubham1206agra@users.noreply.github.com>
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@MelvinBot Add tests in PR description. Also, could you update the explanation of the change? |
|
Updated the PR description: Explanation — Corrected to reflect the actual diff (only Tests / Offline tests / QA Steps — Added steps covering the failure-retry flow: simulating an |
|
@MelvinBot Complete Author Checklist (just mark all of them as done), and make this PR ready for review |
Reviewer Checklist
Screenshots/VideosScreen.Recording.2026-05-01.at.8.06.43.PM.mov |
|
✅ All Author Checklist items have been marked as done. I was unable to programmatically mark the PR as ready for review due to permission restrictions — please click Ready for review on the PR yourself. |
|
@MelvinBot You can mark PR ready for review |
|
✅ PR has been marked as ready for review. |
|
@shubham1206agra Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
joekaufmanexpensify
left a comment
There was a problem hiding this comment.
Good for product
|
🚧 @stitesExpensify 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! 🧪🧪
|
|
Hi @shubham1206agra @joekaufmanexpensify Do we need to test this or it's already tested? |
|
🚀 Deployed to staging by https://github.com/stitesExpensify in version: 9.3.67-0 🚀
Bundle Size Analysis (Sentry): |
|
No help site changes are required for this PR. This PR is a bug fix that adds failure rollback for |
Explanation of Change
When
AcceptSpotnanaTermsfails, the optimistichasAcceptedTerms = trueupdate was never reverted. This leftBookTravelButtonthinking terms were already accepted, so on retry it skipped the terms screen and went straight toopenTravelDotLink()— which also failed since provisioning never completed. The user was stuck with no way to re-accept terms.This PR adds
NVP_TRAVEL_SETTINGS.hasAcceptedTerms = falseto thefailureDataarray inacceptSpotnanaTerms()so the optimistic update is properly rolled back on API failure. This allows the user to retry the terms acceptance flow.Fixed Issues
$ #85944
Tests
AcceptSpotnanaTermsrequest to return an error), verify:hasAcceptedTermsis reverted tofalse(check Onyx state or verify that tapping Book Travel from the workspace again brings you back to the terms screen, not straight toopenTravelDotLink)Offline tests
hasAcceptedTermsreverts tofalseQA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, 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.ScrollViewcomponent to make it scrollable when more elements are added to the page.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
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari