From 3cd8bb8d9fe015ddba4f33f8f94cc5ef2eab4f4e Mon Sep 17 00:00:00 2001 From: Jimmy Madon Date: Sun, 15 Sep 2024 19:37:52 +0530 Subject: [PATCH 01/33] Register RRM setup success subtle notification. --- .../modules/reader-revenue-manager/index.js | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/assets/js/modules/reader-revenue-manager/index.js b/assets/js/modules/reader-revenue-manager/index.js index 371ee67d1a1..b5416be3c56 100644 --- a/assets/js/modules/reader-revenue-manager/index.js +++ b/assets/js/modules/reader-revenue-manager/index.js @@ -34,6 +34,12 @@ import { SetupMain } from './components/setup'; import { SettingsEdit, SettingsView } from './components/settings'; import ReaderRevenueManagerIcon from '../../../svg/graphics/reader-revenue-manager.svg'; import { isURLUsingHTTPS } from './utils/validation'; +import { RRMSetupSuccessSubtleNotification } from './components/dashboard'; +import { NOTIFICATION_AREAS } from '../../googlesitekit/notifications/datastore/constants'; +import { + VIEW_CONTEXT_MAIN_DASHBOARD, + VIEW_CONTEXT_MAIN_DASHBOARD_VIEW_ONLY, +} from '../../googlesitekit/constants'; export { registerStore } from './datastore'; @@ -71,3 +77,18 @@ export const registerModule = ( modules ) => { }, } ); }; + +export const registerNotifications = ( notifications ) => { + notifications.registerNotification( 'setup-success-notification-rrm', { + Component: RRMSetupSuccessSubtleNotification, + priority: 10, + areaSlug: NOTIFICATION_AREAS.BANNERS_BELOW_NAV, + viewContexts: [ + VIEW_CONTEXT_MAIN_DASHBOARD, + VIEW_CONTEXT_MAIN_DASHBOARD_VIEW_ONLY, + ], + checkRequirements: () => { + return false; + }, + } ); +}; From dedf402f64348fe769972c3d734395b7363fa300 Mon Sep 17 00:00:00 2001 From: Jimmy Madon Date: Mon, 16 Sep 2024 18:48:54 +0530 Subject: [PATCH 02/33] Register notifications for RRM. --- assets/js/googlesitekit-modules-reader-revenue-manager.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/assets/js/googlesitekit-modules-reader-revenue-manager.js b/assets/js/googlesitekit-modules-reader-revenue-manager.js index beff1a09e08..ec229043e31 100644 --- a/assets/js/googlesitekit-modules-reader-revenue-manager.js +++ b/assets/js/googlesitekit-modules-reader-revenue-manager.js @@ -21,10 +21,13 @@ */ import Data from 'googlesitekit-data'; import Modules from 'googlesitekit-modules'; +import Notifications from 'googlesitekit-notifications'; import { registerStore, registerModule, + registerNotifications, } from './modules/reader-revenue-manager'; registerStore( Data ); registerModule( Modules ); +registerNotifications( Notifications ); From b79d4d758b4859f06ccefdccf1a7589aca268f57 Mon Sep 17 00:00:00 2001 From: Jimmy Madon Date: Mon, 16 Sep 2024 18:54:56 +0530 Subject: [PATCH 03/33] Add check requirements to RRM Setup Success Notification. --- .../modules/reader-revenue-manager/index.js | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/assets/js/modules/reader-revenue-manager/index.js b/assets/js/modules/reader-revenue-manager/index.js index b5416be3c56..e68e94f7328 100644 --- a/assets/js/modules/reader-revenue-manager/index.js +++ b/assets/js/modules/reader-revenue-manager/index.js @@ -20,6 +20,7 @@ * WordPress dependencies */ import { __ } from '@wordpress/i18n'; +import { getQueryArg } from '@wordpress/url'; /** * Internal dependencies @@ -28,6 +29,7 @@ import { CORE_SITE } from '../../googlesitekit/datastore/site/constants'; import { MODULES_READER_REVENUE_MANAGER, ERROR_CODE_NON_HTTPS_SITE, + READER_REVENUE_MANAGER_MODULE_SLUG, } from './datastore/constants'; import DashboardMainEffectComponent from './components/DashboardMainEffectComponent'; import { SetupMain } from './components/setup'; @@ -87,7 +89,23 @@ export const registerNotifications = ( notifications ) => { VIEW_CONTEXT_MAIN_DASHBOARD, VIEW_CONTEXT_MAIN_DASHBOARD_VIEW_ONLY, ], - checkRequirements: () => { + checkRequirements: async ( { select, resolveSelect } ) => { + const notification = getQueryArg( location.href, 'notification' ); + const slug = getQueryArg( location.href, 'slug' ); + + await resolveSelect( MODULES_READER_REVENUE_MANAGER ).getSettings(); + const publicationOnboardingState = await select( + MODULES_READER_REVENUE_MANAGER + ).getPublicationOnboardingState(); + + if ( + notification === 'authentication_success' && + slug === READER_REVENUE_MANAGER_MODULE_SLUG && + publicationOnboardingState !== undefined + ) { + return true; + } + return false; }, } ); From bb6c8adbeb730f0a76c0b48d4627e5e75245820b Mon Sep 17 00:00:00 2001 From: Jimmy Madon Date: Mon, 16 Sep 2024 19:48:40 +0530 Subject: [PATCH 04/33] Remove rendering logic from RRM Success Notification. --- .../RRMSetupSuccessSubtleNotification.js | 20 ++++--------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.js b/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.js index 1fc3b54cddc..77cb0589505 100644 --- a/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.js +++ b/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.js @@ -52,8 +52,8 @@ const targetOnboardingStates = [ function RRMSetupSuccessSubtleNotification() { const viewContext = useViewContext(); - const [ notification, setNotification ] = useQueryArg( 'notification' ); - const [ slug, setSlug ] = useQueryArg( 'slug' ); + const [ , setNotification ] = useQueryArg( 'notification' ); + const [ , setSlug ] = useQueryArg( 'slug' ); const publicationOnboardingState = useSelect( ( select ) => select( MODULES_READER_REVENUE_MANAGER ).getPublicationOnboardingState() @@ -72,11 +72,6 @@ function RRMSetupSuccessSubtleNotification() { } ) ); - const showNotification = - notification === 'authentication_success' && - slug === READER_REVENUE_MANAGER_MODULE_SLUG && - publicationOnboardingState !== undefined; - const dismissNotice = () => { setNotification( undefined ); setSlug( undefined ); @@ -105,21 +100,14 @@ function RRMSetupSuccessSubtleNotification() { }; useEffect( () => { - if ( - showNotification && - targetOnboardingStates.includes( publicationOnboardingState ) - ) { + if ( targetOnboardingStates.includes( publicationOnboardingState ) ) { trackEvent( `${ viewContext }_rrm-setup-success-notification`, 'view_notification', publicationOnboardingState ); } - }, [ publicationOnboardingState, showNotification, viewContext ] ); - - if ( ! showNotification ) { - return null; - } + }, [ publicationOnboardingState, viewContext ] ); function WithGridWrapped( { children } ) { return ( From 69410c9b02c385d817cfcd7c14ea52ece21d757c Mon Sep 17 00:00:00 2001 From: Jimmy Madon Date: Mon, 16 Sep 2024 20:34:30 +0530 Subject: [PATCH 05/33] Remove direct rendering of RRM Subtle notification. --- assets/js/components/notifications/SubtleNotifications.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/assets/js/components/notifications/SubtleNotifications.js b/assets/js/components/notifications/SubtleNotifications.js index a0fbc381a45..a0660d210dd 100644 --- a/assets/js/components/notifications/SubtleNotifications.js +++ b/assets/js/components/notifications/SubtleNotifications.js @@ -26,7 +26,6 @@ import { Fragment } from '@wordpress/element'; */ import { useFeature } from '../../hooks/useFeature'; import AudienceSegmentationSetupSuccessSubtleNotification from '../../modules/analytics-4/components/audience-segmentation/dashboard/AudienceSegmentationSetupSuccessSubtleNotification'; -import { RRMSetupSuccessSubtleNotification } from '../../modules/reader-revenue-manager/components/dashboard'; import GA4AdSenseLinkedNotification from './GA4AdSenseLinkedNotification'; import useViewContext from '../../hooks/useViewContext'; import Notifications from './Notifications'; @@ -35,7 +34,6 @@ import { NOTIFICATION_AREAS } from '../../googlesitekit/notifications/datastore/ export default function SubtleNotifications() { const viewContext = useViewContext(); const audienceSegmentationEnabled = useFeature( 'audienceSegmentation' ); - const rrmModuleEnabled = useFeature( 'rrmModule' ); // Each notification component rendered here has its own logic to determine // whether it should be displayed; in most cases none of these components @@ -50,7 +48,6 @@ export default function SubtleNotifications() { ) } - { rrmModuleEnabled && } Date: Mon, 16 Sep 2024 20:36:11 +0530 Subject: [PATCH 06/33] Remove redundant non-standard check. --- .../dashboard/RRMSetupSuccessSubtleNotification.js | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.js b/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.js index 77cb0589505..a71abdedd03 100644 --- a/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.js +++ b/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.js @@ -29,13 +29,11 @@ import { useSelect } from 'googlesitekit-data'; import { Cell, Grid, Row } from '../../../../material-components'; import SubtleNotification from '../../../../components/notifications/SubtleNotification'; import useQueryArg from '../../../../hooks/useQueryArg'; -import whenActive from '../../../../util/when-active'; import { trackEvent } from '../../../../util'; import useViewContext from '../../../../hooks/useViewContext'; import { MODULES_READER_REVENUE_MANAGER, PUBLICATION_ONBOARDING_STATES, - READER_REVENUE_MANAGER_MODULE_SLUG, } from '../../datastore/constants'; const { @@ -50,7 +48,7 @@ const targetOnboardingStates = [ ONBOARDING_ACTION_REQUIRED, ]; -function RRMSetupSuccessSubtleNotification() { +export default function RRMSetupSuccessSubtleNotification() { const viewContext = useViewContext(); const [ , setNotification ] = useQueryArg( 'notification' ); const [ , setSlug ] = useQueryArg( 'slug' ); @@ -195,7 +193,3 @@ function RRMSetupSuccessSubtleNotification() { return null; } - -export default whenActive( { moduleName: READER_REVENUE_MANAGER_MODULE_SLUG } )( - RRMSetupSuccessSubtleNotification -); From 4f12f9fde7bcc94a611bcb01f1414a6409fbab8e Mon Sep 17 00:00:00 2001 From: Jimmy Madon Date: Wed, 18 Sep 2024 16:07:38 +0530 Subject: [PATCH 07/33] Add props to the RRM Setup Success Notification. --- .../RRMSetupSuccessSubtleNotification.js | 27 +++++-------------- 1 file changed, 7 insertions(+), 20 deletions(-) diff --git a/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.js b/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.js index a71abdedd03..3e6925bb350 100644 --- a/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.js +++ b/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.js @@ -26,7 +26,6 @@ import { __ } from '@wordpress/i18n'; * Internal dependencies */ import { useSelect } from 'googlesitekit-data'; -import { Cell, Grid, Row } from '../../../../material-components'; import SubtleNotification from '../../../../components/notifications/SubtleNotification'; import useQueryArg from '../../../../hooks/useQueryArg'; import { trackEvent } from '../../../../util'; @@ -48,7 +47,7 @@ const targetOnboardingStates = [ ONBOARDING_ACTION_REQUIRED, ]; -export default function RRMSetupSuccessSubtleNotification() { +export default function RRMSetupSuccessSubtleNotification( { Notification } ) { const viewContext = useViewContext(); const [ , setNotification ] = useQueryArg( 'notification' ); const [ , setSlug ] = useQueryArg( 'slug' ); @@ -107,21 +106,9 @@ export default function RRMSetupSuccessSubtleNotification() { } }, [ publicationOnboardingState, viewContext ] ); - function WithGridWrapped( { children } ) { - return ( - - - - { children } - - - - ); - } - if ( publicationOnboardingState === ONBOARDING_COMPLETE ) { return ( - + - + ); } if ( publicationOnboardingState === PENDING_VERIFICATION ) { return ( - + - + ); } if ( publicationOnboardingState === ONBOARDING_ACTION_REQUIRED ) { return ( - + - + ); } From e90e2284ed755e8a6d344b9ed765a19dbcecf7f1 Mon Sep 17 00:00:00 2001 From: Jimmy Madon Date: Wed, 18 Sep 2024 17:26:10 +0530 Subject: [PATCH 08/33] Create a simple CTALink component for Subtle Notifications. --- .../components/common/CTALinkSubtle.js | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 assets/js/googlesitekit/notifications/components/common/CTALinkSubtle.js diff --git a/assets/js/googlesitekit/notifications/components/common/CTALinkSubtle.js b/assets/js/googlesitekit/notifications/components/common/CTALinkSubtle.js new file mode 100644 index 00000000000..8280e55f104 --- /dev/null +++ b/assets/js/googlesitekit/notifications/components/common/CTALinkSubtle.js @@ -0,0 +1,67 @@ +/** + * Site Kit by Google, Copyright 2024 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * External dependencies + */ +import PropTypes from 'prop-types'; + +/** + * Internal dependencies + */ +import useNotificationEvents from '../../hooks/useNotificationEvents'; +import { Button } from 'googlesitekit-components'; +import ExternalSVG from '../../../../../svg/icons/external.svg'; + +export default function CTALinkSubtle( { + id, + ctaLink, + ctaLabel, + onCTAClick, + isCTALinkExternal = false, +} ) { + const trackEvents = useNotificationEvents( id ); + + const handleCTAClick = async ( event ) => { + await onCTAClick?.( event ); + trackEvents.confirm(); + }; + + return ( + + ); +} + +// eslint-disable-next-line sitekit/acronym-case +CTALinkSubtle.propTypes = { + id: PropTypes.string, + ctaLink: PropTypes.string, + ctaLabel: PropTypes.string, + onCTAClick: PropTypes.func, + isCTALinkExternal: PropTypes.bool, +}; From 5a2af0fb7a29e7151e4fab0e16d9b45cb4e0944f Mon Sep 17 00:00:00 2001 From: Jimmy Madon Date: Wed, 18 Sep 2024 17:26:51 +0530 Subject: [PATCH 09/33] Add a warning variant to the Subtle Notification layout. --- .../components/layout/SubtleNotification.js | 24 +++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/assets/js/googlesitekit/notifications/components/layout/SubtleNotification.js b/assets/js/googlesitekit/notifications/components/layout/SubtleNotification.js index f6c768e4abc..9d21d062871 100644 --- a/assets/js/googlesitekit/notifications/components/layout/SubtleNotification.js +++ b/assets/js/googlesitekit/notifications/components/layout/SubtleNotification.js @@ -20,11 +20,13 @@ * External dependencies */ import PropTypes from 'prop-types'; +import classnames from 'classnames'; /** * Internal dependencies */ import CheckFill from '../../../../../svg/icons/check-fill.svg'; +import WarningSVG from '../../../../../svg/icons/warning.svg'; import { Grid, Cell, Row } from '../../../../material-components'; export default function SubtleNotification( { @@ -32,6 +34,7 @@ export default function SubtleNotification( { description, dismissCTA, additionalCTA, + type = 'success', } ) { return ( @@ -39,18 +42,34 @@ export default function SubtleNotification( {
- + { type === 'success' && ( + + ) } + { type === 'warning' && ( + + ) }
+

