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.

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

Remove redundant Pinterest API error handlers #1087

Merged
merged 21 commits into from
Dec 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
bb7473c
Removing 403 out of action scheduler failed execution action handler.…
message-dimke Nov 18, 2024
e5f0bf1
Removing action scheduler failed execution action Pinterest API excep…
message-dimke Nov 18, 2024
657fc28
Reverting changes back.
message-dimke Nov 18, 2024
1552a0f
Removing action scheduler failed execution action handler check out o…
message-dimke Nov 18, 2024
491118d
Removing unused use directives.
message-dimke Nov 20, 2024
633e40d
- Refactoring exceptions handling: removing try/catch blocks around m…
message-dimke Nov 20, 2024
eea724b
Fixing phpcs.
message-dimke Nov 20, 2024
1c4d4ca
Fixing phpcs.
message-dimke Nov 20, 2024
ca4d5ba
Adding action to hook in to add admin notice depending on the Pintere…
message-dimke Nov 20, 2024
c21386d
Adding show admin notice action for feed registration action to show …
message-dimke Nov 20, 2024
85aa61c
Adding account disapproved admin notice class.
message-dimke Nov 20, 2024
bfdee85
Fixing phpcs.
message-dimke Nov 20, 2024
b49f8af
- Renaming note class into something more general but still related t…
message-dimke Nov 21, 2024
96d9107
Fixing phpcs
message-dimke Nov 21, 2024
91d825a
Updating the phpDoc class and file descriptions.
message-dimke Nov 21, 2024
4a23384
Making feed registration action not to exit silently if there are any…
message-dimke Nov 21, 2024
8fa36f9
Making commerce integration sync action to fail with exception to rep…
message-dimke Nov 21, 2024
8b87c15
Removing the redundant exception which does not provide any meaningfu…
message-dimke Nov 21, 2024
9806164
Merge branch 'refs/heads/update/actions-to-fail-with-exceptions' into…
message-dimke Nov 21, 2024
68a1731
Merge pull request #1089 from woocommerce/add/notices-for-important-a…
message-dimke Dec 3, 2024
55cdff6
Merge pull request #1088 from woocommerce/update/actions-to-fail-with…
message-dimke Dec 3, 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
21 changes: 0 additions & 21 deletions class-pinterest-for-woocommerce.php
Original file line number Diff line number Diff line change
Expand Up @@ -310,8 +310,6 @@ public function init_plugin() {

add_action( 'pinterest_for_woocommerce_disconnect', array( self::class, 'reset_connection' ) );

add_action( 'action_scheduler_failed_execution', array( self::class, 'action_scheduler_reset_connection' ), 10, 2 );

// Handle the Pinterest verification URL.
add_action( 'parse_request', array( $this, 'verification_request' ) );

Expand Down Expand Up @@ -847,25 +845,6 @@ public static function reset_connection() {
TokenInvalidFailure::possibly_add_note();
}

/**
* Resets the connection from action scheduler.
*
* @since 1.4.4
*
* @param string $action_id The ID of the action.
* @param Exception $e The exception that was thrown.
*
* @return void
* @throws NotesUnavailableException If the notes API is not available.
* @throws Exception If the exception is a 401 error.
*/
public static function action_scheduler_reset_connection( $action_id, $e ) {
if ( in_array( $e->getCode(), array( 401, 403 ) ) ) {
self::reset_connection();
throw $e;
}
}

/**
* Flush data option and remove settings.
*
Expand Down
1 change: 0 additions & 1 deletion src/API/APIV5.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

namespace Automattic\WooCommerce\Pinterest\API;

use Automattic\WooCommerce\Pinterest\Feeds;
use Automattic\WooCommerce\Pinterest\PinterestApiException;
use Exception;

Expand Down
12 changes: 12 additions & 0 deletions src/API/Base.php
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,18 @@ public static function make_request( $endpoint, $method = 'POST', $payload = arr
do_action( 'pinterest_for_woocommerce_disconnect' );
}

/**
* Action used to intercept the Pinterest API exception with code and a message and decide whether or not
* to show an admin notice for the exception.
*
* @since x.x.x
*
* @param int $http_code Pinterest API HTTP response code.
* @param int $pinterest_api_code Pinterest API error code.
* @param string $pinterest_api_message Pinterest API error message.
*/
do_action( 'pinterest_for_woocommerce_show_admin_notice_for_api_exception', $e->getCode(), $e->get_pinterest_code(), $e->getMessage() );

throw $e;
}
}
Expand Down
2 changes: 2 additions & 0 deletions src/CommerceIntegration.php
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ public static function handle_create( $attempt = 0 ): array {
* Handles Commerce Integration partner_metadata updates.
*
* @since 1.4.11
* @throws PinterestApiException Pinterest API exception.
* @return void
*/
public static function handle_sync() {
Expand Down Expand Up @@ -171,6 +172,7 @@ public static function handle_sync() {
),
'error'
);
throw $e;
}
}

Expand Down
44 changes: 29 additions & 15 deletions src/FeedRegistration.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@

namespace Automattic\WooCommerce\Pinterest;

use Automattic\WooCommerce\Pinterest\Notes\AccountFailure;
use Exception;
use Pinterest_For_Woocommerce;
use Throwable;
use Automattic\WooCommerce\Pinterest\Utilities\ProductFeedLogger;
use Automattic\WooCommerce\Pinterest\Exception\PinterestApiLocaleException;
Expand Down Expand Up @@ -66,6 +66,9 @@ public function init() {

// We do not want to disconnect the merchant if the authentication fails, since it running action can not be unscheduled.
add_filter( 'pinterest_for_woocommerce_disconnect_on_authentication_failure', '__return_false' );

add_action( 'pinterest_for_woocommerce_show_admin_notice_for_api_exception', array( self::class, 'maybe_account_failure_notice' ), 10, 3 );
add_action( 'pinterest_for_woocommerce_disconnect', array( AccountFailure::class, 'delete_note' ) );
}

/**
Expand All @@ -79,8 +82,9 @@ public function init() {
*
* @return bool
*
* @throws Exception PHP Exception.
* @throws PinterestApiException Pinterest API Exception.
* @throws PinterestApiLocaleException Pinterest API does not support the locale.
* @throws PinterestApiException Pinterest API exception.
* @throws Throwable Any other exception.
*/
public function handle_feed_registration(): bool {

Expand All @@ -94,26 +98,19 @@ public function handle_feed_registration(): bool {
}

try {
if ( self::register_feed() ) {
return true;
}
throw new Exception( esc_html__( 'Could not register feed.', 'pinterest-for-woocommerce' ) );
return self::register_feed();
} catch ( PinterestApiLocaleException $e ) {
Pinterest_For_Woocommerce()::save_data( 'merchant_locale_not_valid', true );

// translators: %s: Error message.
$error_message = "Could not register feed. Error: {$e->getMessage()}";
$error_message = "Could not register the feed. Error: {$e->getMessage()}";
self::log( $error_message, 'error' );
return false;
throw $e;
} catch ( PinterestApiException $e ) {
throw $e;
} catch ( Throwable $th ) {
if ( method_exists( $th, 'get_pinterest_code' ) && 4163 === $th->get_pinterest_code() ) {
Pinterest_For_Woocommerce()::save_data( 'merchant_connected_diff_platform', true );
}

self::log( $th->getMessage(), 'error' );
return false;
throw $th;
}
}

Expand All @@ -136,7 +133,7 @@ private function clear_merchant_error_code() {
*
* @return boolean
*
* @throws Exception PHP Exception.
* @throws PinterestApiException PHP Exception.
*/
private static function register_feed(): bool {
$feed_id = Feeds::match_local_feed_configuration_to_registered_feeds();
Expand Down Expand Up @@ -230,4 +227,21 @@ public static function deregister() {
Pinterest_For_Woocommerce()::save_data( 'feed_registered', false );
self::cancel_jobs();
}

/**
* Maybe show admin notice about account being disapproved.
*
* @since x.x.x
*
* @param int $http_code Pinterest API HTTP response code.
* @param int $pinterest_api_code Pinterest API error code.
* @param string $pinterest_api_message Pinterest API error message.
*
* @return void
*/
public static function maybe_account_failure_notice( int $http_code, int $pinterest_api_code, string $pinterest_api_message ): void {
if ( 403 === $http_code ) {
AccountFailure::maybe_add_note( $pinterest_api_message );
}
}
}
36 changes: 19 additions & 17 deletions src/Feeds.php
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ class Feeds {
* @since 1.4.0
*
* @return string The Feed ID or an empty string if failed.
* @throws Exception PHP Exception if there is an error creating the feed, and we are throttling the requests.
* @throws PinterestApiException Pinterest API Exception if there is an error creating the feed.
*/
public static function create_feed(): string {
Expand Down Expand Up @@ -141,13 +140,21 @@ public static function create_feed(): string {
'default_availability' => 'IN_STOCK',
);

$cache_key = PINTEREST_FOR_WOOCOMMERCE_PREFIX . '_request_' . md5( wp_json_encode( $data ) . (string) $ad_account_id );
$cache = get_transient( $cache_key );

if ( false !== $cache ) {
throw new Exception(
esc_html__( 'There was a previous error trying to create a feed.', 'pinterest-for-woocommerce' ),
(int) $cache
$cache_key = PINTEREST_FOR_WOOCOMMERCE_PREFIX . '_request_' . md5( wp_json_encode( $data ) . (string) $ad_account_id );
$cached_error = get_transient( $cache_key );

if ( false !== $cached_error ) {
// Faking Pinterest API response to 425 Too early.
throw new PinterestApiException(
sprintf(
/* translators: Pinterest API error code and message. 1: Cached error string. */
esc_html__(
'Previous request for the same action failed due to: %1$s. Delaying the next call to prevent repeating errors.',
'pinterest-for-woocommerce'
),
esc_html( $cached_error )
),
425
);
}

Expand All @@ -160,8 +167,9 @@ public static function create_feed(): string {
try {
$feed = APIV5::create_feed( $data, $ad_account_id );
} catch ( PinterestApiException $e ) {
$delay = Pinterest_For_Woocommerce()::get_data( 'create_feed_delay' ) ?? MINUTE_IN_SECONDS;
set_transient( $cache_key, $e->getCode(), $delay );
$delay = Pinterest_For_Woocommerce()::get_data( 'create_feed_delay' ) ?? MINUTE_IN_SECONDS;
$message = sprintf( '%1$s - %2$s', esc_html( $e->get_pinterest_code() ), esc_html( $e->getMessage() ) );
set_transient( $cache_key, $message, $delay );
// Double the delay.
Pinterest_For_Woocommerce()::save_data(
'create_feed_delay',
Expand All @@ -172,11 +180,7 @@ public static function create_feed(): string {

static::invalidate_feeds_cache();

try {
$feed_id = static::match_local_feed_configuration_to_registered_feeds( array( $feed ) );
} catch ( Throwable $th ) {
$feed_id = '';
}
$feed_id = static::match_local_feed_configuration_to_registered_feeds( array( $feed ) );

// Clean the cached delay.
Pinterest_For_Woocommerce()::save_data( 'create_feed_delay', false );
Expand Down Expand Up @@ -278,8 +282,6 @@ public static function invalidate_feeds_cache() {
*
* @param array $feeds The list of feeds to check against. If not set, the list will be fetched from the API.
* @return string Returns the ID of the feed if properly registered or an empty string otherwise.
*
* @throws PinterestApiLocaleException No valid locale found to check for the registered feed.
*/
public static function match_local_feed_configuration_to_registered_feeds( array $feeds = array() ): string {
if ( empty( $feeds ) ) {
Expand Down
82 changes: 82 additions & 0 deletions src/Notes/AccountFailure.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
<?php
/**
* Adds an error note to display Pinterest API account status failure responses.
*
* @since x.x.x
* @package Automattic\WooCommerce\Pinterest\Notes
*/

namespace Automattic\WooCommerce\Pinterest\Notes;

defined( 'ABSPATH' ) || exit;

use Automattic\WooCommerce\Admin\Notes\Note;
use Automattic\WooCommerce\Admin\Notes\Notes;
use Automattic\WooCommerce\Admin\Notes\NotesUnavailableException;
use Automattic\WooCommerce\Admin\Notes\NoteTraits;

/**
* Account Failure admin notice.
*
* @since x.x.x
*/
class AccountFailure {
/**
* Note traits.
*/
use NoteTraits;

/**
* Name of the note for use in the database.
*/
const NOTE_NAME = 'pinterest-for-woocommerce-account-failure';

/**
* Get the note.
*
* @since x.x.x
* @param string $message Pinterest API error message.
* @return Note
*/
public static function get_note( string $message ) {
$additional_data = array(
'role' => 'administrator',
);

$note = new Note();
$note->set_title( __( 'Pinterest For WooCommerce action required.', 'pinterest-for-woocommerce' ) );
$note->set_content( esc_html( $message ) );
$note->set_content_data( (object) $additional_data );
$note->set_type( Note::E_WC_ADMIN_NOTE_ERROR );
$note->set_name( self::NOTE_NAME );
$note->set_source( 'woocommerce-admin' );
return $note;
}

/**
* Used to add an account failure note if the one does not exist.
*
* @since x.x.x
* @param string $message Pinterest API error message.
* @return void
* @throws NotesUnavailableException An exception when notes are unavailable.
*/
public static function maybe_add_note( string $message ): void {
if ( self::note_exists() ) {
return;
}

$note = self::get_note( $message );
$note->save();
}

/**
* Delete the note.
*
* @since x.x.x
* @return void
*/
public static function delete_note() {
Notes::delete_notes_with_name( self::NOTE_NAME );
}
}
6 changes: 3 additions & 3 deletions src/PinterestApiException.php
Original file line number Diff line number Diff line change
Expand Up @@ -88,13 +88,13 @@ class PinterestApiException extends \Exception {
* Pinterest_API_Exception constructor.
*
* @param string|array $error The error message or an array containing the error message + additional data.
* @param int $response_code The HTTP response code of the API call. e.g. 200, 401, 403, 404, etc.
* @param int|array $response_code The error code of the API response body or the whole response body as array.
*/
public function __construct( $error, $response_code ) {
$message = $error['message'] ?? $error;
$this->pinterest_code = (int) $error['response_body']['code'] ?? 0;
$this->pinterest_code = (int) ( $error['response_body']['code'] ?? $response_code ) ?? 0;

parent::__construct( $message ?? $error, $response_code );
parent::__construct( $message, $response_code );
}

/**
Expand Down
7 changes: 0 additions & 7 deletions tests/Unit/PinterestForWoocommerceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -61,13 +61,6 @@ public function test_disconnect_events_added_with_plugin() {
[ Pinterest_For_Woocommerce::class, 'reset_connection' ]
)
);
$this->assertEquals(
10,
has_action(
'action_scheduler_failed_execution',
[ Pinterest_For_Woocommerce::class, 'action_scheduler_reset_connection' ]
)
);
}

/**
Expand Down
Loading