Add "pre-delete" hooks to groups component

[vtowel]

When a group is deleted, the only hook available in groups_delete_group is triggered after the group and all its metadata is deleted. So any group plugins which require access to group information (e.g., for custom cleanup) are denied it.

Some plugins, such as one I am writing to add group-specific Google Docs support, require group metadata to look up the group's associated Google Docs folder ID before deletion. I had to get around it by cumbersomely storing that data in the WP options table instead. Similarly, I needed to hook into groups_remove_data_for_user to unshare the Google Docs folder for each of the groups that user was subscribed to, but encountered the same problem because the user's group memberships had already been deleted before my action was triggered.

There may be other places in BP where a "pre-delete" hook would be useful, but for sure in groups_delete_group and groups_remove_data_for_user.

WordPress solved this problem by supporting both a delete_xyz hook (which occurs before deletion of xyz) and a deleted_xyz hook (which occurs after deletion of xyz). Perhaps BuddyPress components could mimic WordPress in this way.

Alternatively, the current delete hooks could just be moved above the SQL delete commands - but I'm not sure what BP developers would prefer to do.

[kunalb]

Having hooks at both ends seems to be the best way to go about it—but I'm worried about breaking plugins that expect the data to have been deleted by the time these filters are called.

Ideally, I'd go about moving the commands up and add groups_deleted_group and groups_removed_data_for_user after the actual deletion—would that be fine? Or should I leave the given actions where they are and add groups_pre_delete_group and groups_pre_remove_data_for_user to the files (this is probably safer in terms of not breaking any existing plugins). Comments?

Also, a cursory glance at friends, etc. indicates that this can be replicated across the components.

[DJPaul]

My opinion is we need to keep backwards compatibility. Any new post-delete hooks we implement going forwards can be written in the past tense. Rather than "pre", I suggest "before." i.e.

groups_before_delete_group => groups_delete_group
groups_before_remove_data_for_user => groups_remove_data_for_user

[kunalb]

Cool—will add these functions as groups_before_delete_group, etc. for as many components as I can.

[kunalb]

Added a patch for bp-groups.php. I might have been a wee bit too enthusiastic in adding more actions—if this is fine, I'll go ahead and add similar actions to the rest of the components.

[djpaul]

(In [3581]) Add before delete hooks to Groups component (see #2773). Props kunalb

[DJPaul]

Thanks, I've put this in and have left ticket open for other components ;)

[kunalb]

Added another 2 components.Getting a bit worried about inconsistencies in naming, though—these have instances where abc_delete is before the actual deletion, with abc_deleted after. Just might get a but too confusing afterwards.

[djpaul]

(In [3772]) Add before delete hooks to Friends and Activity components (see #2773). Props kunalb

[DJPaul]

Kunal, did you look through the rest of BuddyPress when you made the last patches? Can we close this ticket or are there still some to do?

[kunalb]

I have only covered the groups, friends and activities components as of now.

[DennisSmolek]

Which other components would you like to have this for 1.3? otherwise lets close it now or punt it..

[DJPaul]

It would be nice to have consistency of hooks between all components, where possible.

[djpaul]

(In [4397]) Add some more before delete action hooks. Fixes #2773