Skip to content
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

[Held requests] Approve modal does not appear when one of the expense is held #41655

Closed
1 of 6 tasks
m-natarajan opened this issue May 5, 2024 · 69 comments
Closed
1 of 6 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Internal Requires API changes or must be handled by Expensify staff retest-weekly Apply this label if you want this issue tested on a Weekly basis by Applause

Comments

@m-natarajan
Copy link

m-natarajan commented May 5, 2024

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:
Reproducible in staging?: Needs reproduction
Reproducible in production?: Needs reproduction
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: @puneetlath
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1714845351600779

Deliverable

Provide reliable reproduction steps for the bug below. All assignees must be able to reproduce using the steps to be compensated.

Action Performed:

  1. Go to staging.new.expensify.com
  2. As an employee submit multiple expenses with scanned receipts
  3. Login as approver and open the report
  4. Put one scanned expense on hold
  5. Navigate to another report and navigate back to the original report
  6. Approve the report

NOTE: step 5 is just to forcefully trigger OpenReport which resets the transaction's hold state. The video in OP did not do the same but somehow it might trigger OpenReport (when and where to call this API are quite unreliable and we're working on it).

Expected Result:

Modal prompt to decide to approve the held or unheld total

Actual Result:

No modal displayed and both expense got approved.

Workaround:

Unknown

Platforms:

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

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Screen.Recording.2024-05-04.at.1.47.18.PM.mov

View all open jobs on GitHub

Issue OwnerCurrent Issue Owner: @AndrewGable
@m-natarajan m-natarajan added Daily KSv2 Needs Reproduction Reproducible steps needed Bug Something is broken. Auto assigns a BugZero manager. labels May 5, 2024
Copy link

melvin-bot bot commented May 5, 2024

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.

@MelvinBot
Copy link

This has been labelled "Needs Reproduction". Follow the steps here: https://stackoverflowteams.com/c/expensify/questions/16989

@stephanieelliott
Copy link
Contributor

I'm not able to reproduce this, adding the label to get it on the weekly retest rotation.

@stephanieelliott stephanieelliott added retest-weekly Apply this label if you want this issue tested on a Weekly basis by Applause Weekly KSv2 and removed Daily KSv2 labels May 7, 2024
@puneetlath
Copy link
Contributor

I experience it about 50% of the time when I try to approve with held expenses.

@mvtglobally
Copy link

Issue not reproducible during KI retests. (First week)

@melvin-bot melvin-bot bot added the Overdue label May 15, 2024
@stephanieelliott
Copy link
Contributor

Still trying to repro, I think there is a proposal in the works to create jobs for these so will keep an eye there bc I do think this is worth fixing.

@melvin-bot melvin-bot bot removed the Overdue label May 16, 2024
Copy link

melvin-bot bot commented May 19, 2024

@stephanieelliott this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

@trjExpensify trjExpensify changed the title Approve modal does not appear when one of the expense is held [Held requests] Approve modal does not appear when one of the expense is held May 21, 2024
@mallenexpensify mallenexpensify changed the title [Held requests] Approve modal does not appear when one of the expense is held [$125 Reproducible Steps] Held requests] Approve modal does not appear when one of the expense is held May 21, 2024
@mallenexpensify mallenexpensify added the External Added to denote the issue can be worked on by a contributor label May 21, 2024
Copy link

melvin-bot bot commented May 21, 2024

Unable to auto-create job on Upwork. The BZ team member should create it manually for this issue.

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label May 21, 2024
Copy link

melvin-bot bot commented May 21, 2024

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

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels May 21, 2024
@mallenexpensify
Copy link
Contributor

Updated title with prefix [$125 Reproducible Steps]
Updated OP to inc.

Deliverable

Provide reliable reproduction steps for the bug below. All assignees must be able to reproduce using the steps to be compensated.

Using this as a test issue for

@stephanieelliott
Copy link
Contributor

^^ using this issue to test drive the new process for Create jobs for “Needs Reproduction” issues to document reproducible steps.

@gijoe0295
Copy link
Contributor

