-
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
[$500] Expense - Green dot disappears from LHN when paying one out of many unpaid reports #34767
Comments
Job added to Upwork: https://www.upwork.com/jobs/~01fffa54d6d963be62 |
Triggered auto assignment to @peterdbarkerUK ( |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @allroundexperts ( |
ProposalPlease re-state the problem that we are trying to solve in this issue.In Step 4, LHN for the workspace chat with unpaid report does not show green dot after paying one of the reports. It only shows green dot when revisiting main chat from the expense report. What is the root cause of that problem?We're setting the What changes do you think we should make in order to solve the problem?Check that there're no other outstanding child requests first before setting There're a couple of ways to do this:
What alternative solutions did you explore? (Optional)We can add a back-end field |
Triggered auto assignment to @roryabraham, see https://stackoverflow.com/c/expensify/questions/7972 for more details. |
@roryabraham I believe to implement this properly, we would need to create App version of this Auth method and make sure we use this in backend when the report is reimbursed to update the GBR correctly. |
we do have ReportUtils::requiresAttentionFromCurrentUser which seems about the same – I wish these didn't have to be re-implemented and we could just have some shared logic in a C++ function that we could use in both places, but that's probably out-of-scope for this issue...
This sounds like it needs to be fixed, regardless. I think I prefer @tienifr's solution #1. I'm happy to assign him, but he'll be blocked on the back-end change. Now I'm starting to think more seriously about creating some shared C++ libs for non-trivial business logic like this that's very input/output oriented and needs to be kept in sync across layers. But as that would be a new pattern, we need to discuss it further. Sorry I don't have a more decisive conclusion here just yet – it's very late for me so I want to look at this one more time with fresher eyes. |
@peterdbarkerUK, @allroundexperts, @roryabraham Eep! 4 days overdue now. Issues have feelings too... |
Hi @roryabraham! |
@peterdbarkerUK @allroundexperts @roryabraham 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! |
Triggered auto assignment to @slafortune ( |
@tienifr feel free to provide updated reproduction steps if you'd like |
I suspect you had the workspace set to instant submit @roryabraham, so this needs a coupla' pre-steps to have multiple reports:
|
Yes - I can still recreate this when setting delay submission. I've updated the test steps to include that and leaving submit and close. |
chatted with @trjExpensify in slack, it's unlikely I'll be able to prioritize this one this week so we may look for another volunteer |
Looking for a volunteer here - https://expensify.slack.com/archives/C049HHMV9SM/p1725553790788449 |
|
relevant front-end code:
|
Ok, so I think that the |
Update: I think the linked IOU is not being found here when it should be. Testing in focus mode right now, but also will need to test in most recent mode too. Also, maybe this can be DRYed up but this is performance-sensitive code so I might default to trying to just get this working rather than rocking the boat too much. |
@thienlnam volunteered to grab this one from me since tomorrow's my last day before parental leave and I'm focusing on trying to complete (or almost complete) CP to Prod |
Nice one, thank you Jack! |
Been sidetracked wrapping up things for Hubspot doc, but will look into this in the next couple o days |
Summarizing some things in the thread for my own investigation Auth flow
|
Unassigning since I've got other things on my plate I need to clear out |
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: 1.4.27-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:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:
Action Performed:
Precondition: There is a Collect workspace on Old Dot with admin and employee
pre-steps to have multiple reports for testing -
Expected Result:
In Step 4, green dot should still appear for the workspace chat in LHN because there is still unpaid report
Actual Result:
In Step 4, LHN for the workspace chat with unpaid report does not show green dot after paying one of the reports. It only shows green dot when revisiting main chat from the expense report
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Add any screenshot/video evidence
Bug6346954_1705607247509.20240119_001119.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @thienlnamThe text was updated successfully, but these errors were encountered: