Show Pay badge on LHN for negative expenses#87161
Conversation
When a negative expense is approved, the expense preview correctly shows "Mark as paid" by calling canIOUBePaid with onlyShowPayElsewhere=true as a fallback. The LHN badge logic in getIOUReportActionWithBadge was missing this second call, so no Pay badge appeared for negative amounts. Co-authored-by: Aimane Chnaif <aimane-chnaif@users.noreply.github.com>
| // Only show to the actual payer, exclude admins with bank account access | ||
| if (canIOUBePaid(iouReport, chatReport, policy, undefined, undefined, undefined, undefined, invoiceReceiverPolicy)) { | ||
| if ( | ||
| canIOUBePaid(iouReport, chatReport, policy, undefined, undefined, undefined, undefined, invoiceReceiverPolicy) || |
There was a problem hiding this comment.
The condition is not redundant. The two canIOUBePaid calls cover different scenarios:
- First call (
onlyShowPayElsewhere = false): the standard payment path - Second call (
onlyShowPayElsewhere = true): enables an additional code path for negative-amount expenses wherereimbursementChoice === REIMBURSEMENT_NO
When onlyShowPayElsewhere is false, the function early-returns false at the REIMBURSEMENT_NO policy check. When true, it bypasses that gate and reaches the canShowMarkedAsPaidForNegativeAmount path (onlyShowPayElsewhere && reimbursableSpend < 0) — which is the exact scenario this PR fixes.
This mirrors the same two-step pattern already used in MoneyRequestReportPreviewContent and PayActionButton.
There was a problem hiding this comment.
This mirrors the same two-step pattern already used in
MoneyRequestReportPreviewContentandPayActionButton.
@MelvinBot can you share code permalink?
There was a problem hiding this comment.
Here are the permalinks:
ReportPreviewActionButton.tsx:94—!canIOUBePaid && canIOUBePaidIOUActions(iouReport, chatReport, policy, bankAccountList, transactions, true, ...)PayActionButton.tsx:97—!canIOUBePaid && canIOUBePaidIOUActions(iouReport, chatReport, policy, bankAccountList, transactions, true, ...)
Both first check canIOUBePaid (with default onlyShowPayElsewhere = false), then fall back to a second call with onlyShowPayElsewhere = true.
There was a problem hiding this comment.
Added unit test in tests/unit/getIOUReportActionWithBadgeTest.ts (753fad9) covering:
- Positive reimbursable spend — standard path returns PAY badge
- Negative expense (credit) — the
onlyShowPayElsewherepath returns PAY badge (the scenario this PR fixes) - Settled report — no badge returned
|
@MelvinBot mark as ready for review |
|
Marked this PR as ready for review. ✅ |
|
@aimane-chnaif 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] |
melvin-bot
bot
requested review from
joekaufmanexpensify
and removed request for
a team
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppios.moviOS: mWeb Safarimsafari.movMacOS: Chrome / Safarinegative.movnon-reimbursable.mov |
Co-authored-by: Aimane Chnaif <aimane-chnaif@users.noreply.github.com>
| return waitForBatchedUpdates(); | ||
| } | ||
|
|
||
| describe('getIOUReportActionWithBadge', () => { |
There was a problem hiding this comment.
@MelvinBot can you migrate new test cases into here?
Line 13524 in f01edb1
We don't need this new test file.
There was a problem hiding this comment.
Done. Migrated the negative expense (onlyShowPayElsewhere) test case into the existing getIOUReportActionWithBadge describe block in IOUTest.ts and deleted the standalone test file.
The other two tests (PAY badge for positive spend, undefined badge for settled report) were already covered by existing tests in that block, so only the new case was added.
Code ReviewThe change is correct and mirrors the established two-step 1. Minor optimization: if (
canIOUBePaid(iouReport, chatReport, policy, undefined, undefined, undefined, undefined, invoiceReceiverPolicy) ||
canIOUBePaid(iouReport, chatReport, policy, undefined, undefined, true, undefined, invoiceReceiverPolicy)
) {JavaScript's 2. Misleading comment (suggestion): The comment on line 7170 says "Only show to the actual payer, exclude admins with bank account access", but the second call with Tests look good — correct location, naming, and cover the key scenarios (positive spend, negative/credit spend, settled report). Overall LGTM with the optional comment update. 👍 |
src/libs/actions/IOU/index.ts
Outdated
| @@ -7168,7 +7168,10 @@ function getIOUReportActionWithBadge( | |||
| } | |||
| const iouReport = getReportOrDraftReport(action.childReportID); | |||
| // Only show to the actual payer, exclude admins with bank account access | |||
There was a problem hiding this comment.
@MelvinBot update this comment as per 2. Misleading comment
There was a problem hiding this comment.
Updated the comment to: // Show to the actual payer, or to policy admins via the pay-elsewhere path for negative expenses (cf5ad94)
… file Move the negative expense (onlyShowPayElsewhere) test case into the existing getIOUReportActionWithBadge describe block in IOUTest.ts. The other two test cases (positive spend PAY badge, settled report) were already covered by existing tests. Delete the standalone test file. Co-authored-by: Aimane Chnaif <aimane-chnaif@users.noreply.github.com>
Co-authored-by: Aimane Chnaif <aimane-chnaif@users.noreply.github.com>
|
Codex Review: Didn't find any major issues. 👍 ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
|
The typecheck failure was caused by the This was already fixed by commits |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
joekaufmanexpensify
left a comment
joekaufmanexpensify
left a comment
There was a problem hiding this comment.
Good for product 👍
3 participants
Explanation of Change
The LHN badge logic in
getIOUReportActionWithBadgeonly calledcanIOUBePaidonce withonlyShowPayElsewhere=false(default). For negative-amount expenses,reimbursableSpend < 0so the standardreimbursableSpend > 0check incanIOUBePaidfails, and no Pay badge is shown.The expense preview components (
MoneyRequestReportPreviewContent,PayActionButton) handle this correctly by callingcanIOUBePaida second time withonlyShowPayElsewhere=true, which enables thecanShowMarkedAsPaidForNegativeAmountpath.This PR mirrors that two-step pattern in
getIOUReportActionWithBadgeso negative expenses that show "Mark as paid" also get the Pay badge on the LHN.Fixed Issues
$ #85187
PROPOSAL: #85187 (comment)
Tests
Offline tests
N/A — badge logic is computed from local Onyx data; offline behavior is unchanged.
QA Steps
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