+ Send partner notification request +
- Send partner notification request -
+ + Product/Coupon/Shipping ID + + + + Partner APP Client ID + + + + Partner APP Secret ID + + + + Topic + + >product/create + >product/delete + >product/update + + + Send Notification +
- Product/Coupon/Shipping ID + Product/Coupon ID @@ -650,6 +650,7 @@ protected function render_admin_page() { >coupon.create >coupon.delete >coupon.update + >shipping.update Send Notification From 0bdde07d8d286a0851ba07f812ac4da26e8e122d Mon Sep 17 00:00:00 2001 From: Miguel Perez Pellicer <5908855+puntope@users.noreply.github.com> Date: Thu, 8 Feb 2024 17:02:14 -0300 Subject: [PATCH 014/262] PHPCS --- src/Jobs/Notifications/ShippingNotificationJob.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Jobs/Notifications/ShippingNotificationJob.php b/src/Jobs/Notifications/ShippingNotificationJob.php index ee8a145e3e..8ddecd3c05 100644 --- a/src/Jobs/Notifications/ShippingNotificationJob.php +++ b/src/Jobs/Notifications/ShippingNotificationJob.php @@ -40,7 +40,7 @@ class ShippingNotificationJob extends AbstractActionSchedulerJob implements JobI public function __construct( ActionSchedulerInterface $action_scheduler, ActionSchedulerJobMonitor $monitor, - NotificationsService $notifications_service, + NotificationsService $notifications_service ) { $this->notifications_service = $notifications_service; $this->topic = NotificationsService::TOPIC_SHIPPING_UPDATED; From 72f00f8af290befd45248dbfa5e4a087ef2d4de1 Mon Sep 17 00:00:00 2001 From: Miguel Perez Pellicer <5908855+puntope@users.noreply.github.com> Date: Mon, 12 Feb 2024 10:51:10 -0300 Subject: [PATCH 015/262] Fix typos --- src/Google/NotificationsService.php | 2 +- src/Shipping/SyncerHooks.php | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Google/NotificationsService.php b/src/Google/NotificationsService.php index b1f5a17213..55c6d70264 100644 --- a/src/Google/NotificationsService.php +++ b/src/Google/NotificationsService.php @@ -48,7 +48,7 @@ public function __construct() { * https://public-api.wordpress.com/wpcom/v2/sites/{site}/partners/google/notifications * * @param string $topic The topic to use in the notification. - * @param int|null $item_id Tme item ID to notify. It can be null for topics that doesn't need Item ID + * @param int|null $item_id The item ID to notify. It can be null for topics that doesn't need Item ID * @return bool True is the notification is successful. False otherwise. */ public function notify( string $topic, $item_id = null ): bool { diff --git a/src/Shipping/SyncerHooks.php b/src/Shipping/SyncerHooks.php index 9263880225..d830586764 100644 --- a/src/Shipping/SyncerHooks.php +++ b/src/Shipping/SyncerHooks.php @@ -50,7 +50,7 @@ class SyncerHooks implements Service, Registerable { /** * @var NotificationsService $notifications_service */ - protected $notifiations_service; + protected $notifications_service; /** * @var ShippingNotificationJob $shipping_notification_job @@ -70,7 +70,7 @@ public function __construct( MerchantCenterService $merchant_center, GoogleSetti $this->merchant_center = $merchant_center; $this->update_shipping_job = $job_repository->get( UpdateShippingSettings::class ); $this->shipping_notification_job = $job_repository->get( ShippingNotificationJob::class ); - $this->notifiations_service = $notifications_service; + $this->notifications_service = $notifications_service; } /** @@ -144,7 +144,7 @@ protected function handle_update_shipping_settings() { return; } - if ( $this->notifiations_service->is_enabled() ) { + if ( $this->notifications_service->is_enabled() ) { $this->shipping_notification_job->schedule(); } else { $this->update_shipping_job->schedule(); From 34e13297686e07a5d33a1d21f5d4618eb0e49a4f Mon Sep 17 00:00:00 2001 From: Miguel Perez Pellicer <5908855+puntope@users.noreply.github.com> Date: Mon, 12 Feb 2024 11:11:52 -0300 Subject: [PATCH 016/262] Check notification status when scheduling --- .../Notifications/ProductNotificationJob.php | 31 ++++++++++++------- .../Notifications/ShippingNotificationJob.php | 27 ++++++++++------ 2 files changed, 38 insertions(+), 20 deletions(-) diff --git a/src/Jobs/Notifications/ProductNotificationJob.php b/src/Jobs/Notifications/ProductNotificationJob.php index 1e3cf2b94a..ca421cbedc 100644 --- a/src/Jobs/Notifications/ProductNotificationJob.php +++ b/src/Jobs/Notifications/ProductNotificationJob.php @@ -86,17 +86,7 @@ protected function process_items( array $args ): void { * @param array $args */ public function schedule( array $args = [] ): void { - /** - * Allow users to disable the notification job schedule. - * - * @since x.x.x - * - * @param bool $value The current filter value. By default, it is the result of `$this->can_schedule` function. - * @param array $args The arguments for the schedule function with the item id and the topic. - */ - $can_schedule = apply_filters( 'woocommerce_gla_product_notification_job_can_schedule', $this->can_schedule( [ $args ] ), $args ); - - if ( $can_schedule ) { + if ( $this->can_schedule( $args ) ) { $this->action_scheduler->schedule_immediate( $this->get_process_item_hook(), [ $args ] @@ -150,4 +140,23 @@ protected function can_process( int $product_id, string $topic ): bool { return $this->product_helper->should_trigger_update_notification( $product ); } } + + /** + * Can the job be scheduled. + * + * @param array|null $args + * + * @return bool Returns true if the job can be scheduled. + */ + public function can_schedule( $args = [] ): bool { + /** + * Allow users to disable the notification job schedule. + * + * @since x.x.x + * + * @param bool $value The current filter value. By default, it is the result of `$this->can_schedule` function. + * @param array $args The arguments for the schedule function with the item id and the topic. + */ + return apply_filters( 'woocommerce_gla_product_notification_job_can_schedule', $this->notifications_service->is_enabled() && parent::can_schedule( $args ), $args ); + } } diff --git a/src/Jobs/Notifications/ShippingNotificationJob.php b/src/Jobs/Notifications/ShippingNotificationJob.php index 8ddecd3c05..716e2c77c9 100644 --- a/src/Jobs/Notifications/ShippingNotificationJob.php +++ b/src/Jobs/Notifications/ShippingNotificationJob.php @@ -72,21 +72,30 @@ protected function process_items( array $args ): void { * @param array $args */ public function schedule( array $args = [] ): void { + if ( $this->can_schedule( $args ) ) { + $this->action_scheduler->schedule_immediate( + $this->get_process_item_hook(), + [ $args ] + ); + } + } + + /** + * Can the job be scheduled. + * + * @param array|null $args + * + * @return bool Returns true if the job can be scheduled. + */ + public function can_schedule( $args = [] ): bool { /** * Allow users to disable the notification job schedule. * * @since x.x.x * * @param bool $value The current filter value. By default, it is the result of `$this->can_schedule` function. - * @param array $args The arguments for the schedule function. + * @param array $args The arguments for the schedule function with the item id and the topic. */ - $can_schedule = apply_filters( 'woocommerce_gla_shipping_notification_job_can_schedule', $this->can_schedule( [ $args ] ), $args ); - - if ( $can_schedule ) { - $this->action_scheduler->schedule_immediate( - $this->get_process_item_hook(), - [ $args ] - ); - } + return apply_filters( 'woocommerce_gla_shipping_notification_job_can_schedule', $this->notifications_service->is_enabled() && parent::can_schedule( $args ), $args ); } } From a98ca65d0fd1b65d8e49821fe9cb168fb00c9102 Mon Sep 17 00:00:00 2001 From: Miguel Perez Pellicer <5908855+puntope@users.noreply.github.com> Date: Mon, 12 Feb 2024 18:13:36 -0300 Subject: [PATCH 017/262] Add Tests --- .../Unit/Jobs/ProductNotificationJobTest.php | 18 +-- .../Unit/Jobs/ShippingNotificationJobTest.php | 107 ++++++++++++++++++ 2 files changed, 117 insertions(+), 8 deletions(-) create mode 100644 tests/Unit/Jobs/ShippingNotificationJobTest.php diff --git a/tests/Unit/Jobs/ProductNotificationJobTest.php b/tests/Unit/Jobs/ProductNotificationJobTest.php index 4315b86c70..5b985ff50b 100644 --- a/tests/Unit/Jobs/ProductNotificationJobTest.php +++ b/tests/Unit/Jobs/ProductNotificationJobTest.php @@ -68,9 +68,11 @@ public function test_schedule_schedules_immediate_job() { $topic = 'product.create'; $id = 1; + $this->notification_service->expects( $this->once() )->method( 'is_enabled' )->willReturn( true ); + $this->action_scheduler->expects( $this->once() ) ->method( 'has_scheduled_action' ) - ->with( self::PROCESS_ITEM_HOOK, [ [ $id, $topic ] ] ) + ->with( self::PROCESS_ITEM_HOOK, [ $id, $topic ] ) ->willReturn( false ); $this->action_scheduler->expects( $this->once() ) @@ -87,9 +89,11 @@ public function test_schedule_doesnt_schedules_immediate_job_if_already_schedule $id = 1; $topic = 'product.create'; + $this->notification_service->expects( $this->once() )->method( 'is_enabled' )->willReturn( true ); + $this->action_scheduler->expects( $this->once() ) ->method( 'has_scheduled_action' ) - ->with( self::PROCESS_ITEM_HOOK, [ [ $id, $topic ] ] ) + ->with( self::PROCESS_ITEM_HOOK, [ $id, $topic ] ) ->willReturn( true ); $this->action_scheduler->expects( $this->never() )->method( 'schedule_immediate' ); @@ -97,16 +101,14 @@ public function test_schedule_doesnt_schedules_immediate_job_if_already_schedule $this->job->schedule( [ $id, $topic ] ); } - public function test_schedule_doesnt_schedules_immediate_job_if_filtered() { + public function test_schedule_doesnt_schedules_immediate_job_if_not_enabled() { $id = 1; $topic = 'product.create'; - add_filter( 'woocommerce_gla_product_notification_job_can_schedule', '__return_false' ); + $this->notification_service->expects( $this->once() )->method( 'is_enabled' )->willReturn( false ); - $this->action_scheduler->expects( $this->once() ) - ->method( 'has_scheduled_action' ) - ->with( self::PROCESS_ITEM_HOOK, [ [ $id, $topic ] ] ) - ->willReturn( false ); + $this->action_scheduler->expects( $this->never() ) + ->method( 'has_scheduled_action' ); $this->action_scheduler->expects( $this->never() )->method( 'schedule_immediate' ); diff --git a/tests/Unit/Jobs/ShippingNotificationJobTest.php b/tests/Unit/Jobs/ShippingNotificationJobTest.php new file mode 100644 index 0000000000..8cc10c43d2 --- /dev/null +++ b/tests/Unit/Jobs/ShippingNotificationJobTest.php @@ -0,0 +1,107 @@ +action_scheduler = $this->createMock( ActionScheduler::class ); + $this->monitor = $this->createMock( ActionSchedulerJobMonitor::class ); + $this->notification_service = $this->createMock( NotificationsService::class ); + $this->job = new ShippingNotificationJob( + $this->action_scheduler, + $this->monitor, + $this->notification_service + ); + + $this->job->init(); + } + + public function test_job_name() { + $this->assertEquals( self::JOB_NAME, $this->job->get_name() ); + } + + public function test_schedule_schedules_immediate_job() { + $this->notification_service->expects( $this->once() )->method( 'is_enabled' )->willReturn( true ); + + $this->action_scheduler->expects( $this->once() ) + ->method( 'has_scheduled_action' ) + ->with( self::PROCESS_ITEM_HOOK, [] ) + ->willReturn( false ); + + $this->action_scheduler->expects( $this->once() ) + ->method( 'schedule_immediate' ) + ->with( self::PROCESS_ITEM_HOOK ); + + $this->job->schedule(); + } + + public function test_schedule_doesnt_schedules_immediate_job_if_already_scheduled() { + $this->notification_service->expects( $this->once() )->method( 'is_enabled' )->willReturn( true ); + + $this->action_scheduler->expects( $this->once() ) + ->method( 'has_scheduled_action' ) + ->with( self::PROCESS_ITEM_HOOK, [] ) + ->willReturn( true ); + + $this->action_scheduler->expects( $this->never() )->method( 'schedule_immediate' ); + + $this->job->schedule(); + } + + public function test_schedule_doesnt_schedules_immediate_job_if_not_enabled() { + $this->notification_service->expects( $this->once() )->method( 'is_enabled' )->willReturn( false ); + + $this->action_scheduler->expects( $this->never() ) + ->method( 'has_scheduled_action' ); + + $this->action_scheduler->expects( $this->never() )->method( 'schedule_immediate' ); + + $this->job->schedule(); + } + + public function test_process_items_calls_notify() { + $this->notification_service->expects( $this->once() ) + ->method( 'notify' ) + ->with( NotificationsService::TOPIC_SHIPPING_UPDATED ) + ->willReturn( true ); + + $this->job->handle_process_items_action(); + } +} From 009a8fe462c5c515f5cd23f95781f760a298c17c Mon Sep 17 00:00:00 2001 From: Miguel Perez Pellicer <5908855+puntope@users.noreply.github.com> Date: Tue, 20 Feb 2024 13:13:34 -0300 Subject: [PATCH 018/262] Remove wp_json_Encode from body --- src/Google/NotificationsService.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Google/NotificationsService.php b/src/Google/NotificationsService.php index 55c6d70264..541e3cf205 100644 --- a/src/Google/NotificationsService.php +++ b/src/Google/NotificationsService.php @@ -116,7 +116,7 @@ private function notification_error( string $topic, string $error, $item_id = nu * @return array|\WP_Error */ protected function do_request( array $args ) { - return Client::remote_request( $args, wp_json_encode( $args['body'] ) ); + return Client::remote_request( $args, $args['body'] ); } /** From 8ec6f134f790ee468d7c05b83126e34dca422651 Mon Sep 17 00:00:00 2001 From: Miguel Perez Pellicer <5908855+puntope@users.noreply.github.com> Date: Tue, 20 Feb 2024 16:58:38 -0300 Subject: [PATCH 019/262] Restore json_encoding and enable content type JSON --- src/Google/NotificationsService.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Google/NotificationsService.php b/src/Google/NotificationsService.php index 541e3cf205..7c16d09977 100644 --- a/src/Google/NotificationsService.php +++ b/src/Google/NotificationsService.php @@ -70,6 +70,7 @@ public function notify( string $topic, $item_id = null ): bool { 'timeout' => 30, 'headers' => [ 'x-woocommerce-topic' => $topic, + 'Content-Type' => 'application/json', ], 'body' => [ 'item_id' => $item_id, @@ -116,7 +117,7 @@ private function notification_error( string $topic, string $error, $item_id = nu * @return array|\WP_Error */ protected function do_request( array $args ) { - return Client::remote_request( $args, $args['body'] ); + return Client::remote_request( $args, wp_json_encode( $args['body'] ) ); } /** From 02a3a41ef3fc4d7625b3bd792a0ba85dda5767f4 Mon Sep 17 00:00:00 2001 From: Miguel Perez Pellicer <5908855+puntope@users.noreply.github.com> Date: Tue, 20 Feb 2024 17:08:01 -0300 Subject: [PATCH 020/262] Update tests --- tests/Unit/Google/NotificationsServiceTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/Unit/Google/NotificationsServiceTest.php b/tests/Unit/Google/NotificationsServiceTest.php index ff08f561bc..3949b58d26 100644 --- a/tests/Unit/Google/NotificationsServiceTest.php +++ b/tests/Unit/Google/NotificationsServiceTest.php @@ -69,6 +69,7 @@ public function test_notify() { 'timeout' => 30, 'headers' => [ 'x-woocommerce-topic' => $topic, + 'Content-Type' => 'application/json' ], 'body' => [ 'item_id' => $item_id, From d581864695f19487edd7e5ef27119de2c7cc5f98 Mon Sep 17 00:00:00 2001 From: Miguel Perez Pellicer <5908855+puntope@users.noreply.github.com> Date: Tue, 20 Feb 2024 17:19:18 -0300 Subject: [PATCH 021/262] PHPCS --- tests/Unit/Google/NotificationsServiceTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Unit/Google/NotificationsServiceTest.php b/tests/Unit/Google/NotificationsServiceTest.php index 3949b58d26..89bce65173 100644 --- a/tests/Unit/Google/NotificationsServiceTest.php +++ b/tests/Unit/Google/NotificationsServiceTest.php @@ -69,7 +69,7 @@ public function test_notify() { 'timeout' => 30, 'headers' => [ 'x-woocommerce-topic' => $topic, - 'Content-Type' => 'application/json' + 'Content-Type' => 'application/json', ], 'body' => [ 'item_id' => $item_id, From 8ec358e270e09f6c9fa3d6ae39426b7e2afd4648 Mon Sep 17 00:00:00 2001 From: Miguel Perez Pellicer <5908855+puntope@users.noreply.github.com> Date: Tue, 20 Feb 2024 20:07:54 -0300 Subject: [PATCH 022/262] Settings notifications --- src/Google/NotificationsService.php | 1 + .../JobServiceProvider.php | 11 ++ .../Notifications/SettingsNotificationJob.php | 101 ++++++++++++++++++ src/Settings/SyncerHooks.php | 84 +++++++++++++++ tests/Unit/Settings/SyncerHooksTest.php | 96 +++++++++++++++++ 5 files changed, 293 insertions(+) create mode 100644 src/Jobs/Notifications/SettingsNotificationJob.php create mode 100644 src/Settings/SyncerHooks.php create mode 100644 tests/Unit/Settings/SyncerHooksTest.php diff --git a/src/Google/NotificationsService.php b/src/Google/NotificationsService.php index 55c6d70264..db3f057a13 100644 --- a/src/Google/NotificationsService.php +++ b/src/Google/NotificationsService.php @@ -26,6 +26,7 @@ class NotificationsService implements Service { public const TOPIC_COUPON_DELETED = 'coupon.delete'; public const TOPIC_COUPON_UPDATED = 'coupon.update'; public const TOPIC_SHIPPING_UPDATED = 'shipping.update'; + public const TOPIC_SETTINGS_UPDATED = 'settings.update'; /** * The url to send the notification diff --git a/src/Internal/DependencyManagement/JobServiceProvider.php b/src/Internal/DependencyManagement/JobServiceProvider.php index 1ed4ef411f..5dc54a6168 100644 --- a/src/Internal/DependencyManagement/JobServiceProvider.php +++ b/src/Internal/DependencyManagement/JobServiceProvider.php @@ -24,6 +24,7 @@ use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\JobInterface; use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\JobRepository; use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\Notifications\ProductNotificationJob; +use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\Notifications\SettingsNotificationJob; use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\Notifications\ShippingNotificationJob; use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\ProductSyncerJobInterface; use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\ProductSyncStats; @@ -48,6 +49,7 @@ use Automattic\WooCommerce\GoogleListingsAndAds\Product; use Automattic\WooCommerce\GoogleListingsAndAds\Proxies\WC; use Automattic\WooCommerce\GoogleListingsAndAds\Shipping; +use Automattic\WooCommerce\GoogleListingsAndAds\Settings; defined( 'ABSPATH' ) || exit; @@ -152,6 +154,15 @@ public function register(): void { NotificationsService::class ); + // Share settings notifications job + $this->share_action_scheduler_job( + SettingsNotificationJob::class, + NotificationsService::class + ); + + // Share settings syncer hooks + $this->share_with_tags( Settings\SyncerHooks::class, JobRepository::class, NotificationsService::class ); + // Share shipping settings syncer job and hooks. $this->share_action_scheduler_job( UpdateShippingSettings::class, MerchantCenterService::class, GoogleSettings::class ); $this->share_with_tags( Shipping\SyncerHooks::class, MerchantCenterService::class, GoogleSettings::class, JobRepository::class, NotificationsService::class ); diff --git a/src/Jobs/Notifications/SettingsNotificationJob.php b/src/Jobs/Notifications/SettingsNotificationJob.php new file mode 100644 index 0000000000..a891a1eb3a --- /dev/null +++ b/src/Jobs/Notifications/SettingsNotificationJob.php @@ -0,0 +1,101 @@ +notifications_service = $notifications_service; + $this->topic = NotificationsService::TOPIC_SETTINGS_UPDATED; + parent::__construct( $action_scheduler, $monitor ); + } + + /** + * Get the job name + * + * @return string + */ + public function get_name(): string { + return 'notifications/settings'; + } + + + /** + * Logic when processing the items + * + * @param array $args Arguments for the notification + */ + protected function process_items( array $args ): void { + $this->notifications_service->notify( $this->topic ); + } + + /** + * Schedule the Job + * + * @param array $args + */ + public function schedule( array $args = [] ): void { + if ( $this->can_schedule( $args ) ) { + $this->action_scheduler->schedule_immediate( + $this->get_process_item_hook(), + [ $args ] + ); + } + } + + /** + * Can the job be scheduled. + * + * @param array|null $args + * + * @return bool Returns true if the job can be scheduled. + */ + public function can_schedule( $args = [] ): bool { + /** + * Allow users to disable the notification job schedule. + * + * @since x.x.x + * + * @param bool $value The current filter value. By default, it is the result of `$this->can_schedule` function. + * @param array $args The arguments for the schedule function with the item id and the topic. + */ + return apply_filters( 'woocommerce_gla_settings_notification_job_can_schedule', $this->notifications_service->is_enabled() && parent::can_schedule( $args ), $args ); + } +} diff --git a/src/Settings/SyncerHooks.php b/src/Settings/SyncerHooks.php new file mode 100644 index 0000000000..5e479f934d --- /dev/null +++ b/src/Settings/SyncerHooks.php @@ -0,0 +1,84 @@ +settings_notification_job = $job_repository->get( SettingsNotificationJob::class ); + $this->notifications_service = $notifications_service; + } + + /** + * Register the service. + */ + public function register(): void { + if ( ! $this->notifications_service->is_enabled() ) { + return; + } + + $update_rest = function ( $option ) { + if ( strpos( $option, 'woocommerce_' ) === 0 ) { + $this->handle_update_shipping_settings(); + } + }; + + add_action( 'update_option', $update_rest, 90, 1 ); + } + + /** + * Handle updating of Merchant Center shipping settings. + * + * @return void + */ + protected function handle_update_shipping_settings() { + // Bail if an event is already scheduled in the current request + if ( $this->already_scheduled ) { + return; + } + + $this->settings_notification_job->schedule(); + $this->already_scheduled = true; + } +} diff --git a/tests/Unit/Settings/SyncerHooksTest.php b/tests/Unit/Settings/SyncerHooksTest.php new file mode 100644 index 0000000000..1e5fb4d405 --- /dev/null +++ b/tests/Unit/Settings/SyncerHooksTest.php @@ -0,0 +1,96 @@ +notification_service->expects( $this->once() ) + ->method( 'is_enabled' ) + ->willReturn( true ); + + $this->settings_notification_job->expects( $this->once() ) + ->method( 'schedule' ); + + $this->syncer_hooks->register(); + do_action( 'update_option', 'woocommerce_sample_option' ); + } + + public function test_saving_other_settings_dont_schedules_notification_job() { + $this->notification_service->expects( $this->once() ) + ->method( 'is_enabled' ) + ->willReturn( true ); + + $this->settings_notification_job->expects( $this->never() ) + ->method( 'schedule' ); + + $this->syncer_hooks->register(); + do_action( 'update_option', 'other_option' ); + } + + public function test_dont_register_if_notifications_disabled() { + $this->notification_service->expects( $this->once() ) + ->method( 'is_enabled' ) + ->willReturn( false ); + + $this->settings_notification_job->expects( $this->never() ) + ->method( 'schedule' ); + + $this->syncer_hooks->register(); + do_action( 'update_option', 'woocommerce_sample_option' ); + } + + /** + * Runs before each test is executed. + */ + public function setUp(): void { + parent::setUp(); + + $this->login_as_administrator(); + + $this->notification_service = $this->createMock( NotificationsService::class ); + $this->settings_notification_job = $this->createMock( SettingsNotificationJob::class ); + $this->job_repository = $this->createMock( JobRepository::class ); + $this->job_repository->expects( $this->any() ) + ->method( 'get' ) + ->willReturnMap( + [ + [ SettingsNotificationJob::class, $this->settings_notification_job ], + ] + ); + + $this->syncer_hooks = new SyncerHooks( $this->job_repository, $this->notification_service ); + } +} From 1af92142945aa2f5a719f1c23ec3255938620a2f Mon Sep 17 00:00:00 2001 From: Miguel Perez Pellicer <5908855+puntope@users.noreply.github.com> Date: Tue, 20 Feb 2024 20:14:16 -0300 Subject: [PATCH 023/262] Tests --- .../Unit/Jobs/SettingsNotificationJobTest.php | 106 ++++++++++++++++++ 1 file changed, 106 insertions(+) create mode 100644 tests/Unit/Jobs/SettingsNotificationJobTest.php diff --git a/tests/Unit/Jobs/SettingsNotificationJobTest.php b/tests/Unit/Jobs/SettingsNotificationJobTest.php new file mode 100644 index 0000000000..9f6a385052 --- /dev/null +++ b/tests/Unit/Jobs/SettingsNotificationJobTest.php @@ -0,0 +1,106 @@ +action_scheduler = $this->createMock( ActionScheduler::class ); + $this->monitor = $this->createMock( ActionSchedulerJobMonitor::class ); + $this->notification_service = $this->createMock( NotificationsService::class ); + $this->job = new SettingsNotificationJob( + $this->action_scheduler, + $this->monitor, + $this->notification_service + ); + + $this->job->init(); + } + + public function test_job_name() { + $this->assertEquals( self::JOB_NAME, $this->job->get_name() ); + } + + public function test_schedule_schedules_immediate_job() { + $this->notification_service->expects( $this->once() )->method( 'is_enabled' )->willReturn( true ); + + $this->action_scheduler->expects( $this->once() ) + ->method( 'has_scheduled_action' ) + ->with( self::PROCESS_ITEM_HOOK, [] ) + ->willReturn( false ); + + $this->action_scheduler->expects( $this->once() ) + ->method( 'schedule_immediate' ) + ->with( self::PROCESS_ITEM_HOOK ); + + $this->job->schedule(); + } + + public function test_schedule_doesnt_schedules_immediate_job_if_already_scheduled() { + $this->notification_service->expects( $this->once() )->method( 'is_enabled' )->willReturn( true ); + + $this->action_scheduler->expects( $this->once() ) + ->method( 'has_scheduled_action' ) + ->with( self::PROCESS_ITEM_HOOK, [] ) + ->willReturn( true ); + + $this->action_scheduler->expects( $this->never() )->method( 'schedule_immediate' ); + + $this->job->schedule(); + } + + public function test_schedule_doesnt_schedules_immediate_job_if_not_enabled() { + $this->notification_service->expects( $this->once() )->method( 'is_enabled' )->willReturn( false ); + + $this->action_scheduler->expects( $this->never() ) + ->method( 'has_scheduled_action' ); + + $this->action_scheduler->expects( $this->never() )->method( 'schedule_immediate' ); + + $this->job->schedule(); + } + + public function test_process_items_calls_notify() { + $this->notification_service->expects( $this->once() ) + ->method( 'notify' ) + ->with( NotificationsService::TOPIC_SETTINGS_UPDATED ) + ->willReturn( true ); + + $this->job->handle_process_items_action(); + } +} From 2707f042523b2073df0caf620ed988297bfd9042 Mon Sep 17 00:00:00 2001 From: Miguel Perez Pellicer <5908855+puntope@users.noreply.github.com> Date: Wed, 21 Feb 2024 14:26:34 -0300 Subject: [PATCH 024/262] Apply review suggestions --- .../Notifications/ShippingNotificationJob.php | 4 ++-- src/Settings/SyncerHooks.php | 17 +---------------- tests/Unit/Jobs/SettingsNotificationJobTest.php | 2 -- 3 files changed, 3 insertions(+), 20 deletions(-) diff --git a/src/Jobs/Notifications/ShippingNotificationJob.php b/src/Jobs/Notifications/ShippingNotificationJob.php index 716e2c77c9..f1fea62bc2 100644 --- a/src/Jobs/Notifications/ShippingNotificationJob.php +++ b/src/Jobs/Notifications/ShippingNotificationJob.php @@ -60,14 +60,14 @@ public function get_name(): string { /** * Logic when processing the items * - * @param array $args Arguments with the item id and the topic + * @param array $args Arguments for the notification */ protected function process_items( array $args ): void { $this->notifications_service->notify( $this->topic ); } /** - * Schedule the Product Notification Job + * Schedule the Job * * @param array $args */ diff --git a/src/Settings/SyncerHooks.php b/src/Settings/SyncerHooks.php index 5e479f934d..bdd55ca20a 100644 --- a/src/Settings/SyncerHooks.php +++ b/src/Settings/SyncerHooks.php @@ -60,25 +60,10 @@ public function register(): void { $update_rest = function ( $option ) { if ( strpos( $option, 'woocommerce_' ) === 0 ) { - $this->handle_update_shipping_settings(); + $this->settings_notification_job->schedule(); } }; add_action( 'update_option', $update_rest, 90, 1 ); } - - /** - * Handle updating of Merchant Center shipping settings. - * - * @return void - */ - protected function handle_update_shipping_settings() { - // Bail if an event is already scheduled in the current request - if ( $this->already_scheduled ) { - return; - } - - $this->settings_notification_job->schedule(); - $this->already_scheduled = true; - } } diff --git a/tests/Unit/Jobs/SettingsNotificationJobTest.php b/tests/Unit/Jobs/SettingsNotificationJobTest.php index 9f6a385052..21abc27b11 100644 --- a/tests/Unit/Jobs/SettingsNotificationJobTest.php +++ b/tests/Unit/Jobs/SettingsNotificationJobTest.php @@ -9,8 +9,6 @@ use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\Notifications\SettingsNotificationJob; use Automattic\WooCommerce\GoogleListingsAndAds\Tests\Framework\UnitTest; use PHPUnit\Framework\MockObject\MockObject; -use WC_Helper_Product; - /** * Class SettingsNotificationJobTest * From cc93caff15ebbd4f9bbe2050e71f4b5f074b68e5 Mon Sep 17 00:00:00 2001 From: Miguel Perez Pellicer <5908855+puntope@users.noreply.github.com> Date: Wed, 21 Feb 2024 14:47:17 -0300 Subject: [PATCH 025/262] Fix Can schedule --- src/Jobs/Notifications/ProductNotificationJob.php | 2 +- src/Jobs/Notifications/SettingsNotificationJob.php | 2 +- src/Jobs/Notifications/ShippingNotificationJob.php | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Jobs/Notifications/ProductNotificationJob.php b/src/Jobs/Notifications/ProductNotificationJob.php index ca421cbedc..a05b4ae2e4 100644 --- a/src/Jobs/Notifications/ProductNotificationJob.php +++ b/src/Jobs/Notifications/ProductNotificationJob.php @@ -86,7 +86,7 @@ protected function process_items( array $args ): void { * @param array $args */ public function schedule( array $args = [] ): void { - if ( $this->can_schedule( $args ) ) { + if ( $this->can_schedule( [ $args ] ) ) { $this->action_scheduler->schedule_immediate( $this->get_process_item_hook(), [ $args ] diff --git a/src/Jobs/Notifications/SettingsNotificationJob.php b/src/Jobs/Notifications/SettingsNotificationJob.php index a891a1eb3a..4e786cc47c 100644 --- a/src/Jobs/Notifications/SettingsNotificationJob.php +++ b/src/Jobs/Notifications/SettingsNotificationJob.php @@ -72,7 +72,7 @@ protected function process_items( array $args ): void { * @param array $args */ public function schedule( array $args = [] ): void { - if ( $this->can_schedule( $args ) ) { + if ( $this->can_schedule( [ $args ] ) ) { $this->action_scheduler->schedule_immediate( $this->get_process_item_hook(), [ $args ] diff --git a/src/Jobs/Notifications/ShippingNotificationJob.php b/src/Jobs/Notifications/ShippingNotificationJob.php index f1fea62bc2..8484bac31a 100644 --- a/src/Jobs/Notifications/ShippingNotificationJob.php +++ b/src/Jobs/Notifications/ShippingNotificationJob.php @@ -72,7 +72,7 @@ protected function process_items( array $args ): void { * @param array $args */ public function schedule( array $args = [] ): void { - if ( $this->can_schedule( $args ) ) { + if ( $this->can_schedule( [ $args ] ) ) { $this->action_scheduler->schedule_immediate( $this->get_process_item_hook(), [ $args ] From 8d8f706fb2e47ea05070b417c57d43e17ce889c3 Mon Sep 17 00:00:00 2001 From: Miguel Perez Pellicer <5908855+puntope@users.noreply.github.com> Date: Wed, 21 Feb 2024 15:01:10 -0300 Subject: [PATCH 026/262] Adapt tests --- src/Jobs/Notifications/ProductNotificationJob.php | 2 +- src/Jobs/Notifications/ShippingNotificationJob.php | 2 +- tests/Unit/Jobs/SettingsNotificationJobTest.php | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Jobs/Notifications/ProductNotificationJob.php b/src/Jobs/Notifications/ProductNotificationJob.php index a05b4ae2e4..ca421cbedc 100644 --- a/src/Jobs/Notifications/ProductNotificationJob.php +++ b/src/Jobs/Notifications/ProductNotificationJob.php @@ -86,7 +86,7 @@ protected function process_items( array $args ): void { * @param array $args */ public function schedule( array $args = [] ): void { - if ( $this->can_schedule( [ $args ] ) ) { + if ( $this->can_schedule( $args ) ) { $this->action_scheduler->schedule_immediate( $this->get_process_item_hook(), [ $args ] diff --git a/src/Jobs/Notifications/ShippingNotificationJob.php b/src/Jobs/Notifications/ShippingNotificationJob.php index 8484bac31a..f1fea62bc2 100644 --- a/src/Jobs/Notifications/ShippingNotificationJob.php +++ b/src/Jobs/Notifications/ShippingNotificationJob.php @@ -72,7 +72,7 @@ protected function process_items( array $args ): void { * @param array $args */ public function schedule( array $args = [] ): void { - if ( $this->can_schedule( [ $args ] ) ) { + if ( $this->can_schedule( $args ) ) { $this->action_scheduler->schedule_immediate( $this->get_process_item_hook(), [ $args ] diff --git a/tests/Unit/Jobs/SettingsNotificationJobTest.php b/tests/Unit/Jobs/SettingsNotificationJobTest.php index 21abc27b11..0489123e59 100644 --- a/tests/Unit/Jobs/SettingsNotificationJobTest.php +++ b/tests/Unit/Jobs/SettingsNotificationJobTest.php @@ -59,7 +59,7 @@ public function test_schedule_schedules_immediate_job() { $this->action_scheduler->expects( $this->once() ) ->method( 'has_scheduled_action' ) - ->with( self::PROCESS_ITEM_HOOK, [] ) + ->with( self::PROCESS_ITEM_HOOK ) ->willReturn( false ); $this->action_scheduler->expects( $this->once() ) @@ -74,7 +74,7 @@ public function test_schedule_doesnt_schedules_immediate_job_if_already_schedule $this->action_scheduler->expects( $this->once() ) ->method( 'has_scheduled_action' ) - ->with( self::PROCESS_ITEM_HOOK, [] ) + ->with( self::PROCESS_ITEM_HOOK ) ->willReturn( true ); $this->action_scheduler->expects( $this->never() )->method( 'schedule_immediate' ); From 1ecf5f7f53265fcfd32ed7e59426f6a16fce7391 Mon Sep 17 00:00:00 2001 From: Miguel Perez Pellicer <5908855+puntope@users.noreply.github.com> Date: Wed, 21 Feb 2024 16:48:42 -0300 Subject: [PATCH 027/262] Add connection test entry --- src/ConnectionTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/ConnectionTest.php b/src/ConnectionTest.php index 89224cc613..5bdeabc648 100644 --- a/src/ConnectionTest.php +++ b/src/ConnectionTest.php @@ -651,6 +651,7 @@ protected function render_admin_page() { >coupon.delete >coupon.update >shipping.update + >settings.update Send Notification From e7aa54b7274ef7ec87e708740f6c0e0eb554d98d Mon Sep 17 00:00:00 2001 From: Miguel Perez Pellicer <5908855+puntope@users.noreply.github.com> Date: Wed, 21 Feb 2024 16:49:40 -0300 Subject: [PATCH 028/262] Tweak connection test --- src/ConnectionTest.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ConnectionTest.php b/src/ConnectionTest.php index 5bdeabc648..6f6c742493 100644 --- a/src/ConnectionTest.php +++ b/src/ConnectionTest.php @@ -737,12 +737,12 @@ protected function handle_actions() { } if ( 'partner-notification' === $_GET['action'] && check_admin_referer( 'partner-notification' ) ) { - if ( ! isset( $_GET['topic'], $_GET['item_id'] ) ) { - $this->response .= "\n Topic and Item ID are required."; + if ( ! isset( $_GET['topic'] ) ) { + $this->response .= "\n Topic is required."; return; } - $item = $_GET['item_id']; + $item = $_GET['item_id'] ?? null; $topic = $_GET['topic']; $service = new NotificationsService(); From 26c9861280f93e93a775f1965b7e3ca86b1c4c9e Mon Sep 17 00:00:00 2001 From: Miguel Perez Pellicer <5908855+puntope@users.noreply.github.com> Date: Thu, 22 Feb 2024 17:51:46 -0300 Subject: [PATCH 029/262] Refactor using whitelisted settings --- src/Settings/SyncerHooks.php | 42 ++++++++++++++++++++----- tests/Unit/Settings/SyncerHooksTest.php | 42 ++++++++++++++++++++++--- 2 files changed, 71 insertions(+), 13 deletions(-) diff --git a/src/Settings/SyncerHooks.php b/src/Settings/SyncerHooks.php index bdd55ca20a..ab5b601601 100644 --- a/src/Settings/SyncerHooks.php +++ b/src/Settings/SyncerHooks.php @@ -22,13 +22,6 @@ */ class SyncerHooks implements Service, Registerable { - /** - * This property is used to avoid scheduling duplicate jobs in the same request. - * - * @var bool - */ - protected $already_scheduled = false; - /** * @var NotificationsService $notifications_service */ @@ -39,6 +32,39 @@ class SyncerHooks implements Service, Registerable { */ protected $settings_notification_job; + /** + * WooCommerce General Settings IDs + * Copied from https://github.com/woocommerce/woocommerce/blob/af03815134385c72feb7a70abc597eca57442820/plugins/woocommerce/includes/admin/settings/class-wc-settings-general.php#L34 + */ + protected const ALLOWED_SETTINGS = [ + 'store_address', + 'woocommerce_store_address', + 'woocommerce_store_address_2', + 'woocommerce_store_city', + 'woocommerce_default_country', + 'woocommerce_store_postcode', + 'store_address', + 'general_options', + 'woocommerce_allowed_countries', + 'woocommerce_all_except_countries', + 'woocommerce_specific_allowed_countries', + 'woocommerce_ship_to_countries', + 'woocommerce_specific_ship_to_countries', + 'woocommerce_default_customer_address', + 'woocommerce_calc_taxes', + 'woocommerce_enable_coupons', + 'woocommerce_calc_discounts_sequentially', + 'general_options', + 'pricing_options', + 'woocommerce_currency', + 'woocommerce_currency_pos', + 'woocommerce_price_thousand_sep', + 'woocommerce_price_decimal_sep', + 'woocommerce_price_num_decimals', + 'pricing_options', + ]; + + /** * SyncerHooks constructor. * @@ -59,7 +85,7 @@ public function register(): void { } $update_rest = function ( $option ) { - if ( strpos( $option, 'woocommerce_' ) === 0 ) { + if ( in_array( $option, self::ALLOWED_SETTINGS, true ) ) { $this->settings_notification_job->schedule(); } }; diff --git a/tests/Unit/Settings/SyncerHooksTest.php b/tests/Unit/Settings/SyncerHooksTest.php index 1e5fb4d405..d6d30995ac 100644 --- a/tests/Unit/Settings/SyncerHooksTest.php +++ b/tests/Unit/Settings/SyncerHooksTest.php @@ -35,17 +35,49 @@ class SyncerHooksTest extends UnitTest { */ protected $sample_class_id; - - public function test_saving_woocommerce_settings_schedules_notification_job() { + /** + * WooCommerce General Settings IDs + * Copied from https://github.com/woocommerce/woocommerce/blob/af03815134385c72feb7a70abc597eca57442820/plugins/woocommerce/includes/admin/settings/class-wc-settings-general.php#L34 + */ + protected const ALLOWED_SETTINGS = [ + 'store_address', + 'woocommerce_store_address', + 'woocommerce_store_address_2', + 'woocommerce_store_city', + 'woocommerce_default_country', + 'woocommerce_store_postcode', + 'store_address', + 'general_options', + 'woocommerce_allowed_countries', + 'woocommerce_all_except_countries', + 'woocommerce_specific_allowed_countries', + 'woocommerce_ship_to_countries', + 'woocommerce_specific_ship_to_countries', + 'woocommerce_default_customer_address', + 'woocommerce_calc_taxes', + 'woocommerce_enable_coupons', + 'woocommerce_calc_discounts_sequentially', + 'general_options', + 'pricing_options', + 'woocommerce_currency', + 'woocommerce_currency_pos', + 'woocommerce_price_thousand_sep', + 'woocommerce_price_decimal_sep', + 'woocommerce_price_num_decimals', + 'pricing_options', + ]; + public function test_saving_woocommerce_general_settings_schedules_notification_job() { $this->notification_service->expects( $this->once() ) ->method( 'is_enabled' ) ->willReturn( true ); - $this->settings_notification_job->expects( $this->once() ) + $this->settings_notification_job->expects( $this->exactly ( count( self::ALLOWED_SETTINGS ) ) ) ->method( 'schedule' ); $this->syncer_hooks->register(); - do_action( 'update_option', 'woocommerce_sample_option' ); + foreach ( self::ALLOWED_SETTINGS as $setting ) { + do_action( 'update_option', $setting ); + } } public function test_saving_other_settings_dont_schedules_notification_job() { @@ -69,7 +101,7 @@ public function test_dont_register_if_notifications_disabled() { ->method( 'schedule' ); $this->syncer_hooks->register(); - do_action( 'update_option', 'woocommerce_sample_option' ); + do_action( 'update_option', 'store_address' ); } /** From 2b0675512049346bb3780e821f5bc99162845004 Mon Sep 17 00:00:00 2001 From: Miguel Perez Pellicer <5908855+puntope@users.noreply.github.com> Date: Thu, 22 Feb 2024 17:59:40 -0300 Subject: [PATCH 030/262] Fix Warning in connection test --- src/ConnectionTest.php | 14 +++++++------- tests/Unit/Settings/SyncerHooksTest.php | 3 ++- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/ConnectionTest.php b/src/ConnectionTest.php index 6f6c742493..fb6e46a9f4 100644 --- a/src/ConnectionTest.php +++ b/src/ConnectionTest.php @@ -645,13 +645,13 @@ protected function render_admin_page() { Topic >product.create - >product.delete - >product.update - >coupon.create - >coupon.delete - >coupon.update - >shipping.update - >settings.update + >product.delete + >product.update + >coupon.create + >coupon.delete + >coupon.update + >shipping.update + >settings.update Send Notification diff --git a/tests/Unit/Settings/SyncerHooksTest.php b/tests/Unit/Settings/SyncerHooksTest.php index d6d30995ac..3aa90b8490 100644 --- a/tests/Unit/Settings/SyncerHooksTest.php +++ b/tests/Unit/Settings/SyncerHooksTest.php @@ -66,12 +66,13 @@ class SyncerHooksTest extends UnitTest { 'woocommerce_price_num_decimals', 'pricing_options', ]; + public function test_saving_woocommerce_general_settings_schedules_notification_job() { $this->notification_service->expects( $this->once() ) ->method( 'is_enabled' ) ->willReturn( true ); - $this->settings_notification_job->expects( $this->exactly ( count( self::ALLOWED_SETTINGS ) ) ) + $this->settings_notification_job->expects( $this->exactly( count( self::ALLOWED_SETTINGS ) ) ) ->method( 'schedule' ); $this->syncer_hooks->register(); From a2689f599afefe3e07080068028ac22c3085a4ce Mon Sep 17 00:00:00 2001 From: Miguel Perez Pellicer <5908855+puntope@users.noreply.github.com> Date: Thu, 22 Feb 2024 19:19:32 -0300 Subject: [PATCH 031/262] Refactor Jobs --- .../Notifications/AbstractNotificationJob.php | 122 ++++++++++++++++++ .../Notifications/ProductNotificationJob.php | 48 ++----- .../Notifications/SettingsNotificationJob.php | 78 +---------- .../Notifications/ShippingNotificationJob.php | 74 +---------- .../Unit/Jobs/AbstractNotificationJobTest.php | 109 ++++++++++++++++ .../Unit/Jobs/ProductNotificationJobTest.php | 4 +- .../Unit/Jobs/SettingsNotificationJobTest.php | 92 ++----------- .../Unit/Jobs/ShippingNotificationJobTest.php | 93 ++----------- 8 files changed, 271 insertions(+), 349 deletions(-) create mode 100644 src/Jobs/Notifications/AbstractNotificationJob.php create mode 100644 tests/Unit/Jobs/AbstractNotificationJobTest.php diff --git a/src/Jobs/Notifications/AbstractNotificationJob.php b/src/Jobs/Notifications/AbstractNotificationJob.php new file mode 100644 index 0000000000..22cc88c795 --- /dev/null +++ b/src/Jobs/Notifications/AbstractNotificationJob.php @@ -0,0 +1,122 @@ +notifications_service = $notifications_service; + parent::__construct( $action_scheduler, $monitor ); + } + + /** + * Get the parent job name + * + * @return string + */ + public function get_name(): string { + return 'notifications/' . $this->get_job_name(); + } + + + /** + * Schedule the Job + * + * @param array $args + */ + public function schedule( array $args = [] ): void { + if ( $this->can_schedule( [ $args ] ) ) { + $this->action_scheduler->schedule_immediate( + $this->get_process_item_hook(), + [ $args ] + ); + } + } + + + /** + * Get the Notification Status after the notification happens + * + * @param string $topic + * @return string + */ + protected function get_after_notification_status( string $topic ): string { + if ( str_contains( $topic, '.create' ) ) { + return NotificationStatus::NOTIFICATION_CREATED; + } elseif ( str_contains( $topic, '.delete' ) ) { + return NotificationStatus::NOTIFICATION_DELETED; + } else { + return NotificationStatus::NOTIFICATION_UPDATED; + } + } + + + /** + * Can the job be scheduled. + * + * @param array|null $args + * + * @return bool Returns true if the job can be scheduled. + */ + public function can_schedule( $args = [] ): bool { + /** + * Allow users to disable the notification job schedule. + * + * @since x.x.x + * + * @param bool $value The current filter value. By default, it is the result of `$this->can_schedule` function. + * @param string $job_name The current Job name. + * @param array $args The arguments for the schedule function with the item id and the topic. + */ + return apply_filters( 'woocommerce_gla_notification_job_can_schedule', $this->notifications_service->is_enabled() && parent::can_schedule( $args ), $this->get_job_name(), $args ); + } + + /** + * Get the child job name + * + * @return string + */ + abstract public function get_job_name(): string; + + /** + * Logic when processing the items + * + * @param array $args + */ + abstract protected function process_items( array $args ): void; +} diff --git a/src/Jobs/Notifications/ProductNotificationJob.php b/src/Jobs/Notifications/ProductNotificationJob.php index ca421cbedc..f02ce37634 100644 --- a/src/Jobs/Notifications/ProductNotificationJob.php +++ b/src/Jobs/Notifications/ProductNotificationJob.php @@ -5,9 +5,7 @@ use Automattic\WooCommerce\GoogleListingsAndAds\ActionScheduler\ActionSchedulerInterface; use Automattic\WooCommerce\GoogleListingsAndAds\Google\NotificationsService; -use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\AbstractActionSchedulerJob; use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\ActionSchedulerJobMonitor; -use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\JobInterface; use Automattic\WooCommerce\GoogleListingsAndAds\Product\ProductHelper; use Automattic\WooCommerce\GoogleListingsAndAds\Value\NotificationStatus; @@ -21,7 +19,7 @@ * @since x.x.x * @package Automattic\WooCommerce\GoogleListingsAndAds\Jobs\Notifications */ -class ProductNotificationJob extends AbstractActionSchedulerJob implements JobInterface { +class ProductNotificationJob extends AbstractNotificationJob { /** * @var NotificationsService $notifications_service @@ -49,7 +47,7 @@ public function __construct( ) { $this->notifications_service = $notifications_service; $this->product_helper = $product_helper; - parent::__construct( $action_scheduler, $monitor ); + parent::__construct( $action_scheduler, $monitor, $notifications_service ); } /** @@ -57,8 +55,8 @@ public function __construct( * * @return string */ - public function get_name(): string { - return 'notifications/products'; + public function get_job_name(): string { + return 'products'; } @@ -69,6 +67,11 @@ public function get_name(): string { */ protected function process_items( array $args ): void { if ( ! isset( $args[0] ) || ! isset( $args[1] ) ) { + do_action( + 'woocommerce_gla_error', + 'Error processing Product Notification Job. Item ID and Topic are mandatory.', + __METHOD__ + ); return; } @@ -80,20 +83,6 @@ protected function process_items( array $args ): void { } } - /** - * Schedule the Product Notification Job - * - * @param array $args - */ - public function schedule( array $args = [] ): void { - if ( $this->can_schedule( $args ) ) { - $this->action_scheduler->schedule_immediate( - $this->get_process_item_hook(), - [ $args ] - ); - } - } - /** * Set the notification status for a product. * @@ -140,23 +129,4 @@ protected function can_process( int $product_id, string $topic ): bool { return $this->product_helper->should_trigger_update_notification( $product ); } } - - /** - * Can the job be scheduled. - * - * @param array|null $args - * - * @return bool Returns true if the job can be scheduled. - */ - public function can_schedule( $args = [] ): bool { - /** - * Allow users to disable the notification job schedule. - * - * @since x.x.x - * - * @param bool $value The current filter value. By default, it is the result of `$this->can_schedule` function. - * @param array $args The arguments for the schedule function with the item id and the topic. - */ - return apply_filters( 'woocommerce_gla_product_notification_job_can_schedule', $this->notifications_service->is_enabled() && parent::can_schedule( $args ), $args ); - } } diff --git a/src/Jobs/Notifications/SettingsNotificationJob.php b/src/Jobs/Notifications/SettingsNotificationJob.php index 4e786cc47c..29a4ea41bd 100644 --- a/src/Jobs/Notifications/SettingsNotificationJob.php +++ b/src/Jobs/Notifications/SettingsNotificationJob.php @@ -3,12 +3,6 @@ namespace Automattic\WooCommerce\GoogleListingsAndAds\Jobs\Notifications; -use Automattic\WooCommerce\GoogleListingsAndAds\ActionScheduler\ActionSchedulerInterface; -use Automattic\WooCommerce\GoogleListingsAndAds\Google\NotificationsService; -use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\AbstractActionSchedulerJob; -use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\ActionSchedulerJobMonitor; -use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\JobInterface; - defined( 'ABSPATH' ) || exit; /** @@ -18,44 +12,7 @@ * @since x.x.x * @package Automattic\WooCommerce\GoogleListingsAndAds\Jobs\Notifications */ -class SettingsNotificationJob extends AbstractActionSchedulerJob implements JobInterface { - - /** - * @var NotificationsService $notifications_service - */ - protected $notifications_service; - - /** - * @var string $topic - */ - protected $topic; - - /** - * Notifications Jobs constructor. - * - * @param ActionSchedulerInterface $action_scheduler - * @param ActionSchedulerJobMonitor $monitor - * @param NotificationsService $notifications_service - */ - public function __construct( - ActionSchedulerInterface $action_scheduler, - ActionSchedulerJobMonitor $monitor, - NotificationsService $notifications_service - ) { - $this->notifications_service = $notifications_service; - $this->topic = NotificationsService::TOPIC_SETTINGS_UPDATED; - parent::__construct( $action_scheduler, $monitor ); - } - - /** - * Get the job name - * - * @return string - */ - public function get_name(): string { - return 'notifications/settings'; - } - +class SettingsNotificationJob extends AbstractNotificationJob { /** * Logic when processing the items @@ -63,39 +20,16 @@ public function get_name(): string { * @param array $args Arguments for the notification */ protected function process_items( array $args ): void { - $this->notifications_service->notify( $this->topic ); + $this->notifications_service->notify( $this->notifications_service::TOPIC_SETTINGS_UPDATED ); } - /** - * Schedule the Job - * - * @param array $args - */ - public function schedule( array $args = [] ): void { - if ( $this->can_schedule( [ $args ] ) ) { - $this->action_scheduler->schedule_immediate( - $this->get_process_item_hook(), - [ $args ] - ); - } - } /** - * Can the job be scheduled. - * - * @param array|null $args + * Get the job name * - * @return bool Returns true if the job can be scheduled. + * @return string */ - public function can_schedule( $args = [] ): bool { - /** - * Allow users to disable the notification job schedule. - * - * @since x.x.x - * - * @param bool $value The current filter value. By default, it is the result of `$this->can_schedule` function. - * @param array $args The arguments for the schedule function with the item id and the topic. - */ - return apply_filters( 'woocommerce_gla_settings_notification_job_can_schedule', $this->notifications_service->is_enabled() && parent::can_schedule( $args ), $args ); + public function get_job_name(): string { + return 'settings'; } } diff --git a/src/Jobs/Notifications/ShippingNotificationJob.php b/src/Jobs/Notifications/ShippingNotificationJob.php index f1fea62bc2..b056e4669c 100644 --- a/src/Jobs/Notifications/ShippingNotificationJob.php +++ b/src/Jobs/Notifications/ShippingNotificationJob.php @@ -3,12 +3,6 @@ namespace Automattic\WooCommerce\GoogleListingsAndAds\Jobs\Notifications; -use Automattic\WooCommerce\GoogleListingsAndAds\ActionScheduler\ActionSchedulerInterface; -use Automattic\WooCommerce\GoogleListingsAndAds\Google\NotificationsService; -use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\AbstractActionSchedulerJob; -use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\ActionSchedulerJobMonitor; -use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\JobInterface; - defined( 'ABSPATH' ) || exit; /** @@ -18,42 +12,15 @@ * @since x.x.x * @package Automattic\WooCommerce\GoogleListingsAndAds\Jobs\Notifications */ -class ShippingNotificationJob extends AbstractActionSchedulerJob implements JobInterface { - - /** - * @var NotificationsService $notifications_service - */ - protected $notifications_service; - - /** - * @var string $topic - */ - protected $topic; - - /** - * Notifications Jobs constructor. - * - * @param ActionSchedulerInterface $action_scheduler - * @param ActionSchedulerJobMonitor $monitor - * @param NotificationsService $notifications_service - */ - public function __construct( - ActionSchedulerInterface $action_scheduler, - ActionSchedulerJobMonitor $monitor, - NotificationsService $notifications_service - ) { - $this->notifications_service = $notifications_service; - $this->topic = NotificationsService::TOPIC_SHIPPING_UPDATED; - parent::__construct( $action_scheduler, $monitor ); - } +class ShippingNotificationJob extends AbstractNotificationJob { /** * Get the job name * * @return string */ - public function get_name(): string { - return 'notifications/shipping'; + public function get_job_name(): string { + return 'shipping'; } @@ -63,39 +30,6 @@ public function get_name(): string { * @param array $args Arguments for the notification */ protected function process_items( array $args ): void { - $this->notifications_service->notify( $this->topic ); - } - - /** - * Schedule the Job - * - * @param array $args - */ - public function schedule( array $args = [] ): void { - if ( $this->can_schedule( $args ) ) { - $this->action_scheduler->schedule_immediate( - $this->get_process_item_hook(), - [ $args ] - ); - } - } - - /** - * Can the job be scheduled. - * - * @param array|null $args - * - * @return bool Returns true if the job can be scheduled. - */ - public function can_schedule( $args = [] ): bool { - /** - * Allow users to disable the notification job schedule. - * - * @since x.x.x - * - * @param bool $value The current filter value. By default, it is the result of `$this->can_schedule` function. - * @param array $args The arguments for the schedule function with the item id and the topic. - */ - return apply_filters( 'woocommerce_gla_shipping_notification_job_can_schedule', $this->notifications_service->is_enabled() && parent::can_schedule( $args ), $args ); + $this->notifications_service->notify( $this->notifications_service::TOPIC_SHIPPING_UPDATED ); } } diff --git a/tests/Unit/Jobs/AbstractNotificationJobTest.php b/tests/Unit/Jobs/AbstractNotificationJobTest.php new file mode 100644 index 0000000000..67d1c64f13 --- /dev/null +++ b/tests/Unit/Jobs/AbstractNotificationJobTest.php @@ -0,0 +1,109 @@ +action_scheduler = $this->createMock( ActionScheduler::class ); + $this->monitor = $this->createMock( ActionSchedulerJobMonitor::class ); + $this->notification_service = $this->createMock( NotificationsService::class ); + $this->job = $this->get_job(); + + $this->job->init(); + } + + public function test_job_name() { + $this->assertEquals( $this->get_name(), $this->job->get_name() ); + } + + public function test_schedule_schedules_immediate_job() { + $this->notification_service->expects( $this->once() )->method( 'is_enabled' )->willReturn( true ); + + $this->action_scheduler->expects( $this->once() ) + ->method( 'has_scheduled_action' ) + ->with( $this->get_process_item_hook() ) + ->willReturn( false ); + + $this->action_scheduler->expects( $this->once() ) + ->method( 'schedule_immediate' ) + ->with( $this->get_process_item_hook() ); + + $this->job->schedule(); + } + + public function test_schedule_doesnt_schedules_immediate_job_if_already_scheduled() { + $this->notification_service->expects( $this->once() )->method( 'is_enabled' )->willReturn( true ); + + $this->action_scheduler->expects( $this->once() ) + ->method( 'has_scheduled_action' ) + ->with( $this->get_process_item_hook() ) + ->willReturn( true ); + + $this->action_scheduler->expects( $this->never() )->method( 'schedule_immediate' ); + + $this->job->schedule(); + } + + public function test_schedule_doesnt_schedules_immediate_job_if_not_enabled() { + $this->notification_service->expects( $this->once() )->method( 'is_enabled' )->willReturn( false ); + + $this->action_scheduler->expects( $this->never() ) + ->method( 'has_scheduled_action' ); + + $this->action_scheduler->expects( $this->never() )->method( 'schedule_immediate' ); + + $this->job->schedule(); + } + + public function test_process_items_calls_notify() { + $this->notification_service->expects( $this->once() ) + ->method( 'notify' ) + ->with( $this->get_topic() ) + ->willReturn( true ); + + $this->job->handle_process_items_action(); + } + + public function get_name() { + return 'notifications/' . $this->get_job_name(); + } + + public function get_process_item_hook() { + return 'gla/jobs/' . $this->get_name() . '/process_item'; + } + + abstract public function get_topic(); + abstract public function get_job_name(); + abstract public function get_job(); +} diff --git a/tests/Unit/Jobs/ProductNotificationJobTest.php b/tests/Unit/Jobs/ProductNotificationJobTest.php index 5b985ff50b..abb379794f 100644 --- a/tests/Unit/Jobs/ProductNotificationJobTest.php +++ b/tests/Unit/Jobs/ProductNotificationJobTest.php @@ -72,7 +72,7 @@ public function test_schedule_schedules_immediate_job() { $this->action_scheduler->expects( $this->once() ) ->method( 'has_scheduled_action' ) - ->with( self::PROCESS_ITEM_HOOK, [ $id, $topic ] ) + ->with( self::PROCESS_ITEM_HOOK, [ [ $id, $topic ] ] ) ->willReturn( false ); $this->action_scheduler->expects( $this->once() ) @@ -93,7 +93,7 @@ public function test_schedule_doesnt_schedules_immediate_job_if_already_schedule $this->action_scheduler->expects( $this->once() ) ->method( 'has_scheduled_action' ) - ->with( self::PROCESS_ITEM_HOOK, [ $id, $topic ] ) + ->with( self::PROCESS_ITEM_HOOK, [ [ $id, $topic ] ] ) ->willReturn( true ); $this->action_scheduler->expects( $this->never() )->method( 'schedule_immediate' ); diff --git a/tests/Unit/Jobs/SettingsNotificationJobTest.php b/tests/Unit/Jobs/SettingsNotificationJobTest.php index 0489123e59..7c5e86d72a 100644 --- a/tests/Unit/Jobs/SettingsNotificationJobTest.php +++ b/tests/Unit/Jobs/SettingsNotificationJobTest.php @@ -3,102 +3,30 @@ namespace Automattic\WooCommerce\GoogleListingsAndAds\Tests\Unit\Jobs; -use Automattic\WooCommerce\GoogleListingsAndAds\ActionScheduler\ActionScheduler; -use Automattic\WooCommerce\GoogleListingsAndAds\Google\NotificationsService; -use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\ActionSchedulerJobMonitor; use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\Notifications\SettingsNotificationJob; -use Automattic\WooCommerce\GoogleListingsAndAds\Tests\Framework\UnitTest; -use PHPUnit\Framework\MockObject\MockObject; + /** * Class SettingsNotificationJobTest * * @group Notifications * @package Automattic\WooCommerce\GoogleListingsAndAds\Tests\Unit\Jobs */ -class SettingsNotificationJobTest extends UnitTest { - - /** @var MockObject|ActionScheduler $action_scheduler */ - protected $action_scheduler; - - /** @var MockObject|ActionSchedulerJobMonitor $monitor */ - protected $monitor; - - /** @var MockObject|NotificationsService $notification_service */ - protected $notification_service; +class SettingsNotificationJobTest extends AbstractNotificationJobTest { - /** @var SettingsNotificationJob $job */ - protected $job; - protected const JOB_NAME = 'notifications/settings'; - protected const PROCESS_ITEM_HOOK = 'gla/jobs/' . self::JOB_NAME . '/process_item'; + public function get_topic() { + return $this->notification_service::TOPIC_SETTINGS_UPDATED; + } - /** - * Runs before each test is executed. - */ - public function setUp(): void { - parent::setUp(); + public function get_job_name() { + return 'settings'; + } - $this->action_scheduler = $this->createMock( ActionScheduler::class ); - $this->monitor = $this->createMock( ActionSchedulerJobMonitor::class ); - $this->notification_service = $this->createMock( NotificationsService::class ); - $this->job = new SettingsNotificationJob( + public function get_job() { + return new SettingsNotificationJob( $this->action_scheduler, $this->monitor, $this->notification_service ); - - $this->job->init(); - } - - public function test_job_name() { - $this->assertEquals( self::JOB_NAME, $this->job->get_name() ); - } - - public function test_schedule_schedules_immediate_job() { - $this->notification_service->expects( $this->once() )->method( 'is_enabled' )->willReturn( true ); - - $this->action_scheduler->expects( $this->once() ) - ->method( 'has_scheduled_action' ) - ->with( self::PROCESS_ITEM_HOOK ) - ->willReturn( false ); - - $this->action_scheduler->expects( $this->once() ) - ->method( 'schedule_immediate' ) - ->with( self::PROCESS_ITEM_HOOK ); - - $this->job->schedule(); - } - - public function test_schedule_doesnt_schedules_immediate_job_if_already_scheduled() { - $this->notification_service->expects( $this->once() )->method( 'is_enabled' )->willReturn( true ); - - $this->action_scheduler->expects( $this->once() ) - ->method( 'has_scheduled_action' ) - ->with( self::PROCESS_ITEM_HOOK ) - ->willReturn( true ); - - $this->action_scheduler->expects( $this->never() )->method( 'schedule_immediate' ); - - $this->job->schedule(); - } - - public function test_schedule_doesnt_schedules_immediate_job_if_not_enabled() { - $this->notification_service->expects( $this->once() )->method( 'is_enabled' )->willReturn( false ); - - $this->action_scheduler->expects( $this->never() ) - ->method( 'has_scheduled_action' ); - - $this->action_scheduler->expects( $this->never() )->method( 'schedule_immediate' ); - - $this->job->schedule(); - } - - public function test_process_items_calls_notify() { - $this->notification_service->expects( $this->once() ) - ->method( 'notify' ) - ->with( NotificationsService::TOPIC_SETTINGS_UPDATED ) - ->willReturn( true ); - - $this->job->handle_process_items_action(); } } diff --git a/tests/Unit/Jobs/ShippingNotificationJobTest.php b/tests/Unit/Jobs/ShippingNotificationJobTest.php index 8cc10c43d2..d57ba4efca 100644 --- a/tests/Unit/Jobs/ShippingNotificationJobTest.php +++ b/tests/Unit/Jobs/ShippingNotificationJobTest.php @@ -3,14 +3,7 @@ namespace Automattic\WooCommerce\GoogleListingsAndAds\Tests\Unit\Jobs; -use Automattic\WooCommerce\GoogleListingsAndAds\ActionScheduler\ActionScheduler; -use Automattic\WooCommerce\GoogleListingsAndAds\Google\NotificationsService; -use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\ActionSchedulerJobMonitor; -use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\Notifications\ProductNotificationJob; use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\Notifications\ShippingNotificationJob; -use Automattic\WooCommerce\GoogleListingsAndAds\Tests\Framework\UnitTest; -use PHPUnit\Framework\MockObject\MockObject; -use WC_Helper_Product; /** * Class ShippingNotificationJobTest @@ -18,90 +11,22 @@ * @group Notifications * @package Automattic\WooCommerce\GoogleListingsAndAds\Tests\Unit\Jobs */ -class ShippingNotificationJobTest extends UnitTest { +class ShippingNotificationJobTest extends AbstractNotificationJobTest { - /** @var MockObject|ActionScheduler $action_scheduler */ - protected $action_scheduler; - /** @var MockObject|ActionSchedulerJobMonitor $monitor */ - protected $monitor; - - /** @var MockObject|NotificationsService $notification_service */ - protected $notification_service; - - /** @var ProductNotificationJob $job */ - protected $job; - - protected const JOB_NAME = 'notifications/shipping'; - protected const PROCESS_ITEM_HOOK = 'gla/jobs/' . self::JOB_NAME . '/process_item'; + public function get_topic() { + return $this->notification_service::TOPIC_SHIPPING_UPDATED; + } - /** - * Runs before each test is executed. - */ - public function setUp(): void { - parent::setUp(); + public function get_job_name() { + return 'shipping'; + } - $this->action_scheduler = $this->createMock( ActionScheduler::class ); - $this->monitor = $this->createMock( ActionSchedulerJobMonitor::class ); - $this->notification_service = $this->createMock( NotificationsService::class ); - $this->job = new ShippingNotificationJob( + public function get_job() { + return new ShippingNotificationJob( $this->action_scheduler, $this->monitor, $this->notification_service ); - - $this->job->init(); - } - - public function test_job_name() { - $this->assertEquals( self::JOB_NAME, $this->job->get_name() ); - } - - public function test_schedule_schedules_immediate_job() { - $this->notification_service->expects( $this->once() )->method( 'is_enabled' )->willReturn( true ); - - $this->action_scheduler->expects( $this->once() ) - ->method( 'has_scheduled_action' ) - ->with( self::PROCESS_ITEM_HOOK, [] ) - ->willReturn( false ); - - $this->action_scheduler->expects( $this->once() ) - ->method( 'schedule_immediate' ) - ->with( self::PROCESS_ITEM_HOOK ); - - $this->job->schedule(); - } - - public function test_schedule_doesnt_schedules_immediate_job_if_already_scheduled() { - $this->notification_service->expects( $this->once() )->method( 'is_enabled' )->willReturn( true ); - - $this->action_scheduler->expects( $this->once() ) - ->method( 'has_scheduled_action' ) - ->with( self::PROCESS_ITEM_HOOK, [] ) - ->willReturn( true ); - - $this->action_scheduler->expects( $this->never() )->method( 'schedule_immediate' ); - - $this->job->schedule(); - } - - public function test_schedule_doesnt_schedules_immediate_job_if_not_enabled() { - $this->notification_service->expects( $this->once() )->method( 'is_enabled' )->willReturn( false ); - - $this->action_scheduler->expects( $this->never() ) - ->method( 'has_scheduled_action' ); - - $this->action_scheduler->expects( $this->never() )->method( 'schedule_immediate' ); - - $this->job->schedule(); - } - - public function test_process_items_calls_notify() { - $this->notification_service->expects( $this->once() ) - ->method( 'notify' ) - ->with( NotificationsService::TOPIC_SHIPPING_UPDATED ) - ->willReturn( true ); - - $this->job->handle_process_items_action(); } } From ffdd04b4a650894743e167f3f31ca05fd2fef37d Mon Sep 17 00:00:00 2001 From: Miguel Perez Pellicer <5908855+puntope@users.noreply.github.com> Date: Thu, 22 Feb 2024 19:25:36 -0300 Subject: [PATCH 032/262] PHPCS --- src/Jobs/Notifications/AbstractNotificationJob.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Jobs/Notifications/AbstractNotificationJob.php b/src/Jobs/Notifications/AbstractNotificationJob.php index 22cc88c795..65356ad322 100644 --- a/src/Jobs/Notifications/AbstractNotificationJob.php +++ b/src/Jobs/Notifications/AbstractNotificationJob.php @@ -38,7 +38,7 @@ abstract class AbstractNotificationJob extends AbstractActionSchedulerJob implem public function __construct( ActionSchedulerInterface $action_scheduler, ActionSchedulerJobMonitor $monitor, - NotificationsService $notifications_service, + NotificationsService $notifications_service ) { $this->notifications_service = $notifications_service; parent::__construct( $action_scheduler, $monitor ); From cffa00179111977b4dbaf1aa669615a3d07234bf Mon Sep 17 00:00:00 2001 From: Miguel Perez Pellicer <5908855+puntope@users.noreply.github.com> Date: Fri, 23 Feb 2024 17:20:31 -0300 Subject: [PATCH 033/262] Add copuons notifications --- src/Coupon/CouponHelper.php | 93 +++++++ src/Coupon/CouponMetaHandler.php | 7 + src/Coupon/SyncerHooks.php | 59 +++- .../JobServiceProvider.php | 9 + .../Notifications/CouponNotificationJob.php | 162 +++++++++++ src/Product/ProductHelper.php | 2 +- src/Product/SyncerHooks.php | 1 - tests/Unit/Jobs/CouponNotificationJobTest.php | 260 ++++++++++++++++++ tests/bootstrap.php | 1 + 9 files changed, 587 insertions(+), 7 deletions(-) create mode 100644 src/Jobs/Notifications/CouponNotificationJob.php create mode 100644 tests/Unit/Jobs/CouponNotificationJobTest.php diff --git a/src/Coupon/CouponHelper.php b/src/Coupon/CouponHelper.php index 3c46644d82..8eed04ca78 100644 --- a/src/Coupon/CouponHelper.php +++ b/src/Coupon/CouponHelper.php @@ -8,6 +8,7 @@ use Automattic\WooCommerce\GoogleListingsAndAds\PluginHelper; use Automattic\WooCommerce\GoogleListingsAndAds\Proxies\WC; use Automattic\WooCommerce\GoogleListingsAndAds\Value\ChannelVisibility; +use Automattic\WooCommerce\GoogleListingsAndAds\Value\NotificationStatus; use Automattic\WooCommerce\GoogleListingsAndAds\Value\SyncStatus; use WC_Coupon; defined( 'ABSPATH' ) || exit(); @@ -321,4 +322,96 @@ public function get_validation_errors( WC_Coupon $coupon ): array { return $errors; } + + /** + * Indicates if a coupon is ready for sending Notifications. + * A coupon is ready to send notifications if its sync ready and the post status is publish. + * + * @param WC_Coupon $coupon + * + * @return bool + */ + public function is_ready_to_notify( WC_Coupon $coupon ): bool { + $is_ready = $this->is_sync_ready( $coupon ) && $coupon->get_status() === 'publish'; + + /** + * Allow users to filter if a coupon is ready to notify. + * + * @since x.x.x + * + * @param bool $value The current filter value. + * @param WC_Coupon $coupon The coupon for the notification. + */ + return apply_filters( 'woocommerce_gla_coupon_is_ready_to_notify', $is_ready, $coupon ); + } + + + /** + * Indicates if a coupon was already notified about its creation. + * Notice we consider synced coupons in MC as notified for creation. + * + * @param WC_Coupon $coupon + * + * @return bool + */ + public function has_notified_creation( WC_Coupon $coupon ): bool { + $valid_has_notified_creation_statuses = [ + NotificationStatus::NOTIFICATION_CREATED, + NotificationStatus::NOTIFICATION_UPDATED, + NotificationStatus::NOTIFICATION_PENDING_UPDATE, + NotificationStatus::NOTIFICATION_PENDING_DELETE, + ]; + + return in_array( + $this->meta_handler->get_notification_status( $coupon ), + $valid_has_notified_creation_statuses, + true + ) || $this->is_coupon_synced( $coupon ); + } + + /** + * Set the notification status for a WooCommerce coupon. + * + * @param WC_Coupon $coupon + * @param string $status + */ + public function set_notification_status( WC_Coupon $coupon, $status ): void { + $this->meta_handler->update_notification_status( $coupon, $status ); + } + + /** + * Indicates if a coupon is ready for sending a create Notification. + * A coupon is ready to send create notifications if is ready to notify and has not sent create notification yet. + * + * @param WC_Coupon $coupon + * + * @return bool + */ + public function should_trigger_create_notification( WC_Coupon $coupon ): bool { + return $this->is_ready_to_notify( $coupon ) && ! $this->has_notified_creation( $coupon ); + } + + /** + * Indicates if a coupon is ready for sending an update Notification. + * A coupon is ready to send update notifications if is ready to notify and has sent create notification already. + * + * @param WC_Coupon $coupon + * + * @return bool + */ + public function should_trigger_update_notification( WC_Coupon $coupon ): bool { + return $this->is_ready_to_notify( $coupon ) && $this->has_notified_creation( $coupon ); + } + + /** + * Indicates if a coupon is ready for sending a delete Notification. + * A coupon is ready to send delete notifications if it is not ready to notify and has sent create notification already. + * + * @param WC_Coupon $coupon + * + * @return bool + */ + public function should_trigger_delete_notification( WC_Coupon $coupon ): bool { + return ! $this->is_ready_to_notify( $coupon ) && $this->has_notified_creation( $coupon ); + } } diff --git a/src/Coupon/CouponMetaHandler.php b/src/Coupon/CouponMetaHandler.php index 81bbf405a9..a296f2e632 100644 --- a/src/Coupon/CouponMetaHandler.php +++ b/src/Coupon/CouponMetaHandler.php @@ -38,6 +38,9 @@ * @method update_mc_status( WC_Coupon $coupon, string $value ) * @method delete_mc_status( WC_Coupon $coupon ) * @method get_mc_status( WC_Coupon $coupon ): string|null + * @method update_notification_status( WC_Coupon $coupon, string $value ) + * @method delete_notification_status( WC_Coupon $coupon ) + * @method get_notification_status( WC_Coupon $coupon ): string|null */ class CouponMetaHandler implements Service { @@ -59,6 +62,9 @@ class CouponMetaHandler implements Service { public const KEY_MC_STATUS = 'mc_status'; + public const KEY_NOTIFICATION_STATUS = 'notification_status'; + + protected const TYPES = [ self::KEY_SYNCED_AT => 'int', self::KEY_GOOGLE_IDS => 'array', @@ -68,6 +74,7 @@ class CouponMetaHandler implements Service { self::KEY_SYNC_FAILED_AT => 'int', self::KEY_SYNC_STATUS => 'string', self::KEY_MC_STATUS => 'string', + self::KEY_NOTIFICATION_STATUS => 'string', ]; /** diff --git a/src/Coupon/SyncerHooks.php b/src/Coupon/SyncerHooks.php index 4eb1855a07..1be8687152 100644 --- a/src/Coupon/SyncerHooks.php +++ b/src/Coupon/SyncerHooks.php @@ -3,14 +3,17 @@ namespace Automattic\WooCommerce\GoogleListingsAndAds\Coupon; use Automattic\WooCommerce\GoogleListingsAndAds\Google\DeleteCouponEntry; +use Automattic\WooCommerce\GoogleListingsAndAds\Google\NotificationsService; use Automattic\WooCommerce\GoogleListingsAndAds\Infrastructure\Registerable; use Automattic\WooCommerce\GoogleListingsAndAds\Infrastructure\Service; use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\JobRepository; use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\DeleteCoupon; +use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\Notifications\CouponNotificationJob; use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\UpdateCoupon; use Automattic\WooCommerce\GoogleListingsAndAds\MerchantCenter\MerchantCenterService; use Automattic\WooCommerce\GoogleListingsAndAds\PluginHelper; use Automattic\WooCommerce\GoogleListingsAndAds\Proxies\WC; +use Automattic\WooCommerce\GoogleListingsAndAds\Value\NotificationStatus; use WC_Coupon; defined( 'ABSPATH' ) || exit(); @@ -63,12 +66,23 @@ class SyncerHooks implements Service, Registerable { */ protected $delete_coupon_job; + /** + * @var CouponNotificationJob + */ + protected $coupon_notification_job; + /** * * @var MerchantCenterService */ protected $merchant_center; + /** + * @var NotificationsService + */ + protected $notifications_service; + + /** * * @var WC @@ -81,19 +95,23 @@ class SyncerHooks implements Service, Registerable { * @param CouponHelper $coupon_helper * @param JobRepository $job_repository * @param MerchantCenterService $merchant_center + * @param NotificationsService $notifications_service * @param WC $wc */ public function __construct( CouponHelper $coupon_helper, JobRepository $job_repository, MerchantCenterService $merchant_center, + NotificationsService $notifications_service, WC $wc ) { - $this->update_coupon_job = $job_repository->get( UpdateCoupon::class ); - $this->delete_coupon_job = $job_repository->get( DeleteCoupon::class ); - $this->coupon_helper = $coupon_helper; - $this->merchant_center = $merchant_center; - $this->wc = $wc; + $this->update_coupon_job = $job_repository->get( UpdateCoupon::class ); + $this->delete_coupon_job = $job_repository->get( DeleteCoupon::class ); + $this->coupon_notification_job = $job_repository->get( CouponNotificationJob::class ); + $this->coupon_helper = $coupon_helper; + $this->merchant_center = $merchant_center; + $this->notifications_service = $notifications_service; + $this->wc = $wc; } /** @@ -153,6 +171,11 @@ public function register(): void { protected function handle_update_coupon( WC_Coupon $coupon ) { $coupon_id = $coupon->get_id(); + if ( $this->notifications_service->is_enabled() ) { + $this->handle_update_coupon_notification( $coupon ); + return; + } + // Schedule an update job if product sync is enabled. if ( $this->coupon_helper->is_sync_ready( $coupon ) ) { $this->coupon_helper->mark_as_pending( $coupon ); @@ -232,6 +255,14 @@ protected function get_coupon_to_delete( WC_Coupon $coupon ): WCCouponAdapter { * @param int $coupon_id */ protected function handle_delete_coupon( int $coupon_id ) { + $coupon = $this->wc->maybe_get_coupon( $coupon_id ); + + if ( $coupon instanceof WC_Coupon && $this->notifications_service->is_enabled() && $this->coupon_helper->should_trigger_delete_notification( $coupon ) ) { + $this->coupon_helper->set_notification_status( $coupon, NotificationStatus::NOTIFICATION_PENDING_DELETE ); + $this->coupon_notification_job->schedule( [ $coupon->get_id(), NotificationsService::TOPIC_COUPON_DELETED ] ); + return; + } + if ( ! isset( $this->delete_requests_map[ $coupon_id ] ) ) { return; } @@ -322,4 +353,22 @@ protected function set_already_scheduled_to_update( int $coupon_id ): void { protected function set_already_scheduled_to_delete( int $coupon_id ): void { $this->set_already_scheduled( $coupon_id, self::SCHEDULE_TYPE_DELETE ); } + + /** + * Schedules notifications for an updated product + * + * @param WC_Coupon $coupon + */ + protected function handle_update_coupon_notification( WC_Coupon $coupon ) { + if ( $this->coupon_helper->should_trigger_create_notification( $coupon ) ) { + $this->coupon_helper->set_notification_status( $coupon, NotificationStatus::NOTIFICATION_PENDING_CREATE ); + $this->coupon_notification_job->schedule( [ $coupon->get_id(), NotificationsService::TOPIC_COUPON_CREATED ] ); + } elseif ( $this->coupon_helper->should_trigger_update_notification( $coupon ) ) { + $this->coupon_helper->set_notification_status( $coupon, NotificationStatus::NOTIFICATION_PENDING_UPDATE ); + $this->coupon_notification_job->schedule( [ $coupon->get_id(), NotificationsService::TOPIC_COUPON_UPDATED ] ); + } elseif ( $this->coupon_helper->should_trigger_delete_notification( $coupon ) ) { + $this->coupon_helper->set_notification_status( $coupon, NotificationStatus::NOTIFICATION_PENDING_DELETE ); + $this->coupon_notification_job->schedule( [ $coupon->get_id(), NotificationsService::TOPIC_COUPON_DELETED ] ); + } + } } diff --git a/src/Internal/DependencyManagement/JobServiceProvider.php b/src/Internal/DependencyManagement/JobServiceProvider.php index 5dc54a6168..17bc0ebd3c 100644 --- a/src/Internal/DependencyManagement/JobServiceProvider.php +++ b/src/Internal/DependencyManagement/JobServiceProvider.php @@ -23,6 +23,7 @@ use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\JobInitializer; use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\JobInterface; use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\JobRepository; +use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\Notifications\CouponNotificationJob; use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\Notifications\ProductNotificationJob; use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\Notifications\SettingsNotificationJob; use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\Notifications\ShippingNotificationJob; @@ -117,6 +118,13 @@ public function register(): void { ProductHelper::class ); + // share coupon notifications job + $this->share_action_scheduler_job( + CouponNotificationJob::class, + NotificationsService::class, + CouponHelper::class + ); + $this->share_with_tags( JobRepository::class, JobInterface::class @@ -142,6 +150,7 @@ public function register(): void { CouponHelper::class, JobRepository::class, MerchantCenterService::class, + NotificationsService::class, WC::class ); diff --git a/src/Jobs/Notifications/CouponNotificationJob.php b/src/Jobs/Notifications/CouponNotificationJob.php new file mode 100644 index 0000000000..b31edec5bb --- /dev/null +++ b/src/Jobs/Notifications/CouponNotificationJob.php @@ -0,0 +1,162 @@ +notifications_service = $notifications_service; + $this->coupon_helper = $coupon_helper; + parent::__construct( $action_scheduler, $monitor ); + } + + /** + * Get the job name + * + * @return string + */ + public function get_name(): string { + return 'notifications/coupons'; + } + + + /** + * Logic when processing the items + * + * @param array $args Arguments with the item id and the topic + */ + protected function process_items( array $args ): void { + if ( ! isset( $args[0] ) || ! isset( $args[1] ) ) { + return; + } + + $item = $args[0]; + $topic = $args[1]; + + if ( $this->can_process( $item, $topic ) && $this->notifications_service->notify( $topic, $item ) ) { + $this->set_status( $item, $this->get_after_notification_status( $topic ) ); + } + } + + /** + * Schedule the Coupon Notification Job + * + * @param array $args + */ + public function schedule( array $args = [] ): void { + if ( $this->can_schedule( [ $args ] ) ) { + $this->action_scheduler->schedule_immediate( + $this->get_process_item_hook(), + [ $args ] + ); + } + } + + /** + * Set the notification status for a coupon. + * + * @param int $coupon_id + * @param string $status + */ + protected function set_status( int $coupon_id, string $status ): void { + $coupon = $this->coupon_helper->get_wc_coupon( $coupon_id ); + $this->coupon_helper->set_notification_status( $coupon, $status ); + } + + /** + * Get the Notification Status after the notification happens + * + * @param string $topic + * @return string + */ + protected function get_after_notification_status( string $topic ): string { + if ( str_contains( $topic, '.create' ) ) { + return NotificationStatus::NOTIFICATION_CREATED; + } elseif ( str_contains( $topic, '.delete' ) ) { + return NotificationStatus::NOTIFICATION_DELETED; + } else { + return NotificationStatus::NOTIFICATION_UPDATED; + } + } + + /** + * Checks if the item can be processed based on the topic. + * This is needed because the coupon can change the Notification Status before + * the Job process the item. + * + * @param int $coupon_id + * @param string $topic + * @return bool + */ + protected function can_process( int $coupon_id, string $topic ): bool { + $coupon = $this->coupon_helper->get_wc_coupon( $coupon_id ); + if ( str_contains( $topic, '.create' ) ) { + return $this->coupon_helper->should_trigger_create_notification( $coupon ); + } elseif ( str_contains( $topic, '.delete' ) ) { + return $this->coupon_helper->should_trigger_delete_notification( $coupon ); + } else { + return $this->coupon_helper->should_trigger_update_notification( $coupon ); + } + } + + /** + * Can the job be scheduled. + * + * @param array|null $args + * + * @return bool Returns true if the job can be scheduled. + */ + public function can_schedule( $args = [] ): bool { + /** + * Allow users to disable the notification job schedule. + * + * @since x.x.x + * + * @param bool $value The current filter value. By default, it is the result of `$this->can_schedule` function. + * @param array $args The arguments for the schedule function with the item id and the topic. + */ + return apply_filters( 'woocommerce_gla_coupon_notification_job_can_schedule', $this->notifications_service->is_enabled() && parent::can_schedule( $args ), $args ); + } +} diff --git a/src/Product/ProductHelper.php b/src/Product/ProductHelper.php index 8a1c2611eb..7375ee73ce 100644 --- a/src/Product/ProductHelper.php +++ b/src/Product/ProductHelper.php @@ -354,7 +354,7 @@ public function is_ready_to_notify( WC_Product $product ): bool { * @param bool $value The current filter value. * @param WC_Product $product The product for the notification. */ - return apply_filters( 'woocommerce_gla_is_ready_to_notify', $is_ready, $product ); + return apply_filters( 'woocommerce_gla_product_is_ready_to_notify', $is_ready, $product ); } /** diff --git a/src/Product/SyncerHooks.php b/src/Product/SyncerHooks.php index c398fd4965..75ce19a514 100644 --- a/src/Product/SyncerHooks.php +++ b/src/Product/SyncerHooks.php @@ -121,7 +121,6 @@ public function __construct( */ public function register(): void { // only register the hooks if Merchant Center is connected and ready for syncing data. - // TODO: Potentially change this after API Pull is implemented as we don't need MC to be connected for the API Pull if ( ! $this->merchant_center->is_ready_for_syncing() ) { return; } diff --git a/tests/Unit/Jobs/CouponNotificationJobTest.php b/tests/Unit/Jobs/CouponNotificationJobTest.php new file mode 100644 index 0000000000..88d6ed99f3 --- /dev/null +++ b/tests/Unit/Jobs/CouponNotificationJobTest.php @@ -0,0 +1,260 @@ +action_scheduler = $this->createMock( ActionScheduler::class ); + $this->monitor = $this->createMock( ActionSchedulerJobMonitor::class ); + $this->notification_service = $this->createMock( NotificationsService::class ); + $this->coupon_helper = $this->createMock( CouponHelper::class ); + $this->job = new CouponNotificationJob( + $this->action_scheduler, + $this->monitor, + $this->notification_service, + $this->coupon_helper + ); + + $this->job->init(); + } + + public function test_job_name() { + $this->assertEquals( self::JOB_NAME, $this->job->get_name() ); + } + + public function test_schedule_schedules_immediate_job() { + $topic = 'coupon.create'; + $id = 1; + + $this->notification_service->expects( $this->once() )->method( 'is_enabled' )->willReturn( true ); + + $this->action_scheduler->expects( $this->once() ) + ->method( 'has_scheduled_action' ) + ->with( self::PROCESS_ITEM_HOOK, [ [ $id, $topic ] ] ) + ->willReturn( false ); + + $this->action_scheduler->expects( $this->once() ) + ->method( 'schedule_immediate' ) + ->with( + self::PROCESS_ITEM_HOOK, + [ [ $id, $topic ] ] + ); + + $this->job->schedule( [ $id, $topic ] ); + } + + public function test_schedule_doesnt_schedules_immediate_job_if_already_scheduled() { + $id = 1; + $topic = 'coupon.create'; + + $this->notification_service->expects( $this->once() )->method( 'is_enabled' )->willReturn( true ); + + $this->action_scheduler->expects( $this->once() ) + ->method( 'has_scheduled_action' ) + ->with( self::PROCESS_ITEM_HOOK, [ [ $id, $topic ] ] ) + ->willReturn( true ); + + $this->action_scheduler->expects( $this->never() )->method( 'schedule_immediate' ); + + $this->job->schedule( [ $id, $topic ] ); + } + + public function test_schedule_doesnt_schedules_immediate_job_if_not_enabled() { + $id = 1; + $topic = 'coupon.create'; + + $this->notification_service->expects( $this->once() )->method( 'is_enabled' )->willReturn( false ); + + $this->action_scheduler->expects( $this->never() ) + ->method( 'has_scheduled_action' ); + + $this->action_scheduler->expects( $this->never() )->method( 'schedule_immediate' ); + + $this->job->schedule( [ $id, $topic ] ); + } + + public function test_process_items_calls_notify_and_set_status_on_success() { + /** @var \WC_Coupon $coupon */ + $coupon = WC_Helper_Coupon::create_coupon(); + $id = $coupon->get_id(); + $topic = 'coupon.create'; + + $this->notification_service->expects( $this->once() ) + ->method( 'notify' ) + ->with( $topic, $id ) + ->willReturn( true ); + + $this->coupon_helper->expects( $this->exactly( 2 ) ) + ->method( 'get_wc_coupon' ) + ->with( $id ) + ->willReturn( $coupon ); + + $this->coupon_helper->expects( $this->once() ) + ->method( 'should_trigger_create_notification' ) + ->with( $coupon ) + ->willReturn( true ); + + $this->coupon_helper->expects( $this->once() ) + ->method( 'set_notification_status' ) + ->with( $coupon, NotificationStatus::NOTIFICATION_CREATED ); + + $this->job->handle_process_items_action( [ $id, $topic ] ); + } + + public function test_process_items_doesnt_calls_notify_when_no_args() { + $this->notification_service->expects( $this->never() ) + ->method( 'notify' ); + + $this->job->handle_process_items_action( [] ); + $this->job->handle_process_items_action( [ 1 ] ); + } + + public function test_process_items_doesnt_calls_set_status_on_failure() { + /** @var \WC_Coupon $coupon */ + $coupon = WC_Helper_Coupon::create_coupon(); + $id = $coupon->get_id(); + $topic = 'coupon.create'; + + $this->notification_service->expects( $this->once() ) + ->method( 'notify' ) + ->with( $topic, $id ) + ->willReturn( false ); + + $this->coupon_helper->expects( $this->once() ) + ->method( 'get_wc_coupon' ) + ->with( $id ) + ->willReturn( $coupon ); + + $this->coupon_helper->expects( $this->once() ) + ->method( 'should_trigger_create_notification' ) + ->with( $coupon ) + ->willReturn( true ); + + $this->coupon_helper->expects( $this->never() ) + ->method( 'set_notification_status' ); + + $this->job->handle_process_items_action( [ $id, $topic ] ); + } + + public function test_get_after_notification_status() { + /** @var \WC_Coupon $coupon */ + $coupon = WC_Helper_Coupon::create_coupon(); + $id = $coupon->get_id(); + + $this->notification_service->expects( $this->exactly( 3 ) ) + ->method( 'notify' ) + ->willReturn( true ); + + $this->coupon_helper->expects( $this->exactly( 6 ) ) + ->method( 'get_wc_coupon' ) + ->willReturn( $coupon ); + + $this->coupon_helper->expects( $this->once() ) + ->method( 'should_trigger_create_notification' ) + ->with( $coupon ) + ->willReturn( true ); + + $this->coupon_helper->expects( $this->once() ) + ->method( 'should_trigger_update_notification' ) + ->with( $coupon ) + ->willReturn( true ); + + $this->coupon_helper->expects( $this->once() ) + ->method( 'should_trigger_delete_notification' ) + ->with( $coupon ) + ->willReturn( true ); + + $this->coupon_helper->expects( $this->exactly( 3 ) ) + ->method( 'set_notification_status' ) + ->willReturnCallback( + function ( $id, $topic ) { + if ( $topic === 'coupon.create' ) { + return NotificationStatus::NOTIFICATION_CREATED; + } elseif ( $topic === 'coupon.delete' ) { + return NotificationStatus::NOTIFICATION_DELETED; + } else { + return NotificationStatus::NOTIFICATION_UPDATED; + } + } + ); + + $this->job->handle_process_items_action( [ $id, 'coupon.create' ] ); + $this->job->handle_process_items_action( [ $id, 'coupon.delete' ] ); + $this->job->handle_process_items_action( [ $id, 'coupon.update' ] ); + } + + public function test_dont_process_item_if_status_changed() { + /** @var \WC_Coupon $coupon */ + $coupon = WC_Helper_Coupon::create_coupon(); + $id = $coupon->get_id(); + + $this->notification_service->expects( $this->never() )->method( 'notify' ); + + $this->coupon_helper->expects( $this->exactly( 3 ) ) + ->method( 'get_wc_coupon' ) + ->willReturn( $coupon ); + + $this->coupon_helper->expects( $this->once() ) + ->method( 'should_trigger_create_notification' ) + ->with( $coupon ) + ->willReturn( false ); + + $this->coupon_helper->expects( $this->once() ) + ->method( 'should_trigger_update_notification' ) + ->with( $coupon ) + ->willReturn( false ); + + $this->coupon_helper->expects( $this->once() ) + ->method( 'should_trigger_delete_notification' ) + ->with( $coupon ) + ->willReturn( false ); + + $this->coupon_helper->expects( $this->never() )->method( 'set_notification_status' ); + + $this->job->handle_process_items_action( [ $id, 'coupon.create' ] ); + $this->job->handle_process_items_action( [ $id, 'coupon.delete' ] ); + $this->job->handle_process_items_action( [ $id, 'coupon.update' ] ); + } +} diff --git a/tests/bootstrap.php b/tests/bootstrap.php index cb098d11af..d0b62f5072 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -60,6 +60,7 @@ function () { $wc_tests_dir .= '/legacy'; } +require_once $wc_tests_dir . '/framework/helpers/class-wc-helper-coupon.php'; require_once $wc_tests_dir . '/framework/helpers/class-wc-helper-product.php'; require_once $wc_tests_dir . '/framework/helpers/class-wc-helper-shipping.php'; require_once $wc_tests_dir . '/framework/helpers/class-wc-helper-customer.php'; From 6d6ed016ee82b1d6b6436fcf7306d636ef175d57 Mon Sep 17 00:00:00 2001 From: Miguel Perez Pellicer <5908855+puntope@users.noreply.github.com> Date: Fri, 23 Feb 2024 17:25:24 -0300 Subject: [PATCH 034/262] Add error log --- src/Jobs/Notifications/CouponNotificationJob.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/Jobs/Notifications/CouponNotificationJob.php b/src/Jobs/Notifications/CouponNotificationJob.php index b31edec5bb..954ed10c29 100644 --- a/src/Jobs/Notifications/CouponNotificationJob.php +++ b/src/Jobs/Notifications/CouponNotificationJob.php @@ -69,6 +69,11 @@ public function get_name(): string { */ protected function process_items( array $args ): void { if ( ! isset( $args[0] ) || ! isset( $args[1] ) ) { + do_action( + 'woocommerce_gla_error', + 'Error sending Coupon Notification. Topic and Items are mandatory', + __METHOD__ + ); return; } From dfc7a7f7db316d3075fec2f4940bbb7516edbf55 Mon Sep 17 00:00:00 2001 From: Miguel Perez Pellicer <5908855+puntope@users.noreply.github.com> Date: Fri, 23 Feb 2024 17:42:30 -0300 Subject: [PATCH 035/262] Fix typo --- src/Jobs/Notifications/CouponNotificationJob.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Jobs/Notifications/CouponNotificationJob.php b/src/Jobs/Notifications/CouponNotificationJob.php index 954ed10c29..42f63d3ed7 100644 --- a/src/Jobs/Notifications/CouponNotificationJob.php +++ b/src/Jobs/Notifications/CouponNotificationJob.php @@ -71,7 +71,7 @@ protected function process_items( array $args ): void { if ( ! isset( $args[0] ) || ! isset( $args[1] ) ) { do_action( 'woocommerce_gla_error', - 'Error sending Coupon Notification. Topic and Items are mandatory', + 'Error sending Coupon Notification. Topic and Coupon ID are mandatory', __METHOD__ ); return; From b719d76c7670a1d786af7d59c90d50b28d96b3bc Mon Sep 17 00:00:00 2001 From: Miguel Perez Pellicer <5908855+puntope@users.noreply.github.com> Date: Fri, 23 Feb 2024 18:26:08 -0300 Subject: [PATCH 036/262] Add tests --- tests/Unit/Coupon/SyncerHooksTest.php | 61 ++++++++++++++++++++++++++- 1 file changed, 60 insertions(+), 1 deletion(-) diff --git a/tests/Unit/Coupon/SyncerHooksTest.php b/tests/Unit/Coupon/SyncerHooksTest.php index 672f251e84..740016c1c1 100644 --- a/tests/Unit/Coupon/SyncerHooksTest.php +++ b/tests/Unit/Coupon/SyncerHooksTest.php @@ -6,20 +6,25 @@ use Automattic\WooCommerce\GoogleListingsAndAds\Coupon\SyncerHooks; use Automattic\WooCommerce\GoogleListingsAndAds\Coupon\WCCouponAdapter; use Automattic\WooCommerce\GoogleListingsAndAds\Google\DeleteCouponEntry; +use Automattic\WooCommerce\GoogleListingsAndAds\Google\NotificationsService; use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\DeleteCoupon; +use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\Notifications\CouponNotificationJob; use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\UpdateCoupon; use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\JobRepository; use Automattic\WooCommerce\GoogleListingsAndAds\MerchantCenter\MerchantCenterService; use Automattic\WooCommerce\GoogleListingsAndAds\Proxies\WC; use Automattic\WooCommerce\GoogleListingsAndAds\Tests\Framework\ContainerAwareUnitTest; use Automattic\WooCommerce\GoogleListingsAndAds\Tests\Tools\HelperTrait\CouponTrait; +use Automattic\WooCommerce\GoogleListingsAndAds\Value\ChannelVisibility; +use Automattic\WooCommerce\GoogleListingsAndAds\Value\NotificationStatus; use PHPUnit\Framework\MockObject\MockObject; use WC_Coupon; +use WC_Helper_Coupon; /** * Class SyncerHooksTest * - * @package Automattic\WooCommerce\GoogleListingsAndAds\Tests\Unit\Product + * @package Automattic\WooCommerce\GoogleListingsAndAds\Tests\Unit\Coupon */ class SyncerHooksTest extends ContainerAwareUnitTest { @@ -37,6 +42,12 @@ class SyncerHooksTest extends ContainerAwareUnitTest { /** @var MockObject|UpdateCoupon $update_coupon_job */ protected $update_coupon_job; + /** @var MockObject|CouponNotificationJob $coupon_notification_job */ + protected $coupon_notification_job; + + /** @var MockObject|NotificationsService $notification_service */ + protected $notification_service; + /** @var CouponHelper $coupon_helper */ protected $coupon_helper; @@ -160,6 +171,47 @@ public function test_modify_post_does_not_schedule_update_job() { wp_untrash_post( $post->ID ); } + public function test_create_coupon_triggers_notification_created() { + /** + * @var WC_Coupon $coupon + */ + $coupon = WC_Helper_Coupon::create_coupon( uniqid(), [ 'status' => 'draft' ] ); + $this->notification_service->expects( $this->once() )->method( 'is_enabled' )->willReturn( true ); + $this->coupon_notification_job->expects( $this->once() ) + ->method( 'schedule' )->with( $this->equalTo( [ $coupon->get_id(), NotificationsService::TOPIC_COUPON_CREATED ] ) ); + $coupon->set_status( 'publish' ); + $coupon->add_meta_data( '_wc_gla_visibility', ChannelVisibility::SYNC_AND_SHOW, true ); + $coupon->save(); + } + + public function test_update_coupon_triggers_notification_updated() { + /** + * @var WC_Coupon $coupon + */ + $coupon = WC_Helper_Coupon::create_coupon( uniqid() ); + $this->notification_service->expects( $this->once() )->method( 'is_enabled' )->willReturn( true ); + $this->coupon_notification_job->expects( $this->once() ) + ->method( 'schedule' )->with( $this->equalTo( [ $coupon->get_id(), NotificationsService::TOPIC_COUPON_UPDATED ] ) ); + $this->coupon_helper->set_notification_status( $coupon, NotificationStatus::NOTIFICATION_CREATED ); + $coupon->set_status( 'publish' ); + $coupon->add_meta_data( '_wc_gla_visibility', ChannelVisibility::SYNC_AND_SHOW, true ); + $coupon->save(); + } + + public function test_delete_coupon_triggers_notification_delete() { + /** + * @var WC_Coupon $coupon + */ + $coupon = WC_Helper_Coupon::create_coupon( uniqid() ); + $this->notification_service->expects( $this->once() )->method( 'is_enabled' )->willReturn( true ); + $this->coupon_notification_job->expects( $this->once() ) + ->method( 'schedule' )->with( $this->equalTo( [ $coupon->get_id(), NotificationsService::TOPIC_COUPON_DELETED ] ) ); + $coupon->set_status( 'publish' ); + $coupon->add_meta_data( '_wc_gla_visibility', ChannelVisibility::DONT_SYNC_AND_SHOW, true ); + $this->coupon_helper->set_notification_status( $coupon, NotificationStatus::NOTIFICATION_UPDATED ); + $coupon->save(); + } + /** * Runs before each test is executed. */ @@ -177,6 +229,10 @@ public function setUp(): void { $this->update_coupon_job = $this->createMock( UpdateCoupon::class ); $this->delete_coupon_job = $this->createMock( DeleteCoupon::class ); + $this->delete_coupon_job = $this->createMock( DeleteCoupon::class ); + $this->coupon_notification_job = $this->createMock( CouponNotificationJob::class ); + $this->notification_service = $this->createMock( NotificationsService::class ); + $this->job_repository = $this->createMock( JobRepository::class ); $this->job_repository->expects( $this->any() ) ->method( 'get' ) @@ -184,6 +240,7 @@ public function setUp(): void { [ [ DeleteCoupon::class, $this->delete_coupon_job ], [ UpdateCoupon::class, $this->update_coupon_job ], + [ CouponNotificationJob::class, $this->coupon_notification_job ], ] ); @@ -193,9 +250,11 @@ public function setUp(): void { $this->coupon_helper, $this->job_repository, $this->merchant_center, + $this->notification_service, $this->wc ); + add_filter( 'woocommerce_gla_notifications_enabled', '__return_false' ); $this->syncer_hooks->register(); } } From 82ecd7a727460f3da3b732e3f3731dd504bdb825 Mon Sep 17 00:00:00 2001 From: Miguel Perez Pellicer <5908855+puntope@users.noreply.github.com> Date: Fri, 23 Feb 2024 18:26:53 -0300 Subject: [PATCH 037/262] PHPCS --- tests/Unit/Coupon/SyncerHooksTest.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/Unit/Coupon/SyncerHooksTest.php b/tests/Unit/Coupon/SyncerHooksTest.php index 740016c1c1..d74bc10205 100644 --- a/tests/Unit/Coupon/SyncerHooksTest.php +++ b/tests/Unit/Coupon/SyncerHooksTest.php @@ -227,13 +227,13 @@ public function setUp(): void { ->method( 'is_ready_for_syncing' ) ->willReturn( true ); - $this->update_coupon_job = $this->createMock( UpdateCoupon::class ); - $this->delete_coupon_job = $this->createMock( DeleteCoupon::class ); - $this->delete_coupon_job = $this->createMock( DeleteCoupon::class ); + $this->update_coupon_job = $this->createMock( UpdateCoupon::class ); + $this->delete_coupon_job = $this->createMock( DeleteCoupon::class ); + $this->delete_coupon_job = $this->createMock( DeleteCoupon::class ); $this->coupon_notification_job = $this->createMock( CouponNotificationJob::class ); - $this->notification_service = $this->createMock( NotificationsService::class ); + $this->notification_service = $this->createMock( NotificationsService::class ); - $this->job_repository = $this->createMock( JobRepository::class ); + $this->job_repository = $this->createMock( JobRepository::class ); $this->job_repository->expects( $this->any() ) ->method( 'get' ) ->willReturnMap( From afdf43cb061f63fc9cc1c4e3a44b276cb1d7d712 Mon Sep 17 00:00:00 2001 From: Miguel Perez Pellicer <5908855+puntope@users.noreply.github.com> Date: Mon, 26 Feb 2024 11:49:42 -0300 Subject: [PATCH 038/262] Unsync Coupon when deletion happen --- .../Notifications/CouponNotificationJob.php | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/Jobs/Notifications/CouponNotificationJob.php b/src/Jobs/Notifications/CouponNotificationJob.php index 42f63d3ed7..eaa995c2ed 100644 --- a/src/Jobs/Notifications/CouponNotificationJob.php +++ b/src/Jobs/Notifications/CouponNotificationJob.php @@ -5,6 +5,7 @@ use Automattic\WooCommerce\GoogleListingsAndAds\ActionScheduler\ActionSchedulerInterface; use Automattic\WooCommerce\GoogleListingsAndAds\Coupon\CouponHelper; +use Automattic\WooCommerce\GoogleListingsAndAds\Exception\InvalidValue; use Automattic\WooCommerce\GoogleListingsAndAds\Google\NotificationsService; use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\AbstractActionSchedulerJob; use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\ActionSchedulerJobMonitor; @@ -82,6 +83,7 @@ protected function process_items( array $args ): void { if ( $this->can_process( $item, $topic ) && $this->notifications_service->notify( $topic, $item ) ) { $this->set_status( $item, $this->get_after_notification_status( $topic ) ); + $this->maybe_mark_as_unsynced( $topic, $item ); } } @@ -146,6 +148,25 @@ protected function can_process( int $coupon_id, string $topic ): bool { } } + /** + * If there is a valid Item ID and topic is a deletion topic. Mark the coupon as unsynced. + * + * @param string $topic + * @param int|null $item + */ + protected function maybe_mark_as_unsynced( string $topic, $item = null ): void { + if ( is_null( $item ) || ! str_contains( $topic, '.delete' ) ) { + return; + } + + try { + $coupon = $this->coupon_helper->get_wc_coupon( $item ); + $this->coupon_helper->mark_as_unsynced( $coupon ); + } catch ( InvalidValue $e ) { + return; + } + } + /** * Can the job be scheduled. * From 469410127336d070bfe5e7b38a491ecfd4a63434 Mon Sep 17 00:00:00 2001 From: Miguel Perez Pellicer <5908855+puntope@users.noreply.github.com> Date: Mon, 26 Feb 2024 12:15:20 -0300 Subject: [PATCH 039/262] add tests --- tests/Unit/Jobs/CouponNotificationJobTest.php | 25 ++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/tests/Unit/Jobs/CouponNotificationJobTest.php b/tests/Unit/Jobs/CouponNotificationJobTest.php index 88d6ed99f3..9c044cf57a 100644 --- a/tests/Unit/Jobs/CouponNotificationJobTest.php +++ b/tests/Unit/Jobs/CouponNotificationJobTest.php @@ -187,7 +187,7 @@ public function test_get_after_notification_status() { ->method( 'notify' ) ->willReturn( true ); - $this->coupon_helper->expects( $this->exactly( 6 ) ) + $this->coupon_helper->expects( $this->exactly( 7 ) ) ->method( 'get_wc_coupon' ) ->willReturn( $coupon ); @@ -257,4 +257,27 @@ public function test_dont_process_item_if_status_changed() { $this->job->handle_process_items_action( [ $id, 'coupon.delete' ] ); $this->job->handle_process_items_action( [ $id, 'coupon.update' ] ); } + + public function test_mark_as_unsynced_when_delete() { + /** @var \WC_Coupon $coupon */ + $coupon = WC_Helper_Coupon::create_coupon(); + $id = $coupon->get_id(); + + $this->coupon_helper->expects( $this->once() ) + ->method( 'should_trigger_delete_notification' ) + ->with( $coupon ) + ->willReturn( true ); + + $this->coupon_helper->expects( $this->exactly( 3 ) ) + ->method( 'get_wc_coupon' ) + ->with( $id ) + ->willReturn( $coupon ); + + $this->notification_service->expects( $this->once() )->method( 'notify' )->willReturn( true ); + $this->coupon_helper->expects( $this->once() ) + ->method( 'mark_as_unsynced' ); + + + $this->job->handle_process_items_action( [ $id, 'coupon.delete' ] ); + } } From bb2f8e8a32e0b4b8307d8070679c83764887a346 Mon Sep 17 00:00:00 2001 From: Miguel Perez Pellicer <5908855+puntope@users.noreply.github.com> Date: Mon, 26 Feb 2024 12:19:27 -0300 Subject: [PATCH 040/262] PHPCS --- src/Jobs/Notifications/CouponNotificationJob.php | 4 ++-- tests/Unit/Jobs/CouponNotificationJobTest.php | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Jobs/Notifications/CouponNotificationJob.php b/src/Jobs/Notifications/CouponNotificationJob.php index eaa995c2ed..786247ed1e 100644 --- a/src/Jobs/Notifications/CouponNotificationJob.php +++ b/src/Jobs/Notifications/CouponNotificationJob.php @@ -151,11 +151,11 @@ protected function can_process( int $coupon_id, string $topic ): bool { /** * If there is a valid Item ID and topic is a deletion topic. Mark the coupon as unsynced. * - * @param string $topic + * @param string $topic * @param int|null $item */ protected function maybe_mark_as_unsynced( string $topic, $item = null ): void { - if ( is_null( $item ) || ! str_contains( $topic, '.delete' ) ) { + if ( is_null( $item ) || ! str_contains( $topic, '.delete' ) ) { return; } diff --git a/tests/Unit/Jobs/CouponNotificationJobTest.php b/tests/Unit/Jobs/CouponNotificationJobTest.php index 9c044cf57a..9c7537b8fa 100644 --- a/tests/Unit/Jobs/CouponNotificationJobTest.php +++ b/tests/Unit/Jobs/CouponNotificationJobTest.php @@ -277,7 +277,6 @@ public function test_mark_as_unsynced_when_delete() { $this->coupon_helper->expects( $this->once() ) ->method( 'mark_as_unsynced' ); - $this->job->handle_process_items_action( [ $id, 'coupon.delete' ] ); } } From 28357af7aecb4d0c77d2ffea2c7708f38a5c91e8 Mon Sep 17 00:00:00 2001 From: Miguel Perez Pellicer <5908855+puntope@users.noreply.github.com> Date: Mon, 26 Feb 2024 12:23:54 -0300 Subject: [PATCH 041/262] Add tests --- .../Unit/Jobs/ProductNotificationJobTest.php | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/Unit/Jobs/ProductNotificationJobTest.php b/tests/Unit/Jobs/ProductNotificationJobTest.php index 5b985ff50b..94fda847bf 100644 --- a/tests/Unit/Jobs/ProductNotificationJobTest.php +++ b/tests/Unit/Jobs/ProductNotificationJobTest.php @@ -257,4 +257,26 @@ public function test_dont_process_item_if_status_changed() { $this->job->handle_process_items_action( [ $id, 'product.delete' ] ); $this->job->handle_process_items_action( [ $id, 'product.update' ] ); } + + public function test_mark_as_unsynced_when_delete() { + /** @var \WC_Product $product */ + $product = WC_Helper_Product::create_simple_product(); + $id = $product->get_id(); + + $this->product_helper->expects( $this->once() ) + ->method( 'should_trigger_delete_notification' ) + ->with( $product ) + ->willReturn( true ); + + $this->product_helper->expects( $this->exactly( 3 ) ) + ->method( 'get_wc_product' ) + ->with( $id ) + ->willReturn( $product ); + + $this->notification_service->expects( $this->once() )->method( 'notify' )->willReturn( true ); + $this->product_helper->expects( $this->once() ) + ->method( 'mark_as_unsynced' ); + + $this->job->handle_process_items_action( [ $id, 'product.delete' ] ); + } } From d4656d8977207f371726164edea895f530ed2769 Mon Sep 17 00:00:00 2001 From: Miguel Perez Pellicer <5908855+puntope@users.noreply.github.com> Date: Mon, 26 Feb 2024 12:34:04 -0300 Subject: [PATCH 042/262] Mark product as unsycned when deletion happens --- .../Notifications/ProductNotificationJob.php | 16 ++++++++++++++++ tests/Unit/Jobs/ProductNotificationJobTest.php | 4 ++-- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/Jobs/Notifications/ProductNotificationJob.php b/src/Jobs/Notifications/ProductNotificationJob.php index ca421cbedc..7a6f5090bc 100644 --- a/src/Jobs/Notifications/ProductNotificationJob.php +++ b/src/Jobs/Notifications/ProductNotificationJob.php @@ -77,6 +77,7 @@ protected function process_items( array $args ): void { if ( $this->can_process( $item, $topic ) && $this->notifications_service->notify( $topic, $item ) ) { $this->set_status( $item, $this->get_after_notification_status( $topic ) ); + $this->maybe_mark_as_unsynced( $topic, $item ); } } @@ -121,6 +122,21 @@ protected function get_after_notification_status( string $topic ): string { } } + /** + * If there is a valid Item ID and topic is a deletion topic. Mark the coupon as unsynced. + * + * @param string $topic + * @param int $item + */ + protected function maybe_mark_as_unsynced( string $topic, int $item ): void { + if ( ! str_contains( $topic, '.delete' ) ) { + return; + } + + $product = $this->product_helper->get_wc_product( $item ); + $this->product_helper->mark_as_unsynced( $product ); + } + /** * Checks if the item can be processed based on the topic. * This is needed because the product can change the Notification Status before diff --git a/tests/Unit/Jobs/ProductNotificationJobTest.php b/tests/Unit/Jobs/ProductNotificationJobTest.php index 94fda847bf..7ff3caa805 100644 --- a/tests/Unit/Jobs/ProductNotificationJobTest.php +++ b/tests/Unit/Jobs/ProductNotificationJobTest.php @@ -187,7 +187,7 @@ public function test_get_after_notification_status() { ->method( 'notify' ) ->willReturn( true ); - $this->product_helper->expects( $this->exactly( 6 ) ) + $this->product_helper->expects( $this->exactly( 7 ) ) ->method( 'get_wc_product' ) ->willReturn( $product ); @@ -261,7 +261,7 @@ public function test_dont_process_item_if_status_changed() { public function test_mark_as_unsynced_when_delete() { /** @var \WC_Product $product */ $product = WC_Helper_Product::create_simple_product(); - $id = $product->get_id(); + $id = $product->get_id(); $this->product_helper->expects( $this->once() ) ->method( 'should_trigger_delete_notification' ) From fe8ca1f0f55235640e4cd6fbdccab5804955bafd Mon Sep 17 00:00:00 2001 From: Miguel Perez Pellicer <5908855+puntope@users.noreply.github.com> Date: Mon, 26 Feb 2024 12:36:16 -0300 Subject: [PATCH 043/262] Remove unnecessary check --- src/Jobs/Notifications/CouponNotificationJob.php | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/Jobs/Notifications/CouponNotificationJob.php b/src/Jobs/Notifications/CouponNotificationJob.php index 786247ed1e..7b89435c9b 100644 --- a/src/Jobs/Notifications/CouponNotificationJob.php +++ b/src/Jobs/Notifications/CouponNotificationJob.php @@ -152,19 +152,15 @@ protected function can_process( int $coupon_id, string $topic ): bool { * If there is a valid Item ID and topic is a deletion topic. Mark the coupon as unsynced. * * @param string $topic - * @param int|null $item + * @param int $item */ - protected function maybe_mark_as_unsynced( string $topic, $item = null ): void { - if ( is_null( $item ) || ! str_contains( $topic, '.delete' ) ) { + protected function maybe_mark_as_unsynced( string $topic, int $item ): void { + if ( ! str_contains( $topic, '.delete' ) ) { return; } - try { - $coupon = $this->coupon_helper->get_wc_coupon( $item ); - $this->coupon_helper->mark_as_unsynced( $coupon ); - } catch ( InvalidValue $e ) { - return; - } + $coupon = $this->coupon_helper->get_wc_coupon( $item ); + $this->coupon_helper->mark_as_unsynced( $coupon ); } /** From a73ad24eecf63bf5fef9aa303967948f1658236f Mon Sep 17 00:00:00 2001 From: Miguel Perez Pellicer <5908855+puntope@users.noreply.github.com> Date: Mon, 26 Feb 2024 12:43:22 -0300 Subject: [PATCH 044/262] PHPCS --- src/Jobs/Notifications/CouponNotificationJob.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Jobs/Notifications/CouponNotificationJob.php b/src/Jobs/Notifications/CouponNotificationJob.php index 7b89435c9b..6043e4f3da 100644 --- a/src/Jobs/Notifications/CouponNotificationJob.php +++ b/src/Jobs/Notifications/CouponNotificationJob.php @@ -151,8 +151,8 @@ protected function can_process( int $coupon_id, string $topic ): bool { /** * If there is a valid Item ID and topic is a deletion topic. Mark the coupon as unsynced. * - * @param string $topic - * @param int $item + * @param string $topic + * @param int $item */ protected function maybe_mark_as_unsynced( string $topic, int $item ): void { if ( ! str_contains( $topic, '.delete' ) ) { From 11fd1a1b61dc7bfb3af9ed75be34de91c62839ba Mon Sep 17 00:00:00 2001 From: Miguel Perez Pellicer <5908855+puntope@users.noreply.github.com> Date: Mon, 26 Feb 2024 19:08:56 -0300 Subject: [PATCH 045/262] Refactor coupon and products --- .../AbstractItemNotificationJob.php | 122 ++++++++ .../Notifications/CouponNotificationJob.php | 147 ++-------- .../Notifications/ProductNotificationJob.php | 97 ++----- .../Jobs/AbstractItemNotificationJobTest.php | 258 +++++++++++++++++ .../Unit/Jobs/AbstractNotificationJobTest.php | 4 +- tests/Unit/Jobs/CouponNotificationJobTest.php | 268 +----------------- .../Unit/Jobs/ProductNotificationJobTest.php | 245 +--------------- 7 files changed, 443 insertions(+), 698 deletions(-) create mode 100644 src/Jobs/Notifications/AbstractItemNotificationJob.php create mode 100644 tests/Unit/Jobs/AbstractItemNotificationJobTest.php diff --git a/src/Jobs/Notifications/AbstractItemNotificationJob.php b/src/Jobs/Notifications/AbstractItemNotificationJob.php new file mode 100644 index 0000000000..e32f5009dd --- /dev/null +++ b/src/Jobs/Notifications/AbstractItemNotificationJob.php @@ -0,0 +1,122 @@ +can_process( $item, $topic ) && $this->notifications_service->notify( $topic, $item ) ) { + $this->set_status( $item, $this->get_after_notification_status( $topic ) ); + $this->maybe_mark_as_unsynced( $topic, $item ); + } + } + + /** + * Set the notification status for the item. + * + * @param int $item_id + * @param string $status + */ + protected function set_status( int $item_id, string $status ): void { + $item = $this->get_item( $item_id ); + $this->get_helper()->set_notification_status( $item, $status ); + } + + /** + * Get the Notification Status after the notification happens + * + * @param string $topic + * @return string + */ + protected function get_after_notification_status( string $topic ): string { + if ( str_contains( $topic, '.create' ) ) { + return NotificationStatus::NOTIFICATION_CREATED; + } elseif ( str_contains( $topic, '.delete' ) ) { + return NotificationStatus::NOTIFICATION_DELETED; + } else { + return NotificationStatus::NOTIFICATION_UPDATED; + } + } + + /** + * Checks if the item can be processed based on the topic. + * This is needed because the item can change the Notification Status before + * the Job process the item. + * + * @param int $item_id + * @param string $topic + * @return bool + */ + protected function can_process( int $item_id, string $topic ): bool { + $item = $this->get_item( $item_id ); + + if ( str_contains( $topic, '.create' ) ) { + return $this->get_helper()->should_trigger_create_notification( $item ); + } elseif ( str_contains( $topic, '.delete' ) ) { + return $this->get_helper()->should_trigger_delete_notification( $item ); + } else { + return $this->get_helper()->should_trigger_update_notification( $item ); + } + } + + /** + * If there is a valid Item ID and topic is a deletion topic. Mark the item as unsynced. + * + * @param string $topic + * @param int $item + */ + protected function maybe_mark_as_unsynced( string $topic, int $item ): void { + if ( ! str_contains( $topic, '.delete' ) ) { + return; + } + + $item = $this->get_item( $item ); + $this->get_helper()->mark_as_unsynced( $item ); + } + + /** + * Get the item + * + * @param int $item_id + * @return \WC_Product|\WC_Coupon + */ + abstract protected function get_item( int $item_id ); + + /** + * Get the helper + * + * @return CouponHelper|ProductHelper + */ + abstract public function get_helper(); +} diff --git a/src/Jobs/Notifications/CouponNotificationJob.php b/src/Jobs/Notifications/CouponNotificationJob.php index 6043e4f3da..a07885936b 100644 --- a/src/Jobs/Notifications/CouponNotificationJob.php +++ b/src/Jobs/Notifications/CouponNotificationJob.php @@ -5,34 +5,24 @@ use Automattic\WooCommerce\GoogleListingsAndAds\ActionScheduler\ActionSchedulerInterface; use Automattic\WooCommerce\GoogleListingsAndAds\Coupon\CouponHelper; -use Automattic\WooCommerce\GoogleListingsAndAds\Exception\InvalidValue; use Automattic\WooCommerce\GoogleListingsAndAds\Google\NotificationsService; -use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\AbstractActionSchedulerJob; use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\ActionSchedulerJobMonitor; -use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\JobInterface; -use Automattic\WooCommerce\GoogleListingsAndAds\Value\NotificationStatus; - defined( 'ABSPATH' ) || exit; /** * Class CouponNotificationJob - * Class for all the Coupons Notifications Jobs + * Class for the Coupons Notifications Jobs * * @since x.x.x * @package Automattic\WooCommerce\GoogleListingsAndAds\Jobs\Notifications */ -class CouponNotificationJob extends AbstractActionSchedulerJob implements JobInterface { +class CouponNotificationJob extends AbstractItemNotificationJob { /** - * @var NotificationsService $notifications_service + * @var CouponHelper $helper */ - protected $notifications_service; - - /** - * @var CouponHelper $coupon_helper - */ - protected $coupon_helper; + protected $helper; /** * Notifications Jobs constructor. @@ -49,136 +39,35 @@ public function __construct( CouponHelper $coupon_helper ) { $this->notifications_service = $notifications_service; - $this->coupon_helper = $coupon_helper; - parent::__construct( $action_scheduler, $monitor ); - } - - /** - * Get the job name - * - * @return string - */ - public function get_name(): string { - return 'notifications/coupons'; - } - - - /** - * Logic when processing the items - * - * @param array $args Arguments with the item id and the topic - */ - protected function process_items( array $args ): void { - if ( ! isset( $args[0] ) || ! isset( $args[1] ) ) { - do_action( - 'woocommerce_gla_error', - 'Error sending Coupon Notification. Topic and Coupon ID are mandatory', - __METHOD__ - ); - return; - } - - $item = $args[0]; - $topic = $args[1]; - - if ( $this->can_process( $item, $topic ) && $this->notifications_service->notify( $topic, $item ) ) { - $this->set_status( $item, $this->get_after_notification_status( $topic ) ); - $this->maybe_mark_as_unsynced( $topic, $item ); - } + $this->helper = $coupon_helper; + parent::__construct( $action_scheduler, $monitor, $notifications_service ); } /** - * Schedule the Coupon Notification Job + * Get the coupon * - * @param array $args + * @param int $item_id + * @return \WC_Coupon */ - public function schedule( array $args = [] ): void { - if ( $this->can_schedule( [ $args ] ) ) { - $this->action_scheduler->schedule_immediate( - $this->get_process_item_hook(), - [ $args ] - ); - } + protected function get_item( int $item_id ) { + return $this->helper->get_wc_coupon( $item_id ); } /** - * Set the notification status for a coupon. + * Get the Coupon Helper * - * @param int $coupon_id - * @param string $status + * @return CouponHelper */ - protected function set_status( int $coupon_id, string $status ): void { - $coupon = $this->coupon_helper->get_wc_coupon( $coupon_id ); - $this->coupon_helper->set_notification_status( $coupon, $status ); + public function get_helper() { + return $this->helper; } /** - * Get the Notification Status after the notification happens + * Get the job name * - * @param string $topic * @return string */ - protected function get_after_notification_status( string $topic ): string { - if ( str_contains( $topic, '.create' ) ) { - return NotificationStatus::NOTIFICATION_CREATED; - } elseif ( str_contains( $topic, '.delete' ) ) { - return NotificationStatus::NOTIFICATION_DELETED; - } else { - return NotificationStatus::NOTIFICATION_UPDATED; - } - } - - /** - * Checks if the item can be processed based on the topic. - * This is needed because the coupon can change the Notification Status before - * the Job process the item. - * - * @param int $coupon_id - * @param string $topic - * @return bool - */ - protected function can_process( int $coupon_id, string $topic ): bool { - $coupon = $this->coupon_helper->get_wc_coupon( $coupon_id ); - if ( str_contains( $topic, '.create' ) ) { - return $this->coupon_helper->should_trigger_create_notification( $coupon ); - } elseif ( str_contains( $topic, '.delete' ) ) { - return $this->coupon_helper->should_trigger_delete_notification( $coupon ); - } else { - return $this->coupon_helper->should_trigger_update_notification( $coupon ); - } - } - - /** - * If there is a valid Item ID and topic is a deletion topic. Mark the coupon as unsynced. - * - * @param string $topic - * @param int $item - */ - protected function maybe_mark_as_unsynced( string $topic, int $item ): void { - if ( ! str_contains( $topic, '.delete' ) ) { - return; - } - - $coupon = $this->coupon_helper->get_wc_coupon( $item ); - $this->coupon_helper->mark_as_unsynced( $coupon ); - } - - /** - * Can the job be scheduled. - * - * @param array|null $args - * - * @return bool Returns true if the job can be scheduled. - */ - public function can_schedule( $args = [] ): bool { - /** - * Allow users to disable the notification job schedule. - * - * @since x.x.x - * - * @param bool $value The current filter value. By default, it is the result of `$this->can_schedule` function. - * @param array $args The arguments for the schedule function with the item id and the topic. - */ - return apply_filters( 'woocommerce_gla_coupon_notification_job_can_schedule', $this->notifications_service->is_enabled() && parent::can_schedule( $args ), $args ); + public function get_job_name(): string { + return 'coupons'; } } diff --git a/src/Jobs/Notifications/ProductNotificationJob.php b/src/Jobs/Notifications/ProductNotificationJob.php index f02ce37634..c08e4fd0f8 100644 --- a/src/Jobs/Notifications/ProductNotificationJob.php +++ b/src/Jobs/Notifications/ProductNotificationJob.php @@ -7,29 +7,22 @@ use Automattic\WooCommerce\GoogleListingsAndAds\Google\NotificationsService; use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\ActionSchedulerJobMonitor; use Automattic\WooCommerce\GoogleListingsAndAds\Product\ProductHelper; -use Automattic\WooCommerce\GoogleListingsAndAds\Value\NotificationStatus; - defined( 'ABSPATH' ) || exit; /** * Class ProductNotificationJob - * Class for all the Product Notifications Jobs + * Class for the Product Notifications Jobs * * @since x.x.x * @package Automattic\WooCommerce\GoogleListingsAndAds\Jobs\Notifications */ -class ProductNotificationJob extends AbstractNotificationJob { - - /** - * @var NotificationsService $notifications_service - */ - protected $notifications_service; +class ProductNotificationJob extends AbstractItemNotificationJob { /** - * @var ProductHelper $product_helper + * @var ProductHelper $helper */ - protected $product_helper; + protected $helper; /** * Notifications Jobs constructor. @@ -37,96 +30,44 @@ class ProductNotificationJob extends AbstractNotificationJob { * @param ActionSchedulerInterface $action_scheduler * @param ActionSchedulerJobMonitor $monitor * @param NotificationsService $notifications_service - * @param ProductHelper $product_helper + * @param ProductHelper $helper */ public function __construct( ActionSchedulerInterface $action_scheduler, ActionSchedulerJobMonitor $monitor, NotificationsService $notifications_service, - ProductHelper $product_helper + ProductHelper $helper ) { $this->notifications_service = $notifications_service; - $this->product_helper = $product_helper; + $this->helper = $helper; parent::__construct( $action_scheduler, $monitor, $notifications_service ); } /** - * Get the job name - * - * @return string - */ - public function get_job_name(): string { - return 'products'; - } - - - /** - * Logic when processing the items + * Get the product * - * @param array $args Arguments with the item id and the topic + * @param int $item_id + * @return \WC_Product */ - protected function process_items( array $args ): void { - if ( ! isset( $args[0] ) || ! isset( $args[1] ) ) { - do_action( - 'woocommerce_gla_error', - 'Error processing Product Notification Job. Item ID and Topic are mandatory.', - __METHOD__ - ); - return; - } - - $item = $args[0]; - $topic = $args[1]; - - if ( $this->can_process( $item, $topic ) && $this->notifications_service->notify( $topic, $item ) ) { - $this->set_status( $item, $this->get_after_notification_status( $topic ) ); - } + protected function get_item( int $item_id ) { + return $this->helper->get_wc_product( $item_id ); } /** - * Set the notification status for a product. + * Get the Product Helper * - * @param int $product_id - * @param string $status + * @return ProductHelper */ - protected function set_status( int $product_id, string $status ): void { - $product = $this->product_helper->get_wc_product( $product_id ); - $this->product_helper->set_notification_status( $product, $status ); + public function get_helper() { + return $this->helper; } /** - * Get the Notification Status after the notification happens + * Get the job name * - * @param string $topic * @return string */ - protected function get_after_notification_status( string $topic ): string { - if ( str_contains( $topic, '.create' ) ) { - return NotificationStatus::NOTIFICATION_CREATED; - } elseif ( str_contains( $topic, '.delete' ) ) { - return NotificationStatus::NOTIFICATION_DELETED; - } else { - return NotificationStatus::NOTIFICATION_UPDATED; - } - } - - /** - * Checks if the item can be processed based on the topic. - * This is needed because the product can change the Notification Status before - * the Job process the item. - * - * @param int $product_id - * @param string $topic - * @return bool - */ - protected function can_process( int $product_id, string $topic ): bool { - $product = $this->product_helper->get_wc_product( $product_id ); - if ( str_contains( $topic, '.create' ) ) { - return $this->product_helper->should_trigger_create_notification( $product ); - } elseif ( str_contains( $topic, '.delete' ) ) { - return $this->product_helper->should_trigger_delete_notification( $product ); - } else { - return $this->product_helper->should_trigger_update_notification( $product ); - } + public function get_job_name(): string { + return 'products'; } } diff --git a/tests/Unit/Jobs/AbstractItemNotificationJobTest.php b/tests/Unit/Jobs/AbstractItemNotificationJobTest.php new file mode 100644 index 0000000000..70ba6d2cb5 --- /dev/null +++ b/tests/Unit/Jobs/AbstractItemNotificationJobTest.php @@ -0,0 +1,258 @@ +action_scheduler = $this->createMock( ActionScheduler::class ); + $this->monitor = $this->createMock( ActionSchedulerJobMonitor::class ); + $this->notification_service = $this->createMock( NotificationsService::class ); + $this->job = $this->get_job(); + $this->job->init(); + } + + public function test_job_name() { + $this->assertEquals( $this->get_name(), $this->job->get_name() ); + } + + public function test_schedule_schedules_immediate_job() { + $topic = $this->get_topic_name() . '.create'; + $id = 1; + + $this->notification_service->expects( $this->once() )->method( 'is_enabled' )->willReturn( true ); + + $this->action_scheduler->expects( $this->once() ) + ->method( 'has_scheduled_action' ) + ->with( $this->get_process_item_hook(), [ [ $id, $topic ] ] ) + ->willReturn( false ); + + $this->action_scheduler->expects( $this->once() ) + ->method( 'schedule_immediate' ) + ->with( + $this->get_process_item_hook(), + [ [ $id, $topic ] ] + ); + + $this->job->schedule( [ $id, $topic ] ); + } + + public function test_schedule_doesnt_schedules_immediate_job_if_already_scheduled() { + $topic = $this->get_topic_name() . '.create'; + $id = 1; + + $this->notification_service->expects( $this->once() )->method( 'is_enabled' )->willReturn( true ); + + $this->action_scheduler->expects( $this->once() ) + ->method( 'has_scheduled_action' ) + ->with( $this->get_process_item_hook(), [ [ $id, $topic ] ] ) + ->willReturn( true ); + + $this->action_scheduler->expects( $this->never() )->method( 'schedule_immediate' ); + + $this->job->schedule( [ $id, $topic ] ); + } + + public function test_schedule_doesnt_schedules_immediate_job_if_not_enabled() { + $topic = $this->get_topic_name() . '.create'; + $id = 1; + + $this->notification_service->expects( $this->once() )->method( 'is_enabled' )->willReturn( false ); + + $this->action_scheduler->expects( $this->never() ) + ->method( 'has_scheduled_action' ); + + $this->action_scheduler->expects( $this->never() )->method( 'schedule_immediate' ); + + $this->job->schedule( [ $id, $topic ] ); + } + + public function test_process_items_calls_notify_and_set_status_on_success() { + /** @var \WC_Product|\WC_Coupon $item */ + $item = $this->create_item(); + $id = $item->get_id(); + $topic = $this->get_topic_name() . '.create'; + + $this->notification_service->expects( $this->once() ) + ->method( 'notify' ) + ->with( $topic, $id ) + ->willReturn( true ); + + $this->job->get_helper()->expects( $this->exactly( 2 ) ) + ->method( 'get_wc_' . $this->get_topic_name() ) + ->with( $id ) + ->willReturn( $item ); + + $this->job->get_helper()->expects( $this->once() ) + ->method( 'should_trigger_create_notification' ) + ->with( $item ) + ->willReturn( true ); + + $this->job->get_helper()->expects( $this->once() ) + ->method( 'set_notification_status' ) + ->with( $item, NotificationStatus::NOTIFICATION_CREATED ); + + $this->job->handle_process_items_action( [ $id, $topic ] ); + } + + public function test_process_items_doesnt_calls_notify_when_no_args() { + $this->notification_service->expects( $this->never() ) + ->method( 'notify' ); + + $this->job->handle_process_items_action( [] ); + $this->job->handle_process_items_action( [ 1 ] ); + } + + public function test_process_items_doesnt_calls_set_status_on_failure() { + /** @var \WC_Product|\WC_Coupon $item */ + $item = $this->create_item(); + $id = $item->get_id(); + $topic = $this->get_topic_name() . '.create'; + + $this->notification_service->expects( $this->once() ) + ->method( 'notify' ) + ->with( $topic, $id ) + ->willReturn( false ); + + $this->job->get_helper()->expects( $this->once() ) + ->method( 'get_wc_' . $this->get_topic_name() ) + ->with( $id ) + ->willReturn( $item ); + + $this->job->get_helper()->expects( $this->once() ) + ->method( 'should_trigger_create_notification' ) + ->with( $item ) + ->willReturn( true ); + + $this->job->get_helper()->expects( $this->never() ) + ->method( 'set_notification_status' ); + + $this->job->handle_process_items_action( [ $id, $topic ] ); + } + + public function test_get_after_notification_status() { + /** @var \WC_Product|\WC_Coupon $item */ + $item = $this->create_item(); + $id = $item->get_id(); + + $this->notification_service->expects( $this->exactly( 3 ) ) + ->method( 'notify' ) + ->willReturn( true ); + + $this->job->get_helper()->expects( $this->exactly( 7 ) ) + ->method( 'get_wc_' . $this->get_topic_name() ) + ->willReturn( $item ); + + $this->job->get_helper()->expects( $this->once() ) + ->method( 'should_trigger_create_notification' ) + ->with( $item ) + ->willReturn( true ); + + $this->job->get_helper()->expects( $this->once() ) + ->method( 'should_trigger_update_notification' ) + ->with( $item ) + ->willReturn( true ); + + $this->job->get_helper()->expects( $this->once() ) + ->method( 'should_trigger_delete_notification' ) + ->with( $item ) + ->willReturn( true ); + + $this->job->get_helper()->expects( $this->exactly( 3 ) ) + ->method( 'set_notification_status' ) + ->willReturnCallback( + function ( $id, $topic ) { + if ( $topic === 'product.create' ) { + return NotificationStatus::NOTIFICATION_CREATED; + } elseif ( $topic === 'product.delete' ) { + return NotificationStatus::NOTIFICATION_DELETED; + } else { + return NotificationStatus::NOTIFICATION_UPDATED; + } + } + ); + + $this->job->handle_process_items_action( [ $id, $this->get_topic_name() . '.create' ] ); + $this->job->handle_process_items_action( [ $id, $this->get_topic_name() . '.delete' ] ); + $this->job->handle_process_items_action( [ $id, $this->get_topic_name() . '.update' ] ); + } + + public function test_dont_process_item_if_status_changed() { + /** @var \WC_Product|\WC_Coupon $item */ + $item = $this->create_item(); + $id = $item->get_id(); + + $this->notification_service->expects( $this->never() )->method( 'notify' ); + + $this->job->get_helper()->expects( $this->exactly( 3 ) ) + ->method( 'get_wc_' . $this->get_topic_name() ) + ->willReturn( $item ); + + $this->job->get_helper()->expects( $this->once() ) + ->method( 'should_trigger_create_notification' ) + ->with( $item ) + ->willReturn( false ); + + $this->job->get_helper()->expects( $this->once() ) + ->method( 'should_trigger_update_notification' ) + ->with( $item ) + ->willReturn( false ); + + $this->job->get_helper()->expects( $this->once() ) + ->method( 'should_trigger_delete_notification' ) + ->with( $item ) + ->willReturn( false ); + + $this->job->get_helper()->expects( $this->never() )->method( 'set_notification_status' ); + + $this->job->handle_process_items_action( [ $id, $this->get_topic_name() . '.create' ] ); + $this->job->handle_process_items_action( [ $id, $this->get_topic_name() . '.delete' ] ); + $this->job->handle_process_items_action( [ $id, $this->get_topic_name() . '.update' ] ); + } + + public function get_name() { + return 'notifications/' . $this->get_job_name(); + } + + public function get_process_item_hook() { + return 'gla/jobs/' . $this->get_name() . '/process_item'; + } + + abstract public function get_job_name(); + abstract public function get_topic_name(); + abstract public function get_job(); + abstract public function create_item(); +} diff --git a/tests/Unit/Jobs/AbstractNotificationJobTest.php b/tests/Unit/Jobs/AbstractNotificationJobTest.php index 67d1c64f13..b22761dcbc 100644 --- a/tests/Unit/Jobs/AbstractNotificationJobTest.php +++ b/tests/Unit/Jobs/AbstractNotificationJobTest.php @@ -6,7 +6,7 @@ use Automattic\WooCommerce\GoogleListingsAndAds\ActionScheduler\ActionScheduler; use Automattic\WooCommerce\GoogleListingsAndAds\Google\NotificationsService; use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\ActionSchedulerJobMonitor; -use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\Notifications\SettingsNotificationJob; +use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\Notifications\AbstractNotificationJob; use Automattic\WooCommerce\GoogleListingsAndAds\Tests\Framework\UnitTest; use PHPUnit\Framework\MockObject\MockObject; @@ -26,7 +26,7 @@ abstract class AbstractNotificationJobTest extends UnitTest { /** @var MockObject|NotificationsService $notification_service */ protected $notification_service; - /** @var SettingsNotificationJob $job */ + /** @var AbstractNotificationJob $job */ protected $job; /** diff --git a/tests/Unit/Jobs/CouponNotificationJobTest.php b/tests/Unit/Jobs/CouponNotificationJobTest.php index 9c7537b8fa..3269e6fda9 100644 --- a/tests/Unit/Jobs/CouponNotificationJobTest.php +++ b/tests/Unit/Jobs/CouponNotificationJobTest.php @@ -3,14 +3,8 @@ namespace Automattic\WooCommerce\GoogleListingsAndAds\Tests\Unit\Jobs; -use Automattic\WooCommerce\GoogleListingsAndAds\ActionScheduler\ActionScheduler; -use Automattic\WooCommerce\GoogleListingsAndAds\Google\NotificationsService; -use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\ActionSchedulerJobMonitor; use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\Notifications\CouponNotificationJob; use Automattic\WooCommerce\GoogleListingsAndAds\Coupon\CouponHelper; -use Automattic\WooCommerce\GoogleListingsAndAds\Tests\Framework\UnitTest; -use Automattic\WooCommerce\GoogleListingsAndAds\Value\NotificationStatus; -use PHPUnit\Framework\MockObject\MockObject; use WC_Helper_Coupon; /** @@ -19,264 +13,26 @@ * @group Notifications * @package Automattic\WooCommerce\GoogleListingsAndAds\Tests\Unit\Jobs */ -class CouponNotificationJobTest extends UnitTest { +class CouponNotificationJobTest extends AbstractItemNotificationJobTest { + public function get_job_name() { + return 'coupons'; + } - /** @var MockObject|ActionScheduler $action_scheduler */ - protected $action_scheduler; - - /** @var MockObject|ActionSchedulerJobMonitor $monitor */ - protected $monitor; - - /** @var MockObject|NotificationsService $notification_service */ - protected $notification_service; - - /** @var MockObject|CouponHelper $coupon_helper */ - protected $coupon_helper; - - /** @var CouponNotificationJob $job */ - protected $job; - - protected const JOB_NAME = 'notifications/coupons'; - protected const PROCESS_ITEM_HOOK = 'gla/jobs/' . self::JOB_NAME . '/process_item'; - - /** - * Runs before each test is executed. - */ - public function setUp(): void { - parent::setUp(); + public function get_topic_name() { + return 'coupon'; + } - $this->action_scheduler = $this->createMock( ActionScheduler::class ); - $this->monitor = $this->createMock( ActionSchedulerJobMonitor::class ); - $this->notification_service = $this->createMock( NotificationsService::class ); - $this->coupon_helper = $this->createMock( CouponHelper::class ); - $this->job = new CouponNotificationJob( + public function get_job() { + return new CouponNotificationJob( $this->action_scheduler, $this->monitor, $this->notification_service, - $this->coupon_helper + $this->createMock( CouponHelper::class ) ); - - $this->job->init(); - } - - public function test_job_name() { - $this->assertEquals( self::JOB_NAME, $this->job->get_name() ); - } - - public function test_schedule_schedules_immediate_job() { - $topic = 'coupon.create'; - $id = 1; - - $this->notification_service->expects( $this->once() )->method( 'is_enabled' )->willReturn( true ); - - $this->action_scheduler->expects( $this->once() ) - ->method( 'has_scheduled_action' ) - ->with( self::PROCESS_ITEM_HOOK, [ [ $id, $topic ] ] ) - ->willReturn( false ); - - $this->action_scheduler->expects( $this->once() ) - ->method( 'schedule_immediate' ) - ->with( - self::PROCESS_ITEM_HOOK, - [ [ $id, $topic ] ] - ); - - $this->job->schedule( [ $id, $topic ] ); - } - - public function test_schedule_doesnt_schedules_immediate_job_if_already_scheduled() { - $id = 1; - $topic = 'coupon.create'; - - $this->notification_service->expects( $this->once() )->method( 'is_enabled' )->willReturn( true ); - - $this->action_scheduler->expects( $this->once() ) - ->method( 'has_scheduled_action' ) - ->with( self::PROCESS_ITEM_HOOK, [ [ $id, $topic ] ] ) - ->willReturn( true ); - - $this->action_scheduler->expects( $this->never() )->method( 'schedule_immediate' ); - - $this->job->schedule( [ $id, $topic ] ); - } - - public function test_schedule_doesnt_schedules_immediate_job_if_not_enabled() { - $id = 1; - $topic = 'coupon.create'; - - $this->notification_service->expects( $this->once() )->method( 'is_enabled' )->willReturn( false ); - - $this->action_scheduler->expects( $this->never() ) - ->method( 'has_scheduled_action' ); - - $this->action_scheduler->expects( $this->never() )->method( 'schedule_immediate' ); - - $this->job->schedule( [ $id, $topic ] ); - } - - public function test_process_items_calls_notify_and_set_status_on_success() { - /** @var \WC_Coupon $coupon */ - $coupon = WC_Helper_Coupon::create_coupon(); - $id = $coupon->get_id(); - $topic = 'coupon.create'; - - $this->notification_service->expects( $this->once() ) - ->method( 'notify' ) - ->with( $topic, $id ) - ->willReturn( true ); - - $this->coupon_helper->expects( $this->exactly( 2 ) ) - ->method( 'get_wc_coupon' ) - ->with( $id ) - ->willReturn( $coupon ); - - $this->coupon_helper->expects( $this->once() ) - ->method( 'should_trigger_create_notification' ) - ->with( $coupon ) - ->willReturn( true ); - - $this->coupon_helper->expects( $this->once() ) - ->method( 'set_notification_status' ) - ->with( $coupon, NotificationStatus::NOTIFICATION_CREATED ); - - $this->job->handle_process_items_action( [ $id, $topic ] ); - } - - public function test_process_items_doesnt_calls_notify_when_no_args() { - $this->notification_service->expects( $this->never() ) - ->method( 'notify' ); - - $this->job->handle_process_items_action( [] ); - $this->job->handle_process_items_action( [ 1 ] ); - } - - public function test_process_items_doesnt_calls_set_status_on_failure() { - /** @var \WC_Coupon $coupon */ - $coupon = WC_Helper_Coupon::create_coupon(); - $id = $coupon->get_id(); - $topic = 'coupon.create'; - - $this->notification_service->expects( $this->once() ) - ->method( 'notify' ) - ->with( $topic, $id ) - ->willReturn( false ); - - $this->coupon_helper->expects( $this->once() ) - ->method( 'get_wc_coupon' ) - ->with( $id ) - ->willReturn( $coupon ); - - $this->coupon_helper->expects( $this->once() ) - ->method( 'should_trigger_create_notification' ) - ->with( $coupon ) - ->willReturn( true ); - - $this->coupon_helper->expects( $this->never() ) - ->method( 'set_notification_status' ); - - $this->job->handle_process_items_action( [ $id, $topic ] ); - } - - public function test_get_after_notification_status() { - /** @var \WC_Coupon $coupon */ - $coupon = WC_Helper_Coupon::create_coupon(); - $id = $coupon->get_id(); - - $this->notification_service->expects( $this->exactly( 3 ) ) - ->method( 'notify' ) - ->willReturn( true ); - - $this->coupon_helper->expects( $this->exactly( 7 ) ) - ->method( 'get_wc_coupon' ) - ->willReturn( $coupon ); - - $this->coupon_helper->expects( $this->once() ) - ->method( 'should_trigger_create_notification' ) - ->with( $coupon ) - ->willReturn( true ); - - $this->coupon_helper->expects( $this->once() ) - ->method( 'should_trigger_update_notification' ) - ->with( $coupon ) - ->willReturn( true ); - - $this->coupon_helper->expects( $this->once() ) - ->method( 'should_trigger_delete_notification' ) - ->with( $coupon ) - ->willReturn( true ); - - $this->coupon_helper->expects( $this->exactly( 3 ) ) - ->method( 'set_notification_status' ) - ->willReturnCallback( - function ( $id, $topic ) { - if ( $topic === 'coupon.create' ) { - return NotificationStatus::NOTIFICATION_CREATED; - } elseif ( $topic === 'coupon.delete' ) { - return NotificationStatus::NOTIFICATION_DELETED; - } else { - return NotificationStatus::NOTIFICATION_UPDATED; - } - } - ); - - $this->job->handle_process_items_action( [ $id, 'coupon.create' ] ); - $this->job->handle_process_items_action( [ $id, 'coupon.delete' ] ); - $this->job->handle_process_items_action( [ $id, 'coupon.update' ] ); } - public function test_dont_process_item_if_status_changed() { - /** @var \WC_Coupon $coupon */ - $coupon = WC_Helper_Coupon::create_coupon(); - $id = $coupon->get_id(); - - $this->notification_service->expects( $this->never() )->method( 'notify' ); - - $this->coupon_helper->expects( $this->exactly( 3 ) ) - ->method( 'get_wc_coupon' ) - ->willReturn( $coupon ); - - $this->coupon_helper->expects( $this->once() ) - ->method( 'should_trigger_create_notification' ) - ->with( $coupon ) - ->willReturn( false ); - - $this->coupon_helper->expects( $this->once() ) - ->method( 'should_trigger_update_notification' ) - ->with( $coupon ) - ->willReturn( false ); - - $this->coupon_helper->expects( $this->once() ) - ->method( 'should_trigger_delete_notification' ) - ->with( $coupon ) - ->willReturn( false ); - - $this->coupon_helper->expects( $this->never() )->method( 'set_notification_status' ); - - $this->job->handle_process_items_action( [ $id, 'coupon.create' ] ); - $this->job->handle_process_items_action( [ $id, 'coupon.delete' ] ); - $this->job->handle_process_items_action( [ $id, 'coupon.update' ] ); - } - - public function test_mark_as_unsynced_when_delete() { - /** @var \WC_Coupon $coupon */ - $coupon = WC_Helper_Coupon::create_coupon(); - $id = $coupon->get_id(); - - $this->coupon_helper->expects( $this->once() ) - ->method( 'should_trigger_delete_notification' ) - ->with( $coupon ) - ->willReturn( true ); - - $this->coupon_helper->expects( $this->exactly( 3 ) ) - ->method( 'get_wc_coupon' ) - ->with( $id ) - ->willReturn( $coupon ); - - $this->notification_service->expects( $this->once() )->method( 'notify' )->willReturn( true ); - $this->coupon_helper->expects( $this->once() ) - ->method( 'mark_as_unsynced' ); - - $this->job->handle_process_items_action( [ $id, 'coupon.delete' ] ); + public function create_item() { + return WC_Helper_Coupon::create_coupon(); } } diff --git a/tests/Unit/Jobs/ProductNotificationJobTest.php b/tests/Unit/Jobs/ProductNotificationJobTest.php index abb379794f..86ca89de61 100644 --- a/tests/Unit/Jobs/ProductNotificationJobTest.php +++ b/tests/Unit/Jobs/ProductNotificationJobTest.php @@ -3,14 +3,8 @@ namespace Automattic\WooCommerce\GoogleListingsAndAds\Tests\Unit\Jobs; -use Automattic\WooCommerce\GoogleListingsAndAds\ActionScheduler\ActionScheduler; -use Automattic\WooCommerce\GoogleListingsAndAds\Google\NotificationsService; -use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\ActionSchedulerJobMonitor; use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\Notifications\ProductNotificationJob; use Automattic\WooCommerce\GoogleListingsAndAds\Product\ProductHelper; -use Automattic\WooCommerce\GoogleListingsAndAds\Tests\Framework\UnitTest; -use Automattic\WooCommerce\GoogleListingsAndAds\Value\NotificationStatus; -use PHPUnit\Framework\MockObject\MockObject; use WC_Helper_Product; /** @@ -19,242 +13,27 @@ * @group Notifications * @package Automattic\WooCommerce\GoogleListingsAndAds\Tests\Unit\Jobs */ -class ProductNotificationJobTest extends UnitTest { +class ProductNotificationJobTest extends AbstractItemNotificationJobTest { + public function get_job_name() { + return 'products'; + } - /** @var MockObject|ActionScheduler $action_scheduler */ - protected $action_scheduler; - - /** @var MockObject|ActionSchedulerJobMonitor $monitor */ - protected $monitor; - - /** @var MockObject|NotificationsService $notification_service */ - protected $notification_service; - - /** @var MockObject|ProductHelper $product_helper */ - protected $product_helper; - - /** @var ProductNotificationJob $job */ - protected $job; - - protected const JOB_NAME = 'notifications/products'; - protected const PROCESS_ITEM_HOOK = 'gla/jobs/' . self::JOB_NAME . '/process_item'; - - /** - * Runs before each test is executed. - */ - public function setUp(): void { - parent::setUp(); + public function get_topic_name() { + return 'product'; + } - $this->action_scheduler = $this->createMock( ActionScheduler::class ); - $this->monitor = $this->createMock( ActionSchedulerJobMonitor::class ); - $this->notification_service = $this->createMock( NotificationsService::class ); - $this->product_helper = $this->createMock( ProductHelper::class ); - $this->job = new ProductNotificationJob( + public function get_job() { + return new ProductNotificationJob( $this->action_scheduler, $this->monitor, $this->notification_service, - $this->product_helper + $this->createMock( ProductHelper::class ) ); - - $this->job->init(); - } - - public function test_job_name() { - $this->assertEquals( self::JOB_NAME, $this->job->get_name() ); } - public function test_schedule_schedules_immediate_job() { - $topic = 'product.create'; - $id = 1; - - $this->notification_service->expects( $this->once() )->method( 'is_enabled' )->willReturn( true ); - - $this->action_scheduler->expects( $this->once() ) - ->method( 'has_scheduled_action' ) - ->with( self::PROCESS_ITEM_HOOK, [ [ $id, $topic ] ] ) - ->willReturn( false ); - - $this->action_scheduler->expects( $this->once() ) - ->method( 'schedule_immediate' ) - ->with( - self::PROCESS_ITEM_HOOK, - [ [ $id, $topic ] ] - ); - - $this->job->schedule( [ $id, $topic ] ); - } - - public function test_schedule_doesnt_schedules_immediate_job_if_already_scheduled() { - $id = 1; - $topic = 'product.create'; - - $this->notification_service->expects( $this->once() )->method( 'is_enabled' )->willReturn( true ); - - $this->action_scheduler->expects( $this->once() ) - ->method( 'has_scheduled_action' ) - ->with( self::PROCESS_ITEM_HOOK, [ [ $id, $topic ] ] ) - ->willReturn( true ); - - $this->action_scheduler->expects( $this->never() )->method( 'schedule_immediate' ); - - $this->job->schedule( [ $id, $topic ] ); - } - - public function test_schedule_doesnt_schedules_immediate_job_if_not_enabled() { - $id = 1; - $topic = 'product.create'; - - $this->notification_service->expects( $this->once() )->method( 'is_enabled' )->willReturn( false ); - - $this->action_scheduler->expects( $this->never() ) - ->method( 'has_scheduled_action' ); - - $this->action_scheduler->expects( $this->never() )->method( 'schedule_immediate' ); - - $this->job->schedule( [ $id, $topic ] ); - } - - public function test_process_items_calls_notify_and_set_status_on_success() { - /** @var \WC_Product $product */ - $product = WC_Helper_Product::create_simple_product(); - $id = $product->get_id(); - $topic = 'product.create'; - - $this->notification_service->expects( $this->once() ) - ->method( 'notify' ) - ->with( $topic, $id ) - ->willReturn( true ); - - $this->product_helper->expects( $this->exactly( 2 ) ) - ->method( 'get_wc_product' ) - ->with( $id ) - ->willReturn( $product ); - - $this->product_helper->expects( $this->once() ) - ->method( 'should_trigger_create_notification' ) - ->with( $product ) - ->willReturn( true ); - - $this->product_helper->expects( $this->once() ) - ->method( 'set_notification_status' ) - ->with( $product, NotificationStatus::NOTIFICATION_CREATED ); - - $this->job->handle_process_items_action( [ $id, $topic ] ); - } - - public function test_process_items_doesnt_calls_notify_when_no_args() { - $this->notification_service->expects( $this->never() ) - ->method( 'notify' ); - - $this->job->handle_process_items_action( [] ); - $this->job->handle_process_items_action( [ 1 ] ); - } - - public function test_process_items_doesnt_calls_set_status_on_failure() { - /** @var \WC_Product $product */ - $product = WC_Helper_Product::create_simple_product(); - $id = $product->get_id(); - $topic = 'product.create'; - - $this->notification_service->expects( $this->once() ) - ->method( 'notify' ) - ->with( $topic, $id ) - ->willReturn( false ); - - $this->product_helper->expects( $this->once() ) - ->method( 'get_wc_product' ) - ->with( $id ) - ->willReturn( $product ); - - $this->product_helper->expects( $this->once() ) - ->method( 'should_trigger_create_notification' ) - ->with( $product ) - ->willReturn( true ); - - $this->product_helper->expects( $this->never() ) - ->method( 'set_notification_status' ); - - $this->job->handle_process_items_action( [ $id, $topic ] ); - } - - public function test_get_after_notification_status() { - /** @var \WC_Product $product */ - $product = WC_Helper_Product::create_simple_product(); - $id = $product->get_id(); - - $this->notification_service->expects( $this->exactly( 3 ) ) - ->method( 'notify' ) - ->willReturn( true ); - - $this->product_helper->expects( $this->exactly( 6 ) ) - ->method( 'get_wc_product' ) - ->willReturn( $product ); - - $this->product_helper->expects( $this->once() ) - ->method( 'should_trigger_create_notification' ) - ->with( $product ) - ->willReturn( true ); - - $this->product_helper->expects( $this->once() ) - ->method( 'should_trigger_update_notification' ) - ->with( $product ) - ->willReturn( true ); - - $this->product_helper->expects( $this->once() ) - ->method( 'should_trigger_delete_notification' ) - ->with( $product ) - ->willReturn( true ); - - $this->product_helper->expects( $this->exactly( 3 ) ) - ->method( 'set_notification_status' ) - ->willReturnCallback( - function ( $id, $topic ) { - if ( $topic === 'product.create' ) { - return NotificationStatus::NOTIFICATION_CREATED; - } elseif ( $topic === 'product.delete' ) { - return NotificationStatus::NOTIFICATION_DELETED; - } else { - return NotificationStatus::NOTIFICATION_UPDATED; - } - } - ); - - $this->job->handle_process_items_action( [ $id, 'product.create' ] ); - $this->job->handle_process_items_action( [ $id, 'product.delete' ] ); - $this->job->handle_process_items_action( [ $id, 'product.update' ] ); - } - - public function test_dont_process_item_if_status_changed() { - /** @var \WC_Product $product */ - $product = WC_Helper_Product::create_simple_product(); - $id = $product->get_id(); - - $this->notification_service->expects( $this->never() )->method( 'notify' ); - - $this->product_helper->expects( $this->exactly( 3 ) ) - ->method( 'get_wc_product' ) - ->willReturn( $product ); - - $this->product_helper->expects( $this->once() ) - ->method( 'should_trigger_create_notification' ) - ->with( $product ) - ->willReturn( false ); - - $this->product_helper->expects( $this->once() ) - ->method( 'should_trigger_update_notification' ) - ->with( $product ) - ->willReturn( false ); - - $this->product_helper->expects( $this->once() ) - ->method( 'should_trigger_delete_notification' ) - ->with( $product ) - ->willReturn( false ); - - $this->product_helper->expects( $this->never() )->method( 'set_notification_status' ); - $this->job->handle_process_items_action( [ $id, 'product.create' ] ); - $this->job->handle_process_items_action( [ $id, 'product.delete' ] ); - $this->job->handle_process_items_action( [ $id, 'product.update' ] ); + public function create_item() { + return WC_Helper_Product::create_simple_product(); } } From ecb1746da0c55568e27ed83ddab044a9e7305978 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20P=C3=A9rez=20Pellicer?= <5908855+puntope@users.noreply.github.com> Date: Wed, 28 Feb 2024 08:54:19 -0300 Subject: [PATCH 046/262] Update src/Coupon/SyncerHooks.php Co-authored-by: martynmjones <40762232+martynmjones@users.noreply.github.com> --- src/Coupon/SyncerHooks.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Coupon/SyncerHooks.php b/src/Coupon/SyncerHooks.php index 1be8687152..85bcd03e84 100644 --- a/src/Coupon/SyncerHooks.php +++ b/src/Coupon/SyncerHooks.php @@ -355,7 +355,7 @@ protected function set_already_scheduled_to_delete( int $coupon_id ): void { } /** - * Schedules notifications for an updated product + * Schedules notifications for an updated coupon * * @param WC_Coupon $coupon */ From 3819cca3173caa587ea638fa2f22a71622b59f63 Mon Sep 17 00:00:00 2001 From: Miguel Perez Pellicer <5908855+puntope@users.noreply.github.com> Date: Wed, 28 Feb 2024 09:29:21 -0300 Subject: [PATCH 047/262] Apply review comments --- src/Coupon/SyncerHooks.php | 28 ++++++++++++++++--- .../Notifications/CouponNotificationJob.php | 6 ++-- tests/Unit/Coupon/SyncerHooksTest.php | 27 ++++++++++++++++-- 3 files changed, 51 insertions(+), 10 deletions(-) diff --git a/src/Coupon/SyncerHooks.php b/src/Coupon/SyncerHooks.php index 85bcd03e84..caa9948af6 100644 --- a/src/Coupon/SyncerHooks.php +++ b/src/Coupon/SyncerHooks.php @@ -259,7 +259,12 @@ protected function handle_delete_coupon( int $coupon_id ) { if ( $coupon instanceof WC_Coupon && $this->notifications_service->is_enabled() && $this->coupon_helper->should_trigger_delete_notification( $coupon ) ) { $this->coupon_helper->set_notification_status( $coupon, NotificationStatus::NOTIFICATION_PENDING_DELETE ); - $this->coupon_notification_job->schedule( [ $coupon->get_id(), NotificationsService::TOPIC_COUPON_DELETED ] ); + $this->coupon_notification_job->schedule( + [ + 'item_id' => $coupon->get_id(), + 'topic' => NotificationsService::TOPIC_COUPON_DELETED, + ] + ); return; } @@ -362,13 +367,28 @@ protected function set_already_scheduled_to_delete( int $coupon_id ): void { protected function handle_update_coupon_notification( WC_Coupon $coupon ) { if ( $this->coupon_helper->should_trigger_create_notification( $coupon ) ) { $this->coupon_helper->set_notification_status( $coupon, NotificationStatus::NOTIFICATION_PENDING_CREATE ); - $this->coupon_notification_job->schedule( [ $coupon->get_id(), NotificationsService::TOPIC_COUPON_CREATED ] ); + $this->coupon_notification_job->schedule( + [ + 'item_id' => $coupon->get_id(), + 'topic' => NotificationsService::TOPIC_COUPON_CREATED, + ] + ); } elseif ( $this->coupon_helper->should_trigger_update_notification( $coupon ) ) { $this->coupon_helper->set_notification_status( $coupon, NotificationStatus::NOTIFICATION_PENDING_UPDATE ); - $this->coupon_notification_job->schedule( [ $coupon->get_id(), NotificationsService::TOPIC_COUPON_UPDATED ] ); + $this->coupon_notification_job->schedule( + [ + 'item_id' => $coupon->get_id(), + 'topic' => NotificationsService::TOPIC_COUPON_UPDATED, + ] + ); } elseif ( $this->coupon_helper->should_trigger_delete_notification( $coupon ) ) { $this->coupon_helper->set_notification_status( $coupon, NotificationStatus::NOTIFICATION_PENDING_DELETE ); - $this->coupon_notification_job->schedule( [ $coupon->get_id(), NotificationsService::TOPIC_COUPON_DELETED ] ); + $this->coupon_notification_job->schedule( + [ + 'item_id' => $coupon->get_id(), + 'topic' => NotificationsService::TOPIC_COUPON_DELETED, + ] + ); } } } diff --git a/src/Jobs/Notifications/CouponNotificationJob.php b/src/Jobs/Notifications/CouponNotificationJob.php index 6043e4f3da..e69f3a5735 100644 --- a/src/Jobs/Notifications/CouponNotificationJob.php +++ b/src/Jobs/Notifications/CouponNotificationJob.php @@ -69,7 +69,7 @@ public function get_name(): string { * @param array $args Arguments with the item id and the topic */ protected function process_items( array $args ): void { - if ( ! isset( $args[0] ) || ! isset( $args[1] ) ) { + if ( ! isset( $args['item_id'] ) || ! $args['topic'] ) { do_action( 'woocommerce_gla_error', 'Error sending Coupon Notification. Topic and Coupon ID are mandatory', @@ -78,8 +78,8 @@ protected function process_items( array $args ): void { return; } - $item = $args[0]; - $topic = $args[1]; + $item = $args['item_id']; + $topic = $args['topic']; if ( $this->can_process( $item, $topic ) && $this->notifications_service->notify( $topic, $item ) ) { $this->set_status( $item, $this->get_after_notification_status( $topic ) ); diff --git a/tests/Unit/Coupon/SyncerHooksTest.php b/tests/Unit/Coupon/SyncerHooksTest.php index d74bc10205..51fa1e1dde 100644 --- a/tests/Unit/Coupon/SyncerHooksTest.php +++ b/tests/Unit/Coupon/SyncerHooksTest.php @@ -178,7 +178,14 @@ public function test_create_coupon_triggers_notification_created() { $coupon = WC_Helper_Coupon::create_coupon( uniqid(), [ 'status' => 'draft' ] ); $this->notification_service->expects( $this->once() )->method( 'is_enabled' )->willReturn( true ); $this->coupon_notification_job->expects( $this->once() ) - ->method( 'schedule' )->with( $this->equalTo( [ $coupon->get_id(), NotificationsService::TOPIC_COUPON_CREATED ] ) ); + ->method( 'schedule' )->with( + $this->equalTo( + [ + 'item_id' => $coupon->get_id(), + 'topic' => NotificationsService::TOPIC_COUPON_CREATED, + ] + ) + ); $coupon->set_status( 'publish' ); $coupon->add_meta_data( '_wc_gla_visibility', ChannelVisibility::SYNC_AND_SHOW, true ); $coupon->save(); @@ -191,7 +198,14 @@ public function test_update_coupon_triggers_notification_updated() { $coupon = WC_Helper_Coupon::create_coupon( uniqid() ); $this->notification_service->expects( $this->once() )->method( 'is_enabled' )->willReturn( true ); $this->coupon_notification_job->expects( $this->once() ) - ->method( 'schedule' )->with( $this->equalTo( [ $coupon->get_id(), NotificationsService::TOPIC_COUPON_UPDATED ] ) ); + ->method( 'schedule' )->with( + $this->equalTo( + [ + 'item_id' => $coupon->get_id(), + 'topic' => NotificationsService::TOPIC_COUPON_UPDATED, + ] + ) + ); $this->coupon_helper->set_notification_status( $coupon, NotificationStatus::NOTIFICATION_CREATED ); $coupon->set_status( 'publish' ); $coupon->add_meta_data( '_wc_gla_visibility', ChannelVisibility::SYNC_AND_SHOW, true ); @@ -205,7 +219,14 @@ public function test_delete_coupon_triggers_notification_delete() { $coupon = WC_Helper_Coupon::create_coupon( uniqid() ); $this->notification_service->expects( $this->once() )->method( 'is_enabled' )->willReturn( true ); $this->coupon_notification_job->expects( $this->once() ) - ->method( 'schedule' )->with( $this->equalTo( [ $coupon->get_id(), NotificationsService::TOPIC_COUPON_DELETED ] ) ); + ->method( 'schedule' )->with( + $this->equalTo( + [ + 'item_id' => $coupon->get_id(), + 'topic' => NotificationsService::TOPIC_COUPON_DELETED, + ] + ) + ); $coupon->set_status( 'publish' ); $coupon->add_meta_data( '_wc_gla_visibility', ChannelVisibility::DONT_SYNC_AND_SHOW, true ); $this->coupon_helper->set_notification_status( $coupon, NotificationStatus::NOTIFICATION_UPDATED ); From 7814c4974018ab2da4ac265fd5b1bd7ec7081f67 Mon Sep 17 00:00:00 2001 From: Miguel Perez Pellicer <5908855+puntope@users.noreply.github.com> Date: Wed, 28 Feb 2024 09:32:07 -0300 Subject: [PATCH 048/262] PHPCS --- src/Jobs/Notifications/CouponNotificationJob.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Jobs/Notifications/CouponNotificationJob.php b/src/Jobs/Notifications/CouponNotificationJob.php index e69f3a5735..37247ec37b 100644 --- a/src/Jobs/Notifications/CouponNotificationJob.php +++ b/src/Jobs/Notifications/CouponNotificationJob.php @@ -69,7 +69,7 @@ public function get_name(): string { * @param array $args Arguments with the item id and the topic */ protected function process_items( array $args ): void { - if ( ! isset( $args['item_id'] ) || ! $args['topic'] ) { + if ( ! isset( $args['item_id'] ) || ! isset( $args['topic'] ) ) { do_action( 'woocommerce_gla_error', 'Error sending Coupon Notification. Topic and Coupon ID are mandatory', From ff59b16c97926a92caa15a278f265852619627b2 Mon Sep 17 00:00:00 2001 From: Miguel Perez Pellicer <5908855+puntope@users.noreply.github.com> Date: Wed, 28 Feb 2024 09:42:06 -0300 Subject: [PATCH 049/262] PHPCS --- tests/Unit/Jobs/CouponNotificationJobTest.php | 63 ++++++++++++++++--- 1 file changed, 54 insertions(+), 9 deletions(-) diff --git a/tests/Unit/Jobs/CouponNotificationJobTest.php b/tests/Unit/Jobs/CouponNotificationJobTest.php index 9c7537b8fa..c2c45a4336 100644 --- a/tests/Unit/Jobs/CouponNotificationJobTest.php +++ b/tests/Unit/Jobs/CouponNotificationJobTest.php @@ -140,7 +140,12 @@ public function test_process_items_calls_notify_and_set_status_on_success() { ->method( 'set_notification_status' ) ->with( $coupon, NotificationStatus::NOTIFICATION_CREATED ); - $this->job->handle_process_items_action( [ $id, $topic ] ); + $this->job->handle_process_items_action( + [ + 'item_id' => $id, + 'topic' => $topic, + ] + ); } public function test_process_items_doesnt_calls_notify_when_no_args() { @@ -175,7 +180,12 @@ public function test_process_items_doesnt_calls_set_status_on_failure() { $this->coupon_helper->expects( $this->never() ) ->method( 'set_notification_status' ); - $this->job->handle_process_items_action( [ $id, $topic ] ); + $this->job->handle_process_items_action( + [ + 'item_id' => $id, + 'topic' => $topic, + ] + ); } public function test_get_after_notification_status() { @@ -220,9 +230,24 @@ function ( $id, $topic ) { } ); - $this->job->handle_process_items_action( [ $id, 'coupon.create' ] ); - $this->job->handle_process_items_action( [ $id, 'coupon.delete' ] ); - $this->job->handle_process_items_action( [ $id, 'coupon.update' ] ); + $this->job->handle_process_items_action( + [ + 'item_id' => $id, + 'topic' => 'coupon.create', + ] + ); + $this->job->handle_process_items_action( + [ + 'item_id' => $id, + 'topic' => 'coupon.delete', + ] + ); + $this->job->handle_process_items_action( + [ + 'item_id' => $id, + 'topic' => 'coupon.update', + ] + ); } public function test_dont_process_item_if_status_changed() { @@ -253,9 +278,24 @@ public function test_dont_process_item_if_status_changed() { $this->coupon_helper->expects( $this->never() )->method( 'set_notification_status' ); - $this->job->handle_process_items_action( [ $id, 'coupon.create' ] ); - $this->job->handle_process_items_action( [ $id, 'coupon.delete' ] ); - $this->job->handle_process_items_action( [ $id, 'coupon.update' ] ); + $this->job->handle_process_items_action( + [ + 'item_id' => $id, + 'topic' => 'coupon.create', + ] + ); + $this->job->handle_process_items_action( + [ + 'item_id' => $id, + 'topic' => 'coupon.delete', + ] + ); + $this->job->handle_process_items_action( + [ + 'item_id' => $id, + 'topic' => 'coupon.update', + ] + ); } public function test_mark_as_unsynced_when_delete() { @@ -277,6 +317,11 @@ public function test_mark_as_unsynced_when_delete() { $this->coupon_helper->expects( $this->once() ) ->method( 'mark_as_unsynced' ); - $this->job->handle_process_items_action( [ $id, 'coupon.delete' ] ); + $this->job->handle_process_items_action( + [ + 'item_id' => $id, + 'topic' => 'coupon.delete', + ] + ); } } From 9c172d118be29f328c5c8d03cdf4c20b816caa02 Mon Sep 17 00:00:00 2001 From: Miguel Perez Pellicer <5908855+puntope@users.noreply.github.com> Date: Thu, 29 Feb 2024 17:48:50 -0300 Subject: [PATCH 050/262] Adapt tests --- .../AbstractItemNotificationJob.php | 6 +- .../Notifications/AbstractNotificationJob.php | 18 --- .../Jobs/AbstractItemNotificationJobTest.php | 134 +++++++++++++++--- .../Unit/Jobs/ProductNotificationJobTest.php | 22 --- .../Unit/Jobs/SettingsNotificationJobTest.php | 2 +- .../Unit/Jobs/ShippingNotificationJobTest.php | 2 +- 6 files changed, 123 insertions(+), 61 deletions(-) diff --git a/src/Jobs/Notifications/AbstractItemNotificationJob.php b/src/Jobs/Notifications/AbstractItemNotificationJob.php index e32f5009dd..cff11bed9c 100644 --- a/src/Jobs/Notifications/AbstractItemNotificationJob.php +++ b/src/Jobs/Notifications/AbstractItemNotificationJob.php @@ -24,7 +24,7 @@ abstract class AbstractItemNotificationJob extends AbstractNotificationJob { * @param array $args Arguments with the item id and the topic */ protected function process_items( array $args ): void { - if ( ! isset( $args[0] ) || ! isset( $args[1] ) ) { + if ( ! isset( $args['item_id'] ) || ! isset( $args['topic'] ) ) { do_action( 'woocommerce_gla_error', 'Error sending the Notification. Topic and Item ID are mandatory', @@ -33,8 +33,8 @@ protected function process_items( array $args ): void { return; } - $item = $args[0]; - $topic = $args[1]; + $item = $args['item_id']; + $topic = $args['topic']; if ( $this->can_process( $item, $topic ) && $this->notifications_service->notify( $topic, $item ) ) { $this->set_status( $item, $this->get_after_notification_status( $topic ) ); diff --git a/src/Jobs/Notifications/AbstractNotificationJob.php b/src/Jobs/Notifications/AbstractNotificationJob.php index 65356ad322..a695f2c7e7 100644 --- a/src/Jobs/Notifications/AbstractNotificationJob.php +++ b/src/Jobs/Notifications/AbstractNotificationJob.php @@ -8,7 +8,6 @@ use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\AbstractActionSchedulerJob; use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\ActionSchedulerJobMonitor; use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\JobInterface; -use Automattic\WooCommerce\GoogleListingsAndAds\Value\NotificationStatus; defined( 'ABSPATH' ) || exit; @@ -69,23 +68,6 @@ public function schedule( array $args = [] ): void { } - /** - * Get the Notification Status after the notification happens - * - * @param string $topic - * @return string - */ - protected function get_after_notification_status( string $topic ): string { - if ( str_contains( $topic, '.create' ) ) { - return NotificationStatus::NOTIFICATION_CREATED; - } elseif ( str_contains( $topic, '.delete' ) ) { - return NotificationStatus::NOTIFICATION_DELETED; - } else { - return NotificationStatus::NOTIFICATION_UPDATED; - } - } - - /** * Can the job be scheduled. * diff --git a/tests/Unit/Jobs/AbstractItemNotificationJobTest.php b/tests/Unit/Jobs/AbstractItemNotificationJobTest.php index 70ba6d2cb5..5b63178c46 100644 --- a/tests/Unit/Jobs/AbstractItemNotificationJobTest.php +++ b/tests/Unit/Jobs/AbstractItemNotificationJobTest.php @@ -57,17 +57,35 @@ public function test_schedule_schedules_immediate_job() { $this->action_scheduler->expects( $this->once() ) ->method( 'has_scheduled_action' ) - ->with( $this->get_process_item_hook(), [ [ $id, $topic ] ] ) + ->with( + $this->get_process_item_hook(), + [ + [ + 'item_id' => $id, + 'topic' => $topic, + ], + ] + ) ->willReturn( false ); $this->action_scheduler->expects( $this->once() ) ->method( 'schedule_immediate' ) ->with( $this->get_process_item_hook(), - [ [ $id, $topic ] ] + [ + [ + 'item_id' => $id, + 'topic' => $topic, + ], + ] ); - $this->job->schedule( [ $id, $topic ] ); + $this->job->schedule( + [ + 'item_id' => $id, + 'topic' => $topic, + ] + ); } public function test_schedule_doesnt_schedules_immediate_job_if_already_scheduled() { @@ -78,12 +96,25 @@ public function test_schedule_doesnt_schedules_immediate_job_if_already_schedule $this->action_scheduler->expects( $this->once() ) ->method( 'has_scheduled_action' ) - ->with( $this->get_process_item_hook(), [ [ $id, $topic ] ] ) + ->with( + $this->get_process_item_hook(), + [ + [ + 'item_id' => $id, + 'topic' => $topic, + ], + ] + ) ->willReturn( true ); $this->action_scheduler->expects( $this->never() )->method( 'schedule_immediate' ); - $this->job->schedule( [ $id, $topic ] ); + $this->job->schedule( + [ + 'item_id' => $id, + 'topic' => $topic, + ] + ); } public function test_schedule_doesnt_schedules_immediate_job_if_not_enabled() { @@ -97,7 +128,12 @@ public function test_schedule_doesnt_schedules_immediate_job_if_not_enabled() { $this->action_scheduler->expects( $this->never() )->method( 'schedule_immediate' ); - $this->job->schedule( [ $id, $topic ] ); + $this->job->schedule( + [ + 'item_id' => $id, + 'topic' => $topic, + ] + ); } public function test_process_items_calls_notify_and_set_status_on_success() { @@ -125,7 +161,12 @@ public function test_process_items_calls_notify_and_set_status_on_success() { ->method( 'set_notification_status' ) ->with( $item, NotificationStatus::NOTIFICATION_CREATED ); - $this->job->handle_process_items_action( [ $id, $topic ] ); + $this->job->handle_process_items_action( + [ + 'item_id' => $id, + 'topic' => $topic, + ] + ); } public function test_process_items_doesnt_calls_notify_when_no_args() { @@ -160,7 +201,12 @@ public function test_process_items_doesnt_calls_set_status_on_failure() { $this->job->get_helper()->expects( $this->never() ) ->method( 'set_notification_status' ); - $this->job->handle_process_items_action( [ $id, $topic ] ); + $this->job->handle_process_items_action( + [ + 'item_id' => $id, + 'topic' => $topic, + ] + ); } public function test_get_after_notification_status() { @@ -195,9 +241,9 @@ public function test_get_after_notification_status() { ->method( 'set_notification_status' ) ->willReturnCallback( function ( $id, $topic ) { - if ( $topic === 'product.create' ) { + if ( $topic === $this->get_topic_name() . '.create' ) { return NotificationStatus::NOTIFICATION_CREATED; - } elseif ( $topic === 'product.delete' ) { + } elseif ( $topic === $this->get_topic_name() . '.delete' ) { return NotificationStatus::NOTIFICATION_DELETED; } else { return NotificationStatus::NOTIFICATION_UPDATED; @@ -205,9 +251,24 @@ function ( $id, $topic ) { } ); - $this->job->handle_process_items_action( [ $id, $this->get_topic_name() . '.create' ] ); - $this->job->handle_process_items_action( [ $id, $this->get_topic_name() . '.delete' ] ); - $this->job->handle_process_items_action( [ $id, $this->get_topic_name() . '.update' ] ); + $this->job->handle_process_items_action( + [ + 'item_id' => $id, + 'topic' => $this->get_topic_name() . '.create', + ] + ); + $this->job->handle_process_items_action( + [ + 'item_id' => $id, + 'topic' => $this->get_topic_name() . '.delete', + ] + ); + $this->job->handle_process_items_action( + [ + 'item_id' => $id, + 'topic' => $this->get_topic_name() . '.update', + ] + ); } public function test_dont_process_item_if_status_changed() { @@ -238,9 +299,50 @@ public function test_dont_process_item_if_status_changed() { $this->job->get_helper()->expects( $this->never() )->method( 'set_notification_status' ); - $this->job->handle_process_items_action( [ $id, $this->get_topic_name() . '.create' ] ); - $this->job->handle_process_items_action( [ $id, $this->get_topic_name() . '.delete' ] ); - $this->job->handle_process_items_action( [ $id, $this->get_topic_name() . '.update' ] ); + $this->job->handle_process_items_action( + [ + 'item_id' => $id, + 'topic' => $this->get_topic_name() . '.create', + ] + ); + $this->job->handle_process_items_action( + [ + 'item_id' => $id, + 'topic' => $this->get_topic_name() . '.delete', + ] + ); + $this->job->handle_process_items_action( + [ + 'item_id' => $id, + 'topic' => $this->get_topic_name() . '.update', + ] + ); + } + + public function test_mark_as_unsynced_when_delete() { + $item = $this->create_item(); + $id = $item->get_id(); + + $this->job->get_helper()->expects( $this->once() ) + ->method( 'should_trigger_delete_notification' ) + ->with( $item ) + ->willReturn( true ); + + $this->job->get_helper()->expects( $this->exactly( 3 ) ) + ->method( 'get_wc_' . $this->get_topic_name() ) + ->with( $id ) + ->willReturn( $item ); + + $this->notification_service->expects( $this->once() )->method( 'notify' )->willReturn( true ); + $this->job->get_helper()->expects( $this->once() ) + ->method( 'mark_as_unsynced' ); + + $this->job->handle_process_items_action( + [ + 'item_id' => $id, + 'topic' => $this->get_topic_name() . '.delete', + ] + ); } public function get_name() { diff --git a/tests/Unit/Jobs/ProductNotificationJobTest.php b/tests/Unit/Jobs/ProductNotificationJobTest.php index 5e0f3d5146..86ca89de61 100644 --- a/tests/Unit/Jobs/ProductNotificationJobTest.php +++ b/tests/Unit/Jobs/ProductNotificationJobTest.php @@ -36,26 +36,4 @@ public function get_job() { public function create_item() { return WC_Helper_Product::create_simple_product(); } - - public function test_mark_as_unsynced_when_delete() { - /** @var \WC_Product $product */ - $product = WC_Helper_Product::create_simple_product(); - $id = $product->get_id(); - - $this->product_helper->expects( $this->once() ) - ->method( 'should_trigger_delete_notification' ) - ->with( $product ) - ->willReturn( true ); - - $this->product_helper->expects( $this->exactly( 3 ) ) - ->method( 'get_wc_product' ) - ->with( $id ) - ->willReturn( $product ); - - $this->notification_service->expects( $this->once() )->method( 'notify' )->willReturn( true ); - $this->product_helper->expects( $this->once() ) - ->method( 'mark_as_unsynced' ); - - $this->job->handle_process_items_action( [ $id, 'product.delete' ] ); - } } diff --git a/tests/Unit/Jobs/SettingsNotificationJobTest.php b/tests/Unit/Jobs/SettingsNotificationJobTest.php index 7c5e86d72a..088c46ebb6 100644 --- a/tests/Unit/Jobs/SettingsNotificationJobTest.php +++ b/tests/Unit/Jobs/SettingsNotificationJobTest.php @@ -15,7 +15,7 @@ class SettingsNotificationJobTest extends AbstractNotificationJobTest { public function get_topic() { - return $this->notification_service::TOPIC_SETTINGS_UPDATED; + return 'settings.update'; } public function get_job_name() { diff --git a/tests/Unit/Jobs/ShippingNotificationJobTest.php b/tests/Unit/Jobs/ShippingNotificationJobTest.php index d57ba4efca..1c3ef6351b 100644 --- a/tests/Unit/Jobs/ShippingNotificationJobTest.php +++ b/tests/Unit/Jobs/ShippingNotificationJobTest.php @@ -15,7 +15,7 @@ class ShippingNotificationJobTest extends AbstractNotificationJobTest { public function get_topic() { - return $this->notification_service::TOPIC_SHIPPING_UPDATED; + return 'shipping.update'; } public function get_job_name() { From c753ebecb89909d863e2dd88e4563b2cd276adab Mon Sep 17 00:00:00 2001 From: Miguel Perez Pellicer <5908855+puntope@users.noreply.github.com> Date: Thu, 29 Feb 2024 18:44:55 -0300 Subject: [PATCH 051/262] PHPCS --- src/Product/SyncerHooks.php | 28 ++++++++++++++++++++++---- tests/Unit/Product/SyncerHooksTest.php | 27 ++++++++++++++++++++++--- 2 files changed, 48 insertions(+), 7 deletions(-) diff --git a/src/Product/SyncerHooks.php b/src/Product/SyncerHooks.php index 75ce19a514..314af6e304 100644 --- a/src/Product/SyncerHooks.php +++ b/src/Product/SyncerHooks.php @@ -242,13 +242,28 @@ protected function handle_update_products( array $products ) { protected function handle_update_product_notification( WC_Product $product ) { if ( $this->product_helper->should_trigger_create_notification( $product ) ) { $this->product_helper->set_notification_status( $product, NotificationStatus::NOTIFICATION_PENDING_CREATE ); - $this->product_notification_job->schedule( [ $product->get_id(), NotificationsService::TOPIC_PRODUCT_CREATED ] ); + $this->product_notification_job->schedule( + [ + 'item_id' => $product->get_id(), + 'topic' => NotificationsService::TOPIC_PRODUCT_CREATED, + ] + ); } elseif ( $this->product_helper->should_trigger_update_notification( $product ) ) { $this->product_helper->set_notification_status( $product, NotificationStatus::NOTIFICATION_PENDING_UPDATE ); - $this->product_notification_job->schedule( [ $product->get_id(), NotificationsService::TOPIC_PRODUCT_UPDATED ] ); + $this->product_notification_job->schedule( + [ + 'item_id' => $product->get_id(), + 'topic' => NotificationsService::TOPIC_PRODUCT_UPDATED, + ] + ); } elseif ( $this->product_helper->should_trigger_delete_notification( $product ) ) { $this->product_helper->set_notification_status( $product, NotificationStatus::NOTIFICATION_PENDING_DELETE ); - $this->product_notification_job->schedule( [ $product->get_id(), NotificationsService::TOPIC_PRODUCT_DELETED ] ); + $this->product_notification_job->schedule( + [ + 'item_id' => $product->get_id(), + 'topic' => NotificationsService::TOPIC_PRODUCT_DELETED, + ] + ); } } @@ -261,7 +276,12 @@ protected function handle_delete_product( int $product_id ) { $product = wc_get_product( $product_id ); if ( $product instanceof WC_Product && $this->notifications_service->is_enabled() && $this->product_helper->should_trigger_delete_notification( $product ) ) { $this->product_helper->set_notification_status( $product, NotificationStatus::NOTIFICATION_PENDING_DELETE ); - $this->product_notification_job->schedule( [ $product->get_id(), NotificationsService::TOPIC_PRODUCT_DELETED ] ); + $this->product_notification_job->schedule( + [ + 'item_id' => $product->get_id(), + 'topic' => NotificationsService::TOPIC_PRODUCT_DELETED, + ] + ); return; } diff --git a/tests/Unit/Product/SyncerHooksTest.php b/tests/Unit/Product/SyncerHooksTest.php index 0d7b58ee3d..43bc21ff0c 100644 --- a/tests/Unit/Product/SyncerHooksTest.php +++ b/tests/Unit/Product/SyncerHooksTest.php @@ -282,7 +282,14 @@ public function test_create_product_triggers_notification_created() { $product = WC_Helper_Product::create_simple_product( true, [ 'status' => 'draft' ] ); $this->notification_service->expects( $this->once() )->method( 'is_enabled' )->willReturn( true ); $this->product_notification_job->expects( $this->once() ) - ->method( 'schedule' )->with( $this->equalTo( [ $product->get_id(), NotificationsService::TOPIC_PRODUCT_CREATED ] ) ); + ->method( 'schedule' )->with( + $this->equalTo( + [ + 'item_id' => $product->get_id(), + 'topic' => NotificationsService::TOPIC_PRODUCT_CREATED, + ] + ) + ); $product->set_status( 'publish' ); $product->save(); } @@ -291,7 +298,14 @@ public function test_create_product_triggers_notification_updated() { $product = WC_Helper_Product::create_simple_product( true, [ 'status' => 'draft' ] ); $this->notification_service->expects( $this->once() )->method( 'is_enabled' )->willReturn( true ); $this->product_notification_job->expects( $this->once() ) - ->method( 'schedule' )->with( $this->equalTo( [ $product->get_id(), NotificationsService::TOPIC_PRODUCT_UPDATED ] ) ); + ->method( 'schedule' )->with( + $this->equalTo( + [ + 'item_id' => $product->get_id(), + 'topic' => NotificationsService::TOPIC_PRODUCT_UPDATED, + ] + ) + ); $product->set_status( 'publish' ); $this->product_helper->set_notification_status( $product, NotificationStatus::NOTIFICATION_CREATED ); $product->save(); @@ -301,7 +315,14 @@ public function test_create_product_triggers_notification_delete() { $product = WC_Helper_Product::create_simple_product( true, [ 'status' => 'draft' ] ); $this->notification_service->expects( $this->once() )->method( 'is_enabled' )->willReturn( true ); $this->product_notification_job->expects( $this->once() ) - ->method( 'schedule' )->with( $this->equalTo( [ $product->get_id(), NotificationsService::TOPIC_PRODUCT_DELETED ] ) ); + ->method( 'schedule' )->with( + $this->equalTo( + [ + 'item_id' => $product->get_id(), + 'topic' => NotificationsService::TOPIC_PRODUCT_DELETED, + ] + ) + ); $product->set_status( 'publish' ); $product->add_meta_data( '_wc_gla_visibility', ChannelVisibility::DONT_SYNC_AND_SHOW, true ); $this->product_helper->set_notification_status( $product, NotificationStatus::NOTIFICATION_CREATED ); From 517be8c72dfe858489016790f3deda5ff7e18fac Mon Sep 17 00:00:00 2001 From: Miguel Perez Pellicer <5908855+puntope@users.noreply.github.com> Date: Fri, 1 Mar 2024 09:51:52 -0300 Subject: [PATCH 052/262] Filter allowed topis --- src/Google/NotificationsService.php | 14 +++- .../Unit/Google/NotificationsServiceTest.php | 76 +++++++++++++++---- 2 files changed, 76 insertions(+), 14 deletions(-) diff --git a/src/Google/NotificationsService.php b/src/Google/NotificationsService.php index b0810d0ec3..cde07ab960 100644 --- a/src/Google/NotificationsService.php +++ b/src/Google/NotificationsService.php @@ -28,6 +28,18 @@ class NotificationsService implements Service { public const TOPIC_SHIPPING_UPDATED = 'shipping.update'; public const TOPIC_SETTINGS_UPDATED = 'settings.update'; + // Constant used to get all the allowed topics + public const ALLOWED_TOPICS = [ + self::TOPIC_PRODUCT_CREATED, + self::TOPIC_PRODUCT_DELETED, + self::TOPIC_PRODUCT_UPDATED, + self::TOPIC_COUPON_CREATED, + self::TOPIC_COUPON_DELETED, + self::TOPIC_COUPON_UPDATED, + self::TOPIC_SHIPPING_UPDATED, + self::TOPIC_SETTINGS_UPDATED, + ]; + /** * The url to send the notification * @@ -62,7 +74,7 @@ public function notify( string $topic, $item_id = null ): bool { * @param int $item_id The item_id for the notification. * @param string $topic The topic for the notification. */ - if ( ! apply_filters( 'woocommerce_gla_notify', true, $item_id, $topic ) ) { + if ( ! apply_filters( 'woocommerce_gla_notify', in_array( $topic, self::ALLOWED_TOPICS, true ), $item_id, $topic ) ) { return false; } diff --git a/tests/Unit/Google/NotificationsServiceTest.php b/tests/Unit/Google/NotificationsServiceTest.php index 89bce65173..d45e19630c 100644 --- a/tests/Unit/Google/NotificationsServiceTest.php +++ b/tests/Unit/Google/NotificationsServiceTest.php @@ -1,19 +1,19 @@ service = $this->get_mock(); $this->service->expects( $this->once() )->method( 'do_request' )->willReturn( new \WP_Error( 'error', 'error message' ) ); - $this->assertFalse( $this->service->notify( 'topic', 1 ) ); + $this->assertFalse( $this->service->notify( 'product.create', 1 ) ); $this->assertEquals( did_action( 'woocommerce_gla_error' ), 1 ); } @@ -107,10 +129,38 @@ public function test_notify_response_error() { ], ] ); - $this->assertFalse( $this->service->notify( 'topic', 1 ) ); + $this->assertFalse( $this->service->notify( 'product.create', 1 ) ); $this->assertEquals( did_action( 'woocommerce_gla_error' ), 1 ); } + /** + * Test notify() function with valid and invalid topics + */ + public function test_notify_valid_and_invalid_topics() { + $this->service = $this->get_mock(); + $this->service->expects( $this->exactly( count( self::ALLOWED_TOPICS ) ) ) + ->method( 'do_request' ) + ->willReturn( [ 'code' => 200 ] ); + + foreach ( self::ALLOWED_TOPICS as $topic ) { + $this->assertTrue( $this->service->notify( $topic, 1 ) ); + } + + $this->assertFalse( $this->service->notify( 'invalid.created', 1 ) ); + } + + /** + * Test notify() function when it is blocked by `woocommerce_gla_notify` hook + */ + public function test_notify_blocked_by_hook() { + $this->service = $this->get_mock(); + $this->service->expects( $this->never() )->method( 'do_request' ); + + add_filter( 'woocommerce_gla_notify', '__return_false' ); + $this->assertFalse( $this->service->notify( 'product.created', 1 ) ); + remove_filter( 'woocommerce_gla_notify', '__return_false' ); + } + /** * Mocks the service * From 8950697f0757c02a835a12183eedb669303abd02 Mon Sep 17 00:00:00 2001 From: Ian Yu-Hsun Lin Date: Mon, 4 Mar 2024 18:21:11 +0800 Subject: [PATCH 053/262] Add an API to get Google WPCOM auth URL --- .../Controllers/RestAPI/AuthController.php | 181 ++++++++++++++++++ .../RESTServiceProvider.php | 2 + 2 files changed, 183 insertions(+) create mode 100644 src/API/Site/Controllers/RestAPI/AuthController.php diff --git a/src/API/Site/Controllers/RestAPI/AuthController.php b/src/API/Site/Controllers/RestAPI/AuthController.php new file mode 100644 index 0000000000..bad900229a --- /dev/null +++ b/src/API/Site/Controllers/RestAPI/AuthController.php @@ -0,0 +1,181 @@ + '/google/setup-mc', + 'reconnect' => '/google/settings', + ]; + + /** + * AuthController constructor. + * + * @param RESTServer $server + */ + public function __construct( RESTServer $server ) { + parent::__construct( $server ); + } + + /** + * Registers the routes for the objects of the controller. + */ + public function register_routes() { + $this->register_route( + 'rest-api/authorize', + [ + [ + 'methods' => TransportMethods::READABLE, + 'callback' => $this->get_authorize_callback(), + 'permission_callback' => $this->get_permission_callback(), + 'args' => $this->get_auth_params(), + ], + ] + ); + } + + /** + * Get the callback function for the authorization request. + * + * @return callable + */ + protected function get_authorize_callback(): callable { + return function ( Request $request ) { + /* + The full auth URL example: + + https://public-api.wordpress.com/oauth2/authorize? + client_id=CLIENT_ID& + redirect_uri=PARTNER_REDIRECT_URL& + response_type=code& + blog=BLOD_ID& + scope=wc-partner-access& + state=BASE64_ENCODED_STRING + + Content of state is an URL query string where the value of its parameter "redirect_url" is being URL encoded. + E.g. + nonce=nonce-123&redirect_url=https%3A%2F%2Fmerchant-site.example.com%2Fwp-admin%2Fadmin.php%3Fpage%3Dwc-admin%26path%3D%2Fgoogle%2Fsetup-mc + + where its URL decoded version is: + nonce=nonce-123&redirect_url=https://merchant-site.example.com/wp-admin/admin.php?page=wc-admin&path=/google/setup-mc + + Ref: https://developer.wordpress.com/docs/oauth2/ + */ + + try { + $auth_url = 'https://public-api.wordpress.com/oauth2/authorize'; + + // TODO: Call an API by Google with merchant information and get the below data. + // We'd probably need use WCS to communicate with the new API. + $client_id = '91299'; + $redirect_uri = 'https://woo.com'; + $nonce = 'nonce-123'; + + $response_type = 'code'; + $blog_id = Jetpack_Options::get_option( 'id' ); + $scope = 'wc-partner-access'; + + $next = $request->get_param( 'next_page_name' ); + $path = self::NEXT_PATH_MAPPING[ $next ]; + $merchant_redirect_url = urlencode_deep( admin_url( "admin.php?page=wc-admin&path={$path}" ) ); + + $state = base64_encode( + build_query( + [ + 'nonce' => $nonce, + 'redirect_url' => $merchant_redirect_url, + ] + ) + ); + + $auth_url = esc_url_raw( + add_query_arg( + [ + 'blog' => $blog_id, + 'client_id' => $client_id, + 'redirect_uri' => $redirect_uri, + 'response_type' => $response_type, + 'scope' => $scope, + 'state' => $state, + ], + $auth_url + ) + ); + + $response = [ + 'auth_url' => $auth_url, + ]; + + return $this->prepare_item_for_response( $response, $request ); + } catch ( Exception $e ) { + return $this->response_from_exception( $e ); + } + }; + } + + /** + * Get the query params for the authorize request. + * + * @return array + */ + protected function get_auth_params(): array { + return [ + 'next_page_name' => [ + 'description' => __( 'Indicates the next page name mapped to the redirect URL when redirected back from Google WPCOM App authorization.', 'google-listings-and-ads' ), + 'type' => 'string', + 'default' => array_key_first( self::NEXT_PATH_MAPPING ), + 'enum' => array_keys( self::NEXT_PATH_MAPPING ), + 'validate_callback' => 'rest_validate_request_arg', + ], + ]; + } + + /** + * Get the item schema properties for the controller. + * + * @return array + */ + protected function get_schema_properties(): array { + return [ + 'auth_url' => [ + 'type' => 'string', + 'description' => __( 'The authorization URL for granting access to Google WPCOM App.', 'google-listings-and-ads' ), + 'context' => [ 'view' ], + ], + ]; + } + + /** + * Get the item schema name for the controller. + * + * Used for building the API response schema. + * + * @return string + */ + protected function get_schema_title(): string { + return 'rest_api_authorize'; + } +} diff --git a/src/Internal/DependencyManagement/RESTServiceProvider.php b/src/Internal/DependencyManagement/RESTServiceProvider.php index d8cf483d77..7d69d60465 100644 --- a/src/Internal/DependencyManagement/RESTServiceProvider.php +++ b/src/Internal/DependencyManagement/RESTServiceProvider.php @@ -49,6 +49,7 @@ use Automattic\WooCommerce\GoogleListingsAndAds\API\Site\Controllers\MerchantCenter\SupportedCountriesController; use Automattic\WooCommerce\GoogleListingsAndAds\API\Site\Controllers\MerchantCenter\SyncableProductsCountController; use Automattic\WooCommerce\GoogleListingsAndAds\API\Site\Controllers\MerchantCenter\TargetAudienceController; +use Automattic\WooCommerce\GoogleListingsAndAds\API\Site\Controllers\RestAPI\AuthController as RestAPIAuthController; use Automattic\WooCommerce\GoogleListingsAndAds\DB\ProductFeedQueryHelper; use Automattic\WooCommerce\GoogleListingsAndAds\DB\Query\AttributeMappingRulesQuery; use Automattic\WooCommerce\GoogleListingsAndAds\DB\Query\BudgetRecommendationQuery; @@ -138,6 +139,7 @@ public function register() { $this->share( AttributeMappingCategoriesController::class ); $this->share( AttributeMappingSyncerController::class, ProductSyncStats::class ); $this->share( TourController::class ); + $this->share( RestAPIAuthController::class ); } /** From e4d004cbe009edd904b66b40c115a39d00b5a80e Mon Sep 17 00:00:00 2001 From: Ian Yu-Hsun Lin Date: Tue, 5 Mar 2024 17:05:10 +0800 Subject: [PATCH 054/262] Add unit tests for RestAPI/AuthController --- .../RestAPI/AuthControllerTest.php | 116 ++++++++++++++++++ 1 file changed, 116 insertions(+) create mode 100644 tests/Unit/API/Site/Controllers/RestAPI/AuthControllerTest.php diff --git a/tests/Unit/API/Site/Controllers/RestAPI/AuthControllerTest.php b/tests/Unit/API/Site/Controllers/RestAPI/AuthControllerTest.php new file mode 100644 index 0000000000..6b419576c7 --- /dev/null +++ b/tests/Unit/API/Site/Controllers/RestAPI/AuthControllerTest.php @@ -0,0 +1,116 @@ +controller = new AuthController( $this->server ); + $this->controller->register(); + + // Mock the Blog ID from Jetpack + add_filter( + 'jetpack_options', + function ( $value, $name ) { + if ( $name === 'id' ) { + return self::DUMMY_BLOG_ID; + } + + return $value; + }, + 10, + 2 + ); + + // Mock admin URL. + add_filter( + 'admin_url', + function ( $url, $path ) { + return 'https://admin-example.com/wp-admin/' . $path; + }, + 10, + 2 + ); + } + + public function test_authorize() { + $blog_id = self::DUMMY_BLOG_ID; + + $client_id = '91299'; + $redirect_uri = 'https://woo.com'; + $nonce = 'nonce-123'; + + $merchant_redirect_url = 'https://admin-example.com/wp-admin/admin.php?page=wc-admin&path=/google/setup-mc'; + $merchant_redirect_url_encoded = urlencode_deep( $merchant_redirect_url ); + $state_raw = "nonce={$nonce}&redirect_url={$merchant_redirect_url_encoded}"; + $state = base64_encode( $state_raw ); + + $expected_auth_url = 'https://public-api.wordpress.com/oauth2/authorize'; + $expected_auth_url .= "?blog={$blog_id}"; + $expected_auth_url .= "&client_id={$client_id}"; + $expected_auth_url .= "&redirect_uri={$redirect_uri}"; + $expected_auth_url .= '&response_type=code'; + $expected_auth_url .= '&scope=wc-partner-access'; + $expected_auth_url .= "&state={$state}"; + $expected_auth_url = esc_url_raw( $expected_auth_url ); + + $response = $this->do_request( self::ROUTE_AUTHORIZE, 'GET' ); + $data = $response->get_data(); + $auth_url = $data['auth_url']; + + // Compare the auth URLs. + // Removing the "=" sign from the end of the string because sometimes + // base64_encode function will add "=" signs as paddings. + $this->assertEquals( + rtrim( $expected_auth_url, '=' ), + rtrim( $auth_url, '=' ) + ); + + $this->assertEquals( 200, $response->get_status() ); + + // Compare the state query parameters from the auth URL. + $parsed_url = wp_parse_url( $auth_url ); + parse_str( $parsed_url['query'], $parsed_query ); + $response_state_raw = base64_decode( $parsed_query['state'] ); + $this->assertEquals( + $state_raw, + $response_state_raw + ); + + // Ensure the base64 encoded state query has correct value. + parse_str( $response_state_raw, $parsed_state ); + $this->assertEquals( + $nonce, + $parsed_state['nonce'] + ); + $this->assertEquals( + $merchant_redirect_url, + $parsed_state['redirect_url'] + ); + } + + public function test_authorize_invalid_parameter() { + $response = $this->do_request( self::ROUTE_AUTHORIZE, 'GET', [ 'next_page_name' => 'invalid' ] ); + + $this->assertEquals( 'rest_invalid_param', $response->get_data()['code'] ); + $this->assertEquals( 400, $response->get_status() ); + } +} From d4c009935cd74f907e297dcfa193cd9c665579a4 Mon Sep 17 00:00:00 2001 From: Ian Yu-Hsun Lin Date: Wed, 6 Mar 2024 15:29:41 +0800 Subject: [PATCH 055/262] Extract responsibility from AuthController to a new OAuthService --- .../Controllers/RestAPI/AuthController.php | 75 +++------------ src/API/WP/OAuthService.php | 96 +++++++++++++++++++ .../CoreServiceProvider.php | 5 + .../RESTServiceProvider.php | 3 +- 4 files changed, 116 insertions(+), 63 deletions(-) create mode 100644 src/API/WP/OAuthService.php diff --git a/src/API/Site/Controllers/RestAPI/AuthController.php b/src/API/Site/Controllers/RestAPI/AuthController.php index bad900229a..7cdc743e05 100644 --- a/src/API/Site/Controllers/RestAPI/AuthController.php +++ b/src/API/Site/Controllers/RestAPI/AuthController.php @@ -5,9 +5,9 @@ use Automattic\WooCommerce\GoogleListingsAndAds\API\Site\Controllers\BaseController; use Automattic\WooCommerce\GoogleListingsAndAds\API\TransportMethods; +use Automattic\WooCommerce\GoogleListingsAndAds\API\WP\OAuthService; use Automattic\WooCommerce\GoogleListingsAndAds\Proxies\RESTServer; use Exception; -use Jetpack_Options; use WP_REST_Request as Request; defined( 'ABSPATH' ) || exit; @@ -19,6 +19,11 @@ */ class AuthController extends BaseController { + /** + * @var OAuthService + */ + protected $oauth_service; + /** * Mapping between the client page name and its path. * The first value is also used as a default, @@ -34,10 +39,12 @@ class AuthController extends BaseController { /** * AuthController constructor. * - * @param RESTServer $server + * @param RESTServer $server + * @param OAuthService $oauth_service */ - public function __construct( RESTServer $server ) { + public function __construct( RESTServer $server, OAuthService $oauth_service ) { parent::__construct( $server ); + $this->oauth_service = $oauth_service; } /** @@ -64,66 +71,10 @@ public function register_routes() { */ protected function get_authorize_callback(): callable { return function ( Request $request ) { - /* - The full auth URL example: - - https://public-api.wordpress.com/oauth2/authorize? - client_id=CLIENT_ID& - redirect_uri=PARTNER_REDIRECT_URL& - response_type=code& - blog=BLOD_ID& - scope=wc-partner-access& - state=BASE64_ENCODED_STRING - - Content of state is an URL query string where the value of its parameter "redirect_url" is being URL encoded. - E.g. - nonce=nonce-123&redirect_url=https%3A%2F%2Fmerchant-site.example.com%2Fwp-admin%2Fadmin.php%3Fpage%3Dwc-admin%26path%3D%2Fgoogle%2Fsetup-mc - - where its URL decoded version is: - nonce=nonce-123&redirect_url=https://merchant-site.example.com/wp-admin/admin.php?page=wc-admin&path=/google/setup-mc - - Ref: https://developer.wordpress.com/docs/oauth2/ - */ - try { - $auth_url = 'https://public-api.wordpress.com/oauth2/authorize'; - - // TODO: Call an API by Google with merchant information and get the below data. - // We'd probably need use WCS to communicate with the new API. - $client_id = '91299'; - $redirect_uri = 'https://woo.com'; - $nonce = 'nonce-123'; - - $response_type = 'code'; - $blog_id = Jetpack_Options::get_option( 'id' ); - $scope = 'wc-partner-access'; - - $next = $request->get_param( 'next_page_name' ); - $path = self::NEXT_PATH_MAPPING[ $next ]; - $merchant_redirect_url = urlencode_deep( admin_url( "admin.php?page=wc-admin&path={$path}" ) ); - - $state = base64_encode( - build_query( - [ - 'nonce' => $nonce, - 'redirect_url' => $merchant_redirect_url, - ] - ) - ); - - $auth_url = esc_url_raw( - add_query_arg( - [ - 'blog' => $blog_id, - 'client_id' => $client_id, - 'redirect_uri' => $redirect_uri, - 'response_type' => $response_type, - 'scope' => $scope, - 'state' => $state, - ], - $auth_url - ) - ); + $next = $request->get_param( 'next_page_name' ); + $path = self::NEXT_PATH_MAPPING[ $next ]; + $auth_url = $this->oauth_service->get_auth_url( $path ); $response = [ 'auth_url' => $auth_url, diff --git a/src/API/WP/OAuthService.php b/src/API/WP/OAuthService.php new file mode 100644 index 0000000000..be598cf0ae --- /dev/null +++ b/src/API/WP/OAuthService.php @@ -0,0 +1,96 @@ +get_data_from_google(); + + $merchant_redirect_url = urlencode_deep( admin_url( "admin.php?page=wc-admin&path={$path}" ) ); + + $state = base64_encode( + build_query( + [ + 'nonce' => $google_data['nonce'], + 'redirect_url' => $merchant_redirect_url, + ] + ) + ); + + $auth_url = esc_url_raw( + add_query_arg( + [ + 'blog' => Jetpack_Options::get_option( 'id' ), + 'client_id' => $google_data['client_id'], + 'redirect_uri' => $google_data['redirect_uri'], + 'response_type' => self::RESPONSE_TYPE, + 'scope' => self::SCOPE, + 'state' => $state, + ], + self::AUTH_URL + ) + ); + + return $auth_url; + } + + /** + * Calls an API by Google to get required information in order to form an auth URL. + * + * TODO: Call an actual API by Google. + * We'd probably need use WCS to communicate with the new API. + * + * @return array An associative array contains required information from Google. + */ + private function get_data_from_google(): array { + return [ + 'client_id' => '91299', + 'redirect_uri' => 'https://woo.com', + 'nonce' => 'nonce-123', + ]; + } +} diff --git a/src/Internal/DependencyManagement/CoreServiceProvider.php b/src/Internal/DependencyManagement/CoreServiceProvider.php index 58c5f88510..da6e4caadb 100644 --- a/src/Internal/DependencyManagement/CoreServiceProvider.php +++ b/src/Internal/DependencyManagement/CoreServiceProvider.php @@ -26,6 +26,7 @@ use Automattic\WooCommerce\GoogleListingsAndAds\API\Google\Settings as GoogleSettings; use Automattic\WooCommerce\GoogleListingsAndAds\API\Google\AdsAssetGroupAsset; use Automattic\WooCommerce\GoogleListingsAndAds\API\Site\RESTControllers; +use Automattic\WooCommerce\GoogleListingsAndAds\API\WP\OAuthService; use Automattic\WooCommerce\GoogleListingsAndAds\Assets\AssetsHandler; use Automattic\WooCommerce\GoogleListingsAndAds\Assets\AssetsHandlerInterface; use Automattic\WooCommerce\GoogleListingsAndAds\ConnectionTest; @@ -211,6 +212,7 @@ class CoreServiceProvider extends AbstractServiceProvider { MerchantAccountService::class => true, AttributeMapping::class => true, MarketingChannelRegistrar::class => true, + OAuthService::class => true, ]; /** @@ -252,6 +254,9 @@ public function register(): void { // Set up Notifications service. $this->share_with_tags( NotificationsService::class ); + // Set up OAuthService service. + $this->share_with_tags( OAuthService::class ); + $this->getLeagueContainer() ->inflector( MerchantCenterAwareInterface::class ) ->invokeMethod( 'set_merchant_center_object', [ MerchantCenterService::class ] ); diff --git a/src/Internal/DependencyManagement/RESTServiceProvider.php b/src/Internal/DependencyManagement/RESTServiceProvider.php index 7d69d60465..5b66e9bb26 100644 --- a/src/Internal/DependencyManagement/RESTServiceProvider.php +++ b/src/Internal/DependencyManagement/RESTServiceProvider.php @@ -50,6 +50,7 @@ use Automattic\WooCommerce\GoogleListingsAndAds\API\Site\Controllers\MerchantCenter\SyncableProductsCountController; use Automattic\WooCommerce\GoogleListingsAndAds\API\Site\Controllers\MerchantCenter\TargetAudienceController; use Automattic\WooCommerce\GoogleListingsAndAds\API\Site\Controllers\RestAPI\AuthController as RestAPIAuthController; +use Automattic\WooCommerce\GoogleListingsAndAds\API\WP\OAuthService; use Automattic\WooCommerce\GoogleListingsAndAds\DB\ProductFeedQueryHelper; use Automattic\WooCommerce\GoogleListingsAndAds\DB\Query\AttributeMappingRulesQuery; use Automattic\WooCommerce\GoogleListingsAndAds\DB\Query\BudgetRecommendationQuery; @@ -139,7 +140,7 @@ public function register() { $this->share( AttributeMappingCategoriesController::class ); $this->share( AttributeMappingSyncerController::class, ProductSyncStats::class ); $this->share( TourController::class ); - $this->share( RestAPIAuthController::class ); + $this->share( RestAPIAuthController::class, OAuthService::class ); } /** From b3ff1e68827ef3dd46f504b9c82f064bf6aaea54 Mon Sep 17 00:00:00 2001 From: Ian Yu-Hsun Lin Date: Wed, 6 Mar 2024 16:31:53 +0800 Subject: [PATCH 056/262] Update unit test for RestAPI/AuthController --- .../RestAPI/AuthControllerTest.php | 99 ++++++------------- 1 file changed, 31 insertions(+), 68 deletions(-) diff --git a/tests/Unit/API/Site/Controllers/RestAPI/AuthControllerTest.php b/tests/Unit/API/Site/Controllers/RestAPI/AuthControllerTest.php index 6b419576c7..8a1a95d43f 100644 --- a/tests/Unit/API/Site/Controllers/RestAPI/AuthControllerTest.php +++ b/tests/Unit/API/Site/Controllers/RestAPI/AuthControllerTest.php @@ -3,7 +3,10 @@ namespace Automattic\WooCommerce\GoogleListingsAndAds\Tests\Unit\API\Site\Controllers\RestAPI; use Automattic\WooCommerce\GoogleListingsAndAds\API\Site\Controllers\RestAPI\AuthController; +use Automattic\WooCommerce\GoogleListingsAndAds\API\WP\OAuthService; use Automattic\WooCommerce\GoogleListingsAndAds\Tests\Framework\RESTControllerUnitTest; +use PHPUnit\Framework\MockObject\MockObject; +use Exception; /** @@ -13,7 +16,8 @@ */ class AuthControllerTest extends RESTControllerUnitTest { - public const DUMMY_BLOG_ID = '123'; + /** @var MockObject|OAuthService $oauth_service */ + protected $oauth_service; /** @var AuthController $controller */ protected $controller; @@ -23,88 +27,36 @@ class AuthControllerTest extends RESTControllerUnitTest { public function setUp(): void { parent::setUp(); - $this->controller = new AuthController( $this->server ); + $this->oauth_service = $this->createMock( OAuthService::class ); + $this->controller = new AuthController( $this->server, $this->oauth_service ); $this->controller->register(); - - // Mock the Blog ID from Jetpack - add_filter( - 'jetpack_options', - function ( $value, $name ) { - if ( $name === 'id' ) { - return self::DUMMY_BLOG_ID; - } - - return $value; - }, - 10, - 2 - ); - - // Mock admin URL. - add_filter( - 'admin_url', - function ( $url, $path ) { - return 'https://admin-example.com/wp-admin/' . $path; - }, - 10, - 2 - ); } public function test_authorize() { - $blog_id = self::DUMMY_BLOG_ID; - - $client_id = '91299'; - $redirect_uri = 'https://woo.com'; - $nonce = 'nonce-123'; - - $merchant_redirect_url = 'https://admin-example.com/wp-admin/admin.php?page=wc-admin&path=/google/setup-mc'; - $merchant_redirect_url_encoded = urlencode_deep( $merchant_redirect_url ); - $state_raw = "nonce={$nonce}&redirect_url={$merchant_redirect_url_encoded}"; - $state = base64_encode( $state_raw ); - $expected_auth_url = 'https://public-api.wordpress.com/oauth2/authorize'; - $expected_auth_url .= "?blog={$blog_id}"; - $expected_auth_url .= "&client_id={$client_id}"; - $expected_auth_url .= "&redirect_uri={$redirect_uri}"; + $expected_auth_url .= '?blog=12345'; + $expected_auth_url .= '&client_id=23456'; + $expected_auth_url .= '&redirect_uri=https://example.com'; $expected_auth_url .= '&response_type=code'; $expected_auth_url .= '&scope=wc-partner-access'; - $expected_auth_url .= "&state={$state}"; - $expected_auth_url = esc_url_raw( $expected_auth_url ); + $expected_auth_url .= '&state=base64_encoded_string'; + $expected_auth_url = $expected_auth_url; + + $this->oauth_service->expects( $this->once() ) + ->method( 'get_auth_url' ) + ->willReturn( $expected_auth_url ); $response = $this->do_request( self::ROUTE_AUTHORIZE, 'GET' ); $data = $response->get_data(); - $auth_url = $data['auth_url']; - // Compare the auth URLs. - // Removing the "=" sign from the end of the string because sometimes - // base64_encode function will add "=" signs as paddings. $this->assertEquals( - rtrim( $expected_auth_url, '=' ), - rtrim( $auth_url, '=' ) + [ + 'auth_url' => $expected_auth_url, + ], + $response->get_data() ); $this->assertEquals( 200, $response->get_status() ); - - // Compare the state query parameters from the auth URL. - $parsed_url = wp_parse_url( $auth_url ); - parse_str( $parsed_url['query'], $parsed_query ); - $response_state_raw = base64_decode( $parsed_query['state'] ); - $this->assertEquals( - $state_raw, - $response_state_raw - ); - - // Ensure the base64 encoded state query has correct value. - parse_str( $response_state_raw, $parsed_state ); - $this->assertEquals( - $nonce, - $parsed_state['nonce'] - ); - $this->assertEquals( - $merchant_redirect_url, - $parsed_state['redirect_url'] - ); } public function test_authorize_invalid_parameter() { @@ -113,4 +65,15 @@ public function test_authorize_invalid_parameter() { $this->assertEquals( 'rest_invalid_param', $response->get_data()['code'] ); $this->assertEquals( 400, $response->get_status() ); } + + public function test_authorize_with_error() { + $this->oauth_service->expects( $this->once() ) + ->method( 'get_auth_url' ) + ->willThrowException( new Exception( 'error', 400 ) ); + + $response = $this->do_request( self::ROUTE_AUTHORIZE, 'GET' ); + + $this->assertEquals( [ 'message' => 'error' ], $response->get_data() ); + $this->assertEquals( 400, $response->get_status() ); + } } From ff1a51684085768536ae88ca90602fe2f6fe40ac Mon Sep 17 00:00:00 2001 From: Ian Yu-Hsun Lin Date: Wed, 6 Mar 2024 18:51:46 +0800 Subject: [PATCH 057/262] Add tests for WP/OAuthService --- src/API/WP/OAuthService.php | 2 +- tests/Unit/API/WP/OAuthServiceTest.php | 126 +++++++++++++++++++++++++ 2 files changed, 127 insertions(+), 1 deletion(-) create mode 100644 tests/Unit/API/WP/OAuthServiceTest.php diff --git a/src/API/WP/OAuthService.php b/src/API/WP/OAuthService.php index be598cf0ae..c2c5340551 100644 --- a/src/API/WP/OAuthService.php +++ b/src/API/WP/OAuthService.php @@ -86,7 +86,7 @@ public function get_auth_url( string $path ): string { * * @return array An associative array contains required information from Google. */ - private function get_data_from_google(): array { + protected function get_data_from_google(): array { return [ 'client_id' => '91299', 'redirect_uri' => 'https://woo.com', diff --git a/tests/Unit/API/WP/OAuthServiceTest.php b/tests/Unit/API/WP/OAuthServiceTest.php new file mode 100644 index 0000000000..96f2274ee7 --- /dev/null +++ b/tests/Unit/API/WP/OAuthServiceTest.php @@ -0,0 +1,126 @@ +service = $this->getMockBuilder( OAuthService::class ) + ->onlyMethods( [ 'get_data_from_google' ] ) + ->getMock(); + + // Mock the Blog ID from Jetpack. + add_filter( + 'jetpack_options', + function ( $value, $name ) { + if ( $name === 'id' ) { + return self::DUMMY_BLOG_ID; + } + + return $value; + }, + 10, + 2 + ); + + // Mock admin URL. + add_filter( + 'admin_url', + function ( $url, $path ) { + return self::DUMMY_ADMIN_URL . $path; + }, + 10, + 2 + ); + } + + /** + * Test get_auth_url() function. + */ + public function test_get_auth_url() { + $client_id = '12345'; + $redirect_uri = 'https://example.com'; + $nonce = 'nonce-999'; + $blog_id = self::DUMMY_BLOG_ID; + $admin_url = self::DUMMY_ADMIN_URL; + $path = '/google/setup-mc'; + + $this->service->expects( $this->once() ) + ->method( 'get_data_from_google' ) + ->willReturn( + [ + 'client_id' => $client_id, + 'redirect_uri' => $redirect_uri, + 'nonce' => $nonce, + ] + ); + + $merchant_redirect_url = "{$admin_url}admin.php?page=wc-admin&path={$path}"; + $merchant_redirect_url_encoded = urlencode_deep( $merchant_redirect_url ); + $expected_state_raw = "nonce={$nonce}&redirect_url={$merchant_redirect_url_encoded}"; + $state = base64_encode( $expected_state_raw ); + + $expected_auth_url = 'https://public-api.wordpress.com/oauth2/authorize'; + $expected_auth_url .= "?blog={$blog_id}"; + $expected_auth_url .= "&client_id={$client_id}"; + $expected_auth_url .= "&redirect_uri={$redirect_uri}"; + $expected_auth_url .= '&response_type=code'; + $expected_auth_url .= '&scope=wc-partner-access'; + $expected_auth_url .= "&state={$state}"; + $expected_auth_url = esc_url_raw( $expected_auth_url ); + + $auth_url = $this->service->get_auth_url( $path ); + + // Compare the auth URLs. + // Removing the "=" sign from the end of the string because sometimes + // base64_encode function will add "=" signs as paddings. + $this->assertEquals( + rtrim( $expected_auth_url, '=' ), + rtrim( $auth_url, '=' ) + ); + + // Compare the state query parameters from the auth URL. + $parsed_url = wp_parse_url( $auth_url ); + parse_str( $parsed_url['query'], $parsed_query ); + $state_raw = base64_decode( $parsed_query['state'] ); + $this->assertEquals( + $expected_state_raw, + $state_raw + ); + + // Ensure the base64 encoded state query has correct value. + parse_str( $state_raw, $parsed_state ); + $this->assertEquals( + $nonce, + $parsed_state['nonce'] + ); + $this->assertEquals( + $merchant_redirect_url, + $parsed_state['redirect_url'] + ); + } +} \ No newline at end of file From 7950c9500c7f8820a721f4d08a2e038cfb010af5 Mon Sep 17 00:00:00 2001 From: Ian Yu-Hsun Lin Date: Wed, 6 Mar 2024 19:05:06 +0800 Subject: [PATCH 058/262] Move NotificationsService to the folder API/WP --- src/{Google => API/WP}/NotificationsService.php | 4 ++-- src/ConnectionTest.php | 2 +- src/Coupon/SyncerHooks.php | 2 +- src/Internal/DependencyManagement/CoreServiceProvider.php | 2 +- src/Internal/DependencyManagement/JobServiceProvider.php | 2 +- src/Jobs/Notifications/AbstractNotificationJob.php | 2 +- src/Jobs/Notifications/CouponNotificationJob.php | 2 +- src/Jobs/Notifications/ProductNotificationJob.php | 2 +- src/Product/SyncerHooks.php | 2 +- src/Settings/SyncerHooks.php | 2 +- src/Shipping/SyncerHooks.php | 2 +- tests/Unit/{Google => API/WP}/NotificationsServiceTest.php | 6 +++--- tests/Unit/Coupon/SyncerHooksTest.php | 2 +- tests/Unit/Jobs/AbstractItemNotificationJobTest.php | 2 +- tests/Unit/Jobs/AbstractNotificationJobTest.php | 2 +- tests/Unit/Product/SyncerHooksTest.php | 2 +- tests/Unit/Settings/SyncerHooksTest.php | 2 +- tests/Unit/Shipping/SyncerHooksTest.php | 2 +- 18 files changed, 21 insertions(+), 21 deletions(-) rename src/{Google => API/WP}/NotificationsService.php (97%) rename tests/Unit/{Google => API/WP}/NotificationsServiceTest.php (98%) diff --git a/src/Google/NotificationsService.php b/src/API/WP/NotificationsService.php similarity index 97% rename from src/Google/NotificationsService.php rename to src/API/WP/NotificationsService.php index cde07ab960..d176fa986d 100644 --- a/src/Google/NotificationsService.php +++ b/src/API/WP/NotificationsService.php @@ -1,7 +1,7 @@ Date: Wed, 6 Mar 2024 19:12:28 +0800 Subject: [PATCH 059/262] Fix PHPCS --- tests/Unit/API/WP/OAuthServiceTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Unit/API/WP/OAuthServiceTest.php b/tests/Unit/API/WP/OAuthServiceTest.php index 96f2274ee7..afa8f024b0 100644 --- a/tests/Unit/API/WP/OAuthServiceTest.php +++ b/tests/Unit/API/WP/OAuthServiceTest.php @@ -123,4 +123,4 @@ public function test_get_auth_url() { $parsed_state['redirect_url'] ); } -} \ No newline at end of file +} From 3c673ca420e918b7e0384a689da8afcd55139963 Mon Sep 17 00:00:00 2001 From: Ian Yu-Hsun Lin Date: Mon, 11 Mar 2024 17:45:47 +0800 Subject: [PATCH 060/262] Add helper functions base64url_encode/base64url_decode --- src/API/WP/OAuthService.php | 5 ++- src/HelperTraits/Utilities.php | 42 ++++++++++++++++++++++++++ tests/Unit/API/WP/OAuthServiceTest.php | 13 ++++---- 3 files changed, 53 insertions(+), 7 deletions(-) diff --git a/src/API/WP/OAuthService.php b/src/API/WP/OAuthService.php index c2c5340551..f981a01499 100644 --- a/src/API/WP/OAuthService.php +++ b/src/API/WP/OAuthService.php @@ -4,6 +4,7 @@ namespace Automattic\WooCommerce\GoogleListingsAndAds\API\WP; use Automattic\Jetpack\Connection\Client; +use Automattic\WooCommerce\GoogleListingsAndAds\HelperTraits\Utilities as UtilitiesTrait; use Automattic\WooCommerce\GoogleListingsAndAds\Infrastructure\Service; use Jetpack_Options; @@ -18,6 +19,8 @@ */ class OAuthService implements Service { + use UtilitiesTrait; + public const AUTH_URL = 'https://public-api.wordpress.com/oauth2/authorize'; public const RESPONSE_TYPE = 'code'; public const SCOPE = 'wc-partner-access'; @@ -52,7 +55,7 @@ public function get_auth_url( string $path ): string { $merchant_redirect_url = urlencode_deep( admin_url( "admin.php?page=wc-admin&path={$path}" ) ); - $state = base64_encode( + $state = $this->base64url_encode( build_query( [ 'nonce' => $google_data['nonce'], diff --git a/src/HelperTraits/Utilities.php b/src/HelperTraits/Utilities.php index ca63e28a82..1c5af03b00 100644 --- a/src/HelperTraits/Utilities.php +++ b/src/HelperTraits/Utilities.php @@ -78,4 +78,46 @@ protected function gla_setup_for( $seconds ): bool { protected function is_jetpack_connected(): bool { return boolval( $this->options->get( OptionsInterface::JETPACK_CONNECTED, false ) ); } + + /** + * Encode data to Base64URL + * + * @since x.x.x + * + * @param string $data The string that will be base64 URL encoded. + * + * @return boolean|string + */ + protected function base64url_encode( $data ): bool|string { + // First of all you should encode $data to Base64 string + $b64 = base64_encode( $data ); + + // Make sure you get a valid result, otherwise, return FALSE, as the base64_encode() function do + if ( $b64 === false ) { + return false; + } + + // Convert Base64 to Base64URL by replacing "+" with "-" and "/" with "_" + $url = strtr( $b64, '+/', '-_' ); + + // Remove padding character from the end of line and return the Base64URL result + return rtrim( $url, '=' ); + } + + /** + * Decode Base64URL string + * + * @since x.x.x + * + * @param string $data The data that will be base64 URL encoded. + * + * @return boolean|string + */ + protected function base64url_decode( $data ): bool|string { + // Convert Base64URL to Base64 by replacing "-" with "+" and "_" with "/" + $b64 = strtr( $data, '-_', '+/' ); + + // Decode Base64 string and return the original data + return base64_decode( $b64 ); + } } diff --git a/tests/Unit/API/WP/OAuthServiceTest.php b/tests/Unit/API/WP/OAuthServiceTest.php index afa8f024b0..b5d4a8c0cf 100644 --- a/tests/Unit/API/WP/OAuthServiceTest.php +++ b/tests/Unit/API/WP/OAuthServiceTest.php @@ -4,6 +4,7 @@ namespace Automattic\WooCommerce\GoogleListingsAndAds\Tests\Unit\API\WP; use Automattic\WooCommerce\GoogleListingsAndAds\API\WP\OAuthService; +use Automattic\WooCommerce\GoogleListingsAndAds\HelperTraits\Utilities as UtilitiesTrait; use Automattic\WooCommerce\GoogleListingsAndAds\Tests\Framework\UnitTest; defined( 'ABSPATH' ) || exit; @@ -15,6 +16,8 @@ */ class OAuthServiceTest extends UnitTest { + use UtilitiesTrait; + /** * @var OAuthService */ @@ -82,7 +85,7 @@ public function test_get_auth_url() { $merchant_redirect_url = "{$admin_url}admin.php?page=wc-admin&path={$path}"; $merchant_redirect_url_encoded = urlencode_deep( $merchant_redirect_url ); $expected_state_raw = "nonce={$nonce}&redirect_url={$merchant_redirect_url_encoded}"; - $state = base64_encode( $expected_state_raw ); + $state = $this->base64url_encode( $expected_state_raw ); $expected_auth_url = 'https://public-api.wordpress.com/oauth2/authorize'; $expected_auth_url .= "?blog={$blog_id}"; @@ -96,17 +99,15 @@ public function test_get_auth_url() { $auth_url = $this->service->get_auth_url( $path ); // Compare the auth URLs. - // Removing the "=" sign from the end of the string because sometimes - // base64_encode function will add "=" signs as paddings. $this->assertEquals( - rtrim( $expected_auth_url, '=' ), - rtrim( $auth_url, '=' ) + $expected_auth_url, + $auth_url ); // Compare the state query parameters from the auth URL. $parsed_url = wp_parse_url( $auth_url ); parse_str( $parsed_url['query'], $parsed_query ); - $state_raw = base64_decode( $parsed_query['state'] ); + $state_raw = $this->base64url_decode( $parsed_query['state'] ); $this->assertEquals( $expected_state_raw, $state_raw From 49df5c9dc138dac24c2c33e43298a904e85e7c00 Mon Sep 17 00:00:00 2001 From: Ian Yu-Hsun Lin Date: Mon, 11 Mar 2024 18:09:03 +0800 Subject: [PATCH 061/262] Remove functions return union types As PHP 7.4 does not support it and we still support 7.4 --- src/HelperTraits/Utilities.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/HelperTraits/Utilities.php b/src/HelperTraits/Utilities.php index 1c5af03b00..c68c2801d6 100644 --- a/src/HelperTraits/Utilities.php +++ b/src/HelperTraits/Utilities.php @@ -88,7 +88,7 @@ protected function is_jetpack_connected(): bool { * * @return boolean|string */ - protected function base64url_encode( $data ): bool|string { + protected function base64url_encode( $data ): string { // First of all you should encode $data to Base64 string $b64 = base64_encode( $data ); @@ -113,7 +113,7 @@ protected function base64url_encode( $data ): bool|string { * * @return boolean|string */ - protected function base64url_decode( $data ): bool|string { + protected function base64url_decode( $data ): string { // Convert Base64URL to Base64 by replacing "-" with "+" and "_" with "/" $b64 = strtr( $data, '-_', '+/' ); From 5163fe35a9ef0d4639855edadd8cc661458aee6b Mon Sep 17 00:00:00 2001 From: Ian Yu-Hsun Lin Date: Wed, 13 Mar 2024 16:01:24 +0800 Subject: [PATCH 062/262] Return WPCOM REST API status from GET /mc/connection API --- src/MerchantCenter/AccountService.php | 9 ++++++--- src/Options/OptionsInterface.php | 2 ++ .../Unit/MerchantCenter/AccountServiceTest.php | 17 ++++++++++++----- 3 files changed, 20 insertions(+), 8 deletions(-) diff --git a/src/MerchantCenter/AccountService.php b/src/MerchantCenter/AccountService.php index f3af91c15b..cdf4fcf1e3 100644 --- a/src/MerchantCenter/AccountService.php +++ b/src/MerchantCenter/AccountService.php @@ -216,10 +216,13 @@ public function overwrite_claim( int $account_id ): array { * @return array */ public function get_connected_status(): array { - $id = $this->options->get_merchant_id(); + $id = $this->options->get_merchant_id(); + $wpcom_rest_api_status = $this->options->get( OptionsInterface::WPCOM_REST_API_STATUS ); + $status = [ - 'id' => $id, - 'status' => $id ? 'connected' : 'disconnected', + 'id' => $id, + 'status' => $id ? 'connected' : 'disconnected', + 'wpcom_rest_api_status' => $wpcom_rest_api_status === 'approved' ? 'approved' : 'disapproved', ]; $incomplete = $this->state->last_incomplete_step(); diff --git a/src/Options/OptionsInterface.php b/src/Options/OptionsInterface.php index 3dafa5a93b..63ec62e9b2 100644 --- a/src/Options/OptionsInterface.php +++ b/src/Options/OptionsInterface.php @@ -41,6 +41,7 @@ interface OptionsInterface { public const TOURS = 'tours'; public const UPDATE_ALL_PRODUCTS_LAST_SYNC = 'update_all_products_last_sync'; public const WP_TOS_ACCEPTED = 'wp_tos_accepted'; + public const WPCOM_REST_API_STATUS = 'wpcom_rest_api_status'; public const VALID_OPTIONS = [ self::ADS_ACCOUNT_CURRENCY => true, @@ -72,6 +73,7 @@ interface OptionsInterface { self::TOURS => true, self::UPDATE_ALL_PRODUCTS_LAST_SYNC => true, self::WP_TOS_ACCEPTED => true, + self::WPCOM_REST_API_STATUS => true, ]; public const OPTION_TYPES = [ diff --git a/tests/Unit/MerchantCenter/AccountServiceTest.php b/tests/Unit/MerchantCenter/AccountServiceTest.php index d759110129..bb77574fa6 100644 --- a/tests/Unit/MerchantCenter/AccountServiceTest.php +++ b/tests/Unit/MerchantCenter/AccountServiceTest.php @@ -657,10 +657,16 @@ public function test_get_connected_status() { ->method( 'get_merchant_id' ) ->willReturn( self::TEST_ACCOUNT_ID ); + $this->options->expects( $this->once() ) + ->method( 'get' ) + ->with( OptionsInterface::WPCOM_REST_API_STATUS ) + ->willReturn( 'approved' ); + $this->assertEquals( [ - 'id' => self::TEST_ACCOUNT_ID, - 'status' => 'connected', + 'id' => self::TEST_ACCOUNT_ID, + 'status' => 'connected', + 'wpcom_rest_api_status' => 'approved', ], $this->account->get_connected_status() ); @@ -677,9 +683,10 @@ public function test_get_connected_status_incomplete() { $this->assertEquals( [ - 'id' => self::TEST_ACCOUNT_ID, - 'status' => 'incomplete', - 'step' => 'verify', + 'id' => self::TEST_ACCOUNT_ID, + 'status' => 'incomplete', + 'step' => 'verify', + 'wpcom_rest_api_status' => 'disapproved', ], $this->account->get_connected_status() ); From 22e546b2e7544d318f7a0397737c8b23c6a30be7 Mon Sep 17 00:00:00 2001 From: Ian Yu-Hsun Lin Date: Thu, 14 Mar 2024 17:05:43 +0800 Subject: [PATCH 063/262] Add a notice in the settings page for enabling new product sync feature --- .../enable-new-product-sync-notice.js | 78 +++++++++++++++++++ js/src/settings/index.js | 2 + 2 files changed, 80 insertions(+) create mode 100644 js/src/components/enable-new-product-sync-notice.js diff --git a/js/src/components/enable-new-product-sync-notice.js b/js/src/components/enable-new-product-sync-notice.js new file mode 100644 index 0000000000..173d172ea4 --- /dev/null +++ b/js/src/components/enable-new-product-sync-notice.js @@ -0,0 +1,78 @@ +/** + * External dependencies + */ +import { __ } from '@wordpress/i18n'; +import { Notice } from '@wordpress/components'; +import { createInterpolateElement } from '@wordpress/element'; +import { addQueryArgs } from '@wordpress/url'; + +/** + * Internal dependencies + */ +import AppButton from '.~/components/app-button'; +import { glaData } from '.~/constants'; +import { API_NAMESPACE } from '.~/data/constants'; +import useApiFetchCallback from '.~/hooks/useApiFetchCallback'; +import useDispatchCoreNotices from '.~/hooks/useDispatchCoreNotices'; +import useGoogleMCAccount from '.~/hooks/useGoogleMCAccount'; + +/** + * Shows info {@link Notice} + * about enabling new product sync when the feature flag is turned on and hasn't switched to new product sync. + * + * @return {JSX.Element} {@link Notice} element with the info message and the button to enable new product sync. + */ +const EnableNewProductSyncNotice = () => { + const { + hasFinishedResolution: hasGoogleMCAccountFinishedResolution, + googleMCAccount, + } = useGoogleMCAccount(); + + const { createNotice } = useDispatchCoreNotices(); + + const nextPageName = glaData.mcSetupComplete ? 'reconnect' : 'setup-mc'; + const query = { next_page_name: nextPageName }; + const path = addQueryArgs( `${ API_NAMESPACE }/rest-api/authorize`, query ); + const [ fetchRestAPIAuthorize ] = useApiFetchCallback( { path } ); + const handleEnableClick = async () => { + try { + const d = await fetchRestAPIAuthorize(); + window.location.href = d.auth_url; + } catch ( error ) { + createNotice( + 'error', + __( + 'Unable to enable new product sync. Please try again later.', + 'google-listings-and-ads' + ) + ); + } + }; + + // Do not render if already switch to new product sync. + // TODO: also check if the new product sync feature flag is enabled. + if ( + ! hasGoogleMCAccountFinishedResolution || + googleMCAccount.wpcom_rest_api_status === 'approved' + ) { + return null; + } + + return ( + + { createInterpolateElement( + __( + 'Enable new product sync. Enable', + 'google-listings-and-ads' + ), + { + enableButton: ( + + ), + } + ) } + + ); +}; + +export default EnableNewProductSyncNotice; diff --git a/js/src/settings/index.js b/js/src/settings/index.js index 4189ca0059..f63414a435 100644 --- a/js/src/settings/index.js +++ b/js/src/settings/index.js @@ -17,6 +17,7 @@ import ReconnectWPComAccount from './reconnect-wpcom-account'; import ReconnectGoogleAccount from './reconnect-google-account'; import EditStoreAddress from './edit-store-address'; import EditPhoneNumber from './edit-phone-number'; +import EnableNewProductSyncNotice from '.~/components/enable-new-product-sync-notice'; import NavigationClassic from '.~/components/navigation-classic'; import './index.scss'; @@ -59,6 +60,7 @@ const Settings = () => { return ( + From 94886fc6e5693e66333819a282e7da1b3a1020a9 Mon Sep 17 00:00:00 2001 From: Ian Yu-Hsun Lin Date: Fri, 15 Mar 2024 12:51:16 +0800 Subject: [PATCH 064/262] Link response schema to /rest-api/authorize endpoint --- src/API/Site/Controllers/RestAPI/AuthController.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/API/Site/Controllers/RestAPI/AuthController.php b/src/API/Site/Controllers/RestAPI/AuthController.php index 7cdc743e05..962c908a2e 100644 --- a/src/API/Site/Controllers/RestAPI/AuthController.php +++ b/src/API/Site/Controllers/RestAPI/AuthController.php @@ -60,6 +60,7 @@ public function register_routes() { 'permission_callback' => $this->get_permission_callback(), 'args' => $this->get_auth_params(), ], + 'schema' => $this->get_api_response_schema_callback(), ] ); } From efb5e5f423aa0cb92c2573d5e3787f206344085c Mon Sep 17 00:00:00 2001 From: Ian Yu-Hsun Lin Date: Fri, 15 Mar 2024 12:52:31 +0800 Subject: [PATCH 065/262] Remove unused dependency in WP/OAuthService --- src/API/WP/OAuthService.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/API/WP/OAuthService.php b/src/API/WP/OAuthService.php index f981a01499..af623b460e 100644 --- a/src/API/WP/OAuthService.php +++ b/src/API/WP/OAuthService.php @@ -3,7 +3,6 @@ namespace Automattic\WooCommerce\GoogleListingsAndAds\API\WP; -use Automattic\Jetpack\Connection\Client; use Automattic\WooCommerce\GoogleListingsAndAds\HelperTraits\Utilities as UtilitiesTrait; use Automattic\WooCommerce\GoogleListingsAndAds\Infrastructure\Service; use Jetpack_Options; From 70442c023863c5684f3cbbffb60eae83bb7592db Mon Sep 17 00:00:00 2001 From: Ian Yu-Hsun Lin Date: Fri, 15 Mar 2024 13:10:29 +0800 Subject: [PATCH 066/262] Update the PHPDoc to tell the state will be url safe base64 encoded --- src/API/WP/OAuthService.php | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/API/WP/OAuthService.php b/src/API/WP/OAuthService.php index af623b460e..1a59c2bf15 100644 --- a/src/API/WP/OAuthService.php +++ b/src/API/WP/OAuthService.php @@ -36,9 +36,13 @@ class OAuthService implements Service { * response_type=code& * blog=BLOD_ID& * scope=wc-partner-access& - * state=BASE64_ENCODED_STRING + * state=URL_SAFE_BASE64_ENCODED_STRING * - * Content of state is an URL query string where the value of its parameter "redirect_url" is being URL encoded. + * State is an URL safe base64 encoded string. + * E.g. + * state=bm9uY2UtMTIzJnJlZGlyZWN0X3VybD1odHRwcyUzQSUyRiUyRm1lcmNoYW50LXNpdGUuZXhhbXBsZS5jb20lMkZ3cC1hZG1pbiUyRmFkbWluLnBocCUzRnBhZ2UlM0R3Yy1hZG1pbiUyNnBhdGglM0QlMkZnb29nbGUlMkZzZXR1cC1tYw + * + * The decoded content of state is an URL query string where the value of its parameter "redirect_url" is being URL encoded. * E.g. * nonce=nonce-123&redirect_url=https%3A%2F%2Fmerchant-site.example.com%2Fwp-admin%2Fadmin.php%3Fpage%3Dwc-admin%26path%3D%2Fgoogle%2Fsetup-mc * From 9c70657f59a3dddef1adcefc90129f551955d7ec Mon Sep 17 00:00:00 2001 From: Ian Yu-Hsun Lin Date: Fri, 15 Mar 2024 13:30:43 +0800 Subject: [PATCH 067/262] Update PHPDoc to explain the response data from Google --- src/API/WP/OAuthService.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/API/WP/OAuthService.php b/src/API/WP/OAuthService.php index 1a59c2bf15..e52e335b44 100644 --- a/src/API/WP/OAuthService.php +++ b/src/API/WP/OAuthService.php @@ -90,7 +90,10 @@ public function get_auth_url( string $path ): string { * TODO: Call an actual API by Google. * We'd probably need use WCS to communicate with the new API. * - * @return array An associative array contains required information from Google. + * @return array{client_id: string, redirect_uri: string, nonce: string} An associative array contains required information that is retrived from Google. + * client_id: Google's WPCOM app client ID, will be used to form the authorization URL. + * redirect_uri: A Google's URL that will be redirected to when the merchant approve the app access. Note that it needs to be matched with the Google WPCOM app client settings. + * nonce: A string returned by Google that we will put it in the auth URL and the redirect_uri. Google will use it to verify the call. */ protected function get_data_from_google(): array { return [ From 614df20b5f2741f1830bcb84efe3c699a9b52ace Mon Sep 17 00:00:00 2001 From: Ian Yu-Hsun Lin Date: Fri, 15 Mar 2024 14:35:02 +0800 Subject: [PATCH 068/262] Do not return false because base64_encode always returns string --- src/HelperTraits/Utilities.php | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/HelperTraits/Utilities.php b/src/HelperTraits/Utilities.php index c68c2801d6..cfdbf0454c 100644 --- a/src/HelperTraits/Utilities.php +++ b/src/HelperTraits/Utilities.php @@ -86,17 +86,12 @@ protected function is_jetpack_connected(): bool { * * @param string $data The string that will be base64 URL encoded. * - * @return boolean|string + * @return string */ protected function base64url_encode( $data ): string { // First of all you should encode $data to Base64 string $b64 = base64_encode( $data ); - // Make sure you get a valid result, otherwise, return FALSE, as the base64_encode() function do - if ( $b64 === false ) { - return false; - } - // Convert Base64 to Base64URL by replacing "+" with "-" and "/" with "_" $url = strtr( $b64, '+/', '-_' ); From 2b6f25108bb41c9056f5f7e887a4517b7ad5ae52 Mon Sep 17 00:00:00 2001 From: Ian Yu-Hsun Lin Date: Fri, 15 Mar 2024 14:36:43 +0800 Subject: [PATCH 069/262] Remove unused variable --- tests/Unit/API/Site/Controllers/RestAPI/AuthControllerTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/Unit/API/Site/Controllers/RestAPI/AuthControllerTest.php b/tests/Unit/API/Site/Controllers/RestAPI/AuthControllerTest.php index 8a1a95d43f..35cfa6af98 100644 --- a/tests/Unit/API/Site/Controllers/RestAPI/AuthControllerTest.php +++ b/tests/Unit/API/Site/Controllers/RestAPI/AuthControllerTest.php @@ -47,7 +47,6 @@ public function test_authorize() { ->willReturn( $expected_auth_url ); $response = $this->do_request( self::ROUTE_AUTHORIZE, 'GET' ); - $data = $response->get_data(); $this->assertEquals( [ From ba600aed027de83f7a88936131cb246b44b03913 Mon Sep 17 00:00:00 2001 From: Ian Yu-Hsun Lin Date: Mon, 18 Mar 2024 15:07:33 +0800 Subject: [PATCH 070/262] Remove an unnecessary comment since the code is self-explanatory --- src/HelperTraits/Utilities.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/HelperTraits/Utilities.php b/src/HelperTraits/Utilities.php index cfdbf0454c..40f90b8867 100644 --- a/src/HelperTraits/Utilities.php +++ b/src/HelperTraits/Utilities.php @@ -89,7 +89,6 @@ protected function is_jetpack_connected(): bool { * @return string */ protected function base64url_encode( $data ): string { - // First of all you should encode $data to Base64 string $b64 = base64_encode( $data ); // Convert Base64 to Base64URL by replacing "+" with "-" and "/" with "_" From b41bad3a26a87bbf011e95773305a08ab63eeef7 Mon Sep 17 00:00:00 2001 From: Ian Yu-Hsun Lin Date: Mon, 18 Mar 2024 15:57:22 +0800 Subject: [PATCH 071/262] Rename reconnect to settings for page mapping --- src/API/Site/Controllers/RestAPI/AuthController.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/API/Site/Controllers/RestAPI/AuthController.php b/src/API/Site/Controllers/RestAPI/AuthController.php index 962c908a2e..bc1d590a39 100644 --- a/src/API/Site/Controllers/RestAPI/AuthController.php +++ b/src/API/Site/Controllers/RestAPI/AuthController.php @@ -32,8 +32,8 @@ class AuthController extends BaseController { * @var string[] */ private const NEXT_PATH_MAPPING = [ - 'setup-mc' => '/google/setup-mc', - 'reconnect' => '/google/settings', + 'setup-mc' => '/google/setup-mc', + 'settings' => '/google/settings', ]; /** From a510cab0deebf29b57f66a619736b0cbe0974ad0 Mon Sep 17 00:00:00 2001 From: Miguel Perez Pellicer <5908855+puntope@users.noreply.github.com> Date: Tue, 19 Mar 2024 23:42:37 +0100 Subject: [PATCH 072/262] add api pull grant access statuses in GMC cards --- js/src/components/app-notice/index.js | 33 +++++ js/src/components/app-notice/index.scss | 6 + .../enable-new-product-sync-button.js | 54 +++++++++ .../enable-new-product-sync-notice.js | 35 +----- .../connected-google-mc-account-card.js | 113 ++++++++++++++++-- .../google-mc-account-card.js | 7 +- package-lock.json | 2 +- .../Controllers/RestAPI/AuthController.php | 37 +++++- .../RESTServiceProvider.php | 2 +- src/MerchantCenter/AccountService.php | 11 +- 10 files changed, 252 insertions(+), 48 deletions(-) create mode 100644 js/src/components/app-notice/index.js create mode 100644 js/src/components/app-notice/index.scss create mode 100644 js/src/components/enable-new-product-sync-button.js diff --git a/js/src/components/app-notice/index.js b/js/src/components/app-notice/index.js new file mode 100644 index 0000000000..841b089562 --- /dev/null +++ b/js/src/components/app-notice/index.js @@ -0,0 +1,33 @@ +/** + * External dependencies + */ +import classnames from 'classnames'; +import { Notice } from '@wordpress/components'; + +/** + * Internal dependencies + */ +import './index.scss'; + +/** + * Renders a Notice component with extra props. + * + * It supports all the props from @wordpress/components - Notice Component + * + * ## Usage + * + * ```jsx + * + * My Notice + * + * ``` + * + * @param {*} props Props to be forwarded to {@link Notice}. + */ +const AppNotice = ( props ) => { + const { className, ...rest } = props; + const classes = [ 'app-notice', className ]; + return ; +}; + +export default AppNotice; diff --git a/js/src/components/app-notice/index.scss b/js/src/components/app-notice/index.scss new file mode 100644 index 0000000000..8c38d351e0 --- /dev/null +++ b/js/src/components/app-notice/index.scss @@ -0,0 +1,6 @@ +.app-notice { + border: 0; + font-size: $helptext-font-size; + margin: 0 var(--large-gap) var(--main-gap); + padding: $grid-unit-20; +} diff --git a/js/src/components/enable-new-product-sync-button.js b/js/src/components/enable-new-product-sync-button.js new file mode 100644 index 0000000000..b3917f4b9c --- /dev/null +++ b/js/src/components/enable-new-product-sync-button.js @@ -0,0 +1,54 @@ +/** + * External dependencies + */ +import { __ } from '@wordpress/i18n'; +import { addQueryArgs } from '@wordpress/url'; + +/** + * Internal dependencies + */ +import AppButton from '.~/components/app-button'; +import { glaData } from '.~/constants'; +import { API_NAMESPACE } from '.~/data/constants'; +import useApiFetchCallback from '.~/hooks/useApiFetchCallback'; +import useDispatchCoreNotices from '.~/hooks/useDispatchCoreNotices'; + +/** + * Button to initiate auth process for WC Rest API + * + * @param {Object} params The component params + * @param {string} params.buttonText The text to show in the enable button + * @return {JSX.Element} The button. + */ +const EnableNewProductSyncButton = ( { + buttonText = __( 'Enable it', 'google-listings-and-ads' ), +} ) => { + const { createNotice } = useDispatchCoreNotices(); + + const nextPageName = glaData.mcSetupComplete ? 'reconnect' : 'setup-mc'; + const query = { next_page_name: nextPageName }; + const path = addQueryArgs( `${ API_NAMESPACE }/rest-api/authorize`, query ); + const [ fetchRestAPIAuthorize ] = useApiFetchCallback( { path } ); + const handleEnableClick = async () => { + try { + const d = await fetchRestAPIAuthorize(); + window.location.href = d.auth_url; + } catch ( error ) { + createNotice( + 'error', + __( + 'Unable to enable new product sync. Please try again later.', + 'google-listings-and-ads' + ) + ); + } + }; + + return ( + + { buttonText } + + ); +}; + +export default EnableNewProductSyncButton; diff --git a/js/src/components/enable-new-product-sync-notice.js b/js/src/components/enable-new-product-sync-notice.js index 173d172ea4..3c2ca27ef3 100644 --- a/js/src/components/enable-new-product-sync-notice.js +++ b/js/src/components/enable-new-product-sync-notice.js @@ -4,17 +4,12 @@ import { __ } from '@wordpress/i18n'; import { Notice } from '@wordpress/components'; import { createInterpolateElement } from '@wordpress/element'; -import { addQueryArgs } from '@wordpress/url'; /** * Internal dependencies */ -import AppButton from '.~/components/app-button'; -import { glaData } from '.~/constants'; -import { API_NAMESPACE } from '.~/data/constants'; -import useApiFetchCallback from '.~/hooks/useApiFetchCallback'; -import useDispatchCoreNotices from '.~/hooks/useDispatchCoreNotices'; import useGoogleMCAccount from '.~/hooks/useGoogleMCAccount'; +import EnableNewProductSyncButton from '.~/components/enable-new-product-sync-button'; /** * Shows info {@link Notice} @@ -28,32 +23,10 @@ const EnableNewProductSyncNotice = () => { googleMCAccount, } = useGoogleMCAccount(); - const { createNotice } = useDispatchCoreNotices(); - - const nextPageName = glaData.mcSetupComplete ? 'reconnect' : 'setup-mc'; - const query = { next_page_name: nextPageName }; - const path = addQueryArgs( `${ API_NAMESPACE }/rest-api/authorize`, query ); - const [ fetchRestAPIAuthorize ] = useApiFetchCallback( { path } ); - const handleEnableClick = async () => { - try { - const d = await fetchRestAPIAuthorize(); - window.location.href = d.auth_url; - } catch ( error ) { - createNotice( - 'error', - __( - 'Unable to enable new product sync. Please try again later.', - 'google-listings-and-ads' - ) - ); - } - }; - // Do not render if already switch to new product sync. - // TODO: also check if the new product sync feature flag is enabled. if ( ! hasGoogleMCAccountFinishedResolution || - googleMCAccount.wpcom_rest_api_status === 'approved' + googleMCAccount.wpcom_rest_api_status ) { return null; } @@ -66,9 +39,7 @@ const EnableNewProductSyncNotice = () => { 'google-listings-and-ads' ), { - enableButton: ( - - ), + enableButton: , } ) } diff --git a/js/src/components/google-mc-account-card/connected-google-mc-account-card.js b/js/src/components/google-mc-account-card/connected-google-mc-account-card.js index 6976d0f846..1d1001ce4f 100644 --- a/js/src/components/google-mc-account-card/connected-google-mc-account-card.js +++ b/js/src/components/google-mc-account-card/connected-google-mc-account-card.js @@ -17,6 +17,8 @@ import { API_NAMESPACE } from '.~/data/constants'; import useDispatchCoreNotices from '.~/hooks/useDispatchCoreNotices'; import useApiFetchCallback from '.~/hooks/useApiFetchCallback'; import { useAppDispatch } from '.~/data'; +import EnableNewProductSyncButton from '.~/components/enable-new-product-sync-button'; +import AppNotice from '.~/components/app-notice'; /** * Clicking on the "connect to a different Google Merchant Center account" button. @@ -32,10 +34,12 @@ import { useAppDispatch } from '.~/data'; * @param {Object} props React props. * @param {{ id: number }} props.googleMCAccount A data payload object containing the user's Google Merchant Center account ID. * @param {boolean} [props.hideAccountSwitch=false] Indicate whether hide the account switch block at the card footer. + * @param {boolean} [props.hideNotificationService=true] Indicate whether hide the enable Notification service block at the card footer. */ const ConnectedGoogleMCAccountCard = ( { googleMCAccount, hideAccountSwitch = false, + hideNotificationService = false, } ) => { const { createNotice, removeNotice } = useDispatchCoreNotices(); const { invalidateResolution } = useAppDispatch(); @@ -46,6 +50,14 @@ const ConnectedGoogleMCAccountCard = ( { method: 'DELETE', } ); + const [ + fetchDisableNotifications, + { loading: loadingDisableNotifications }, + ] = useApiFetchCallback( { + path: `${ API_NAMESPACE }/rest-api/authorize`, + method: 'DELETE', + } ); + const domain = new URL( getSetting( 'homeUrl' ) ).host; /** @@ -85,6 +97,41 @@ const ConnectedGoogleMCAccountCard = ( { removeNotice( notice.id ); }; + const disableNotifications = async () => { + const { notice } = await createNotice( + 'info', + __( + 'Disabling the new Product Sync feature, please wait…', + 'google-listings-and-ads' + ) + ); + + try { + await fetchDisableNotifications(); + invalidateResolution( 'getGoogleMCAccount', [] ); + } catch ( error ) { + createNotice( + 'error', + __( + 'Unable to disconnect your Google Merchant Center account. Please try again later.', + 'google-listings-and-ads' + ) + ); + } + + removeNotice( notice.id ); + }; + + const showDisconnectNotificationsButton = + ! hideNotificationService && + googleMCAccount.wpcom_rest_api_status === 'approved'; + + const showErrorNotificationsNotice = + ! hideNotificationService && + googleMCAccount.wpcom_rest_api_status === 'error'; + + const showFooter = ! hideAccountSwitch || showDisconnectNotificationsButton; + return ( } - > - { ! hideAccountSwitch && ( - - + ) : ( + + ) + } + > + { showDisconnectNotificationsButton && ( + + { __( + 'Google has been granted access to fetch your product data.', + 'google-listings-and-ads' + ) } + + ) } + + { showErrorNotificationsNotice && ( + + { __( + 'It was a problem granting access to Google for fetching your products.', + 'google-listings-and-ads' + ) } + + ) } + + { showFooter && ( + + { ! hideAccountSwitch && ( + + ) } + { showDisconnectNotificationsButton && ( + + ) } ) } diff --git a/js/src/components/google-mc-account-card/google-mc-account-card.js b/js/src/components/google-mc-account-card/google-mc-account-card.js index 8254fd345a..5f835c103e 100644 --- a/js/src/components/google-mc-account-card/google-mc-account-card.js +++ b/js/src/components/google-mc-account-card/google-mc-account-card.js @@ -23,7 +23,12 @@ const GoogleMCAccountCard = () => { return ; } - return ; + return ( + + ); }; export default GoogleMCAccountCard; diff --git a/package-lock.json b/package-lock.json index fdf35685c5..c2a8522fab 100644 --- a/package-lock.json +++ b/package-lock.json @@ -6,7 +6,7 @@ "packages": { "": { "name": "google-listings-and-ads", - "version": "2.5.18", + "version": "2.6.2", "license": "GPL-3.0-or-later", "dependencies": { "@woocommerce/components": "^10.3.0", diff --git a/src/API/Site/Controllers/RestAPI/AuthController.php b/src/API/Site/Controllers/RestAPI/AuthController.php index 7cdc743e05..1f75a9014c 100644 --- a/src/API/Site/Controllers/RestAPI/AuthController.php +++ b/src/API/Site/Controllers/RestAPI/AuthController.php @@ -6,6 +6,7 @@ use Automattic\WooCommerce\GoogleListingsAndAds\API\Site\Controllers\BaseController; use Automattic\WooCommerce\GoogleListingsAndAds\API\TransportMethods; use Automattic\WooCommerce\GoogleListingsAndAds\API\WP\OAuthService; +use Automattic\WooCommerce\GoogleListingsAndAds\MerchantCenter\AccountService; use Automattic\WooCommerce\GoogleListingsAndAds\Proxies\RESTServer; use Exception; use WP_REST_Request as Request; @@ -24,6 +25,11 @@ class AuthController extends BaseController { */ protected $oauth_service; + /** + * @var AccountService + */ + protected $account_service; + /** * Mapping between the client page name and its path. * The first value is also used as a default, @@ -41,10 +47,12 @@ class AuthController extends BaseController { * * @param RESTServer $server * @param OAuthService $oauth_service + * @param AccountService $account_service */ - public function __construct( RESTServer $server, OAuthService $oauth_service ) { + public function __construct( RESTServer $server, OAuthService $oauth_service, AccountService $account_service ) { parent::__construct( $server ); $this->oauth_service = $oauth_service; + $this->account_service = $account_service; } /** @@ -62,6 +70,17 @@ public function register_routes() { ], ] ); + + $this->register_route( + 'rest-api/authorize', + [ + [ + 'methods' => TransportMethods::DELETABLE, + 'callback' => $this->delete_authorize_callback(), + 'permission_callback' => $this->get_permission_callback(), + ], + ] + ); } /** @@ -87,6 +106,22 @@ protected function get_authorize_callback(): callable { }; } + /** + * Get the callback function for the delete authorization request. + * + * @return callable + */ + protected function delete_authorize_callback(): callable { + return function ( Request $request ) { + try { + $this->account_service->reset_wpcom_api_authorization(); + return $this->prepare_item_for_response( [], $request ); + } catch ( Exception $e ) { + return $this->response_from_exception( $e ); + } + }; + } + /** * Get the query params for the authorize request. * diff --git a/src/Internal/DependencyManagement/RESTServiceProvider.php b/src/Internal/DependencyManagement/RESTServiceProvider.php index 5b66e9bb26..378852f360 100644 --- a/src/Internal/DependencyManagement/RESTServiceProvider.php +++ b/src/Internal/DependencyManagement/RESTServiceProvider.php @@ -140,7 +140,7 @@ public function register() { $this->share( AttributeMappingCategoriesController::class ); $this->share( AttributeMappingSyncerController::class, ProductSyncStats::class ); $this->share( TourController::class ); - $this->share( RestAPIAuthController::class, OAuthService::class ); + $this->share( RestAPIAuthController::class, OAuthService::class, MerchantAccountService::class ); } /** diff --git a/src/MerchantCenter/AccountService.php b/src/MerchantCenter/AccountService.php index cdf4fcf1e3..5f9f114cc0 100644 --- a/src/MerchantCenter/AccountService.php +++ b/src/MerchantCenter/AccountService.php @@ -222,7 +222,7 @@ public function get_connected_status(): array { $status = [ 'id' => $id, 'status' => $id ? 'connected' : 'disconnected', - 'wpcom_rest_api_status' => $wpcom_rest_api_status === 'approved' ? 'approved' : 'disapproved', + 'wpcom_rest_api_status' => $wpcom_rest_api_status, ]; $incomplete = $this->state->last_incomplete_step(); @@ -492,4 +492,13 @@ private function prepare_exception( string $message, array $data = [], int $code return new ExceptionWithResponseData( $message, $code ?: 400, null, $data ); } + + /** + * Delete the option regarding WPCOM authorization + * + * @return bool + */ + public function reset_wpcom_api_authorization(): bool { + return $this->options->delete( OptionsInterface::WPCOM_REST_API_STATUS ); + } } From f5abf843e5b4672690973c9ebc5b42d3f2a4ae2b Mon Sep 17 00:00:00 2001 From: Miguel Perez Pellicer <5908855+puntope@users.noreply.github.com> Date: Wed, 20 Mar 2024 00:11:02 +0100 Subject: [PATCH 073/262] Restore package-lock --- package-lock.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package-lock.json b/package-lock.json index c2a8522fab..fdf35685c5 100644 --- a/package-lock.json +++ b/package-lock.json @@ -6,7 +6,7 @@ "packages": { "": { "name": "google-listings-and-ads", - "version": "2.6.2", + "version": "2.5.18", "license": "GPL-3.0-or-later", "dependencies": { "@woocommerce/components": "^10.3.0", From ea98c41db5f1af20c387a1797a13afd33786ef63 Mon Sep 17 00:00:00 2001 From: Miguel Perez Pellicer <5908855+puntope@users.noreply.github.com> Date: Wed, 20 Mar 2024 00:13:55 +0100 Subject: [PATCH 074/262] PHPCS --- src/API/Site/Controllers/RestAPI/AuthController.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/API/Site/Controllers/RestAPI/AuthController.php b/src/API/Site/Controllers/RestAPI/AuthController.php index 1f75a9014c..a43ee6d200 100644 --- a/src/API/Site/Controllers/RestAPI/AuthController.php +++ b/src/API/Site/Controllers/RestAPI/AuthController.php @@ -45,13 +45,13 @@ class AuthController extends BaseController { /** * AuthController constructor. * - * @param RESTServer $server - * @param OAuthService $oauth_service + * @param RESTServer $server + * @param OAuthService $oauth_service * @param AccountService $account_service */ public function __construct( RESTServer $server, OAuthService $oauth_service, AccountService $account_service ) { parent::__construct( $server ); - $this->oauth_service = $oauth_service; + $this->oauth_service = $oauth_service; $this->account_service = $account_service; } From 7a1d5336cf6e17655cb9c91d8a227a48c8a03af5 Mon Sep 17 00:00:00 2001 From: Miguel Perez Pellicer <5908855+puntope@users.noreply.github.com> Date: Wed, 20 Mar 2024 00:27:02 +0100 Subject: [PATCH 075/262] Update tests --- .../Controllers/RestAPI/AuthControllerTest.php | 14 +++++++++----- tests/Unit/MerchantCenter/AccountServiceTest.php | 2 +- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/tests/Unit/API/Site/Controllers/RestAPI/AuthControllerTest.php b/tests/Unit/API/Site/Controllers/RestAPI/AuthControllerTest.php index 8a1a95d43f..c977d2057d 100644 --- a/tests/Unit/API/Site/Controllers/RestAPI/AuthControllerTest.php +++ b/tests/Unit/API/Site/Controllers/RestAPI/AuthControllerTest.php @@ -4,6 +4,7 @@ use Automattic\WooCommerce\GoogleListingsAndAds\API\Site\Controllers\RestAPI\AuthController; use Automattic\WooCommerce\GoogleListingsAndAds\API\WP\OAuthService; +use Automattic\WooCommerce\GoogleListingsAndAds\MerchantCenter\AccountService; use Automattic\WooCommerce\GoogleListingsAndAds\Tests\Framework\RESTControllerUnitTest; use PHPUnit\Framework\MockObject\MockObject; use Exception; @@ -19,6 +20,9 @@ class AuthControllerTest extends RESTControllerUnitTest { /** @var MockObject|OAuthService $oauth_service */ protected $oauth_service; + /** @var MockObject|AccountService $oauth_service */ + protected $account_service; + /** @var AuthController $controller */ protected $controller; @@ -27,8 +31,9 @@ class AuthControllerTest extends RESTControllerUnitTest { public function setUp(): void { parent::setUp(); - $this->oauth_service = $this->createMock( OAuthService::class ); - $this->controller = new AuthController( $this->server, $this->oauth_service ); + $this->oauth_service = $this->createMock( OAuthService::class ); + $this->account_service = $this->createMock( AccountService::class ); + $this->controller = new AuthController( $this->server, $this->oauth_service, $this->account_service ); $this->controller->register(); } @@ -40,13 +45,12 @@ public function test_authorize() { $expected_auth_url .= '&response_type=code'; $expected_auth_url .= '&scope=wc-partner-access'; $expected_auth_url .= '&state=base64_encoded_string'; - $expected_auth_url = $expected_auth_url; $this->oauth_service->expects( $this->once() ) ->method( 'get_auth_url' ) ->willReturn( $expected_auth_url ); - $response = $this->do_request( self::ROUTE_AUTHORIZE, 'GET' ); + $response = $this->do_request( self::ROUTE_AUTHORIZE ); $data = $response->get_data(); $this->assertEquals( @@ -71,7 +75,7 @@ public function test_authorize_with_error() { ->method( 'get_auth_url' ) ->willThrowException( new Exception( 'error', 400 ) ); - $response = $this->do_request( self::ROUTE_AUTHORIZE, 'GET' ); + $response = $this->do_request( self::ROUTE_AUTHORIZE ); $this->assertEquals( [ 'message' => 'error' ], $response->get_data() ); $this->assertEquals( 400, $response->get_status() ); diff --git a/tests/Unit/MerchantCenter/AccountServiceTest.php b/tests/Unit/MerchantCenter/AccountServiceTest.php index bb77574fa6..674b0dacfd 100644 --- a/tests/Unit/MerchantCenter/AccountServiceTest.php +++ b/tests/Unit/MerchantCenter/AccountServiceTest.php @@ -686,7 +686,7 @@ public function test_get_connected_status_incomplete() { 'id' => self::TEST_ACCOUNT_ID, 'status' => 'incomplete', 'step' => 'verify', - 'wpcom_rest_api_status' => 'disapproved', + 'wpcom_rest_api_status' => null, ], $this->account->get_connected_status() ); From dc23ef9b530f722aecbf83cff28d4022fab94abe Mon Sep 17 00:00:00 2001 From: Miguel Perez Pellicer <5908855+puntope@users.noreply.github.com> Date: Wed, 20 Mar 2024 15:16:49 +0100 Subject: [PATCH 076/262] Apply review comments --- js/src/components/enable-new-product-sync-button.js | 2 +- .../connected-google-mc-account-card.js | 9 ++++++--- src/API/Site/Controllers/RestAPI/AuthController.php | 6 ------ 3 files changed, 7 insertions(+), 10 deletions(-) diff --git a/js/src/components/enable-new-product-sync-button.js b/js/src/components/enable-new-product-sync-button.js index b3917f4b9c..f4ac288c4e 100644 --- a/js/src/components/enable-new-product-sync-button.js +++ b/js/src/components/enable-new-product-sync-button.js @@ -14,7 +14,7 @@ import useApiFetchCallback from '.~/hooks/useApiFetchCallback'; import useDispatchCoreNotices from '.~/hooks/useDispatchCoreNotices'; /** - * Button to initiate auth process for WC Rest API + * Button to initiate auth process for WP Rest API * * @param {Object} params The component params * @param {string} params.buttonText The text to show in the enable button diff --git a/js/src/components/google-mc-account-card/connected-google-mc-account-card.js b/js/src/components/google-mc-account-card/connected-google-mc-account-card.js index 1d1001ce4f..ac8a61a51c 100644 --- a/js/src/components/google-mc-account-card/connected-google-mc-account-card.js +++ b/js/src/components/google-mc-account-card/connected-google-mc-account-card.js @@ -113,7 +113,7 @@ const ConnectedGoogleMCAccountCard = ( { createNotice( 'error', __( - 'Unable to disconnect your Google Merchant Center account. Please try again later.', + 'Unable to disable new product sync. Please try again later.', 'google-listings-and-ads' ) ); @@ -122,13 +122,16 @@ const ConnectedGoogleMCAccountCard = ( { removeNotice( notice.id ); }; + // Show the button if the status is "approved" and the Notification Service is not hidden. const showDisconnectNotificationsButton = ! hideNotificationService && googleMCAccount.wpcom_rest_api_status === 'approved'; + // Show the error if the status is set but is not "approved" and the Notification Service is not hidden. const showErrorNotificationsNotice = ! hideNotificationService && - googleMCAccount.wpcom_rest_api_status === 'error'; + googleMCAccount.wpcom_rest_api_status && + googleMCAccount.wpcom_rest_api_status !== 'approved'; const showFooter = ! hideAccountSwitch || showDisconnectNotificationsButton; @@ -166,7 +169,7 @@ const ConnectedGoogleMCAccountCard = ( { { showErrorNotificationsNotice && ( { __( - 'It was a problem granting access to Google for fetching your products.', + 'There was an issue granting access to Google for fetching your products.', 'google-listings-and-ads' ) } diff --git a/src/API/Site/Controllers/RestAPI/AuthController.php b/src/API/Site/Controllers/RestAPI/AuthController.php index a43ee6d200..7cb81cf34a 100644 --- a/src/API/Site/Controllers/RestAPI/AuthController.php +++ b/src/API/Site/Controllers/RestAPI/AuthController.php @@ -68,12 +68,6 @@ public function register_routes() { 'permission_callback' => $this->get_permission_callback(), 'args' => $this->get_auth_params(), ], - ] - ); - - $this->register_route( - 'rest-api/authorize', - [ [ 'methods' => TransportMethods::DELETABLE, 'callback' => $this->delete_authorize_callback(), From bf3d8435c381a8b5dad18bf7121de82c5f208530 Mon Sep 17 00:00:00 2001 From: Miguel Perez Pellicer <5908855+puntope@users.noreply.github.com> Date: Wed, 20 Mar 2024 15:58:26 +0100 Subject: [PATCH 077/262] Apply review comments --- js/src/components/enable-new-product-sync-button.js | 9 ++------- js/src/components/enable-new-product-sync-notice.js | 7 ++++++- .../connected-google-mc-account-card.js | 7 +++---- src/API/Site/Controllers/DisconnectController.php | 1 + src/ConnectionTest.php | 13 +++++++++++++ 5 files changed, 25 insertions(+), 12 deletions(-) diff --git a/js/src/components/enable-new-product-sync-button.js b/js/src/components/enable-new-product-sync-button.js index f4ac288c4e..cf0f408702 100644 --- a/js/src/components/enable-new-product-sync-button.js +++ b/js/src/components/enable-new-product-sync-button.js @@ -17,12 +17,9 @@ import useDispatchCoreNotices from '.~/hooks/useDispatchCoreNotices'; * Button to initiate auth process for WP Rest API * * @param {Object} params The component params - * @param {string} params.buttonText The text to show in the enable button * @return {JSX.Element} The button. */ -const EnableNewProductSyncButton = ( { - buttonText = __( 'Enable it', 'google-listings-and-ads' ), -} ) => { +const EnableNewProductSyncButton = ( params ) => { const { createNotice } = useDispatchCoreNotices(); const nextPageName = glaData.mcSetupComplete ? 'reconnect' : 'setup-mc'; @@ -45,9 +42,7 @@ const EnableNewProductSyncButton = ( { }; return ( - - { buttonText } - + ); }; diff --git a/js/src/components/enable-new-product-sync-notice.js b/js/src/components/enable-new-product-sync-notice.js index 3c2ca27ef3..b4d8fa64f3 100644 --- a/js/src/components/enable-new-product-sync-notice.js +++ b/js/src/components/enable-new-product-sync-notice.js @@ -39,7 +39,12 @@ const EnableNewProductSyncNotice = () => { 'google-listings-and-ads' ), { - enableButton: , + enableButton: ( + + ), } ) } diff --git a/js/src/components/google-mc-account-card/connected-google-mc-account-card.js b/js/src/components/google-mc-account-card/connected-google-mc-account-card.js index ac8a61a51c..2e9d460a42 100644 --- a/js/src/components/google-mc-account-card/connected-google-mc-account-card.js +++ b/js/src/components/google-mc-account-card/connected-google-mc-account-card.js @@ -147,10 +147,9 @@ const ConnectedGoogleMCAccountCard = ( { indicator={ showErrorNotificationsNotice ? ( ) : ( diff --git a/src/API/Site/Controllers/DisconnectController.php b/src/API/Site/Controllers/DisconnectController.php index 8eb5d06658..c0d48efc60 100644 --- a/src/API/Site/Controllers/DisconnectController.php +++ b/src/API/Site/Controllers/DisconnectController.php @@ -46,6 +46,7 @@ protected function get_disconnect_callback(): callable { 'mc/connection', 'google/connect', 'jetpack/connect', + 'rest-api/authorize', ]; $errors = []; diff --git a/src/ConnectionTest.php b/src/ConnectionTest.php index 112283ae07..a45b68d1fb 100644 --- a/src/ConnectionTest.php +++ b/src/ConnectionTest.php @@ -669,6 +669,14 @@ protected function render_admin_page() {
+ Disconnect +
+ + Disconnect +
- Disconnect -
+ Get API Pull Integration Status +
+ integration_status_response['site'] ?? ''; ?> +
integration_status_response['site'] ?? ''; ?>
+ integration_status_response['is_healthy'] ) && $this->integration_status_response['is_healthy'] === true ? 'Healthy' : 'Unhealthy'; ?> +
integration_status_response['is_healthy'] ) && $this->integration_status_response['is_healthy'] === true ? 'Healthy' : 'Unhealthy'; ?>
+ integration_status_response['last_jetpack_contact'] ) ? date( 'Y-m-d H:i:s', $this->integration_status_response['last_jetpack_contact'] ) : '-'; ?> +
integration_status_response['last_jetpack_contact'] ) ? date( 'Y-m-d H:i:s', $this->integration_status_response['last_jetpack_contact'] ) : '-'; ?>
+ integration_status_response['is_wc_rest_api_healthy'] ) && $this->integration_status_response['is_wc_rest_api_healthy'] === true ? 'Healthy' : 'Unhealthy'; ?> +
integration_status_response['is_wc_rest_api_healthy'] ) && $this->integration_status_response['is_wc_rest_api_healthy'] === true ? 'Healthy' : 'Unhealthy'; ?>
+ integration_status_response['is_partner_token_healthy'] ) && $this->integration_status_response['is_partner_token_healthy'] === true ? 'Connected' : 'Disconnected'; ?> +
integration_status_response['is_partner_token_healthy'] ) && $this->integration_status_response['is_partner_token_healthy'] === true ? 'Connected' : 'Disconnected'; ?>
+ integration_status_response['errors'] ) ? wp_kses_post( json_encode( $this->integration_status_response['errors'] ) ) ?? '' : '-'; ?> +
integration_status_response['errors'] ) ? wp_kses_post( json_encode( $this->integration_status_response['errors'] ) ) ?? '' : '-'; ?>
+ Revoke WPCOM Partner Access +
We will soon transition to a new and improved method for synchronizing product data with Google.
- Revoke WPCOM Partner Access -
+ is_enabled() ? 'yes' : 'no' ?> +
is_enabled() ? 'yes' : 'no' ?>
+ is_ready() ? 'yes' : 'no' ?> +
is_ready() ? 'yes' : 'no' ?>
+
Get API Pull Integration Status
Revoke WPCOM Partner Access
{ __( - 'Create a new Google Ads account through Google Listings & Ads and a promotional code will be automatically applied to your account. You’ll have 60 days to spend $500 to qualify for the $500 ads credit.', + 'Create a new Google Ads account through Google for WooCommerce and a promotional code will be automatically applied to your account. You’ll have 60 days to spend $500 to qualify for the $500 ads credit.', 'google-listings-and-ads' ) }
' . $policy_text . '
Google Listings & Ads connection testing page used for debugging purposes. Debug responses are output at the top of the page.
Google for WooCommerce connection testing page used for debugging purposes. Debug responses are output at the top of the page.