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

[Awaiting Payment Sept 10th] [$250] [Held requests] Hold badge remains on paid expense until refresh the page #44616

Closed
1 of 6 tasks
lanitochka17 opened this issue Jun 28, 2024 · 38 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor

Comments

@lanitochka17
Copy link

lanitochka17 commented Jun 28, 2024

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.3-1
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/4678849&group_by=cases:section_id&group_order=asc&group_id=309128
Email or phone of affected tester (no customers): gocemate+a445@gmail.com
Issue reported by: Applause - Internal Team

Action Performed:

  1. User A creates expense with User B
  2. User B navigates to transaction thread and select Hold from 3-dot menu
  3. User B pays the expense elsewhere
  4. Take a note of Hold badge on the chat header on both User A and User B

Expected Result:

Since the expense is payed hold status should not be displayed anymore

Actual Result:

Hold status remains displayed even expense is payed

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

Bug6527187_1719565919023.Recording__3361.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0197333211c4d3993e
  • Upwork Job ID: 1821527955232770128
  • Last Price Increase: 2024-08-30
Issue OwnerCurrent Issue Owner: @
@lanitochka17 lanitochka17 added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jun 28, 2024
Copy link

melvin-bot bot commented Jun 28, 2024

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

@lanitochka17
Copy link
Author

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

@bernhardoj
Copy link
Contributor

Proposal

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

The hold banner doesn't disappear until we refresh the page for a paid expense.

What is the root cause of that problem?

When we pay the expense, we don't optimistically clear the hold status of the report. The pusher or API response doesn't clear the hold status either until we reload the page (from OpenReport).

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

BE: clears the hold status from the API response and/or pusher.
FE: Optimistically clear the hold status
In IOU.getPayMoneyRequestParams, add below optimistic data:

...(full ? TransactionUtils.getAllReportTransactions(iouReport.reportID).map((transaction) => ({
    onyxMethod: Onyx.METHOD.MERGE,
    key: `${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`,
    value: {
        comment: {
            hold: null,
        },
    }
})) : []),

(and revert it back in failureData)

What the code above does is, get all the expense report transactions and clear the hold status. We only do this if the full is true which means we pay all hold and non-hold requests.

@trjExpensify
Copy link
Contributor

I have a feeling @robertjchen is working on fixing this already. Maybe he can confirm.

Bit of a sidebar, but for the whack copy and stuff, you're talking care of that here: #42736

@melvin-bot melvin-bot bot added the Overdue label Jul 1, 2024
@robertjchen
Copy link
Contributor

Yes! This should be addressed with the optimistic changes in: #42573

We can put this on hold until those changes are out and then re-test after to make sure this is fixed 👍

@trjExpensify trjExpensify changed the title Hold - Hold badge remains on paid expense until refresh the page [Hold #42573] [Held requests] Hold badge remains on paid expense until refresh the page Jul 1, 2024
@trjExpensify
Copy link
Contributor

Cool, done!

@melvin-bot melvin-bot bot removed the Overdue label Jul 1, 2024
@trjExpensify trjExpensify added Weekly KSv2 and removed Daily KSv2 labels Jul 2, 2024
@melvin-bot melvin-bot bot added the Overdue label Jul 10, 2024
@trjExpensify
Copy link
Contributor

Still on hold for #42573

@melvin-bot melvin-bot bot removed the Overdue label Jul 11, 2024
@melvin-bot melvin-bot bot added the Overdue label Jul 19, 2024
@trjExpensify
Copy link
Contributor

Same, melvin. Keep holdin', holdin', holdin'... yeah.

@melvin-bot melvin-bot bot removed the Overdue label Jul 22, 2024
@melvin-bot melvin-bot bot added the Overdue label Jul 31, 2024
@trjExpensify
Copy link
Contributor

Alright, that PR has been deployed to prod: #42573 (comment)

@melvin-bot melvin-bot bot removed the Overdue label Aug 5, 2024
@trjExpensify trjExpensify changed the title [Hold #42573] [Held requests] Hold badge remains on paid expense until refresh the page [Held requests] Hold badge remains on paid expense until refresh the page Aug 5, 2024
@trjExpensify trjExpensify added the retest-weekly Apply this label if you want this issue tested on a Weekly basis by Applause label Aug 5, 2024
@trjExpensify
Copy link
Contributor

Taking this off hold. @lanitochka17 can we get a re-test please? Thanks!

@lanitochka17
Copy link
Author

Issue is still reproducible

Recording.3666.mp4

@trjExpensify
Copy link
Contributor

Cool, shipping on.

@trjExpensify trjExpensify moved this from Done to Polish in [#whatsnext] #wave-collect Aug 23, 2024
@trjExpensify trjExpensify removed the retest-weekly Apply this label if you want this issue tested on a Weekly basis by Applause label Aug 26, 2024
@robertjchen
Copy link
Contributor

PR in final review

@robertjchen robertjchen changed the title [Held requests] Hold badge remains on paid expense until refresh the page [$250] [Held requests] Hold badge remains on paid expense until refresh the page Aug 30, 2024
Copy link

melvin-bot bot commented Aug 30, 2024

Upwork job price has been updated to $250

@robertjchen robertjchen added External Added to denote the issue can be worked on by a contributor and removed Internal Requires API changes or must be handled by Expensify staff labels Aug 30, 2024
@robertjchen
Copy link
Contributor

FE PR merged 👍

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

melvin-bot bot commented Aug 30, 2024

Current assignee @getusha is eligible for the External assigner, not assigning anyone new.

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Aug 30, 2024
@robertjchen robertjchen added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production Reviewing Has a PR in review and removed Daily KSv2 Help Wanted Apply this label when an issue is open to proposals by contributors Reviewing Has a PR in review Awaiting Payment Auto-added when associated PR is deployed to production labels Aug 30, 2024
@trjExpensify trjExpensify changed the title [$250] [Held requests] Hold badge remains on paid expense until refresh the page [Awaiting Payment Sept 10th] [$250] [Held requests] Hold badge remains on paid expense until refresh the page Sep 10, 2024
@trjExpensify trjExpensify added Daily KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Reviewing Has a PR in review Weekly KSv2 labels Sep 10, 2024
@trjExpensify
Copy link
Contributor

Payments are due here today. Payment summary as follows:

Both of you can go ahead and request on NewDot, closing!

@bernhardoj
Copy link
Contributor

Requested in ND.

@JmillsExpensify
Copy link

$250 approved for @bernhardoj

@garrettmknight
Copy link
Contributor

$250 approved for @getusha

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor
Projects
No open projects
Status: Done
Development

No branches or pull requests

8 participants