[$250] Invoice - 0.00 displayed on Pay button for invoice receiver
If you havenβt already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!
Version Number: 9.0.52-5 Reproducible in staging?: Y Reproducible in production?: Y If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/5113365 Issue reported by: Applause Internal Team
Action Performed:
- Go to https://staging.new.expensify.com/ and log in
- Create a workspace if there is no any
- Navigate to the workspace settings > More features
- Enable the Invoice option
- Click on FAB > Send invoice
- Select an invoice receiver
- Enter the company info and send the invoice
- Log in with the invoice receiver account in incognito
- Navigate to the invoice room
Expected Result:
The correct amount is displayed on the Pay button
Actual Result:
0.00 is displayed on the Pay button
Workaround:
Unknown
Platforms:
- [ ] Android: Standalone
- [ ] Android: HybridApp
- [ ] Android: mWeb Chrome
- [ ] iOS: Standalone
- [x] iOS: HybridApp
- [x] iOS: mWeb Safari
- [x] MacOS: Chrome / Safari
- [ ] MacOS: Desktop
Screenshots/Videos
https://github.com/user-attachments/assets/d265440b-b54a-44c7-b8c4-bc5581f0b9ba
Upwork Automation - Do Not Edit
- Upwork Job URL: https://www.upwork.com/jobs/~021849917912281837951
- Upwork Job ID: 1849917912281837951
- Last Price Increase: 2024-11-29
Issue Owner
Current Issue Owner: @MitchExpensifyIssue Owner
Current Issue Owner: @NikkiWines
Triggered auto assignment to @MitchExpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.
We think that this bug might be related to #vip-bills
@MitchExpensify FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors
Edited by proposal-police: This proposal was edited at 2024-11-05 08:36:07 UTC.
Proposal
Please re-state the problem that we are trying to solve in this issue.
0.00 displayed on Pay button for invoice receiver
What is the root cause of that problem?
We get the settlement amount using getSettlementAmount:
https://github.com/Expensify/App/blob/f251dbcdfdafc492d9a9908cbf40e4630b7b6d9d/src/components/ReportActionItem/ReportPreview.tsx#L237
We get the total reimbursableSpend to be displayed in the pay button form getMoneyRequestSpendBreakdown:
https://github.com/Expensify/App/blob/f251dbcdfdafc492d9a9908cbf40e4630b7b6d9d/src/components/ReportActionItem/ReportPreview.tsx#L133
We calculate reimbursableSpend using the formula:
https://github.com/Expensify/App/blob/f251dbcdfdafc492d9a9908cbf40e4630b7b6d9d/src/libs/ReportUtils.ts#L2761
But for a invoice report nonReimbursableSpend is going to be equal to the totalDisplaySpend as for invoices we request the amount from the user and not ask for reimbursement. So the value goes to zero and that is what is displayed on the FE.
This causes the 0 amount error
What changes do you think we should make in order to solve the problem?
We should check if the current report is an invoice report and for that case should directly pass the totalDisplaySpend as that would be the requested invoice amount:
const reimbursableSpend = isInvoiceReport(report) ? totalDisplaySpend : totalDisplaySpend - nonReimbursableSpend;
We also need to update the below: https://github.com/Expensify/App/blob/b87ec1ffed2887cf53cfb3bf25a34f691313a9bd/src/libs/actions/IOU.ts#L6798
To:
let total = (iouReport?.total ?? 0) - (isInvoiceReport ? 0 : iouReport?.nonReimbursableTotal ?? 0);
And then in MoneyReportView we calculate the formattedOutOfPocketAmount in the wrong way, it should be totalDisplaySpend - nonReimbursableSpend as that is what is owed for reimbursement.
https://github.com/Expensify/App/blob/cd3f30f73a18c29532d6886b4bb3744fd700ab9d/src/components/ReportActionItem/MoneyReportView.tsx#L62
const formattedOutOfPocketAmount = CurrencyUtils.convertToDisplayString(totalDisplaySpend - nonReimbursableSpend, report.currency);
What alternative solutions did you explore? (Optional)
Or when we get settlement amount below: https://github.com/Expensify/App/blob/f251dbcdfdafc492d9a9908cbf40e4630b7b6d9d/src/components/ReportActionItem/ReportPreview.tsx#L535
We can check if the current report is invoice and if so then call getDisplayAmount() for that case
Edited by proposal-police: This proposal was edited at 2024-11-04 16:27:36 UTC.
Proposal
Please re-state the problem that we are trying to solve in this issue.
0.00 displayed on Pay button for invoice receiver
What is the root cause of that problem?
Since the nonReimbursableTotal for invoice reports is the same as it's total, the reimbursableSpend, which is calculated from the ReportPreview.getMoneyRequestSpendBreakdown function, will be 0.
https://github.com/Expensify/App/blob/5e975f5aedac0d150984282ada45eed1693bcdb6/src/libs/ReportUtils.ts#L2760-L2761
What changes do you think we should make in order to solve the problem?
Let's create a condition in the ReportPreview and MoneyReportHeader components to display the total instead of the reimbursableSpend if the report is an invoice report.
https://github.com/Expensify/App/blob/5e975f5aedac0d150984282ada45eed1693bcdb6/src/components/ReportActionItem/ReportPreview.tsx#L246
const formattedAmount = CurrencyUtils.convertToDisplayString(
ReportUtils.isInvoiceReport(moneyRequestReport) ? moneyRequestReport.total : reimbursableSpend,
iouReport?.currency
);
https://github.com/Expensify/App/blob/aedbbde0317b720151a75dbe0b6a7ddea52a20cd/src/components/MoneyReportHeader.tsx#L171
return CurrencyUtils.convertToDisplayString(
isInvoiceRoom ? totalDisplaySpend : reimbursableSpend,
iouReport?.currency
);
Finally, let's change how we calculate total in the getPayMoneyRequestParams function, like below, to only use the value of iouReport?.total by excluding the value of iouReport?.nonReimbursableTotal in the calculation only when if the report is an invoice report.
https://github.com/Expensify/App/blob/b87ec1ffed2887cf53cfb3bf25a34f691313a9bd/src/libs/actions/IOU.ts#L6798
let total = (iouReport?.total ?? 0) - (!isInvoiceReport && (iouReport?.nonReimbursableTotal ?? 0));
Job added to Upwork: https://www.upwork.com/jobs/~021849917912281837951
Lets fix!
Triggered auto assignment to Contributor-plus team member for initial proposal review - @dukenv0307 (External)
Edited by proposal-police: This proposal was edited at 2024-11-05 11:05:55 UTC.
Proposal
Please re-state the problem that we are trying to solve in this issue.
Invoice - 0.00 displayed on Pay button for invoice receiver
What is the root cause of that problem?
reimbursable spend is calculated by subtracting non-reimbursable from the total
https://github.com/Expensify/App/blob/a86ee35a6c9706d8acb35202acb4cdaa1cc769e7/src/libs/ReportUtils.ts#L2776
but for invoice nonReimbursableSpend is equal to the total
What changes do you think we should make in order to solve the problem?
We should add an exception for invoices and set nonReimbursableSpend 0, so:
nonReimbursableSpend = isInvoiceReport(report) ? 0 : nonReimbursableSpend;
const reimbursableSpend = totalDisplaySpend - nonReimbursableSpend;
then also here https://github.com/Expensify/App/blob/a86ee35a6c9706d8acb35202acb4cdaa1cc769e7/src/libs/actions/IOU.ts#L6784
let total = (iouReport?.total ?? 0) - (isInvoiceReport ? 0 : iouReport?.nonReimbursableTotal ?? 0);
What alternative solutions did you explore? (Optional)
If we don't want to update the nonReimbursableSpend We can update only the reimbursableSpend
const reimbursableSpend = totalDisplaySpend - isInvoiceReport(report) ? 0 : nonReimbursableSpend;
and we disable showing break down in money report view if it is invoice report because for invoices the total amount is all reimbursable https://github.com/Expensify/App/blob/cd3f30f73a18c29532d6886b4bb3744fd700ab9d/src/components/ReportActionItem/MoneyReportView.tsx#L60
const shouldShowBreakdown = !ReportUtils.isInvoiceReport(report) && nonReimbursableSpend && reimbursableSpend && shouldShowTotal;
we can also use nonReimbursableSpend!==totalDisplaySpend condition instead
Reviewing...
@FitseTLT @twilight2294 Do you think it's BE bug since it returns nonReimbursableTotal same as total
No, Non-reimbursable expenses are costs that an organization or employer will not compensate for.
For invoices we ask the end user to pay the amount and hence it i same as total, @dukenv0307 my proposal here solves the bug correctly
@twilight2294 If so, we still need to fix the total here like @FitseTLT's suggestion.
Here's the result when I apply your solution
@dukenv0307 , @FitseTLT ' RCA and solution is essential the exact same as my proposal with one extra case. I feel I was the first one to propose the changes required to fix the originally reported bug so i should be assigned to this bug. Our contributor guidelines also state:
Before submitting a proposal on an issue, be sure to read any other existing proposals. ALL NEW PROPOSALS MUST BE DIFFERENT FROM EXISTING PROPOSALS.
If at all that extra case is important, i can fix that in my PR and can share say ~$25 out of my compensation as the solution is same and it's just a minor change during PR phase.
And also as @FitseTLT suggests to change the nonReimbursableSpend, nonReimbursableSpend shouldn't be changed here, as it is a non-zero value in the Backend so essentially he is saying that BE is giving a wrong value than frontend. so his solution is a workaround (hacky solution) than a clean solution depending on the type of report. My solution deals with the way we get the total amount for the spend, but @FitseTLT proposal suggests to change the nonReimbursableSpend altogether which is in contrast with the BE value.
@dukenv0307 I am surprised how you are taking this matter into a discussion because the missed point is important and will obviously create a regression and also you shouldn't forget the suggested changes were
const reimbursableSpend = isInvoiceReport(report) ? totalDisplaySpend : totalDisplaySpend - nonReimbursableSpend;
which will only fix reimbursableSpend but doesn't fix the value for nonReimbursableSpend here
https://github.com/Expensify/App/blob/f251dbcdfdafc492d9a9908cbf40e4630b7b6d9d/src/libs/ReportUtils.ts#L2764
that would cause the out-of-pocket amount to be displayed in money report
Thanks for all your proposal. You guys provide the correct RCA.
I'm not sure the nonReimbursableSpend returned from BE is correct or not, if we don't use it anywhere, it should be 0.
In case it's correct, I like @FitseTLT's proposal since it does not only fix the displayed amount in pay button, but also the amount in action (after pay).
πππ C+ reviewed
Triggered auto assignment to @NikkiWines, see https://stackoverflow.com/c/expensify/questions/7972 for more details.
@NikkiWines I don't think @FitseTLT 's solution is correct:
@FitseTLT suggests to change the nonReimbursableSpend, nonReimbursableSpend shouldn't be changed here, as it is a non-zero value in the Backend so essentially he is saying that BE is giving a wrong value than frontend. so his solution is a workaround (hacky solution) than a clean solution depending on the type of report. My solution deals with the way we get the total amount for the spend, but @FitseTLT proposal suggests to change the nonReimbursableSpend altogether which is in contrast with the BE value.
π£ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? πΈ
Sorry for the delay. I'm not sure either solution is entirely correct to be honest.
@twilight2294 you're correct in your statement here:
For invoices we ask the end user to pay the amount and hence it i same as total
But your solution doesn't fully resolve the issue, since (as @FitseTLT outlined) there are a couple of other cases where we end up having a display mismatches due to the same type of totalDisplaySpend - nonReimbursableSpend calculation.
It's true that this issue was opened for this particular flow, but we prefer more comprehensive solutions that solve the problem in its entirety, if there's the same bug in multiple flows/areas.
@FitseTLT, I don't think we want to modify nonReimbursableSpend here - @twilight2294 is right that value correct, it's the logic used to determine totalDisplaySpend (or total) that's incorrect (in multiple locations)
Ideally, I think we'd add a case or adjustment in all the relevant areas to adjust the totalSpend/total calculation, based on whether or not the report is an invoice - instead of adjusting the value for reimbursableSpend/ nonReimbursableSpend themselves. Hope this makes sense!
I agree with you @NikkiWines !, how should we move forward here ?
@FitseTLT, I don't think we want to modify nonReimbursableSpend here - @twilight2294 is right that value correct, it's the logic used to determine totalDisplaySpend (or total) that's incorrect (in multiple locations)
@NikkiWines @twilight2294 is not correct. If the value of nonReimbursableSpend is not going to be fixed then the money report view will show out of pocket(non reimbursable) and Company Spend(reimbursable) value equal to the total which is wrong and confusing and that should be fixed too.
Proposal
- Added solution for calculating the value of
totalin thegetPayMoneyRequestParamsfunction.
Ideally, I think we'd add a case or adjustment in all the relevant areas to adjust the totalSpend/total calculation, based on whether or not the report is an invoice - instead of adjusting the value for reimbursableSpend/ nonReimbursableSpend themselves. Hope this makes sense!
According this comment, the solution for my proposal already did not adjust or modify the value for reimbursableSpend/ nonReimbursableSpend in the getMoneyRequestSpendBreakdown function.
Not overdue, we're discussing the solution and the ball seems to be in @NikkiWines 's court right now
@NikkiWines, @MitchExpensify, @dukenv0307 Eep! 4 days overdue now. Issues have feelings too...
What are your thoughts on next steps here @dukenv0307 ?
I agree with you @NikkiWines !, how should we move forward here ?
@twilight2294 The best thing to do would be to update your proposal, as @Tony-MK has done, to account for a new approach.
@NikkiWines @twilight2294 is not correct. If the value of nonReimbursableSpend is not going to be fixed then the money report view will show out of pocket(non reimbursable) and Company Spend(reimbursable) value equal to the total which is wrong and confusing and that should be fixed too.
@FitseTLT I think there may be a misunderstanding. I agree that we need to update the view to show the correct value for out of pocket spend when the report is an invoice - but I think the correct way to do that is by accounting for whether or not the report is an invoice when we're determining the amount to display in the component itself - not by changing the nonReimbursableSpend value that we're returning from the backend. Hopefully that makes sense?
According this https://github.com/Expensify/App/issues/51312#issuecomment-2452582144, the solution for my https://github.com/Expensify/App/issues/51312#issuecomment-2433575175 already did not adjust or modify the value for reimbursableSpend/ nonReimbursableSpend in the getMoneyRequestSpendBreakdown function.
@dukenv0307, can you review @Tony-MK's updated proposal, please π However, @Tony-MK, I don't think we'd want to update convertToDisplayString to do anything over than convert the inputted value to a formatted string, as that muddies the purpose of the function.
@NikkiWines I'm checking the new proposal, will give the updates soon
@dukenv0307, can you review @Tony-MK's updated proposal, please π However, @Tony-MK, I don't think we'd want to update convertToDisplayString to do anything over than convert the inputted value to a formatted string, as that muddies the purpose of the function.
I believe totalDisplaySpend is already a number but there are already several functions where totalDisplaySpend is used with convertToDisplayString like in my proposal such as getReportAutomaticallyForwardedMessage, getIOUForwardedMessage and getIOUReportActionMessage.