Skip to:
Content

#7023 closed defect (bug) (fixed)

Administrator not able to delete user notifications

Reported by: VibeThemes Owned by: r-a-y
Milestone: 2.6 Priority: normal
Severity: normal Version: 2.5.0
Component: Toolbar & Notifications Keywords: has-patch has-screenshots
Cc:

Description

Description
A site administrator can not delete or mark read/unread user notifications.

Desired Behaviour

Site administrators should have full access to user profiles and should be able to mark notifications read/unread/delete from user profiles.

Steps to replicate :

  • Login as Site administrator in your BuddyPress site
  • Open any user profile
  • Go to the Notifications section
  • Click on "Read" or "Delete" link.

You'll see a message "There was a problem in deleting/reading that notification"

Please refer attached screenshots for further description.

Suggested Fix

I have attached two patches for the Github repository (https://github.com/buddypress/BuddyPress)

  1. Function check_access of BP_Notifications_Notification should have a check for Site administrator or users who can edit the users. (Patch 1 )
  1. Function bp_get_notifications_unread_permalink and bp_get_notifications_read_permalink should use the function bp_displayed_user_domain instead of bp_loggedin_user_domain (Patch 2)

Attachments (5)

notworking1.png (101.4 KB) - added by VibeThemes 15 months ago.
Issue screenshot 1
displayed_error.png (64.4 KB) - added by VibeThemes 15 months ago.
Error displayed to Site administrator
patch1.patch (1.2 KB) - added by VibeThemes 15 months ago.
check_access in BP_Notifications_Notification
patch2.patch (1.8 KB) - added by VibeThemes 15 months ago.
bp_displayed_user_domain instead of bp_loggedin_user_domain
7023.01.patch (19.5 KB) - added by r-a-y 14 months ago.

Download all attachments as: .zip

Change History (12)

@VibeThemes
15 months ago

Issue screenshot 1

@VibeThemes
15 months ago

Error displayed to Site administrator

@VibeThemes
15 months ago

check_access in BP_Notifications_Notification

@VibeThemes
15 months ago

bp_displayed_user_domain instead of bp_loggedin_user_domain

#1 @r-a-y
14 months ago

  • Milestone changed from Awaiting Review to 2.6

Thanks for the patch, @VibeThemes.

However, we can't just change the user domain to the displayed user's as some devs might already be using those functions to want the logged-in user's notification links.

Instead 01.patch adds a $user_id parameter to all notification action links and uses the displayed user as the default there.

Also for the check access mods, I modified bp_notifications_delete_notification() and bp_notifications_mark_notification() instead.

@r-a-y
14 months ago

#2 @dcavins
14 months ago

@r-a-y: Nice! Works well for me and is a definite improvement.

Comments: There's an errant backslash opening line 626 of bp-notifications-template.php.

For my own education, can you say why you chose this pattern:

function bp_get_notifications_unread_permalink( $user_id = 0 ) {
        if ( 0 === $user_id ) {
                $user_id = bp_loggedin_user_id();
                $domain  = bp_loggedin_user_domain();
        } else {
                $domain = bp_core_get_user_domain( (int) $user_id );
        }


instead of

        if ( $user_id ) {
                $domain = bp_core_get_user_domain( (int) $user_id );
        } else {
                $user_id = bp_loggedin_user_id();
                $domain  = bp_loggedin_user_domain();
        }

#3 @r-a-y
14 months ago

Comments: There's an errant backslash opening line 626 of bp-notifications-template.php.

Nice catch!

For my own education, can you say why you chose this pattern

Since we're adding a new parameter with a default value, we can do a strict type check for the default value, so 0 === $user_id is going to be faster.

if ( $user_id ) will look for non-falsey values. Micro-optimization FTW!

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


14 months ago

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


13 months ago

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


13 months ago

#7 @r-a-y
13 months ago

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

In 10816:

Notifications: Ensure moderators are able to manage a user's notifications.

Previously, due to hardcoded logged-in user checks, it was not possible
for moderators to manage a displayed user's notifications.

This commit adds a user ID parameter to all appropriate notification
template tag functions and sets some of those functions to default to the
displayed user ID where necessary. This will allow moderators to manage
a displayed user's notifications as intended.

Props r-a-y, VibeThemes.

Fixes #7023.

Note: See TracTickets for help on using tickets.