From 2c1a52609633b5331232aa2bc547dc84c65e07c7 Mon Sep 17 00:00:00 2001 From: Tom Rees-Herdman Date: Fri, 24 Jun 2022 11:39:46 +0100 Subject: [PATCH 01/18] Add RecoverableModules component. --- assets/js/components/RecoverableModules.js | 77 ++++++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 assets/js/components/RecoverableModules.js diff --git a/assets/js/components/RecoverableModules.js b/assets/js/components/RecoverableModules.js new file mode 100644 index 00000000000..bcfa674f4a3 --- /dev/null +++ b/assets/js/components/RecoverableModules.js @@ -0,0 +1,77 @@ +/** + * RecoverableModules component. + * + * Site Kit by Google, Copyright 2022 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'; + +/** + * WordPress dependencies + */ +import { __, _x, sprintf } from '@wordpress/i18n'; + +/** + * Internal dependencies + */ +import Data from 'googlesitekit-data'; +import { CORE_MODULES } from '../googlesitekit/modules/datastore/constants'; +import CTA from './notifications/CTA'; + +const { useSelect } = Data; + +export default function RecoverableModules( { moduleSlugs } ) { + const moduleNames = useSelect( ( select ) => + moduleSlugs.map( + ( moduleSlug ) => + select( CORE_MODULES ).getModule( moduleSlug ).name + ) + ); + + const description = + moduleNames.length === 1 + ? sprintf( + /* translators: %s: Module name */ + __( + '%s data was previously shared by an admin who no longer has access. Please contact another admin to restore it.', + 'google-site-kit' + ), + moduleNames[ 0 ] + ) + : sprintf( + /* translators: %s: List of module names */ + __( + 'The data for the following modules was previously shared by an admin who no longer has access: %s. Please contact another admin to restore it.', + 'google-site-kit' + ), + moduleNames.join( + _x( ', ', 'Recoverable modules', 'google-site-kit' ) + ) + ); + + return ( + + ); +} + +RecoverableModules.propTypes = { + moduleSlugs: PropTypes.arrayOf( PropTypes.string ).isRequired, +}; From 9185554f3e59650eeaab47b4edbd345129089a1e Mon Sep 17 00:00:00 2001 From: Tom Rees-Herdman Date: Fri, 24 Jun 2022 11:40:02 +0100 Subject: [PATCH 02/18] Add WidgetRecoverableModules component. --- .../components/WidgetRecoverableModules.js | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 assets/js/googlesitekit/widgets/components/WidgetRecoverableModules.js diff --git a/assets/js/googlesitekit/widgets/components/WidgetRecoverableModules.js b/assets/js/googlesitekit/widgets/components/WidgetRecoverableModules.js new file mode 100644 index 00000000000..3266b13ffea --- /dev/null +++ b/assets/js/googlesitekit/widgets/components/WidgetRecoverableModules.js @@ -0,0 +1,50 @@ +/** + * WidgetRecoverableModules component. + * + * Site Kit by Google, Copyright 2022 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'; + +/** + * WordPress dependencies + */ +import { useMemo } from '@wordpress/element'; + +/** + * Internal dependencies + */ +import useWidgetStateEffect from '../hooks/useWidgetStateEffect'; +import RecoverableModules from '../../../components/RecoverableModules'; + +// The supported props must match `RecoverableModules` (except `widgetSlug`). +export default function WidgetRecoverableModules( { + widgetSlug, + moduleSlugs, + ...props +} ) { + const metadata = useMemo( () => ( { moduleSlugs } ), [ moduleSlugs ] ); + useWidgetStateEffect( widgetSlug, RecoverableModules, metadata ); + + return ; +} + +WidgetRecoverableModules.propTypes = { + widgetSlug: PropTypes.string.isRequired, + ...RecoverableModules.propTypes, +}; From 3bcde1cdd345816e5263adf90783f2c7efdd4a32 Mon Sep 17 00:00:00 2001 From: Tom Rees-Herdman Date: Fri, 24 Jun 2022 11:40:33 +0100 Subject: [PATCH 03/18] Memoize recoverable modules, preventing rerender issues. --- .../modules/datastore/modules.js | 37 ++++++++++++------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/assets/js/googlesitekit/modules/datastore/modules.js b/assets/js/googlesitekit/modules/datastore/modules.js index 0fdee0de3cf..d0462116032 100644 --- a/assets/js/googlesitekit/modules/datastore/modules.js +++ b/assets/js/googlesitekit/modules/datastore/modules.js @@ -100,6 +100,29 @@ const normalizeModules = memize( ( serverDefinitions, clientDefinitions ) => { }, {} ); } ); +/** + * Memoized function to build an object mapping recoverable module slugs to their corresponding + * module objects. + * + * @since n.e.x.t + * + * @param {Object} modules Module definitions. + * @param {Array} recoverableModules Array of recoverable module slugs. + * @return {Object} Map of recoverable module slugs to their corresponding module objects. + */ +const calculateRecoverableModules = memize( ( modules, recoverableModules ) => + Object.values( modules ).reduce( ( recoverable, module ) => { + if ( recoverableModules.includes( module.slug ) ) { + return { + ...recoverable, + [ module.slug ]: module, + }; + } + + return recoverable; + }, {} ) +); + const fetchGetModulesStore = createFetchStore( { baseName: 'getModules', controlCallback: () => { @@ -1244,19 +1267,7 @@ const baseSelectors = { return undefined; } - return Object.values( modules ).reduce( - ( recoverableModules, module ) => { - if ( state.recoverableModules.includes( module.slug ) ) { - return { - ...recoverableModules, - [ module.slug ]: module, - }; - } - - return recoverableModules; - }, - {} - ); + return calculateRecoverableModules( modules, state.recoverableModules ); } ), /** From 0739d7571aa85c9173486a4746579b660565c17e Mon Sep 17 00:00:00 2001 From: Tom Rees-Herdman Date: Fri, 24 Jun 2022 11:41:25 +0100 Subject: [PATCH 04/18] Add RecoverableModules as special widget state. --- assets/js/googlesitekit/widgets/util/constants.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/assets/js/googlesitekit/widgets/util/constants.js b/assets/js/googlesitekit/widgets/util/constants.js index d619abd737e..2790ed84ecb 100644 --- a/assets/js/googlesitekit/widgets/util/constants.js +++ b/assets/js/googlesitekit/widgets/util/constants.js @@ -21,6 +21,7 @@ */ import { WIDGET_WIDTHS } from '../datastore/constants'; import ReportZero from '../../../components/ReportZero'; +import RecoverableModules from '../../../components/RecoverableModules'; import CompleteModuleActivationCTA from '../../../components/CompleteModuleActivationCTA'; import ActivateModuleCTA from '../../../components/ActivateModuleCTA'; @@ -35,4 +36,5 @@ export const SPECIAL_WIDGET_STATES = [ ActivateModuleCTA, CompleteModuleActivationCTA, ReportZero, + RecoverableModules, ]; From 6f9d0f29d1d5f68a03d5714f9c4d0d4116dfc00a Mon Sep 17 00:00:00 2001 From: Tom Rees-Herdman Date: Fri, 24 Jun 2022 11:42:00 +0100 Subject: [PATCH 05/18] Render WidgetRecoverableModules in WidgetRenderer. --- .../widgets/components/WidgetRenderer.js | 25 ++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/assets/js/googlesitekit/widgets/components/WidgetRenderer.js b/assets/js/googlesitekit/widgets/components/WidgetRenderer.js index 77463f13dd2..43907216745 100644 --- a/assets/js/googlesitekit/widgets/components/WidgetRenderer.js +++ b/assets/js/googlesitekit/widgets/components/WidgetRenderer.js @@ -20,18 +20,21 @@ * External dependencies */ import PropTypes from 'prop-types'; +import intersection from 'lodash/intersection'; /** * WordPress dependencies */ -import { Fragment } from '@wordpress/element'; +import { useMemo, Fragment } from '@wordpress/element'; /** * Internal dependencies */ import Data from 'googlesitekit-data'; import { CORE_WIDGETS } from '../datastore/constants'; +import { CORE_MODULES } from '../../modules/datastore/constants'; import BaseWidget from './Widget'; +import WidgetRecoverableModules from './WidgetRecoverableModules'; import { getWidgetComponentProps } from '../util'; import { HIDDEN_CLASS } from '../util/constants'; @@ -44,6 +47,17 @@ const WidgetRenderer = ( { slug, OverrideComponent } ) => { const widgetComponentProps = getWidgetComponentProps( slug ); const { Widget, WidgetNull } = widgetComponentProps; + const recoverableModules = useSelect( ( select ) => + select( CORE_MODULES ).getRecoverableModules() + ); + + const widgetRecoverableModules = useMemo( + () => + recoverableModules && + intersection( widget.modules, Object.keys( recoverableModules ) ), + [ recoverableModules, widget.modules ] + ); + if ( ! widget ) { return ; } @@ -52,6 +66,15 @@ const WidgetRenderer = ( { slug, OverrideComponent } ) => { let widgetElement = ; + if ( widgetRecoverableModules?.length ) { + widgetElement = ( + + ); + } + if ( OverrideComponent ) { // If OverrideComponent passed, render it instead of the actual widget. // It always needs to be wrapped as it is expected to be a From 404312e163075fb47118494eaaa2e65f5478d292 Mon Sep 17 00:00:00 2001 From: Tom Rees-Herdman Date: Fri, 24 Jun 2022 11:53:25 +0100 Subject: [PATCH 06/18] Return null from RecoverableModules when modules are not resolved. --- assets/js/components/RecoverableModules.js | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/assets/js/components/RecoverableModules.js b/assets/js/components/RecoverableModules.js index bcfa674f4a3..816ea459e4f 100644 --- a/assets/js/components/RecoverableModules.js +++ b/assets/js/components/RecoverableModules.js @@ -36,12 +36,19 @@ import CTA from './notifications/CTA'; const { useSelect } = Data; export default function RecoverableModules( { moduleSlugs } ) { - const moduleNames = useSelect( ( select ) => - moduleSlugs.map( - ( moduleSlug ) => - select( CORE_MODULES ).getModule( moduleSlug ).name - ) - ); + const moduleNames = useSelect( ( select ) => { + const modules = select( CORE_MODULES ).getModules(); + + if ( modules === undefined ) { + return undefined; + } + + return moduleSlugs.map( ( moduleSlug ) => modules[ moduleSlug ].name ); + } ); + + if ( moduleNames === undefined ) { + return null; + } const description = moduleNames.length === 1 From 6246bbfe88fc9fc5033aeb46b6ca91b28881dd89 Mon Sep 17 00:00:00 2001 From: Tom Rees-Herdman Date: Fri, 24 Jun 2022 11:53:55 +0100 Subject: [PATCH 07/18] Fix WidgetRenderer for case where widget is not found. --- assets/js/googlesitekit/widgets/components/WidgetRenderer.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/assets/js/googlesitekit/widgets/components/WidgetRenderer.js b/assets/js/googlesitekit/widgets/components/WidgetRenderer.js index 43907216745..cbb16e577d2 100644 --- a/assets/js/googlesitekit/widgets/components/WidgetRenderer.js +++ b/assets/js/googlesitekit/widgets/components/WidgetRenderer.js @@ -53,9 +53,10 @@ const WidgetRenderer = ( { slug, OverrideComponent } ) => { const widgetRecoverableModules = useMemo( () => + widget && recoverableModules && intersection( widget.modules, Object.keys( recoverableModules ) ), - [ recoverableModules, widget.modules ] + [ recoverableModules, widget ] ); if ( ! widget ) { From ca648eb57b21345edbcfe09b3501c60b4b4974af Mon Sep 17 00:00:00 2001 From: Tom Rees-Herdman Date: Fri, 24 Jun 2022 12:35:53 +0100 Subject: [PATCH 08/18] Fix tests. --- .../widgets/components/WidgetAreaRenderer.test.js | 11 +++++++++-- .../widgets/components/WidgetContextRenderer.test.js | 5 +++++ .../widgets/components/WidgetRenderer.test.js | 10 ++++++++-- 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/assets/js/googlesitekit/widgets/components/WidgetAreaRenderer.test.js b/assets/js/googlesitekit/widgets/components/WidgetAreaRenderer.test.js index 609804bb9c5..656872a9489 100644 --- a/assets/js/googlesitekit/widgets/components/WidgetAreaRenderer.test.js +++ b/assets/js/googlesitekit/widgets/components/WidgetAreaRenderer.test.js @@ -27,6 +27,7 @@ import { WIDGET_AREA_STYLES, } from '../datastore/constants'; import { CORE_SITE } from '../../../googlesitekit/datastore/site/constants'; +import { CORE_MODULES } from '../../modules/datastore/constants'; import { createTestRegistry, render, @@ -88,10 +89,14 @@ describe( 'WidgetAreaRenderer', () => { const areaName = 'gridcell-test'; let registry; - beforeEach( async () => { + beforeEach( () => { registry = createTestRegistryWithArea( areaName ); + + provideModules( registry ); + registry.dispatch( CORE_MODULES ).receiveRecoverableModules( [] ); + const connection = { connected: true }; - await registry.dispatch( CORE_SITE ).receiveGetConnection( connection ); + registry.dispatch( CORE_SITE ).receiveGetConnection( connection ); } ); afterEach( () => { @@ -623,6 +628,8 @@ describe( 'WidgetAreaRenderer', () => { areaName, WIDGET_AREA_STYLES.COMPOSITE ); + provideModules( registry ); + registry.dispatch( CORE_MODULES ).receiveRecoverableModules( [] ); registry .dispatch( CORE_SITE ) .receiveGetConnection( { connected: true } ); diff --git a/assets/js/googlesitekit/widgets/components/WidgetContextRenderer.test.js b/assets/js/googlesitekit/widgets/components/WidgetContextRenderer.test.js index 07855424dad..57485d1df7c 100644 --- a/assets/js/googlesitekit/widgets/components/WidgetContextRenderer.test.js +++ b/assets/js/googlesitekit/widgets/components/WidgetContextRenderer.test.js @@ -21,7 +21,9 @@ */ import WidgetContextRenderer from './WidgetContextRenderer'; import { CORE_WIDGETS } from '../datastore/constants'; +import { CORE_MODULES } from '../../modules/datastore/constants'; import { + provideModules, createTestRegistry, render, waitFor, @@ -41,6 +43,9 @@ describe( 'WidgetContextRenderer', () => { beforeEach( () => { registry = createTestRegistry(); + provideModules( registry ); + registry.dispatch( CORE_MODULES ).receiveRecoverableModules( [] ); + // Register a widget area. registry.dispatch( CORE_WIDGETS ).registerWidgetArea( 'TestArea1', { title: 'Dashboard Header', diff --git a/assets/js/googlesitekit/widgets/components/WidgetRenderer.test.js b/assets/js/googlesitekit/widgets/components/WidgetRenderer.test.js index 879aebab726..129baefa5d5 100644 --- a/assets/js/googlesitekit/widgets/components/WidgetRenderer.test.js +++ b/assets/js/googlesitekit/widgets/components/WidgetRenderer.test.js @@ -20,14 +20,20 @@ * Internal dependencies */ import WidgetRenderer from './WidgetRenderer'; +import { CORE_MODULES } from '../../modules/datastore/constants'; import { CORE_WIDGETS } from '../datastore/constants'; -import { render } from '../../../../../tests/js/test-utils'; +import { provideModules, render } from '../../../../../tests/js/test-utils'; const setupRegistry = ( { Component = () =>
Test
, wrapWidget = false, } = {} ) => { - return ( { dispatch } ) => { + return ( registry ) => { + const { dispatch } = registry; + + provideModules( registry ); + dispatch( CORE_MODULES ).receiveRecoverableModules( [] ); + dispatch( CORE_WIDGETS ).registerWidgetArea( 'dashboard-header', { title: 'Dashboard Header', subtitle: 'Cool stuff for yoursite.com', From 21f9a8f24dacfebe99a3180fd2d6b9927be5c26a Mon Sep 17 00:00:00 2001 From: Tom Rees-Herdman Date: Fri, 24 Jun 2022 14:10:47 +0100 Subject: [PATCH 09/18] Only display RecoverableModules component in view-only mode. --- assets/js/googlesitekit/widgets/components/WidgetRenderer.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/assets/js/googlesitekit/widgets/components/WidgetRenderer.js b/assets/js/googlesitekit/widgets/components/WidgetRenderer.js index cbb16e577d2..aa9e743a8e5 100644 --- a/assets/js/googlesitekit/widgets/components/WidgetRenderer.js +++ b/assets/js/googlesitekit/widgets/components/WidgetRenderer.js @@ -37,6 +37,7 @@ import BaseWidget from './Widget'; import WidgetRecoverableModules from './WidgetRecoverableModules'; import { getWidgetComponentProps } from '../util'; import { HIDDEN_CLASS } from '../util/constants'; +import useViewOnly from '../../../hooks/useViewOnly'; const { useSelect } = Data; @@ -51,6 +52,7 @@ const WidgetRenderer = ( { slug, OverrideComponent } ) => { select( CORE_MODULES ).getRecoverableModules() ); + const viewOnly = useViewOnly(); const widgetRecoverableModules = useMemo( () => widget && @@ -67,7 +69,7 @@ const WidgetRenderer = ( { slug, OverrideComponent } ) => { let widgetElement = ; - if ( widgetRecoverableModules?.length ) { + if ( viewOnly && widgetRecoverableModules?.length ) { widgetElement = ( Date: Mon, 27 Jun 2022 09:51:37 +0100 Subject: [PATCH 10/18] Add tests for conditionally rendering RecoverableModules. --- .../widgets/components/WidgetRenderer.test.js | 49 ++++++++++++++++++- 1 file changed, 48 insertions(+), 1 deletion(-) diff --git a/assets/js/googlesitekit/widgets/components/WidgetRenderer.test.js b/assets/js/googlesitekit/widgets/components/WidgetRenderer.test.js index 129baefa5d5..ca70f6ad2ab 100644 --- a/assets/js/googlesitekit/widgets/components/WidgetRenderer.test.js +++ b/assets/js/googlesitekit/widgets/components/WidgetRenderer.test.js @@ -20,6 +20,8 @@ * Internal dependencies */ import WidgetRenderer from './WidgetRenderer'; +import { Provider as ViewContextProvider } from '../../../components/Root/ViewContextContext'; +import { VIEW_CONTEXT_DASHBOARD_VIEW_ONLY } from '../../../googlesitekit/constants'; import { CORE_MODULES } from '../../modules/datastore/constants'; import { CORE_WIDGETS } from '../datastore/constants'; import { provideModules, render } from '../../../../../tests/js/test-utils'; @@ -27,12 +29,15 @@ import { provideModules, render } from '../../../../../tests/js/test-utils'; const setupRegistry = ( { Component = () =>
Test
, wrapWidget = false, + recoverableModules = [], } = {} ) => { return ( registry ) => { const { dispatch } = registry; provideModules( registry ); - dispatch( CORE_MODULES ).receiveRecoverableModules( [] ); + dispatch( CORE_MODULES ).receiveRecoverableModules( + recoverableModules + ); dispatch( CORE_WIDGETS ).registerWidgetArea( 'dashboard-header', { title: 'Dashboard Header', @@ -46,6 +51,7 @@ const setupRegistry = ( { dispatch( CORE_WIDGETS ).registerWidget( 'TestWidget', { Component, wrapWidget, + modules: [ 'search-console', 'pagespeed-insights' ], } ); dispatch( CORE_WIDGETS ).assignWidget( 'TestWidget', @@ -84,4 +90,45 @@ describe( 'WidgetRenderer', () => { expect( container.firstChild ).toEqual( null ); } ); + + it( 'should output the recoverable modules component when the widget depends on a recoverable module', async () => { + const { getByText } = render( + + + , + { + setupRegistry: setupRegistry( { + recoverableModules: [ 'search-console' ], + } ), + } + ); + + expect( + getByText( + /Search Console data was previously shared by an admin who no longer has access/ + ) + ).toBeInTheDocument(); + } ); + + it( 'should output the recoverable modules component when the widget depends on multiple recoverable modules', async () => { + const { getByText } = render( + + + , + { + setupRegistry: setupRegistry( { + recoverableModules: [ + 'search-console', + 'pagespeed-insights', + ], + } ), + } + ); + + expect( + getByText( + /The data for the following modules was previously shared by an admin who no longer has access: Search Console, PageSpeed Insights/ + ) + ).toBeInTheDocument(); + } ); } ); From 6e516b20b37426e98e3c229f9d34b678193abcb9 Mon Sep 17 00:00:00 2001 From: Tom Rees-Herdman Date: Tue, 28 Jun 2022 09:46:09 +0100 Subject: [PATCH 11/18] Serialize module slugs to metadata.moduleSlug for compatibility with combineWidgets(). --- .../widgets/components/WidgetRecoverableModules.js | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/assets/js/googlesitekit/widgets/components/WidgetRecoverableModules.js b/assets/js/googlesitekit/widgets/components/WidgetRecoverableModules.js index 3266b13ffea..42783ada131 100644 --- a/assets/js/googlesitekit/widgets/components/WidgetRecoverableModules.js +++ b/assets/js/googlesitekit/widgets/components/WidgetRecoverableModules.js @@ -38,7 +38,19 @@ export default function WidgetRecoverableModules( { moduleSlugs, ...props } ) { - const metadata = useMemo( () => ( { moduleSlugs } ), [ moduleSlugs ] ); + const metadata = useMemo( + () => ( { + // Here we serialize to `moduleSlug` for compatibility with the logic in + // `combineWidgets()`. In future we may wish to take a less "hacky" approach. + // See https://github.com/google/site-kit-wp/issues/5376#issuecomment-1165771399. + moduleSlug: JSON.stringify( moduleSlugs.sort() ), + // We also store `moduleSlugs` in the metadata in order for it to be passed back + // into RecoverableModules as a prop. + // See https://github.com/google/site-kit-wp/blob/c272c20eddcca61aae24c9812b6b11dbc15ec673/assets/js/googlesitekit/widgets/components/WidgetAreaRenderer.js#L171. + moduleSlugs, + } ), + [ moduleSlugs ] + ); useWidgetStateEffect( widgetSlug, RecoverableModules, metadata ); return ; From 942af840b6888dad9483ff0edeaf171e0f7dfb49 Mon Sep 17 00:00:00 2001 From: Tom Rees-Herdman Date: Tue, 28 Jun 2022 12:25:00 +0100 Subject: [PATCH 12/18] Update WidgetRenderer test. --- .../widgets/components/WidgetRenderer.test.js | 38 +++++++------------ 1 file changed, 13 insertions(+), 25 deletions(-) diff --git a/assets/js/googlesitekit/widgets/components/WidgetRenderer.test.js b/assets/js/googlesitekit/widgets/components/WidgetRenderer.test.js index ca70f6ad2ab..4d37d5a0393 100644 --- a/assets/js/googlesitekit/widgets/components/WidgetRenderer.test.js +++ b/assets/js/googlesitekit/widgets/components/WidgetRenderer.test.js @@ -20,7 +20,6 @@ * Internal dependencies */ import WidgetRenderer from './WidgetRenderer'; -import { Provider as ViewContextProvider } from '../../../components/Root/ViewContextContext'; import { VIEW_CONTEXT_DASHBOARD_VIEW_ONLY } from '../../../googlesitekit/constants'; import { CORE_MODULES } from '../../modules/datastore/constants'; import { CORE_WIDGETS } from '../datastore/constants'; @@ -92,16 +91,12 @@ describe( 'WidgetRenderer', () => { } ); it( 'should output the recoverable modules component when the widget depends on a recoverable module', async () => { - const { getByText } = render( - - - , - { - setupRegistry: setupRegistry( { - recoverableModules: [ 'search-console' ], - } ), - } - ); + const { getByText } = render( , { + setupRegistry: setupRegistry( { + recoverableModules: [ 'search-console' ], + } ), + viewContext: VIEW_CONTEXT_DASHBOARD_VIEW_ONLY, + } ); expect( getByText( @@ -111,23 +106,16 @@ describe( 'WidgetRenderer', () => { } ); it( 'should output the recoverable modules component when the widget depends on multiple recoverable modules', async () => { - const { getByText } = render( - - - , - { - setupRegistry: setupRegistry( { - recoverableModules: [ - 'search-console', - 'pagespeed-insights', - ], - } ), - } - ); + const { getByText } = render( , { + setupRegistry: setupRegistry( { + recoverableModules: [ 'search-console', 'pagespeed-insights' ], + } ), + viewContext: VIEW_CONTEXT_DASHBOARD_VIEW_ONLY, + } ); expect( getByText( - /The data for the following modules was previously shared by an admin who no longer has access: Search Console, PageSpeed Insights/ + /The data for the following modules was previously shared by an admin who no longer has access: PageSpeed Insights, Search Console/ ) ).toBeInTheDocument(); } ); From 7b01287e2c191dbabb3c51461e90d6f2de616e3f Mon Sep 17 00:00:00 2001 From: Tom Rees-Herdman Date: Tue, 28 Jun 2022 12:25:26 +0100 Subject: [PATCH 13/18] Add test for combining multiple widgets in RecoverableModules state. --- .../components/WidgetAreaRenderer.test.js | 63 ++++++++++++++ .../WidgetAreaRenderer.test.js.snap | 87 +++++++++++++++++++ 2 files changed, 150 insertions(+) diff --git a/assets/js/googlesitekit/widgets/components/WidgetAreaRenderer.test.js b/assets/js/googlesitekit/widgets/components/WidgetAreaRenderer.test.js index 656872a9489..6169cffc64e 100644 --- a/assets/js/googlesitekit/widgets/components/WidgetAreaRenderer.test.js +++ b/assets/js/googlesitekit/widgets/components/WidgetAreaRenderer.test.js @@ -16,6 +16,11 @@ * limitations under the License. */ +/** + * External dependencies + */ +import { getByText } from '@testing-library/dom'; + /** * Internal dependencies */ @@ -709,4 +714,62 @@ describe( 'WidgetAreaRenderer', () => { ) ).toHaveLength( 1 ); } ); + + it( 'should combine multiple widgets in RecoverableModules state with the same metadata into a single widget', async () => { + registry + .dispatch( CORE_MODULES ) + .receiveRecoverableModules( [ 'search-console' ] ); + + provideUserCapabilities( registry, { + [ PERMISSION_VIEW_DASHBOARD ]: true, + [ `${ PERMISSION_READ_SHARED_MODULE_DATA }::["search-console"]` ]: true, + } ); + + createWidgets( registry, areaName, [ + { + Component: WidgetComponent, + slug: 'one', + modules: [ 'search-console' ], + }, + { + Component: WidgetComponent, + slug: 'two', + modules: [ 'search-console' ], + }, + ] ); + + const { container } = render( + , + { registry, viewContext: VIEW_CONTEXT_DASHBOARD_VIEW_ONLY } + ); + + const visibleWidgetSelector = + '.googlesitekit-widget-area-widgets > .mdc-layout-grid__inner > .mdc-layout-grid__cell > .googlesitekit-widget'; + + // There should be a single visible widget. + expect( + container.firstChild.querySelectorAll( visibleWidgetSelector ) + ).toHaveLength( 1 ); + + // The visible widget should be rendered as the RecoverableModules component. + expect( + getByText( + container.firstChild.querySelector( visibleWidgetSelector ), + 'Search Console data was previously shared by an admin who no longer has access. Please contact another admin to restore it.' + ) + ).toBeInTheDocument(); + + // There should also be a hidden widget. + expect( + container.firstChild.querySelectorAll( + '.googlesitekit-widget-area-widgets .googlesitekit-hidden .googlesitekit-widget' + ) + ).toHaveLength( 1 ); + + expect( + container.firstChild.querySelector( + '.googlesitekit-widget-area-widgets' + ) + ).toMatchSnapshot(); + } ); } ); diff --git a/assets/js/googlesitekit/widgets/components/__snapshots__/WidgetAreaRenderer.test.js.snap b/assets/js/googlesitekit/widgets/components/__snapshots__/WidgetAreaRenderer.test.js.snap index 25d73722829..e7c0faeab3a 100644 --- a/assets/js/googlesitekit/widgets/components/__snapshots__/WidgetAreaRenderer.test.js.snap +++ b/assets/js/googlesitekit/widgets/components/__snapshots__/WidgetAreaRenderer.test.js.snap @@ -1,5 +1,92 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`WidgetAreaRenderer should combine multiple widgets in RecoverableModules state with the same metadata into a single widget 1`] = ` +
+
+
+
+
+
+

+ Data Unavailable +

+

+ Search Console data was previously shared by an admin who no longer has access. Please contact another admin to restore it. +

+ + +
+
+
+
+
+

+ Data Unavailable +

+

+ Search Console data was previously shared by an admin who no longer has access. Please contact another admin to restore it. +

+ + +
+
+
+
+
+
+
+

+ Data Unavailable +

+

+ Search Console data was previously shared by an admin who no longer has access. Please contact another admin to restore it. +

+ + +
+
+
+
+
+
+`; + exports[`WidgetAreaRenderer should not resize widgets in a row that is smaller than 9 columns (3, 12, 3-3) 1`] = `
Date: Tue, 28 Jun 2022 13:40:48 +0100 Subject: [PATCH 14/18] Only call getRecoverableModules() when dashboard sharing is enabled. --- .../googlesitekit/widgets/components/WidgetRenderer.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/assets/js/googlesitekit/widgets/components/WidgetRenderer.js b/assets/js/googlesitekit/widgets/components/WidgetRenderer.js index aa9e743a8e5..105c71dbd16 100644 --- a/assets/js/googlesitekit/widgets/components/WidgetRenderer.js +++ b/assets/js/googlesitekit/widgets/components/WidgetRenderer.js @@ -38,18 +38,23 @@ import WidgetRecoverableModules from './WidgetRecoverableModules'; import { getWidgetComponentProps } from '../util'; import { HIDDEN_CLASS } from '../util/constants'; import useViewOnly from '../../../hooks/useViewOnly'; +import { useFeature } from '../../../hooks/useFeature'; const { useSelect } = Data; const WidgetRenderer = ( { slug, OverrideComponent } ) => { + const dashboardSharingEnabled = useFeature( 'dashboardSharing' ); + const widget = useSelect( ( select ) => select( CORE_WIDGETS ).getWidget( slug ) ); const widgetComponentProps = getWidgetComponentProps( slug ); const { Widget, WidgetNull } = widgetComponentProps; - const recoverableModules = useSelect( ( select ) => - select( CORE_MODULES ).getRecoverableModules() + const recoverableModules = useSelect( + ( select ) => + dashboardSharingEnabled && + select( CORE_MODULES ).getRecoverableModules() ); const viewOnly = useViewOnly(); From 129888322131059d179438ef6866fd7cad41745d Mon Sep 17 00:00:00 2001 From: Tom Rees-Herdman Date: Tue, 28 Jun 2022 13:51:01 +0100 Subject: [PATCH 15/18] Fix tests. --- .../components/WidgetAreaRenderer.test.js | 18 +++++++++--------- .../components/WidgetContextRenderer.test.js | 5 ----- .../widgets/components/WidgetRenderer.test.js | 2 ++ 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/assets/js/googlesitekit/widgets/components/WidgetAreaRenderer.test.js b/assets/js/googlesitekit/widgets/components/WidgetAreaRenderer.test.js index 6169cffc64e..a403913c517 100644 --- a/assets/js/googlesitekit/widgets/components/WidgetAreaRenderer.test.js +++ b/assets/js/googlesitekit/widgets/components/WidgetAreaRenderer.test.js @@ -94,14 +94,11 @@ describe( 'WidgetAreaRenderer', () => { const areaName = 'gridcell-test'; let registry; - beforeEach( () => { + beforeEach( async () => { registry = createTestRegistryWithArea( areaName ); - provideModules( registry ); - registry.dispatch( CORE_MODULES ).receiveRecoverableModules( [] ); - const connection = { connected: true }; - registry.dispatch( CORE_SITE ).receiveGetConnection( connection ); + await registry.dispatch( CORE_SITE ).receiveGetConnection( connection ); } ); afterEach( () => { @@ -633,8 +630,6 @@ describe( 'WidgetAreaRenderer', () => { areaName, WIDGET_AREA_STYLES.COMPOSITE ); - provideModules( registry ); - registry.dispatch( CORE_MODULES ).receiveRecoverableModules( [] ); registry .dispatch( CORE_SITE ) .receiveGetConnection( { connected: true } ); @@ -715,7 +710,8 @@ describe( 'WidgetAreaRenderer', () => { ).toHaveLength( 1 ); } ); - it( 'should combine multiple widgets in RecoverableModules state with the same metadata into a single widget', async () => { + it( 'should combine multiple widgets in RecoverableModules state with the same metadata into a single widget', () => { + provideModules( registry ); registry .dispatch( CORE_MODULES ) .receiveRecoverableModules( [ 'search-console' ] ); @@ -740,7 +736,11 @@ describe( 'WidgetAreaRenderer', () => { const { container } = render( , - { registry, viewContext: VIEW_CONTEXT_DASHBOARD_VIEW_ONLY } + { + registry, + viewContext: VIEW_CONTEXT_DASHBOARD_VIEW_ONLY, + features: [ 'dashboardSharing' ], + } ); const visibleWidgetSelector = diff --git a/assets/js/googlesitekit/widgets/components/WidgetContextRenderer.test.js b/assets/js/googlesitekit/widgets/components/WidgetContextRenderer.test.js index 57485d1df7c..07855424dad 100644 --- a/assets/js/googlesitekit/widgets/components/WidgetContextRenderer.test.js +++ b/assets/js/googlesitekit/widgets/components/WidgetContextRenderer.test.js @@ -21,9 +21,7 @@ */ import WidgetContextRenderer from './WidgetContextRenderer'; import { CORE_WIDGETS } from '../datastore/constants'; -import { CORE_MODULES } from '../../modules/datastore/constants'; import { - provideModules, createTestRegistry, render, waitFor, @@ -43,9 +41,6 @@ describe( 'WidgetContextRenderer', () => { beforeEach( () => { registry = createTestRegistry(); - provideModules( registry ); - registry.dispatch( CORE_MODULES ).receiveRecoverableModules( [] ); - // Register a widget area. registry.dispatch( CORE_WIDGETS ).registerWidgetArea( 'TestArea1', { title: 'Dashboard Header', diff --git a/assets/js/googlesitekit/widgets/components/WidgetRenderer.test.js b/assets/js/googlesitekit/widgets/components/WidgetRenderer.test.js index 4d37d5a0393..d9ea846bad3 100644 --- a/assets/js/googlesitekit/widgets/components/WidgetRenderer.test.js +++ b/assets/js/googlesitekit/widgets/components/WidgetRenderer.test.js @@ -96,6 +96,7 @@ describe( 'WidgetRenderer', () => { recoverableModules: [ 'search-console' ], } ), viewContext: VIEW_CONTEXT_DASHBOARD_VIEW_ONLY, + features: [ 'dashboardSharing' ], } ); expect( @@ -111,6 +112,7 @@ describe( 'WidgetRenderer', () => { recoverableModules: [ 'search-console', 'pagespeed-insights' ], } ), viewContext: VIEW_CONTEXT_DASHBOARD_VIEW_ONLY, + features: [ 'dashboardSharing' ], } ); expect( From 481d3a55ef536f639370311809c5ca4de33fb1f2 Mon Sep 17 00:00:00 2001 From: Tom Rees-Herdman Date: Wed, 29 Jun 2022 11:26:20 +0100 Subject: [PATCH 16/18] Update JSDoc to adhere to conventions. Co-authored-by: Evan Mattson --- assets/js/googlesitekit/modules/datastore/modules.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assets/js/googlesitekit/modules/datastore/modules.js b/assets/js/googlesitekit/modules/datastore/modules.js index d0462116032..09324299359 100644 --- a/assets/js/googlesitekit/modules/datastore/modules.js +++ b/assets/js/googlesitekit/modules/datastore/modules.js @@ -101,7 +101,7 @@ const normalizeModules = memize( ( serverDefinitions, clientDefinitions ) => { } ); /** - * Memoized function to build an object mapping recoverable module slugs to their corresponding + * Gets a memoized object mapping recoverable module slugs to their corresponding * module objects. * * @since n.e.x.t From eb3c10f136bf4a2f95345141c3053d2f9ffbcc54 Mon Sep 17 00:00:00 2001 From: Tom Rees-Herdman Date: Wed, 29 Jun 2022 11:34:48 +0100 Subject: [PATCH 17/18] Add test case for not rendering recoverable modules in non view-only mode. --- .../widgets/components/WidgetRenderer.test.js | 30 +++++++++++++++++-- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/assets/js/googlesitekit/widgets/components/WidgetRenderer.test.js b/assets/js/googlesitekit/widgets/components/WidgetRenderer.test.js index d9ea846bad3..aef076268a7 100644 --- a/assets/js/googlesitekit/widgets/components/WidgetRenderer.test.js +++ b/assets/js/googlesitekit/widgets/components/WidgetRenderer.test.js @@ -20,7 +20,10 @@ * Internal dependencies */ import WidgetRenderer from './WidgetRenderer'; -import { VIEW_CONTEXT_DASHBOARD_VIEW_ONLY } from '../../../googlesitekit/constants'; +import { + VIEW_CONTEXT_DASHBOARD, + VIEW_CONTEXT_DASHBOARD_VIEW_ONLY, +} from '../../../googlesitekit/constants'; import { CORE_MODULES } from '../../modules/datastore/constants'; import { CORE_WIDGETS } from '../datastore/constants'; import { provideModules, render } from '../../../../../tests/js/test-utils'; @@ -90,7 +93,7 @@ describe( 'WidgetRenderer', () => { expect( container.firstChild ).toEqual( null ); } ); - it( 'should output the recoverable modules component when the widget depends on a recoverable module', async () => { + it( 'should output the recoverable modules component when the widget depends on a recoverable module in view-only mode', async () => { const { getByText } = render( , { setupRegistry: setupRegistry( { recoverableModules: [ 'search-console' ], @@ -106,7 +109,7 @@ describe( 'WidgetRenderer', () => { ).toBeInTheDocument(); } ); - it( 'should output the recoverable modules component when the widget depends on multiple recoverable modules', async () => { + it( 'should output the recoverable modules component when the widget depends on multiple recoverable modules in view-only mode', async () => { const { getByText } = render( , { setupRegistry: setupRegistry( { recoverableModules: [ 'search-console', 'pagespeed-insights' ], @@ -121,4 +124,25 @@ describe( 'WidgetRenderer', () => { ) ).toBeInTheDocument(); } ); + + it( 'should not output the recoverable modules component when the widget depends on a recoverable module and is not in view-only mode ', async () => { + const { getByText, queryByText } = render( + , + { + setupRegistry: setupRegistry( { + recoverableModules: [ 'search-console' ], + } ), + viewContext: VIEW_CONTEXT_DASHBOARD, + features: [ 'dashboardSharing' ], + } + ); + + expect( + queryByText( + /Search Console data was previously shared by an admin who no longer has access/ + ) + ).toBeNull(); + + expect( getByText( 'Test' ) ).toBeInTheDocument(); + } ); } ); From 7f824d15e459c16672276dd819c14ac0e4abe948 Mon Sep 17 00:00:00 2001 From: Tom Rees-Herdman Date: Wed, 29 Jun 2022 11:54:27 +0100 Subject: [PATCH 18/18] Update moduleSlug serialization, preventing unwanted mutation. --- .../widgets/components/WidgetRecoverableModules.js | 2 +- .../js/googlesitekit/widgets/components/WidgetRenderer.test.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/assets/js/googlesitekit/widgets/components/WidgetRecoverableModules.js b/assets/js/googlesitekit/widgets/components/WidgetRecoverableModules.js index 42783ada131..0668faeba56 100644 --- a/assets/js/googlesitekit/widgets/components/WidgetRecoverableModules.js +++ b/assets/js/googlesitekit/widgets/components/WidgetRecoverableModules.js @@ -43,7 +43,7 @@ export default function WidgetRecoverableModules( { // Here we serialize to `moduleSlug` for compatibility with the logic in // `combineWidgets()`. In future we may wish to take a less "hacky" approach. // See https://github.com/google/site-kit-wp/issues/5376#issuecomment-1165771399. - moduleSlug: JSON.stringify( moduleSlugs.sort() ), + moduleSlug: [ ...moduleSlugs ].sort().join( ',' ), // We also store `moduleSlugs` in the metadata in order for it to be passed back // into RecoverableModules as a prop. // See https://github.com/google/site-kit-wp/blob/c272c20eddcca61aae24c9812b6b11dbc15ec673/assets/js/googlesitekit/widgets/components/WidgetAreaRenderer.js#L171. diff --git a/assets/js/googlesitekit/widgets/components/WidgetRenderer.test.js b/assets/js/googlesitekit/widgets/components/WidgetRenderer.test.js index aef076268a7..c48b91d78df 100644 --- a/assets/js/googlesitekit/widgets/components/WidgetRenderer.test.js +++ b/assets/js/googlesitekit/widgets/components/WidgetRenderer.test.js @@ -120,7 +120,7 @@ describe( 'WidgetRenderer', () => { expect( getByText( - /The data for the following modules was previously shared by an admin who no longer has access: PageSpeed Insights, Search Console/ + /The data for the following modules was previously shared by an admin who no longer has access: Search Console, PageSpeed Insights/ ) ).toBeInTheDocument(); } );