Skip to content

Commit

Permalink
Merge pull request #2547 from woocommerce/tweak/check-wpcom-api-auth
Browse files Browse the repository at this point in the history
Use remote-site-status to check the WPCOM Auth status
  • Loading branch information
puntope committed Sep 5, 2024
2 parents 12c09f8 + fd5d8aa commit 7210b14
Show file tree
Hide file tree
Showing 8 changed files with 164 additions and 9 deletions.
17 changes: 14 additions & 3 deletions src/API/WP/NotificationsService.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

use Automattic\Jetpack\Connection\Client;
use Automattic\WooCommerce\GoogleListingsAndAds\Infrastructure\Service;
use Automattic\WooCommerce\GoogleListingsAndAds\MerchantCenter\AccountService;
use Automattic\WooCommerce\GoogleListingsAndAds\MerchantCenter\MerchantCenterService;
use Automattic\WooCommerce\GoogleListingsAndAds\Options\OptionsAwareInterface;
use Automattic\WooCommerce\GoogleListingsAndAds\Options\OptionsAwareTrait;
Expand Down Expand Up @@ -59,15 +60,24 @@ class NotificationsService implements Service, OptionsAwareInterface {
*/
public MerchantCenterService $merchant_center;

/**
* The AccountService service
*
* @var AccountService $account_service
*/
public AccountService $account_service;


/**
* Class constructor
*
* @param MerchantCenterService $merchant_center
* @param AccountService $account_service
*/
public function __construct( MerchantCenterService $merchant_center ) {
public function __construct( MerchantCenterService $merchant_center, AccountService $account_service ) {
$blog_id = Jetpack_Options::get_option( 'id' );
$this->merchant_center = $merchant_center;
$this->account_service = $account_service;
$this->notification_url = "https://public-api.wordpress.com/wpcom/v2/sites/{$blog_id}/partners/google/notifications";
}

Expand Down Expand Up @@ -161,10 +171,11 @@ public function get_notification_url(): string {
* If the Notifications are ready
* This happens when the WPCOM API is Authorized and the feature is enabled.
*
* @param bool $with_health_check If true. Performs a remote request to WPCOM API to get the status.
* @return bool
*/
public function is_ready(): bool {
return $this->options->is_wpcom_api_authorized() && $this->is_enabled() && $this->merchant_center->is_ready_for_syncing();
public function is_ready( bool $with_health_check = true ): bool {
return $this->options->is_wpcom_api_authorized() && $this->is_enabled() && $this->merchant_center->is_ready_for_syncing() && ( $with_health_check === false || $this->account_service->is_wpcom_api_status_healthy() );
}

/**
Expand Down
5 changes: 3 additions & 2 deletions src/ConnectionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\DeleteAllProducts;
use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\UpdateAllProducts;
use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\UpdateProducts;
use Automattic\WooCommerce\GoogleListingsAndAds\MerchantCenter\AccountService;
use Automattic\WooCommerce\GoogleListingsAndAds\MerchantCenter\MerchantCenterService;
use Automattic\WooCommerce\GoogleListingsAndAds\MerchantCenter\MerchantStatuses;
use Automattic\WooCommerce\GoogleListingsAndAds\Options\AdsAccountState;
Expand Down Expand Up @@ -652,7 +653,7 @@ protected function render_admin_page() {
<?php
$options = $this->container->get( OptionsInterface::class );
$wp_api_status = $options->get( OptionsInterface::WPCOM_REST_API_STATUS );
$notification_service = new NotificationsService( $this->container->get( MerchantCenterService::class ) );
$notification_service = new NotificationsService( $this->container->get( MerchantCenterService::class ), $this->container->get( AccountService::class ) );
$notification_service->set_options_object( $options );
?>
<h2 class="title">Partner API Pull Integration</h2>
Expand Down Expand Up @@ -865,7 +866,7 @@ protected function handle_actions() {
$mc = $this->container->get( MerchantCenterService::class );
/** @var OptionsInterface $options */
$options = $this->container->get( OptionsInterface::class );
$service = new NotificationsService( $mc );
$service = new NotificationsService( $mc, $this->container->get( AccountService::class ) );
$service->set_options_object( $options );

if ( $service->notify( $topic, $item ) ) {
Expand Down
3 changes: 2 additions & 1 deletion src/Internal/DependencyManagement/CoreServiceProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
use Automattic\WooCommerce\GoogleListingsAndAds\Menu\Settings;
use Automattic\WooCommerce\GoogleListingsAndAds\Menu\SetupAds;
use Automattic\WooCommerce\GoogleListingsAndAds\Menu\SetupMerchantCenter;
use Automattic\WooCommerce\GoogleListingsAndAds\MerchantCenter\AccountService;
use Automattic\WooCommerce\GoogleListingsAndAds\MerchantCenter\AccountService as MerchantAccountService;
use Automattic\WooCommerce\GoogleListingsAndAds\MerchantCenter\ContactInformation;
use Automattic\WooCommerce\GoogleListingsAndAds\MerchantCenter\MerchantCenterAwareInterface;
Expand Down Expand Up @@ -254,7 +255,7 @@ public function register(): void {
$this->share_with_tags( MerchantCenterService::class );

// Set up Notifications service.
$this->share_with_tags( NotificationsService::class, MerchantCenterService::class );
$this->share_with_tags( NotificationsService::class, MerchantCenterService::class, AccountService::class );

// Set up OAuthService service.
$this->share_with_tags( OAuthService::class );
Expand Down
51 changes: 51 additions & 0 deletions src/MerchantCenter/AccountService.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@

namespace Automattic\WooCommerce\GoogleListingsAndAds\MerchantCenter;

use Automattic\Jetpack\Connection\Client;
use Automattic\WooCommerce\GoogleListingsAndAds\API\Google\Ads;
use Automattic\WooCommerce\GoogleListingsAndAds\API\Google\Merchant;
use Automattic\WooCommerce\GoogleListingsAndAds\API\Google\Middleware;
use Automattic\WooCommerce\GoogleListingsAndAds\API\Google\SiteVerification;
use Automattic\WooCommerce\GoogleListingsAndAds\API\WP\NotificationsService;
use Automattic\WooCommerce\GoogleListingsAndAds\API\WP\OAuthService;
use Automattic\WooCommerce\GoogleListingsAndAds\DB\Table\MerchantIssueTable;
use Automattic\WooCommerce\GoogleListingsAndAds\DB\Table\ShippingRateTable;
use Automattic\WooCommerce\GoogleListingsAndAds\DB\Table\ShippingTimeTable;
Expand Down Expand Up @@ -227,6 +229,12 @@ public function get_connected_status(): array {
$id = $this->options->get_merchant_id();
$wpcom_rest_api_status = $this->options->get( OptionsInterface::WPCOM_REST_API_STATUS );

// If token is revoked outside the extension. Set the status as error to force the merchant to grant access again.
if ( $wpcom_rest_api_status === 'approved' && ! $this->is_wpcom_api_status_healthy() ) {
$wpcom_rest_api_status = OAuthService::STATUS_ERROR;
$this->options->update( OptionsInterface::WPCOM_REST_API_STATUS, $wpcom_rest_api_status );
}

$status = [
'id' => $id,
'status' => $id ? 'connected' : 'disconnected',
Expand Down Expand Up @@ -539,6 +547,7 @@ private function prepare_exception( string $message, array $data = [], int $code
*/
public function reset_wpcom_api_authorization_data(): bool {
$this->delete_wpcom_api_auth_nonce();
$this->delete_wpcom_api_status_transient();
return $this->options->delete( OptionsInterface::WPCOM_REST_API_STATUS );
}

Expand Down Expand Up @@ -592,6 +601,7 @@ public function update_wpcom_api_authorization( string $status, string $nonce ):
]
);

$this->delete_wpcom_api_status_transient();
return $this->options->update( OptionsInterface::WPCOM_REST_API_STATUS, $status );
} catch ( ExceptionWithResponseData $e ) {

Expand Down Expand Up @@ -623,4 +633,45 @@ public function update_wpcom_api_authorization( string $status, string $nonce ):
public function delete_wpcom_api_auth_nonce(): bool {
return $this->options->delete( OptionsInterface::GOOGLE_WPCOM_AUTH_NONCE );
}

/**
* Deletes the transient storing the WPCOM Status data.
*/
public function delete_wpcom_api_status_transient(): void {
$transients = $this->container->get( TransientsInterface::class );
$transients->delete( TransientsInterface::WPCOM_API_STATUS );
}

/**
* Check if the WPCOM API Status is healthy by doing a request to /wc/partners/google/remote-site-status endpoint in WPCOM.
*
* @return bool True when the status is healthy, false otherwise.
*/
public function is_wpcom_api_status_healthy() {
/** @var TransientsInterface $transients */
$transients = $this->container->get( TransientsInterface::class );
$status = $transients->get( TransientsInterface::WPCOM_API_STATUS );

if ( ! $status ) {

$integration_status_args = [
'method' => 'GET',
'timeout' => 30,
'url' => 'https://public-api.wordpress.com/wpcom/v2/sites/' . Jetpack_Options::get_option( 'id' ) . '/wc/partners/google/remote-site-status',
'user_id' => get_current_user_id(),
];

$integration_remote_request_response = Client::remote_request( $integration_status_args, null );

if ( is_wp_error( $integration_remote_request_response ) ) {
$status = [ 'is_healthy' => false ];
} else {
$status = json_decode( wp_remote_retrieve_body( $integration_remote_request_response ), true ) ?? [ 'is_healthy' => false ];
}

$transients->set( TransientsInterface::WPCOM_API_STATUS, $status, MINUTE_IN_SECONDS * 30 );
}

return isset( $status['is_healthy'] ) && $status['is_healthy'] && $status['is_wc_rest_api_healthy'] && $status['is_partner_token_healthy'];
}
}
2 changes: 2 additions & 0 deletions src/Options/TransientsInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ interface TransientsInterface {
public const MC_IS_SUBACCOUNT = 'mc_is_subaccount';
public const MC_STATUSES = 'mc_statuses';
public const URL_MATCHES = 'url_matches';
public const WPCOM_API_STATUS = 'wpcom_api_status';

public const VALID_OPTIONS = [
self::ADS_CAMPAIGN_COUNT => true,
Expand All @@ -26,6 +27,7 @@ interface TransientsInterface {
self::MC_IS_SUBACCOUNT => true,
self::MC_STATUSES => true,
self::URL_MATCHES => true,
self::WPCOM_API_STATUS => true,
];

/**
Expand Down
2 changes: 1 addition & 1 deletion src/Settings/SyncerHooks.php
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ public function __construct( JobRepository $job_repository, NotificationsService
* Register the service.
*/
public function register(): void {
if ( ! $this->notifications_service->is_ready() ) {
if ( ! $this->notifications_service->is_ready( false ) ) {
return;
}

Expand Down
35 changes: 33 additions & 2 deletions tests/Unit/API/WP/NotificationsServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

use Automattic\WooCommerce\Admin\RemoteInboxNotifications\TransformerService;
use Automattic\WooCommerce\GoogleListingsAndAds\API\WP\NotificationsService;
use Automattic\WooCommerce\GoogleListingsAndAds\MerchantCenter\AccountService;
use Automattic\WooCommerce\GoogleListingsAndAds\MerchantCenter\MerchantCenterService;
use Automattic\WooCommerce\GoogleListingsAndAds\Options\OptionsInterface;
use Automattic\WooCommerce\GoogleListingsAndAds\Tests\Framework\UnitTest;
Expand All @@ -30,6 +31,11 @@ class NotificationsServiceTest extends UnitTest {
*/
public $merchant_center;

/**
* @var MockObject|AccountService
*/
public $account;

public const DUMMY_BLOG_ID = '123';

// List of Topics to be used.
Expand Down Expand Up @@ -207,22 +213,47 @@ public function test_notify_show_error_when_disabled() {
remove_filter( 'woocommerce_gla_notifications_enabled', '__return_false' );
}

/**
* Test notify() function logs an error when WPCOM Auth is not healthy
*/
public function test_notify_show_error_when_wpcom_not_healthy() {
$this->service = $this->get_mock( true, true, false );
$this->service->expects( $this->never() )->method( 'do_request' );
$this->assertFalse( $this->service->notify( 'product.create', 1 ) );
$this->assertEquals( did_action( 'woocommerce_gla_error' ), 1 );
}

public function test_is_ready_not_calling_status_api_if_with_health_check_is_false() {
$this->service = $this->get_mock( true, true, false );
$this->account->expects( $this->never() )->method( 'is_wpcom_api_status_healthy' );
$this->assertTrue( $this->service->is_ready( false ) );
}

public function test_is_ready_calling_status_api_if_with_health_check_is_true() {
$this->service = $this->get_mock();
$this->account->expects( $this->once() )->method( 'is_wpcom_api_status_healthy' );
$this->assertTrue( $this->service->is_ready() );
}

/**
* Mocks the service
*
* @param bool $mc_ready
* @param bool $wpcom_authorized
* @param bool $is_wpcom_api_status_healthy
* @return TransformerService
*/
public function get_mock( $mc_ready = true, $wpcom_authorized = true ) {
public function get_mock( $mc_ready = true, $wpcom_authorized = true, $is_wpcom_api_status_healthy = true ) {
$this->merchant_center = $this->createMock( MerchantCenterService::class );
$this->merchant_center->method( 'is_ready_for_syncing' )->willReturn( $mc_ready );
$this->account = $this->createMock( AccountService::class );
$this->account->method( 'is_wpcom_api_status_healthy' )->willReturn( $is_wpcom_api_status_healthy );
$this->options = $this->createMock( OptionsInterface::class );
$this->options->method( 'is_wpcom_api_authorized' )->willReturn( $wpcom_authorized );

/** @var NotificationsService $mock */
$mock = $this->getMockBuilder( NotificationsService::class )
->setConstructorArgs( [ $this->merchant_center ] )
->setConstructorArgs( [ $this->merchant_center, $this->account ] )
->onlyMethods( [ 'do_request' ] )
->getMock();
$mock->set_options_object( $this->options );
Expand Down
58 changes: 58 additions & 0 deletions tests/Unit/MerchantCenter/AccountServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -762,6 +762,17 @@ public function test_get_connected_status() {
->method( 'is_enabled' )
->willReturn( true );

$this->transients->expects( $this->exactly( 1 ) )
->method( 'get' )
->with( TransientsInterface::WPCOM_API_STATUS )
->willReturn(
[
'is_healthy' => true,
'is_wc_rest_api_healthy' => true,
'is_partner_token_healthy' => true,
]
);

$this->options->method( 'get' )
->with( OptionsInterface::WPCOM_REST_API_STATUS )
->willReturn( 'approved' );
Expand All @@ -786,6 +797,17 @@ public function test_get_connected_status_when_notifications_disabled() {
->method( 'is_enabled' )
->willReturn( false );

$this->transients->expects( $this->exactly( 1 ) )
->method( 'get' )
->with( TransientsInterface::WPCOM_API_STATUS )
->willReturn(
[
'is_healthy' => true,
'is_wc_rest_api_healthy' => true,
'is_partner_token_healthy' => true,
]
);

$this->options->method( 'get' )
->with( OptionsInterface::WPCOM_REST_API_STATUS )
->willReturn( 'approved' );
Expand Down Expand Up @@ -826,6 +848,42 @@ public function test_get_connected_status_incomplete() {
);
}

public function test_get_connected_status_not_healthy() {
$this->options->expects( $this->once() )
->method( 'get_merchant_id' )
->willReturn( self::TEST_ACCOUNT_ID );

$this->notifications_service->expects( $this->once() )
->method( 'is_enabled' )
->willReturn( true );

$this->transients->expects( $this->exactly( 1 ) )
->method( 'get' )
->with( TransientsInterface::WPCOM_API_STATUS )
->willReturn(
[
'is_healthy' => true,
'is_wc_rest_api_healthy' => true,
'is_partner_token_healthy' => false,
]
);

$this->options->method( 'get' )
->with( OptionsInterface::WPCOM_REST_API_STATUS )
->willReturn( 'approved' );

$this->assertEquals(
[
'id' => self::TEST_ACCOUNT_ID,
'status' => 'connected',
'notification_service_enabled' => true,
'wpcom_rest_api_status' => 'error',
],
$this->account->get_connected_status()
);
}


public function test_get_setup_status() {
$this->mc_service->expects( $this->once() )
->method( 'get_setup_status' )
Expand Down

0 comments on commit 7210b14

Please sign in to comment.