Merge branch 'develop' into enhance/#8143-zero-data-audience-tile.

assets/js/googlesitekit/data/create-settings-store.js

@@ -58,16 +58,18 @@ const ROLLBACK_SETTINGS = 'ROLLBACK_SETTINGS';
  * while the fourth defines the names of the sub-settings to support.
  *
  * @since 1.6.0
+ * @since n.e.x.t Added haveSettingsChanged optional paramter.
  * @private
  *
- * @param {string} type                         The data to access. One of 'core' or 'modules'.
- * @param {string} identifier                   The data identifier, eg. a module slug like 'search-console'.
- * @param {string} datapoint                    The endpoint to request data from, e.g. 'settings'.
- * @param {Object} options                      Optional. Options to consider for the store.
- * @param {Array}  [options.ownedSettingsSlugs] Optional. List of "owned settings" for this module, if they exist.
- * @param {number} [options.storeName]          Store name to use. Default is '{type}/{identifier}'.
- * @param {Array}  [options.settingSlugs]       List of the slugs that are part of the settings object handled by the respective API endpoint.
- * @param {Object} [options.initialSettings]    Optional. An initial set of settings as key-value pairs.
+ * @param {string}        type                                  The data to access. One of 'core' or 'modules'.
+ * @param {string}        identifier                            The data identifier, eg. a module slug like 'search-console'.
+ * @param {string}        datapoint                             The endpoint to request data from, e.g. 'settings'.
+ * @param {Object}        options                               Optional. Options to consider for the store.
+ * @param {Array}         [options.ownedSettingsSlugs]          Optional. List of "owned settings" for this module, if they exist.
+ * @param {number}        [options.storeName]                   Store name to use. Default is '{type}/{identifier}'.
+ * @param {Array}         [options.settingSlugs]                List of the slugs that are part of the settings object handled by the respective API endpoint.
+ * @param {Object}        [options.initialSettings]             Optional. An initial set of settings as key-value pairs.
+ * @param {Function|null} [options.validateHaveSettingsChanged] Optional. Custom callback to determine if settings have changed.
  * @return {Object} The settings store object, with additional `STORE_NAME` and
  *                  `initialState` properties.
  */
