[wp-trac] [WordPress Trac] #40364: Improve site creation in multisite

[WordPress Trac]

#40364: Improve site creation in multisite
-------------------------------------------------+-------------------------
 Reporter:  jeremyfelt                           |       Owner:  flixos90
     Type:  enhancement                          |      Status:  assigned
 Priority:  normal                               |   Milestone:  5.0
Component:  Networks and Sites                   |     Version:
 Severity:  normal                               |  Resolution:
 Keywords:  ms-roadmap has-patch has-unit-tests  |     Focuses:  multisite
  needs-dev-note                                 |
-------------------------------------------------+-------------------------
Changes (by flixos90):

 * keywords:  ms-roadmap has-patch has-unit-tests => ms-roadmap has-patch
     has-unit-tests needs-dev-note


Comment:

 [attachment:"40364.13.diff"] takes the feedback from before into account.

 @spacedmonkey
 * The defaults parsing and the whitelist intersection are duplicate code
 in `wp_insert_site()` and `wp_update_site()`, but they need to remain in
 those functions as those two are the only absolutely critical parts that
 should not be unhooked. `wp_normalize_site_data` is a filter, so these
 crucial parts must happen outside.
 * The same applies to renaming `network_id` back to `site_id` prior to the
 DB transaction. If that happened in `wp_normalize_site_data()`, it would
 affect `wp_validate_site_data()`, and it would be unexpected having to
 deal with `network_id` in one function and with `site_id` in the other.
 * However, I agree the duplicate code there is ugly and unnecessary.
 Therefore I introduced a `wp_prepare_site_data()` function that contains
 all of it. That function now executes the `wp_normalize_site_data` filter,
 parses against defaults and whitelist, and executes the
 `wp_validate_site_data` action. This way we don't have duplicate code,
 while it is still obligatory.

 @jeremyfelt
 * I changed the `$domain` argument description to `Site domain. Default
 empty string.`.
 * `$validity` is now called `$errors`.
 * I agree it makes sense to make the network ID assurance optional, as
 technically 0 is an allowed value. I adjusted the respective code (now
 part of `wp_prepare_site_data()`): The current network ID (or previous
 network ID in case of an update) is now just a default against which the
 data is parsed. There is still a clause in the validation function that
 prevents a network ID of 0, but that again can theoretically be unhooked.
 * I don't think we need to validate a network exists for the given ID.
 Let's keep this for the dev note.
 * I didn't introduce any checks for deleting the main site.
 `wpmu_delete_blog()` currently does not have a check like that, I think
 the functions here are too low-level in order to account for such things.
 _If_ we need truly need this, it can also be an enhancement in the future.
 * Regarding the database tables for a site, those will be covered by
 #41333. That is why I'd like to have this ticket here merged asap so that
 we can start with the other ticket. `wp_install_site()` and
 `wp_uninstall_site()` will be hooked into `wp_insert_site` and
 `wp_delete_site` respectively, so that then the DB tables are handled
 correctly. Regarding deletion of sites in general, that is a matter of
 developer education.
 * I'm not sure we need to deal with someone passing `$blog_id` in the
 array of arguments. It is not a valid argument for inserting or updating a
 site. For updates, the `$site_id` parameter is there for that.
 * I added a check that ensures `wp_update_site( 0 )` and `wp_delete_site(
 0 )` return an error. We shouldn't fall back to the current site in either
 case here.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/40364#comment:59>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform