diff --git a/js/src/hooks/useMCProductStatistics.js b/js/src/hooks/useMCProductStatistics.js index 01628136ed..ec66acb68d 100644 --- a/js/src/hooks/useMCProductStatistics.js +++ b/js/src/hooks/useMCProductStatistics.js @@ -1,13 +1,14 @@ /** * External dependencies */ -import { useEffect } from '@wordpress/element'; +import { useEffect, useCallback } from '@wordpress/element'; /** * Internal dependencies */ import useAppSelectDispatch from './useAppSelectDispatch'; import useCountdown from './useCountdown'; +import useApiFetchCallback from '.~/hooks/useApiFetchCallback'; import { useAppDispatch } from '.~/data'; /** @@ -27,6 +28,16 @@ const useMCProductStatistics = () => { hasFinishedResolution && data?.loading ? true : false; const hasStats = hasFinishedResolution && data?.statistics ? true : false; + const [ refreshProductStats ] = useApiFetchCallback( { + path: `/wc/gla/mc/product-statistics/refresh`, + method: 'GET', + } ); + + const refreshStats = useCallback( async () => { + await refreshProductStats(); + invalidateResolution(); + }, [ refreshProductStats, invalidateResolution ] ); + useEffect( () => { // If the job is still processing the data, start the countdown. if ( isCalculatingStats && second === 0 ) { @@ -57,6 +68,7 @@ const useMCProductStatistics = () => { data, invalidateResolution, hasFinishedResolution, + refreshStats, ...rest, }; }; 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 ); + } ); + } ); } ); diff --git a/js/src/product-feed/product-statistics/index.js b/js/src/product-feed/product-statistics/index.js index 26816934a8..429313809d 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 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'; @@ -29,7 +30,8 @@ import AppSpinner from '.~/components/app-spinner'; import './index.scss'; const ProductStatistics = () => { - const { hasFinishedResolution, data } = useMCProductStatistics(); + const { hasFinishedResolution, data, refreshStats } = + useMCProductStatistics(); if ( hasFinishedResolution && ! data ) { return __( @@ -135,6 +137,12 @@ const ProductStatistics = () => { + { hasFinishedResolution && data?.error && ( + + ) } ); diff --git a/js/src/product-feed/product-statistics/index.scss b/js/src/product-feed/product-statistics/index.scss index da323039f1..95a76b2c51 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: 0; + padding-left: 0; + } } } 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(); + } ); + } ); } ); 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 new file mode 100644 index 0000000000..c24aed40ca --- /dev/null +++ b/js/src/product-feed/product-statistics/status-box/sync-product-statistics.js @@ -0,0 +1,48 @@ +/** + * 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 AppButton from '.~/components/app-button'; + +/** + * @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 SyncProductStatistics( { refreshStats, error } ) { + return ( + } + label={ + + { __( + 'There was an error loading the Overview Stats. Click to retry.', + 'google-listings-and-ads' + ) } + + } + description={ error } + /> + ); +} + +export default SyncProductStatistics; diff --git a/src/DB/ProductMetaQueryHelper.php b/src/DB/ProductMetaQueryHelper.php index 976e16d8e2..0fc99a1c02 100644 --- a/src/DB/ProductMetaQueryHelper.php +++ b/src/DB/ProductMetaQueryHelper.php @@ -59,6 +59,23 @@ public function get_all_values( string $meta_key ): array { return $return; } + /** + * 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. + */ + 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. * diff --git a/src/Jobs/UpdateMerchantProductStatuses.php b/src/Jobs/UpdateMerchantProductStatuses.php index 52d7e8c778..12d9c43ea4 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|void 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 1b00f40921..b3611fa826 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. @@ -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' => null, + 'loading' => false, + 'error' => __( 'The scheduled job has been paused due to a high failure rate.', 'google-listings-and-ads' ), + ]; + } + if ( $job->is_scheduled() || null === $this->mc_statuses ) { return [ 'timestamp' => $this->cache_created_time->getTimestamp(), @@ -227,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. * @@ -234,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(); } @@ -278,7 +300,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 ) ) ) { - // Schedule job to update the statuses. + // 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. If the failure rate is too high, the job will not be scheduled. $job->schedule(); } 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() ); + } } diff --git a/tests/Unit/MerchantCenter/MerchantStatusesTest.php b/tests/Unit/MerchantCenter/MerchantStatusesTest.php index 5b17c0bee4..7dd0319280 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' ); @@ -432,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( 'The scheduled job has been paused due to a high 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( + 'The scheduled job has been paused due to a high 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(); @@ -975,15 +1025,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_all_values' )->with( ProductMetaHandler::KEY_MC_STATUS ); $this->merchant_statuses->clear_product_statuses_cache_and_issues(); }