SpringAll icon indicating copy to clipboard operation
SpringAll copied to clipboard

Unable to mark notifications as read that are not loaded via the dropdown

Open Flaburgan opened this issue 9 years ago • 8 comments

I don't know how I succeed to be in that state because I don't reproduce locally but I'm actually unable to mark as read a notification on a delete post:

Robert Biloute - on diaspora-fr.org, Elm, Augier, et 17 autres ont commenté un message supprimé. il y a environ 9 jours

Each time I click the eye on the /notifications page I have a TypeError in the JS console of Firefox:

TypeError: this.find(...) is undefined
  main-e347267177f10ff883fa5c79737bf787.js:26:16928
	app.collections.Notifications<.setRead https://diaspora-fr.org/assets/main-e347267177f10ff883fa5c79737bf787.js:26:16928
	app.views.Notifications<.toggleUnread https://diaspora-fr.org/assets/main-e347267177f10ff883fa5c79737bf787.js:27:24833
	bound  self-hosted
	oe.event.dispatch https://diaspora-fr.org/assets/jquery2-525ab65a7ded5e0df6886f31e2b92e03.js:2:15899
	oe.event.add/v.handle https://diaspora-fr.org/assets/jquery2-525ab65a7ded5e0df6886f31e2b92e03.js:2:14009

Here is the same error seen by chromium:

main-e347267….js:26 Uncaught TypeError: Cannot read property 'setRead' of undefinedsetRead @ main-e347267….js:26toggleUnread @ main-e347267….js:27dispatch @ jquery2-525ab65….js:2v.handle @ jquery2-525ab65….js:2

I can't reproduce locally and didn't try to "Mark all as read" at the moment as I want to be able to test more.

Flaburgan avatar Dec 14 '16 15:12 Flaburgan

That doesn't have anything to do with the deleted post. We fetch the first 10 notifications for the notifications collection on page load, so if you try to mark some older notification as read/unread that fails.

That's kind of ugly because we use the notifications collection for the dropdown and the notifications page. For the dropdown we want the first 10 notifications and load more when the user scrolls down. For the notifications page we want all notifications which are displayed on the page.

svbergerem avatar Dec 14 '16 15:12 svbergerem

But I have only one notification when I visit /notifications?show=unread.

Flaburgan avatar Dec 14 '16 16:12 Flaburgan

Then I guess you have at least 10 more recent notifications which already have been marked as read.

svbergerem avatar Dec 14 '16 16:12 svbergerem

This bug is still there, [email protected] is affected as it has a lot of incoming notifications. It can of course be bypassed using the "Mark all as read" button but is still disturbing for the user.

Flaburgan avatar Jul 04 '17 10:07 Flaburgan

I've just hit this too, for pod.diaspora.software and diaspora-fr.org (both 0.6.99), and pod.orkz.net (0.6.4.1). The issue I encountered is slightly different: about re-marking a notification as unread once it's been marked as read.

You can individually mark the 10 most recent notifications as read/unread, but no previous ones. It's not about how recent they are; the 11th most recent notification for [email protected] was 4 hours ago, whereas the 10th most recent for my diaspora-fr.org account (which I can manually toggle un/read) was was from November 2015.

It's a problem because I need to manually mark one older notification as unread on the HQ account so one of the other admins will be able to see it.

One other thing I noticed: when you manually mark notifications as unread from a 'clean' state in which all notifications have been marked as read, the 'Mark all as read' button does not reactivate, so it's not possible to clear these now 'unread' notifications in bulk without refreshing the page. I think it would be good if re-marking any notification as unread would re-enable that button.

Hope that helps diagnose it!

goobertron avatar Jul 26 '17 15:07 goobertron

@goobertron Yes, toggle in both directions is broken. As a workaround, you can mark something as unread/read when you find it in the dropdown. It's not about the 10 most recent, it's about how many notifications are loaded in the dropdown. So when you load 30 notifications in the dropdown, you can toggle the 30 most recent notifications.

(edit: we know already pretty well what's broken here, it only needs somebody to fix it in a clean way)

SuperTux88 avatar Jul 26 '17 15:07 SuperTux88

OK, fair enough. I thought the extra information might help.

(Thanks for the tip. I discovered that the last time I needed to do this, but this time, I'd have to load c. 300 notifications in the drop-down, and I just couldn't be bothered to go through all that!)

goobertron avatar Jul 26 '17 15:07 goobertron

Thanks for the information anyway 👍

(and yes, for 300 notifications that isn't a good workaround ;) )

SuperTux88 avatar Jul 26 '17 15:07 SuperTux88