Skip to:
Content

Opened 8 years ago

Closed 8 years ago

#2773 closed enhancement (fixed)

Add "pre-delete" hooks to groups component

Reported by: vtowel Owned by:
Milestone: 1.5 Priority: normal
Severity: Version: 1.5
Component: Groups Keywords:
Cc: bhalla.kunal@…

Description

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.

Attachments (3)

before-delete.patch (2.3 KB) - added by kunalb 8 years ago.
before-friends.patch (1.1 KB) - added by kunalb 8 years ago.
before-activity.patch (809 bytes) - added by kunalb 8 years ago.

Download all attachments as: .zip

Change History (17)

#1 @kunalb
8 years ago

  • Cc bhalla.kunal@… added
  • Owner set to kunalb
  • Status changed from new to assigned
  • Version changed from 1.2.5 to 1.3

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.

#2 @DJPaul
8 years ago

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

#3 @kunalb
8 years ago

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

#4 @kunalb
8 years ago

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.

#5 @djpaul
8 years ago

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

#6 @DJPaul
8 years ago

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

#7 @kunalb
8 years ago

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.

#8 @djpaul
8 years ago

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

#9 @DJPaul
8 years ago

  • Keywords plugins extending removed

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?

#10 @kunalb
8 years ago

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

#11 @DJPaul
8 years ago

  • Owner kunalb deleted

#12 @DennisSmolek
8 years ago

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

#13 @DJPaul
8 years ago

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

#14 @djpaul
8 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

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

Note: See TracTickets for help on using tickets.