zooniverse / Panoptes-Front-End

Front end for zooniverse/Panoptes
https://www.zooniverse.org
Apache License 2.0
65 stars 76 forks source link

Mark All Read fails on notifications page #5433

Open eatyourgreens opened 5 years ago

eatyourgreens commented 5 years ago

Expected behavior

Mark All Read should mark my unread notifications as read.

Current behavior

Please include any error messages from the browser console and/or screenshots The button sometimes fails to work. When it does fail, reloading the page does not fix it. The error is Uncaught TypeError: Cannot read property 'update' of undefined and it seems to be happening inside a forEach loop, according to the stack trace.

Steps to replicate

It's an intermittent bug, that only seems to be triggered by certain notifications. When it does happen, it can be triggered by pressing Mark All Read on a section in the notifications page.

Additional information

This seems to be the loop that's failing. https://github.com/zooniverse/Panoptes-Front-End/blob/cedef380766d05dde2151e09ba367ba5d8fbaee1/app/pages/notifications/notification-section.jsx#L131-L133

lcjohnso commented 5 years ago

A bump here to say this is actively happening to me now -- behavior I'm seeing seems identical to bug described. Could be related to a deleted mention on Zooniverse Talk that triggered the error state (see screenshot below)? Screen Shot 2019-07-15 at 10 04 27 PM

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

lcjohnso commented 4 years ago

Bumping this here to prevent from going stale: this is still occuring (I still have one unread DM I cannot read or delete). One possible source of this issue: deleting of spam messages without reading them first. Spam messages are removed from view, but not actually deleted, leading to this error state.

srallen commented 4 years ago

To be honest, this seems like more a back end issue than front end. I don't think we have a good way to deal with deleting spam and my guess is that when those messages were "deleted", their read state wasn't changed from unread to read at the same time of deletion. I think the Talk API needs an administrative method to correctly mark unread messages as deleted then read in a batch or something like that.

camallen commented 4 years ago

yes I agree - these messages need to be cleaned up properly in the talk api.

One other point is that talk doesn't have a notion of read messages :( see https://github.com/zooniverse/Talk-Api/issues/160

srallen commented 4 years ago

That's unfortunate. I made an assumption that it did.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] commented 4 years ago

Closed by probot-stale due to a lack of recent activity. Please feel free to re-open if you wish to take on this change.

eatyourgreens commented 4 years ago

Bumping this because I'm seeing it again for my Talk notifications. The JS error is unable to read property 'update' of undefined during a forEach loop.

Screenshot of Talk notifications showing the console error when Mark All Read fails.

eatyourgreens commented 4 years ago

An existence check for data.notification should fix this, right? https://github.com/zooniverse/Panoptes-Front-End/blob/cedef380766d05dde2151e09ba367ba5d8fbaee1/app/pages/notifications/notification-section.jsx#L131-L133

goplayoutside3 commented 2 years ago

@eatyourgreens @camallen There's a new report on Talk of this notifications bug, and I wanted to bring it to your attention to further document - https://www.zooniverse.org/talk/17/2409220

eatyourgreens commented 2 years ago

@goplayoutside3 Thanks! I've also been having this same problem on my account recently. After reading back through the conversation here, I think it's down to the Talk API not having a concept of 'read messages'.