From 71da3a6565f1d01688047792b13d847fab5cc6cf Mon Sep 17 00:00:00 2001 From: Jorge M Date: Tue, 26 Mar 2024 21:05:02 +0100 Subject: [PATCH 1/8] Add HelperNotificationInterface --- src/Coupon/CouponHelper.php | 13 ++-- .../AbstractItemNotificationJob.php | 7 +-- .../Notifications/CouponNotificationJob.php | 7 ++- .../HelperNotificationInterface.php | 63 +++++++++++++++++++ .../Notifications/ProductNotificationJob.php | 5 +- src/Product/ProductHelper.php | 13 ++-- 6 files changed, 87 insertions(+), 21 deletions(-) create mode 100644 src/Jobs/Notifications/HelperNotificationInterface.php diff --git a/src/Coupon/CouponHelper.php b/src/Coupon/CouponHelper.php index 8eed04ca78..9d7b5b642d 100644 --- a/src/Coupon/CouponHelper.php +++ b/src/Coupon/CouponHelper.php @@ -10,6 +10,7 @@ use Automattic\WooCommerce\GoogleListingsAndAds\Value\ChannelVisibility; use Automattic\WooCommerce\GoogleListingsAndAds\Value\NotificationStatus; use Automattic\WooCommerce\GoogleListingsAndAds\Value\SyncStatus; +use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\Notifications\HelperNotificationInterface; use WC_Coupon; defined( 'ABSPATH' ) || exit(); @@ -18,7 +19,7 @@ * * @package Automattic\WooCommerce\GoogleListingsAndAds\Coupon */ -class CouponHelper implements Service { +class CouponHelper implements Service, HelperNotificationInterface { use PluginHelper; @@ -92,7 +93,7 @@ public function mark_as_synced( * * @param WC_Coupon $coupon */ - public function mark_as_unsynced( WC_Coupon $coupon ) { + public function mark_as_unsynced( $coupon ): void { $this->meta_handler->delete_synced_at( $coupon ); $this->meta_handler->update_sync_status( $coupon, SyncStatus::NOT_SYNCED ); $this->meta_handler->delete_google_ids( $coupon ); @@ -375,7 +376,7 @@ public function has_notified_creation( WC_Coupon $coupon ): bool { * @param WC_Coupon $coupon * @param string $status */ - public function set_notification_status( WC_Coupon $coupon, $status ): void { + public function set_notification_status( $coupon, $status ): void { $this->meta_handler->update_notification_status( $coupon, $status ); } @@ -387,7 +388,7 @@ public function set_notification_status( WC_Coupon $coupon, $status ): void { * * @return bool */ - public function should_trigger_create_notification( WC_Coupon $coupon ): bool { + public function should_trigger_create_notification( $coupon ): bool { return $this->is_ready_to_notify( $coupon ) && ! $this->has_notified_creation( $coupon ); } @@ -399,7 +400,7 @@ public function should_trigger_create_notification( WC_Coupon $coupon ): bool { * * @return bool */ - public function should_trigger_update_notification( WC_Coupon $coupon ): bool { + public function should_trigger_update_notification( $coupon ): bool { return $this->is_ready_to_notify( $coupon ) && $this->has_notified_creation( $coupon ); } @@ -411,7 +412,7 @@ public function should_trigger_update_notification( WC_Coupon $coupon ): bool { * * @return bool */ - public function should_trigger_delete_notification( WC_Coupon $coupon ): bool { + public function should_trigger_delete_notification( $coupon ): bool { return ! $this->is_ready_to_notify( $coupon ) && $this->has_notified_creation( $coupon ); } } diff --git a/src/Jobs/Notifications/AbstractItemNotificationJob.php b/src/Jobs/Notifications/AbstractItemNotificationJob.php index cff11bed9c..607dc627ce 100644 --- a/src/Jobs/Notifications/AbstractItemNotificationJob.php +++ b/src/Jobs/Notifications/AbstractItemNotificationJob.php @@ -3,9 +3,8 @@ namespace Automattic\WooCommerce\GoogleListingsAndAds\Jobs\Notifications; -use Automattic\WooCommerce\GoogleListingsAndAds\Coupon\CouponHelper; -use Automattic\WooCommerce\GoogleListingsAndAds\Product\ProductHelper; use Automattic\WooCommerce\GoogleListingsAndAds\Value\NotificationStatus; +use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\Notifications\HelperNotificationInterface; defined( 'ABSPATH' ) || exit; @@ -116,7 +115,7 @@ abstract protected function get_item( int $item_id ); /** * Get the helper * - * @return CouponHelper|ProductHelper + * @return HelperNotificationInterface */ - abstract public function get_helper(); + abstract public function get_helper(): HelperNotificationInterface; } diff --git a/src/Jobs/Notifications/CouponNotificationJob.php b/src/Jobs/Notifications/CouponNotificationJob.php index c6d4d8e4fa..b7e1c9bca0 100644 --- a/src/Jobs/Notifications/CouponNotificationJob.php +++ b/src/Jobs/Notifications/CouponNotificationJob.php @@ -7,6 +7,7 @@ use Automattic\WooCommerce\GoogleListingsAndAds\API\WP\NotificationsService; use Automattic\WooCommerce\GoogleListingsAndAds\Coupon\CouponHelper; use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\ActionSchedulerJobMonitor; +use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\Notifications\HelperNotificationInterface; defined( 'ABSPATH' ) || exit; @@ -36,7 +37,7 @@ public function __construct( ActionSchedulerInterface $action_scheduler, ActionSchedulerJobMonitor $monitor, NotificationsService $notifications_service, - CouponHelper $coupon_helper + HelperNotificationInterface $coupon_helper ) { $this->notifications_service = $notifications_service; $this->helper = $coupon_helper; @@ -56,9 +57,9 @@ protected function get_item( int $item_id ) { /** * Get the Coupon Helper * - * @return CouponHelper + * @return HelperNotificationInterface */ - public function get_helper() { + public function get_helper(): HelperNotificationInterface { return $this->helper; } diff --git a/src/Jobs/Notifications/HelperNotificationInterface.php b/src/Jobs/Notifications/HelperNotificationInterface.php new file mode 100644 index 0000000000..b713bbd3c3 --- /dev/null +++ b/src/Jobs/Notifications/HelperNotificationInterface.php @@ -0,0 +1,63 @@ +notifications_service = $notifications_service; $this->helper = $helper; @@ -58,7 +59,7 @@ protected function get_item( int $item_id ) { * * @return ProductHelper */ - public function get_helper() { + public function get_helper(): HelperNotificationInterface { return $this->helper; } diff --git a/src/Product/ProductHelper.php b/src/Product/ProductHelper.php index 06480657c5..359fca460c 100644 --- a/src/Product/ProductHelper.php +++ b/src/Product/ProductHelper.php @@ -14,6 +14,7 @@ use Automattic\WooCommerce\GoogleListingsAndAds\Value\MCStatus; use Automattic\WooCommerce\GoogleListingsAndAds\Value\SyncStatus; use Automattic\WooCommerce\GoogleListingsAndAds\Vendor\Google\Service\ShoppingContent\Product as GoogleProduct; +use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\Notifications\HelperNotificationInterface; use WC_Product; use WC_Product_Variation; use WP_Post; @@ -25,7 +26,7 @@ * * @package Automattic\WooCommerce\GoogleListingsAndAds\Product */ -class ProductHelper implements Service { +class ProductHelper implements Service, HelperNotificationInterface { use PluginHelper; @@ -103,7 +104,7 @@ public function mark_as_synced( WC_Product $product, GoogleProduct $google_produ /** * @param WC_Product $product */ - public function mark_as_unsynced( WC_Product $product ) { + public function mark_as_unsynced( $product ): void { $this->meta_handler->delete_synced_at( $product ); if ( ! $this->is_sync_ready( $product ) ) { $this->meta_handler->delete_sync_status( $product ); @@ -390,7 +391,7 @@ public function is_ready_to_notify( WC_Product $product ): bool { * * @return bool */ - public function should_trigger_create_notification( WC_Product $product ): bool { + public function should_trigger_create_notification( $product ): bool { return $this->is_ready_to_notify( $product ) && ! $this->has_notified_creation( $product ); } @@ -402,7 +403,7 @@ public function should_trigger_create_notification( WC_Product $product ): bool * * @return bool */ - public function should_trigger_update_notification( WC_Product $product ): bool { + public function should_trigger_update_notification( $product ): bool { return $this->is_ready_to_notify( $product ) && $this->has_notified_creation( $product ); } @@ -414,7 +415,7 @@ public function should_trigger_update_notification( WC_Product $product ): bool * * @return bool */ - public function should_trigger_delete_notification( WC_Product $product ): bool { + public function should_trigger_delete_notification( $product ): bool { return ! $this->is_ready_to_notify( $product ) && $this->has_notified_creation( $product ); } @@ -447,7 +448,7 @@ public function has_notified_creation( WC_Product $product ): bool { * @param WC_Product $product * @param string $status */ - public function set_notification_status( WC_Product $product, $status ): void { + public function set_notification_status( $product, $status ): void { $this->meta_handler->update_notification_status( $product, $status ); } From 1a5e3a92defd88e57e80becdcddc44e66442b577 Mon Sep 17 00:00:00 2001 From: Jorge M Date: Wed, 27 Mar 2024 18:08:07 +0100 Subject: [PATCH 2/8] Mark as notified coupon --- src/Coupon/CouponHelper.php | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/Coupon/CouponHelper.php b/src/Coupon/CouponHelper.php index 9d7b5b642d..7a02b25ce2 100644 --- a/src/Coupon/CouponHelper.php +++ b/src/Coupon/CouponHelper.php @@ -58,6 +58,19 @@ public function __construct( $this->merchant_center = $merchant_center; } + /** + * Mark the item as notified. + * + * @param WC_Coupon $coupon + * + * @return void + */ + public function mark_as_notified( $coupon ): void { + $this->meta_handler->update_synced_at( $coupon, time() ); + $this->meta_handler->update_sync_status( $coupon, SyncStatus::SYNCED ); + $this->update_empty_visibility( $coupon ); + } + /** * Mark a coupon as synced. This function accepts nullable $google_id, * which guarantees version compatibility for Alpha, Beta and stable verison promtoion APIs. From 583f2c2be00af77236f457f323929f077c8706bd Mon Sep 17 00:00:00 2001 From: Jorge M Date: Wed, 27 Mar 2024 18:08:28 +0100 Subject: [PATCH 3/8] Mark as notified product --- src/Product/ProductHelper.php | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/Product/ProductHelper.php b/src/Product/ProductHelper.php index 359fca460c..9572bbc4f1 100644 --- a/src/Product/ProductHelper.php +++ b/src/Product/ProductHelper.php @@ -58,6 +58,31 @@ public function __construct( ProductMetaHandler $meta_handler, WC $wc, TargetAud $this->target_audience = $target_audience; } + /** + * Mark the item as notified. + * + * @param WC_Product $product + * + * @return void + */ + public function mark_as_notified( $product ): void { + $this->meta_handler->delete_failed_delete_attempts( $product ); + $this->meta_handler->update_synced_at( $product, time() ); + $this->meta_handler->update_sync_status( $product, SyncStatus::SYNCED ); + $this->update_empty_visibility( $product ); + + // mark the parent product as synced if it's a variation + if ( $product instanceof WC_Product_Variation ) { + try { + $parent_product = $this->get_wc_product( $product->get_parent_id() ); + } catch ( InvalidValue $exception ) { + return; + } + + $this->mark_as_notified( $parent_product ); + } + } + /** * Mark a product as synced in the local database. * This function also handles the following cleanup tasks: From 43ee65a22314d1fde07447ae695c82b39ed43589 Mon Sep 17 00:00:00 2001 From: Jorge M Date: Wed, 27 Mar 2024 18:08:49 +0100 Subject: [PATCH 4/8] Add handle notified --- .../Notifications/AbstractItemNotificationJob.php | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/Jobs/Notifications/AbstractItemNotificationJob.php b/src/Jobs/Notifications/AbstractItemNotificationJob.php index 607dc627ce..e9ac1a3df4 100644 --- a/src/Jobs/Notifications/AbstractItemNotificationJob.php +++ b/src/Jobs/Notifications/AbstractItemNotificationJob.php @@ -37,7 +37,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 ); + $this->handle_notified( $topic, $item ); } } @@ -90,18 +90,19 @@ protected function can_process( int $item_id, string $topic ): bool { } /** - * If there is a valid Item ID and topic is a deletion topic. Mark the item as unsynced. + * Handle the item after the notification. * * @param string $topic * @param int $item */ - protected function maybe_mark_as_unsynced( string $topic, int $item ): void { - if ( ! str_contains( $topic, '.delete' ) ) { - return; + protected function handle_notified( string $topic, int $item ): void { + if ( str_contains( $topic, '.delete' ) ) { + $this->get_helper()->mark_as_unsynced( $this->get_item( $item ) ); } - $item = $this->get_item( $item ); - $this->get_helper()->mark_as_unsynced( $item ); + if ( str_contains( $topic, '.create' ) ) { + $this->get_helper()->mark_as_notified( $this->get_item( $item ) ); + } } /** From 54f08d4e4e349587f60f335101598fb2f620ace3 Mon Sep 17 00:00:00 2001 From: Jorge M Date: Wed, 27 Mar 2024 18:09:21 +0100 Subject: [PATCH 5/8] Adjust PHPDocs for the new interface --- .../Notifications/CouponNotificationJob.php | 8 +++--- .../HelperNotificationInterface.php | 26 ++++++++++++------- .../Notifications/ProductNotificationJob.php | 8 +++--- 3 files changed, 25 insertions(+), 17 deletions(-) diff --git a/src/Jobs/Notifications/CouponNotificationJob.php b/src/Jobs/Notifications/CouponNotificationJob.php index b7e1c9bca0..2ecc891a35 100644 --- a/src/Jobs/Notifications/CouponNotificationJob.php +++ b/src/Jobs/Notifications/CouponNotificationJob.php @@ -28,10 +28,10 @@ class CouponNotificationJob extends AbstractItemNotificationJob { /** * Notifications Jobs constructor. * - * @param ActionSchedulerInterface $action_scheduler - * @param ActionSchedulerJobMonitor $monitor - * @param NotificationsService $notifications_service - * @param CouponHelper $coupon_helper + * @param ActionSchedulerInterface $action_scheduler + * @param ActionSchedulerJobMonitor $monitor + * @param NotificationsService $notifications_service + * @param HelperNotificationInterface $coupon_helper */ public function __construct( ActionSchedulerInterface $action_scheduler, diff --git a/src/Jobs/Notifications/HelperNotificationInterface.php b/src/Jobs/Notifications/HelperNotificationInterface.php index b713bbd3c3..a675200225 100644 --- a/src/Jobs/Notifications/HelperNotificationInterface.php +++ b/src/Jobs/Notifications/HelperNotificationInterface.php @@ -11,7 +11,7 @@ * @since x.x.x * @package Automattic\WooCommerce\GoogleListingsAndAds\Jobs\Notifications */ -interface HelperNotificationInterface { +interface HelperNotificationInterface { /** * Checks if the item can be processed based on the topic. @@ -39,25 +39,33 @@ public function should_trigger_delete_notification( $item ): bool; * * @return bool */ - public function should_trigger_update_notification( $item ): bool; + public function should_trigger_update_notification( $item ): bool; /** * Marks the item as unsynced. * * @param WC_Product|WC_Coupon $item * - * @return bool - */ + * @return void + */ public function mark_as_unsynced( $item ): void; /** * Set the notification status for an item. * * @param WC_Product|WC_Coupon $product - * @param string $status + * @param string $status * - * @return bool - */ - public function set_notification_status( $item, $status ): void; + * @return void + */ + public function set_notification_status( $item, $status ): void; -} \ No newline at end of file + /** + * Marks the item as notified. + * + * @param WC_Product|WC_Coupon $item + * + * @return void + */ + public function mark_as_notified( $item ): void; +} diff --git a/src/Jobs/Notifications/ProductNotificationJob.php b/src/Jobs/Notifications/ProductNotificationJob.php index 9ad3753496..5779f13c77 100644 --- a/src/Jobs/Notifications/ProductNotificationJob.php +++ b/src/Jobs/Notifications/ProductNotificationJob.php @@ -28,10 +28,10 @@ class ProductNotificationJob extends AbstractItemNotificationJob { /** * Notifications Jobs constructor. * - * @param ActionSchedulerInterface $action_scheduler - * @param ActionSchedulerJobMonitor $monitor - * @param NotificationsService $notifications_service - * @param ProductHelper $helper + * @param ActionSchedulerInterface $action_scheduler + * @param ActionSchedulerJobMonitor $monitor + * @param NotificationsService $notifications_service + * @param HelperNotificationInterface $helper */ public function __construct( ActionSchedulerInterface $action_scheduler, From ebf45a7bad74a401bbd4a71c434e02cfdfc7d91b Mon Sep 17 00:00:00 2001 From: Jorge M Date: Wed, 27 Mar 2024 18:09:40 +0100 Subject: [PATCH 6/8] Adjust tests for create notification --- .../Jobs/AbstractItemNotificationJobTest.php | 30 +++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/tests/Unit/Jobs/AbstractItemNotificationJobTest.php b/tests/Unit/Jobs/AbstractItemNotificationJobTest.php index d883646e33..66115750e2 100644 --- a/tests/Unit/Jobs/AbstractItemNotificationJobTest.php +++ b/tests/Unit/Jobs/AbstractItemNotificationJobTest.php @@ -147,7 +147,7 @@ public function test_process_items_calls_notify_and_set_status_on_success() { ->with( $topic, $id ) ->willReturn( true ); - $this->job->get_helper()->expects( $this->exactly( 2 ) ) + $this->job->get_helper()->expects( $this->exactly( 3 ) ) ->method( 'get_wc_' . $this->get_topic_name() ) ->with( $id ) ->willReturn( $item ); @@ -218,7 +218,7 @@ public function test_get_after_notification_status() { ->method( 'notify' ) ->willReturn( true ); - $this->job->get_helper()->expects( $this->exactly( 7 ) ) + $this->job->get_helper()->expects( $this->exactly( 8 ) ) ->method( 'get_wc_' . $this->get_topic_name() ) ->willReturn( $item ); @@ -345,6 +345,32 @@ public function test_mark_as_unsynced_when_delete() { ); } + public function test_mark_as_notified_when_create() { + $item = $this->create_item(); + $id = $item->get_id(); + + $this->job->get_helper()->expects( $this->once() ) + ->method( 'should_trigger_create_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_notified' ); + + $this->job->handle_process_items_action( + [ + 'item_id' => $id, + 'topic' => $this->get_topic_name() . '.create', + ] + ); + } + public function get_name() { return 'notifications/' . $this->get_job_name(); } From d2472c1ee125810ee6d215cf67d16b2b2232b0ed Mon Sep 17 00:00:00 2001 From: Jorge M Date: Wed, 27 Mar 2024 18:34:34 +0100 Subject: [PATCH 7/8] Fix phpcs --- src/Jobs/Notifications/HelperNotificationInterface.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Jobs/Notifications/HelperNotificationInterface.php b/src/Jobs/Notifications/HelperNotificationInterface.php index a675200225..5fe61b6db7 100644 --- a/src/Jobs/Notifications/HelperNotificationInterface.php +++ b/src/Jobs/Notifications/HelperNotificationInterface.php @@ -53,7 +53,7 @@ public function mark_as_unsynced( $item ): void; /** * Set the notification status for an item. * - * @param WC_Product|WC_Coupon $product + * @param WC_Product|WC_Coupon $item * @param string $status * * @return void From 3c57d12c2d45ee45d01e7585f6533ea1b46e2404 Mon Sep 17 00:00:00 2001 From: Jorge M Date: Wed, 27 Mar 2024 18:39:32 +0100 Subject: [PATCH 8/8] Fix phpcs tests --- tests/Unit/Jobs/AbstractItemNotificationJobTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Unit/Jobs/AbstractItemNotificationJobTest.php b/tests/Unit/Jobs/AbstractItemNotificationJobTest.php index 66115750e2..47d4d78c48 100644 --- a/tests/Unit/Jobs/AbstractItemNotificationJobTest.php +++ b/tests/Unit/Jobs/AbstractItemNotificationJobTest.php @@ -369,7 +369,7 @@ public function test_mark_as_notified_when_create() { 'topic' => $this->get_topic_name() . '.create', ] ); - } + } public function get_name() { return 'notifications/' . $this->get_job_name();