diff --git a/src/DB/ProductFeedQueryHelper.php b/src/DB/ProductFeedQueryHelper.php index 7c8edf5049..34de20127a 100644 --- a/src/DB/ProductFeedQueryHelper.php +++ b/src/DB/ProductFeedQueryHelper.php @@ -15,6 +15,7 @@ use Automattic\WooCommerce\GoogleListingsAndAds\Product\ProductRepository; use Automattic\WooCommerce\GoogleListingsAndAds\Product\ProductSyncer; use Automattic\WooCommerce\GoogleListingsAndAds\Value\ChannelVisibility; +use Automattic\WooCommerce\GoogleListingsAndAds\Value\MCStatus; use WP_Query; use WP_REST_Request; use wpdb; @@ -97,7 +98,8 @@ public function get( WP_REST_Request $request ): array { foreach ( $this->product_repository->find( $args, $limit, $offset ) as $product ) { $id = $product->get_id(); $errors = $product_helper->get_validation_errors( $product ); - $mc_status = $product_helper->get_mc_status( $product ) ?: $product_helper->get_sync_status( $product ); + $mc_status = $product_helper->get_mc_status( $product ) + ?? ( $product_helper->is_product_synced( $product ) ? MCStatus::PENDING : MCStatus::NOT_SYNCED ); // If the refresh_status_data_job is scheduled, we don't know the status yet as it is being refreshed. if ( $refresh_status_data_job && $refresh_status_data_job->is_scheduled() ) { diff --git a/src/Product/ProductHelper.php b/src/Product/ProductHelper.php index c0c7781b06..3552eb45a1 100644 --- a/src/Product/ProductHelper.php +++ b/src/Product/ProductHelper.php @@ -382,10 +382,7 @@ public function get_wc_product_by_wp_post( int $product_id ): ?WP_Post { * @return bool */ public function is_product_synced( WC_Product $product ): bool { - $synced_at = $this->meta_handler->get_synced_at( $product ); - $google_ids = $this->meta_handler->get_google_ids( $product ); - - return ! empty( $synced_at ) && ! empty( $google_ids ); + return SyncStatus::SYNCED === $this->meta_handler->get_sync_status( $product ); } /** @@ -642,8 +639,7 @@ public function get_sync_status( WC_Product $wc_product ): ?string { */ public function get_mc_status( WC_Product $wc_product ): ?string { try { - // If the mc_status is not set, return NOT_SYNCED. - return $this->meta_handler->get_mc_status( $this->maybe_swap_for_parent( $wc_product ) ) ?: MCStatus::NOT_SYNCED; + return $this->meta_handler->get_mc_status( $this->maybe_swap_for_parent( $wc_product ) ) ?: null; } catch ( InvalidValue $exception ) { do_action( 'woocommerce_gla_debug_message', diff --git a/src/Product/ProductRepository.php b/src/Product/ProductRepository.php index 7f2fe55c2e..caf4a17d7e 100644 --- a/src/Product/ProductRepository.php +++ b/src/Product/ProductRepository.php @@ -6,6 +6,7 @@ use Automattic\WooCommerce\GoogleListingsAndAds\Infrastructure\Service; use Automattic\WooCommerce\GoogleListingsAndAds\PluginHelper; use Automattic\WooCommerce\GoogleListingsAndAds\Value\ChannelVisibility; +use Automattic\WooCommerce\GoogleListingsAndAds\Value\SyncStatus; use WC_Product; defined( 'ABSPATH' ) || exit; @@ -153,10 +154,16 @@ public function find_synced_product_ids( array $args = [], int $limit = -1, int */ protected function get_synced_products_meta_query(): array { return [ + 'relation' => 'OR', [ 'key' => ProductMetaHandler::KEY_GOOGLE_IDS, 'compare' => 'EXISTS', ], + [ + 'key' => ProductMetaHandler::KEY_SYNC_STATUS, + 'value' => SyncStatus::SYNCED, + 'compare' => '=', + ], ]; } diff --git a/tests/Unit/Product/ProductHelperTest.php b/tests/Unit/Product/ProductHelperTest.php index 088a630c25..779f98de48 100644 --- a/tests/Unit/Product/ProductHelperTest.php +++ b/tests/Unit/Product/ProductHelperTest.php @@ -608,29 +608,30 @@ public function test_is_product_synced( string $callback ) { } /** + * Pull-synced products (via mark_as_notified) have no google_ids but sync_status = synced. + * is_product_synced() must return true for them. + * * @param string $callback * * @dataProvider return_test_product_callbacks */ - public function test_is_product_synced_return_false_if_no_google_id( string $callback ) { + public function test_is_product_synced_return_true_for_pull_synced_product( string $callback ) { $product = call_user_func( $callback ); - $this->product_helper->mark_as_synced( $product, $this->generate_google_product_mock() ); - $this->product_meta->delete_google_ids( $product ); - $is_product_synced = $this->product_helper->is_product_synced( $product ); - $this->assertFalse( $is_product_synced ); + $this->product_helper->mark_as_notified( $product ); + $this->assertEmpty( $this->product_meta->get_google_ids( $product ) ); + $this->assertTrue( $this->product_helper->is_product_synced( $product ) ); } /** + * A product with no sync_status set must not be considered synced. + * * @param string $callback * * @dataProvider return_test_product_callbacks */ - public function test_is_product_synced_return_false_if_no_synced_at( string $callback ) { + public function test_is_product_synced_return_false_when_not_synced( string $callback ) { $product = call_user_func( $callback ); - $this->product_helper->mark_as_synced( $product, $this->generate_google_product_mock() ); - $this->product_meta->delete_synced_at( $product ); - $is_product_synced = $this->product_helper->is_product_synced( $product ); - $this->assertFalse( $is_product_synced ); + $this->assertFalse( $this->product_helper->is_product_synced( $product ) ); } /** @@ -989,6 +990,19 @@ public function test_get_mc_status( string $callback ) { $this->assertEquals( MCStatus::APPROVED, $this->product_helper->get_mc_status( $product ) ); } + /** + * When no mc_status meta is stored, get_mc_status() must return null so the + * fallback in ProductFeedQueryHelper can read sync_status instead. + * + * @param string $callback + * + * @dataProvider return_test_product_callbacks + */ + public function test_get_mc_status_returns_null_when_not_set( string $callback ) { + $product = call_user_func( $callback ); + $this->assertNull( $this->product_helper->get_mc_status( $product ) ); + } + public function test_get_mc_status_variation_product() { $parent = WC_Helper_Product::create_variation_product(); $variation = $this->wc->get_product( $parent->get_children()[0] ); diff --git a/tests/Unit/Product/ProductRepositoryTest.php b/tests/Unit/Product/ProductRepositoryTest.php index e9eb6f2435..ea00b4a6e9 100644 --- a/tests/Unit/Product/ProductRepositoryTest.php +++ b/tests/Unit/Product/ProductRepositoryTest.php @@ -201,6 +201,21 @@ public function test_find_synced_products() { ); } + public function test_find_synced_products_includes_pull_synced() { + $product_1 = WC_Helper_Product::create_simple_product(); + $this->product_helper->mark_as_notified( $product_1 ); + + WC_Helper_Product::create_simple_product(); + + $result_ids = array_map( [ __CLASS__, 'get_product_id' ], $this->product_repository->find_synced_products() ); + $this->assertEquals( [ $product_1->get_id() ], $result_ids ); + + $this->assertEquals( + [ $product_1->get_id() ], + $this->product_repository->find_synced_product_ids() + ); + } + public function test_find_sync_ready_products() { // create some products that are not sync ready