Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Error and prevent deleting root site #73

Merged
merged 5 commits into from Sep 15, 2017
Merged

Error and prevent deleting root site #73

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
86a084c
GH#66 Added flag --delete-root
Sidsector9 Aug 31, 2017
2fa9d0b
GH#66 Added functional test
Sidsector9 Aug 31, 2017
a89f6fb
GH#66 Correct the order of test cases
Sidsector9 Aug 31, 2017
9f79413
GH#66 Added message while trying to deleting root blog
Sidsector9 Sep 11, 2017
abae414
Update language a bit
danielbachhuber Sep 15, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions features/site.feature
View file
Open in desktop
Expand Up @@ -56,6 +56,12 @@ Feature: Manage sites in a multisite installation
http://example.com/first/
"""

When I run `wp site delete 1`
Then STDOUT should be:
"""
'http://example.com/' is the root site. Pass the --delete-root flag to delete it.
"""

When I run `wp site delete {SITE_ID} --yes`
Then STDOUT should be:
"""
Expand Down
7 changes: 6 additions & 1 deletion src/Site_Command.php
View file
Open in desktop
Expand Up @@ -265,7 +265,12 @@ function delete( $args, $assoc_args ) {

$site_url = trailingslashit( $blog->siteurl );

WP_CLI::confirm( "Are you sure you want to delete the '$site_url' site?", $assoc_args );
if ( $blog_id == BLOG_ID_CURRENT_SITE && ! \WP_CLI\Utils\get_flag_value( $assoc_args, 'delete-root' ) ) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use is_main_site here

WP_CLI::line( WP_CLI::colorize( "%R'$site_url' is the root site. Pass the --delete-root flag to delete it.%n" ), $assoc_args );
return;
} else {
WP_CLI::confirm( "Are you sure you want to delete the '$site_url' site?", $assoc_args );
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given we already have a confirmation prompt before deleting a site, I don't think it's necessary to require another flag to delete the root site.

Instead, when deleting the root site we should customize the confirmation message to indicate what happens when you delete the root site.

As currently implemented, this would be a breaking change

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For a scenario as:

http://wpcli.com
http://one.wpcli.com
http://two.wpcli.com

Following observations have been made after executing
wp site delete 1

  1. The tables aren't dropped but the users from wpcli.com blog are deleted.
  2. wpcli.com gets deactivated ( You can only activate it again through direct updating in the database since the admin dashboard doesn't give you an option to activate/deactivate the main site. )

Irrelevant of the fact that whether you want to keep/drop tables, the users will get deleted as evident here


wpmu_delete_blog( $blog->blog_id, ! \WP_CLI\Utils\get_flag_value( $assoc_args, 'keep-tables' ) );

Expand Down