{ title }

{ description }

+ { dismissCTA } + { additionalCTA } @@ -63,4 +82,5 @@ SubtleNotification.propTypes = { description: PropTypes.node, dismissCTA: PropTypes.node, additionalCTA: PropTypes.node, + type: PropTypes.string, }; From 81fca07b2b0c03d7d44cc523f59ddc3420a9d26a Mon Sep 17 00:00:00 2001 From: Jimmy Madon Date: Wed, 18 Sep 2024 17:29:03 +0530 Subject: [PATCH 10/33] Refactor RRM success notification to use new Subtle Notification layout and common components. --- .../RRMSetupSuccessSubtleNotification.js | 97 +++++++++++++------ 1 file changed, 70 insertions(+), 27 deletions(-) diff --git a/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.js b/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.js index 3e6925bb350..786d7653925 100644 --- a/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.js +++ b/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.js @@ -26,7 +26,7 @@ import { __ } from '@wordpress/i18n'; * Internal dependencies */ import { useSelect } from 'googlesitekit-data'; -import SubtleNotification from '../../../../components/notifications/SubtleNotification'; +import SubtleNotification from '../../../../googlesitekit/notifications/components/layout/SubtleNotification'; import useQueryArg from '../../../../hooks/useQueryArg'; import { trackEvent } from '../../../../util'; import useViewContext from '../../../../hooks/useViewContext'; @@ -34,6 +34,8 @@ import { MODULES_READER_REVENUE_MANAGER, PUBLICATION_ONBOARDING_STATES, } from '../../datastore/constants'; +import CTALinkSubtle from '../../../../googlesitekit/notifications/components/common/CTALinkSubtle'; +import Dismiss from '../../../../googlesitekit/notifications/components/common/Dismiss'; const { ONBOARDING_COMPLETE, @@ -47,7 +49,10 @@ const targetOnboardingStates = [ ONBOARDING_ACTION_REQUIRED, ]; -export default function RRMSetupSuccessSubtleNotification( { Notification } ) { +export default function RRMSetupSuccessSubtleNotification( { + id, + Notification, +} ) { const viewContext = useViewContext(); const [ , setNotification ] = useQueryArg( 'notification' ); const [ , setSlug ] = useQueryArg( 'slug' ); @@ -118,12 +123,29 @@ export default function RRMSetupSuccessSubtleNotification( { Notification } ) { 'Unlock your full reader opportunity by enabling features like subscriptions, contributions and newsletter sign ups in the Reader Revenue Manager settings.', 'google-site-kit' ) } - onDismiss={ handleDismiss } - dismissLabel={ __( 'Maybe later', 'google-site-kit' ) } - ctaLabel={ __( 'Customize settings', 'google-site-kit' ) } - ctaLink={ serviceURL } - onCTAClick={ onCTAClick } - isCTALinkExternal + dismissCTA={ + + } + additionalCTA={ + + } /> ); @@ -141,15 +163,26 @@ export default function RRMSetupSuccessSubtleNotification( { Notification } ) { 'Your publication is still awaiting review, you can check its status in Reader Revenue Manager.', 'google-site-kit' ) } - onDismiss={ handleDismiss } - dismissLabel={ __( 'Got it', 'google-site-kit' ) } - ctaLabel={ __( - 'Check publication status', - 'google-site-kit' - ) } - ctaLink={ serviceURL } - onCTAClick={ onCTAClick } - isCTALinkExternal + dismissCTA={ + + } + additionalCTA={ + + } /> ); @@ -163,16 +196,26 @@ export default function RRMSetupSuccessSubtleNotification( { Notification } ) { 'Your Reader Revenue Manager account was successfully set up, but your publication still requires further setup in Reader Revenue Manager.', 'google-site-kit' ) } - onDismiss={ handleDismiss } - dismissLabel={ __( 'Got it', 'google-site-kit' ) } - ctaLabel={ __( - 'Complete publication setup', - 'google-site-kit' - ) } - ctaLink={ serviceURL } - onCTAClick={ onCTAClick } - isCTALinkExternal - variant="warning" + dismissCTA={ + + } + additionalCTA={ + + } /> ); From d2262241e4f6c4178677ec23ca135b5b6a0e3cd2 Mon Sep 17 00:00:00 2001 From: Jimmy Madon Date: Wed, 18 Sep 2024 18:20:44 +0530 Subject: [PATCH 11/33] Allow passing GA event tracking label for dismiss event. --- .../googlesitekit/notifications/components/common/Dismiss.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/assets/js/googlesitekit/notifications/components/common/Dismiss.js b/assets/js/googlesitekit/notifications/components/common/Dismiss.js index 127582a1a7c..b6bd0f3f77a 100644 --- a/assets/js/googlesitekit/notifications/components/common/Dismiss.js +++ b/assets/js/googlesitekit/notifications/components/common/Dismiss.js @@ -34,6 +34,7 @@ export default function Dismiss( { dismissExpires = 0, disabled, onDismiss = () => {}, + gaDismissEventLabel, } ) { const trackEvents = useNotificationEvents( id ); @@ -41,7 +42,7 @@ export default function Dismiss( { const handleDismiss = async ( event ) => { await onDismiss?.( event ); - trackEvents.dismiss(); + trackEvents.dismiss( gaDismissEventLabel ); dismissNotification( id, { expiresInSeconds: dismissExpires } ); }; From 7349a9f2d19218c736526bcc245add9680c52057 Mon Sep 17 00:00:00 2001 From: Jimmy Madon Date: Wed, 18 Sep 2024 18:21:15 +0530 Subject: [PATCH 12/33] Pass GA event tracking label for dismiss event. --- .../RRMSetupSuccessSubtleNotification.js | 21 ++++++------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.js b/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.js index 786d7653925..ed6024b7a49 100644 --- a/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.js +++ b/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.js @@ -79,18 +79,6 @@ export default function RRMSetupSuccessSubtleNotification( { setSlug( undefined ); }; - const handleDismiss = () => { - if ( targetOnboardingStates.includes( publicationOnboardingState ) ) { - trackEvent( - `${ viewContext }_rrm-setup-success-notification`, - 'dismiss_notification', - publicationOnboardingState - ); - } - - dismissNotice(); - }; - const onCTAClick = () => { if ( targetOnboardingStates.includes( publicationOnboardingState ) ) { trackEvent( @@ -131,7 +119,8 @@ export default function RRMSetupSuccessSubtleNotification( { 'Maybe later', 'google-site-kit' ) } - onDismiss={ handleDismiss } + onDismiss={ dismissNotice } + gaDismissEventLabel={ publicationOnboardingState } /> } additionalCTA={ @@ -168,7 +157,8 @@ export default function RRMSetupSuccessSubtleNotification( { id={ id } primary={ false } dismissLabel={ __( 'Got it', 'google-site-kit' ) } - onDismiss={ handleDismiss } + onDismiss={ dismissNotice } + gaDismissEventLabel={ publicationOnboardingState } /> } additionalCTA={ @@ -201,7 +191,8 @@ export default function RRMSetupSuccessSubtleNotification( { id={ id } primary={ false } dismissLabel={ __( 'Got it', 'google-site-kit' ) } - onDismiss={ handleDismiss } + onDismiss={ dismissNotice } + gaDismissEventLabel={ publicationOnboardingState } /> } additionalCTA={ From cfa1191e77a6a733019df66c0001cff4c6135dde Mon Sep 17 00:00:00 2001 From: Jimmy Madon Date: Wed, 18 Sep 2024 19:09:58 +0530 Subject: [PATCH 13/33] Allow passing GA event tracking label for confirm CTA click event. --- .../notifications/components/common/CTALinkSubtle.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/assets/js/googlesitekit/notifications/components/common/CTALinkSubtle.js b/assets/js/googlesitekit/notifications/components/common/CTALinkSubtle.js index 8280e55f104..999b5275739 100644 --- a/assets/js/googlesitekit/notifications/components/common/CTALinkSubtle.js +++ b/assets/js/googlesitekit/notifications/components/common/CTALinkSubtle.js @@ -30,14 +30,13 @@ export default function CTALinkSubtle( { id, ctaLink, ctaLabel, - onCTAClick, isCTALinkExternal = false, + gaConfirmEventLabel, } ) { const trackEvents = useNotificationEvents( id ); - const handleCTAClick = async ( event ) => { - await onCTAClick?.( event ); - trackEvents.confirm(); + const handleCTAClick = () => { + trackEvents.confirm( gaConfirmEventLabel ); }; return ( @@ -62,6 +61,6 @@ CTALinkSubtle.propTypes = { id: PropTypes.string, ctaLink: PropTypes.string, ctaLabel: PropTypes.string, - onCTAClick: PropTypes.func, isCTALinkExternal: PropTypes.bool, + gaConfirmEventLabel: PropTypes.string, }; From 8b11d79f97ec92532efd1ee36aca86915149c30f Mon Sep 17 00:00:00 2001 From: Jimmy Madon Date: Wed, 18 Sep 2024 19:12:10 +0530 Subject: [PATCH 14/33] Pass GA event tracking label for confirm CTA click event. --- .../RRMSetupSuccessSubtleNotification.js | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.js b/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.js index ed6024b7a49..822337f53d9 100644 --- a/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.js +++ b/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.js @@ -79,16 +79,6 @@ export default function RRMSetupSuccessSubtleNotification( { setSlug( undefined ); }; - const onCTAClick = () => { - if ( targetOnboardingStates.includes( publicationOnboardingState ) ) { - trackEvent( - `${ viewContext }_rrm-setup-success-notification`, - 'confirm_notification', - publicationOnboardingState - ); - } - }; - useEffect( () => { if ( targetOnboardingStates.includes( publicationOnboardingState ) ) { trackEvent( @@ -131,8 +121,8 @@ export default function RRMSetupSuccessSubtleNotification( { 'google-site-kit' ) } ctaLink={ serviceURL } - onCTAClick={ onCTAClick } isCTALinkExternal + gaConfirmEventLabel={ publicationOnboardingState } /> } /> @@ -169,8 +159,8 @@ export default function RRMSetupSuccessSubtleNotification( { 'google-site-kit' ) } ctaLink={ serviceURL } - onCTAClick={ onCTAClick } isCTALinkExternal + gaConfirmEventLabel={ publicationOnboardingState } /> } /> @@ -203,8 +193,8 @@ export default function RRMSetupSuccessSubtleNotification( { 'google-site-kit' ) } ctaLink={ serviceURL } - onCTAClick={ onCTAClick } isCTALinkExternal + gaConfirmEventLabel={ publicationOnboardingState } /> } /> From 0ee7585262e1fd2fef8501e8d4b70ea9596fb064 Mon Sep 17 00:00:00 2001 From: Jimmy Madon Date: Wed, 18 Sep 2024 19:14:27 +0530 Subject: [PATCH 15/33] Allow passing GA event tracking label for view notification event. --- .../notifications/components/Notification/index.js | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/assets/js/googlesitekit/notifications/components/Notification/index.js b/assets/js/googlesitekit/notifications/components/Notification/index.js index da01cb29951..727e377e739 100644 --- a/assets/js/googlesitekit/notifications/components/Notification/index.js +++ b/assets/js/googlesitekit/notifications/components/Notification/index.js @@ -26,7 +26,12 @@ import ViewedStateObserver from './ViewedStateObserver'; import { useHasBeenViewed } from '../../hooks/useHasBeenViewed'; import useNotificationEvents from '../../hooks/useNotificationEvents'; -export default function Notification( { id, className, children } ) { +export default function Notification( { + id, + className, + gaViewEventLabel, + children, +} ) { const ref = useRef(); const viewed = useHasBeenViewed( id ); const trackEvents = useNotificationEvents( id ); @@ -36,10 +41,10 @@ export default function Notification( { id, className, children } ) { // Track view once. useEffect( () => { if ( ! isViewedOnce && viewed ) { - trackEvents.view(); + trackEvents.view( gaViewEventLabel ); setIsViewedOnce( true ); } - }, [ viewed, trackEvents, isViewedOnce ] ); + }, [ viewed, trackEvents, isViewedOnce, gaViewEventLabel ] ); return (
From 8d1bc10bd30b87455a37fb85826407436411bfb1 Mon Sep 17 00:00:00 2001 From: Jimmy Madon Date: Wed, 18 Sep 2024 19:18:19 +0530 Subject: [PATCH 16/33] Pass GA event tracking label for view notification event. --- .../RRMSetupSuccessSubtleNotification.js | 26 +++---------------- 1 file changed, 3 insertions(+), 23 deletions(-) diff --git a/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.js b/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.js index 822337f53d9..05ce39a2baa 100644 --- a/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.js +++ b/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.js @@ -19,7 +19,6 @@ /** * WordPress dependencies */ -import { useEffect } from '@wordpress/element'; import { __ } from '@wordpress/i18n'; /** @@ -28,8 +27,6 @@ import { __ } from '@wordpress/i18n'; import { useSelect } from 'googlesitekit-data'; import SubtleNotification from '../../../../googlesitekit/notifications/components/layout/SubtleNotification'; import useQueryArg from '../../../../hooks/useQueryArg'; -import { trackEvent } from '../../../../util'; -import useViewContext from '../../../../hooks/useViewContext'; import { MODULES_READER_REVENUE_MANAGER, PUBLICATION_ONBOARDING_STATES, @@ -43,17 +40,10 @@ const { ONBOARDING_ACTION_REQUIRED, } = PUBLICATION_ONBOARDING_STATES; -const targetOnboardingStates = [ - ONBOARDING_COMPLETE, - PENDING_VERIFICATION, - ONBOARDING_ACTION_REQUIRED, -]; - export default function RRMSetupSuccessSubtleNotification( { id, Notification, } ) { - const viewContext = useViewContext(); const [ , setNotification ] = useQueryArg( 'notification' ); const [ , setSlug ] = useQueryArg( 'slug' ); @@ -79,19 +69,9 @@ export default function RRMSetupSuccessSubtleNotification( { setSlug( undefined ); }; - useEffect( () => { - if ( targetOnboardingStates.includes( publicationOnboardingState ) ) { - trackEvent( - `${ viewContext }_rrm-setup-success-notification`, - 'view_notification', - publicationOnboardingState - ); - } - }, [ publicationOnboardingState, viewContext ] ); - if ( publicationOnboardingState === ONBOARDING_COMPLETE ) { return ( - + + + Date: Wed, 18 Sep 2024 19:20:27 +0530 Subject: [PATCH 17/33] Add missing PropTypes for the Notification component. --- .../notifications/components/Notification/index.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/assets/js/googlesitekit/notifications/components/Notification/index.js b/assets/js/googlesitekit/notifications/components/Notification/index.js index 727e377e739..72a38cb24b3 100644 --- a/assets/js/googlesitekit/notifications/components/Notification/index.js +++ b/assets/js/googlesitekit/notifications/components/Notification/index.js @@ -14,6 +14,11 @@ * limitations under the License. */ +/** + * External dependencies + */ +import PropTypes from 'prop-types'; + /** * WordPress dependencies */ @@ -61,3 +66,10 @@ export default function Notification( {
); } + +Notification.propTypes = { + id: PropTypes.string, + className: PropTypes.string, + gaViewEventLabel: PropTypes.string, + children: PropTypes.node, +}; From edfadd988374f2e63eb69ad9d91cddcf876d49c3 Mon Sep 17 00:00:00 2001 From: Jimmy Madon Date: Wed, 18 Sep 2024 19:38:54 +0530 Subject: [PATCH 18/33] Add missing PropTypes for common Dismiss component. --- .../notifications/components/common/Dismiss.js | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/assets/js/googlesitekit/notifications/components/common/Dismiss.js b/assets/js/googlesitekit/notifications/components/common/Dismiss.js index b6bd0f3f77a..c22d6a333b5 100644 --- a/assets/js/googlesitekit/notifications/components/common/Dismiss.js +++ b/assets/js/googlesitekit/notifications/components/common/Dismiss.js @@ -14,6 +14,11 @@ * limitations under the License. */ +/** + * External dependencies + */ +import PropTypes from 'prop-types'; + /** * WordPress dependencies */ @@ -56,3 +61,13 @@ export default function Dismiss( { ); } + +Dismiss.propTypes = { + id: PropTypes.string, + primary: PropTypes.bool, + dismissLabel: PropTypes.string, + dismissExpires: PropTypes.number, + disabled: PropTypes.bool, + onDismiss: PropTypes.func, + gaDismissEventLabel: PropTypes.string, +}; From efedf0dfb8f7b6b1d9ecf9a43b2d9018f3d1f427 Mon Sep 17 00:00:00 2001 From: Jimmy Madon Date: Sat, 21 Sep 2024 13:48:38 +0530 Subject: [PATCH 19/33] Add support to pass custom CTA click callback. --- .../components/common/CTALinkSubtle.js | 21 +++++++++++++++++-- .../RRMSetupSuccessSubtleNotification.js | 6 +----- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/assets/js/googlesitekit/notifications/components/common/CTALinkSubtle.js b/assets/js/googlesitekit/notifications/components/common/CTALinkSubtle.js index 999b5275739..d52cb24956d 100644 --- a/assets/js/googlesitekit/notifications/components/common/CTALinkSubtle.js +++ b/assets/js/googlesitekit/notifications/components/common/CTALinkSubtle.js @@ -22,6 +22,8 @@ import PropTypes from 'prop-types'; /** * Internal dependencies */ +import { useDispatch } from 'googlesitekit-data'; +import { CORE_LOCATION } from '../../../datastore/location/constants'; import useNotificationEvents from '../../hooks/useNotificationEvents'; import { Button } from 'googlesitekit-components'; import ExternalSVG from '../../../../../svg/icons/external.svg'; @@ -30,13 +32,27 @@ export default function CTALinkSubtle( { id, ctaLink, ctaLabel, + onCTAClick, isCTALinkExternal = false, gaConfirmEventLabel, } ) { const trackEvents = useNotificationEvents( id ); + const { navigateTo } = useDispatch( CORE_LOCATION ); - const handleCTAClick = () => { - trackEvents.confirm( gaConfirmEventLabel ); + const handleCTAClick = async ( event ) => { + if ( ! event.defaultPrevented ) { + event.preventDefault(); + } + + await onCTAClick?.( event ); + + await trackEvents.confirm( gaConfirmEventLabel ); + + if ( isCTALinkExternal ) { + global.open( ctaLink, '_blank' ); + } else { + navigateTo( ctaLink ); + } }; return ( @@ -61,6 +77,7 @@ CTALinkSubtle.propTypes = { id: PropTypes.string, ctaLink: PropTypes.string, ctaLabel: PropTypes.string, + onCTAClick: PropTypes.func, isCTALinkExternal: PropTypes.bool, gaConfirmEventLabel: PropTypes.string, }; diff --git a/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.js b/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.js index af498c93e9b..fa7f1d63ff1 100644 --- a/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.js +++ b/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.js @@ -93,9 +93,7 @@ export default function RRMSetupSuccessSubtleNotification( { setSlug( undefined ); }; - const onCTAClick = ( event ) => { - event.preventDefault(); - + const onCTAClick = () => { // Set publication data to be reset when user re-focuses window. if ( actionableOnboardingStates.includes( publicationOnboardingState ) @@ -104,8 +102,6 @@ export default function RRMSetupSuccessSubtleNotification( { [ SYNC_PUBLICATION ]: true, } ); } - - global.open( serviceURL, '_blank' ); }; const syncPublication = useCallback( () => { From bbd619be57e820ca2d396cb3828942069707f820 Mon Sep 17 00:00:00 2001 From: Jimmy Madon Date: Sat, 21 Sep 2024 13:59:43 +0530 Subject: [PATCH 20/33] Fix merge conflict resolution error. --- .../components/dashboard/RRMSetupSuccessSubtleNotification.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.js b/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.js index fa7f1d63ff1..085c47c1b52 100644 --- a/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.js +++ b/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.js @@ -26,7 +26,6 @@ import { __ } from '@wordpress/i18n'; * Internal dependencies */ import { useSelect, useDispatch } from 'googlesitekit-data'; -import SubtleNotification from '../../../../components/notifications/SubtleNotification'; import useQueryArg from '../../../../hooks/useQueryArg'; import { useRefocus } from '../../../../hooks/useRefocus'; import { CORE_FORMS } from '../../../../googlesitekit/datastore/forms/constants'; @@ -36,6 +35,7 @@ import { READER_REVENUE_MANAGER_NOTICES_FORM, SYNC_PUBLICATION, } from '../../datastore/constants'; +import SubtleNotification from '../../../../googlesitekit/notifications/components/layout/SubtleNotification'; import CTALinkSubtle from '../../../../googlesitekit/notifications/components/common/CTALinkSubtle'; import Dismiss from '../../../../googlesitekit/notifications/components/common/Dismiss'; From a19e6fd17ac277603a343cb9d52347ceb0b43fc6 Mon Sep 17 00:00:00 2001 From: Jimmy Madon Date: Sat, 21 Sep 2024 13:59:57 +0530 Subject: [PATCH 21/33] Fix storybook stories. --- .../dashboard/RRMSetupSuccessSubtleNotification.stories.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.stories.js b/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.stories.js index 5b87d479849..7d859a64fc2 100644 --- a/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.stories.js +++ b/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.stories.js @@ -31,9 +31,14 @@ import { READER_REVENUE_MANAGER_MODULE_SLUG, } from '../../datastore/constants'; import WithRegistrySetup from '../../../../../../tests/js/WithRegistrySetup'; +import { withNotificationComponentProps } from '../../../../googlesitekit/notifications/util/component-props'; + +const NotificationWithComponentProps = withNotificationComponentProps( + 'setup-success-notification-rrm' +)( RRMSetupSuccessSubtleNotification ); function Template() { - return ; + return ; } export const OnboardingComplete = Template.bind( {} ); From 523770f45243fa46545604e8c2cb41a0d53075d4 Mon Sep 17 00:00:00 2001 From: Jimmy Madon Date: Sun, 22 Sep 2024 00:23:40 +0530 Subject: [PATCH 22/33] Render RRM Setup Success notification in tests with new props. --- .../RRMSetupSuccessSubtleNotification.test.js | 34 ++++++++++--------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.test.js b/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.test.js index 0282bdc4d95..0d1cb6e2beb 100644 --- a/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.test.js +++ b/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.test.js @@ -37,6 +37,7 @@ import { } from '../../datastore/constants'; import { VIEW_CONTEXT_MAIN_DASHBOARD } from '../../../../googlesitekit/constants'; import useQueryArg from '../../../../hooks/useQueryArg'; +import { withNotificationComponentProps } from '../../../../googlesitekit/notifications/util/component-props'; jest.mock( '../../../../hooks/useQueryArg' ); @@ -50,6 +51,10 @@ const { UNSPECIFIED, } = PUBLICATION_ONBOARDING_STATES; +const NotificationWithComponentProps = withNotificationComponentProps( + 'setup-success-notification-rrm' +)( RRMSetupSuccessSubtleNotification ); + describe( 'RRMSetupSuccessSubtleNotification', () => { let registry; @@ -120,13 +125,10 @@ describe( 'RRMSetupSuccessSubtleNotification', () => { .dispatch( MODULES_READER_REVENUE_MANAGER ) .setPublicationOnboardingState( onboardingState ); - const { container } = render( - , - { - registry, - viewContext: VIEW_CONTEXT_MAIN_DASHBOARD, - } - ); + const { container } = render( , { + registry, + viewContext: VIEW_CONTEXT_MAIN_DASHBOARD, + } ); expect( container ).toBeEmptyDOMElement(); @@ -146,7 +148,7 @@ describe( 'RRMSetupSuccessSubtleNotification', () => { .setPublicationID( 'ABCDEFGH' ); const { container, getByText } = render( - , + , { registry, viewContext: VIEW_CONTEXT_MAIN_DASHBOARD, @@ -164,12 +166,12 @@ describe( 'RRMSetupSuccessSubtleNotification', () => { const dismissElement = getByText( dismissText ); expect( dismissElement ).toBeInTheDocument(); - expect( mockTrackEvent ).toHaveBeenNthCalledWith( - 1, - `${ VIEW_CONTEXT_MAIN_DASHBOARD }_setup-success-notification-rrm`, - 'view_notification', - onboardingState - ); + // expect( mockTrackEvent ).toHaveBeenNthCalledWith( + // 1, + // `${ VIEW_CONTEXT_MAIN_DASHBOARD }_setup-success-notification-rrm`, + // 'view_notification', + // onboardingState + // ); act( () => { fireEvent.click( ctaElement ); @@ -196,7 +198,7 @@ describe( 'RRMSetupSuccessSubtleNotification', () => { .setPublicationID( 'ABCDEFGH' ); const { container, getByText } = render( - , + , { registry, viewContext: VIEW_CONTEXT_MAIN_DASHBOARD, @@ -258,7 +260,7 @@ describe( 'RRMSetupSuccessSubtleNotification', () => { } ); const { getByText, queryByText } = render( - , + , { registry, viewContext: VIEW_CONTEXT_MAIN_DASHBOARD, From 5fe4d306c5114fb9d26a66d06ccc7c85b9454929 Mon Sep 17 00:00:00 2001 From: Jimmy Madon Date: Mon, 23 Sep 2024 15:09:40 +0530 Subject: [PATCH 23/33] Add notification component ID prop to the utility method instead of passing separately. --- assets/js/components/notifications/Notifications.js | 2 +- assets/js/googlesitekit/notifications/util/component-props.js | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/assets/js/components/notifications/Notifications.js b/assets/js/components/notifications/Notifications.js index c8ef69bc2b8..8563e14563b 100644 --- a/assets/js/components/notifications/Notifications.js +++ b/assets/js/components/notifications/Notifications.js @@ -43,7 +43,7 @@ export default function Notifications( { areaSlug } ) { } const { id, Component: ActiveNotification } = queuedNotifications[ 0 ]; - const props = { id, ...getNotificationComponentProps( id ) }; + const props = { ...getNotificationComponentProps( id ) }; return ; } diff --git a/assets/js/googlesitekit/notifications/util/component-props.js b/assets/js/googlesitekit/notifications/util/component-props.js index 60063e72aeb..88bd7a70021 100644 --- a/assets/js/googlesitekit/notifications/util/component-props.js +++ b/assets/js/googlesitekit/notifications/util/component-props.js @@ -34,6 +34,7 @@ import Notification from '../components/Notification'; */ export const getNotificationComponentProps = memize( ( id ) => { return { + id, Notification: withNotificationID( id )( Notification ), }; } ); From d0c48cf9ca031834e325f0d746dc7fed7160aa3b Mon Sep 17 00:00:00 2001 From: Jimmy Madon Date: Mon, 23 Sep 2024 15:25:47 +0530 Subject: [PATCH 24/33] Update the dismiss button for onboarding complete version of the RRM Setup Success Notification. --- .../dashboard/RRMSetupSuccessSubtleNotification.js | 5 +---- .../dashboard/RRMSetupSuccessSubtleNotification.test.js | 2 +- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.js b/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.js index 085c47c1b52..b69ea63d011 100644 --- a/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.js +++ b/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.js @@ -131,10 +131,7 @@ export default function RRMSetupSuccessSubtleNotification( { diff --git a/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.test.js b/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.test.js index 0d1cb6e2beb..b49f98ae70a 100644 --- a/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.test.js +++ b/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.test.js @@ -64,7 +64,7 @@ describe( 'RRMSetupSuccessSubtleNotification', () => { [ ONBOARDING_COMPLETE, 'Customize settings', - 'Maybe later', + 'Got it', 'Your Reader Revenue Manager account was successfully set up!', ], [ From 478701c38b80fd8d24d064d65ed129f883ee9dcb Mon Sep 17 00:00:00 2001 From: Jimmy Madon Date: Mon, 23 Sep 2024 23:02:40 +0530 Subject: [PATCH 25/33] Remove direct call to the audience segmentation setup success notification. --- assets/js/components/notifications/SubtleNotifications.js | 6 ------ 1 file changed, 6 deletions(-) diff --git a/assets/js/components/notifications/SubtleNotifications.js b/assets/js/components/notifications/SubtleNotifications.js index a0660d210dd..a1fdbd3454c 100644 --- a/assets/js/components/notifications/SubtleNotifications.js +++ b/assets/js/components/notifications/SubtleNotifications.js @@ -24,8 +24,6 @@ import { Fragment } from '@wordpress/element'; /** * Internal dependencies */ -import { useFeature } from '../../hooks/useFeature'; -import AudienceSegmentationSetupSuccessSubtleNotification from '../../modules/analytics-4/components/audience-segmentation/dashboard/AudienceSegmentationSetupSuccessSubtleNotification'; import GA4AdSenseLinkedNotification from './GA4AdSenseLinkedNotification'; import useViewContext from '../../hooks/useViewContext'; import Notifications from './Notifications'; @@ -33,7 +31,6 @@ import { NOTIFICATION_AREAS } from '../../googlesitekit/notifications/datastore/ export default function SubtleNotifications() { const viewContext = useViewContext(); - const audienceSegmentationEnabled = useFeature( 'audienceSegmentation' ); // Each notification component rendered here has its own logic to determine // whether it should be displayed; in most cases none of these components @@ -44,9 +41,6 @@ export default function SubtleNotifications() { // Because these notifications are subtle and small, this is acceptable UX. return ( - { audienceSegmentationEnabled && ( - - ) } Date: Mon, 23 Sep 2024 23:06:48 +0530 Subject: [PATCH 26/33] Revert "Remove direct call to the audience segmentation setup success notification." This reverts commit 478701c38b80fd8d24d064d65ed129f883ee9dcb. --- assets/js/components/notifications/SubtleNotifications.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/assets/js/components/notifications/SubtleNotifications.js b/assets/js/components/notifications/SubtleNotifications.js index a1fdbd3454c..a0660d210dd 100644 --- a/assets/js/components/notifications/SubtleNotifications.js +++ b/assets/js/components/notifications/SubtleNotifications.js @@ -24,6 +24,8 @@ import { Fragment } from '@wordpress/element'; /** * Internal dependencies */ +import { useFeature } from '../../hooks/useFeature'; +import AudienceSegmentationSetupSuccessSubtleNotification from '../../modules/analytics-4/components/audience-segmentation/dashboard/AudienceSegmentationSetupSuccessSubtleNotification'; import GA4AdSenseLinkedNotification from './GA4AdSenseLinkedNotification'; import useViewContext from '../../hooks/useViewContext'; import Notifications from './Notifications'; @@ -31,6 +33,7 @@ import { NOTIFICATION_AREAS } from '../../googlesitekit/notifications/datastore/ export default function SubtleNotifications() { const viewContext = useViewContext(); + const audienceSegmentationEnabled = useFeature( 'audienceSegmentation' ); // Each notification component rendered here has its own logic to determine // whether it should be displayed; in most cases none of these components @@ -41,6 +44,9 @@ export default function SubtleNotifications() { // Because these notifications are subtle and small, this is acceptable UX. return ( + { audienceSegmentationEnabled && ( + + ) } Date: Wed, 25 Sep 2024 03:32:24 +0600 Subject: [PATCH 27/33] Fix failing tests. --- .../RRMSetupSuccessSubtleNotification.test.js | 63 +++++++------------ 1 file changed, 21 insertions(+), 42 deletions(-) diff --git a/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.test.js b/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.test.js index b49f98ae70a..bcbeb3b3e5c 100644 --- a/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.test.js +++ b/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.test.js @@ -29,7 +29,7 @@ import { } from '../../../../../../tests/js/test-utils'; import RRMSetupSuccessSubtleNotification from './RRMSetupSuccessSubtleNotification'; import * as fixtures from '../../datastore/__fixtures__'; -import * as tracking from '../../../../util/tracking'; +import { CORE_NOTIFICATIONS } from '../../../../googlesitekit/notifications/datastore/constants'; import { MODULES_READER_REVENUE_MANAGER, PUBLICATION_ONBOARDING_STATES, @@ -41,9 +41,6 @@ import { withNotificationComponentProps } from '../../../../googlesitekit/notifi jest.mock( '../../../../hooks/useQueryArg' ); -const mockTrackEvent = jest.spyOn( tracking, 'trackEvent' ); -mockTrackEvent.mockImplementation( () => Promise.resolve() ); - const { ONBOARDING_COMPLETE, PENDING_VERIFICATION, @@ -51,9 +48,11 @@ const { UNSPECIFIED, } = PUBLICATION_ONBOARDING_STATES; -const NotificationWithComponentProps = withNotificationComponentProps( - 'setup-success-notification-rrm' -)( RRMSetupSuccessSubtleNotification ); +const id = 'setup-success-notification-rrm'; + +const NotificationWithComponentProps = withNotificationComponentProps( id )( + RRMSetupSuccessSubtleNotification +); describe( 'RRMSetupSuccessSubtleNotification', () => { let registry; @@ -87,9 +86,11 @@ describe( 'RRMSetupSuccessSubtleNotification', () => { const settingsEndpoint = new RegExp( '^/google-site-kit/v1/modules/reader-revenue-manager/data/settings' ); + const dismissItemEndpoint = new RegExp( + '^/google-site-kit/v1/core/user/data/dismiss-item' + ); beforeEach( () => { - mockTrackEvent.mockClear(); registry = createTestRegistry(); provideModules( registry, [ @@ -119,7 +120,7 @@ describe( 'RRMSetupSuccessSubtleNotification', () => { } ); it.each( invalidPublicationOnboardingStates )( - 'should not render a notification and not trigger view_notification event when the publication onboarding state is %s', + 'should not render a notification when the publication onboarding state is %s', ( onboardingState ) => { registry .dispatch( MODULES_READER_REVENUE_MANAGER ) @@ -131,13 +132,11 @@ describe( 'RRMSetupSuccessSubtleNotification', () => { } ); expect( container ).toBeEmptyDOMElement(); - - expect( mockTrackEvent ).not.toHaveBeenCalled(); } ); it.each( publicationStatesData )( - 'should render a notification and trigger confirm_notification event when the publication onboarding state is %s', + 'should render a notification when the publication onboarding state is %s', ( onboardingState, ctaText, dismissText, message ) => { registry .dispatch( MODULES_READER_REVENUE_MANAGER ) @@ -166,29 +165,19 @@ describe( 'RRMSetupSuccessSubtleNotification', () => { const dismissElement = getByText( dismissText ); expect( dismissElement ).toBeInTheDocument(); - // expect( mockTrackEvent ).toHaveBeenNthCalledWith( - // 1, - // `${ VIEW_CONTEXT_MAIN_DASHBOARD }_setup-success-notification-rrm`, - // 'view_notification', - // onboardingState - // ); - act( () => { fireEvent.click( ctaElement ); } ); - - expect( mockTrackEvent ).toHaveBeenNthCalledWith( - 2, - `${ VIEW_CONTEXT_MAIN_DASHBOARD }_setup-success-notification-rrm`, - 'confirm_notification', - onboardingState - ); } ); it.each( publicationStatesData )( - 'should dismiss the notification and trigger dismiss_notification event when the onboarding state is %s with CTA text %s and the dismiss CTA %s is clicked', - ( onboardingState, ctaText, dismissText ) => { + 'should dismiss the notification when the onboarding state is %s with CTA text %s and the dismiss CTA %s is clicked', + async ( onboardingState, ctaText, dismissText ) => { + fetchMock.postOnce( dismissItemEndpoint, { + body: [ id ], + } ); + registry .dispatch( MODULES_READER_REVENUE_MANAGER ) .setPublicationOnboardingState( onboardingState ); @@ -197,6 +186,10 @@ describe( 'RRMSetupSuccessSubtleNotification', () => { .dispatch( MODULES_READER_REVENUE_MANAGER ) .setPublicationID( 'ABCDEFGH' ); + await registry + .dispatch( CORE_NOTIFICATIONS ) + .receiveQueuedNotifications( [ { id } ] ); + const { container, getByText } = render( , { @@ -210,23 +203,9 @@ describe( 'RRMSetupSuccessSubtleNotification', () => { const dismissElement = getByText( dismissText ); expect( dismissElement ).toBeInTheDocument(); - expect( mockTrackEvent ).toHaveBeenNthCalledWith( - 1, - `${ VIEW_CONTEXT_MAIN_DASHBOARD }_setup-success-notification-rrm`, - 'view_notification', - onboardingState - ); - act( () => { fireEvent.click( dismissElement ); } ); - - expect( mockTrackEvent ).toHaveBeenNthCalledWith( - 2, - `${ VIEW_CONTEXT_MAIN_DASHBOARD }_setup-success-notification-rrm`, - 'dismiss_notification', - onboardingState - ); } ); From a5282c347ac00c8d61e015156f87eeee5634dec5 Mon Sep 17 00:00:00 2001 From: Jimmy Madon Date: Wed, 25 Sep 2024 13:05:52 +0530 Subject: [PATCH 28/33] Add a selector to get a notifcation from the state by its ID. --- .../notifications/datastore/notifications.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/assets/js/googlesitekit/notifications/datastore/notifications.js b/assets/js/googlesitekit/notifications/datastore/notifications.js index e3404c5a705..76c1db61da9 100644 --- a/assets/js/googlesitekit/notifications/datastore/notifications.js +++ b/assets/js/googlesitekit/notifications/datastore/notifications.js @@ -281,6 +281,18 @@ export const selectors = { getNotifications: ( state ) => { return state.notifications; }, + /** + * Fetches a registered notification by ID from state. + * + * @since n.e.x.t + * + * @param {Object} state Data store's state. + * @param {string} id Notification ID. + * @return {(Object|undefined)} The registered notification object or undefined if a notification with the given ID is not registered. + */ + getNotification: ( state, id ) => { + return state.notifications[ id ]; + }, /** * Fetches the queue of registered notifications which are filtered and sorted. * From 541ad911bc448226bbb56cc6a8763a0008896e29 Mon Sep 17 00:00:00 2001 From: Jimmy Madon Date: Wed, 25 Sep 2024 13:06:47 +0530 Subject: [PATCH 29/33] Dismiss a notification in the DB only if it is dismissible. --- .../notifications/datastore/notifications.js | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/assets/js/googlesitekit/notifications/datastore/notifications.js b/assets/js/googlesitekit/notifications/datastore/notifications.js index 76c1db61da9..25e99a28b8a 100644 --- a/assets/js/googlesitekit/notifications/datastore/notifications.js +++ b/assets/js/googlesitekit/notifications/datastore/notifications.js @@ -155,9 +155,16 @@ export const actions = { }; } - return registry - .dispatch( CORE_USER ) - .dismissItem( id, { expiresInSeconds } ); + const notification = registry + .select( CORE_NOTIFICATIONS ) + .getNotification( id ); + + // Persist the dismissal in the database only if the notification is dismissible. + if ( notification.isDismissible ) { + return registry + .dispatch( CORE_USER ) + .dismissItem( id, { expiresInSeconds } ); + } } ), }; From f54affb65442ed1f0492181dbe234cfc6b5efffc Mon Sep 17 00:00:00 2001 From: Jimmy Madon Date: Wed, 25 Sep 2024 14:14:00 +0530 Subject: [PATCH 30/33] Add tests for skipping persistent dismissal if notification is not dismissible. --- .../datastore/notifications.test.js | 81 +++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/assets/js/googlesitekit/notifications/datastore/notifications.test.js b/assets/js/googlesitekit/notifications/datastore/notifications.test.js index f00d3a546f9..5300a84dd41 100644 --- a/assets/js/googlesitekit/notifications/datastore/notifications.test.js +++ b/assets/js/googlesitekit/notifications/datastore/notifications.test.js @@ -155,6 +155,20 @@ describe( 'core/notifications Notifications', () => { } ); } ); describe( 'dismissNotification', () => { + function TestNotificationComponent() { + return
Test notification!
; + } + beforeEach( () => { + // dismissNotification checks for a registered notification's isDismissible property. + registry + .dispatch( CORE_NOTIFICATIONS ) + .registerNotification( 'test-notification', { + Component: TestNotificationComponent, + areaSlug: NOTIFICATION_AREAS.BANNERS_ABOVE_NAV, + viewContexts: [ 'mainDashboard' ], + isDismissible: true, + } ); + } ); it( 'should require a valid id to be provided', () => { expect( () => registry @@ -229,6 +243,73 @@ describe( 'core/notifications Notifications', () => { expect( fetchMock ).toHaveFetchedTimes( 1 ); } ); + it( 'should not persist dismissal if notification is not dismissible', async () => { + // dismissNotification checks for a registered notification's isDismissible property. + registry + .dispatch( CORE_NOTIFICATIONS ) + .registerNotification( 'not-dismissible-notification', { + Component: TestNotificationComponent, + areaSlug: NOTIFICATION_AREAS.BANNERS_ABOVE_NAV, + viewContexts: [ 'mainDashboard' ], + isDismissible: false, + } ); + + await registry + .dispatch( CORE_NOTIFICATIONS ) + .receiveQueuedNotifications( [ + { id: 'not-dismissible-notification' }, + ] ); + + await registry + .dispatch( CORE_NOTIFICATIONS ) + .dismissNotification( 'not-dismissible-notification', { + expiresInSeconds: 3, + } ); + + expect( fetchMock ).not.toHaveFetched( fetchDismissItem, { + body: { + data: { + slug: 'not-dismissible-notification', + expiration: 3, + }, + }, + } ); + } ); + it( 'should persist dismissal if notification is not dismissible', async () => { + fetchMock.postOnce( fetchDismissItem, { + body: [ 'dismissible-notification' ], + } ); + // dismissNotification checks for a registered notification's isDismissible property. + registry + .dispatch( CORE_NOTIFICATIONS ) + .registerNotification( 'dismissible-notification', { + Component: TestNotificationComponent, + areaSlug: NOTIFICATION_AREAS.BANNERS_ABOVE_NAV, + viewContexts: [ 'mainDashboard' ], + isDismissible: true, + } ); + + await registry + .dispatch( CORE_NOTIFICATIONS ) + .receiveQueuedNotifications( [ + { id: 'dismissible-notification' }, + ] ); + + await registry + .dispatch( CORE_NOTIFICATIONS ) + .dismissNotification( 'dismissible-notification', { + expiresInSeconds: 3, + } ); + + expect( fetchMock ).toHaveFetched( fetchDismissItem, { + body: { + data: { + slug: 'dismissible-notification', + expiration: 3, + }, + }, + } ); + } ); it( 'should remove a notification from queue if skipHidingFromQueue option is not passed', async () => { fetchMock.postOnce( fetchDismissItem, { body: [ 'test-notification' ], From 339fd109a43350456eb88bb40de4edb71f27d292 Mon Sep 17 00:00:00 2001 From: Jimmy Madon Date: Wed, 25 Sep 2024 14:21:22 +0530 Subject: [PATCH 31/33] Flip dismissal condition to record a dismissal if notification is undefined (for tests). --- .../notifications/datastore/notifications.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/assets/js/googlesitekit/notifications/datastore/notifications.js b/assets/js/googlesitekit/notifications/datastore/notifications.js index 25e99a28b8a..b3bafe07b11 100644 --- a/assets/js/googlesitekit/notifications/datastore/notifications.js +++ b/assets/js/googlesitekit/notifications/datastore/notifications.js @@ -159,12 +159,14 @@ export const actions = { .select( CORE_NOTIFICATIONS ) .getNotification( id ); - // Persist the dismissal in the database only if the notification is dismissible. - if ( notification.isDismissible ) { - return registry - .dispatch( CORE_USER ) - .dismissItem( id, { expiresInSeconds } ); + // Skip persisting notification dismissal in database if the notification is not dismissible. + if ( notification?.isDismissible === false ) { + return; } + + return registry + .dispatch( CORE_USER ) + .dismissItem( id, { expiresInSeconds } ); } ), }; From b835233b0faa174adea9ae9d49a40e4f1e2086ed Mon Sep 17 00:00:00 2001 From: nfmohit Date: Thu, 26 Sep 2024 13:23:41 +0600 Subject: [PATCH 32/33] Fix dismissible issue. --- .../RRMSetupSuccessSubtleNotification.test.js | 16 +++++++++------- .../js/modules/reader-revenue-manager/index.js | 1 + 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.test.js b/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.test.js index bcbeb3b3e5c..5f98b683398 100644 --- a/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.test.js +++ b/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.test.js @@ -86,9 +86,6 @@ describe( 'RRMSetupSuccessSubtleNotification', () => { const settingsEndpoint = new RegExp( '^/google-site-kit/v1/modules/reader-revenue-manager/data/settings' ); - const dismissItemEndpoint = new RegExp( - '^/google-site-kit/v1/core/user/data/dismiss-item' - ); beforeEach( () => { registry = createTestRegistry(); @@ -174,10 +171,6 @@ describe( 'RRMSetupSuccessSubtleNotification', () => { it.each( publicationStatesData )( 'should dismiss the notification when the onboarding state is %s with CTA text %s and the dismiss CTA %s is clicked', async ( onboardingState, ctaText, dismissText ) => { - fetchMock.postOnce( dismissItemEndpoint, { - body: [ id ], - } ); - registry .dispatch( MODULES_READER_REVENUE_MANAGER ) .setPublicationOnboardingState( onboardingState ); @@ -186,6 +179,15 @@ describe( 'RRMSetupSuccessSubtleNotification', () => { .dispatch( MODULES_READER_REVENUE_MANAGER ) .setPublicationID( 'ABCDEFGH' ); + await registry + .dispatch( CORE_NOTIFICATIONS ) + .registerNotification( id, { + Component: NotificationWithComponentProps, + areaSlug: 'notification-area-banners-above-nav', + viewContexts: [ 'mainDashboard' ], + isDismissible: false, + } ); + await registry .dispatch( CORE_NOTIFICATIONS ) .receiveQueuedNotifications( [ { id } ] ); diff --git a/assets/js/modules/reader-revenue-manager/index.js b/assets/js/modules/reader-revenue-manager/index.js index e68e94f7328..9966f567f53 100644 --- a/assets/js/modules/reader-revenue-manager/index.js +++ b/assets/js/modules/reader-revenue-manager/index.js @@ -108,5 +108,6 @@ export const registerNotifications = ( notifications ) => { return false; }, + isDismissible: false, } ); }; From 9c0e918b6c72fc86ed852cd39f850a6fd9466b42 Mon Sep 17 00:00:00 2001 From: nfmohit Date: Sat, 28 Sep 2024 17:09:54 +0600 Subject: [PATCH 33/33] Improve test coverage. --- .../RRMSetupSuccessSubtleNotification.test.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.test.js b/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.test.js index 5f98b683398..ed6aab27816 100644 --- a/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.test.js +++ b/assets/js/modules/reader-revenue-manager/components/dashboard/RRMSetupSuccessSubtleNotification.test.js @@ -87,6 +87,8 @@ describe( 'RRMSetupSuccessSubtleNotification', () => { '^/google-site-kit/v1/modules/reader-revenue-manager/data/settings' ); + const setValueMock = jest.fn(); + beforeEach( () => { registry = createTestRegistry(); @@ -99,7 +101,6 @@ describe( 'RRMSetupSuccessSubtleNotification', () => { ] ); useQueryArg.mockImplementation( ( arg ) => { - const setValueMock = jest.fn(); switch ( arg ) { case 'notification': return [ 'authentication_success', setValueMock ]; @@ -113,6 +114,8 @@ describe( 'RRMSetupSuccessSubtleNotification', () => { } ); afterEach( () => { + setValueMock.mockClear(); + useQueryArg.mockClear(); global.open.mockClear(); } ); @@ -161,10 +164,6 @@ describe( 'RRMSetupSuccessSubtleNotification', () => { const dismissElement = getByText( dismissText ); expect( dismissElement ).toBeInTheDocument(); - - act( () => { - fireEvent.click( ctaElement ); - } ); } ); @@ -208,6 +207,9 @@ describe( 'RRMSetupSuccessSubtleNotification', () => { act( () => { fireEvent.click( dismissElement ); } ); + + expect( setValueMock ).toHaveBeenCalledTimes( 2 ); + expect( setValueMock ).toHaveBeenCalledWith( undefined ); } );