-
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
[HOLD for payment 2024-06-06] CRITICAL: [$250] Clicking New messages
when there are no new messages inexplicably calls OpenReport
, and doesn't clear
#41188
Comments
Hm, can't reproduce, closing. |
Actually I can reproduce this in the #new-expensify-feedback room reliably. I think it might have something to do with it being a public room and having so many hidden whispers. (As a reminder, for bidirectional scrolling and comment linking, we need to send all reportActions to the client -- even "whispers" not addressed to them. We're aware of the privacy violation and that'll be addressed separately. But a consequence of that design is it mean when you enter a public room that hasn't had any visible activity for a long time, there can be pages and pages of invisible whispers it needs to return for you to get a full page of visible. This has performance implications we're also aware of and working on separately. But for now, this is the way it is, and we need to make sure it works well.) |
Job added to Upwork: https://www.upwork.com/jobs/~0103902e78e2cd9c80 |
New messages
when there are no new messages inexplicably calls OpenReport
, and doesn't clearNew messages
when there are no new messages inexplicably calls OpenReport
, and doesn't clear
Triggered auto assignment to Contributor-plus team member for initial proposal review - @shubham1206agra ( |
ProposalPlease re-state the problem that we are trying to solve in this issue.Floating New Message displayed where there are no new messages and does not disappear when clicked. What is the root cause of that problem?We are currently not verifying if the current report/chat has new unread messages, What changes do you think we should make in order to solve the problem?We should include the verification for new unread message in the current chat
Test: Screen.Recording.2024-05-05.at.10.49.06.PM.mov |
@samilabud Thanks for the proposal, but I don't think your RCA/solution is right. By adding unread condition, we are breaking intended functionality for comment linking. |
Related Tests: Currently these are the behaviors with the change applied and the comment linking:Comment.linking.in.dev.-.new.message.floating.mp4This is a test in Staging with the same link:Comment.linking.in.staging.-.new.message.floating.mp4 |
@shubham1206agra Uh oh! This issue is overdue by 2 days. Don't forget to update your issues! |
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸 |
New messages
when there are no new messages inexplicably calls OpenReport
, and doesn't clearNew messages
when there are no new messages inexplicably calls OpenReport
, and doesn't clear
@samilabud I tested your solution on #new-expensify-feedback. It doesn't work. |
Tagging @adhorodyski @TMisiukiewicz for some help |
@burczu is going to take a look at this one |
@burczu, @shubham1206agra Whoops! This issue is 2 days overdue. Let's get this updated quick! |
not overdue - I'll provide the PR soon |
Triggered auto assignment to @aldo-expensify, see https://stackoverflow.com/c/expensify/questions/7972 for more details. |
New messages
when there are no new messages inexplicably calls OpenReport
, and doesn't clearNew messages
when there are no new messages inexplicably calls OpenReport
, and doesn't clear
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.77-11 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2024-06-06. 🎊 For reference, here are some details about the assignees on this issue:
|
Issue is ready for payment but no BZ is assigned. @johncschuster you are the lucky winner! Please verify the payment summary looks correct and complete the checklist. Thanks! |
Payment Summary:Contributor+: @shubham1206agra $250 - paid via Upwork - please accept the offer! |
(Discussing in the linked PR above to make sure there wasn't a regression) |
@johncschuster Accepted |
Payment has been issued! Thanks, @shubham1206agra! |
Being discussed here: https://expensify.slack.com/archives/C049HHMV9SM/p1714365067885319 -- please update this post with the final report.
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: v1.4.67-2
Reproducible in staging?: Yes
Reproducible in production?: Yes
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: @quinthar
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1714365067885319
Action Performed:
Expected Result:
There should be no new message pill unless there is one and pill disappears after clicking it.
Actual Result:
Clicking New message pill calls
Open report
, Displayed even when no message to read and not disappearing after clicking it.Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Add any screenshot/video evidence
Screen.Recording.2024-04-28.at.9.27.15.PM.mov
Recording.3066.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @shubham1206agraThe text was updated successfully, but these errors were encountered: