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/CoreServiceProvider.php

@@ -315,7 +315,7 @@ function ( ...$arguments ) {
 		$this->share_with_tags( ProductMetaHandler::class );
 		$this->share( ProductHelper::class, ProductMetaHandler::class, WC::class, TargetAudience::class );
 		$this->share_with_tags( ProductFilter::class, ProductHelper::class );
-		$this->share_with_tags( ProductRepository::class, ProductMetaHandler::class, ProductFilter::class );
+		$this->share_with_tags( ProductRepository::class, ProductMetaHandler::class, ProductFilter::class, ProductHelper::class );
 		$this->share_with_tags( ProductFactory::class, AttributeManager::class, WC::class );
 		$this->share_with_tags(
 			BatchProductHelper::class,

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/MerchantCenter/MerchantStatuses.php

@@ -230,9 +230,15 @@ protected function get_synced_google_ids(): array {
 		/** @var ProductMetaQueryHelper $product_meta_query_helper */
 		$product_meta_query_helper = $this->container->get( ProductMetaQueryHelper::class );
 
-		// Get only synced simple and variations
+		// Get only synced simple and variations, and group variation products if they have the same parent
+		/** @var ProductHelper $product_helper */
+		$product_helper       = $this->container->get( ProductHelper::class );
 		$args['type']         = array_diff( ProductSyncer::get_supported_product_types(), [ 'variable' ] );
-		$filtered_product_ids = array_flip( $product_repository->find_synced_product_ids( $args ) );
+		$filtered_product_ids = array_flip(
+			$product_helper->maybe_swap_for_parent_ids(
+				$product_repository->find_synced_product_ids( $args )
+			)
+		);
 		$all_google_ids       = $product_meta_query_helper->get_all_values( ProductMetaHandler::KEY_GOOGLE_IDS );
 		$filtered_google_ids  = [];
 		foreach ( array_intersect_key( $all_google_ids, $filtered_product_ids ) as $product_ids ) {

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).

src/Product/ProductRepository.php

@@ -5,9 +5,11 @@
 
 use Automattic\WooCommerce\GoogleListingsAndAds\Infrastructure\Service;
 use Automattic\WooCommerce\GoogleListingsAndAds\PluginHelper;
+use Automattic\WooCommerce\GoogleListingsAndAds\Product\ProductHelper;
 use Automattic\WooCommerce\GoogleListingsAndAds\Value\ChannelVisibility;
 use Automattic\WooCommerce\GoogleListingsAndAds\Value\MCStatus;
 use WC_Product;
+use WC_Product_Variable;
 
 defined( 'ABSPATH' ) || exit;
 
@@ -32,15 +34,22 @@ class ProductRepository implements Service {
 	 */
 	protected $product_filter;
 
+	/**
+	 * @var ProductHelper
+	 */
+	protected $product_helper;
+
 	/**
 	 * ProductRepository constructor.
 	 *
 	 * @param ProductMetaHandler $meta_handler
 	 * @param ProductFilter      $product_filter
+	 * @param ProductHelper      $product_helper
 	 */
-	public function __construct( ProductMetaHandler $meta_handler, ProductFilter $product_filter ) {
+	public function __construct( ProductMetaHandler $meta_handler, ProductFilter $product_filter, ProductHelper $product_helper ) {
 		$this->meta_handler   = $meta_handler;
 		$this->product_filter = $product_filter;
+		$this->product_helper = $product_helper;
 	}
 
 	/**
@@ -248,20 +257,21 @@ public function find_expiring_product_ids( int $limit = - 1, int $offset = 0 ):
 	}
 
 	/**
-	 * Find and return an array of WooCommerce product IDs that are marked as MC not_synced.
-	 * Excludes variations and variable products without variations.
+	 * Find and return an array of WooCommerce product IDs that are syncable but marked as MC not_synced.
+	 * Excludes:
+	 * - variable products without variations, and
+	 * - variable products with all of their variations are not syncable.
 	 *
 	 * @param int $limit  Maximum number of results to retrieve or -1 for unlimited.
 	 * @param int $offset Amount to offset product results.
 	 *
 	 * @return int[] Array of WooCommerce product IDs
 	 */
 	public function find_mc_not_synced_product_ids( int $limit = -1, int $offset = 0 ): array {
-		$types = ProductSyncer::get_supported_product_types();
-		$types = array_diff( $types, [ 'variation' ] );
-		$args  = [
-			'status'     => 'publish',
-			'type'       => $types,
+		// Get simple and variable products that are marked as MC 'not synced'.
+		$args = [
+			'status'     => [ 'publish' ],
+			'type'       => array_diff( ProductSyncer::get_supported_product_types(), [ 'variation' ] ),
 			'meta_query' => [
 				[
 					'key'     => ProductMetaHandler::KEY_MC_STATUS,
@@ -271,7 +281,39 @@ public function find_mc_not_synced_product_ids( int $limit = -1, int $offset = 0
 			],
 		];
 
-		return $this->find_ids( $args, $limit, $offset );
+		$not_synced_ids = $this->find_ids( $args, $limit, $offset );
+
+		// Get only the product that is syncable from 'not synced' products,
+		// including variation products of a variable product.
+		$product_ids = array_filter(
+			$not_synced_ids,
+			function ( $product_id ) {
+				$product = $this->product_helper->get_wc_product( $product_id );
+
+				// If it's a variable product, check its children variation products.
+				if ( $product instanceof WC_Product_Variable ) {
+					if ( empty( $product->get_children() ) ) {
+						// Return false if no variation products. I.e. it's not syncable.
+						return false;
+					} else {
+						// Get a list of variation products, return true if any of a varation product
+						// is syncable, otherwise return false.
+						foreach ( $product->get_children() as $variation_id ) {
+							$variation = $this->product_helper->get_wc_product( $variation_id );
+							if ( $this->product_helper->is_sync_ready( $variation ) ) {
+								return true;
+							}
+						}
+						return false;
+					}
+				}
+
+				// Check if a simple product is syncable.
+				return $this->product_helper->is_sync_ready( $product );
+			}
+		);
+
+		return $product_ids;
 	}
 
 	/**

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 );