-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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] [$250] Expense on hold is displayed as single expense when employee hold it second time #45276
Comments
Triggered auto assignment to @sakluger ( |
@sakluger 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 |
We think that this bug might be related to #wave-collect - Release 1 |
Job added to Upwork: https://www.upwork.com/jobs/~0126f16401c1dcd30e |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @hoangzinh ( |
Issue Report: Expense on Hold is Displayed as a Single Expense when Employee Holds it a Second Time on Expensify Summary: When an employee puts an expense on hold for the second time, it is displayed as a single expense on Expensify, rather than showing the hold history. This can lead to confusion and make it difficult to track the expense's status. Steps to Reproduce: An employee submits an expense report with an expense. The expense should display a hold history, showing that it was previously held and the reasons for the hold. Actual Result: The expense is displayed as a single expense, with no indication that it was previously held. Impact: This issue can cause confusion and make it difficult to track the status of expenses. It may also lead to delays in expense approval and reimbursement. Proposed Solution: To resolve this issue, we recommend updating the Expensify platform to display the hold history for expenses that have been put on hold multiple times. This could be achieved by: Adding a "Hold History" section to the expense details page, which displays a list of all holds, including the date, reason, and user who initiated the hold. We estimate X hours to implement this solution and update the Expensify platform to display hold history for expenses. Priority: We consider this issue a medium priority, as it affects the usability and functionality of the Expensify platform. Please let us know if you would like to discuss this issue further or proceed with implementation. |
Hi @kabeer95, can you follow the PROPOSAL_TEMPLATE and also provide a root cause of this issue? Thank you. |
Proposal: Fix Expense on Hold Display IssueProblem Statement: Root Cause: Changes to Solve the Problem: Update
|
@hoangzinh it's time first time trying to contribute to the Expensify issues , kindly avoid the basic mistakes |
Sure @kabeer95. Can you follow the proposal format here again, please https://github.com/Expensify/App/blob/main/contributingGuides/PROPOSAL_TEMPLATE.md? This is an example of a proposal that follows the format look like #44153 (comment).
Moreover, can you elaborate on what is not correct about current implementation of hold features? (link to existing LOC/method and explain which lines or points are not correct).
I couldn't find out the existing method name |
Unable to reproduce |
@sakluger, @hoangzinh Whoops! This issue is 2 days overdue. Let's get this updated quick! |
awaiting for proposals |
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸 |
@sakluger, @hoangzinh Eep! 4 days overdue now. Issues have feelings too... |
We're still waiting for more proposals. |
Not overdue. It's still waiting for new proposals and updates from @Zakpak0 as well on his proposal |
Adding the |
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸 |
@Zakpak0 any updates please? |
@dylanexpensify will update today |
I'm back! Thanks @dylanexpensify for monitoring while I was gone, and thanks @trjExpensify for fixing the labels. |
ProposalUpdated |
Hey there!
The result: This keeps our linked message stable in its position, even when new content is loaded above/below it. It prevents that annoying "jump" effect where the message you're trying to show suddenly moves. |
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸 |
Not overdue, still review proposals |
Hi @Zakpak0. Thanks for the updates. Can you confirm if your main solution works? It didn't work for me. Besides that, can you explain a little bit about your main solution? What is purpose of "typeof loading == "boolean" ? loading ? 1 : 0 : 1"? Screen.Recording.2024-09-03.at.22.17.38.mov |
@hoangzinh pretty positive, it's working here on my end.
So I made an assumption that this value is safe to change after loading is complete.
This information clarified that this was an incorrect assumption. Lastly, I believe on your branch the solution many not be working because the loading prop is not being passed down. Currently I'm thinking that I will make the clause reportActions.length > 3 ? 1 : 0 or something of that nature. |
Proposal |
@hoangzinh thoughts? Keen to keep this moving! |
Yep, I'm still reviewing @Zakpak0's proposal. It's quite tricky so I have to take time to review it carefully. Btw, I'm unable to reproduce this issue in staging at the moment. Screen.Recording.2024-09-04.at.22.37.15.movIs there anyone still able to reproduce this issue? |
Hm, I don't think I can actually. 🤔 Maybe @Zakpak0 can share some up to date steps if the ones in the OP aren't right or clear. |
I'm also unable to reproduce in Staging using the reproduction steps in the OP. |
The bug isn't reproducible anymore before of this. |
Thanks for your insight and effort in this issue. Yeah, it's unlucky that this issue was fixed there before we can finalize our solution here. |
Appreciate all the discussion and efforts here @Zakpak0 @hoangzinh 🙇 Definitely unfortunate given the circumstances- we'll get the next one for sure! 👍 |
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: 9.0.6-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/4706804&group_by=cases:section_id&group_order=asc&group_id=309128
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause - Internal Team
Action Performed:
Preconditions:
Steps:
Expected Result:
User should remain on expenses page and both expenses (expense on hold and other expense) should be visible
Actual Result:
Expense on hold is displayed as single expense when employee hold it second time
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Add any screenshot/video evidence
Bug6538072_1720627498474.Recording__3473.mp4
Bug6538072_1720650477562.Recording__3486.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: