Opened 6 years ago
Closed 6 years ago
#46351 closed defect (bug) (fixed)
Properly re-fill the `$meta` param of deprecated `wpmu_new_blog` action
Reported by: | david.binda | Owned by: | jeremyfelt |
---|---|---|---|
Milestone: | 5.1.1 | Priority: | normal |
Severity: | normal | Version: | 5.1 |
Component: | Networks and Sites | Keywords: | has-patch commit fixed-major |
Focuses: | multisite | Cc: |
Description
The updates in r43548 changed the way how $meta
value passed to wpmu_new_blog
is being populated.
It introduces the $site_data_whitelist
array (see https://core.trac.wordpress.org/browser/trunk/src/wp-includes/ms-functions.php?rev=43548#L1288 ) which contains keys of properties which are now being passed to wp_insert_site
, where those are being processed, and removes those keys from the $meta
array.
The list of keys was previously used for identifying what properties are being saved as site options, but those were still passed to wpmu_new_blog
action in terms of $meta
param. (see https://core.trac.wordpress.org/browser/trunk/src/wp-includes/ms-functions.php?rev=42976#L1309 )
That said, after the 5.1.0, the $meta
in wpmu_new_blog
no longer contain any of following properies:
<?php $site_data_whitelist = array( 'public', 'archived', 'mature', 'spam', 'deleted', 'lang_id' );
Further, in r43654 the WPLANG
, previously default meta in wpmu_create_blog
was moved elsewhere, and is also not available among $meta
in wpmu_new_blog
action.
While the wpmu_create_blog
is no longer the recommend way of creating new blogs, and wpmu_new_blog
action have been deprecated, there still are some integrations out there, which may rely on the previous version of the $meta
array.
I'm attaching a patch for properly re-filling the values and a unit test demonstrating the issue. The patch is introducing more values to the $meta
, since the current implementation really wants to re-populate the $meta
variable in case there are some callbacks hooked to wpmu_new_blog
action, so it's easier to pass more data (with default values), than adjusting the wp_insert_site
function the way it only passes the same limited set of properties.
Attachments (3)
Change History (18)
This ticket was mentioned in Slack in #core by lukecarbis. View the logs.
6 years ago
#3
@
6 years ago
@davidbinda: I've just been testing your patch. Is your intention to exactly duplicate the behavior of WP <=5.0.3? If so, it does not do that. I don't think it does anything wrong, but is results in $meta
containing more info than would be provided in 5.0.3.
For example, when adding a new site via /wp-admin/network/site-new.php
, the value of $meta
passed to functions hooked to wpmu_new_blog
is:
in 5.0.3
<?php $meta = array( 'public' => 1, 'WP_LANG' => get_network_option( $network_id, 'WPLANG' ), )
in 5.1, without your patch
<?php $meta = array( 'WPLANG' => get_network_option( $network_id, 'WPLANG' ), )
- so, yes, without the patch the value of
public
is not passed
in 5.1, with your patch
<?php $meta = array( 'public' => 1, 'archived' => 0, 'mature' => 0, 'spam' => 0, 'deleted' => 0, 'lang_id' => 0, 'WPLANG' => get_network_option( $network_id, 'WPLANG' ), )
- the values of
archived
,mature
,spam
,deleted
andlang_id
where not passed in 5.0.3
So, again, I'm not suggesting that your patch is wrong, just that it results in slightly different behavior (for the default case of creating a site in the WP admin module) than 5.0.3.
#4
@
6 years ago
Thanks for testing the patch!
So, again, I'm not suggesting that your patch is wrong, just that it results in slightly different behavior (for the default case of creating a site in the WP admin module) than 5.0.3.
Yup, I wanted to introduce just minimal changes to the existing code, and was accepting the slightly different behaviour as a good enough.
But, I've re-visited the code and the patch in order to achieve the same behaviour, and turns out the necessary changes are not that huge and do no make the code less readable. Please, see the updated patch.
Thanks again!
#5
@
6 years ago
@davidbinda: the revised patches looks good!
When adding a site interactively, I get the same array passed to my function hooked to wpmy_new_blog
as I do in 5.0.3. When I "manually" call wpmu_create_blog()
with additional $meta
, that additional $meta
also gets passed the same as it does in 5.0.3.
Something is screwy with my local setup and I can't run unit tests right now, so I can't test the unit test in your patch. However, visually inspecting it, in test_wpmu_new_blog_action_backward_compatible()
, shouldn't
<?php remove_action( 'populate_options', array( $this, 'wpmu_new_blog_callback' ), 10 );
be
<?php remove_action( 'wpmu_new_blog', array( $this, 'wpmu_new_blog_callback' ), 10 );
#6
@
6 years ago
Thanks for the code review and the feedback!
You're right, a wrong action name was used in the remove_action
call. However, inspecting the WP_UnitTestCase_Base::__restore_hooks
showed that there is no need to remove action/filter set in one test, as the method "Restores the hook-related globals to their state at setUp() so that future tests aren't affected by hooks set during this last test."
I've adjusted the patch accordingly (removing the remove_action
call altogether).
This ticket was mentioned in Slack in #core by pbiron. View the logs.
6 years ago
#11
@
6 years ago
This looks good, thanks @davidbinda and @pbiron!
Clarification for my own future sake:
The $meta
that was passed to wpmu_create_blog()
prior to 5.1.0 was "promoted" (for lack of a better word) to top level keys in $data
when passed to wp_insert_site()
in 5.1.0+.
The 5.1.0 back-compat handling for the wpmu_new_blog
hook accounted for $meta
, but only for data passed to the new options
key (yet to be documented, see #45061).
Thanks, @davidbinda - this makes sense at first glance. I'm going to move to the 5.1.1 milestone.