Group Moderator Unable Delete Group Posts

[kainelabsteam]

Dear BuddyPress Team

On this article

​https://codex.buddypress.org/administrator-guide/group-settings-and-roles/

Group moderator advertised able to delete posts from group wall. However, we found that moderator have no ability to do that. Only Super Admin can delete the posts. Is it normal or a bug?

Best Regards, KaineLabs Team.

[imath]

Thanks for your feedback.

Before assigning a ticket to a milestone, we need to review it. That's why I've edited this to Awaiting Review.

After looking at the part you're mentioning, I believe there's a confusion about the term "post".

here's the part of the documentation:

Moderators: When a group member is promoted to be a moderator of the group, it means that the member receives the following additional abilities:
Edit group details, including the group name and group description (see: #4737)
Edit, close, and delete any forum topic or post in the group
Edit and delete other kinds of content, as produced by certain plugins

I guess you're referring to Edit, close, and delete any forum topic or post in the group. Forum "topics or posts" are not what you call "post on the group wall". I understand "post on the group wall" to be what we call "Activities". Activities can only be deleted by their author or the site Administrator.

imho this ticket should be closed, what do you think?

[vapvarun]

Taking over this ticket, will submit a patch shortly

[emaralive]

screenshot-win10-me-2024.04.14-08_10_55.png

[emaralive]

screenshot-win10-me-2024.04.14-08_17_41.png

[emaralive]

In alphabetical order, @dcavins, @espellcaste, @imath, @vapvarun; perhaps the following will satisfy most of what was stipulated during the ​BP Dev Chat on April 3, 2024 and reiterated within the ​BP Dev-Chat Summary April 3, 2024.

The reason why the the ​callback (bp_groups_filter_activity_user_can_delete) for the ​filter (bp_activity_user_can_delete) does not work is due to the conditional starting on ​line 658, for example:

	if ( isset( $activity->component ) || 'groups' !== $activity->component ) {
		return $retval;
	}

The conditional contains a logical OR operator and since isset( $activity->component ) will, in all likelihood, be true, the variable $retval will be the the parameter value that was initially passed to the callback. IOW, the function ends at this point and never reaches any of the downstream conditionals that would determine a different outcome, e.g., the determination of whether group admins or group mods can delete activity posts. The fix is to change the logical OR to a logical AND, for example:

	if ( isset( $activity->component ) && 'groups' !== $activity->component ) {
		return $retval;
	}

In retrospect, was this intentional or an oversight as to how the callback was written/coded for release? And, since the callback existed since version 6.0.0; why was this not recognized as a bug before and after this ticket was submitted? However, with the fix, there are a couple of undesirable consequences, as I see it, that allow for group admins and group mods to:

• Delete site admin activity posts (within the confines of a respective group and if the group is public within sitewide or user activity streams)
• Delete group members (group admin, group mods & group members) activity posts if the group is public within sitewide or user activity streams.

To avert the aforementioned undesirable consequences, I propose the callback to be initially rewritten/recoded as, which serves as POC:

function bp_groups_filter_activity_user_can_delete( $retval, $activity ) {
	// Bail if no current user.
        // Todo: add second conditional statement to check the state to allow group activity deletions.
        // e.g., "|| ! bp_enable_group_activity_deletions()".
	if ( ! is_user_logged_in() ) {
		return $retval;
	}
        
        // The second conditional statement does not allow "site admin" activity posts to be deleted by "non site admins".
	if ( bp_current_user_can( 'bp_moderate' ) || bp_user_can( $activity->user_id, 'bp_moderate' ) ) {
		return $retval;
	}

	$bp = buddypress();

        // Confine group activity deletion to the confines of the respective group where it is allowed.
	if ( ! empty( $bp->groups->current_group->id ) ) {
		$group_id = $bp->groups->current_group->id;
                
                // Todo: break this out into additional conditionals that provide granularity as to who can to what, when.
  		if ( groups_is_user_creator( bp_loggedin_user_id(), $group_id ) || groups_is_user_admin( bp_loggedin_user_id(), $group_id ) || groups_is_user_mod( bp_loggedin_user_id(), $group_id ) ) {
			$retval = true;
		}
	}

	return $retval;
}

Later I plan to add granularity of control to the proposed callback that will delineate who can delete activity posts as well as when, for example:

• group creator only
• group creator and group admins
• group creator, group admins and group mods

At least, that would be the plan. Currently, I have the site admin control as to whether group activity deletions are allowed, in which the default state is not enabled or disabled (see screenshots screenshot-win10-me-2024.04.14-08_10_55.png & screenshot-win10-me-2024.04.14-08_17_41.png).

Role Capabilities

NOTE

1. By default a group creator is a group admin.
2. X is default capability.
3. Z is new capability based on parameters of proposed change to callback.

	site admin	group admin	group mod
mark/unmark user as spammer	X
mark/unmark activity as spam	X
delete group activity post	X	Z	Z
ban/unban any group member	X	X
remove any group member	X	X
promote/demote any group member	X	X

Group Roles

ID	Name	Note
admin	Administrator	As noted earlier group creator by default is a group admin
mod	Moderator
member	Member
banned	Banned

Mark/Unmark User as Spammer

Only available to site admin via various locations

• Backend - [All] Users screen/page.
• Backend - Individual User's Extended Profile screen/page - Status metabox.
• Frontend - Individual User's Members Capabilities screen/page, e.g. site.urlmembers/{user}/settings/capabilities/

Note

1. user_status is a field in the users table.
2. hide_sitewide is a field in the bp_activity table, visible from backend - activity page/screen.
3. is_spam is a field in the bp_activity table, visible from backend - activity page/screen - spam tab.

state	user_status	hide_sitewide	is_spam	Note
Normal	0	0	0
Spammed	1	1	1	Kicked from site unable to log back in until unspammed.
Unspammed	0	1	0	Able to log in, but previous activity post are hidden

Mark/Unmark Activity as Spam

Only available to site admin via backend Activity page/screen. Toggle is_spam field from 0 to 1 or 1 to 0.

Delete Group Activity Post

Currently, only available to site admin via delete button on activity post (or backend activity page/screen) or group member who created the activity post via delete button visible on activity post. Proposed to be available to group creator, group admin or group mod depending on configuration defined by the code implemented by this ticket or as defined by a 3rd party plugin.

Ban/Unban Any Group Member

Only available to site admin or group admin via Manage Group Members page/screen. The banned member's activity posts remain visible and cannot rejoin group until unbanned.

Edit:
site admin can also accomplish this via backend - Edit Group page/screen for each respective group.

Remove Any Group Member

Only available to site admin or group admin via Manage Group Members page/screen. The removed member's activity posts remain visible and can rejoin group at any time.

Edit:
site admin can also accomplish this via backend - Edit Group page/screen for each respective group.

Promote/Demote Any Group Member

Only available to site admin or group admin via Manage Group Members page/screen with the exception that they cannot promote/demote themselves. What I find odd is that a group creator (by default group role is a group admin) can be demoted from group admin status (which also means, if a site admin creates a group, the same applies).

Edit:
Only available to site admin or group admin via Manage Group Members page/screen with the exception that they cannot promote/demote themselves, if they are the sole group admin that happens to not be a site admin. What I find odd is that a group creator (by default group role is a group admin) can be demoted from group admin status by another group admin who happens to not be a site admin (maybe this is just my imagination of being odd). Additionally, if a site admin is a member they can be promoted/demoted but, this has no bearing as to the group capabilities of a site admin.

site admin can also accomplish this via backend - Edit Group page/screen for each respective group.

[espellcaste]

I bet we need more unit tests here to not only confirm the issue is fixed, but also to avoid breaking something else.

The fact this has been broken since 6.0.0, also comes to confirm @imath's idea that this feature is not used often. But we should fix it regardless.

@emaralive Could you supply a patch, with unit tests, for review for this issue only (without the granularity)?

[emaralive]

@espellcaste,

I would have replied earlier but, I was trying to provide time for others to weigh-in. As to unit tests, those are not within my current repertoire because:

• I don't have PHPUnit installed.
• I'm not up to speed in writing unit tests.

Thus, someone who would be kindly inclined as to writing unit tests, could do so. So, here is what I will do shortly, I will upload 2 patches:

• 8728.01.patch - Fixes the issue (changes the logical OR to a logical AND).
• 8728.02.patch - Proposed change to the callback.

This will provide a comparison of how it was originally intended to work and what has been proposed.

[emaralive]

Fix for callback

[emaralive]

Proposed change to callback

[espellcaste]

Sounds good. Thanks for the patches. I'll work on the unit tests.

[imath]

Hi @emaralive

First: thank you so much for the amazing work you accomplished about reviewing Group roles capabilities 💪 👏. This work will be very helpful to update our documentation and keep things clear about it.

I'm personaly in favor of adding the "Site Wide" Group's content deletion setting you suggested
+ adding another one into the group's management/creation screen on front-end.

Thanks for fixing the bp_groups_filter_activity_user_can_delete filter. Ideally it should include the bp_enable_group_activity_deletions() you suggested or something plugin could rely on, eg: bp_enable_group_content_deletions( 'activity' ).

One very important thing you need to improve about 8728.02.patch: you cannot rely on buddypress()->groups->current_group->id or bp_get_current_group_id() as this global data is only available when a group is displayed: it would fail within the REST API context. So the right way to get the group's ID the activity is attached to is:
$group_id = $activity->item_id;

Thanks a lot @espellcaste for working on unit tests, I agree this is important.

Let's wait for 1 more feedback about the Admin / Group Admin settings to see if we're fine with adding these. What do you think @dcavins?

[emaralive]

@imath,

Thanks for the enlightenment and I've managed to come up with 8728.03.patch that is a hybrid of the 8728.01.patch and the 8728.02.patch, that looks like the following:

function bp_groups_filter_activity_user_can_delete( $retval, $activity ) {
	// Bail if no current user.
	// Todo: add second conditional statement to check the state to allow group activity deletions.
	// e.g., "|| ! bp_enable_group_activity_deletions()".
	if ( ! is_user_logged_in() ) {
		return $retval;
	}

	if ( isset( $activity->component ) && 'groups' !== $activity->component ) {
		return $retval;
	}

	// The second conditional statement does not allow "site admin" activity posts to be deleted by "non site admins".
	if ( bp_current_user_can( 'bp_moderate' ) || bp_user_can( $activity->user_id, 'bp_moderate' ) ) {
		return $retval;
	}

	$group_id = $activity->item_id;

	// Group creator, administrators or moderators can delete content in which deletions are allowed for that group.
	// Todo: break this out into additional conditionals that provide granularity as to who can to what, when.
	if ( groups_is_user_creator( bp_loggedin_user_id(), $group_id ) || groups_is_user_admin( bp_loggedin_user_id(), $group_id ) || groups_is_user_mod( bp_loggedin_user_id(), $group_id ) ) {
		$retval = true;
	}

	return $retval;
}

@espellcaste,

8728.03.patch to be uploaded shortly to replace 8728.02.patch.

Next is for the both of you, when testing the REST API for activity deletion, I'm receiving the following error notices with successful deletions:

Notice: Trying to get property 'is_spam' of non-object in /wp-content/plugins/buddypress/src/bp-activity/bp-activity-functions.php on line 3376

Notice: Trying to get property 'component' of non-object in /wp-content/plugins/buddypress/src/bp-activity/bp-activity-functions.php on line 3357

​Line 3376 and ​line 3357

Scenario

WP 6.5.2
BP 14.0.0-alpha and BP 12.4.0
PHP 7.4.33
BuddyPress Nouveau template pack
BP Rewrites API
Plugin: JSON Basic Authentication

Curl statement on the command line:

curl -X DELETE --user <user>:<password> -i site.url/wp-json/buddypress/v1/activity/<id>

Is it just me or are these valid error notices?

[emaralive]

Replacement for 8728.02.patch

[dcavins]

I agree that allowing the site admin to decide if group admins and mods can delete group-associated activity is a good idea. My only suggestion is to make the setting text as clear as can be. Like "Allow group administrators and moderators to delete activity items from their groups's activity stream".

Thanks!

[espellcaste]

@emaralive Here are the unit tests: ​https://github.com/buddypress/buddypress/pull/273

I tested with your patch and it fixes the issues.

[emaralive]

screenshot-win10-me-2024.04.18-09_07_40.png

[emaralive]

Replacement for 8728.03.patch

[emaralive]

Just to make sure everyone is notified, in alphabetical order, @dcavins, @espellcaste, @imath & @vapvarun:

• Thanks to @espellcaste, there are unit tests.
• Thanks to @imath for providing enlightenment regarding consideration for the BP REST API
• Thanks to @dcavins, the text he provided for BuddyPress Settings has been incorporated (see screenshot screenshot-win10-me-2024.04.18-09_07_40.png)

Additionally, after some deliberation, I am forgoing the notion I had about adding radio buttons (granularity of control) on the Group Settings page/screen with the rational that Group Admins will, already, have control over who can delete Group Activity content via the promote/demote capability.

Furthermore, I removed the groups_is_user_creator function from the bp_groups_filter_activity_user_can_delete callback, due to the fact that a:

1. Group Creator is by default a Group Admin
2. Group Creator is actually not considered a part of Group Roles, e.g., admin, mod, member, banned
3. Group Creator can actually be demoted, banned or removed by another Group Admin or by a Site Admin

In the event of item 3, a loophole would have been present (with the presence of the groups_is_user_creator function) that would have allowed the Group Creator to delete Group Activity content.

The 8728.04 patch should represent the aforementioned additions, removals and omissions. Barring any additional recommendations, errors, etc., this may be the final patch.

[needle]

@emaralive I see that your patch changes ​this line in the filter callback. I agree that it currently looks wrong - but are you sure that your change does what it's supposed to do? It looks to me like it should read:

if ( ! isset( $activity->component ) || 'groups' !== $activity->component ) {

Your change to && allows Activities with no component at all to be processed as Group Activities.

+ Include tests from this PR ​https://github.com/buddypress/buddypress/pull/273 (putting them into a more appropriate file).
+ Add some other tests to make sure a group admin cannot delete any activity, an activity posted in a group he's not a member of, or activities of the group he's an admin of when the Admin globally disabled the group activity deletion.
+ Use the same logic than other options: by default group Admins/Mods can delete activities. The Admin needs to uncheck the global settings to disable it.
+ Let's rename options/functions the "disable" way !

PS: @emaralive, @needle is right (thanks a lot for your input), so I've updated the patch accordingly.

Trac ticket: ​https://buddypress.trac.wordpress.org/ticket/8728

Trac ticket: ​https://buddypress.trac.wordpress.org/ticket/8728

@imath, I appear to be experiencing unexpected behavior with the way the Group Activity Deletions checkbox is working. IOW, by default, the checkbox is now checked (selected), I would expect that deselecting the checkbox and then saving the change (save settings) would result with a deselected checkbox.

However, the checkbox remains checked (selected) or reverts back to being checked (selected). Can you confirm or deny this behavior?

Closing pr since the code was bunbled into the original patch.

Ouch! Thanks for your alert about it @emaralive I'll give the PR more testing to see if I missed something 👌

@emaralive just updated the PR, the option should be saved now, can you confirm?

@imath I just finished testing the Group Activity Deletions checkbox and can confirm that the checkbox now behaves as expected.

[imath]

In 13870:

Groups: improve group activity moderation delegation to admins & mods

• Introduce a new BP Groups global setting to let Site Administrators decide whether group admins & mods can delete their group activities or not.
• NB: Group creators can keep the control about who can delete their group activities using group role promotion (allow: promote some members as mods or admins. Disallow: do not promote anyone).
• Make sure the bp_groups_filter_activity_user_can_delete() filter takes in account this setting and behaves as expected (eventually allowing group admins/mods to delete group activities).
• Add unit tests

This ticket is a perfect example of a great collaborative work between all members of the BP Team. Great job 💪.

Props: emaralive, vapvarun, dcavins, espellcaste, needle

Fixes #8728
Closes ​https://github.com/buddypress/buddypress/pull/278