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.

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

Add hooks to support customize-concurrency plugin. #87

Merged
merged 3 commits into from Aug 28, 2016

Conversation

Projects
None yet
3 participants
@lgedeon
Copy link
Contributor

commented Aug 28, 2016

No description provided.

@coveralls

This comment has been minimized.

Copy link

commented Aug 28, 2016

Coverage Status

Coverage increased (+0.02%) to 91.003% when pulling e853ab6 on feature/concurrency-hooks into 7d1e373 on develop.

@@ -590,7 +590,8 @@
api.trigger( 'customize-snapshots-update', {
previewUrl: url,
customizeUrl: customizeUrl,
uuid: component.data.uuid
uuid: component.data.uuid,
response: response

This comment has been minimized.

Copy link
@westonruter

westonruter Aug 28, 2016

Contributor

Good call! Adds parity with the save event.

@@ -1181,6 +1186,8 @@ function( $value ) {
wp_send_json_error( $data );
}
/** This filter is documented in wp-includes/class-wp-customize-manager.php */
$data = apply_filters( 'customize_save_response', $data, $this );

This comment has been minimized.

Copy link
@westonruter

westonruter Aug 28, 2016

Contributor

Not quite, as $this is CustomizeSnapshots\Customize_Snapshot_Manager not WP_Customize_Manager. So you should replace $this with $this->customize_manager.

@@ -1125,6 +1125,11 @@ public function handle_update_snapshot_request() {
) );
}
/**
* Add any additional checks before saving snapshot.

This comment has been minimized.

Copy link
@westonruter

westonruter Aug 28, 2016

Contributor

Missing @param tags.

/**
* Add any additional checks before saving snapshot.
*/
do_action( 'customize_snapshot_save_before', $this->snapshot, $this->customize_manager );

This comment has been minimized.

Copy link
@westonruter

westonruter Aug 28, 2016

Contributor

Since this action is specific to snapshots, I think $this->customize_manager should be replaced with just $this, the Customize_Snapshot_Manager.

$that->actioned_customizer = $test_customizer;
}, 10, 2 );
add_filter( 'customize_save_response', function( $data, $test_snapshot_manager ) use ( $that ) {
$that->filtered_snapshot_manager = $test_snapshot_manager;

This comment has been minimized.

Copy link
@westonruter

westonruter Aug 28, 2016

Contributor

The filtered_snapshot_manager, actioned_snapshot, and actioned_customizer should all be declared as public class variables, instead of defining them as ad hoc “expando properties”.

$this->assertEquals( $manager->snapshot(), $this->actioned_snapshot );
$this->assertEquals( $manager->customize_manager, $this->actioned_customizer );
$this->assertEquals( $manager, $this->filtered_snapshot_manager );

This comment has been minimized.

Copy link
@westonruter

westonruter Aug 28, 2016

Contributor

Good.

This comment has been minimized.

Copy link
@westonruter

westonruter Aug 28, 2016

Contributor

Oh, you should probably clean these up using something like:


    /**
     * Clean up global scope.
     */
    function clean_up_global_scope() {
        parent::clean_up_global_scope();
        $this->actioned_snapshot = null;
        $this->actioned_customizer = null;
        $this->filtered_snapshot_manager = null;
    }

Or you could add this to a tearDown method.

@coveralls

This comment has been minimized.

Copy link

commented Aug 28, 2016

Coverage Status

Coverage decreased (-0.03%) to 90.951% when pulling dacdf9b on feature/concurrency-hooks into 7d1e373 on develop.

@westonruter

This comment has been minimized.

Copy link
Contributor

commented Aug 28, 2016

Unit tests are failing because WordPress.org's SVN is down:

Installing WP from https://develop.svn.wordpress.org/trunk/ to /tmp/wordpress
svn: OPTIONS of 'https://develop.svn.wordpress.org/trunk': could not connect to server (https://develop.svn.wordpress.org)
dev-lib/check-diff.sh: line 336: /tmp/wordpress/src/wp-content/db.php: No such file or directory
@coveralls

This comment has been minimized.

Copy link

commented Aug 28, 2016

Coverage Status

Coverage decreased (-0.03%) to 90.951% when pulling cad91c8 on feature/concurrency-hooks into 7d1e373 on develop.

@westonruter westonruter added this to the 0.6.0 milestone Aug 28, 2016

@westonruter westonruter merged commit b074773 into develop Aug 28, 2016

2 checks passed

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

@westonruter westonruter deleted the feature/concurrency-hooks branch Aug 28, 2016

@westonruter

This comment has been minimized.

Copy link
Contributor

commented Aug 28, 2016

@lgedeon 👏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.