-
Notifications
You must be signed in to change notification settings - Fork 3k
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
[$250] IOU - GBR is missing from the partially approved expense #52650
Comments
Triggered auto assignment to @OfstadC ( |
Not overdue Mel - it was the weekend. Will review this tomorrow AM |
What specifically is missing here |
It seems normal to me 🤔 |
Still reproducible. Actions performed are same, updated the expected result bandicam.2024-11-22.12-34-26-431.mp4 |
OMG - It was not clear what GBR was to me - April was a saint and helped me! |
I actually think this is a dupe of this |
Re-reviewing the other issue it seems they are closely related but differ a bit. Adding to #expense |
Job added to Upwork: https://www.upwork.com/jobs/~021861120032414784544 |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @DylanDylann ( |
Edited by proposal-police: This proposal was edited at 2024-11-26 05:05:16 UTC. ProposalPlease re-state the problem that we are trying to solve in this issue.GBR is missing from the partially approved expense What is the root cause of that problem?If we approve partially, a new expense report will be created with Line 6324 in 38568ed
If Lines 4646 to 4649 in 38568ed
But in this case Line 6990 in 38568ed
So no GBR is displayed in LHN What changes do you think we should make in order to solve the problem?If we approve in full, we can update Line 6990 in 38568ed
In this case, it is not full, we need to update Line 7077 in 38568ed
Line 6986 in 38568ed
Line 6405 in 38568ed
What alternative solutions did you explore? (Optional) |
@nkdengineer Do we need to require BE change to fix this issue? |
It seems the BE return wrong hasOutstandingChildRequest in this case |
@DylanDylann you're right, updated proposal. |
There are some new updates that block partially approve flow. Just ask to clarify #50479 (comment) |
Oh sound good, the hasOutstandingChildRequest set to false was implemented before we had partial approvements, so yeah it makes sense to check if is full approval is true for this case |
Backend PR is up |
Backend PR is merged, will update here when it's deployed |
@OfstadC, @NikkiWines, @DylanDylann, @nkdengineer Eep! 4 days overdue now. Issues have feelings too... |
waiting for BE deployment |
@OfstadC @NikkiWines @DylanDylann @nkdengineer this issue is now 4 weeks old, please consider:
Thanks! |
@nkdengineer It seems we are ready to implement the PR |
thanks @DylanDylann, I'll check and open PR soon |
@DylanDylann we have a open PR here |
@NikkiWines In this issue, ApproveMoneyRequest API return chatReport.hasOutstandingChildRequest is true in A, but the openReport API return chatReport.hasOutstandingChildRequest is false --> the bug isn't fixed if we click on report again |
@DylanDylann sorry, can you clarify what steps you take to reproduce this? (also sorry for not updating when that BE PR went live 🙈) |
Steps:
Screen.Recording.2024-12-23.at.17.16.48.mov |
@NikkiWines Kindly bump |
Sorry for the delay here @DylanDylann - was OOO for the holidays and then discussing with some folks about when and where we really need the GBR in this flow. Things get a little murky because the expense can be held by the submitter or by the approver. In the case of the submitter holding the expense, the GBR makes a bit more sense for the approver, since they can override the action. In the case of the approver holding the expense, the GBR makes less sense as they've explicitly taken action to put the expense in a held state. There's nothing further for them to really do at this point, though they can override their own hold. The problem is that we don't distinguish on the backend who has held the expense, and so the flow is unclear. After discussing it with @garrettmknight, I think we can actually omit the GBR here since in both hold flows, the submitter will have the RBR notifying them of the held expense, so at least one user is aware that the expense needs some action. In that case, I'll actually need to revert my earlier PR and there's no further action herd |
@NikkiWines Thanks for your information. @nkdengineer let's close your PR, we do nothing here |
@NikkiWines and I chatted and decided we'll still issue some payment here as you both did some work. We'll be issuing $100 to both of you. Offers are below: |
@NikkiWines Can you review this and let me know your thoughts 😃 |
Ah, ok - I was unaware of that SO. That makes sense, we can issue full comp here. |
Payment Summary
|
@OfstadC Accepted thanks |
Thanks @nkdengineer ! |
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.63-1
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N/A
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/5229984&group_by=cases:section_id&group_id=309128&group_order=asc
Issue reported by: Applause - Internal Team
Action Performed:
Expected Result:
Newly created report for the held expense is GBR’ed in the LHN (green dot)
Actual Result:
GBR is missing from the partially approved expense
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Add any screenshot/video evidence
Bug6666642_1731695994415.bandicam_2024-11-15_19-27-51-840.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @DylanDylannThe text was updated successfully, but these errors were encountered: