Repository navigation
fix (batch 5): isNewManualExpenseFlowEnabled beta flag regressions - #99947
Conversation
…essions-from-93854-part-5
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@ikevin127 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] |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8b71811b7f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
|
This issue #96614 is not including in this fix yet, I'm still investigating the solution |
Reviewer Checklist
Screenshots/Videos |
|
@thelullabyy Reviewer checklist completed ✅ 1/3 issues confirmed as fixed - |
…essions-from-93854-part-5
|
I fixed the regression and all other comments. @ikevin127 Back to you again Screen.Recording.2026-09-05.at.02.22.31.mov |
| function createMockScreenNavigation(): MockScreenNavigation { | ||
| const focusListeners: Array<() => void> = []; | ||
|
|
||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion -- test-only navigation stub. The single assertion lives here so no render site needs one. |
There was a problem hiding this comment.
🟢 The one type assertion in the PR. createMockScreenNavigation.ts uses as unknown as ConfirmationScreenNavigation with an eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion and a written justification.
Worth a conscious sign-off given the no-casting guidance, but I would keep it: it is test-only, contained to one line in one helper, and it replaces roughly twenty @ts-expect-error directives spread across two files.
The alternative is strictly worse.
cc @blimpich for a take - given our effort with the seatbelt series
ikevin127
left a comment
There was a problem hiding this comment.
🟢 LGTM - The last regression was fixed and comments were addressed
|
NAB, but for the future can you (or can you teach your AI) to make the comments a little more succinct. |
|
🚧 puneetlath has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/puneetlath in version: 9.4.74-0 🚀
|
|
🤖 No help site changes are required for this PR. No draft docs PR was created. Why: Every change here is internal bug-fix logic behind the
Verification detailsDocs checked
Searches run across all of Scope note: 11 of the 16 changed files are tests or test utilities. The 5 source files are About the parent feature: the beta this batch stabilizes — #93854, showing Merchant, Date, and Amount fields in the Scan flow — will need help site updates once it ships to everyone, because @thelullabyy, I found no help site changes to make, so there's no linked docs PR to review. Please confirm you agree with that assessment. If you'd rather document the new manual expense flow now (ahead of the beta going GA), reply with |
|
🚀 Deployed to production by https://github.com/mountiny in version: 9.4.74-2 🚀
|
Explanation of Change
Fixed Issues
$ #93854
$ #96565
$ #96568
$ #96562
PROPOSAL:
Tests
Verify that these bugs are not reproducible when new manual expense flow beta flag is enabled
#96568
#96565
#96562
Offline tests
QA Steps
Verify that these bugs are not reproducible when new manual expense flow beta flag is enabled
#96568
#96565
#96562
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
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
Screen.Recording.2026-09-01.at.02.29.00.mov
Screen.Recording.2026-09-01.at.02.41.16.mov
Screen.Recording.2026-09-01.at.03.02.42.mov