Merge pull request #1713 from woocommerce/update/1610-group-variation…

…s-type-for-syncable-products

Free Listings + Paid Ads: Group variations type for syncable products

src/API/Site/Controllers/MerchantCenter/SyncableProductsCountController.php

@@ -88,6 +88,7 @@ protected function get_syncable_products_count_callback(): callable {
 	protected function update_syncable_products_count_callback(): callable {
 		return function( Request $request ) {
 			$this->options->delete( OptionsInterface::SYNCABLE_PRODUCTS_COUNT );
+			$this->options->delete( OptionsInterface::SYNCABLE_PRODUCTS_COUNT_INTERMEDIATE_DATA );
 
 			$job = $this->job_repository->get( UpdateSyncableProductsCount::class );
 			$job->schedule();

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, array_unique( [ ...$product_ids, ...$grouped_items ] ) );
 	}
 
 	/**

src/Product/ProductHelper.php

@@ -465,12 +465,46 @@ 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.
+	 * @param bool  $check_product_status    (Optional) Check if the product status is publish.
+	 * @param bool  $ignore_product_on_error (Optional) Ignore the product when invalid value error occurs.
+	 *
+	 * @return int[] A list of parent ID or product ID if it doesn't have a parent.
+	 *
+	 * @throws InvalidValue If the given param ignore_product_on_error is false and any of 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).
+	 *
+	 * @since x.x.x
+	 */
+	public function maybe_swap_for_parent_ids( array $product_ids, bool $check_product_status = true, bool $ignore_product_on_error = true ) {
+		$new_product_ids = [];
+
+		foreach ( $product_ids as $index => $product_id ) {
+			try {
+				$product     = $this->get_wc_product( $product_id );
+				$new_product = $this->maybe_swap_for_parent( $product );
+				if ( ! $check_product_status || 'publish' === $new_product->get_status() ) {
+					$new_product_ids[ $index ] = $new_product->get_id();
+				}
+			} catch ( InvalidValue $exception ) {
+				if ( ! $ignore_product_on_error ) {
+					throw $exception;
+				}
+			}
+		}
+
+		return array_unique( $new_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,10 +9,12 @@
 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;
 use PHPUnit\Framework\MockObject\MockObject;
+use WC_Helper_Product;
 
 /**
  * Class UpdateSyncableProductsCountTest
@@ -21,6 +23,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 +45,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 +99,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 +130,74 @@ 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 );
+
+		$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,
+				[ 12345 ],
+				[ 12345, 23456 ]
+			);
+
+		$this->options->expects( $this->exactly( 3 ) )
+			->method( 'update' )
+			->withConsecutive(
+				[ OptionsInterface::SYNCABLE_PRODUCTS_COUNT_INTERMEDIATE_DATA, [ 12345 ] ],
+				[ OptionsInterface::SYNCABLE_PRODUCTS_COUNT_INTERMEDIATE_DATA, [ 12345, 23456 ] ],
+				[ 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 );
@@ -189,4 +267,83 @@ public function test_update_syncable_products_count_no_syncable_products_multipl
 		do_action( self::CREATE_BATCH_HOOK, 2 );
 		do_action( self::CREATE_BATCH_HOOK, 3 );
 	}
+
+	public function test_update_syncable_products_count_product_gets_deleted_after_batches_get_created() {
+		// This is a product in the first batch that will be deleted after the first batch gets created.
+		$product_to_be_deleted = WC_Helper_Product::create_simple_product( true, [ 'status' => 'publish' ] );
+
+		// syncable products count: 2, total products count: 2
+		$batch_a = new filteredproductlist(
+			[
+				$product_to_be_deleted,
+				$this->generate_simple_product_mock( 23456 ),
+			],
+			2
+		);
+
+		// Syncable products count: 2, total products count: 2
+		$batch_b = new filteredproductlist(
+			[
+				$this->generate_simple_product_mock( 34567 ),
+				$this->generate_simple_product_mock( 45678 ),
+			],
+			2
+		);
+
+		// Syncable products count: 0, total products count: 0
+		$batch_c = new FilteredProductList( [], 0 );
+
+		$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(), true ], [ $batch_b->get_product_ids(), true ] )
+			->willReturnOnConsecutiveCalls(
+				// The first product gets deleted after the first batch gets created,
+				// so only return the second product of batch a after calling maybe_swap_for_parent_ids.
+				[ $batch_a->get_product_ids()[1] ],
+				$batch_b->get_product_ids()
+			);
+
+		$this->options->expects( $this->exactly( 3 ) )
+			->method( 'get' )
+			->with( OptionsInterface::SYNCABLE_PRODUCTS_COUNT_INTERMEDIATE_DATA )
+			->willReturnOnConsecutiveCalls(
+				null,
+				[ 23456 ],
+				[ 23456, 34567, 45678 ]
+			);
+
+		$this->options->expects( $this->exactly( 3 ) )
+			->method( 'update' )
+			->withConsecutive(
+				[ OptionsInterface::SYNCABLE_PRODUCTS_COUNT_INTERMEDIATE_DATA, [ 23456 ] ],
+				[ OptionsInterface::SYNCABLE_PRODUCTS_COUNT_INTERMEDIATE_DATA, [ 23456, 34567, 45678 ] ],
+				[ OptionsInterface::SYNCABLE_PRODUCTS_COUNT, 3 ]
+			);
+
+		$this->job->schedule();
+
+		do_action( self::CREATE_BATCH_HOOK, 1 );
+		wp_trash_post( $product_to_be_deleted->get_id() );
+		$product_to_be_deleted->set_status( 'trash' );
+		$product_to_be_deleted->save();
+		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 );
+	}
 }