Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.56k stars 2.9k forks source link

[HOLD for payment 2024-11-22] [$250] Held expense - Negative amount in the payment button when the unheld expense comes from self #50497

Open IuliiaHerets opened 1 month ago

IuliiaHerets commented 1 month ago

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.46-4 Reproducible in staging?: Y Reproducible in production?: Y Email or phone of affected tester (no customers): applausetester+pso@applause.expensifail.com Issue reported by: Applause Internal Team

Action Performed:

  1. Go to staging.new.expensify.com
  2. [User A] Submit $100 to User B.
  3. [User B] Submit $20 to User A.
  4. [User B] Hold the expense submitted by User A.
  5. [User B] Go back to expense report.
  6. [User B] Note that the payment button has negative amount.

Expected Result:

The payment button will not have negative amount.

Actual Result:

The payment button shows negative amount. Error shows up when trying to pay the negative amount.

Workaround:

Unknown

Platforms:

Screenshots/Videos

https://github.com/user-attachments/assets/00a2003f-b674-414d-908e-8cc0e58d5f23

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021844594617993665700
  • Upwork Job ID: 1844594617993665700
  • Last Price Increase: 2024-10-11
  • Automatic offers:
    • akinwale | Reviewer | 104459635
    • mkzie2 | Contributor | 104459637
Issue OwnerCurrent Issue Owner: @stephanieelliott
melvin-bot[bot] commented 1 month ago

Triggered auto assignment to @stephanieelliott (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.

IuliiaHerets commented 1 month ago

We think that this bug might be related to #wave-collect - Release 1

IuliiaHerets commented 1 month ago

@stephanieelliott 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

mkzie2 commented 1 month ago

Proposal

Please re-state the problem that we are trying to solve in this issue.

The payment button shows negative amount. Error shows up when trying to pay the negative amount.

What is the root cause of that problem?

If any transaction is held, we display the unheld option first without checking whether the nonHeldAmount is negative or not

https://github.com/Expensify/App/blob/5d25b40eab1416d2d67624a376099853a2b6af86/src/components/MoneyReportHeader.tsx#L147

What changes do you think we should make in order to solve the problem?

  1. We can return other data like isNegativeNonHeldAmount in this function that will be true if the unheldTotal is a negative amount
if (hasUpdatedTotal(iouReport, policy) && hasPendingTransaction) {
    const unheldTotal = reportTransactions.reduce((currentVal, transaction) => currentVal + (!TransactionUtils.isOnHold(transaction) ? transaction.amount : 0), 0);

    return [
        CurrencyUtils.convertToDisplayString(unheldTotal * coefficient, iouReport?.currency),
        CurrencyUtils.convertToDisplayString((iouReport?.total ?? 0) * coefficient, iouReport?.currency),
        unheldTotal * coefficient < 0,
    ];
}

return [
    CurrencyUtils.convertToDisplayString((iouReport?.unheldTotal ?? 0) * coefficient, iouReport?.currency),
    CurrencyUtils.convertToDisplayString((iouReport?.total ?? 0) * coefficient, iouReport?.currency),
    (iouReport?.unheldTotal ?? 0) * coefficient < 0,
];

https://github.com/Expensify/App/blob/5d25b40eab1416d2d67624a376099853a2b6af86/src/libs/ReportUtils.ts#L7497-L7499

  1. Then we will check if isNegativeNonHeldAmount is true we should display the formattedAmount
const displayedAmount = isAnyTransactionOnHold && canAllowSettlement && !isNegativeNonHeldAmount ? nonHeldAmount : formattedAmount; 

https://github.com/Expensify/App/blob/5d25b40eab1416d2d67624a376099853a2b6af86/src/components/MoneyReportHeader.tsx#L147

  1. We also need to pass nonHeldAmount as undefined if isNegativeNonHeldAmount is true so we will not display the wrong paid-only option
nonHeldAmount={!hasOnlyHeldExpenses && !isNegativeNonHeldAmount  ? nonHeldAmount : undefined} 

https://github.com/Expensify/App/blob/5d25b40eab1416d2d67624a376099853a2b6af86/src/components/MoneyReportHeader.tsx#L406

Or we can add a !isNegativeNonHeldAmount check in these places to prevent showing the choose approve/paid option https://github.com/Expensify/App/blob/5d25b40eab1416d2d67624a376099853a2b6af86/src/components/MoneyReportHeader.tsx#L176

https://github.com/Expensify/App/blob/5d25b40eab1416d2d67624a376099853a2b6af86/src/components/MoneyReportHeader.tsx#L161

We need to do the same fix in ReportPreview

What alternative solutions did you explore? (Optional)

If we don't want to return isNegativeNonHeldAmount in getNonHeldAndFullAmount function, we can check inside each component

melvin-bot[bot] commented 1 month ago

Job added to Upwork: https://www.upwork.com/jobs/~021844594617993665700

melvin-bot[bot] commented 1 month ago

Triggered auto assignment to Contributor-plus team member for initial proposal review - @akinwale (External)

Jaytechent commented 1 month ago

Do I need to fund my account real time because I tested the web platform and it went well Screenshot_20241011-092120_Firefox

I sent 100 from user A, den user B submit 20. And user B still return 80( no negative ) and paid button didn't encounter error, I wanted to send video on upwork but it seems the handler may not reply me and in turn waste my connect and I can't afford to do that. Let me know if its solved or sth else.

melvin-bot[bot] commented 1 month ago

πŸ“£ @Jaytechent! πŸ“£ Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork. Please follow these steps:

  1. Make sure you've read and understood the contributing guidelines.
  2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
  3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
  4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details. Screen Shot 2022-11-16 at 4 42 54 PM Format:
    Contributor details
    Your Expensify account email: <REPLACE EMAIL HERE>
    Upwork Profile Link: <REPLACE LINK HERE>
Jaytechent commented 1 month ago

Contributor details Your Expensify account email: olemilek100@gmail.com Upwork Profile Link: https://www.upwork.com/freelancers/~0128e4d43ce1ad1755

melvin-bot[bot] commented 1 month ago

βœ… Contributor details stored successfully. Thank you for contributing to Expensify!

melvin-bot[bot] commented 1 month ago

@akinwale, @stephanieelliott Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

stephanieelliott commented 1 month ago

Thanks @Jaytechent! No, you should not need to fund the account. Did you Hold the $100 expense (step 4)? It doesn't look like you did, otherwise it would have a comment on it (the hold reason, which is required).

I was able to repro this: IMG_0943

stephanieelliott commented 1 month ago

@akinwale we have a proposal here -- can you review when you get a chance?

stephanieelliott commented 1 month ago

@akinwale we have a proposal https://github.com/Expensify/App/issues/50497#issuecomment-2402022672 -- can you review when you get a chance?

^^ bump @akinwale

akinwale commented 1 month ago

@stephanieelliott Thanks for the bump.

@mkzie2's proposal is adequate here.

πŸŽ€πŸ‘€πŸŽ€ C+ reviewed.

melvin-bot[bot] commented 1 month ago

Triggered auto assignment to @Beamanator, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

melvin-bot[bot] commented 1 month ago

πŸ“£ @akinwale πŸŽ‰ An offer has been automatically sent to your Upwork account for the Reviewer role πŸŽ‰ Thanks for contributing to the Expensify app!

Offer link Upwork job

melvin-bot[bot] commented 1 month ago

πŸ“£ @mkzie2 πŸŽ‰ An offer has been automatically sent to your Upwork account for the Contributor role πŸŽ‰ Thanks for contributing to the Expensify app!

Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review πŸ§‘β€πŸ’» Keep in mind: Code of Conduct | Contributing πŸ“–

stephanieelliott commented 3 weeks ago

Hey @akinwale this PR is awaiting your review, can you take a look?

stephanieelliott commented 2 weeks ago

PR is still being reviewed

stephanieelliott commented 5 days ago

PR on staging πŸŽ‰

melvin-bot[bot] commented 5 days ago

This issue has not been updated in over 15 days. @Beamanator, @akinwale, @stephanieelliott, @mkzie2 eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

mkzie2 commented 5 days ago

@stephanieelliott Please help to remove Monthly label.

melvin-bot[bot] commented 3 days ago

Reviewing label has been removed, please complete the "BugZero Checklist".

melvin-bot[bot] commented 3 days ago

The solution for this issue has been :rocket: deployed to production :rocket: in version 9.0.62-4 and is now subject to a 7-day regression period :calendar:. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2024-11-22. :confetti_ball:

For reference, here are some details about the assignees on this issue:

melvin-bot[bot] commented 3 days ago

@akinwale @stephanieelliott @akinwale The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]