[No QA] Fix stale pending-transaction submit tests broken on main#95654
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ac6794e5cd
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| isProduction: false, | ||
| }); | ||
| expect(result.includes(CONST.REPORT.SECONDARY_ACTIONS.SUBMIT)).toBe(true); | ||
| expect(result.includes(CONST.REPORT.SECONDARY_ACTIONS.SUBMIT)).toBe(false); |
There was a problem hiding this comment.
Make the pending-transaction test exercise submit eligibility
This assertion now passes even if the pending-transaction guard in isSubmitAction is removed, because the policy mock above omits type, so isGroupPolicy(policy) is false and getSecondaryReportActions suppresses SUBMIT before this scenario distinguishes pending transactions from a normal submittable report. Since this test is meant to cover the all-pending transaction behavior, please make the fixture otherwise eligible to submit, e.g. set type: CONST.POLICY.TYPE.CORPORATE, so it would fail for the intended regression.
Useful? React with 👍 / 👎.
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
|
🚀 Deployed to staging by https://github.com/robertjchen in version: 9.4.32-0 🚀
|
Explanation of Change
Two tests started failing on
mainin the "Process new code merged to main" workflow (test jobs 8 and 3) after #91231 merged:tests/actions/ReportPreviewActionUtilsTest.ts→getReportPreviewAction ... only pending transactionstests/unit/ReportSecondaryActionUtilsTest.ts→getSecondaryAction ... only pending transactionsThis is a semantic merge conflict between two independently-green PRs:
hasOnlyPendingCardTransactionssubmit-blocking checks incanSubmitandgetSecondaryAction, plus these tests — which asserted a generic pending transaction still allows submit, passing becausehasOnlyPendingCardTransactionsthen required an Expensify Card (isExpensifyCardTransaction(t) && isPending(t)).hasOnlyPendingCardTransactionsto match any pending transaction (every(t => isPending(t))), to cover pending BYOC card transactions.91231's branch was not synced with
mainafter the first PR landed, so neither PR's CI saw the combined behavior; it only surfaced on the post-merge run. The broadened behavior is intended (an all-pending report can't be submitted until the transactions post). These two tests were left asserting the old behavior.This PR updates both tests to reflect the intended behavior: with only pending transactions,
getReportPreviewActionreturnsVIEW(notSUBMIT), andSUBMITis not offered as a secondary action.Fixed Issues
$ #95652
$ #95653
PROPOSAL:
Tests
npx jest tests/actions/ReportPreviewActionUtilsTest.ts.getReportPreviewAction should return VIEW for expense preview report with only pending transactionspasses.npx jest tests/unit/ReportSecondaryActionUtilsTest.ts.should not include SUBMIT option for admin with only pending transactionspasses.Offline tests
N/A — test-only change.
QA Steps
N/A — test-only change ([No QA]).
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)Avatar, 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
N/A — test-only change.
Android: mWeb Chrome
N/A — test-only change.
iOS: Native
N/A — test-only change.
iOS: mWeb Safari
N/A — test-only change.
MacOS: Chrome / Safari
N/A — test-only change.
MacOS: Desktop
N/A — test-only change.
– written by Claude on Amy's behalf