Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[API Pull] Add Track Events to the Auth API responses #2452

Merged
merged 8 commits into from
Jul 8, 2024
Merged

[API Pull] Add Track Events to the Auth API responses #2452

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
009ec0d
Add track evens when revoking token
jorgemd24 Jul 4, 2024
5388e06
Add tests when revoking token
jorgemd24 Jul 4, 2024
db27c06
Add track evens when updating Auth status
jorgemd24 Jul 4, 2024
792ef21
Adjust test
jorgemd24 Jul 4, 2024
be2876d
Add blog_id tracking
jorgemd24 Jul 4, 2024
fb8b49c
Add event property directives and descriptions
jorgemd24 Jul 7, 2024
2171ce6
Adjust event properties
jorgemd24 Jul 8, 2024
e7be1ca
Adjust event property
jorgemd24 Jul 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 58 additions & 2 deletions src/API/WP/OAuthService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,16 +159,72 @@ public function revoke_wpcom_api_auth(): string {
$request = $this->container->get( Jetpack::class )->remote_request( $args );

if ( is_wp_error( $request ) ) {

/**
* When the WPCOM token has been revoked with errors.
*
* @event revoke_wpcom_api_authorization
* @property int status The status of the request.
* @property string error The error message.
* @property int|null blog_id The blog ID.
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add here the property directives? Also, I miss the event description.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks I add them here: fb8b49c

do_action(
'woocommerce_gla_track_event',
'revoke_wpcom_api_authorization',
[
'status' => 400,
'error' => $request->get_error_message(),
'blog_id' => Jetpack_Options::get_option( 'id' ),
]
);

throw new Exception( $request->get_error_message(), 400 );
} else {
$body = wp_remote_retrieve_body( $request );
$status = wp_remote_retrieve_response_code( $request );

if ( ! $status || $status !== 200 ) {
$data = json_decode( $body, true );
throw new Exception( $data['message'] ?? 'Error revoking access to WPCOM.', $status );
$data = json_decode( $body, true );
$message = $data['message'] ?? 'Error revoking access to WPCOM.';

/**
*
* When the WPCOM token has been revoked with errors.
*
* @event revoke_wpcom_api_authorization
* @property int status The status of the request.
* @property string error The error message.
* @property int|null blog_id The blog ID.
*/
do_action(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as https://github.com/woocommerce/google-listings-and-ads/pull/2452/files#r1666846943

'woocommerce_gla_track_event',
'revoke_wpcom_api_authorization',
[
'status' => $status,
'error' => $message,
'blog_id' => Jetpack_Options::get_option( 'id' ),
]
);

throw new Exception( $message, $status );
}

/**
* When the WPCOM token has been revoked successfully.
*
* @event revoke_wpcom_api_authorization
* @property string status The status of the request.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing to refactor the status, is also int here.

* @property int|null blog_id The blog ID.
*/
do_action(
'woocommerce_gla_track_event',
'revoke_wpcom_api_authorization',
[
'status' => 200,
'blog_id' => Jetpack_Options::get_option( 'id' ),
]
);

$this->container->get( AccountService::class )->reset_wpcom_api_authorization_data();
return $body;
}
Expand Down
73 changes: 56 additions & 17 deletions src/MerchantCenter/AccountService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
use Automattic\WooCommerce\GoogleListingsAndAds\PluginHelper;
use Automattic\WooCommerce\GoogleListingsAndAds\Vendor\Psr\Container\ContainerInterface;
use Exception;
use Jetpack_Options;

defined( 'ABSPATH' ) || exit;

