Updates to use centralised account status function (#8028)

Co-authored-by: Oleksandr Aratovskyi <[email protected]> Co-authored-by: Vlad Olaru <[email protected]>
Automattic · Jan 19, 2024 · 21a024b · 21a024b
1 parent fd41bcc
commit 21a024b
Show file tree

Hide file tree

Showing 6 changed files with 69 additions and 105 deletions.
diff --git a/changelog/dev-centralize-account-status-logic b/changelog/dev-centralize-account-status-logic
@@ -0,0 +1,4 @@
+Significance: minor
+Type: dev
+
+Updates to account status logic to streamline it.
diff --git a/includes/admin/class-wc-payments-admin.php b/includes/admin/class-wc-payments-admin.php
@@ -351,7 +351,7 @@ public function add_payments_menu() {
 		}
 		try {
 			// Render full payments menu with sub-items only if the merchant completed the KYC (details_submitted = true).
-			$should_render_full_menu = $this->account->is_account_fully_onboarded();
+			$should_render_full_menu = $this->account->is_stripe_connected() && $this->account->is_details_submitted();
 		} catch ( Exception $e ) {
 			// There is an issue with connection, don't render full menu, user will get redirected to the connect page.
 			$should_render_full_menu = false;
@@ -799,6 +799,7 @@ private function get_js_settings(): array {
 		}
 
 		$locale_info = include $path;
+
 		// Get symbols for those currencies without a short one.
 		$symbols       = get_woocommerce_currency_symbols();
 		$currency_data = [];

diff --git a/includes/class-wc-payment-gateway-wcpay.php b/includes/class-wc-payment-gateway-wcpay.php
@@ -679,7 +679,7 @@ public function is_connected() {
 	 * @return bool
 	 */
 	public function is_account_partially_onboarded(): bool {
-		return $this->account->is_account_partially_onboarded();
+		return $this->account->is_stripe_connected() && ! $this->account->is_details_submitted();
 	}
 
 	/**

diff --git a/includes/class-wc-payments-account.php b/includes/class-wc-payments-account.php
@@ -200,6 +200,7 @@ public function is_stripe_account_valid(): bool {
 		if ( ! $this->is_stripe_connected() ) {
 			return false;
 		}
+
 		$account = $this->get_cached_account_data();
 
 		if ( ! isset( $account['capabilities']['card_payments'] ) ) {
@@ -225,41 +226,26 @@ public function is_account_rejected(): bool {
 	}
 
 	/**
-	 * Checks if the account has not completed onboarding due to users abandoning the process half way.
-	 * Returns true if the onboarding is started but did not finish.
+	 * Checks if the account "details_submitted" flag is true.
+	 * This is a proxy for telling if an account has completed onboarding.
+	 * If the "details_submitted" flag is false, it means that the account has not
+	 * yet finished the initial KYC.
 	 *
-	 * @return bool True if the account is connected and details are not submitted, false otherwise.
+	 * @return boolean True if the account is connected and details are not submitted, false otherwise.
 	 */
-	public function is_account_partially_onboarded(): bool {
-		if ( ! $this->is_stripe_connected() ) {
-			return false;
-		}
-
+	public function is_details_submitted(): bool {
 		$account = $this->get_cached_account_data();
-		return false === $account['details_submitted'];
-	}
-
-	/**
-	 * Checks if the account has completed onboarding/KYC.
-	 * Returns true if the onboarding/KYC is completed.
-	 *
-	 * @return bool True if the account is connected and details are submitted, false otherwise.
-	 */
-	public function is_account_fully_onboarded(): bool {
-		if ( ! $this->is_stripe_connected() ) {
-			return false;
-		}
 
-		$account = $this->get_cached_account_data();
-		return true === $account['details_submitted'];
+		$details_submitted = $account['details_submitted'] ?? false;
+		return true === $details_submitted;
 	}
 
 	/**
 	 * Gets the account status data for rendering on the settings page.
 	 *
 	 * @return array An array containing the status data, or [ 'error' => true ] on error or no connected account.
 	 */
-	public function get_account_status_data() {
+	public function get_account_status_data(): array {
 		$account = $this->get_cached_account_data();
 
 		if ( empty( $account ) ) {
@@ -859,31 +845,33 @@ public function maybe_redirect_settings_to_connect_or_overview(): bool {
 			return false;
 		}
 
-		// Account fully onboarded, don't redirect.
-		if ( $this->is_account_fully_onboarded() ) {
-			return false;
-		}
-
-		// Account partially onboarded, redirect to overview.
-		if ( $this->is_account_partially_onboarded() ) {
+		// Not able to establish Stripe connection, redirect to the Connect page.
+		if ( ! $this->is_stripe_connected() ) {
 			$this->redirect_to(
 				admin_url(
 					add_query_arg(
 						[
 							'page' => 'wc-admin',
-							'path' => '/payments/overview',
+							'path' => '/payments/connect',
 						],
 						'admin.php'
 					)
 				)
 			);
+			return true;
+		}
+
+		if ( $this->is_details_submitted() ) {
+			// Account fully onboarded, don't redirect.
+			return false;
 		} else {
+			// Account not yet fully onboarded so redirect to overview page.
 			$this->redirect_to(
 				admin_url(
 					add_query_arg(
 						[
 							'page' => 'wc-admin',
-							'path' => '/payments/connect',
+							'path' => '/payments/overview',
 						],
 						'admin.php'
 					)
@@ -967,7 +955,7 @@ public function maybe_handle_onboarding() {
 
 		if ( isset( $_GET['wcpay-login'] ) && check_admin_referer( 'wcpay-login' ) ) {
 			try {
-				if ( $this->is_account_partially_onboarded() ) {
+				if ( $this->is_stripe_connected() && ! $this->is_details_submitted() ) {
 					$args         = $_GET;
 					$args['type'] = 'complete_kyc_link';
 

diff --git a/tests/unit/admin/test-class-wc-payments-admin.php b/tests/unit/admin/test-class-wc-payments-admin.php
@@ -134,19 +134,13 @@ public function tear_down() {
 		parent::tear_down();
 	}
 
-	/**
-	 * @dataProvider feature_flag_combinations_not_causing_settings_badge_render_provider
-	 *
-	 * @param bool $is_upe_settings_preview_enabled
-	 * @param bool $is_upe_enabled
-	 */
-	public function test_it_does_not_render_settings_badge( $is_upe_settings_preview_enabled, $is_upe_enabled ) {
+	public function test_it_does_not_render_settings_badge(): void {
 		global $submenu;
 
 		$this->mock_current_user_is_admin();
 
 		// Make sure we render the menu with submenu items.
-		$this->mock_account->method( 'is_account_fully_onboarded' )->willReturn( true );
+		$this->mock_account->method( 'is_details_submitted' )->willReturn( true );
 		$this->mock_account->method( 'is_stripe_connected' )->willReturn( true );
 		$this->payments_admin->add_payments_menu();
 
@@ -161,7 +155,7 @@ public function test_it_does_not_render_payments_badge_if_stripe_is_connected()
 		$this->mock_current_user_is_admin();
 
 		// Make sure we render the menu with submenu items.
-		$this->mock_account->method( 'is_account_fully_onboarded' )->willReturn( true );
+		$this->mock_account->method( 'is_details_submitted' )->willReturn( true );
 		$this->mock_account->method( 'is_stripe_connected' )->willReturn( true );
 		$this->payments_admin->add_payments_menu();
 
@@ -180,7 +174,7 @@ public function test_it_refreshes_the_cache_if_get_param_exists() {
 		];
 
 		// Make sure we render the menu with submenu items.
-		$this->mock_account->method( 'is_account_fully_onboarded' )->willReturn( true );
+		$this->mock_account->method( 'is_details_submitted' )->willReturn( true );
 		$this->mock_account->method( 'is_stripe_connected' )->willReturn( true );
 		$this->mock_account->expects( $this->once() )->method( 'refresh_account_data' );
 		$this->payments_admin->add_payments_menu();
@@ -195,7 +189,7 @@ public function test_it_renders_payments_badge_if_activation_date_is_older_than_
 		$this->mock_current_user_is_admin();
 
 		// Make sure we render the menu without submenu items.
-		$this->mock_account->method( 'is_account_fully_onboarded' )->willReturn( false );
+		$this->mock_account->method( 'is_details_submitted' )->willReturn( false );
 		$this->mock_account->method( 'is_stripe_connected' )->willReturn( false );
 		update_option( 'wcpay_activation_timestamp', time() - ( 3 * DAY_IN_SECONDS ) );
 		$this->payments_admin->add_payments_menu();
@@ -210,7 +204,7 @@ public function test_it_does_not_render_payments_badge_if_activation_date_is_les
 		$this->mock_current_user_is_admin();
 
 		// Make sure we render the menu without submenu items.
-		$this->mock_account->method( 'is_account_fully_onboarded' )->willReturn( false );
+		$this->mock_account->method( 'is_details_submitted' )->willReturn( false );
 		$this->mock_account->method( 'is_stripe_connected' )->willReturn( false );
 		update_option( 'wcpay_menu_badge_hidden', 'no' );
 		update_option( 'wcpay_activation_timestamp', time() - ( DAY_IN_SECONDS * 2 ) );
@@ -221,15 +215,6 @@ public function test_it_does_not_render_payments_badge_if_activation_date_is_les
 		$this->assertArrayNotHasKey( 'wc-admin&path=/payments/overview', $item_names_by_urls );
 	}
 
-	public function feature_flag_combinations_not_causing_settings_badge_render_provider() {
-		return [
-			[ false, false ],
-			[ false, true ],
-			[ true, false ],
-			[ true, true ],
-		];
-	}
-
 	private function mock_current_user_is_admin() {
 		$admin_user = self::factory()->user->create( [ 'role' => 'administrator' ] );
 		wp_set_current_user( $admin_user );
@@ -514,7 +499,7 @@ public function test_disputes_notification_badge_display() {
 		$this->mock_current_user_is_admin();
 
 		// Make sure we render the menu with submenu items.
-		$this->mock_account->method( 'is_account_fully_onboarded' )->willReturn( true );
+		$this->mock_account->method( 'is_details_submitted' )->willReturn( true );
 		$this->mock_account->method( 'is_stripe_connected' )->willReturn( true );
 		$this->payments_admin->add_payments_menu();
 
@@ -556,7 +541,7 @@ public function test_disputes_notification_badge_no_display() {
 		$this->mock_current_user_is_admin();
 
 		// Make sure we render the menu with submenu items.
-		$this->mock_account->method( 'is_account_fully_onboarded' )->willReturn( true );
+		$this->mock_account->method( 'is_details_submitted' )->willReturn( true );
 		$this->mock_account->method( 'is_stripe_connected' )->willReturn( true );
 		$this->payments_admin->add_payments_menu();
 
@@ -600,7 +585,7 @@ public function test_transactions_notification_badge_display() {
 		$this->mock_current_user_is_admin();
 
 		// Make sure we render the menu with submenu items.
-		$this->mock_account->method( 'is_account_fully_onboarded' )->willReturn( true );
+		$this->mock_account->method( 'is_details_submitted' )->willReturn( true );
 		$this->mock_account->method( 'is_stripe_connected' )->willReturn( true );
 		$this->payments_admin->add_payments_menu();
 
@@ -646,7 +631,7 @@ public function test_transactions_notification_badge_no_display() {
 		$this->mock_current_user_is_admin();
 
 		// Make sure we render the menu with submenu items.
-		$this->mock_account->method( 'is_account_fully_onboarded' )->willReturn( true );
+		$this->mock_account->method( 'is_details_submitted' )->willReturn( true );
 		$this->mock_account->method( 'is_stripe_connected' )->willReturn( true );
 		$this->payments_admin->add_payments_menu();
 

diff --git a/tests/unit/test-class-wc-payments-account.php b/tests/unit/test-class-wc-payments-account.php
@@ -931,53 +931,39 @@ public function test_is_account_rejected_returns_false_on_error() {
 		$this->assertFalse( $this->wcpay_account->is_account_rejected() );
 	}
 
-	public function test_is_account_partially_onboarded_returns_true() {
-		$this->mock_database_cache->expects( $this->exactly( 2 ) )->method( 'get_or_add' )->willReturn(
-			[
-				'account_id'               => 'acc_test',
-				'live_publishable_key'     => 'pk_test_',
-				'test_publishable_key'     => 'pk_live_',
-				'has_pending_requirements' => true,
-				'current_deadline'         => 12345,
-				'is_live'                  => true,
-				'status'                   => 'restricted',
-				'details_submitted'        => false,
-			]
-		);
-
-		$this->assertTrue( $this->wcpay_account->is_account_partially_onboarded() );
-
-	}
-
-	public function test_is_account_partially_onboarded_returns_false() {
-		$this->mock_database_cache->expects( $this->exactly( 2 ) )->method( 'get_or_add' )->willReturn(
-			[
-				'account_id'               => 'acc_test',
-				'live_publishable_key'     => 'pk_test_',
-				'test_publishable_key'     => 'pk_live_',
-				'has_pending_requirements' => true,
-				'current_deadline'         => 12345,
-				'is_live'                  => true,
-				'status'                   => 'restricted',
-				'details_submitted'        => true,
-			]
-		);
-
-		$this->assertFalse( $this->wcpay_account->is_account_partially_onboarded() );
+	/**
+	 * Test the is_details_submitted method.
+	 *
+	 * @param bool $details_submitted Whether details_submitted is true for the account.
+	 *
+	 * @return void
+	 *
+	 * @dataProvider is_details_submitted_provider
+	 */
+	public function test_is_details_submitted( bool $details_submitted ): void {
+		$this->mock_database_cache->expects( $this->once() )
+			->method( 'get_or_add' )
+			->willReturn(
+				[
+					'account_id'               => 'acc_test',
+					'live_publishable_key'     => 'pk_test_',
+					'test_publishable_key'     => 'pk_live_',
+					'has_pending_requirements' => true,
+					'current_deadline'         => 12345,
+					'is_live'                  => true,
+					'status'                   => 'restricted',
+					'details_submitted'        => $details_submitted,
+				]
+			);
 
+		$this->assertEquals( $details_submitted, $this->wcpay_account->is_details_submitted() );
 	}
 
-	public function test_is_account_partially_onboarded_returns_false_when_stripe_not_connected() {
-		$this->mock_empty_cache();
-
-		$this->mock_wcpay_request( Get_Account::class )
-			->expects( $this->once() )
-			->method( 'format_response' )
-			->willThrowException(
-				new API_Exception( 'test', 'wcpay_account_not_found', 401 )
-			);
-
-		$this->assertFalse( $this->wcpay_account->is_account_partially_onboarded() );
+	public function is_details_submitted_provider(): array {
+		return [
+			[ true ],
+			[ false ],
+		];
 	}
 
 	public function test_is_account_partially_onboarded_returns_false_if_account_not_connected() {