Swap variations for parent IDs for UpdateSyncableProductsCount job

src/Internal/DependencyManagement/JobServiceProvider.php

@@ -143,7 +143,7 @@ public function register(): void {
 		$this->share_product_syncer_job( CleanupProductTargetCountriesJob::class );
 
 		// Share update syncable products count job
-		$this->share_action_scheduler_job( UpdateSyncableProductsCount::class, ProductRepository::class );
+		$this->share_action_scheduler_job( UpdateSyncableProductsCount::class, ProductRepository::class, ProductHelper::class );
 	}
 
 	/**

src/Jobs/UpdateSyncableProductsCount.php

@@ -10,6 +10,7 @@
 use Automattic\WooCommerce\GoogleListingsAndAds\Options\OptionsAwareInterface;
 use Automattic\WooCommerce\GoogleListingsAndAds\Options\OptionsAwareTrait;
 use Automattic\WooCommerce\GoogleListingsAndAds\Options\OptionsInterface;
+use Automattic\WooCommerce\GoogleListingsAndAds\Product\ProductHelper;
 use Automattic\WooCommerce\GoogleListingsAndAds\Product\ProductRepository;
 
 defined( 'ABSPATH' ) || exit;
@@ -31,16 +32,23 @@ class UpdateSyncableProductsCount extends AbstractBatchedActionSchedulerJob impl
 	 */
 	protected $product_repository;
 
+	/**
+	 * @var ProductHelper
+	 */
+	protected $product_helper;
+
 	/**
 	 * UpdateSyncableProductsCount constructor.
 	 *
 	 * @param ActionSchedulerInterface  $action_scheduler
 	 * @param ActionSchedulerJobMonitor $monitor
 	 * @param ProductRepository         $product_repository
+	 * @param ProductHelper             $product_helper
 	 */
-	public function __construct( ActionSchedulerInterface $action_scheduler, ActionSchedulerJobMonitor $monitor, ProductRepository $product_repository ) {
+	public function __construct( ActionSchedulerInterface $action_scheduler, ActionSchedulerJobMonitor $monitor, ProductRepository $product_repository, ProductHelper $product_helper ) {
 		parent::__construct( $action_scheduler, $monitor );
 		$this->product_repository = $product_repository;
+		$this->product_helper     = $product_helper;
 	}
 
 	/**
@@ -78,7 +86,9 @@ protected function process_items( array $items ) {
 			$product_ids = [];
 		}
 
-		$this->options->update( OptionsInterface::SYNCABLE_PRODUCTS_COUNT_INTERMEDIATE_DATA, [ ...$product_ids, ...$items ] );
+		$grouped_items = $this->product_helper->maybe_swap_for_parent_ids( $items );
+
+		$this->options->update( OptionsInterface::SYNCABLE_PRODUCTS_COUNT_INTERMEDIATE_DATA, [ ...$product_ids, ...$grouped_items ] );
 	}
 
 	/**
@@ -89,7 +99,7 @@ protected function process_items( array $items ) {
 	 */
 	protected function handle_complete( int $final_batch_number ) {
 		$product_ids = $this->options->get( OptionsInterface::SYNCABLE_PRODUCTS_COUNT_INTERMEDIATE_DATA );
-		$count       = is_array( $product_ids ) ? count( $product_ids ) : 0;
+		$count       = is_array( $product_ids ) ? count( array_unique( $product_ids ) ) : 0;
 		$this->options->update( OptionsInterface::SYNCABLE_PRODUCTS_COUNT, $count );
 		$this->options->delete( OptionsInterface::SYNCABLE_PRODUCTS_COUNT_INTERMEDIATE_DATA );
 	}

src/Product/ProductHelper.php

@@ -465,12 +465,29 @@ public function get_mc_status( WC_Product $wc_product ): ?string {
 		}
 	}
 
