+ 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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/246] 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