Skip to content
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

Mark All Read fails on notifications page #5433

Open
eatyourgreens opened this issue Jul 4, 2019 · 12 comments
Open

Mark All Read fails on notifications page #5433

eatyourgreens opened this issue Jul 4, 2019 · 12 comments

Comments

@eatyourgreens
Copy link
Contributor

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.

this.state.notificationData.forEach((data) => {
data.notification.update({ delivered: true });
});

  • Operating system: OSX Mojave
  • Browser: Chrome
@lcjohnso
Copy link
Member

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

@srallen srallen added this to the Bug fixes milestone Sep 20, 2019
@stale
Copy link

stale bot commented Jan 19, 2020

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 stale bot added the stale label Jan 19, 2020
@lcjohnso
Copy link
Member

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.

@stale stale bot removed the stale label Jan 21, 2020
@srallen
Copy link
Contributor

srallen commented Jan 22, 2020

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
Copy link
Contributor

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 zooniverse/talk-api#160

@srallen
Copy link
Contributor

srallen commented Jan 22, 2020

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

@stale
Copy link

stale bot commented May 22, 2020

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 stale bot added the stale label May 22, 2020
@stale
Copy link

stale bot commented May 30, 2020

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.

@stale stale bot closed this as completed May 30, 2020
@eatyourgreens
Copy link
Contributor Author

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 eatyourgreens reopened this Jul 27, 2020
@stale stale bot removed the stale label Jul 27, 2020
@eatyourgreens
Copy link
Contributor Author

An existence check for data.notification should fix this, right?

this.state.notificationData.forEach((data) => {
data.notification.update({ delivered: true });
});

@goplayoutside3
Copy link
Contributor

@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
Copy link
Contributor Author

@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'.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants