From 7dc57e31498552bde4ce124e5ad73436b276d23e Mon Sep 17 00:00:00 2001 From: Igor Zinovyev Date: Wed, 19 Dec 2018 23:32:32 +0300 Subject: [PATCH 1/7] Added a bare bones unit test suite for the Site SAL class. --- phpunit.xml.dist | 1 + .../test_class.json-api-platform-jetpack.php | 37 +++++++++++++++++++ 2 files changed, 38 insertions(+) create mode 100644 tests/php/sal/test_class.json-api-platform-jetpack.php diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 94ab4e6a28990..8c7a106c69a3d 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -34,6 +34,7 @@ tests/php/test_class.json-api-jetpack-endpoints.php + tests/php/sal tests/php/modules/infinite-scroll diff --git a/tests/php/sal/test_class.json-api-platform-jetpack.php b/tests/php/sal/test_class.json-api-platform-jetpack.php new file mode 100644 index 0000000000000..ac376885fdc27 --- /dev/null +++ b/tests/php/sal/test_class.json-api-platform-jetpack.php @@ -0,0 +1,37 @@ + get_current_blog_id(), + 'user_id' => get_current_user_id(), + 'external_user_id' => 2, + 'role' => 'administrator' + ); + + $platform = wpcom_get_sal_platform( static::$token ); + + static::$site = $platform->get_site( static::$token->blog_id ); + } + + static function tearDownAfterClass() { + // renable https + $_SERVER['HTTPS'] = 'on'; + parent::tearDownAfterClass(); + } + + function test_uses_synced_api_post_type_whitelist_if_available() { + + $this->assertFalse( static::$site->is_post_type_allowed( 'my_new_type' ) ); + } +} From 98eb251152ffe386205f1d3d917ce4ccab416b37 Mon Sep 17 00:00:00 2001 From: Igor Zinovyev Date: Thu, 20 Dec 2018 00:04:56 +0300 Subject: [PATCH 2/7] Added a simple test suite for the Post object. --- .../sal/test_class.json-api-post-jetpack.php | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 tests/php/sal/test_class.json-api-post-jetpack.php diff --git a/tests/php/sal/test_class.json-api-post-jetpack.php b/tests/php/sal/test_class.json-api-post-jetpack.php new file mode 100644 index 0000000000000..848c6f398dc55 --- /dev/null +++ b/tests/php/sal/test_class.json-api-post-jetpack.php @@ -0,0 +1,39 @@ + get_current_blog_id(), + 'user_id' => get_current_user_id(), + 'external_user_id' => 2, + 'role' => 'administrator' + ); + + $platform = wpcom_get_sal_platform( static::$token ); + + static::$site = $platform->get_site( static::$token->blog_id ); + } + + function test_returns_content_wrapped_in_a_post_object() { + // Insert the post into the database + $post_id = wp_insert_post( array( + 'post_title' => 'Title', + 'post_content' => 'The content.', + 'post_status' => 'publish', + 'post_author' => get_current_user_id() + ) ); + + $post = get_post( $post_id ); + + $wrapped_post = static::$site->wrap_post( $post, 'display' ); + + $this->assertEquals( $post->post_type, $wrapped_post->get_type() ); + } +} From 3ed138ef666165f24c69cc0819c41bc1074ec025 Mon Sep 17 00:00:00 2001 From: Igor Zinovyev Date: Thu, 20 Dec 2018 00:37:34 +0300 Subject: [PATCH 3/7] Added a mandatory is_module_active method to the Site SAL class. --- sal/class.json-api-site-base.php | 2 ++ sal/class.json-api-site-jetpack-base.php | 8 ++++++ .../test_class.json-api-platform-jetpack.php | 26 +++++++++++++++++++ 3 files changed, 36 insertions(+) diff --git a/sal/class.json-api-site-base.php b/sal/class.json-api-site-base.php index 4d413efcd3bfe..02e8959c34102 100644 --- a/sal/class.json-api-site-base.php +++ b/sal/class.json-api-site-base.php @@ -77,6 +77,8 @@ abstract public function is_jetpack(); abstract public function get_jetpack_modules(); + abstract public function is_module_active( $module ); + abstract public function is_vip(); abstract public function is_multisite(); diff --git a/sal/class.json-api-site-jetpack-base.php b/sal/class.json-api-site-jetpack-base.php index 156fe2b0d7c6d..d13b60b032cb7 100644 --- a/sal/class.json-api-site-jetpack-base.php +++ b/sal/class.json-api-site-jetpack-base.php @@ -96,6 +96,14 @@ function get_jetpack_modules() { return null; } + function is_module_active( $module ) { + if ( is_user_member_of_blog() ) { + return in_array ( $module, Jetpack_Options::get_option( 'active_modules', array() ), true ); + } + + return false; + } + function is_vip() { return false; // this may change for VIP Go sites, which sync using Jetpack } diff --git a/tests/php/sal/test_class.json-api-platform-jetpack.php b/tests/php/sal/test_class.json-api-platform-jetpack.php index ac376885fdc27..5178941efd8c9 100644 --- a/tests/php/sal/test_class.json-api-platform-jetpack.php +++ b/tests/php/sal/test_class.json-api-platform-jetpack.php @@ -34,4 +34,30 @@ function test_uses_synced_api_post_type_whitelist_if_available() { $this->assertFalse( static::$site->is_post_type_allowed( 'my_new_type' ) ); } + + function test_is_module_active() { + + // Picking random 5 modules from an array of existing ones to not slow down the test + $modules = array_rand( Jetpack::get_available_modules(), 3 ); + + foreach ( $modules as $module ) { + Jetpack::deactivate_module( $module ); + + $this->assertEquals( + Jetpack::is_module_active( $module ), + static::$site->is_module_active( $module ) + ); + + Jetpack::activate_module( $module ); + + $this->assertEquals( + Jetpack::is_module_active( $module ), + static::$site->is_module_active( $module ) + ); + } + } + + function test_interface() { + $this->assertTrue( method_exists( 'SAL_Site', 'is_module_active' ) ); + } } From 6df6eebdd7a955b24e18dd8ea67e7aa550bdb9d1 Mon Sep 17 00:00:00 2001 From: Igor Zinovyev Date: Thu, 20 Dec 2018 00:54:05 +0300 Subject: [PATCH 4/7] Fixed a number in a comment, removed fiddling with HTTPS. --- .../php/sal/test_class.json-api-platform-jetpack.php | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/tests/php/sal/test_class.json-api-platform-jetpack.php b/tests/php/sal/test_class.json-api-platform-jetpack.php index 5178941efd8c9..37df9e3391b01 100644 --- a/tests/php/sal/test_class.json-api-platform-jetpack.php +++ b/tests/php/sal/test_class.json-api-platform-jetpack.php @@ -9,9 +9,6 @@ class SalSiteTest extends WP_UnitTestCase { static function setUpBeforeClass( ) { parent::setUpBeforeClass(); - // temporarily disable https - $_SERVER['HTTPS'] = 'off'; - static::$token = (object) array( 'blog_id' => get_current_blog_id(), 'user_id' => get_current_user_id(), @@ -24,12 +21,6 @@ static function setUpBeforeClass( ) { static::$site = $platform->get_site( static::$token->blog_id ); } - static function tearDownAfterClass() { - // renable https - $_SERVER['HTTPS'] = 'on'; - parent::tearDownAfterClass(); - } - function test_uses_synced_api_post_type_whitelist_if_available() { $this->assertFalse( static::$site->is_post_type_allowed( 'my_new_type' ) ); @@ -37,7 +28,7 @@ function test_uses_synced_api_post_type_whitelist_if_available() { function test_is_module_active() { - // Picking random 5 modules from an array of existing ones to not slow down the test + // Picking random 3 modules from an array of existing ones to not slow down the test $modules = array_rand( Jetpack::get_available_modules(), 3 ); foreach ( $modules as $module ) { From 88371849b794a4ddaa9363396fc9a8a6763655b5 Mon Sep 17 00:00:00 2001 From: Igor Zinovyev Date: Thu, 20 Dec 2018 14:27:24 +0300 Subject: [PATCH 5/7] Switched static var usage to self. --- .../php/sal/test_class.json-api-platform-jetpack.php | 12 ++++++------ tests/php/sal/test_class.json-api-post-jetpack.php | 8 ++++---- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/php/sal/test_class.json-api-platform-jetpack.php b/tests/php/sal/test_class.json-api-platform-jetpack.php index 37df9e3391b01..9082812ae249c 100644 --- a/tests/php/sal/test_class.json-api-platform-jetpack.php +++ b/tests/php/sal/test_class.json-api-platform-jetpack.php @@ -9,21 +9,21 @@ class SalSiteTest extends WP_UnitTestCase { static function setUpBeforeClass( ) { parent::setUpBeforeClass(); - static::$token = (object) array( + self::$token = (object) array( 'blog_id' => get_current_blog_id(), 'user_id' => get_current_user_id(), 'external_user_id' => 2, 'role' => 'administrator' ); - $platform = wpcom_get_sal_platform( static::$token ); + $platform = wpcom_get_sal_platform( self::$token ); - static::$site = $platform->get_site( static::$token->blog_id ); + self::$site = $platform->get_site( self::$token->blog_id ); } function test_uses_synced_api_post_type_whitelist_if_available() { - $this->assertFalse( static::$site->is_post_type_allowed( 'my_new_type' ) ); + $this->assertFalse( self::$site->is_post_type_allowed( 'my_new_type' ) ); } function test_is_module_active() { @@ -36,14 +36,14 @@ function test_is_module_active() { $this->assertEquals( Jetpack::is_module_active( $module ), - static::$site->is_module_active( $module ) + self::$site->is_module_active( $module ) ); Jetpack::activate_module( $module ); $this->assertEquals( Jetpack::is_module_active( $module ), - static::$site->is_module_active( $module ) + self::$site->is_module_active( $module ) ); } } diff --git a/tests/php/sal/test_class.json-api-post-jetpack.php b/tests/php/sal/test_class.json-api-post-jetpack.php index 848c6f398dc55..6fd2132af4977 100644 --- a/tests/php/sal/test_class.json-api-post-jetpack.php +++ b/tests/php/sal/test_class.json-api-post-jetpack.php @@ -9,16 +9,16 @@ class SalPostsTest extends WP_UnitTestCase { static function setUpBeforeClass() { parent::setUpBeforeClass(); - static::$token = (object) array( + self::$token = (object) array( 'blog_id' => get_current_blog_id(), 'user_id' => get_current_user_id(), 'external_user_id' => 2, 'role' => 'administrator' ); - $platform = wpcom_get_sal_platform( static::$token ); + $platform = wpcom_get_sal_platform( self::$token ); - static::$site = $platform->get_site( static::$token->blog_id ); + self::$site = $platform->get_site( self::$token->blog_id ); } function test_returns_content_wrapped_in_a_post_object() { @@ -32,7 +32,7 @@ function test_returns_content_wrapped_in_a_post_object() { $post = get_post( $post_id ); - $wrapped_post = static::$site->wrap_post( $post, 'display' ); + $wrapped_post = self::$site->wrap_post( $post, 'display' ); $this->assertEquals( $post->post_type, $wrapped_post->get_type() ); } From 9c3bff1c70775d4615098eeb6edbfa05912808a9 Mon Sep 17 00:00:00 2001 From: Igor Zinovyev Date: Thu, 20 Dec 2018 23:40:21 +0300 Subject: [PATCH 6/7] Removed is_user_member_of_blog calls in favor of API permissions. --- sal/class.json-api-site-jetpack-base.php | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/sal/class.json-api-site-jetpack-base.php b/sal/class.json-api-site-jetpack-base.php index d13b60b032cb7..d87a2bce7b5f9 100644 --- a/sal/class.json-api-site-jetpack-base.php +++ b/sal/class.json-api-site-jetpack-base.php @@ -89,19 +89,11 @@ function after_render_options( &$options ) { } function get_jetpack_modules() { - if ( is_user_member_of_blog() ) { - return array_values( Jetpack_Options::get_option( 'active_modules', array() ) ); - } - - return null; + return array_values( Jetpack_Options::get_option( 'active_modules', array() ) ); } function is_module_active( $module ) { - if ( is_user_member_of_blog() ) { - return in_array ( $module, Jetpack_Options::get_option( 'active_modules', array() ), true ); - } - - return false; + return in_array ( $module, Jetpack_Options::get_option( 'active_modules', array() ), true ); } function is_vip() { From 6fcd58d56c81005c50707d49d6b5d8dd7ccc7b9a Mon Sep 17 00:00:00 2001 From: Igor Zinovyev Date: Fri, 21 Dec 2018 14:51:40 +0300 Subject: [PATCH 7/7] Added a user blog membership check in the API endpoint instead of SAL. --- json-endpoints/class.wpcom-json-api-get-site-endpoint.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/json-endpoints/class.wpcom-json-api-get-site-endpoint.php b/json-endpoints/class.wpcom-json-api-get-site-endpoint.php index c94e504b14ddc..a7868dea07b14 100644 --- a/json-endpoints/class.wpcom-json-api-get-site-endpoint.php +++ b/json-endpoints/class.wpcom-json-api-get-site-endpoint.php @@ -355,9 +355,8 @@ protected function render_response_key( $key, &$response, $is_user_logged_in ) { $response[ $key ] = $this->site->get_capabilities(); break; case 'jetpack_modules': - $jetpack_modules = $this->site->get_jetpack_modules(); - if ( ! is_null( $jetpack_modules ) ) { - $response[ $key ] = $jetpack_modules; + if ( is_user_member_of_blog() ) { + $response[ $key ] = $this->site->get_jetpack_modules(); } break; case 'plan' :