@@ -80,6 +82,7 @@ export const createSettingsStore = (
 		storeName = undefined,
 		settingSlugs = [],
 		initialSettings = undefined,
+		validateHaveSettingsChanged = makeDefaultHaveSettingsChanged(),
 	} = {}
 ) => {
 	invariant( type, 'type is required.' );
@@ -280,23 +283,17 @@ export const createSettingsStore = (
 		 *
 		 * @since 1.6.0
 		 * @since 1.77.0 Added ability to filter settings using `keys` argument.
+		 * @since n.e.x.t Changed the approach to use validateHaveSettingsChanged callback.
 		 *
 		 * @param {Object}     state Data store's state.
 		 * @param {Array|null} keys  Settings keys to check; if not provided, all settings are checked.
 		 * @return {boolean} True if the settings have changed, false otherwise.
 		 */
-		haveSettingsChanged( state, keys = null ) {
-			const { settings, savedSettings } = state;
-
-			if ( keys ) {
-				return ! isEqual(
-					pick( settings, keys ),
-					pick( savedSettings, keys )
-				);
-			}
-
-			return ! isEqual( settings, savedSettings );
-		},
+		haveSettingsChanged: createRegistrySelector(
+			( select ) =>
+				( state, ...args ) =>
+					validateHaveSettingsChanged( select, state, ...args )
+		),
 
 		/**
 		 * Indicates whether the provided setting has changed from what is saved.
@@ -499,3 +496,25 @@ export function makeDefaultCanSubmitChanges( storeName ) {
 		invariant( haveSettingsChanged(), INVARIANT_SETTINGS_NOT_CHANGED );
 	};
 }
+
+/**
+ * Creates Default haveSettingsChanged.
+ *
+ * @since n.e.x.t
+ *
+ * @return {boolean} True if the settings have changed, false otherwise.
+ */
+export function makeDefaultHaveSettingsChanged() {
+	return ( select, state, keys ) => {
+		const { settings, savedSettings } = state;
+
+		if ( keys ) {
+			return ! isEqual(
+				pick( settings, keys ),
+				pick( savedSettings, keys )
+			);
+		}
+
+		return ! isEqual( settings, savedSettings );
+	};
+}

assets/js/googlesitekit/datastore/site/conversion-tracking.js

@@ -33,6 +33,7 @@ const { createRegistrySelector } = Data;
 const { getRegistry } = Data.commonActions;
 
 const SET_CONVERSION_TRACKING_ENABLED = 'SET_CONVERSION_TRACKING_ENABLED';
+const RESET_CONVERSION_TRACKING_SETTINGS = 'RESET_CONVERSION_TRACKING_SETTINGS';
 
 const settingsReducerCallback = createReducer( ( state, settings ) => {
 	state.conversionTracking.settings = settings;
@@ -104,6 +105,21 @@ const baseActions = {
 			payload: { enabled },
 		};
 	},
+
+	/**
+	 * Returns the current settings back to the current saved values.
+	 *
+	 * @since n.e.x.t
+	 * @private
+	 *
+	 * @return {Object} Redux-style action.
+	 */
+	resetConversionTrackingSettings() {
+		return {
+			payload: {},
+			type: RESET_CONVERSION_TRACKING_SETTINGS,
+		};
+	},
 };
 
 const baseControls = {};
@@ -116,6 +132,11 @@ const baseReducer = createReducer( ( state, { type, payload } ) => {
 			state.conversionTracking.settings.enabled = !! payload.enabled;
 			break;
 
+		case RESET_CONVERSION_TRACKING_SETTINGS:
+			state.conversionTracking.settings =
+				state.conversionTracking.savedSettings;
+			break;
+
 		default:
 			break;
 	}

assets/js/googlesitekit/modules/create-module-store.js

@@ -31,6 +31,7 @@ import {
 	makeDefaultSubmitChanges,
 	makeDefaultCanSubmitChanges,
 	makeDefaultRollbackChanges,
+	makeDefaultHaveSettingsChanged,
 } from '../data/create-settings-store';
 import { createErrorStore } from '../data/create-error-store';
 import { createInfoStore } from './create-info-store';
@@ -72,6 +73,7 @@ export function createModuleStore( slug, args = {} ) {
 		requiresSetup = true,
 		submitChanges,
 		rollbackChanges,
+		validateHaveSettingsChanged = null,
 		validateCanSubmitChanges,
 		validateIsSetupBlocked = undefined,
 	} = args;
@@ -118,6 +120,9 @@ export function createModuleStore( slug, args = {} ) {
 				storeName,
 				settingSlugs,
 				initialSettings,
+				validateHaveSettingsChanged:
+					validateHaveSettingsChanged ||
+					makeDefaultHaveSettingsChanged(),
 			}
 		);
 

assets/js/googlesitekit/widgets/components/WidgetAreaRenderer.js

@@ -43,6 +43,7 @@ import {
 	BREAKPOINT_TABLET,
 	BREAKPOINT_SMALL,
 } from '../../../hooks/useBreakpoint';
+import ErrorHandler from '../../../components/ErrorHandler';
 import InViewProvider from '../../../components/InViewProvider';
 import WidgetRenderer from './WidgetRenderer';
 import WidgetCellWrapper from './WidgetCellWrapper';
@@ -208,18 +209,20 @@ export default function WidgetAreaRenderer( { slug, contextID } ) {
 			key={ `${ widget.slug }-wrapper` }
 			gridColumnWidth={ gridColumnWidths[ i ] }
 		>
-			<WidgetRenderer
-				OverrideComponent={
-					overrideComponents[ i ]
-						? () => {
-								const { Component, metadata } =
-									overrideComponents[ i ];
-								return <Component { ...metadata } />;
-						  }
-						: undefined
-				}
-				slug={ widget.slug }
-			/>
+			<ErrorHandler>
+				<WidgetRenderer
+					OverrideComponent={
+						overrideComponents[ i ]
+							? () => {
+									const { Component, metadata } =
+										overrideComponents[ i ];
+									return <Component { ...metadata } />;
+							  }
+							: undefined
+					}
+					slug={ widget.slug }
+				/>
+			</ErrorHandler>
 		</WidgetCellWrapper>
 	) );
 

assets/js/googlesitekit/widgets/components/WidgetAreaRenderer.test.js

@@ -26,6 +26,7 @@ import { getByText } from '@testing-library/dom';
  */
 import Data from 'googlesitekit-data';
 import WidgetAreaRenderer from './WidgetAreaRenderer';
+import * as tracking from '../../../util/tracking';
 import {
 	CORE_WIDGETS,
 	WIDGET_WIDTHS,
@@ -79,6 +80,10 @@ function WidgetComponentEmpty( { WidgetNull } ) {
 	return <WidgetNull />;
 }
 
+function WidgetComponentErrored() {
+	throw new Error( 'Site Kit error message.' );
+}
+
 const createWidgets = ( registry, areaName, widgets ) => {
 	widgets.forEach( ( { Component, modules, slug, width } ) => {
 		registry.dispatch( CORE_WIDGETS ).registerWidget( slug, {
@@ -811,4 +816,104 @@ describe( 'WidgetAreaRenderer', () => {
 			)
 		).toMatchSnapshot();
 	} );
+
+	describe( 'Error handling', () => {
+		const mockTrackEvent = jest.spyOn( tracking, 'trackEvent' );
+		mockTrackEvent.mockImplementation( () => Promise.resolve() );
+
+		it( 'should display the error using `ErrorHandler` component within a widget', async () => {
+			createWidgets( registry, areaName, [
+				{
+					Component: WidgetComponentErrored,
+					modules: 'search-console',
+					slug: 'one',
+					width: WIDGET_WIDTHS.FULL,
+				},
+			] );
+
+			const { container, waitForRegistry } = render(
+				<WidgetAreaRenderer slug={ areaName } />,
+				{ registry, viewContext: VIEW_CONTEXT_MAIN_DASHBOARD }
+			);
+
+			await waitForRegistry();
+
+			expect( container.firstChild ).toHaveTextContent(
+				'Site Kit encountered an error'
+			);
+			expect( container.firstChild ).toHaveTextContent(
+				'Site Kit error message.'
+			);
+
+			expect( console ).toHaveErrored();
+		} );
+
+		it( 'should display other widgets when there is error in one of the widgets', async () => {
+			createWidgets( registry, areaName, [
+				{
+					Component: WidgetComponent,
+					modules: 'search-console',
+					slug: 'one',
+					width: WIDGET_WIDTHS.FULL,
+				},
+				{
+					Component: WidgetComponentErrored,
+					modules: 'search-console',
+					slug: 'two',
+					width: WIDGET_WIDTHS.FULL,
+				},
+				{
+					Component() {
+						return <div>AdSense is here</div>;
+					},
+					modules: 'adsense',
+					slug: 'three',
+					width: WIDGET_WIDTHS.FULL,
+				},
+			] );
+
+			const { container, waitForRegistry } = render(
+				<WidgetAreaRenderer slug={ areaName } />,
+				{ registry, viewContext: VIEW_CONTEXT_MAIN_DASHBOARD }
+			);
+
+			await waitForRegistry();
+
+			expect( container.firstChild ).toHaveTextContent(
+				'Site Kit encountered an error'
+			);
+
+			expect(
+				container.firstChild.querySelectorAll( '.googlesitekit-widget' )
+			).toHaveLength( 2 );
+
+			expect( container.firstChild ).toHaveTextContent(
+				'AdSense is here'
+			);
+
+			expect( console ).toHaveErrored();
+		} );
+
+		it( 'should track the error event', async () => {
+			createWidgets( registry, areaName, [
+				{
+					Component: WidgetComponentErrored,
+					modules: 'search-console',
+					slug: 'one',
+					width: WIDGET_WIDTHS.FULL,
+				},
+			] );
+
+			const { waitForRegistry } = render(
+				<WidgetAreaRenderer slug={ areaName } />,
+				{ registry, viewContext: VIEW_CONTEXT_MAIN_DASHBOARD }
+			);
+
+			await waitForRegistry();
+
+			expect( mockTrackEvent ).toHaveBeenCalled();
+
+			expect( console ).toHaveErrored();
+		} );
+	} );
 } );

assets/js/modules/ads/datastore/base.js

@@ -21,7 +21,12 @@
  */
 import Modules from 'googlesitekit-modules';
 import { MODULES_ADS } from './constants';
-import { submitChanges, validateCanSubmitChanges } from './settings';
+import {
+	submitChanges,
+	rollbackChanges,
+	validateCanSubmitChanges,
+	validateHaveSettingsChanged,
+} from './settings';
 
 const baseModuleStore = Modules.createModuleStore( 'ads', {
 	ownedSettingsSlugs: [ 'conversionID', 'paxConversionID', 'extCustomerID' ],
@@ -34,7 +39,9 @@ const baseModuleStore = Modules.createModuleStore( 'ads', {
 	],
 	requiresSetup: true,
 	submitChanges,
+	rollbackChanges,
 	validateCanSubmitChanges,
+	validateHaveSettingsChanged,
 } );
 
 export default baseModuleStore;

assets/js/modules/ads/datastore/settings.js

@@ -20,6 +20,7 @@
  * External dependencies
  */
 import invariant from 'invariant';
+import { isEqual, pick } from 'lodash';
 
 /**
  * Internal dependencies
@@ -80,3 +81,28 @@ export function validateCanSubmitChanges( select ) {
 		INVARIANT_INVALID_CONVERSION_ID
 	);
 }
+
+export function rollbackChanges( { select, dispatch } ) {
+	if ( select( MODULES_ADS ).haveSettingsChanged() ) {
+		dispatch( MODULES_ADS ).rollbackSettings();
+		dispatch( CORE_SITE ).resetConversionTrackingSettings();
+	}
+}
+
+export function validateHaveSettingsChanged( select, state, keys ) {
+	const { settings, savedSettings } = state;
+	const haveConversionTrackingSettingsChanged =
+		select( CORE_SITE ).haveConversionTrackingSettingsChanged();
+
+	if ( keys ) {
+		return (
+			! isEqual( pick( settings, keys ), pick( savedSettings, keys ) ) ||
+			haveConversionTrackingSettingsChanged
+		);
+	}
+
+	return (
+		! isEqual( settings, savedSettings ) ||
+		haveConversionTrackingSettingsChanged
+	);
+}