-
Notifications
You must be signed in to change notification settings - Fork 3k
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
[HOLD for payment 2024-07-24] [$250] Expense - Payment buttons show different amount for report consisting of submitted & tracked expense #44158
Comments
Triggered auto assignment to @slafortune ( |
@slafortune 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 |
We think that this bug might be related to #wave-collect - Release 1 |
ProposalPlease re-state the problem that we are trying to solve in this issue.The amount in report preview button and in expense report button is different. What is the root cause of that problem?In report preview, we always use the total amount of the expense. App/src/components/ReportActionItem/ReportPreview.tsx Lines 205 to 208 in 68374a5
In expense report header, we use the App/src/components/MoneyReportHeader.tsx Line 123 in 68374a5
The expense report header also have more condition to show the amount, for example, if there is an held expense, then we only show the non-hold amount. App/src/components/MoneyReportHeader.tsx Line 125 in 68374a5
But if every expense are held, then we don't show the amount. App/src/components/MoneyReportHeader.tsx Line 242 in 68374a5
What changes do you think we should make in order to solve the problem?To make it consistent, we can follow the condition/logic from the expense report header. We will create a new function called
Also, I found that it's possible that the amount is 0 when we have 1 submitted expense and 1 tracked expense and hold the submitted expense. The non-held amount in this case will be -0. If we don't want to show 0, then we can return an empty string if it's 0. |
Job added to Upwork: https://www.upwork.com/jobs/~019e4af48063736ac0 |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @thesahindia ( |
@bernhardoj's proposal looks good to me! 🎀 👀 🎀 C+ reviewed |
Triggered auto assignment to @dangrous, see https://stackoverflow.com/c/expensify/questions/7972 for more details. |
whattt I thought I posted a comment here yesterday. Well, it said basically: This proposal makes sense! But, I want to confirm internally to make sure there's not a specific reason that these DON'T currently match (like, from an accounting or other perspective). I've asked in Slack and should hear back soon! (internal link: https://expensify.slack.com/archives/C03U7DCU4/p1719527792898199) |
reasked, that was a bad time to do it I think. Hopefully will have an answer soon! If I hear nothing, I think we can move forward because to me it seems to make sense that they should match. |
rude, melvin. |
Okay after discussing internally, we're closing this out as expected behavior - thanks so much for the proposal and review @thesahindia and @bernhardoj Basically the thought is the preview shows all charges, the pay button only shows what you can actually pay currently, and so both serve different purposes. |
PR is ready cc: @thesahindia |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.7-8 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2024-07-24. 🎊 For reference, here are some details about the assignees on this issue:
|
BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:
|
not overdue, payment tomorrow. |
Requested in ND. |
Payment Summary
BugZero Checklist (@slafortune)
|
@bernhardoj can you please complete the checklist? |
@slafortune @thesahindia is the C+ here. |
Ah yes - sorry about that! @thesahindia can you please complete the checklist? |
@bernhardoj role Contributor - owed $250 via NewDot |
$250 approved for @bernhardoj |
@dangrous, @slafortune, @bernhardoj, @thesahindia Uh oh! This issue is overdue by 2 days. Don't forget to update your issues! |
It was like this from the beginning. If we want we can add a test case, here are the steps:-
|
cc - @JmillsExpensify |
$250 approved for @thesahindia |
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Version Number: 9.0.0.0
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause internal team
Slack conversation:
Action Performed:
Expected Result:
The amount on the payment button in expense preview in the main chat and expense report should be the same.
Actual Result:
The amount on the payment button in expense preview in the main chat and expense report are different.
Main chat shows $300.
Expense report shows $200.
Workaround:
unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Bug6520331_1718960940842.20240621_170327.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @slafortuneThe text was updated successfully, but these errors were encountered: