Skip to:

Opened 6 years ago

Last modified 6 years ago

#7647 new defect (bug)

Group membership events need "after" actions

Reported by: boonebgorges's profile boonebgorges Owned by:
Milestone: Awaiting Contributions Priority: normal
Severity: normal Version:
Component: Groups Keywords: needs-patch good-first-bug


Our hooks 'groups_join_group' and 'groups_leave_group' fire after the database update has taken place. The hooks 'groups_promote_member', 'groups_demote_member', 'groups_ban_member', 'groups_unban_member', and 'groups_remove_member' fire *before* the database update.

There are two problems here. One is the inconsistency. We may not be able to fix this, for backward compatibility reasons. Two is the fact that it's clunky to fire an action *after* a removal/ban/etc. I think we can more easily fix this, by the introduction of 'groups_promoted_member', etc hooks. This is more or less what WP does in various places.

Does this sound like a reasonable approach?

Change History (2)

#1 @dcavins
6 years ago

I've run into this oddity before, too. The existing "before" action points do make some sense to me, especially when an association is about to change. However, having a complete set of "we just did this thing" actions would be a great improvement--especially if they include the old status and the new status, sort of like the transition_post_status action points.

I'm wondering if it would make sense to add "before" action points for 'groups_join_group' and 'groups_leave_group', too.

#2 @boonebgorges
6 years ago

A generic transition_ hook was actually my original thought here, but since we don't do this elsewhere in BP, I decided not to suggest it :) But something like that could be cool, and for more than just group membership status.

I'm wondering if it would make sense to add "before" action points for 'groups_join_group' and 'groups_leave_group', too.


Another thought I had while writing the ticket: It would be nice for at least one hook in BP_Groups_Member::save() to receive both the old and the new data. This would be sorta like a transition hook.

Note: See TracTickets for help on using tickets.