Block theme updates if they have been modified by PM #104
Conversation
…into try/block-theme-updates-if-pattern-added # Conflicts: # wp-modules/pattern-data-handlers/pattern-data-handlers.php
There was a problem hiding this comment.
Hi @johnstonphilip,
What do you think about waiting to see how many people this is a problem for?
You know I'm a code snob, so maybe I'm overreacting 😆
Now, update_pattern() and delete_pattern() do 3 things:
- Saves the
pm_mod_to the DB - Saves a pattern to disk
- Tree shakes
It's getting really complicated, and we haven't even released to wp.org.
Thanks for your patience with all of my nitpicking 😄
You want the best for users, and you're working on a lot of approaches.
| function block_theme_updates_if_modified_by_pm( $update_themes_transient_data ) { | ||
|
|
||
| // No update object exists. Return early. | ||
| if ( empty( $update_themes_transient_data ) || ! isset( $update_themes_transient_data->response ) ) { |
There was a problem hiding this comment.
What do you think about removing this if, and simply blocking theme auto-updates if PM is active?
That'll probably anger some people.
But they're not going to benefit from PM anyway, and we could notify them on plugin activation that we're blocking theme auto-updates.
There was a problem hiding this comment.
I can see the argument. Maybe a reason not to (and keep it how it is) is that someone might have another theme where they are testing update functionality with some custom server, and this would have them waste a lot of time trying to figure out why "their update code isn't working".
There was a problem hiding this comment.
Side, note: how many people are auto-updating in their local?
PM is for local development.
There was a problem hiding this comment.
I would offer that updates likely start in Local. So devs can test and make sure things are not breaking.
| @@ -314,6 +314,9 @@ function update_pattern( $pattern ) { | |||
| FS_CHMOD_FILE | |||
| ); | |||
|
|
|||
| // Put a flag in the database to indicate this theme has been modified by PM. | |||
| update_option( 'pm_mod_' . get_stylesheet(), true ); | |||
There was a problem hiding this comment.
If we're going to do this PR, would it be possible to move this into a helper function? So ideally it's only adding 1 line to update_pattern() and delete_pattern()
| $expected = $value; | ||
| $expected->response = array(); | ||
|
|
||
| $result = \PatternManager\PreventThemeUpdates\block_theme_updates_if_modified_by_pm( $value, 'update_themes' ); |
There was a problem hiding this comment.
These are great unit tests. Very thorough, and very realistic data.
| foreach ( $update_themes_transient_data->response as $theme_slug => $theme_with_update_available ) { | ||
| $theme_has_been_modified_by_pm = get_option( 'pm_mod_' . $theme_slug ); | ||
|
|
||
| if ( $theme_has_been_modified_by_pm ) { |
There was a problem hiding this comment.
Hi @johnstonphilip,
What do you think about only blocking updates for the active theme when a pm_pattern post exists?
$theme_has_been_modified_by_pm = basename( get_stylesheet_directory() ) === $theme_slug && get_pm_post_ids();As you know, editing a pattern creates a pm_pattern post.
Still, this will miss this case:
- Edit a pattern
- Activate another theme
- Go back to the theme from step 1
- Try to update the active theme via
/wp-admin - Expected: this code blocks the theme update
- Actual: it doesn't, as the
pm_patternposts were deleted on changing themes
This approach won't require saving any new data.
But your approach is much more complete.
Thanks for being so patient about this.
This PR makes it so that if a theme has been modified by Pattern Manager, it will not be available for auto-update within the WordPress dashboard. This is to prevent people accidentally wiping out their patterns.
How to test
Before adding a pattern with PM
After adding a pattern with PM
Documentation
Suggested changelog entry
Notes