Merge pull request #10095 from google/9933-dismiss-item-fix

Improve `dismissItem` behaviour in SiwG.
google · Jan 29, 2025 · 64baa8f · 64baa8f
2 parents b6163c2 + 4f9637c
commit 64baa8f
Show file tree

Hide file tree

Showing 4 changed files with 20 additions and 10 deletions.
diff --git a/assets/js/components/SettingsNotice/SettingsNotice.js b/assets/js/components/SettingsNotice/SettingsNotice.js
@@ -92,8 +92,11 @@ const SettingsNotice = forwardRef( ( props, ref ) => {
 					<Button
 						tertiary
 						onClick={ () => {
-							dismissItem( dismiss );
-							dismissCallback();
+							if ( typeof dismiss === 'string' ) {
+								dismissItem( dismiss );
+							}
+
+							dismissCallback?.();
 						} }
 					>
 						{ dismissLabel }
@@ -119,6 +122,7 @@ SettingsNotice.propTypes = {
 	LearnMore: PropTypes.elementType,
 	CTA: PropTypes.elementType,
 	OuterCTA: PropTypes.elementType,
+	dismiss: PropTypes.string,
 	dismissLabel: PropTypes.string,
 	dismissCallback: PropTypes.func,
 };

diff --git a/assets/js/googlesitekit/datastore/user/dismissed-items.js b/assets/js/googlesitekit/datastore/user/dismissed-items.js
@@ -115,7 +115,8 @@ const baseActions = {
 	dismissItem: createValidatedAction(
 		( slug, options = {} ) => {
 			const { expiresInSeconds = 0 } = options;
-			invariant( slug, 'A tour slug is required to dismiss a tour.' );
+			invariant( slug, 'A slug is required to dismiss an item.' );
+			invariant( typeof slug === 'string', 'A slug must be a string.' );
 			invariant(
 				Number.isInteger( expiresInSeconds ),
 				'expiresInSeconds must be an integer.'

diff --git a/assets/js/googlesitekit/datastore/user/dismissed-items.test.js b/assets/js/googlesitekit/datastore/user/dismissed-items.test.js
@@ -89,6 +89,16 @@ describe( 'core/user dismissed-items', () => {
 				).toMatchObject( response );
 				expect( console ).toHaveErrored();
 			} );
+
+			it( 'should dispatch an error if the slug is not a string', async () => {
+				try {
+					await registry.dispatch( CORE_USER ).dismissItem( true );
+				} catch ( error ) {
+					expect( error.message ).toMatch(
+						'A slug must be a string.'
+					);
+				}
+			} );
 		} );
 
 		describe( 'removeDismissItems', () => {

diff --git a/assets/js/modules/sign-in-with-google/components/common/AnyoneCanRegisterDisabledNotice.js b/assets/js/modules/sign-in-with-google/components/common/AnyoneCanRegisterDisabledNotice.js
@@ -28,7 +28,7 @@ import { __, _x, sprintf } from '@wordpress/i18n';
 /**
  * Internal dependencies
  */
-import { useSelect, useDispatch } from 'googlesitekit-data';
+import { useSelect } from 'googlesitekit-data';
 import {
 	CORE_USER,
 	PERMISSION_MANAGE_OPTIONS,
@@ -74,8 +74,6 @@ export default function AnyoneCanRegisterDisabledNotice( { className } ) {
 		)
 	);
 
-	const { dismissItem } = useDispatch( CORE_USER );
-
 	if ( isDismissed === true || anyoneCanRegister === true ) {
 		return null;
 	}
@@ -88,10 +86,7 @@ export default function AnyoneCanRegisterDisabledNotice( { className } ) {
 			) }
 			type={ TYPE_INFO }
 			Icon={ InfoIcon }
-			dismiss
-			dismissCallback={ () =>
-				dismissItem( ANYONE_CAN_REGISTER_DISABLED_NOTICE )
-			}
+			dismiss={ ANYONE_CAN_REGISTER_DISABLED_NOTICE }
 			dismissLabel={ __( 'Got it', 'google-site-kit' ) }
 			notice={ createInterpolateElement(
 				sprintf(