Skip to content

Avoid deleting theme if the specified version cannot be found #349

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

Merged
schlessera merged 3 commits into from
Feb 7, 2023
Merged

Avoid deleting theme if the specified version cannot be found #349

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
42bc674
Add a failing test case for accidental theme deletion
danielbachhuber Jan 27, 2023
d21b50f
Move a few other relevant tests to this new file
danielbachhuber Jan 27, 2023
daae9fb
Deleting the theme is handled by `install()` if necessary
danielbachhuber Jan 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
115 changes: 115 additions & 0 deletions features/theme-update.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
Feature: Update WordPress themes

Scenario: Updating a theme with no version in the WordPress.org directory shouldn't delete the original theme
Given a WP install

When I run `wp scaffold underscores wpclitesttheme`
Then STDOUT should contain:
"""
Success: Created theme
"""
And the wp-content/themes/wpclitesttheme directory should exist

When I try `wp theme update wpclitesttheme --version=100.0.0`
Then STDERR should contain:
"""
Error: No themes installed
"""
And the wp-content/themes/wpclitesttheme directory should exist

Scenario: Install a theme, then update to a specific version of that theme
Given a WP install

When I run `wp theme install p2 --version=1.4.1`
Then STDOUT should not be empty

When I run `wp theme update p2 --version=1.4.2`
Then STDOUT should not be empty

When I run `wp theme list --fields=name,version`
Then STDOUT should be a table containing rows:
| name | version |
| p2 | 1.4.2 |

Scenario: Not giving a slug on update should throw an error unless --all given
Given a WP install
And I run `wp theme path`
And save STDOUT as {THEME_DIR}
And an empty {THEME_DIR} directory

# No themes installed. Don't give an error if --all given for BC.
When I run `wp theme update --all`
Then STDOUT should be:
"""
Success: No themes installed.
"""

When I run `wp theme update --version=0.6 --all`
Then STDOUT should be:
"""
Success: No themes installed.
"""

# One theme installed.
Given I run `wp theme install p2 --version=1.4.2`

When I try `wp theme update`
Then the return code should be 1
And STDERR should be:
"""
Error: Please specify one or more themes, or use --all.
"""
And STDOUT should be empty

When I run `wp theme update --all`
Then STDOUT should contain:
"""
Success: Updated
"""

When I run the previous command again
Then STDOUT should be:
"""
Success: Theme already updated.
"""

# Note: if given version then re-installs.
When I run `wp theme update --version=1.4.2 --all`
Then STDOUT should contain:
"""
Success: Installed 1 of 1 themes.
"""

When I run the previous command again
Then STDOUT should contain:
"""
Success: Installed 1 of 1 themes.
"""

# Two themes installed.
Given I run `wp theme install --force twentytwelve --version=1.0`

When I run `wp theme update --all`
Then STDOUT should contain:
"""
Success: Updated
"""

When I run the previous command again
# BUG: Message should be in plural.
Then STDOUT should be:
"""
Success: Theme already updated.
"""

# Using version with all rarely makes sense and should probably error and do nothing.
When I try `wp theme update --version=1.4.2 --all`
Then the return code should be 1
And STDOUT should contain:
"""
Success: Installed 1 of 1 themes.
"""
And STDERR should be:
"""
Error: Can't find the requested theme's version 1.4.2 in the WordPress.org theme repository (HTTP code 404).
"""
97 changes: 0 additions & 97 deletions features/theme.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -366,20 +366,6 @@ Feature: Manage WordPress themes
And STDOUT should be empty
And the return code should be 1

Scenario: Install a theme, then update to a specific version of that theme
Given a WP install

When I run `wp theme install p2 --version=1.4.1`
Then STDOUT should not be empty

When I run `wp theme update p2 --version=1.4.2`
Then STDOUT should not be empty

When I run `wp theme list --fields=name,version`
Then STDOUT should be a table containing rows:
| name | version |
| p2 | 1.4.2 |

Scenario: Install and attempt to activate a child theme without its parent
Given a WP install
And I run `wp theme install moina-blog`
Expand Down Expand Up @@ -493,89 +479,6 @@ Feature: Manage WordPress themes
"""
And STDERR should be empty

Scenario: Not giving a slug on update should throw an error unless --all given
Given a WP install
And I run `wp theme path`
And save STDOUT as {THEME_DIR}
And an empty {THEME_DIR} directory

# No themes installed. Don't give an error if --all given for BC.
When I run `wp theme update --all`
Then STDOUT should be:
"""
Success: No themes installed.
"""

When I run `wp theme update --version=0.6 --all`
Then STDOUT should be:
"""
Success: No themes installed.
"""

# One theme installed.
Given I run `wp theme install p2 --version=1.4.2`

When I try `wp theme update`
Then the return code should be 1
And STDERR should be:
"""
Error: Please specify one or more themes, or use --all.
"""
And STDOUT should be empty

When I run `wp theme update --all`
Then STDOUT should contain:
"""
Success: Updated
"""

When I run the previous command again
Then STDOUT should be:
"""
Success: Theme already updated.
"""

# Note: if given version then re-installs.
When I run `wp theme update --version=1.4.2 --all`
Then STDOUT should contain:
"""
Success: Installed 1 of 1 themes.
"""

When I run the previous command again
Then STDOUT should contain:
"""
Success: Installed 1 of 1 themes.
"""

# Two themes installed.
Given I run `wp theme install --force twentytwelve --version=1.0`

When I run `wp theme update --all`
Then STDOUT should contain:
"""
Success: Updated
"""

When I run the previous command again
# BUG: Message should be in plural.
Then STDOUT should be:
"""
Success: Theme already updated.
"""

# Using version with all rarely makes sense and should probably error and do nothing.
When I try `wp theme update --version=1.4.2 --all`
Then the return code should be 1
And STDOUT should contain:
"""
Success: Installed 1 of 1 themes.
"""
And STDERR should be:
"""
Error: Can't find the requested theme's version 1.4.2 in the WordPress.org theme repository (HTTP code 404).
"""

Scenario: Get status field in theme detail
Given a WP install

Expand Down
9 changes: 2 additions & 7 deletions src/Theme_Command.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -663,13 +663,8 @@ public function update( $args, $assoc_args ) {

if ( isset( $assoc_args['version'] ) ) {
foreach ( $this->fetcher->get_many( $args ) as $theme ) {
$r = delete_theme( $theme->stylesheet );
if ( is_wp_error( $r ) ) {
WP_CLI::warning( $r );
} else {
$assoc_args['force'] = true;
$this->install( array( $theme->stylesheet ), $assoc_args );
}
$assoc_args['force'] = true;
$this->install( array( $theme->stylesheet ), $assoc_args );
}
} else {
parent::update_many( $args, $assoc_args );
Expand Down