+	/**
+	 * If an item from the provided list of products has a parent, replace it with the parent ID.
+	 *
+	 * @param int[] $product_ids A list of WooCommerce product ID.
+	 *
+	 * @return int[] A list of parent ID or product ID if it doesn't have a parent.
+	 *
+	 * @throws InvalidValue If a given ID doesn't reference a valid product. Or if a variation product does not have a
+	 *                      valid parent ID (i.e. it's an orphan).
+	 */
+	public function maybe_swap_for_parent_ids( array $product_ids ) {
+		foreach ( $product_ids as $index => $product_id ) {
+			$product_ids[ $index ] = $this->maybe_swap_for_parent_id( $product_id );
+		}
+		return array_unique( $product_ids );
+	}
+
 	/**
 	 * If the provided product has a parent, return its ID. Otherwise, return the given (valid product) ID.
 	 *
 	 * @param int $product_id WooCommerce product ID.
 	 *
-	 * @return int The parent ID or product ID of it doesn't have a parent.
+	 * @return int The parent ID or product ID if it doesn't have a parent.
 	 *
 	 * @throws InvalidValue If a given ID doesn't reference a valid product. Or if a variation product does not have a
 	 *                      valid parent ID (i.e. it's an orphan).

tests/Tools/HelperTrait/ProductTrait.php

@@ -90,14 +90,16 @@ public function generate_google_product_mock( $id = null, $target_country = null
 	/**
 	 * Generates and returns a mock of a simple WC_Product object
 	 *
+	 * @param int|null $product_id
+	 *
 	 * @return MockObject|WC_Product
 	 */
-	public function generate_simple_product_mock() {
+	public function generate_simple_product_mock( $product_id = null ) {
 		$product = $this->createMock( WC_Product::class );
 
 		$product->expects( $this->any() )
 				->method( 'get_id' )
-				->willReturn( rand() );
+				->willReturn( $product_id ?: rand() );
 
 		$product->expects( $this->any() )
 				->method( 'get_type' )
@@ -320,6 +322,10 @@ public function get_sample_attributes(): array {
 	 * @return WC_Product[]
 	 */
 	protected function generate_simple_product_mocks_set( int $number ) {
-		return array_fill( 0, $number, $this->generate_simple_product_mock() );
+		$products = [];
+		for ( $i = 0; $i < $number; ++$i ) {
+			$products[] = $this->generate_simple_product_mock();
+		}
+		return $products;
 	}
 }

tests/Unit/Jobs/UpdateSyncableProductsCountTest.php

@@ -9,6 +9,7 @@
 use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\UpdateSyncableProductsCount;
 use Automattic\WooCommerce\GoogleListingsAndAds\Options\OptionsInterface;
 use Automattic\WooCommerce\GoogleListingsAndAds\Product\FilteredProductList;
+use Automattic\WooCommerce\GoogleListingsAndAds\Product\ProductHelper;
 use Automattic\WooCommerce\GoogleListingsAndAds\Product\ProductRepository;
 use Automattic\WooCommerce\GoogleListingsAndAds\Tests\Framework\UnitTest;
 use Automattic\WooCommerce\GoogleListingsAndAds\Tests\Tools\HelperTrait\ProductTrait;
@@ -21,6 +22,7 @@
  *
  * @property MockObject|ActionScheduler           $action_scheduler
  * @property MockObject|ActionSchedulerJobMonitor $monitor
+ * @property MockObject|ProductHelper             $product_helper
  * @property MockObject|ProductRepository         $product_repository
  * @property MockObject|OptionsInterface          $options
  * @property UpdateSyncableProductsCount          $job
@@ -42,12 +44,14 @@ public function setUp(): void {
 
 		$this->action_scheduler   = $this->createMock( ActionSchedulerInterface::class );
 		$this->monitor            = $this->createMock( ActionSchedulerJobMonitor::class );
+		$this->product_helper     = $this->createMock( ProductHelper::class );
 		$this->product_repository = $this->createMock( ProductRepository::class );
 		$this->options            = $this->createMock( OptionsInterface::class );
 		$this->job                = new UpdateSyncableProductsCount(
 			$this->action_scheduler,
 			$this->monitor,
-			$this->product_repository
+			$this->product_repository,
+			$this->product_helper
 		);
 		$this->job->set_options_object( $this->options );
 
@@ -94,6 +98,11 @@ public function test_update_syncable_products_count() {
 			->withConsecutive( [ [], self::BATCH_SIZE, 0 ], [ [], self::BATCH_SIZE, 2 ], [ [], self::BATCH_SIZE, 4 ] )
 			->willReturnOnConsecutiveCalls( $batch_a, $batch_b, $batch_c );
 
+		$this->product_helper->expects( $this->exactly( 2 ) )
+			->method( 'maybe_swap_for_parent_ids' )
+			->withConsecutive( [ $batch_a->get_product_ids() ], [ $batch_b->get_product_ids() ] )
+			->willReturnOnConsecutiveCalls( $batch_a->get_product_ids(), $batch_b->get_product_ids() );
+
 		$this->options->expects( $this->exactly( 3 ) )
 			->method( 'get' )
 			->with( OptionsInterface::SYNCABLE_PRODUCTS_COUNT_INTERMEDIATE_DATA )
@@ -120,6 +129,76 @@ public function test_update_syncable_products_count() {
 		do_action( self::CREATE_BATCH_HOOK, 3 );
 	}
 
+	public function test_update_syncable_products_count_duplicate_product_ids() {
+		// syncable products count: 2, total products count: 2
+		$batch_a = new filteredproductlist(
+			[
+				$this->generate_simple_product_mock( 12345 ),
+				$this->generate_simple_product_mock( 12345 ),
+			],
+			2
+		);
+
+		// Syncable products count: 2, total products count: 2
+		$batch_b = new filteredproductlist(
+			[
+				$this->generate_simple_product_mock( 23456 ),
+				$this->generate_simple_product_mock( 23456 ),
+			],
+			2
+		);
+
+		// Syncable products count: 0, total products count: 0
+		$batch_c = new FilteredProductList( [], 0 );
+
+		$syncable_products = [ ...$batch_a->get_product_ids(), ...$batch_b->get_product_ids() ];
+
+		$this->action_scheduler->expects( $this->exactly( 5 ) )
+			->method( 'schedule_immediate' )
+			->withConsecutive(
+				[ self::CREATE_BATCH_HOOK, [ 1 ] ],
+				[ self::PROCESS_ITEM_HOOK, [ $batch_a->get_product_ids() ] ],
+				[ self::CREATE_BATCH_HOOK, [ 2 ] ],
+				[ self::PROCESS_ITEM_HOOK, [ $batch_b->get_product_ids() ] ],
+				[ self::CREATE_BATCH_HOOK, [ 3 ] ],
+			);
+
+		$this->product_repository->expects( $this->exactly( 3 ) )
+			->method( 'find_sync_ready_products' )
+			->withConsecutive( [ [], self::BATCH_SIZE, 0 ], [ [], self::BATCH_SIZE, 2 ], [ [], self::BATCH_SIZE, 4 ] )
+			->willReturnOnConsecutiveCalls( $batch_a, $batch_b, $batch_c );
+
+		$this->product_helper->expects( $this->exactly( 2 ) )
+			->method( 'maybe_swap_for_parent_ids' )
+			->withConsecutive( [ $batch_a->get_product_ids() ], [ $batch_b->get_product_ids() ] )
+			->willReturnOnConsecutiveCalls( $batch_a->get_product_ids(), $batch_b->get_product_ids() );
+
+		$this->options->expects( $this->exactly( 3 ) )
+			->method( 'get' )
+			->with( OptionsInterface::SYNCABLE_PRODUCTS_COUNT_INTERMEDIATE_DATA )
+			->willReturnOnConsecutiveCalls(
+				null,
+				$batch_a->get_product_ids(),
+				$syncable_products
+			);
+
+		$this->options->expects( $this->exactly( 3 ) )
+			->method( 'update' )
+			->withConsecutive(
+				[ OptionsInterface::SYNCABLE_PRODUCTS_COUNT_INTERMEDIATE_DATA, $batch_a->get_product_ids() ],
+				[ OptionsInterface::SYNCABLE_PRODUCTS_COUNT_INTERMEDIATE_DATA, $syncable_products ],
+				[ OptionsInterface::SYNCABLE_PRODUCTS_COUNT, 2 ]
+			);
+
+		$this->job->schedule();
+
+		do_action( self::CREATE_BATCH_HOOK, 1 );
+		do_action( self::PROCESS_ITEM_HOOK, $batch_a->get_product_ids() );
+		do_action( self::CREATE_BATCH_HOOK, 2 );
+		do_action( self::PROCESS_ITEM_HOOK, $batch_b->get_product_ids() );
+		do_action( self::CREATE_BATCH_HOOK, 3 );
+	}
+
 	public function test_update_syncable_products_count_no_syncable_products_single_batch() {
 		// Syncable products count: 0, total products count: 0
 		$batch = new FilteredProductList( [], 0 );