Skip to:
Content

BuddyPress.org

Opened 5 years ago

Closed 5 years ago

#7135 closed defect (bug) (fixed)

Activity reply notifications should use the comment ID instead of the parent activity ID

Reported by: r-a-y Owned by: r-a-y
Milestone: 2.6 Priority: normal
Severity: normal Version:
Component: Activity Keywords: has-patch
Cc:

Description (last modified by r-a-y)

In #6057, we introduced activity reply screen notifications.

Currently, activity reply notifications use the parent activity ID to record the notification.

We should use the activity comment ID instead.

This is important when blog comment <-> activity comment synchronization is on as it is not possible to mark the notification as read on a blog post.

The attached patch does four things:

  1. Switches activity reply notifications to use the activity comment ID instead of the parent activity ID.
  2. Marks individual activity reply notification as read. See the new bp_activity_remove_screen_notifications_for_non_mentions() function. This handles both single activity permalinks and blog posts.
  3. Alters bp_activity_get_permalink() to add the #acomment-X anchor directly. This is so notification links go directly to the comment in question.
  4. Reverts bp_activity_remove_screen_notifications_single_activity_permalink() to before #6057 was introduced.

Ping @imath.

Attachments (5)

7135.01.patch (6.1 KB) - added by r-a-y 5 years ago.
7135.02.patch (7.2 KB) - added by r-a-y 5 years ago.
7135.03.patch (10.7 KB) - added by r-a-y 5 years ago.
7135.04.patch (10.6 KB) - added by r-a-y 5 years ago.
7135.05.patch (13.9 KB) - added by r-a-y 5 years ago.

Download all attachments as: .zip

Change History (16)

@r-a-y
5 years ago

#1 @r-a-y
5 years ago

  • Description modified (diff)

#2 @r-a-y
5 years ago

  • Keywords has-patch added

@r-a-y
5 years ago

#3 @r-a-y
5 years ago

02.patch uses a slightly different approach.

For point 2, to mark the individual activity reply notification as read, we use the notification ID ('nid') for the query argument instead of the activity comment ID ('aid') and notification action ('n') from 01.patch.

Last edited 5 years ago by r-a-y (previous) (diff)

This ticket was mentioned in Slack in #buddypress by djpaul. View the logs.


5 years ago

This ticket was mentioned in Slack in #buddypress by dcavins. View the logs.


5 years ago

#6 @dcavins
5 years ago

@r-a-y Which approach do you prefer? Are you hoping to land this today for 2.6 or is this a 2.6.1 fix?

#7 @r-a-y
5 years ago

I probably prefer the second patch. Would prefer to land this today as activity reply notifications is a new feature we are introducing and it should be bug-free.

Here's how to duplicate the blog post notification issue:

  1. Under "Settings > BuddyPress > Options", make sure "Allow activity stream commenting on blog and forum posts" is checked.
  2. Under "Settings > BuddyPress > Components", turn on "Site Tracking"
  3. Create a blog post and enable commenting for that post.
  4. Login as another user and go to the activity directory and find the activity item for the blog post.
  5. Create an activity reply. This should create a notification for the blog post author.
  6. Login as the blog post author and go to your notifications.
  7. Click on the activity reply notification. The notification will not be marked as read.
Last edited 5 years ago by r-a-y (previous) (diff)

@r-a-y
5 years ago

#8 @r-a-y
5 years ago

Thanks for testing, @dcavins.

Regarding your comment on Slack:

Related: if a member replies to a comment, should the comment author be notified?

If a member posts a comment from the blog post page, a screen notification will not be added.

03.patch solves this. Screen notifications are now added if a comment is added from the blog post page and not just from the activity directory page. See bp_activity_add_notification_for_synced_blog_comment() and the new 'bp_blogs_comment_sync_activity_comment' hook.

This piggybacks off of the activity reply notification format so "activity stream commenting on blog and forum posts" needs to be enabled.

This ticket was mentioned in Slack in #buddypress by r-a-y. View the logs.


5 years ago

@r-a-y
5 years ago

@r-a-y
5 years ago

#10 @r-a-y
5 years ago

In 10894:

Activity: Move activity anchor addition from bp_get_activity_comment_permalink() to bp_get_activity_permalink().

See #7135.

#11 @r-a-y
5 years ago

  • Owner set to r-a-y
  • Resolution set to fixed
  • Status changed from new to closed

In 10895:

Activity: Fine-tune activity reply screen notifications when activity stream commenting is enabled.

Previously, if "activity stream commenting on blog and forum posts" is
enabled and a reply was created from the Activity Directory page for a
blog post activity item, clicking on the activity reply notification
would not mark the notification as read.

Also, if a member decides to post a comment from the blog post permalink,
a screen notification would not be added.

This commit fixes these issues and adds a unit test.

Props r-a-y, dcavins, imath.

Fixes #7135.

Note: See TracTickets for help on using tickets.