'joined_group' activity item not removed when user leaves group or is removed from the group

[r-a-y]

There are a few issues when a user leaves a group and with their corresponding 'joined_group' activity item:

1. When you join a group and leave the group, the 'joined_group' activity item is not removed

This was last touched in ​https://github.com/buddypress/buddypress/commit/8db16ebbcc4eb7cbf955d415635e4c6a00ebc1e1#diff-5e3670bc398e40e7b.

The issue is grabbing the group membership object after the user has been removed from the group: ​https://github.com/buddypress/buddypress/blob/8c0406454b260566fb17fc6cb21d755838eac675/src/bp-groups/bp-groups-activity.php#L903-L916, ​https://github.com/buddypress/buddypress/blob/8c0406454b260566fb17fc6cb21d755838eac675/src/bp-groups/bp-groups-functions.php#L638-L648.

This means that the 'date_modified' property doesn't exist, which leads to the membership check failing and the activity deletion not occurring.

2. There is a five-minute conditional check to determine whether the 'joined_group' activity item should be removed or not

If a user has been a member of a group longer than five minutes and if that user is removed (whether by manually leaving or removed/banned by a group admin), the corresponding 'joined_group' activity entry will not be deleted.

Before I work up a patch, I generally think that this 5-minute membership check is unnecessary. It was introduced way back in ​https://github.com/buddypress/buddypress/commit/bbad97385015a171c8c52cb788ced2c1705221fd#diff-6bc6696e8c90e746617fe12d44d16641280cccb4d292c2ed278c74dfaaaff345, so this is quite old, but I'd like to consider removing this membership time check.

According to the commit message, the time check was meant to be some form of activity flood protection, but wouldn't this type of functionality be best suited in a plugin? Thoughts?

[emaralive]

I'm not sure that I'm following this correctly, is the intent to delete/remove the 'joined_group' activity item when someone leaves the group, regardless of the 5 minute rule? Meaning, the 5 minute rule is considered irrelevant?

[dcavins]

I'm not sure that the joined group item should be deleted. Doesn't it kind of make sense like in Slack where you see the join activity and the leave activity for a channel? I get that if you only join the group mistakenly then leave almost immediately, that maybe it makes sense to remove the activity item. I only know how Slack handles this; what do the other big social networks do, like facebook?

[r-a-y]

Forgot to CC myself to this ticket.

Meaning, the 5 minute rule is considered irrelevant?

Yes, the proposal would be to remove the 5-minute rule.

Doesn't it kind of make sense like in Slack where you see the join activity and the leave activity for a channel?

Currently, BP doesn't record when someone leaves a group into the group activity stream though.

I think your idea might make sense when a user manually leaves, but if a group admin is removing or banning the user from the group, the removed user's 'joined_group' activity item should be deleted from the group activity stream.

[emaralive]

In a way, deleting the "joined_group" activity item is an indirect way of indicating that someone has left a group (there is no record of joining). In essence, when someone joins a group the record is created and when someone leaves the group, the "joined_group" record is removed/deleted, for any of the 3 reasons.

Currently, group activity items may/can be removed/deleted, at will, by the group admin or mod. So, I'm not seeing much of any adverse effect by the proposal, well at least, not at the moment. Why don't we move forward and see what unintended consequences, if any, might arise from the proposal?

[johnjamesjacoby]

I think your idea might make sense when a user manually leaves, but if a group admin is removing or banning the user from the group, the removed user's 'joined_group' activity item should be deleted from the group activity stream.

This sums up my opinion nicely.

It seems best to:

• create activity items when a member intentionally joins/leaves, but not when a mod/admin does it on their behalf
• clean up the related activity items when a mod/admin does it on their behalf, but not when a member joins/leaves

This PR adjusts when the joined_group activity item is cleaned up.

When a member voluntarily leaves a group, the corresponding joined_group
activity is now preserved. Cleanup is still performed when a member is
removed or banned by a group admin or moderator.

This aligns with the consensus discussed in #9305 to distinguish between
voluntary member actions and admin/moderator actions.

### How to test

1. Create a group and join it as a user.
2. Leave the group as that user → the joined_group activity item should remain.
3. Join the group again, then remove or ban the user as an admin → the joined_group activity item should be deleted.

[manhphucofficial]

Thanks for taking a look!

This PR keeps the change intentionally small and focused on the direction discussed in #9305:
preserving the joined_group activity item when a member voluntarily leaves a group, while still
cleaning it up when a member is removed or banned by an admin or moderator.

Happy to adjust if there’s anything you’d like to see changed. Appreciate the review!

[dcavins]

What do we think about adding a left_group activity item to use when the user voluntarily leaves the group?

[emaralive]

Replying to dcavins:

What do we think about adding a left_group activity item to use when the user voluntarily leaves the group?

I don't have a problem with this notion, which appears to mimic the Slack behavior. If this is the case, wouldn't we also make use of a rejoined_group activity item?

[johnjamesjacoby]

Agree about both.

I like Slack as a good recent example of this being a normal thing.

The reason these don't exist yet is because I was always a bit concerned about the gossipy part of "why did they leave", and I do think that concern is still a valid one, but it hasn't really seemed to be a major issue in Slack or elsewhere.

[manhphucofficial]

Thanks for the discussion — really helpful context.

This PR is intentionally scoped to just fixing the current behavior:
preserving the existing joined_group activity when a user voluntarily leaves a group.

I agree that explicit left_group / rejoined_group activity types make sense long-term,
but that feels more like a follow-up enhancement than something this fix needs to solve.

Happy to help explore that in a follow-up ticket or PR if we decide to go that way.

[espellcaste]

In 14186:

Misc changes to the BuddyPress Groups Activity Functions.

See #9305
See #9173

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

[espellcaste]

I'd welcome some testing on this new approach: ​https://github.com/buddypress/buddypress/pull/432

• rejoined_group is added if a user had previously left the group.
• left_group is added when user intentionally leaves the group.
• joined_group, left_group, and rejoined_group are removed when an admin does it on their behalf.

I also removed the 5 min "activity flood protection". Only because I agree with @r-a-y. (For communities where this "flood"/scale would be an issue, this can be simply resolved by a plugin with a few hooks).

:)

[emaralive]

Replying to espellcaste:

I'd welcome some testing on this new approach: ​https://github.com/buddypress/buddypress/pull/432

@espellcaste - just for clarity; is testing to be performed with ​https://github.com/buddypress/buddypress/pull/428 applied, as well?

[espellcaste]

@emaralive Correct. This is ​applied too.

[emaralive]

@espellcaste - It appears that ​428 is already incorporated within ​432. My previous question was whether I needed to apply the changes from both Pull Requests and this was not the case. So apparently I misunderstood your response.

[emaralive]

Seems to me, in order to understand the pass/fail criteria, one should base this on the functional requirements. Some of these requirements may need further clarification, in an effort to reduce ambiguity and misinterpretations and for sake of an argument, let's pursue the following as an example.

From johnjamesjacoby comment 6:

It seems best to:

• create activity items when a member intentionally joins/leaves, but not when a mod/admin does it on their behalf
• clean up the related activity items when a mod/admin does it on their behalf, but not when a member joins/leaves

If we take the first bulleted item, in context to this ticket an activity item is defined as any of the following:

• joined_group
• rejoined_group
• left_group

NOTE: Since "leaves" is dependent on left_group, for now I'm just framing "joins".

The first clause of the sentence for joins, to me, indicates/infers:

• Public Group(s) - Only when a member joins or accepts a group invitation.
• Private Group(s) - Only when a member initiates a group membership request and it is accepted or accepts a group invitation.
• Hidden Group(s) - Only when a member accepts a group invitation.

The second clause may not be needed since, to me, it implies the first clause. However, I'm not sure if I've framed the requirements accurately nor do others have the same interpretation. Nevertheless, let's proceed on, since this example is academic.

Scenario

- User A is a site admin
- User B is a typical member
- Performed against a Public Group

1. User A adds User B to a group via the back-end (Edit Group page) - a joined_group activity item is created.
2. User A adds User B to a group via the REST API - a joined_group activity item is not created.
3. User A adds User B to a group via BP CLI - a joined_group activity item is created.

Assuming User B can accomplish the requirements for the first clause, i.e., joins (those tests pass), could we conclude that items 1 and 3 are failures and item 2 is a pass for User A? So, if this framing is incorrect then, here is your opportunity to provide the correct framing/interpretation of the requirements for the first bulleted item. Additionally, a caveat, I may have neglected to include other pertinent info.

We can move on to the second bulleted item after hashing through what has been presented here.

BTW, it appears to me that the left_group routines require additional work because with the BP Nouveau template pack, WP 7.0-alpha-61446 and PHP 8.3, I'm getting a fatal error when a member/user leaves a group.

---

Error Details

Fatal error: Uncaught ValueError: strpos(): Argument #3 ($offset) must be contained in argument #1 ($haystack) in buddypress/src/bp-templates/bp-nouveau/includes/activity/functions.php:371

Stack Trace
1.	
strpos()
buddypress/src/bp-templates/bp-nouveau/includes/activity/functions.php:371
2.	
bp_nouveau_activity_secondary_avatars()
/wp-includes/class-wp-hook.php:343
3.	
WP_Hook->apply_filters()
/wp-includes/plugin.php:256
4.	
apply_filters_ref_array()
buddypress/src/bp-activity/bp-activity-template.php:1382
5.	
bp_get_activity_action()
buddypress/src/bp-activity/bp-activity-template.php:1348
6.	
bp_activity_action()
buddypress/src/bp-templates/bp-nouveau/buddypress/activity/entry.php:30
7.	
require('...')
/wp-includes/template.php:816
8.	
load_template()
buddypress/src/bp-core/bp-core-template-loader.php:225
9.	
bp_locate_template()
buddypress/src/bp-core/bp-core-template-loader.php:67
10.	
bp_get_template_part()
buddypress/src/bp-templates/bp-nouveau/buddypress/activity/activity-loop.php:21
11.	
require_once('...')
/wp-includes/template.php:814
12.	
load_template()
buddypress/src/bp-templates/bp-nouveau/includes/ajax.php:150
13.	
bp_nouveau_ajax_object_template_loader()
/wp-includes/class-wp-hook.php:341
14.	
WP_Hook->apply_filters()
/wp-includes/class-wp-hook.php:365
15.	
WP_Hook->do_action()
/wp-includes/plugin.php:522
16.	
do_action()
/wp-admin/admin-ajax.php:192
17.	
{main}
thrown in buddypress/src/bp-templates/bp-nouveau/includes/activity/functions.php on line 371