From 73a242db0e139a8f831a0bd056d1fb5410383648 Mon Sep 17 00:00:00 2001 From: Jorge M Date: Sat, 16 Mar 2024 21:34:21 +0100 Subject: [PATCH 01/24] Allow to refresh Stats --- js/src/hooks/useMCProductStatistics.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/js/src/hooks/useMCProductStatistics.js b/js/src/hooks/useMCProductStatistics.js index 0c4e9a0d3d..8e009b369c 100644 --- a/js/src/hooks/useMCProductStatistics.js +++ b/js/src/hooks/useMCProductStatistics.js @@ -8,6 +8,7 @@ import { useEffect } from '@wordpress/element'; */ import useAppSelectDispatch from './useAppSelectDispatch'; import useCountdown from './useCountdown'; +import useApiFetchCallback from '.~/hooks/useApiFetchCallback'; /** * Call `useAppSelectDispatch` with `"getMCProductStatistics"`. @@ -22,6 +23,16 @@ const useMCProductStatistics = () => { const isLoading = hasFinishedResolution && data?.loading ? true : false; + const [ refreshProductStats ] = useApiFetchCallback( { + path: `/wc/gla/mc/product-statistics/refresh`, + method: 'GET', + } ); + + const refreshStats = async () => { + await refreshProductStats(); + invalidateResolution( 'getMCProductStatistics', [] ); + }; + if ( isLoading && callCount === 0 ) { startCountdown( 30 ); } @@ -37,6 +48,7 @@ const useMCProductStatistics = () => { data, invalidateResolution, hasFinishedResolution, + refreshStats, ...rest, }; }; From 0530ff8ddb23f9ab8bb8fb72f48d1664c01144f4 Mon Sep 17 00:00:00 2001 From: Jorge M Date: Sat, 16 Mar 2024 21:35:14 +0100 Subject: [PATCH 02/24] Add SyncMCStatus --- .../product-feed/product-statistics/index.js | 8 +++- .../product-statistics/index.scss | 6 +++ .../status-box/sync-mc-status.js | 46 +++++++++++++++++++ 3 files changed, 59 insertions(+), 1 deletion(-) create mode 100644 js/src/product-feed/product-statistics/status-box/sync-mc-status.js diff --git a/js/src/product-feed/product-statistics/index.js b/js/src/product-feed/product-statistics/index.js index 688dba6b32..637187525c 100644 --- a/js/src/product-feed/product-statistics/index.js +++ b/js/src/product-feed/product-statistics/index.js @@ -22,6 +22,7 @@ import { import useMCProductStatistics from '.~/hooks/useMCProductStatistics'; import ProductStatusHelpPopover from './product-status-help-popover'; import SyncStatus from '.~/product-feed/product-statistics/status-box/sync-status'; +import SyncMCStatus from '.~/product-feed/product-statistics/status-box/sync-mc-status'; import FeedStatus from '.~/product-feed/product-statistics/status-box/feed-status'; import AccountStatus from '.~/product-feed/product-statistics/status-box/account-status'; import AppSpinner from '.~/components/app-spinner'; @@ -29,7 +30,8 @@ import Text from '.~/components/app-text'; import './index.scss'; const ProductStatistics = () => { - const { hasFinishedResolution, data } = useMCProductStatistics(); + const { hasFinishedResolution, data, refreshStats } = + useMCProductStatistics(); if ( hasFinishedResolution && ! data ) { return __( @@ -125,6 +127,10 @@ const ProductStatistics = () => { + ); diff --git a/js/src/product-feed/product-statistics/index.scss b/js/src/product-feed/product-statistics/index.scss index 5329d1b931..f70aa6bad6 100644 --- a/js/src/product-feed/product-statistics/index.scss +++ b/js/src/product-feed/product-statistics/index.scss @@ -93,5 +93,11 @@ .gridicons-sync { transform: rotateZ(90deg); } + + .overview-stats-error-button { + height: auto; + padding-top: 0px; + padding-left: 0px; + } } } diff --git a/js/src/product-feed/product-statistics/status-box/sync-mc-status.js b/js/src/product-feed/product-statistics/status-box/sync-mc-status.js new file mode 100644 index 0000000000..8635ad7024 --- /dev/null +++ b/js/src/product-feed/product-statistics/status-box/sync-mc-status.js @@ -0,0 +1,46 @@ +/** + * External dependencies + */ +import { __ } from '@wordpress/i18n'; + +/** + * Internal dependencies + */ +import Status from '.~/product-feed/product-statistics/status-box/status'; +import ErrorIcon from '.~/components/error-icon'; +import { Button } from '@wordpress/components'; + +/** + * @typedef {import('.~/data/actions').ProductStatistics } ProductStatistics + */ + +/** + * Renders status information for the Product Sync + * + * @param {Object} props The component props + * @param {Function} props.refreshStats + * @param {string} props.error + * @return {JSX.Element} The status for the Product Sync + */ +function SyncMCStatus( { refreshStats, error } ) { + return ( + } + label={ + + } + description={ null } + /> + ); +} + +export default SyncMCStatus; From 989d2259f737595c43d3340f1770d73dcd723321 Mon Sep 17 00:00:00 2001 From: Jorge M Date: Tue, 19 Mar 2024 10:51:45 +0100 Subject: [PATCH 03/24] Rename component to SyncProductStatistics --- js/src/product-feed/product-statistics/index.js | 12 +++++++----- ...{sync-mc-status.js => sync-product-statistics.js} | 7 ++++--- 2 files changed, 11 insertions(+), 8 deletions(-) rename js/src/product-feed/product-statistics/status-box/{sync-mc-status.js => sync-product-statistics.js} (87%) diff --git a/js/src/product-feed/product-statistics/index.js b/js/src/product-feed/product-statistics/index.js index 57ed31a457..87888bf977 100644 --- a/js/src/product-feed/product-statistics/index.js +++ b/js/src/product-feed/product-statistics/index.js @@ -22,7 +22,7 @@ import { import useMCProductStatistics from '.~/hooks/useMCProductStatistics'; import ProductStatusHelpPopover from './product-status-help-popover'; import SyncStatus from '.~/product-feed/product-statistics/status-box/sync-status'; -import SyncMCStatus from '.~/product-feed/product-statistics/status-box/sync-mc-status'; +import SyncProductStatistics from '.~/product-feed/product-statistics/status-box/sync-product-statistics'; import FeedStatus from '.~/product-feed/product-statistics/status-box/feed-status'; import AccountStatus from '.~/product-feed/product-statistics/status-box/account-status'; import Text from '.~/components/app-text'; @@ -107,10 +107,12 @@ const ProductStatistics = () => { - + { hasFinishedResolution && data?.error && ( + + ) } ); diff --git a/js/src/product-feed/product-statistics/status-box/sync-mc-status.js b/js/src/product-feed/product-statistics/status-box/sync-product-statistics.js similarity index 87% rename from js/src/product-feed/product-statistics/status-box/sync-mc-status.js rename to js/src/product-feed/product-statistics/status-box/sync-product-statistics.js index 8635ad7024..575d804027 100644 --- a/js/src/product-feed/product-statistics/status-box/sync-mc-status.js +++ b/js/src/product-feed/product-statistics/status-box/sync-product-statistics.js @@ -22,7 +22,7 @@ import { Button } from '@wordpress/components'; * @param {string} props.error * @return {JSX.Element} The status for the Product Sync */ -function SyncMCStatus( { refreshStats, error } ) { +function SyncProductStatistics( { refreshStats, error } ) { return ( { __( - 'There was an error loading the Overview Stats. Click to retry.' + 'There was an error loading the Overview Stats. Click to retry.', + 'google-listings-and-ads' ) } } @@ -43,4 +44,4 @@ function SyncMCStatus( { refreshStats, error } ) { ); } -export default SyncMCStatus; +export default SyncProductStatistics; From f37e94e5b7f912d1dec955ed8826f5553cbe1651 Mon Sep 17 00:00:00 2001 From: Jorge M Date: Tue, 19 Mar 2024 11:07:34 +0100 Subject: [PATCH 04/24] Handle case where failure rate message is too high --- src/Jobs/UpdateMerchantProductStatuses.php | 13 +++++++++++++ src/MerchantCenter/MerchantStatuses.php | 11 +++++++++++ 2 files changed, 24 insertions(+) diff --git a/src/Jobs/UpdateMerchantProductStatuses.php b/src/Jobs/UpdateMerchantProductStatuses.php index 52d7e8c778..6632f6ff33 100644 --- a/src/Jobs/UpdateMerchantProductStatuses.php +++ b/src/Jobs/UpdateMerchantProductStatuses.php @@ -128,4 +128,17 @@ public function is_scheduled(): bool { // We set 'args' to null so it matches any arguments. This is because it's possible to have multiple instances of the job running with different page tokens return $this->is_running( null ); } + + /** + * Validate the failure rate of the job. + * + * @return string|null Returns an error message if the failure rate is too high, otherwise null. + */ + public function get_failure_rate_message() { + try { + $this->monitor->validate_failure_rate( $this, $this->get_process_item_hook() ); + } catch ( JobException $e ) { + return $e->getMessage(); + } + } } diff --git a/src/MerchantCenter/MerchantStatuses.php b/src/MerchantCenter/MerchantStatuses.php index 7c8d4a0751..039b7e5000 100644 --- a/src/MerchantCenter/MerchantStatuses.php +++ b/src/MerchantCenter/MerchantStatuses.php @@ -136,8 +136,19 @@ public function __construct() { */ public function get_product_statistics( bool $force_refresh = false ): array { $job = $this->maybe_refresh_status_data( $force_refresh ); + $failure_rate_msg = $job->get_failure_rate_message(); $this->mc_statuses = $this->container->get( TransientsInterface::class )->get( Transients::MC_STATUSES ); + // If the failure rate is too high, return an error message so the UI can stop polling. + if ( $failure_rate_msg && null === $this->mc_statuses ) { + return [ + 'timestamp' => $this->cache_created_time->getTimestamp(), + 'statistics' => [], + 'loading' => false, + 'error' => $failure_rate_msg, + ]; + } + if ( $job->is_scheduled() || null === $this->mc_statuses ) { return [ 'timestamp' => $this->cache_created_time->getTimestamp(), From 3079d4a410c7ae389eec0ae1c9c6be72b324b339 Mon Sep 17 00:00:00 2001 From: Jorge M Date: Tue, 19 Mar 2024 21:21:44 +0100 Subject: [PATCH 05/24] Clear cache before job runs so we can delete any stale error --- src/MerchantCenter/MerchantStatuses.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/MerchantCenter/MerchantStatuses.php b/src/MerchantCenter/MerchantStatuses.php index a66b0f3508..3ecd06b498 100644 --- a/src/MerchantCenter/MerchantStatuses.php +++ b/src/MerchantCenter/MerchantStatuses.php @@ -289,6 +289,8 @@ public function maybe_refresh_status_data( bool $force_refresh = false ): Update // If force_refresh is true or if not transient, return empty array and scheduled the job to update the statuses. if ( ! $job->is_scheduled() && ( $force_refresh || ( ! $force_refresh && null === $this->mc_statuses ) ) ) { + // Delete the transient before scheduling the job because some errors, like the failure rate message, can occur before the job is scheduled. + $this->clear_cache(); // Schedule job to update the statuses. $job->schedule(); } From 762937b95ac8d6fac7127ef207d28b84ec4d1e03 Mon Sep 17 00:00:00 2001 From: Jorge M Date: Tue, 19 Mar 2024 21:22:15 +0100 Subject: [PATCH 06/24] Add tests for clear cache before schedule a job --- tests/Unit/MerchantCenter/MerchantStatusesTest.php | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/Unit/MerchantCenter/MerchantStatusesTest.php b/tests/Unit/MerchantCenter/MerchantStatusesTest.php index 5b17c0bee4..1e15c5b527 100644 --- a/tests/Unit/MerchantCenter/MerchantStatusesTest.php +++ b/tests/Unit/MerchantCenter/MerchantStatusesTest.php @@ -334,6 +334,18 @@ public function test_get_product_statistics_without_transient() { $this->update_merchant_product_statuses_job->expects( $this->exactly( 1 ) ) ->method( 'schedule' ); + $this->update_all_product_job->expects( $this->exactly( 1 ) ) + ->method( 'can_schedule' ) + ->with( null ) + ->willReturn( true ); + + $this->delete_all_product_job->expects( $this->exactly( 1 ) ) + ->method( 'can_schedule' ) + ->with( null ) + ->willReturn( true ); + + $this->transients->expects( $this->exactly( 1 ) )->method( 'delete' )->with( Transients::MC_STATUSES ); + $this->update_merchant_product_statuses_job->expects( $this->exactly( 2 ) ) ->method( 'is_scheduled' ); From 7f3eba466fa1d622d6f938d01edfa78582d60bef Mon Sep 17 00:00:00 2001 From: Jorge M Date: Tue, 19 Mar 2024 21:22:49 +0100 Subject: [PATCH 07/24] Add the description for SyncProductStatistics --- .../product-statistics/status-box/sync-product-statistics.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/src/product-feed/product-statistics/status-box/sync-product-statistics.js b/js/src/product-feed/product-statistics/status-box/sync-product-statistics.js index 575d804027..0e8459018d 100644 --- a/js/src/product-feed/product-statistics/status-box/sync-product-statistics.js +++ b/js/src/product-feed/product-statistics/status-box/sync-product-statistics.js @@ -39,7 +39,7 @@ function SyncProductStatistics( { refreshStats, error } ) { ) } } - description={ null } + description={ error } /> ); } From 1146e15337b7a5742029fa9325a7dbaa9f30a756 Mon Sep 17 00:00:00 2001 From: Jorge M Date: Tue, 19 Mar 2024 21:23:57 +0100 Subject: [PATCH 08/24] Pass the error to SyncProductStatistics --- js/src/product-feed/product-statistics/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/src/product-feed/product-statistics/index.js b/js/src/product-feed/product-statistics/index.js index 86c3c85ebe..429313809d 100644 --- a/js/src/product-feed/product-statistics/index.js +++ b/js/src/product-feed/product-statistics/index.js @@ -140,7 +140,7 @@ const ProductStatistics = () => { { hasFinishedResolution && data?.error && ( ) } From 24af8c290f4efc0c1b86bb80d7a249db47aea977 Mon Sep 17 00:00:00 2001 From: Jorge M Date: Tue, 19 Mar 2024 21:50:19 +0100 Subject: [PATCH 09/24] Add tests When there is an error --- .../product-statistics/index.test.js | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/js/src/product-feed/product-statistics/index.test.js b/js/src/product-feed/product-statistics/index.test.js index 1158f4a91b..cf136a7a0f 100644 --- a/js/src/product-feed/product-statistics/index.test.js +++ b/js/src/product-feed/product-statistics/index.test.js @@ -119,4 +119,33 @@ describe( 'Product Statistics', () => { } ); } ); } ); + describe( `When there is an error`, () => { + it( 'Should render the error', () => { + useMCProductStatistics.mockImplementation( () => { + return { + hasFinishedResolution: true, + data: { + loading: false, + statistics: null, + error: 'Error loading stats!', + }, + }; + } ); + + const { getByText, container } = render( ); + + const notAvailableStats = container.querySelectorAll( + '.woocommerce-summary__item-value' + ); + + expect( notAvailableStats.length ).toBe( 5 ); + + for ( let i = 0; i < notAvailableStats.length; i++ ) { + expect( notAvailableStats[ i ].textContent ).toBe( 'N/A' ); + } + + expect( getByText( 'Overview Stats:' ) ).toBeInTheDocument(); + expect( getByText( 'Error loading stats!' ) ).toBeInTheDocument(); + } ); + } ); } ); From c2139550918a20a4ec75beca7be9472a39945a61 Mon Sep 17 00:00:00 2001 From: Jorge M Date: Tue, 19 Mar 2024 22:04:24 +0100 Subject: [PATCH 10/24] Js linting --- js/src/product-feed/product-statistics/index.scss | 4 ++-- .../product-statistics/status-box/sync-product-statistics.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/js/src/product-feed/product-statistics/index.scss b/js/src/product-feed/product-statistics/index.scss index 52d87ff75a..95a76b2c51 100644 --- a/js/src/product-feed/product-statistics/index.scss +++ b/js/src/product-feed/product-statistics/index.scss @@ -96,8 +96,8 @@ .overview-stats-error-button { height: auto; - padding-top: 0px; - padding-left: 0px; + padding-top: 0; + padding-left: 0; } } } diff --git a/js/src/product-feed/product-statistics/status-box/sync-product-statistics.js b/js/src/product-feed/product-statistics/status-box/sync-product-statistics.js index 0e8459018d..8b044f5153 100644 --- a/js/src/product-feed/product-statistics/status-box/sync-product-statistics.js +++ b/js/src/product-feed/product-statistics/status-box/sync-product-statistics.js @@ -2,13 +2,13 @@ * External dependencies */ import { __ } from '@wordpress/i18n'; +import { Button } from '@wordpress/components'; /** * Internal dependencies */ import Status from '.~/product-feed/product-statistics/status-box/status'; import ErrorIcon from '.~/components/error-icon'; -import { Button } from '@wordpress/components'; /** * @typedef {import('.~/data/actions').ProductStatistics } ProductStatistics From 065ff6dd3ae43beb4eb5118a1d2236c230f66124 Mon Sep 17 00:00:00 2001 From: Jorge M Date: Tue, 19 Mar 2024 22:18:56 +0100 Subject: [PATCH 11/24] Add tests for refreshStats --- js/src/hooks/useMCProductStatistics.js | 2 +- js/src/hooks/useMCProductStatistics.test.js | 41 +++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/js/src/hooks/useMCProductStatistics.js b/js/src/hooks/useMCProductStatistics.js index 65dfab34a4..418e1f29b0 100644 --- a/js/src/hooks/useMCProductStatistics.js +++ b/js/src/hooks/useMCProductStatistics.js @@ -35,7 +35,7 @@ const useMCProductStatistics = () => { const refreshStats = async () => { await refreshProductStats(); - invalidateResolution( 'getMCProductStatistics', [] ); + invalidateResolution(); }; useEffect( () => { diff --git a/js/src/hooks/useMCProductStatistics.test.js b/js/src/hooks/useMCProductStatistics.test.js index 32448c2ef5..073f308612 100644 --- a/js/src/hooks/useMCProductStatistics.test.js +++ b/js/src/hooks/useMCProductStatistics.test.js @@ -10,10 +10,17 @@ import useMCProductStatistics from '.~/hooks/useMCProductStatistics'; import useCountdown from '.~/hooks/useCountdown'; import useAppSelectDispatch from '.~/hooks/useAppSelectDispatch'; import { useAppDispatch } from '.~/data'; +import useApiFetchCallback from '.~/hooks/useApiFetchCallback'; jest.mock( '.~/hooks/useAppSelectDispatch' ); jest.mock( '.~/hooks/useCountdown' ); jest.mock( '.~/data' ); +jest.mock( '.~/hooks/useApiFetchCallback', () => ( { + __esModule: true, + default: jest.fn().mockImplementation( () => { + return [ jest.fn(), null ]; + } ), +} ) ); describe( 'useMCProductStatistics', () => { const startCountdown = jest.fn(); @@ -139,4 +146,38 @@ describe( 'useMCProductStatistics', () => { ); } ); } ); + describe( 'Refresh the product statistics', () => { + beforeAll( () => { + jest.clearAllMocks(); + } ); + it( 'The user clicks the refresh button', async () => { + const refreshProductStats = jest.fn(); + useCountdown.mockImplementation( () => { + return { + second: 0, + callCount: 0, + startCountdown, + }; + } ); + + useAppSelectDispatch.mockImplementation( () => { + return { + hasFinishedResolution: true, + invalidateResolution, + data: statsData, + }; + } ); + + useApiFetchCallback.mockImplementation( () => { + return [ refreshProductStats, null ]; + } ); + + const { result } = renderHook( () => useMCProductStatistics() ); + + await result.current.refreshStats(); + + expect( refreshProductStats ).toHaveBeenCalledTimes( 1 ); + expect( invalidateResolution ).toHaveBeenCalledTimes( 1 ); + } ); + } ); } ); From 6b0b4f91c1be83c2e2c1a474342e0d05c568ebbd Mon Sep 17 00:00:00 2001 From: Jorge M Date: Wed, 20 Mar 2024 15:22:11 +0100 Subject: [PATCH 12/24] Delete MC statuses --- src/DB/ProductMetaQueryHelper.php | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/DB/ProductMetaQueryHelper.php b/src/DB/ProductMetaQueryHelper.php index 976e16d8e2..11612aeaa0 100644 --- a/src/DB/ProductMetaQueryHelper.php +++ b/src/DB/ProductMetaQueryHelper.php @@ -59,6 +59,21 @@ public function get_all_values( string $meta_key ): array { return $return; } + /** + * Delete all values for a given meta_key. + * + * @param string $meta_key The meta key to delete. + * + * @throws InvalidMeta If the meta key isn't valid. + */ + public function delete_all_values( string $meta_key ) { + self::validate_meta_key( $meta_key ); + $meta_key = $this->prefix_meta_key( $meta_key ); + $query = "DELETE FROM {$this->wpdb->postmeta} WHERE meta_key = %s"; + // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared + $this->wpdb->query( $this->wpdb->prepare( $query, $meta_key ) ); + } + /** * Insert a meta value for multiple posts. * From 6a33e7574ae84ecfeb89710e5615a81a2c0da236 Mon Sep 17 00:00:00 2001 From: Jorge M Date: Wed, 20 Mar 2024 16:19:37 +0100 Subject: [PATCH 13/24] Add tests for get_failure_rate_message --- src/Jobs/UpdateMerchantProductStatuses.php | 2 +- .../UpdateMerchantProductStatusesTest.php | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/Jobs/UpdateMerchantProductStatuses.php b/src/Jobs/UpdateMerchantProductStatuses.php index 6632f6ff33..12d9c43ea4 100644 --- a/src/Jobs/UpdateMerchantProductStatuses.php +++ b/src/Jobs/UpdateMerchantProductStatuses.php @@ -132,7 +132,7 @@ public function is_scheduled(): bool { /** * Validate the failure rate of the job. * - * @return string|null Returns an error message if the failure rate is too high, otherwise null. + * @return string|void Returns an error message if the failure rate is too high, otherwise null. */ public function get_failure_rate_message() { try { diff --git a/tests/Unit/Jobs/UpdateMerchantProductStatusesTest.php b/tests/Unit/Jobs/UpdateMerchantProductStatusesTest.php index e57a24f3f0..16155d2495 100644 --- a/tests/Unit/Jobs/UpdateMerchantProductStatusesTest.php +++ b/tests/Unit/Jobs/UpdateMerchantProductStatusesTest.php @@ -248,4 +248,23 @@ public function test_update_merchant_product_statuses_when_view_report_throws_ex do_action( self::PROCESS_ITEM_HOOK, [] ); } + + public function test_get_failure_rate_message() { + $this->merchant_center_service->method( 'is_connected' ) + ->willReturn( true ); + + $this->monitor->expects( $this->exactly( 1 ) )->method( 'validate_failure_rate' ) + ->willThrowException( new JobException( 'The "update_merchant_product_statuses" job was stopped because its failure rate is above the allowed threshold.' ) ); + + $this->assertEquals( 'The "update_merchant_product_statuses" job was stopped because its failure rate is above the allowed threshold.', $this->job->get_failure_rate_message() ); + } + + public function test_get_with_no_failure_rate_message() { + $this->merchant_center_service->method( 'is_connected' ) + ->willReturn( true ); + + $this->monitor->expects( $this->exactly( 1 ) )->method( 'validate_failure_rate' ); + + $this->assertNull( $this->job->get_failure_rate_message() ); + } } From ef985e8c73494ae89b8dac20fdbb47bb3438eb38 Mon Sep 17 00:00:00 2001 From: Jorge M Date: Wed, 20 Mar 2024 16:30:42 +0100 Subject: [PATCH 14/24] Tweak response of failure rate and docs --- src/MerchantCenter/MerchantStatuses.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/MerchantCenter/MerchantStatuses.php b/src/MerchantCenter/MerchantStatuses.php index 3ecd06b498..50c2150046 100644 --- a/src/MerchantCenter/MerchantStatuses.php +++ b/src/MerchantCenter/MerchantStatuses.php @@ -143,7 +143,7 @@ public function get_product_statistics( bool $force_refresh = false ): array { if ( $failure_rate_msg && null === $this->mc_statuses ) { return [ 'timestamp' => $this->cache_created_time->getTimestamp(), - 'statistics' => [], + 'statistics' => null, 'loading' => false, 'error' => $failure_rate_msg, ]; @@ -289,9 +289,9 @@ public function maybe_refresh_status_data( bool $force_refresh = false ): Update // If force_refresh is true or if not transient, return empty array and scheduled the job to update the statuses. if ( ! $job->is_scheduled() && ( $force_refresh || ( ! $force_refresh && null === $this->mc_statuses ) ) ) { - // Delete the transient before scheduling the job because some errors, like the failure rate message, can occur before the job is scheduled. + // Delete the transient before scheduling the job because some errors, like the failure rate message, can occur before the job is executed. $this->clear_cache(); - // Schedule job to update the statuses. + // Schedule job to update the statuses. If the failure rate is too high, the job will not be scheduled. $job->schedule(); } From 0d0b1c18bfa8d7fa20ae66c196497a4b32076d9a Mon Sep 17 00:00:00 2001 From: Jorge M Date: Wed, 20 Mar 2024 16:31:03 +0100 Subject: [PATCH 15/24] Add tests for get statistics with failure rate --- .../MerchantCenter/MerchantStatusesTest.php | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/tests/Unit/MerchantCenter/MerchantStatusesTest.php b/tests/Unit/MerchantCenter/MerchantStatusesTest.php index 1e15c5b527..27ab1ee6d2 100644 --- a/tests/Unit/MerchantCenter/MerchantStatusesTest.php +++ b/tests/Unit/MerchantCenter/MerchantStatusesTest.php @@ -444,6 +444,44 @@ public function test_get_product_statistics_with_force_refresh() { ); } + public function test_get_product_statistics_with_failure_rate() { + $this->merchant_center_service->expects( $this->once() ) + ->method( 'is_connected' ) + ->willReturn( true ); + + $this->transients->expects( $this->exactly( 2 ) ) + ->method( 'get' ) + ->willReturn( + null + ); + + $this->update_merchant_product_statuses_job->expects( $this->exactly( 1 ) ) + ->method( 'get_failure_rate_message' )->willReturn( 'Failure rate!' ); + + $this->update_merchant_product_statuses_job->expects( $this->exactly( 1 ) ) + ->method( 'schedule' ); + + $this->update_merchant_product_statuses_job->expects( $this->exactly( 1 ) ) + ->method( 'is_scheduled' )->willReturn( false ); + + $product_statistics = $this->merchant_statuses->get_product_statistics(); + + $this->assertEquals( + null, + $product_statistics['statistics'] + ); + + $this->assertEquals( + false, + $product_statistics['loading'] + ); + + $this->assertEquals( + 'Failure rate!', + $product_statistics['error'] + ); + } + public function test_process_product_statuses() { $product_1 = WC_Helper_Product::create_simple_product(); $product_2 = WC_Helper_Product::create_simple_product(); From 15df732e793aa0d33b36d330a33e2c3a114eae9f Mon Sep 17 00:00:00 2001 From: Jorge M Date: Thu, 21 Mar 2024 11:45:43 +0100 Subject: [PATCH 16/24] Add delete_stale_mc_statuses method --- src/MerchantCenter/MerchantStatuses.php | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/MerchantCenter/MerchantStatuses.php b/src/MerchantCenter/MerchantStatuses.php index 3ecd06b498..95be76e057 100644 --- a/src/MerchantCenter/MerchantStatuses.php +++ b/src/MerchantCenter/MerchantStatuses.php @@ -238,6 +238,16 @@ protected function delete_stale_issues(): void { $this->container->get( MerchantIssueTable::class )->delete_stale( $this->cache_created_time ); } + /** + * Delete the stale mc statuses from the database. + * + * @since x.x.x + */ + protected function delete_stale_mc_statuses(): void { + $product_meta_query_helper = $this->container->get( ProductMetaQueryHelper::class ); + $product_meta_query_helper->delete_all_values( ProductMetaHandler::KEY_MC_STATUS ); + } + /** * Clear the product statuses cache and delete stale issues. * @@ -245,6 +255,7 @@ protected function delete_stale_issues(): void { */ public function clear_product_statuses_cache_and_issues(): void { $this->delete_stale_issues(); + $this->delete_stale_mc_statuses(); $this->delete_product_statuses_count_intermediate_data(); } From 7a055ce7762791a92788f56f1f8b88b0b6427475 Mon Sep 17 00:00:00 2001 From: Jorge M Date: Thu, 21 Mar 2024 11:45:58 +0100 Subject: [PATCH 17/24] Add tests for delete_stale_mc_statuses --- tests/Unit/MerchantCenter/MerchantStatusesTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/Unit/MerchantCenter/MerchantStatusesTest.php b/tests/Unit/MerchantCenter/MerchantStatusesTest.php index 1e15c5b527..2a84497674 100644 --- a/tests/Unit/MerchantCenter/MerchantStatusesTest.php +++ b/tests/Unit/MerchantCenter/MerchantStatusesTest.php @@ -996,6 +996,7 @@ protected function test_clear_product_statuses_cache_and_issues() { $this->merchant_issue_table->expects( $this->once() )->method( 'delete_stale' )->with( $this->merchant_statuses->get_cache_created_time() ); $this->options->expects( $this->once() )->method( 'delete' )->with( OptionsInterface::PRODUCT_STATUSES_COUNT_INTERMEDIATE_DATA ); + $this->product_meta_query_helper->expects($this->once())->method('delete_meta_key')->with(ProductMetaHandler::KEY_MC_STATUS); $this->merchant_statuses->clear_product_statuses_cache_and_issues(); } From 68e6df3542f1ae919445ea406a02016c5c24bc72 Mon Sep 17 00:00:00 2001 From: Jorge M Date: Thu, 21 Mar 2024 15:42:13 +0100 Subject: [PATCH 18/24] Update transient time --- src/MerchantCenter/MerchantStatuses.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/MerchantCenter/MerchantStatuses.php b/src/MerchantCenter/MerchantStatuses.php index 95be76e057..91a611f664 100644 --- a/src/MerchantCenter/MerchantStatuses.php +++ b/src/MerchantCenter/MerchantStatuses.php @@ -58,7 +58,7 @@ class MerchantStatuses implements Service, ContainerAwareInterface, OptionsAware /** * The lifetime of the status-related data. */ - public const STATUS_LIFETIME = HOUR_IN_SECONDS; + public const STATUS_LIFETIME = 12 * HOUR_IN_SECONDS; /** * The types of issues. From e0327689fee205b5b88653575f69eb4c19fb8bc0 Mon Sep 17 00:00:00 2001 From: Jorge M Date: Thu, 21 Mar 2024 19:46:33 +0100 Subject: [PATCH 19/24] Adjust tests for clear_product_statuses_cache_and_issues --- tests/Unit/MerchantCenter/MerchantStatusesTest.php | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/tests/Unit/MerchantCenter/MerchantStatusesTest.php b/tests/Unit/MerchantCenter/MerchantStatusesTest.php index 2a84497674..df2920a5ed 100644 --- a/tests/Unit/MerchantCenter/MerchantStatusesTest.php +++ b/tests/Unit/MerchantCenter/MerchantStatusesTest.php @@ -987,16 +987,10 @@ public function test_clear_cache_with_bulk_sync() { $this->merchant_statuses->clear_cache(); } - protected function test_clear_product_statuses_cache_and_issues() { - $this->transients->expects( $this->exactly( 1 ) ) - ->method( 'delete' ) - ->with( - TransientsInterface::MC_STATUSES - ); - + public function test_clear_product_statuses_cache_and_issues() { $this->merchant_issue_table->expects( $this->once() )->method( 'delete_stale' )->with( $this->merchant_statuses->get_cache_created_time() ); $this->options->expects( $this->once() )->method( 'delete' )->with( OptionsInterface::PRODUCT_STATUSES_COUNT_INTERMEDIATE_DATA ); - $this->product_meta_query_helper->expects($this->once())->method('delete_meta_key')->with(ProductMetaHandler::KEY_MC_STATUS); + $this->product_meta_query_helper->expects( $this->once() )->method( 'delete_all_values' )->with( ProductMetaHandler::KEY_MC_STATUS ); $this->merchant_statuses->clear_product_statuses_cache_and_issues(); } From 8ec00b8ede848120a5f95b772ed3699d7f9b51b5 Mon Sep 17 00:00:00 2001 From: Jorge M Date: Fri, 22 Mar 2024 20:36:55 +0100 Subject: [PATCH 20/24] Add missing since --- src/DB/ProductMetaQueryHelper.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/DB/ProductMetaQueryHelper.php b/src/DB/ProductMetaQueryHelper.php index 11612aeaa0..0fc99a1c02 100644 --- a/src/DB/ProductMetaQueryHelper.php +++ b/src/DB/ProductMetaQueryHelper.php @@ -62,6 +62,8 @@ public function get_all_values( string $meta_key ): array { /** * Delete all values for a given meta_key. * + * @since x.x.x + * * @param string $meta_key The meta key to delete. * * @throws InvalidMeta If the meta key isn't valid. From 345d019019e423eb7df00d3487e1d77b9c74f137 Mon Sep 17 00:00:00 2001 From: Jorge M Date: Fri, 22 Mar 2024 20:53:35 +0100 Subject: [PATCH 21/24] Add useCallback to refresh product stats --- js/src/hooks/useMCProductStatistics.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/js/src/hooks/useMCProductStatistics.js b/js/src/hooks/useMCProductStatistics.js index 418e1f29b0..ec66acb68d 100644 --- a/js/src/hooks/useMCProductStatistics.js +++ b/js/src/hooks/useMCProductStatistics.js @@ -1,7 +1,7 @@ /** * External dependencies */ -import { useEffect } from '@wordpress/element'; +import { useEffect, useCallback } from '@wordpress/element'; /** * Internal dependencies @@ -33,10 +33,10 @@ const useMCProductStatistics = () => { method: 'GET', } ); - const refreshStats = async () => { + const refreshStats = useCallback( async () => { await refreshProductStats(); invalidateResolution(); - }; + }, [ refreshProductStats, invalidateResolution ] ); useEffect( () => { // If the job is still processing the data, start the countdown. From 4aa642660327c9e9084f3db594119cbe04dc7f06 Mon Sep 17 00:00:00 2001 From: Jorge M Date: Fri, 22 Mar 2024 20:54:05 +0100 Subject: [PATCH 22/24] Use AppButton instead of Button --- .../status-box/sync-product-statistics.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/js/src/product-feed/product-statistics/status-box/sync-product-statistics.js b/js/src/product-feed/product-statistics/status-box/sync-product-statistics.js index 8b044f5153..c24aed40ca 100644 --- a/js/src/product-feed/product-statistics/status-box/sync-product-statistics.js +++ b/js/src/product-feed/product-statistics/status-box/sync-product-statistics.js @@ -2,13 +2,13 @@ * External dependencies */ import { __ } from '@wordpress/i18n'; -import { Button } from '@wordpress/components'; /** * Internal dependencies */ import Status from '.~/product-feed/product-statistics/status-box/status'; import ErrorIcon from '.~/components/error-icon'; +import AppButton from '.~/components/app-button'; /** * @typedef {import('.~/data/actions').ProductStatistics } ProductStatistics @@ -28,16 +28,17 @@ function SyncProductStatistics( { refreshStats, error } ) { title={ __( 'Overview Stats:', 'google-listings-and-ads' ) } icon={ } label={ - + } description={ error } /> From 986c6d24d2177caeba2d902ab9970f7384479291 Mon Sep 17 00:00:00 2001 From: Jorge M Date: Fri, 22 Mar 2024 21:03:17 +0100 Subject: [PATCH 23/24] Change error message for the failure rate msg --- src/MerchantCenter/MerchantStatuses.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/MerchantCenter/MerchantStatuses.php b/src/MerchantCenter/MerchantStatuses.php index 50c2150046..7a0a0837a0 100644 --- a/src/MerchantCenter/MerchantStatuses.php +++ b/src/MerchantCenter/MerchantStatuses.php @@ -145,7 +145,7 @@ public function get_product_statistics( bool $force_refresh = false ): array { 'timestamp' => $this->cache_created_time->getTimestamp(), 'statistics' => null, 'loading' => false, - 'error' => $failure_rate_msg, + 'error' => __( 'The scheduled job has been paused due to a high failure rate.', 'google-listings-and-ads' ), ]; } From 7adeb2493fc997c90cdb927cecff1c2e3b8a52ff Mon Sep 17 00:00:00 2001 From: Jorge M Date: Fri, 22 Mar 2024 21:03:56 +0100 Subject: [PATCH 24/24] Update tests --- tests/Unit/MerchantCenter/MerchantStatusesTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Unit/MerchantCenter/MerchantStatusesTest.php b/tests/Unit/MerchantCenter/MerchantStatusesTest.php index 27ab1ee6d2..b8ca9b8ded 100644 --- a/tests/Unit/MerchantCenter/MerchantStatusesTest.php +++ b/tests/Unit/MerchantCenter/MerchantStatusesTest.php @@ -456,7 +456,7 @@ public function test_get_product_statistics_with_failure_rate() { ); $this->update_merchant_product_statuses_job->expects( $this->exactly( 1 ) ) - ->method( 'get_failure_rate_message' )->willReturn( 'Failure rate!' ); + ->method( 'get_failure_rate_message' )->willReturn( 'The scheduled job has been paused due to a high failure rate.' ); $this->update_merchant_product_statuses_job->expects( $this->exactly( 1 ) ) ->method( 'schedule' ); @@ -477,7 +477,7 @@ public function test_get_product_statistics_with_failure_rate() { ); $this->assertEquals( - 'Failure rate!', + 'The scheduled job has been paused due to a high failure rate.', $product_statistics['error'] ); }