Conversation
|
@mananjadhav 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] |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
| } | ||
|
|
||
| // eslint-disable-next-line no-restricted-syntax -- Legacy route generation | ||
| return getUrlWithBackToParam(`workspaces/${policyID}/accounting/xero/advanced/autosync/accounting-method` as const, backTo); |
There was a problem hiding this comment.
We had these prefixed like advanced/autosync/ etc. for different URLs. Are we not going to have those?
There was a problem hiding this comment.
We’re using entry screens in the routing now, so we don’t need those prefixes like advanced/autosync/ anymore.
There was a problem hiding this comment.
While I agree with your comment, the prefixes weren't added for routing. They were a part of the design doc. I think @mjasikowski would be best to confirm here, instead of @collectioneur.
There was a problem hiding this comment.
@mananjadhav We actually don't need to carry over the prefixes from the static route here. While the design doc used verify-account as an example, the only real rule for dynamic suffixes is that they should logically flow with the basePath and clearly reflect the user's current context. Plus, if we can keep the suffix shorter, we should definitely go for it! So, for this and future cases, there's no need to perfectly mirror the static routes. Everything looks great here 🙂
mananjadhav
left a comment
There was a problem hiding this comment.
Raised a question on path, which is going to be for all the accounting integrations.
#86509 (comment) This is my opinion. if you’re not concerned, we can ask an expert contributor to clarify. |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppandroid-bt-xero.movandroid-bt-xero-1.movAndroid: mWeb Chromemweb-chrome-bt-xero.moviOS: HybridAppios-bt-xero.moviOS: mWeb Safarimweb-safari-bt-xero.movMacOS: Chrome / Safariweb-bt-xero.mov |
|
🚧 @mjasikowski 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! 🧪🧪
|
|
✋ 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/mjasikowski in version: 9.3.54-0 🚀
Bundle Size Analysis (Sentry): |
|
No help site changes are required for this PR. This is a purely technical navigation refactoring — it migrates three Xero workspace screens (bank account select, purchase bill status selector, accounting method) from legacy route generation to the dynamic routes pattern. No user-facing text, labels, features, or behaviors were changed. |
Explanation of Change
Fixed Issues
$ #83842
PROPOSAL:
Tests
Same QA step
Offline tests
QA Steps
Precondition:
Case 1: Purchase Bill Status
Case 2: Xero Bank Account
Case 3: When to Export
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand 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
Screen.Recording.2026-03-27.at.15.45.11.mp4
Android: mWeb Chrome
Screen.Recording.2026-03-27.at.15.46.25.mp4
iOS: Native
Screen.Recording.2026-03-27.at.15.49.39.mp4
iOS: mWeb Safari
Screen.Recording.2026-03-27.at.15.50.46.mp4
MacOS: Chrome / Safari
Screen.Recording.2026-03-27.at.15.41.39.mp4