-
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] IOU - All of the IOU chats disappear when I do the same IOU and turn the connection on #30839
Comments
Job added to Upwork: https://www.upwork.com/jobs/~018104751b5e6c6df2 |
Triggered auto assignment to @alexpensify ( |
Bug0 Triage Checklist (Main S/O)
|
Triggered auto assignment to Contributor-plus team member for initial proposal review - @ntdiary ( |
ProposalPlease re-state the problem that we are trying to solve in this issue.
What is the root cause of that problem?
What changes do you think we should make in order to solve the problem?
This will allow the users to have a chance to read the error message from the back-end, and if the user wants, they can clear the error by click "x" button, which will clear the error data properly. What alternative solutions did you explore? (Optional)In 2, an alternative is to modify the |
@alexpensify, @ntdiary Uh oh! This issue is overdue by 2 days. Don't forget to update your issues! |
@ntdiary - when you get a chance, can you please review the proposals? Thanks! |
Sure, sorry for the delay, will review later. : ) |
I tested this issue using It looks like an edge case, I think we need to reconfirm the expected result, maybe we can provide some more friendly guidance to the user instead.
Just some initial ideas, please feel free to let me know if you have different thoughts. 😂 cc @alexpensify |
@ntdiary when the user opens the failed report from the LHN, they'll see the error message, and upon clicking "x" next to the error message, the failed report will be completely removed. I think this is the standard way we handle all server errors in the app and we should keep to that here. |
Triggered auto assignment to @dylanexpensify ( |
This comment was marked as duplicate.
This comment was marked as duplicate.
Reassigning another 🐛 team member, I'm going OOO until Tuesday, November 14, and will take it back if it's still open by my return date. @dylanexpensify - Required action from the team: Please keep an eye out that this one is moving forward and we find a proposal, thanks! |
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸 |
If I remember correctly, clicking 'x' will only delete that specific message, not the entire report. Are you suggesting making changes just to delete the entire report in this specific case? |
bump @tienifr for reply to above 🙇♂️ |
Yes, when we try to open an optimistic report that didn't get created due to an error, let's not call openReport. Let's show the report with the error and allow the user to clear it himself. |
Current assignee @ntdiary is eligible for the External assigner, not assigning anyone new. |
📣 @ntdiary 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app! |
📣 @tienifr 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app! Offer link |
Catching up from being OOO, it looks like the PR is being worked on and then can go into review. 🥳 |
PR is still in review! |
Daily Update: PR is in review |
Daily Update: PR has a few comments but moving forward. |
Daily Update: The PR has been merged, so we will need to wait on automation here. |
Daily Update: No automation yet. 🤔 |
@nkuoch can you share if this one is complete? I'm trying to identify if automation failed here and I need to prepare the manual payment process. Thanks! |
@ntdiary - can you provide feedback if this one is complete? I'm trying to identify why automation has failed here or if it should remain open. Thanks! |
@alexpensify, ah, sure, the PR #31903 was deployed to production last week and has passed the regression period. :) |
Ok, thanks, I was OOO last Friday and working on the payment process. |
Here is the payment summary:
Upwork Job: https://www.upwork.com/jobs/~018104751b5e6c6df2 Extra Notes regarding payment: No automation here, so I'll just manually write this one out. |
@ntdiary - if you can add the checklist details, that will be great and we can close out. Everyone has been paid in Upwork. |
This is a edge case that we didn't consider before, the fix is simple and stable, so I personally feel that it is not necessary to add a regression test. :) |
I agree then, no regression test is needed here. With this news, I'm going to close out this GH. |
Thank you for the help here! |
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.3.95.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:
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:
Expected Result:
I should see permanent, duplicated chats of the ones with errors.
Actual Result:
I can only see the chats with errors and they disappear when tapped.
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Main deviceBug6262197_1699014740468.Main.mp4
Bug6262197_1699014740458.Secondary.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: