search

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.54k stars 2.89k forks source link

[HOLD for payment 2024-10-24] [$250] Hold - Edited amount is not updated for hold report when approve partial report #49269

Closed IuliiaHerets closed 1 week 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.35-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: https://expensify.testrail.io/index.php?/tests/view/4964020&group_by=cases:section_id&group_order=asc&group_id=309128 Email or phone of affected tester (no customers): gocemate+workspaceapprover183@gmail.com Issue reported by: Applause Internal Team

Action Performed:

Precondition:

  1. Login as the owner of the workspace
  2. Create a workspace
  3. Invite the approver and employee
  4. Navigate to more features
  5. Enable "workflows"
  6. On the "Workflow" editor - enable "Add Approvals"
  7. Set the Approver account as the Approver

Steps:

  1. As Employee submit 2 manual expenses to the workspace chat
  2. As Approver hold the first expense
  3. As Employee update the first expense amount
  4. As Approver click on green Approve button> Approve the report partially
  5. As Approver navigate back to held expense
  6. Check the amount of the held request

Expected Result:

Amount should be updated since employee change the amount

Actual Result:

Held expense shows old amount, instead of the updated amount

Workaround:

Unknown

Platforms:

Screenshots/Videos

https://github.com/user-attachments/assets/21293f4f-e383-4b5c-9c05-42bfb3e72549

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021837119590958615915
  • Upwork Job ID: 1837119590958615915
  • Last Price Increase: 2024-09-20
  • Automatic offers:
    • shubham1206agra | Reviewer | 104090038
    • nkdengineer | Contributor | 104090041
Issue OwnerCurrent Issue Owner: @JmillsExpensify
melvin-bot[bot] commented 1 month ago

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

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

nkdengineer commented 1 month ago

Proposal

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

Held expense shows old amount, instead of the updated amount

What is the root cause of that problem?

We pass the total param when creating an optimistic expense report as firstHoldTransaction?.amount which is the original amount when we create this transaction, it's not the updated amount

https://github.com/Expensify/App/blob/14b99ca0a12e9686818bc3e937f091199de69750/src/libs/actions/IOU.ts#L6447

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

We should use firstHoldTransaction?.modifiedAmount first if it exists

(firstHoldTransaction?.modifiedAmount ?? firstHoldTransaction?.amount  ?? 0) * -1,

https://github.com/Expensify/App/blob/14b99ca0a12e9686818bc3e937f091199de69750/src/libs/actions/IOU.ts#L6447

Or we can use TransactionUtils.getAmount function that already implemented this logic

TransactionUtils.getAmount(firstHoldTransaction) * -1,

https://github.com/Expensify/App/blob/14b99ca0a12e9686818bc3e937f091199de69750/src/libs/actions/IOU.ts#L6447

What alternative solutions did you explore? (Optional)

melvin-bot[bot] commented 1 month ago

@miljakljajic Whoops! This issue is 2 days overdue. Let's get this updated quick!

melvin-bot[bot] commented 1 month ago

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

melvin-bot[bot] commented 1 month ago

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

melvin-bot[bot] commented 1 month ago

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

miljakljajic commented 1 month ago

Headed out on my parental leave so reassigning this one out. Thank you @JmillsExpensify

Next step: @shubham1206agra please review @nkdengineer 's proposal when you have a chance

shubham1206agra commented 1 month ago

@nkdengineer's proposal looks good.

🎀👀🎀 C+ reviewed

melvin-bot[bot] commented 1 month ago

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

melvin-bot[bot] commented 1 month ago

📣 @shubham1206agra 🎉 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

📣 @nkdengineer 🎉 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 📖

melvin-bot[bot] commented 3 weeks ago

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

melvin-bot[bot] commented 3 weeks ago

The solution for this issue has been :rocket: deployed to production :rocket: in version 9.0.49-2 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-10-24. :confetti_ball:

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

melvin-bot[bot] commented 3 weeks ago

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:

JmillsExpensify commented 2 weeks ago

Payment summary:

Please don't forget the checklist!

melvin-bot[bot] commented 1 week ago

@JmillsExpensify, @blimpich, @shubham1206agra, @nkdengineer Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

JmillsExpensify commented 1 week ago

Contributor paid out. Still waiting on reviewer's checklist to issue last payment.

shubham1206agra commented 1 week ago

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:

  1. [Employee] Submit 2 manual expenses to the workspace chat.
  2. [Approver] Hold the first expense.
  3. [Employee] Update the first expense amount.
  4. [Approver] Click on green Approve button> Approve the report partially.
  5. [Approver] Navigate back to held expense.
  6. Confirm that the amount of the held request is correct.
JmillsExpensify commented 1 week ago

Regression test linked above. @shubham1206agra it looks like you didn't accept the automatic offer, so I've just send you a new one.

JmillsExpensify commented 1 week ago

I'm trying to pay you out @shubham1206agra but I'm having technical issues on the Upwork side. Will pay you out as soon as it's resolved.

JmillsExpensify commented 1 week ago

Sent you another offer. The original one is affected by some Upwork bug.

JmillsExpensify commented 1 week ago

That one worked. Closing this issue out.