[not verified] Allow TOS agreement before Jetpack is fully active so …

…we track… (#14041) * 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 <smart@automattic.com> * 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 e9c0f21. Co-authored-by: Brandon Kraft <public@brandonkraft.com> Co-authored-by: Enej Bajgoric <enej.bajgoric@gmail.com>
Automattic · Nov 15, 2019 · ba4b09d · ba4b09d
1 parent 4003bc5
commit ba4b09d
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 34 deletions.
diff --git 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
@@ -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,36 +38,35 @@ 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();
 	}
 
 	/**
 	 * @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() );
 	}
 
 	/**
 	 * @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() );
 	}
 
 	/**
 	 * @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.
 	 *