Sanitize database at the end of install to prevent duplicate data#76
Sanitize database at the end of install to prevent duplicate data#76schlessera merged 4 commits intowp-cli:masterfrom javorszky:issue/75
Conversation
src/Core_Command.php
Outdated
| WP_CLI::warning( "Multisite constants could not be written to 'wp-config.php'. You may need to add them manually:" . PHP_EOL . $ms_config ); | ||
| } | ||
| } else { | ||
| /** |
There was a problem hiding this comment.
This is not a valid docblock, as it is not attached to an entity that can receive docblocks. Please use a regular multiline comment instead.
src/Core_Command.php
Outdated
| } | ||
| } else { | ||
| /** | ||
| * Multisite constants are defined, therefore we already have an empty site_admins site meta |
There was a problem hiding this comment.
CS: Terminate first comment line with a period.
src/Core_Command.php
Outdated
| * | ||
| * Code based on parts of delete_network_option | ||
| */ | ||
| $rows = $wpdb->get_results( "SELECT meta_id, site_id FROM {$wpdb->sitemeta} WHERE meta_key = 'site_admins'" ); |
There was a problem hiding this comment.
I'd prefer this to only delete site admin options that are empty (by just adding that as an additional WHERE clause). Otherwise, someone might already have imported a pre-existing DB that contained the proper admin settings, and we are then deleting everything.
|
@javorszky Thanks for the PR, the code looks good to me. Would you be up to writing a Behat regression test for that? |
|
I can try. Not sure how I could test having object caching in place though.... bear with, I'll make an attempt. |
|
You don't need to test object caching. Just test to make sure you don't leave empty site_admin options behind. |
|
Thanks for the pull-request, @javorszky ! |
Closes #75
If the multisite constants are already in
wp-config.php, there's no way to stoppopulate_networkto add an emptysite_adminsoption when creating a new install.This PR adds code that, at the end, would remove all empty
site_adminsoptions. Latermultisite-installcallsadd_site_adminswhich checks whether a user is a super admin, which would fail because the option exists, but is empty, so it would add another entry to the sitemeta table.So normally the
add_site_adminswould just update the site_admins, and that should work. Not in this case.update_network_optionwill check the existing value of the option, and if it'sfalse, then it will useadd_network_optionbecausefalsemeans the option doesn't exist in the database.get_network_optionwill return whatever is stored in the cache. If that happens to be nothing, then the first time a query is ran, it will check in the database, and if it's not in the database, it will store it in a$notoptionsarray, and store that in the cache.The first time
site_adminsis queried, the database is practically empty, so thesite_adminskey ends up being part of the$notoptionsvariable and cache.Subsequent query to
get_network_optionwill hit the part where it's anotoptionand return the default value - which isfalse.Which means in order to get this working, we need to actually delete the empty
site_adminsoption, because a new one will be added anyways.