-
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
[HOLD #8267] Web - Read messages are marked unread after a while #3981
Comments
Triggered auto assignment to @thienlnam ( |
Triggered auto assignment to @jboniface ( |
Since this hasn't been reproduced, I am hesitant to post the job |
Looks like @tgolen @stitesExpensify and @cead22 were able to reproduce this issue too 🤔 @iwiznia I know this is a long shot but I have some questions that could help me get solid repro steps:
|
@iwiznia also check that you don't have policy rooms with unread messages (eg, #announce) -- Cole let me know recently that this could be another cause of 3.cash showing the unread indicator |
- Do you have an estimate of how much time it passes from reading the
message to the unread status?
For me, it seems to happen more often when I go to the e.cash tab, sit for
1-2 seconds (and read the most recent message), then go to another tab
quickly. I suspect this is the cause of the issue.
- Are there other tabs opened with e.cash active? Or other devices
logged in with the same account that were being used at the same time?
There should only be one tab open when it's happened to me. I very rarely
use mobile and web at the same time, so I don't think this would be the
cause of it.
- Does this happen with the same chat each time or are several chats
affected?
It's always one chat affected at a time, but not always the same chat, no.
…On Mon, Jul 12, 2021 at 4:25 PM Carlos Alvarez ***@***.***> wrote:
@iwiznia <https://github.com/iwiznia> also check that you don't have
policy rooms with unread messages (eg, #announce) -- Cole let me know
recently that this could be another cause of 3.cash showing the unread
indicator
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#3981 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAJMAB2VWKW3PT757XBXWKDTXNTW5ANCNFSM5AHEFDCQ>
.
|
It varies, I don't think the time is a determining factor here.
Nope
Same as Tim, only one at a time, but not the same one each time
Already did, this is not it. If it was, then the unread would not go away when I re-read the chat |
Thanks for the extra info! 🙇 No luck yet on being able to repro the issue yet. Will update if I find something. |
@puneetlath Had this same behaviour with his own message: (slack link here https://expensify.slack.com/archives/C01GTK53T8Q/p1626387107365600)
|
@joaniew was able to reproduce this too (slack thread: https://expensify.slack.com/archives/C01GTK53T8Q/p16263903423766000)
|
Yep, I get that too seemingly randomly, not even sure if I need to go away for it to happen, I think not. |
@thienlnam Uh oh! This issue is overdue by 2 days. Don't forget to update your issues! |
Still having trouble getting consistent reproduction steps for this - with how random it is maybe it has to do with having instances on other devices and an older set of data overwriting the newer data |
Throwing on a weekly until we can get better reproduction steps |
No proposals or clear re-production steps yet |
@johnmlee101 can you review @kidroca 's comment above, #3981 (comment) and provide feedback I just had this happen again, steps:
I have a guess/feeling that all 'closing lid' actions are equal, ie. if the lid is closed for longer than a certain time, something different might happen. Kinda like screensaver vs sleep |
Okay so I have a few thoughts right now. Some of which deal with how we might want to deal with requests when they are "offline" and it starts tying into the larger discussion on how we should deal with our offline logic. Basically we should be able to support newer requests overriding older ones in the queue, if I'm understanding correctly? And we need the information from the response to make a proper judgement on which requests to ignore. |
cc @roryabraham because of our recent discussion 😄 |
One more ➕ for addressing this on the backend would be accidentally introduced bugs like loading a conversation from the middle would not cause everything past that point to be considered unread We should be able to say (to the backend)
|
Yeah, I am still in favor of fixing it on the backend as well. I think this
API endpoint is just confusing and complex and splitting it apart would
simplify things. I'd like to propose that we do this in stages:
1. Create brand new endpoints for each functionality
2. Update App to use the new endpoints
4. Deprecate the old endpoint and return an error plus also log if it's
ever called
3. Remove the old endpoint once App is all using the new endpoints
…On Fri, Jan 7, 2022 at 9:59 AM Peter Velkov ***@***.***> wrote:
One more ➕ for addressing this on the backend would be accidentally
introduced bugs like loading a conversation from the middle would not cause
everything past that point to be considered unread
We should be able to say (to the backend)
- "mark everything up to this comment as read" and it's read anyway
it'll just do nothing
- "mark this comment as unread" then and only then it would mark
everything past that point as unread
—
Reply to this email directly, view it on GitHub
<#3981 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAJMABZRWHFJ7IJDB4HHPLTUU4LWNANCNFSM5AHEFDCQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Should I go ahead and split these assignments into two parts? One for the front-end changes in this repo and one for the back-end internal development? |
@tgolen can you reply to #3981 (comment) I'm off next week, not reassiging to another CM cuz doubt this will have much action |
Splitting it out is fine, yes. |
@johnmlee101 can you go ahead and do this plz |
@johnmlee101 can you please help with the above and split the assignments into two parts? |
Sorry, just saw this update now! On it |
This issue is responsible for adding the two API end points needed. Once those endpoints exist, then we'll go ahead and come back to this issue to make sure we're properly using the new API endpoint to update the read/unread markers (which can be done externally). This step will be combined with the deprecation of the API command and we'll revisit once things are all moved over. |
Put on hold pending #8267 (internal), updated title, removed |
I will be keeping this on hold as we'll be pursuing a potential high-level refactor of this logic to simplify and inevitably fix these issues as put through our proposal process. |
From @stitesExpensify 8 days ago
Should we take off hold? And/or, is this related to Offline First? |
I think that this issue might be fixed? My PR made it so that we can never set the lastRead backwards unless manually done by the user, that should fix this issue right? |
Since it doesn't sound like it's easy to reliably reproduce, it's hard to test. @iwiznia any idea of this has happened to you the past week?!?! If not, I'm down to close this then reopen if/when needed |
I don't know, since the unread message indicator is totally broken for me right now. Let's close, since Marc is revamping all this soon anyway |
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Action Performed:
Expected Result:
Unread chat should stay marked as unread.
Actual Result:
The chat that was opened earlier is marked as unread. Explained by @iwiznia:
Workaround:
None needed. It's just confusing to the user to see a chat marked as unread when it was opened before.
Platform:
Where is this issue occurring?
Web ✔️
iOS
Android
Desktop App ✔️
Mobile Web
Version Number: 1.0.77-0
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:
I haven't been able to reproduce this yet on my side. Will update here if I find any more information about this.
Expensify/Expensify Issue URL:
View all open jobs on Upwork
From @iwiznia https://expensify.slack.com/archives/C01GTK53T8Q/p1626078601100700
The text was updated successfully, but these errors were encountered: