Check status before processing

woocommerce · Jan 31, 2024 · fd6db69 · fd6db69
1 parent bd3e0b6
commit fd6db69
Show file tree

Hide file tree

Showing 3 changed files with 110 additions and 29 deletions.
diff --git a/src/Jobs/Notifications/ProductNotificationJob.php b/src/Jobs/Notifications/ProductNotificationJob.php
@@ -75,7 +75,7 @@ protected function process_items( array $args ): void {
 		$item  = $args[0];
 		$topic = $args[1];
 
-		if ( $this->notifications_service->notify( $item, $topic ) ) {
+		if ( $this->can_process( $item, $topic ) && $this->notifications_service->notify( $item, $topic ) ) {
 			$this->set_status( $item, $this->get_after_notification_status( $topic ) );
 		}
 	}
@@ -130,4 +130,24 @@ protected function get_after_notification_status( string $topic ): string {
 			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 );
+		}
+	}
 }
diff --git a/src/Product/ProductHelper.php b/src/Product/ProductHelper.php
@@ -403,13 +403,17 @@ public function should_trigger_delete_notification( WC_Product $product ): bool
 	 */
 	public function has_notified_creation( WC_Product $product ): bool {
 		$valid_has_notified_creation_statuses = [
-			NotificationStatus::NOTIFICATION_PENDING_CREATE,
 			NotificationStatus::NOTIFICATION_CREATED,
 			NotificationStatus::NOTIFICATION_UPDATED,
-			NotificationStatus::NOTIFICATION_PENDING_UPDATE
+			NotificationStatus::NOTIFICATION_PENDING_UPDATE,
+			NotificationStatus::NOTIFICATION_PENDING_DELETE,
 		];
 
-		return in_array( $this->meta_handler->get_notification_status( $product ), $valid_has_notified_creation_statuses, true ) || $this->is_product_synced( $product );
+		return in_array(
+			$this->meta_handler->get_notification_status( $product ),
+			$valid_has_notified_creation_statuses,
+			true
+		) || $this->is_product_synced( $product );
 	}
 
 	/**

diff --git a/tests/Unit/Jobs/ProductNotificationJobTest.php b/tests/Unit/Jobs/ProductNotificationJobTest.php
@@ -15,6 +15,7 @@
 
 /**
  * Class ProductNotificationJobTest
+ *
  * @group Notifications
  * @package Automattic\WooCommerce\GoogleListingsAndAds\Tests\Unit\Jobs
  */
@@ -65,7 +66,7 @@ public function test_job_name() {
 
 	public function test_schedule_schedules_immediate_job() {
 		$topic = 'product.create';
-		$id = 1;
+		$id    = 1;
 
 		$this->action_scheduler->expects( $this->once() )
 			->method( 'has_scheduled_action' )
@@ -83,7 +84,7 @@ public function test_schedule_schedules_immediate_job() {
 	}
 
 	public function test_schedule_doesnt_schedules_immediate_job_if_already_scheduled() {
-		$id = 1;
+		$id    = 1;
 		$topic = 'product.create';
 
 		$this->action_scheduler->expects( $this->once() )
@@ -94,11 +95,10 @@ public function test_schedule_doesnt_schedules_immediate_job_if_already_schedule
 		$this->action_scheduler->expects( $this->never() )->method( 'schedule_immediate' );
 
 		$this->job->schedule( [ $id, $topic ] );
-
 	}
 
 	public function test_schedule_doesnt_schedules_immediate_job_if_filtered() {
-		$id = 1;
+		$id    = 1;
 		$topic = 'product.create';
 
 		add_filter( 'woocommerce_gla_product_notification_job_can_schedule', '__return_false' );
@@ -111,31 +111,34 @@ public function test_schedule_doesnt_schedules_immediate_job_if_filtered() {
 		$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';
+		$id      = $product->get_id();
+		$topic   = 'product.create';
 
 		$this->notification_service->expects( $this->once() )
 			->method( 'notify' )
 			->with( $id, $topic )
 			->willReturn( true );
 
-		$this->product_helper->expects( $this->once() )
+		$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() {
@@ -149,53 +152,107 @@ public function test_process_items_doesnt_calls_notify_when_no_args() {
 	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';
+		$id      = $product->get_id();
+		$topic   = 'product.create';
 
 		$this->notification_service->expects( $this->once() )
 			->method( 'notify' )
 			->with( $id, $topic )
 			->willReturn( false );
 
-		$this->product_helper->expects( $this->never() )
-			->method( 'get_wc_product' );
+		$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();
+		$id      = $product->get_id();
 
 		$this->notification_service->expects( $this->exactly( 3 ) )
 			->method( 'notify' )
 			->willReturn( true );
 
-		$this->product_helper->expects( $this->exactly( 3 ) )
+		$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;
-				} else if ( $topic === 'product.delete' ) {
-					return  NotificationStatus::NOTIFICATION_DELETED;
-				} else {
-					return  NotificationStatus::NOTIFICATION_UPDATED;
+			->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' ] );
+	}
 }