Commit 7d8a678a08a for woocommerce
commit 7d8a678a08a688a741e258412168a763b09ef29e
Author: Anand Rajaram <anandrajaram21@gmail.com>
Date: Thu Aug 20 09:15:51 2026 +0530
Invalidate analytics report data after settings changes (#67578)
* Invalidate analytics report data after settings changes
* Invalidate analytics leaderboard data after settings changes
* Fix Analytics refresh and save error handling
* Fix missing settings group error fallback
* Add changelog entry for Analytics data fixes
* Fix paged report chart data memoization
diff --git a/packages/js/data/changelog/fix-analytics-settings-report-cache b/packages/js/data/changelog/fix-analytics-settings-report-cache
new file mode 100644
index 00000000000..d88a264c370
--- /dev/null
+++ b/packages/js/data/changelog/fix-analytics-settings-report-cache
@@ -0,0 +1,4 @@
+Significance: patch
+Type: fix
+
+Return refreshed report data and safely handle errors for unloaded settings groups.
diff --git a/packages/js/data/src/reports/test/utils.ts b/packages/js/data/src/reports/test/utils.ts
new file mode 100644
index 00000000000..95c268aa81a
--- /dev/null
+++ b/packages/js/data/src/reports/test/utils.ts
@@ -0,0 +1,95 @@
+/**
+ * Internal dependencies
+ */
+import { getReportChartData } from '../utils';
+
+describe( 'getReportChartData()', () => {
+ it( 'returns updated data when a report is refetched for the same query', () => {
+ let stats = {
+ data: { totals: { customers: 1 } },
+ totalResults: 1,
+ };
+ const selector = {
+ getReportStats: jest.fn( () => stats ),
+ getReportStatsError: jest.fn( () => undefined ),
+ isResolving: jest.fn( () => false ),
+ };
+ const options = {
+ endpoint: 'customers' as const,
+ dataType: 'primary' as const,
+ query: {},
+ limitBy: [],
+ filters: [],
+ advancedFilters: {},
+ defaultDateRange: 'period=month&compare=previous_year',
+ tableQuery: {},
+ fields: [],
+ selector: selector as never,
+ select: undefined as never,
+ };
+
+ const firstResponse = getReportChartData( options );
+ stats = {
+ data: { totals: { customers: 2 } },
+ totalResults: 1,
+ };
+ const secondResponse = getReportChartData( options );
+
+ expect( firstResponse.data.totals ).toEqual( { customers: 1 } );
+ expect( secondResponse.data.totals ).toEqual( { customers: 2 } );
+ } );
+
+ it( 'reuses chart data when paged report responses are unchanged', () => {
+ const responses = [
+ {
+ data: {
+ totals: { orders_count: 231 },
+ intervals: [ { interval: 'page-1' } ],
+ },
+ totalResults: 231,
+ },
+ { data: { intervals: [ { interval: 'page-2' } ] } },
+ { data: { intervals: [ { interval: 'page-3' } ] } },
+ ];
+ const selector = {
+ getReportStats: jest.fn(
+ ( _endpoint, query ) => responses[ ( query.page || 1 ) - 1 ]
+ ),
+ getReportStatsError: jest.fn( () => undefined ),
+ isResolving: jest.fn( () => false ),
+ };
+ const options = {
+ endpoint: 'orders' as const,
+ dataType: 'primary' as const,
+ query: {},
+ limitBy: [],
+ filters: [],
+ advancedFilters: {},
+ defaultDateRange: 'period=year&compare=previous_year',
+ tableQuery: {},
+ fields: [],
+ selector: selector as never,
+ select: undefined as never,
+ };
+
+ const firstResponse = getReportChartData( options );
+ const secondResponse = getReportChartData( options );
+
+ expect( secondResponse ).toBe( firstResponse );
+ expect( secondResponse.data.intervals ).toEqual( [
+ { interval: 'page-1' },
+ { interval: 'page-2' },
+ { interval: 'page-3' },
+ ] );
+
+ responses[ 1 ] = {
+ data: { intervals: [ { interval: 'updated-page-2' } ] },
+ };
+ const updatedResponse = getReportChartData( options );
+
+ expect( updatedResponse ).not.toBe( firstResponse );
+ expect( updatedResponse.data.intervals[ 1 ] ).toEqual( {
+ interval: 'updated-page-2',
+ } );
+ } );
+} );
diff --git a/packages/js/data/src/reports/utils.ts b/packages/js/data/src/reports/utils.ts
index bac8d482d46..dcf214cac62 100644
--- a/packages/js/data/src/reports/utils.ts
+++ b/packages/js/data/src/reports/utils.ts
@@ -1,8 +1,9 @@
/**
* External dependencies
*/
-import { find, forEach, isNull, get, includes, memoize } from 'lodash';
+import { find, isNull, get, includes } from 'lodash';
import moment from 'moment';
+import createSelector from 'rememo';
import {
appendTimestamp,
getCurrentDates,
@@ -388,18 +389,24 @@ const EMPTY_ARRAY = [] as const;
/**
* Cache helper for returning the full chart dataset after multiple
- * requests. Memoized on the request query (string), only called after
+ * requests. Memoized on the response data references, only called after
* all the requests have resolved successfully.
*/
-const getReportChartDataResponse = memoize(
- ( _requestString, totals, intervals ) => ( {
+const getReportChartDataResponse = createSelector(
+ ( _requestString, totals, intervals, ...pagedIntervals ) => ( {
isEmpty: false,
isError: false,
isRequesting: false,
- data: { totals, intervals },
+ data: {
+ totals,
+ intervals: intervals.concat( ...pagedIntervals ),
+ },
} ),
- ( requestString, totals, intervals ) =>
- [ requestString, totals.length, intervals.length ].join( ':' )
+ ( _requestString, totals, intervals, ...pagedIntervals ) => [
+ totals,
+ intervals,
+ ...pagedIntervals,
+ ]
);
/**
@@ -449,7 +456,7 @@ export function getReportChartData< T extends ReportStatEndpoint >(
}
const totals = ( stats && stats.data && stats.data.totals ) || null;
- let intervals =
+ const intervals =
( stats && stats.data && stats.data.intervals ) || EMPTY_ARRAY;
// If we have more than 100 results for this time period,
@@ -488,15 +495,16 @@ export function getReportChartData< T extends ReportStatEndpoint >(
return reportChartDataResponses.error;
}
- forEach( pagedData, function ( _data ) {
- if (
- _data.data &&
- _data.data.intervals &&
- Array.isArray( _data.data.intervals )
- ) {
- intervals = intervals.concat( _data.data.intervals );
- }
- } );
+ return getReportChartDataResponse(
+ getResourceName( endpoint, requestQuery ),
+ totals,
+ intervals,
+ ...pagedData.map( ( _data ) =>
+ Array.isArray( _data.data?.intervals )
+ ? _data.data.intervals
+ : EMPTY_ARRAY
+ )
+ );
}
return getReportChartDataResponse(
diff --git a/packages/js/data/src/settings/selectors.ts b/packages/js/data/src/settings/selectors.ts
index 6660633db9e..3b291f2bd55 100644
--- a/packages/js/data/src/settings/selectors.ts
+++ b/packages/js/data/src/settings/selectors.ts
@@ -98,13 +98,7 @@ export function getSetting(
export const getLastSettingsErrorForGroup = (
state: SettingsState,
group: string
-) => {
- const settingsIds = state[ group ].data;
- if ( ! Array.isArray( settingsIds ) || settingsIds.length === 0 ) {
- return state[ group ].error;
- }
- return [ ...settingsIds ].pop().error;
-};
+) => ( state[ group ] && state[ group ].error ) || false;
export const getSettingsError = (
state: SettingsState,
diff --git a/packages/js/data/src/settings/test/selectors.ts b/packages/js/data/src/settings/test/selectors.ts
new file mode 100644
index 00000000000..8fbafd8248d
--- /dev/null
+++ b/packages/js/data/src/settings/test/selectors.ts
@@ -0,0 +1,21 @@
+/**
+ * Internal dependencies
+ */
+import { getLastSettingsErrorForGroup } from '../selectors';
+
+describe( 'getLastSettingsErrorForGroup()', () => {
+ it( 'returns false when the group does not exist', () => {
+ expect( getLastSettingsErrorForGroup( {}, 'wc_admin' ) ).toBe( false );
+ } );
+
+ it( 'returns the latest error for the group', () => {
+ const error = new Error( 'Could not save settings.' );
+ const state = {
+ wc_admin: { data: [ 'first', 'last' ], error },
+ };
+
+ expect( getLastSettingsErrorForGroup( state, 'wc_admin' ) ).toBe(
+ error
+ );
+ } );
+} );
diff --git a/plugins/woocommerce/changelog/fix-analytics-settings-report-cache b/plugins/woocommerce/changelog/fix-analytics-settings-report-cache
new file mode 100644
index 00000000000..3a1140dea2e
--- /dev/null
+++ b/plugins/woocommerce/changelog/fix-analytics-settings-report-cache
@@ -0,0 +1,4 @@
+Significance: patch
+Type: fix
+
+Refresh Analytics reports after saving report-affecting settings.
diff --git a/plugins/woocommerce/client/admin/client/analytics/settings/index.js b/plugins/woocommerce/client/admin/client/analytics/settings/index.js
index ec5616f30ff..1a17fff6e37 100644
--- a/plugins/woocommerce/client/admin/client/analytics/settings/index.js
+++ b/plugins/woocommerce/client/admin/client/analytics/settings/index.js
@@ -4,10 +4,9 @@
import { __ } from '@wordpress/i18n';
import { Button } from '@wordpress/components';
import { Fragment, useEffect, useRef, useState } from '@wordpress/element';
-import { compose } from '@wordpress/compose';
-import { withDispatch } from '@wordpress/data';
+import { useDispatch } from '@wordpress/data';
import { SectionHeader, ScrollTo } from '@woocommerce/components';
-import { useSettings } from '@woocommerce/data';
+import { itemsStore, reportsStore, useSettings } from '@woocommerce/data';
import { recordEvent } from '@woocommerce/tracks';
/**
@@ -19,7 +18,13 @@ import Setting from './setting';
import HistoricalData from './historical-data';
import { ImportModeConfirmationModal } from './import-mode-confirmation-modal';
-const Settings = ( { createNotice, query } ) => {
+const Settings = ( { query } ) => {
+ const { createNotice } = useDispatch( 'core/notices' );
+ const {
+ invalidateResolutionForStoreSelector: invalidateReportResolutions,
+ } = useDispatch( reportsStore );
+ const { invalidateResolutionForStoreSelector: invalidateItemResolutions } =
+ useDispatch( itemsStore );
const {
settingsError,
isRequesting,
@@ -57,6 +62,9 @@ const Settings = ( { createNotice, query } ) => {
}
if ( ! isRequesting && hasSaved.current ) {
if ( ! settingsError ) {
+ invalidateReportResolutions( 'getReportItems' );
+ invalidateReportResolutions( 'getReportStats' );
+ invalidateItemResolutions( 'getItems' );
createNotice(
'success',
__(
@@ -75,7 +83,13 @@ const Settings = ( { createNotice, query } ) => {
}
hasSaved.current = false;
}
- }, [ isRequesting, settingsError, createNotice ] );
+ }, [
+ isRequesting,
+ settingsError,
+ createNotice,
+ invalidateReportResolutions,
+ invalidateItemResolutions,
+ ] );
const resetDefaults = () => {
if (
@@ -224,12 +238,4 @@ const Settings = ( { createNotice, query } ) => {
);
};
-export default compose(
- withDispatch( ( dispatch ) => {
- const { createNotice } = dispatch( 'core/notices' );
-
- return {
- createNotice,
- };
- } )
-)( Settings );
+export default Settings;
diff --git a/plugins/woocommerce/client/admin/client/analytics/settings/test/index.test.js b/plugins/woocommerce/client/admin/client/analytics/settings/test/index.test.js
index 4adf541d8fb..0a68ef0f5f8 100644
--- a/plugins/woocommerce/client/admin/client/analytics/settings/test/index.test.js
+++ b/plugins/woocommerce/client/admin/client/analytics/settings/test/index.test.js
@@ -2,7 +2,8 @@
* External dependencies
*/
import { render, screen, waitFor, fireEvent } from '@testing-library/react';
-import { useSettings } from '@woocommerce/data';
+import { useDispatch } from '@wordpress/data';
+import { itemsStore, reportsStore, useSettings } from '@woocommerce/data';
/**
* Internal dependencies
@@ -12,9 +13,15 @@ import { SCHEDULED_IMPORT_SETTING_NAME } from '../config';
// Mock dependencies.
jest.mock( '@woocommerce/data', () => ( {
+ ...jest.requireActual( '@woocommerce/data' ),
useSettings: jest.fn(),
} ) );
+jest.mock( '@wordpress/data', () => ( {
+ ...jest.requireActual( '@wordpress/data' ),
+ useDispatch: jest.fn(),
+} ) );
+
jest.mock( '@woocommerce/tracks', () => ( {
recordEvent: jest.fn(),
} ) );
@@ -51,25 +58,45 @@ jest.mock( '../historical-data', () => ( {
describe( 'Settings - Import Mode Modal', () => {
const mockUpdateSettings = jest.fn();
const mockPersistSettings = jest.fn();
+ const mockUpdateAndPersistSettings = jest.fn();
+ const mockInvalidateReportResolutions = jest.fn();
+ const mockInvalidateItemResolutions = jest.fn();
+ const mockCreateNotice = jest.fn();
+ let settingsState;
beforeEach( () => {
jest.clearAllMocks();
- useSettings.mockReturnValue( {
+ settingsState = {
settingsError: false,
isRequesting: false,
isDirty: false,
persistSettings: mockPersistSettings,
- updateAndPersistSettings: jest.fn(),
+ updateAndPersistSettings: mockUpdateAndPersistSettings,
updateSettings: mockUpdateSettings,
wcAdminSettings: {
[ SCHEDULED_IMPORT_SETTING_NAME ]: 'yes',
},
+ };
+ useSettings.mockImplementation( () => settingsState );
+ useDispatch.mockImplementation( ( store ) => {
+ if ( store === 'core/notices' ) {
+ return { createNotice: mockCreateNotice };
+ }
+ return {
+ invalidateResolutionForStoreSelector:
+ store === reportsStore
+ ? mockInvalidateReportResolutions
+ : mockInvalidateItemResolutions,
+ };
} );
+ window.wpNavMenuUrlUpdate = jest.fn();
} );
afterEach( () => {
delete window.wcAdminFeatures;
+ delete window.wpNavMenuUrlUpdate;
+ jest.restoreAllMocks();
} );
it( 'renders import mode radio control', () => {
render( <Settings createNotice={ jest.fn() } query={ {} } /> );
@@ -191,4 +218,98 @@ describe( 'Settings - Import Mode Modal', () => {
woocommerce_analytics_scheduled_import: 'yes',
} );
} );
+
+ it( 'invalidates report resolutions only after settings are saved', () => {
+ const { rerender } = render(
+ <Settings createNotice={ jest.fn() } query={ {} } />
+ );
+
+ fireEvent.click(
+ screen.getByRole( 'button', { name: /save settings/i } )
+ );
+
+ expect( mockPersistSettings ).toHaveBeenCalled();
+ expect( mockInvalidateReportResolutions ).not.toHaveBeenCalled();
+ expect( mockInvalidateItemResolutions ).not.toHaveBeenCalled();
+
+ settingsState = { ...settingsState, isRequesting: true };
+ rerender( <Settings createNotice={ jest.fn() } query={ {} } /> );
+ expect( mockInvalidateReportResolutions ).not.toHaveBeenCalled();
+ expect( mockInvalidateItemResolutions ).not.toHaveBeenCalled();
+
+ settingsState = { ...settingsState, isRequesting: false };
+ rerender( <Settings createNotice={ jest.fn() } query={ {} } /> );
+
+ expect( useDispatch ).toHaveBeenCalledWith( 'core/notices' );
+ expect( useDispatch ).toHaveBeenCalledWith( reportsStore );
+ expect( useDispatch ).toHaveBeenCalledWith( itemsStore );
+ expect( mockInvalidateReportResolutions ).toHaveBeenNthCalledWith(
+ 1,
+ 'getReportItems'
+ );
+ expect( mockInvalidateReportResolutions ).toHaveBeenNthCalledWith(
+ 2,
+ 'getReportStats'
+ );
+ expect( mockInvalidateItemResolutions ).toHaveBeenCalledWith(
+ 'getItems'
+ );
+ } );
+
+ it( 'does not invalidate report resolutions when saving fails', () => {
+ const { rerender } = render(
+ <Settings createNotice={ jest.fn() } query={ {} } />
+ );
+
+ settingsState = { ...settingsState, isRequesting: true };
+ rerender( <Settings createNotice={ jest.fn() } query={ {} } /> );
+ settingsState = {
+ ...settingsState,
+ isRequesting: false,
+ settingsError: true,
+ };
+ rerender( <Settings createNotice={ jest.fn() } query={ {} } /> );
+
+ expect( mockInvalidateReportResolutions ).not.toHaveBeenCalled();
+ expect( mockInvalidateItemResolutions ).not.toHaveBeenCalled();
+ expect( mockCreateNotice ).toHaveBeenCalledWith(
+ 'error',
+ 'There was an error saving your settings. Please try again.'
+ );
+ } );
+
+ it( 'invalidates report resolutions after resetting defaults', () => {
+ jest.spyOn( window, 'confirm' ).mockReturnValue( true );
+ const { rerender } = render(
+ <Settings createNotice={ jest.fn() } query={ {} } />
+ );
+
+ fireEvent.click(
+ screen.getByRole( 'button', { name: /reset defaults/i } )
+ );
+
+ expect( mockUpdateAndPersistSettings ).toHaveBeenCalledWith(
+ 'wcAdminSettings',
+ {
+ [ SCHEDULED_IMPORT_SETTING_NAME ]: 'yes',
+ }
+ );
+ expect( mockInvalidateReportResolutions ).not.toHaveBeenCalled();
+ expect( mockInvalidateItemResolutions ).not.toHaveBeenCalled();
+
+ settingsState = { ...settingsState, isRequesting: true };
+ rerender( <Settings createNotice={ jest.fn() } query={ {} } /> );
+ settingsState = { ...settingsState, isRequesting: false };
+ rerender( <Settings createNotice={ jest.fn() } query={ {} } /> );
+
+ expect( mockInvalidateReportResolutions ).toHaveBeenCalledWith(
+ 'getReportItems'
+ );
+ expect( mockInvalidateReportResolutions ).toHaveBeenCalledWith(
+ 'getReportStats'
+ );
+ expect( mockInvalidateItemResolutions ).toHaveBeenCalledWith(
+ 'getItems'
+ );
+ } );
} );