-
Notifications
You must be signed in to change notification settings - Fork 3.2k
feat: Reset non USD flow #58100
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: Reset non USD flow #58100
Conversation
…91-reset-non-usd-flow
…91-reset-non-usd-flow
…91-reset-non-usd-flow
…91-reset-non-usd-flow
@hungvu193 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] |
Hey, @hungvu193! It's ready for review but there is one small issue we know about and will resolved by @madmax330 Currently reset for non USD accounts only works correctly the 1st time you do it for the workspace. This is not a FE issue so I've decided to move this PR to review. |
Reviewing shortly 🤖 |
src/pages/ReimbursementAccount/NonUSD/NonUSDVerifiedBankAccountFlow.tsx
Outdated
Show resolved
Hide resolved
src/pages/ReimbursementAccount/USD/ConnectBankAccount/ConnectBankAccount.tsx
Outdated
Show resolved
Hide resolved
@hungvu193 Addressed all the comments |
Reviewer Checklist
Screenshots/VideosAndroid: NativeScreen.Recording.2025-03-18.at.16.59.41.movAndroid: mWeb ChromeScreen.Recording.2025-03-18.at.16.42.23.moviOS: NativeiOS: mWeb SafariScreen.Recording.2025-03-18.at.16.00.19.movMacOS: Chrome / SafariScreen.Recording.2025-03-18.at.15.54.26.movMacOS: DesktopScreen.Recording.2025-03-18.at.15.56.53.mov |
@madmax330 let me know when the BE bug is fixed so I can retest it before merging |
@hungvu193 Pushed additional commit with a fix for one of the edge cases I've found when helping Max with debugging this. PolicyID was undefined when user reset the flow and started it again immediately without closing RHP. It now works correctly |
Ty. Let me retest |
Thanks I can confirm it worked! |
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
🚀 Deployed to staging by https://github.com/madmax330 in version: 9.1.16-0 🚀
|
🚀 Deployed to production by https://github.com/luacmartins in version: 9.1.16-4 🚀
|
Explanation of Change
Implements reset method for non USD VBA flow
Renames some existing methods
Fixed Issues
$ #58091
PROPOSAL:
Tests
USD workspace
Almost there! We need your help verifying a few last bits of information over chat. Ready?
is displayedNon USD workspace
Offline tests
QA Steps
Almost there! We need your help verifying a few last bits of information over chat. Ready?
is displayedPR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)src/languages/*
files and using the translation methodSTYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Design
label and/or tagged@Expensify/design
so the design team can review the changes.ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
Android: Native
android.native.mp4
Android: mWeb Chrome
android.web.mp4
iOS: Native
ios.native.mp4
iOS: mWeb Safari
ios.web.mp4
MacOS: Chrome / Safari
web.mp4
MacOS: Desktop
desktop.mp4