gijoe0295 commented May 22, 2024

Proposal

Reproducible steps:

  1. Submit multiple expenses with scan expenses
  2. Put one scan expense on hold
  3. Navigate to another report and navigate back to the expense report
  4. Approve the expense report

I guess this has the same root cause with #40829 as I mentioned in #40829 (comment). That issue is still reproducible:

This is a backend issue, OpenReport API resets all the transaction's comment.

Note that step 3 is just to forcefully trigger OpenReport which resets the transaction's hold state. The video in OP did not do the same but somehow it might trigger OpenReport (when and where to call this API are quite unreliable and we're working on it).

Screenshot 2024-05-22 at 08 26 36 Screenshot 2024-05-22 at 08 12 14

@mvtglobally
Copy link

Issue not reproducible during KI retests. (Second week)

@gijoe0295
Copy link
Contributor

@eh2077 Could you review the reproduce steps above ^?

@melvin-bot melvin-bot bot removed the Overdue label Jul 22, 2024
@robertjchen
Copy link
Contributor

Just getting to this now, update shortly

@robertjchen
Copy link
Contributor

So, the reason we were holding off is that it's tangentially related to the optimistic changes from: #42573

We encountered this during testing there and needed to get those changes out to better isolate the issue.

Copy link

melvin-bot bot commented Jul 30, 2024

@robertjchen, @stephanieelliott, @eh2077 Eep! 4 days overdue now. Issues have feelings too...

Copy link

melvin-bot bot commented Aug 1, 2024

@robertjchen, @stephanieelliott, @eh2077 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

@robertjchen
Copy link
Contributor

Getting to this one before EOW, was focused on clearing out the other held issues raised this week.

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Aug 1, 2024
@robertjchen
Copy link
Contributor

Started looking into this on the backend over the weekend, was unable to pinpoint the source as the flows don't reveal anything obvious that would cause this discrepancy. Investigation continues!

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Aug 5, 2024
Copy link

melvin-bot bot commented Aug 8, 2024

@robertjchen, @stephanieelliott, @eh2077 Whoops! This issue is 2 days overdue. Let's get this updated quick!

@robertjchen
Copy link
Contributor

@melvin-bot melvin-bot bot removed the Overdue label Aug 9, 2024
@robertjchen
Copy link
Contributor

Potentially it would be a frontend fix- it seems like we're not optimistically updating that report/expense (preview?) such that we prevent immediate approval after the hold action

Though, I wonder if we wait long enough at the last step- eventually that preview becomes held? If so, that means the backend is okay and we can just focus on the frontend.

@melvin-bot melvin-bot bot added the Overdue label Aug 12, 2024
Copy link

melvin-bot bot commented Aug 12, 2024

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

@robertjchen
Copy link
Contributor

Confirming if it's backend or frontend or both

@melvin-bot melvin-bot bot removed the Overdue label Aug 13, 2024
@robertjchen
Copy link
Contributor

Slack Discussion here. We have a backend fix being deployed for this!

@robertjchen
Copy link
Contributor

The fix just went out! Let's re-test this

@robertjchen robertjchen added the retest-weekly Apply this label if you want this issue tested on a Weekly basis by Applause label Aug 15, 2024
@melvin-bot melvin-bot bot added the Overdue label Aug 19, 2024
@puneetlath
Copy link
Contributor

I confirmed it seems to be fixed for me. Let's make sure we get a new regression test for this scenario with scanning receipts @stephanieelliott

@robertjchen
Copy link
Contributor

Awesome, any other remaining steps here before we can close this one out?

@melvin-bot melvin-bot bot removed the Overdue label Aug 19, 2024
@stephanieelliott
Copy link
Contributor

@isagoico created the QA test for us ❤️ https://github.com/Expensify/Expensify/issues/421472

Seems we're all set to close now!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Internal Requires API changes or must be handled by Expensify staff retest-weekly Apply this label if you want this issue tested on a Weekly basis by Applause
Projects
No open projects
Status: Done
Development

No branches or pull requests