From ba4b09d423c696921f0248ffaf406d02ee98f386 Mon Sep 17 00:00:00 2001 From: Daniel Walmsley Date: Fri, 15 Nov 2019 08:03:27 -0800 Subject: [PATCH] =?UTF-8?q?[not=20verified]=20Allow=20TOS=20agreement=20be?= =?UTF-8?q?fore=20Jetpack=20is=20fully=20active=20so=20we=20track=E2=80=A6?= =?UTF-8?q?=20(#14041)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Allow TOS agreement before Jetpack is fully active so we track the connection flow * Update packages/terms-of-service/src/class-terms-of-service.php Co-Authored-By: Derek Smart * Also update the action that is being called in jetpack We renamed the action lets also rename it in Jetpack. * [not verified] Revert "Also update the action that is being called in jetpack" This reverts commit e9c0f21d51151900e218e8253832ab1a5245575e. Co-authored-by: Brandon Kraft Co-authored-by: Enej Bajgoric --- .../terms-of-service/src/Terms_Of_Service.php | 15 +++------ .../tests/php/test_TermsOfService.php | 33 +++++-------------- 2 files changed, 14 insertions(+), 34 deletions(-) diff --git a/packages/terms-of-service/src/Terms_Of_Service.php b/packages/terms-of-service/src/Terms_Of_Service.php index 4541c221e551c..bf7ec844c4232 100644 --- a/packages/terms-of-service/src/Terms_Of_Service.php +++ b/packages/terms-of-service/src/Terms_Of_Service.php @@ -44,9 +44,9 @@ public function reject() { /** * Acton fired when the master user has revoked their agreement to the terms of service. * - * @since 7.9.0 + * @since 7.9.1 */ - do_action( 'jetpack_reject_to_terms_of_service' ); + do_action( 'jetpack_reject_terms_of_service' ); } /** @@ -60,15 +60,11 @@ public function reject() { * @return bool */ public function has_agreed() { - if ( ! $this->get_raw_has_agreed() ) { - return false; - } - if ( $this->is_development_mode() ) { return false; } - return $this->is_active(); + return $this->get_raw_has_agreed() || $this->is_active(); } /** @@ -89,8 +85,7 @@ protected function is_development_mode() { * @return bool */ protected function is_active() { - $manager = new Manager(); - return $manager->is_active(); + return ( new Manager() )->is_active(); } /** @@ -100,7 +95,7 @@ protected function is_active() { * @return bool */ protected function get_raw_has_agreed() { - return \Jetpack_Options::get_option( self::OPTION_NAME ); + return \Jetpack_Options::get_option( self::OPTION_NAME, false ); } /** diff --git a/packages/terms-of-service/tests/php/test_TermsOfService.php b/packages/terms-of-service/tests/php/test_TermsOfService.php index 8cc7eff1106ff..503eae6cfa1f7 100644 --- a/packages/terms-of-service/tests/php/test_TermsOfService.php +++ b/packages/terms-of-service/tests/php/test_TermsOfService.php @@ -13,7 +13,7 @@ class Test_Terms_Of_Service extends TestCase { */ public function setUp() { $this->terms_of_service = $this->createPartialMock( __NAMESPACE__ .'\\Terms_Of_Service', - [ 'get_raw_has_agreed', 'is_development_mode', 'is_active', 'set_agree', 'set_reject' ] + [ 'get_raw_has_agreed', 'is_development_mode', 'set_agree', 'is_active', 'set_reject' ] ); } @@ -29,7 +29,7 @@ public function tearDown() { */ public function test_agree() { $this->mock_function( 'do_action', null, 'jetpack_agreed_to_terms_of_service' ); - $this->terms_of_service->expects( $this->once() )->method( 'set_agree' )->willReturn( null ); + $this->terms_of_service->expects( $this->once() )->method( 'set_agree' ); $this->terms_of_service->agree(); } @@ -38,8 +38,8 @@ public function test_agree() { * @covers Automattic\Jetpack\Terms_Of_Service->revoke */ public function test_revoke() { - $this->mock_function( 'do_action', null, 'jetpack_reject_to_terms_of_service' ); - $this->terms_of_service->expects( $this->once() )->method( 'set_reject' )->willReturn( null ); + $this->mock_function( 'do_action', null, 'jetpack_reject_terms_of_service' ); + $this->terms_of_service->expects( $this->once() )->method( 'set_reject' ); $this->terms_of_service->reject(); } @@ -47,7 +47,7 @@ public function test_revoke() { /** * @covers Automattic\Jetpack\Terms_Of_Service->has_agreed */ - public function test_has_agreed_before_the_site_agrees() { + public function test_returns_false_if_not_agreed() { $this->terms_of_service->expects( $this->once() )->method( 'get_raw_has_agreed' )->willReturn( false ); $this->assertFalse( $this->terms_of_service->has_agreed() ); } @@ -55,9 +55,9 @@ public function test_has_agreed_before_the_site_agrees() { /** * @covers Automattic\Jetpack\Terms_Of_Service->has_agreed */ - public function test_has_agreed_is_development_mode() { - $this->terms_of_service->expects( $this->once() )->method( 'get_raw_has_agreed' )->willReturn( true ); + public function test_returns_false_if_has_agreed_but_is_development_mode() { // is_development_mode + $this->terms_of_service->method( 'get_raw_has_agreed' )->willReturn( true ); $this->terms_of_service->expects( $this->once() )->method( 'is_development_mode' )->willReturn( true ); $this->assertFalse( $this->terms_of_service->has_agreed() ); } @@ -65,9 +65,8 @@ public function test_has_agreed_is_development_mode() { /** * @covers Automattic\Jetpack\Terms_Of_Service->has_agreed */ - public function test_has_agreed_is_active_mode() { - $this->terms_of_service->expects( $this->once() )->method( 'get_raw_has_agreed' )->willReturn( true ); - // Not in dev mode... + public function test_returns_true_if_active_even_if_not_agreed() { + $this->terms_of_service->expects( $this->once() )->method( 'get_raw_has_agreed' )->willReturn( false ); $this->terms_of_service->expects( $this->once() )->method( 'is_development_mode' )->willReturn( false ); // Jetpack is active @@ -76,20 +75,6 @@ public function test_has_agreed_is_active_mode() { $this->assertTrue( $this->terms_of_service->has_agreed() ); } - /** - * @covers Automattic\Jetpack\Terms_Of_Service->has_agreed - */ - public function test_has_agreed_is_not_active_mode() { - $this->terms_of_service->expects( $this->once() )->method( 'get_raw_has_agreed' )->willReturn( true ); - // not in dev mode... - $this->terms_of_service->expects( $this->once() )->method( 'is_development_mode' )->willReturn( false ); - - // Jetpack is not active - $this->terms_of_service->expects( $this->once() )->method( 'is_active' )->willReturn( false ); - - $this->assertFalse( $this->terms_of_service->has_agreed() ); - } - /** * Mock a global function and make it return a certain value. *