fix: Pay with Expensify-Cancel payment option is not displayed for the paid expense#69731
Conversation
|
Let's test this with an adhoc build before merging. Please let me know when it's ready do that and I'll spin one up 👍 |
|
Overall, the change set looks good. Since most contributors don’t have a business bank account so we'll test with mock data. cc: @mitarachim |
|
🚧 @chiragsalian has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
Thank you! @TaduJR We can start creating mock data to complete the PR checklist |
Got it. Please let me know once it is ready to retest. We should make sure it works on the ad-hoc build, or figure out what the other condition is, if there is one. |
@TaduJR how about your test, do you think there might be any other condition blocking this flow? |
|
@suneox Investigating |
|
A migrated user brought this up to @twisterdotcom today. Can we keep this moving to get this fixed? Thanks! |
|
@suneox After investigating the complete 1. isPayer Check FailingThe
In test environments, the user might not be properly configured as the designated reimburser for the test bank account. 2. isPaymentProcessing Check FailingThis requires both:
Test payments might not trigger the proper Extensive Debugging Steps:
function isCancelPaymentAction(report: Report, reportTransactions: Transaction[], policy?: Policy): boolean {
const isExpenseReport = isExpenseReportUtils(report);
if (!isExpenseReport) {
return false;
}
const isAdmin = policy?.role === CONST.POLICY.ROLE.ADMIN;
const session = getSession(); // Store in variable first
const isPayer = isPayerUtils(session, report, false, policy);
// Log with already computed values, not function calls
console.log('🔍 Cancel Payment Debug - Initial checks:', {
reportID: report.reportID,
isExpenseReport,
isAdmin,
isPayer,
policyRole: policy?.role,
userAccountID: session?.accountID, // Use stored variable
userEmail: session?.email, // Use stored variable
managerID: report.managerID,
});
if (!isAdmin || !isPayer) {
console.log('❌ Cancel Payment - Failed admin/payer check:', { isAdmin, isPayer });
return false;
}
const isReportPaidElsewhere = report.stateNum === CONST.REPORT.STATE_NUM.APPROVED && report.statusNum === CONST.REPORT.STATUS_NUM.REIMBURSED;
if (isReportPaidElsewhere) {
console.log('✅ Cancel Payment - Paid elsewhere, showing button');
return true;
}
const isPaymentProcessing = !!report.isWaitingOnBankAccount && report.statusNum === CONST.REPORT.STATUS_NUM.APPROVED;
console.log('🔍 Cancel Payment - Payment state:', {
isPaymentProcessing,
isWaitingOnBankAccount: report.isWaitingOnBankAccount,
statusNum: report.statusNum,
expectedStatus: CONST.REPORT.STATUS_NUM.APPROVED,
});
const payActions = reportTransactions.reduce((acc, transaction) => {
const action = getIOUActionForReportID(report.reportID, transaction.transactionID);
if (action && isPayAction(action)) {
acc.push(action);
}
return acc;
}, [] as ReportAction[]);
console.log('🔍 Cancel Payment - Pay actions found:', payActions.length);
const hasDailyNachaCutoffPassed = payActions.some((action) => {
const now = new Date();
const paymentDatetime = new Date(action.created);
const nowUTC = new Date(Date.UTC(now.getUTCFullYear(), now.getUTCMonth(), now.getUTCDate(), now.getUTCHours(), now.getUTCMinutes(), now.getUTCSeconds()));
const cutoffTimeUTC = new Date(Date.UTC(paymentDatetime.getUTCFullYear(), paymentDatetime.getUTCMonth(), paymentDatetime.getUTCDate(), 23, 45, 0));
const cutoffPassed = nowUTC.getTime() > cutoffTimeUTC.getTime();
console.log('🔍 NACHA cutoff check:', {
actionCreated: action.created,
nowUTC: nowUTC.toISOString(),
cutoffTimeUTC: cutoffTimeUTC.toISOString(),
cutoffPassed,
});
return cutoffPassed;
});
const finalResult = isPaymentProcessing && !hasDailyNachaCutoffPassed;
console.log('🎯 Cancel Payment - Final decision:', {
isPaymentProcessing,
hasDailyNachaCutoffPassed,
finalResult,
willShowButton: finalResult
});
return finalResult;
}This will log all the key conditions so you can see exactly which one is failing in the ad-hoc build testing. @joekaufmanexpensify Could you please add the above loggers, and give us the result? Thank you 🙏 |
|
@TaduJR Just to make sure I am following, I should run that function in my console after paying a report when we expect the cancel payment button to appear, but when it is not? |
|
@joekaufmanexpensify If possible in order to get more insights both would be great, but the main one is running after paying a report when we expect the cancel payment button to appear |
|
Discussing to make sure I can run this before I do. |
I've updated the debugging code to store values in variables first before logging them so it uses all console.log statements only use already-computed values. |
|
Checking again internally |
|
@TaduJR I chatted with some colleages again, they recommended you temporarily commit this updated version of the function with the logging, and then I can spin up another adhoc build and test with it. Sound good? |
|
I’m very sorry for the inconvenience @joekaufmanexpensify, but I’m currently unwell with a high fever and flu and haven’t been able to make progress on this PR despite my efforts. I’ll do my best to catch up and complete the work as soon as I’m able. Thank you for your understanding, and I apologize again for the delay. |
|
Thanks for the update, hope you feel better soon! |
|
@joekaufmanexpensify Made the temporary commit |
|
🚧 @joekaufmanexpensify 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, Desktop, and Web. Happy testing! 🧪🧪
|
|
🚧 @joekaufmanexpensify 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, Desktop, and Web. Happy testing! 🧪🧪
|
|
🚧 @joekaufmanexpensify 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, Desktop, and Web. Happy testing! 🧪🧪
|
|
🚧 @joekaufmanexpensify 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, Desktop, and Web. Happy testing! 🧪🧪
|
TaduJR
force-pushed
the
fix-Pay-with-Expensify-Cancel-payment-option-is-not-displayed-for-the-paid-expense
branch
from
e11ba5a to
3c01bf0
Compare
|
🚧 @joekaufmanexpensify 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, Desktop, and Web. Happy testing! 🧪🧪
|
TaduJR
force-pushed
the
fix-Pay-with-Expensify-Cancel-payment-option-is-not-displayed-for-the-paid-expense
branch
from
e39cea2 to
a0af1ca
Compare
|
Hey @suneox Thanks! |
From my previous review, the current logic change getting I’m just curious whether we should also check the final case where STATE_NUM = AUTOREIMBURSED. |
…h-Expensify-Cancel-payment-option-is-not-displayed-for-the-paid-expense
- Added AUTOREIMBURSED (stateNum: 6) state check for bank processing - Added STATUS_NUM.REIMBURSED check to isInBillingState for consistency
Added that also |
…h-Expensify-Cancel-payment-option-is-not-displayed-for-the-paid-expense
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppCleanShot.2025-12-21.at.22.28.06__2.mp4Android: mWeb ChromeCleanShot.2025-12-21.at.22.30.18__2.mp4iOS: HybridAppCleanShot.2025-12-21.at.22.23.19__2.mp4iOS: mWeb SafariCleanShot.2025-12-21.at.22.24.25__2.mp4MacOS: Chrome / SafariEnable approval CleanShot.2025-12-21.at.22.20.23__2.mp4Disable approval CleanShot.2025-12-21.at.22.17.54__2.mp4 |
suneox
left a comment
suneox
left a comment
There was a problem hiding this comment.
Currently, the NACHA cutoff logic is not available for test bank accounts. I’ve verified the normal flow with approval enabled/disabled and auto-submission. The change LGTM
|
✋ 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/chiragsalian in version: 9.2.86-0 🚀
|
|
🚀 Deployed to production by https://github.com/marcaaron in version: 9.2.86-4 🚀
|
TaduJR
deleted the
fix-Pay-with-Expensify-Cancel-payment-option-is-not-displayed-for-the-paid-expense
branch
| violations: {}, | ||
| policy, | ||
| }); | ||
| expect(result.includes(CONST.REPORT.SECONDARY_ACTIONS.CANCEL_PAYMENT)).toBe(true); |
There was a problem hiding this comment.
Hey @situchan
Thanks for the notice!
We already discussed with arosiclair yesterday on slack. Prepared a PR raising it.
8 participants
Explanation of Change
The cancel payment button was not displaying for bank payments because pay actions were being fetched per-transaction using
getIOUActionForReportID, but pay actions are actually stored at the report level, not per transaction. This resulted in an emptypayActionsarray, causing the cancel button logic to fail.Key changes:
Fixed pay actions retrieval: Changed from per-transaction lookup to fetching all report actions at the report level using
getAllReportActions(report.reportID), then filtering for pay actions.Added payment type detection: Introduced
isPaidViaBankAccountto distinguish between bank payments (VBBA/ACH) and manual payments (ELSEWHERE). Uses type narrowing with'paymentType' in originalMessageforTypeScript safety.
Added bank processing state detection: Added
isBankProcessingcheck that handles both:Fixed NACHA cutoff operator: Changed from
<to>sohasDailyNachaCutoffPassedcorrectly returnstruewhen the current time is after the 23:45 UTC cutoff.Result:
Fixed Issues
$ #66225
PROPOSAL: #66225 (comment)
Tests
Action Performed:
Preconditions: 1- Admin of a workspace with the business bank account added. 2 - Member of the workspace, has a personal bank account.
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
Same as tests
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
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop