Notifications component minor issues in 1.9 beta 1

[henrywright]

I've come across the following minor issues after testing the new notifications component (in BP 1.9 Beta 1)

1. Visiting your own notifications page members/my-username/notifications/ is allowed yet viewing another member's notifications page isn't members/their-username/notifications/. With this in mind, should the 'Notifications' navigation tab be shown when viewing a member's profile page members/their-username/? Currently it is. Might be worth hiding it in the same way as Settings and Messages are hidden. - see r7593

2. When an @-mention is made in a blog post or blog post comment, no notification is created. My understanding is @-mentions notify the mentioned member only if the @-mention is in the first sentence (the excerpt) of the text. For me, in 1.9 beta 1, @-mentions do not trigger BP core notifications or emails regardless of where the @-mention appears in the blog post or comment text.

3. Using messages notiications as an example, when a notification is clicked on the notifications screen and the member is taken to their message inbox, the notifications count next to the notifications navigation tab isn't updated. Only once the user navigates away from the inbox does the count update to reflect that the notification has been read. i.e. decrease by 1. - see r7621

4. On deleting a notification, should the member be promoted that there is no way back? Using BP whilst on my tablet is particularly tricky and sometimes I tap in the wrong area. This could result in notifications being deleted unintentionally. - see r7594

Aside from these minor issues, it seems to work great - entirely as intended and expected. I hope this helps

[r-a-y]

In 7593:

Notifications: Only show profile nav item if user can edit settings.

See #5251 - point 1.

[r-a-y]

Point 1 is addressed. I used the bp_core_can_edit_settings() function for now to determine whether or not the "Notifications" profile nav tab should be shown, but in the future, we should have individual caps for this as well ('moderate_notifications', 'moderate_pms', 'moderate_settings', etc.).

Point 2 - I cannot duplicate this. I just tested by creating a short blog post with an at-mention and I was able to get a notification. Needs further testing with longer blog posts, etc. Probably should be looked at in #2372 instead.

Point 3 - I think this is a separate enhancement issue. This is more cosmetic and requires some JS to be written.

Point 4 - Sounds like a good idea. Need some dev-feedback before proceeding.

---

I also have a point 5 - the "Read" anchor text on the "Notifications" screen should be renamed to "Mark as read" for UX reasons.

[henrywright]

Hi r-a-y

With reference to point 2, please feel free to register 2 test members on this test install (WP 3.7.1 and BP 1.9 beta 1) and let me know if it works for you. I set it up earlier today purely to test 1.9 with out-of-the-box default settings: ​http://jobtain.co.uk/

I just tried twice again and each time a BP core notification or email alert wasn't triggered.

[johnjamesjacoby]

In 7594:

Notifications: Add confirm class to delete action link.

See #5251 - point 4.

[johnjamesjacoby]

Regarding 3:

I believe this worked identically in 1.8, can you confirm?

If so, this is a consequence of how BuddyPress core notifications are cleared -- each component handles this in their own unique way. In the future, Notifications should have a universal bp_notifications_clear action, that fires before template output, to handle this for us, maybe even without redirect (I.E. with ajax.)

Related, we should nonce all of these notifications links in 2.0. Marking and deleting notifications isn't a huge deal, but it's currently possible to trick someone into hiding notifications. It's not a regression, so I left it untouched for 1.9 until the dust settles.

[henrywright]

Yes, I can confirm. I just tested in 1.8 and the count doesn't get updated straight away. Only after navigating away from the inbox does the count decrease by 1. Seems as though this has always worked this way. Perhaps it is more noticeable in 1.9 because there is a notifications tab.

[r-a-y]

Regarding point 2 - The "Site Tracking" component needs to be enabled so blog posts are recorded in the activity stream in order for the at-mention filter to fire.

[henrywright]

r-a-y yes of course. Re-tested and can confirm point 2 is invalid. core notifications and email alerts trigger with Site Tracking enabled. Sorry about that :)

[johnjamesjacoby]

Replying to henrywright:

r-a-y yes of course. Re-tested and can confirm point 2 is invalid. core notifications and email alerts trigger with Site Tracking enabled. Sorry about that :)

Moving to 2.0, since these remaining 2 issues are not regressions.

Thanks everyone!

[henrywright]

I've been thinking about the wording of notifications and it doesn't quite make sense. Take the following as an example:

You have 1 new message from Henry Wright - Read | Delete 24 minutes ago

You have 1 new message from Henry Wright - Read | Delete 25 minutes ago

You have 1 new message from Henry Wright - Read | Delete 26 minutes ago

Of course each notification in this list makes sense when read as a stand alone item. However, as a whole, the member actually has 3 new messages from Henry Wright so "You have 1 new message from Henry Wright" isn't actually true.

Would it be better to use wording such as:

Henry Wright sent you a message - Read | Delete 26 minutes ago

Am I being too finicky?

Also (but this is merely a personal preference) I'm thinking of adding Henry Wright's avatar to the beginning of the notification to keep the list consistent with the various parts of BP such as the members list or messages list where user avatars are displayed.

[henrywright]

Another observation is where single notifications are linked to.

Take private message notifications as an example.

1. Clicking on You have 1 new message from Henry Wright will take the user to the messages inbox /members/username/messages/.

This would be the most appropriate place to link to if the notification was for multiple private messages received e.g. linking You have 6 new messages to the messages inbox makes sense. However, for single notifications such as You have 1 new message from Henry Wright the link would be better if it took the user to the actual message thread e.g. /members/username/messages/view/62/

The same goes for @-mentions. Currently a single @-mention notification is linked to the activity mentions page /members/username/activity/mentions. Would a better place to link to in this situation be the actual post or comment?

My attempt at a solution would be to change the $at_mention_link variable in bp_activity_format_notifications() to something like:

$activity = bp_activity_get_specific( array( 'activity_ids' => $activity_id ) );
$activity = $activity['activities'][0];
$at_mention_link = $activity->primary_link;

The problem with this approach is that notifications are not cleared from the unread list after they've been clicked. It seems only a manual click on 'Read' or a manual visit to the /members/username/activity/mentions/ page clears the @-mention notification from the unread list.

[r-a-y]

Point 3 should be solved due to the changes in r7621.

[henrywright]

r-a-y - in that case this ticket can be marked as resolved. See my comment:

r-a-y yes of course. Re-tested and can confirm point 2 is invalid. core notifications and email alerts trigger with Site Tracking enabled. Sorry about that :)

[r-a-y]

Forgot to close this one!