Jetpack Sync: Add sync action for theme edit#7151
Conversation
enejb
left a comment
There was a problem hiding this comment.
Lets make sure that the filter wp_redirect always returns $redirect_url.
| add_action( 'jetpack_updated_theme', $callable, 10, 2 ); | ||
| add_action( 'delete_site_transient_update_themes', array( $this, 'detect_theme_deletion') ); | ||
| add_action( 'jetpack_deleted_theme', $callable ); | ||
| add_filter( 'wp_redirect', array( $this, 'detect_theme_edit') ); |
| public function detect_theme_edit( $redirect_url ) { | ||
| $url = wp_parse_url( $redirect_url ); | ||
| if ( '/wp-admin/theme-editor.php' !== $url['path'] ) { | ||
| return; |
There was a problem hiding this comment.
This will make make all the redirect stop working.
Since the filter returns null instead of the the passed in value.
Also we should only check the expected value I am not sure why we are omitting the domain in the check?
There was a problem hiding this comment.
Does it make sense to check the existence of $_POST['newcontent] here ?
| public function test_edit_theme_sync() { | ||
| $theme_slug = 'itek'; | ||
| $theme_name = 'iTek'; | ||
| $this->install_theme( $theme_slug ); |
There was a problem hiding this comment.
Doing this in every test make the test really slow. Can we make them faster somehow?
roccotripaldi
left a comment
There was a problem hiding this comment.
Very cool! I was reviewing just as you were addressing @enejb 's feedback. LGTM!
| public function detect_theme_edit( $redirect_url ) { | ||
| $url = wp_parse_url( $redirect_url ); | ||
| if ( '/wp-admin/theme-editor.php' !== $url['path'] ) { | ||
| $expected_url = admin_url() . 'theme-editor.php'; |
There was a problem hiding this comment.
admin_url( 'theme-editor.php' ); should work as well.
| * @since 1.5.1 | ||
| */ | ||
| $_POST['newcontent'] = 'foo'; | ||
| apply_filters( 'wp_redirect', 'theme-editor.php?file=style.css&theme=' . $theme_slug . '&scrollto=0&updated=true'); |
There was a problem hiding this comment.
'&scrollto=0&updated=true' ); missing space.
There was a problem hiding this comment.
Should we try testing this with different types of urls passed into the redirect?
Such as the full url?
| wp_die( $api ); | ||
| } | ||
|
|
||
| $upgrader = new Theme_Upgrader( new Test_Upgrader_Skin( compact('title', 'nonce', 'url', 'theme') ) ); |
There was a problem hiding this comment.
There are some spaces missing here as well.
| } | ||
|
|
||
| public function detect_theme_edit( $redirect_url ) { | ||
| $url = wp_parse_url( admin_url( $redirect_url ) ); |
There was a problem hiding this comment.
$redirect_url is a full url. such as google.com
we would expected $url = wp_parse_url( "http://example.com/wp-admin/http://google.com" );
is that what we want?
There was a problem hiding this comment.
$redirect_url is just the file name when/where we need to examine it, see https://github.com/WordPress/WordPress/blob/master/wp-admin/theme-editor.php#L124
|
I have tested this on MU and it work. |
* Changelog: first pass at a changelog for 5.0 * Changelog: delete 4.9 testing list. * Changelog: update minimum WP version to match ver. in jetpack.php Fixes #7158 * Changelog: add #6051 * Changelog: add #6753 * Changelog: add #6928 * Changelog: add #6964 * Changelog: add #7014 * Changelog: add #7057 * Changelog: add #7060 * Changelog: add #7068 * Changelog: add #7070 * Changelog: add #7072 * Changelog: add #7071 * Changelog: add release date and post shortlink. * Changelog: add #7094 * Changelog: add #7100 * Changelog: add #7108 * Changelog: add #7113 * Changelog: add #7123 * Changelog: add #7135 * Changelog: add #7143 * Changelog: add #7151 * Changelog: add #6996 * Changelog: add #7105 * Changelog: add #7132 * Changelog: add #7166 * Changelog: fix typo in 4.9 changelog. * Changelog: remove older releases' changelogs. @see p1HpG7-42e-p2 * Changelog: add #7090 * Changelog: add #7095 * Changelog: add #7112 * Changelog: add #7115 * Changelog: add #7122 * Changelog: add #7137 * Changelog: add #7138 * Changelog: add #7140 * Changelog: add #7154 * Changelog: add ##7155 * Changelog: add #7163 * Changelog: add #7167 * Changelog: add #7171 * Changelog: add #7180 * Changelog: add #7181 * Changelog: add #7183 * Changelog: add #7184 * Changelog: add #7189 * Changelog: add #7191 * Changelog: add #7193 * Changelog: add #7198 * Changelog: add #7200 * Changelog: add #7209 * Changelog: add #7212 * Testing list: add instructions for #7115 * Changelog: add #7188 * Changelog: add #7205 * Changelog: add #7225 * Changelog: add #6872 * Changelog: add #7107 * Changelog: add #7118 * Changelog: add #7142 * Changelog: add #7170 * Changelog: add #7210 * Changelog: add #7218 * Changelog: add #7232 * Changelog: add #7211 * Changelog: add #7213 * Changelog: add #7229 * Changelog: add #7230 * Changelog: add #7214 * Draft changelog for 5.0 * Changelog updates: 2nd pass at a clearer changelog. - Fix typos. - Use consistent tense and tone across all changelog. - Remove unclear items. * Changelog: add #7026 * Changelog: add #7058 * Changelog: add #7125 * Changelog: add #7249 * Changelog: add #7185 * add mentions of image widget migration * Changelog: add info about new output for CLI command. * Changelog: add WP version number matching the new Image Widget.
Jetpack Sync: Add sync action for theme edit
Changes proposed in this Pull Request:
Adds a sync action for theme edit
Testing instructions:
phpunit runs a new test that tests the functionality
Proposed changelog entry for your changes: