-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Fix: "Waiting for you to pay expenses" shown incorrectly for $0.00 reports without Pay button #88612
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix: "Waiting for you to pay expenses" shown incorrectly for $0.00 reports without Pay button #88612
Changes from all commits
d9a1fec
4f47b80
bdadbdc
b094089
7219dd8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -102,6 +102,7 @@ function buildOptimisticNextStep(params: BuildNextStepNewParams): ReportNextStep | |
| const approverAccountID = bypassNextApproverID ?? getNextApproverAccountID(report, isUnapprove); | ||
| const reimburserAccountID = getReimburserAccountID(policy); | ||
| const hasValidAccount = !!policy?.achAccount?.accountNumber || policy?.reimbursementChoice !== CONST.POLICY.REIMBURSEMENT_CHOICES.REIMBURSEMENT_YES; | ||
| const {reimbursableSpend} = getMoneyRequestSpendBreakdown(report); | ||
|
|
||
| const nextStepFixOrPayExpense: ReportNextStep = { | ||
| messageKey: shouldShowFixMessage ? CONST.NEXT_STEP.MESSAGE_KEY.WAITING_TO_FIX_ISSUES : CONST.NEXT_STEP.MESSAGE_KEY.WAITING_TO_PAY, | ||
|
|
@@ -210,7 +211,7 @@ function buildOptimisticNextStep(params: BuildNextStepNewParams): ReportNextStep | |
| // Generates an optimistic nextStep once a report has been submitted | ||
| case CONST.REPORT.STATUS_NUM.SUBMITTED: { | ||
| if (policy?.approvalMode === CONST.POLICY.APPROVAL_MODE.OPTIONAL) { | ||
| nextStep = nextStepFixOrPayExpense; | ||
| nextStep = reimbursableSpend === 0 ? nextStepNoActionRequired : nextStepFixOrPayExpense; | ||
| break; | ||
| } | ||
|
|
||
|
|
@@ -244,7 +245,7 @@ function buildOptimisticNextStep(params: BuildNextStepNewParams): ReportNextStep | |
|
|
||
| // Generates an optimistic nextStep once a report has been approved | ||
| case CONST.REPORT.STATUS_NUM.APPROVED: | ||
| if (isInvoiceReport(report) || !isPayer(currentUserAccountIDParam, currentUserEmailParam, report, undefined)) { | ||
| if (isInvoiceReport(report) || !isPayer(currentUserAccountIDParam, currentUserEmailParam, report, undefined) || reimbursableSpend === 0) { | ||
| nextStep = nextStepNoActionRequired; | ||
| break; | ||
| } | ||
|
|
@@ -363,6 +364,12 @@ function getReportNextStep( | |
| currentUserEmail: string, | ||
| currentUserAccountID: number, | ||
| ) { | ||
| const {reimbursableSpend} = getMoneyRequestSpendBreakdown(moneyRequestReport); | ||
| const shouldShowNoFurtherAction = | ||
| reimbursableSpend === 0 && | ||
| (moneyRequestReport?.statusNum === CONST.REPORT.STATUS_NUM.APPROVED || | ||
| (moneyRequestReport?.statusNum === CONST.REPORT.STATUS_NUM.SUBMITTED && policy?.approvalMode === CONST.POLICY.APPROVAL_MODE.OPTIONAL)); | ||
|
Comment on lines
+368
to
+371
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. does this need to be hardcoded? has was it shown before
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I think this override is needed. The header still reads from the deprecated nextStep object via |
||
|
|
||
| if ( | ||
| isOpenExpenseReport(moneyRequestReport) && | ||
| transactions.length > 0 && | ||
|
|
@@ -381,6 +388,19 @@ function getReportNextStep( | |
| return buildOptimisticNextStepForPreventSelfApprovalsEnabled(); | ||
| } | ||
|
|
||
| if (shouldShowNoFurtherAction) { | ||
| // eslint-disable-next-line @typescript-eslint/no-deprecated -- The report header still consumes the deprecated nextStep shape, so we intentionally reuse the legacy builder here to override stale server nextStep data for $0 reports. | ||
| return buildNextStepNew({ | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Lint issue |
||
| report: moneyRequestReport, | ||
| policy, | ||
| currentUserAccountIDParam: currentUserAccountID, | ||
| currentUserEmailParam: currentUserEmail, | ||
| hasViolations: false, | ||
| isASAPSubmitBetaEnabled: false, | ||
| predictedNextStatus: moneyRequestReport?.statusNum ?? CONST.REPORT.STATUS_NUM.OPEN, | ||
| }); | ||
| } | ||
|
|
||
| return currentNextStep; | ||
| } | ||
| function buildOptimisticNextStepForDynamicExternalWorkflowSubmitError(iconFill?: string) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
App/src/libs/SearchUIUtils.ts
Line 2312 in c4ba24a
@marufsharifi just checking- does the report slip through these checks?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it can.
getActions()correctly removes PAY for a$0report, but before this change the section builders could still render the row based on status/type alone, so it could still appear in Ready to Pay.