New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sanitize database at the end of install to prevent duplicate data #76

Merged
merged 4 commits into from Jul 20, 2018

Conversation

2 participants
@javorszky
Contributor

javorszky commented Jul 20, 2018

Closes #75

If the multisite constants are already in wp-config.php, there's no way to stop populate_network to add an empty site_admins option when creating a new install.

This PR adds code that, at the end, would remove all empty site_admins options. Later multisite-install calls add_site_admins which 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_admins would just update the site_admins, and that should work. Not in this case.

update_network_option will check the existing value of the option, and if it's false, then it will use add_network_option because false means the option doesn't exist in the database.

get_network_option will 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 $notoptions array, and store that in the cache.

The first time site_admins is queried, the database is practically empty, so the site_admins key ends up being part of the $notoptions variable and cache.

Subsequent query to get_network_option will hit the part where it's a notoption and return the default value - which is false.

Which means in order to get this working, we need to actually delete the empty site_admins option, because a new one will be added anyways.

Show outdated Hide outdated src/Core_Command.php
Show outdated Hide outdated src/Core_Command.php
Show outdated Hide outdated src/Core_Command.php
@schlessera

This comment has been minimized.

Show comment
Hide comment
@schlessera

schlessera Jul 20, 2018

Member

@javorszky Thanks for the PR, the code looks good to me.

Would you be up to writing a Behat regression test for that?

Member

schlessera commented Jul 20, 2018

@javorszky Thanks for the PR, the code looks good to me.

Would you be up to writing a Behat regression test for that?

@javorszky

This comment has been minimized.

Show comment
Hide comment
@javorszky

javorszky Jul 20, 2018

Contributor

I can try. Not sure how I could test having object caching in place though.... bear with, I'll make an attempt.

Contributor

javorszky commented Jul 20, 2018

I can try. Not sure how I could test having object caching in place though.... bear with, I'll make an attempt.

@schlessera

This comment has been minimized.

Show comment
Hide comment
@schlessera

schlessera Jul 20, 2018

Member

You don't need to test object caching. Just test to make sure you don't leave empty site_admin options behind.

Member

schlessera commented Jul 20, 2018

You don't need to test object caching. Just test to make sure you don't leave empty site_admin options behind.

@schlessera schlessera added this to the 1.0.11 milestone Jul 20, 2018

@schlessera schlessera merged commit ecb6bcd into wp-cli:master Jul 20, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@schlessera

This comment has been minimized.

Show comment
Hide comment
@schlessera

schlessera Jul 20, 2018

Member

Thanks for the pull-request, @javorszky !

Member

schlessera commented Jul 20, 2018

Thanks for the pull-request, @javorszky !

@javorszky javorszky deleted the javorszky:issue/75 branch Jul 23, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment