Make WordPress Core

Opened 11 years ago

Closed 8 years ago

#25584 closed enhancement (fixed)

Add an action hook at the end of wpmu_delete_blog

Reported by: pauldewouters's profile pauldewouters Owned by: jeremyfelt's profile jeremyfelt
Milestone: 4.8 Priority: normal
Severity: normal Version: 3.0
Component: Networks and Sites Keywords: has-patch commit
Focuses: multisite Cc:

Description

I have the need of an additional hook in the wpmu_delete_blog function in wp-admin/includes/ms.php.

I'm maintaining a list of active blogs in a DB option which also has extra data associated. I update this list on 2 hooks currently: wpmu_new_blog and delete_blog

Unfortunately, the delete_blog occurs before the actual blog deletion, so at that point, an SQL query will return that blog too.

If there were a hook after the actual deletion, I could retrieve the list of active blogs.

Attachments (4)

ms.diff (399 bytes) - added by pauldewouters 11 years ago.
25584.diff (504 bytes) - added by jeremyfelt 11 years ago.
25584.2.diff (504 bytes) - added by jeremyfelt 11 years ago.
25584.3.diff (537 bytes) - added by johnjamesjacoby 8 years ago.
Move action outside of $drop check and pass $drop as a parameter to match delete_blog usage

Download all attachments as: .zip

Change History (24)

@pauldewouters
11 years ago

#1 @SergeyBiryukov
11 years ago

  • Milestone changed from Awaiting Review to 3.8
  • Type changed from feature request to enhancement

#2 @SergeyBiryukov
11 years ago

  • Keywords has-patch added

@jeremyfelt
11 years ago

#3 @jeremyfelt
11 years ago

25584.diff is slightly different from the original patch. I think deleted_site aligns with the naming of some other hooks in core. Docs for the hook have also been updated to reflect the proper version for @since.

#4 @SergeyBiryukov
11 years ago

  • Keywords commit added

#5 @DrewAPicture
11 years ago

  • Keywords dev-feedback added

Worth noting that the parent function is named wpmu_delete_blog, and this hook would be preceded by a delete_blog hook. So perhaps in the name of consistency we should go with deleted_blog vs deleted_site.

If at some time in the future the parent function gets renamed to say, wp_delete_site, I'd imagine the hooks would remain in the name of posterity and back-compat. I think either situation (renaming or not renaming the function), I'd prefer the hooks match. That's just me.

Last edited 11 years ago by DrewAPicture (previous) (diff)

#6 @nacin
11 years ago

  • Keywords commit dev-feedback removed
  • Milestone changed from 3.8 to Future Release

I'd tend to agree with keeping the hooks the same style.

@jeremyfelt
11 years ago

#7 @jeremyfelt
11 years ago

  • Milestone changed from Future Release to 3.9

25584.2.diff is a refresh for 3.9, goes with deleted_blog as the filter.

#9 @pauldewouters
11 years ago

thanks for the improvements

#10 @jeremyfelt
11 years ago

  • Component changed from Multisite to Networks and Sites
  • Focuses multisite added

#11 @jeremyfelt
11 years ago

  • Keywords commit added

25584.2.diff still applies cleanly. Adds deleted_blog filter with docs.

#12 @DrewAPicture
11 years ago

The hook docs in 25584.2.diff look great. Let's do this thing.

Last edited 11 years ago by DrewAPicture (previous) (diff)

#13 @nacin
11 years ago

  • Keywords 2nd-opinion added
  • Milestone changed from 3.9 to Future Release

This filter is inside a if ( $drop ) conditional which means it would only be fired when data is actually deleted.

I'll be honest, I don't see much of a purpose for this hook. The existing delete_blog action is enough, even though it is on pre-transaction. If you're trying to query only active blogs, simply exclude the blog for which the hook has fired. While we would generally provide and benefit from a post-transaction hook, I'd hesitate to bother adding one until we re-do this function at some point down the line (with better terminology and less hackery).

#14 @virgodesign
10 years ago

+1 for this request.

#15 @wonderboymusic
9 years ago

  • Keywords commit removed

#16 @johnjamesjacoby
8 years ago

  • Keywords needs-refresh added; 2nd-opinion removed
  • Milestone changed from Future Release to 4.8
  • Owner set to jeremyfelt
  • Status changed from new to assigned
  • Version set to 3.0

I also just ran into a need for this.

Core already has deleted_post and deleted_user, so the proposed deleted_blog action makes sense.

I'm -1 on naming it deleted_site for the reasons @DrewAPicture stated.

@johnjamesjacoby
8 years ago

Move action outside of $drop check and pass $drop as a parameter to match delete_blog usage

#17 @johnjamesjacoby
8 years ago

  • Keywords commit added; needs-refresh removed

This ticket was mentioned in Slack in #core-multisite by jjj. View the logs.


8 years ago

#19 @jeremyfelt
8 years ago

25584.3.diff looks good. I think it makes sense adding this to help with other cleanup related operations. If we ever redo this function, it will likely be very different and have different actions.

#20 @jeremyfelt
8 years ago

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

In 40351:

Multisite: Add deleted_blog action after site has been deleted.

Props pauldewouters, johnjamesjacoby.
Fixes #25584.

Note: See TracTickets for help on using tickets.