Expand Down Expand Up @@ -551,28 +552,66 @@ public function reset_wpcom_api_authorization_data(): bool {
* @throws ExceptionWithResponseData If the stored nonce / nonce from query param is not provided, or the nonces mismatch.
*/
public function update_wpcom_api_authorization( string $status, string $nonce ): bool {
$stored_nonce = $this->options->get( OptionsInterface::GOOGLE_WPCOM_AUTH_NONCE );
if ( empty( $stored_nonce ) ) {
throw $this->prepare_exception(
__( 'No stored nonce found in the database, skip updating auth status.', 'google-listings-and-ads' )
);
}
try {
$stored_nonce = $this->options->get( OptionsInterface::GOOGLE_WPCOM_AUTH_NONCE );
if ( empty( $stored_nonce ) ) {
throw $this->prepare_exception(
__( 'No stored nonce found in the database, skip updating auth status.', 'google-listings-and-ads' )
);
}

if ( empty( $nonce ) ) {
throw $this->prepare_exception(
__( 'Nonce is not provided, skip updating auth status.', 'google-listings-and-ads' )
);
}
if ( empty( $nonce ) ) {
throw $this->prepare_exception(
__( 'Nonce is not provided, skip updating auth status.', 'google-listings-and-ads' )
);
}

if ( $stored_nonce !== $nonce ) {
$this->delete_wpcom_api_auth_nonce();
throw $this->prepare_exception(
__( 'Nonces mismatch, skip updating auth status.', 'google-listings-and-ads' )
);
}

if ( $stored_nonce !== $nonce ) {
$this->delete_wpcom_api_auth_nonce();
throw $this->prepare_exception(
__( 'Nonces mismatch, skip updating auth status.', 'google-listings-and-ads' )

/**
* When the WPCOM Authorization status has been updated.
*
* @event update_wpcom_api_authorization
* @property string status The status of the request.
* @property int|null blog_id The blog ID.
*/
do_action(
'woocommerce_gla_track_event',
'update_wpcom_api_authorization',
[
'status' => $status,
'blog_id' => Jetpack_Options::get_option( 'id' ),
]
);
}

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

/**
* When the WPCOM Authorization status has been updated with errors.
*
* @event update_wpcom_api_authorization
* @property string status The status of the request.
* @property int|null blog_id The blog ID.
*/
do_action(
'woocommerce_gla_track_event',
'update_wpcom_api_authorization',
[
'status' => $e->getMessage(),
'blog_id' => Jetpack_Options::get_option( 'id' ),
]
);

throw $e;
}
}

/**
Expand Down
87 changes: 87 additions & 0 deletions tests/Unit/API/WP/OAuthServiceTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,11 @@
use Automattic\WooCommerce\GoogleListingsAndAds\Infrastructure\Deactivateable;
use Automattic\WooCommerce\GoogleListingsAndAds\Proxies\Jetpack;
use Automattic\WooCommerce\GoogleListingsAndAds\MerchantCenter\AccountService;
use Automattic\WooCommerce\GoogleListingsAndAds\Tests\Tools\HelperTrait\TrackingTrait;
use PHPUnit\Framework\MockObject\MockObject;
use WP_Error;
use Exception;
use Jetpack_Options;

defined( 'ABSPATH' ) || exit;

Expand All @@ -25,6 +27,7 @@
*/
class OAuthServiceTest extends UnitTest {

use TrackingTrait;
use UtilitiesTrait;

/**
Expand Down Expand Up @@ -202,4 +205,88 @@ public function test_get_auth_url() {
$parsed_state['store_url']
);
}

public function test_revoke_wpcom_api_auth() {
$this->jp->expects( $this->once() )
->method( 'remote_request' )
->willReturn(
[
'body' => '{"success":true}',
'response' => [ 'code' => 200 ],
]
);

$this->account_service->expects( $this->once() )
->method( 'reset_wpcom_api_authorization_data' );

$this->account_service->expects( $this->once() )
->method( 'reset_wpcom_api_authorization_data' );

$this->expect_track_event(
'revoke_wpcom_api_authorization',
[
'status' => 200,
'blog_id' => Jetpack_Options::get_option( 'id' ),
]
);

$response = $this->service->revoke_wpcom_api_auth();

$this->assertEquals( '{"success":true}', $response );
}

public function test_revoke_wpcom_api_auth_wp_error() {
$this->jp->expects( $this->once() )
->method( 'remote_request' )
->willReturn(
new WP_Error( 'error', 'error message' )
);

$this->expectException( Exception::class );
$this->expectExceptionMessage( 'error message' );
$this->expectExceptionCode( 400 );

$this->account_service->expects( $this->never() )
->method( 'reset_wpcom_api_authorization_data' );

$this->expect_track_event(
'revoke_wpcom_api_authorization',
[
'status' => 400,
'error' => 'error message',
'blog_id' => Jetpack_Options::get_option( 'id' ),
]
);

$this->service->revoke_wpcom_api_auth();
}

public function test_revoke_wpcom_api_auth_status_error() {
$this->jp->expects( $this->once() )
->method( 'remote_request' )
->willReturn(
[
'body' => '{"message":"error message"}',
'response' => [ 'code' => 400 ],
]
);

$this->expectException( Exception::class );
$this->expectExceptionMessage( 'error message' );
$this->expectExceptionCode( 400 );

$this->account_service->expects( $this->never() )
->method( 'reset_wpcom_api_authorization_data' );

$this->expect_track_event(
'revoke_wpcom_api_authorization',
[
'status' => 400,
'error' => 'error message',
'blog_id' => Jetpack_Options::get_option( 'id' ),
]
);

$this->service->revoke_wpcom_api_auth();
}
}
35 changes: 35 additions & 0 deletions tests/Unit/MerchantCenter/AccountServiceTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,10 @@
use Automattic\WooCommerce\GoogleListingsAndAds\Tests\Framework\UnitTest;
use Automattic\WooCommerce\GoogleListingsAndAds\Tests\Tools\HelperTrait\MerchantTrait;
use Automattic\WooCommerce\GoogleListingsAndAds\Vendor\League\Container\Container;
use Automattic\WooCommerce\GoogleListingsAndAds\Tests\Tools\HelperTrait\TrackingTrait;
use Exception;
use PHPUnit\Framework\MockObject\MockObject;
use Jetpack_Options;

defined( 'ABSPATH' ) || exit;

Expand All @@ -37,6 +39,7 @@
*/
class AccountServiceTest extends UnitTest {

use TrackingTrait;
use MerchantTrait;

/** @var MockObject|Ads $ads */
Expand Down Expand Up @@ -877,6 +880,14 @@ public function test_update_wpcom_api_authorization() {
->method( 'delete' )
->with( OptionsInterface::GOOGLE_WPCOM_AUTH_NONCE );

$this->expect_track_event(
'update_wpcom_api_authorization',
[
'status' => 'approved',
'blog_id' => Jetpack_Options::get_option( 'id' ),
]
);

$this->account->update_wpcom_api_authorization( $status, $nonce );
}

Expand All @@ -897,6 +908,14 @@ public function test_update_wpcom_api_authorization_nonce_not_provided() {
$this->expectException( ExceptionWithResponseData::class );
$this->expectExceptionMessage( 'Nonce is not provided, skip updating auth status.' );

$this->expect_track_event(
'update_wpcom_api_authorization',
[
'status' => 'Nonce is not provided, skip updating auth status.',
'blog_id' => Jetpack_Options::get_option( 'id' ),
]
);

$this->account->update_wpcom_api_authorization( $status, $nonce );
}

Expand All @@ -917,6 +936,14 @@ public function test_update_wpcom_api_authorization_stored_nonce_not_in_db() {
$this->expectException( ExceptionWithResponseData::class );
$this->expectExceptionMessage( 'No stored nonce found in the database, skip updating auth status.' );

$this->expect_track_event(
'update_wpcom_api_authorization',
[
'status' => 'No stored nonce found in the database, skip updating auth status.',
'blog_id' => Jetpack_Options::get_option( 'id' ),
]
);

$this->account->update_wpcom_api_authorization( $status, $nonce );
}

Expand All @@ -938,6 +965,14 @@ public function test_update_wpcom_api_authorization_nonces_mismatch() {
$this->expectException( ExceptionWithResponseData::class );
$this->expectExceptionMessage( 'Nonces mismatch, skip updating auth status.' );

$this->expect_track_event(
'update_wpcom_api_authorization',
[
'status' => 'Nonces mismatch, skip updating auth status.',
'blog_id' => Jetpack_Options::get_option( 'id' ),
]
);

$this->account->update_wpcom_api_authorization( $status, $nonce );
}

Expand Down