fix (batch 2): isNewManualExpenseFlowEnabled beta flag regressions - #97264
fix (batch 2): isNewManualExpenseFlowEnabled beta flag regressions#97264thelullabyy wants to merge 5 commits into
Conversation
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
|
@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: a4d38ff75e
ℹ️ 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".
| // embedded participant picker sets it via setTransactionReport), not a leftover report from a stale draft, | ||
| // so exclude it — otherwise selecting self DM in global create makes transactionRequestType fall back to the | ||
| // persisted SELECTED_TAB and the RHP jumps to whatever tab was last visited (e.g. Scan). | ||
| const isStaleTransactionDraft = !!transaction?.reportID && transaction.reportID !== CONST.REPORT.UNREPORTED_REPORT_ID && transaction.reportID !== reportID; |
There was a problem hiding this comment.
Treat unreported drafts as stale for fresh tab deeplinks
When a direct create-tab deeplink reaches this page without startMoneyRequest after a previous self-DM/global-create draft is still stored under OPTIMISTIC_TRANSACTION_ID, that stale draft has reportID === UNREPORTED_REPORT_ID and now makes isStaleTransactionDraft false even though the URL's reportID is different. The transactionRequestType branch below then trusts the old draft's iouRequestType, so a fresh /scan, /distance, etc. shortcut can open on the previous request type instead of the deeplinked tab. Consider limiting the sentinel exception to the current in-flight flow rather than exempting every unreported draft.
Useful? React with 👍 / 👎.
| // Tracks whether the inline tax amount field is currently left empty. The stored tax amount collapses an empty | ||
| // field to 0, so validation can't tell "empty" from a legitimate 0; TaxFields reports emptiness here so submission | ||
| // can be blocked when the field is cleared (new manual expense flow). See #96577. | ||
| const [isTaxAmountEmpty, setIsTaxAmountEmpty] = useState(false); |
There was a problem hiding this comment.
Reset tax emptiness when changing expenses
This state is scoped to the confirmation list, not to a transaction, and it is only cleared by TaxFields when the stored tax amount differs or the section unmounts. In a multi-expense confirmation, if the user clears the inline tax amount on one expense and then switches to another expense whose stored tax amount is also 0, the field remounts with 0.00 but isTaxAmountEmpty remains true, so submitting the second expense is blocked with the empty-tax validation even though its tax input is populated. Reset this flag on transactionID changes or store it per transaction.
Useful? React with 👍 / 👎.
Explanation of Change
Fixed Issues
$ #93854
$ #96558
$ #96632
$ #96653
$ #96559
$ #96577
PROPOSAL:
Tests
Verify that these bugs are not reproducible when new manual expense flag is on
$ #96558
$ #96632
$ #96653
$ #96559
$ #96577
Offline tests
QA Steps
Please refer to Tests section
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
96653.mov
96577.mov
96559.mov
96632.mov
96558.mov