Skip to content

Commit

Permalink
Merge pull request #2324 from woocommerce/add/display-error-job-mc-st…
Browse files Browse the repository at this point in the history
…atus

Display error message if update_merchant_product_statuses  job throws an error
  • Loading branch information
jorgemd24 authored Mar 25, 2024
2 parents 963a605 + cbdcba1 commit fd022f8
Show file tree
Hide file tree
Showing 11 changed files with 273 additions and 11 deletions.
14 changes: 13 additions & 1 deletion js/src/hooks/useMCProductStatistics.js
Original file line number Diff line number Diff line change
@@ -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';

/**
Expand All @@ -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 ) {
Expand Down Expand Up @@ -57,6 +68,7 @@ const useMCProductStatistics = () => {
data,
invalidateResolution,
hasFinishedResolution,
refreshStats,
...rest,
};
};
Expand Down
41 changes: 41 additions & 0 deletions js/src/hooks/useMCProductStatistics.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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 );
} );
} );
} );
10 changes: 9 additions & 1 deletion js/src/product-feed/product-statistics/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,16 @@ 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';
import AppSpinner from '.~/components/app-spinner';
import './index.scss';

const ProductStatistics = () => {
const { hasFinishedResolution, data } = useMCProductStatistics();
const { hasFinishedResolution, data, refreshStats } =
useMCProductStatistics();

if ( hasFinishedResolution && ! data ) {
return __(

Check warning on line 37 in js/src/product-feed/product-statistics/index.js

View check run for this annotation

Codecov / codecov/patch

js/src/product-feed/product-statistics/index.js#L37

Added line #L37 was not covered by tests
Expand Down Expand Up @@ -135,6 +137,12 @@ const ProductStatistics = () => {
<FeedStatus />
<SyncStatus />
<AccountStatus />
{ hasFinishedResolution && data?.error && (
<SyncProductStatistics
refreshStats={ refreshStats }
error={ data.error }
/>
) }
</CardFooter>
</Card>
);
Expand Down
6 changes: 6 additions & 0 deletions js/src/product-feed/product-statistics/index.scss
Original file line number Diff line number Diff line change
Expand Up @@ -93,5 +93,11 @@
.gridicons-sync {
transform: rotateZ(90deg);
}

.overview-stats-error-button {
height: auto;
padding-top: 0;
padding-left: 0;
}
}
}
29 changes: 29 additions & 0 deletions js/src/product-feed/product-statistics/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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( <ProductStatistics /> );

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();
} );
} );
} );
Original file line number Diff line number Diff line change
@@ -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 (
<Status
title={ __( 'Overview Stats:', 'google-listings-and-ads' ) }
icon={ <ErrorIcon size={ 24 } /> }
label={
<AppButton
aria-label={ error }
onClick={ refreshStats }
className="overview-stats-error-button"
eventName="gla_retry_loading_product_stats"
>
{ __(
'There was an error loading the Overview Stats. Click to retry.',
'google-listings-and-ads'
) }
</AppButton>
}
description={ error }
/>
);
}

export default SyncProductStatistics;
17 changes: 17 additions & 0 deletions src/DB/ProductMetaQueryHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Check warning on line 74 in src/DB/ProductMetaQueryHelper.php

View check run for this annotation

Codecov / codecov/patch

src/DB/ProductMetaQueryHelper.php#L71-L74

Added lines #L71 - L74 were not covered by tests
// phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared
$this->wpdb->query( $this->wpdb->prepare( $query, $meta_key ) );

Check warning on line 76 in src/DB/ProductMetaQueryHelper.php

View check run for this annotation

Codecov / codecov/patch

src/DB/ProductMetaQueryHelper.php#L76

Added line #L76 was not covered by tests
}

/**
* Insert a meta value for multiple posts.
*
Expand Down
13 changes: 13 additions & 0 deletions src/Jobs/UpdateMerchantProductStatuses.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 );

Check warning on line 129 in src/Jobs/UpdateMerchantProductStatuses.php

View check run for this annotation

Codecov / codecov/patch

src/Jobs/UpdateMerchantProductStatuses.php#L129

Added line #L129 was not covered by tests
}

/**
* 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();
}
}
}
28 changes: 26 additions & 2 deletions src/MerchantCenter/MerchantStatuses.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -227,13 +238,24 @@ 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.
*
* @since x.x.x
*/
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();
}

Expand Down Expand Up @@ -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();
}

Expand Down
19 changes: 19 additions & 0 deletions tests/Unit/Jobs/UpdateMerchantProductStatusesTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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() );
}
}
Loading

0 comments on commit fd022f8

Please sign in to comment.