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.29k stars 2.72k forks source link

[$250] IOU - Multiple IOUs are unapproved when a single IOU is unapproved #44969

Closed izarutskaya closed 1 month ago

izarutskaya 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: v9.0.5-3 Reproducible in staging?: Y Reproducible in production?: N Email or phone of affected tester (no customers): natnael.expensify+0a89fdndafadf@gmail.com Logs: https://stackoverflow.com/c/expensify/questions/4856 Issue reported by: Applause-Internal team

Action Performed:

Precondition: Create workspace > More features > Enable workflow > Enable Add approvals > Make yourself approver

  1. Navigate to a workspace chat
  2. Submit 2 manual requests with different amounts
  3. Click on the IOU preview component
  4. Click Approve button
  5. Click on the IOUs
  6. Click header
  7. Click unapprove
  8. Navigate back to expense report

Expected Result:

Only the unapproved report should be unapproved

Actual Result:

Both IOUs are unapproved

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

Screenshots/Videos

https://github.com/Expensify/App/assets/115492554/d19669f2-21cd-4e44-8a10-9f2ea11e20ea

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0166593468459204f3
  • Upwork Job ID: 1810443215436393315
  • Last Price Increase: 2024-07-08
Issue OwnerCurrent Issue Owner: @lschurr
melvin-bot[bot] commented 1 month ago

Triggered auto assignment to @stitesExpensify (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

melvin-bot[bot] commented 1 month ago

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

github-actions[bot] commented 1 month ago

:wave: Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.
izarutskaya commented 1 month ago

@lschurr 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.

izarutskaya commented 1 month ago

We think this issue might be related to the #collect project.

izarutskaya commented 1 month ago

Production image

lschurr commented 1 month ago

Hi hi @stitesExpensify - Can you check on this and confirm if it's a deploy blocker? Do we need to add External here?

stitesExpensify commented 1 month ago

Checking

melvin-bot[bot] commented 1 month ago

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

melvin-bot[bot] commented 1 month ago

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

stitesExpensify commented 1 month ago

Commented here https://github.com/Expensify/App/pull/44229

bernhardoj commented 1 month ago

Proposal

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

All transactions are unapproved when a single transaction is unapproved.

What is the root cause of that problem?

The approve option is available on the expense report which will approve all the transactions, so it makes sense if unapprove will unapprove all transactions too. They both work on the expense report level, not on each transaction.

So, I think the real issue here is that the Unapprove option is shown on the transaction thread details page. That's because when we check whether the report is a money request report or not, we always pass the money request report itself, which is true for the transaction thread too. https://github.com/Expensify/App/blob/7b558ef7fb9908223d899b54ad3d0156bc8fcb81/src/pages/ReportDetailsPage.tsx#L202-L205

The moneyRequestReport variable holds the parentx money request report of the transaction. https://github.com/Expensify/App/blob/7b558ef7fb9908223d899b54ad3d0156bc8fcb81/src/pages/ReportDetailsPage.tsx#L184-L189

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

We should check for the current report instead of the moneyRequestReport. https://github.com/Expensify/App/blob/7b558ef7fb9908223d899b54ad3d0156bc8fcb81/src/pages/ReportDetailsPage.tsx#L202-L205

ReportUtils.isMoneyRequestReport(report) && ...
suneox commented 1 month ago

Thank @bernhardoj for the proposal. I’ll start reviewing it.

suneox commented 1 month ago

I’m still stuck when trying to approve an expense due to the requirement to add a payment card. I have tried a lot of test cards provided by stripe, bluesnap, plaid but none of them worked. @stitesExpensify, do you have documentation on how to bypass the “add payment card” step in a dev environment?

danieldoglas commented 1 month ago

I don't think we need to block the backend on this!

Beamanator commented 1 month ago

@bernhardoj i agree with your assessment & proposed solution 👍 I'm already working on a PR to fix another blocker so I will test out your theory & implement it, if you don't mind (you'll be compensated for the solution if it works, of course)

Beamanator commented 1 month ago

Fix merged & CPing

Beamanator commented 1 month ago

Should be good now, IOUs are no longer Unapprovable 👍 tested in 9.0.5-6 👍

Beamanator commented 1 month ago

Keeping open so @bernhardoj can get paid for his solution here that I implemented in https://github.com/Expensify/App/pull/45049 (along with another small change) 🙏 - cc @lschurr

melvin-bot[bot] commented 1 month ago

@Beamanator, @suneox, @lschurr, @stitesExpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

Beamanator commented 1 month ago

@lschurr can you help us retest? This should have been fixed already

lschurr commented 1 month ago

Yes, I'm a bit behind but can test tomorrow - @bernhardoj @suneox are you able to jump in and test this?

bernhardoj commented 1 month ago

Tested on staging, the Unapprove button doesn't show on the transaction thread anymore 🎉

https://github.com/user-attachments/assets/2b08c4ed-0991-44f4-8b0a-ae96464062f1

lschurr commented 1 month ago

Woop! Amazing. Is there a PR associated with this specific GH that requires payment or are we good to close?

Beamanator commented 1 month ago

PR was https://github.com/Expensify/App/pull/45049! But it fixed multiple issues FYI 🙏

lschurr commented 1 month ago

Ok, so payment is being issued elsewhere? Or do we need to pay here @Beamanator?

melvin-bot[bot] commented 1 month ago

@Beamanator @suneox @lschurr @stitesExpensify this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

garrettmknight commented 1 month ago

@lschurr I think it's just payment for @bernhardoj here via this comment.

lschurr commented 1 month ago

Great, thanks!

Payment summary:

bernhardoj commented 1 month ago

Requested in ND.

JmillsExpensify commented 1 month ago

$250 approved for @bernhardoj