From 44d4ff6bd014a19bb493e5e9c8ed2aeb37a42988 Mon Sep 17 00:00:00 2001 From: Jimmy Madon Date: Mon, 15 Jul 2024 13:56:47 +0100 Subject: [PATCH 1/8] Display the admin column when recovering a module. --- .../DashboardSharingSettings/index.js | 29 ++++++++++++------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/assets/js/components/dashboard-sharing/DashboardSharingSettings/index.js b/assets/js/components/dashboard-sharing/DashboardSharingSettings/index.js index 9f5b9f03fc1..febbaeff4d4 100644 --- a/assets/js/components/dashboard-sharing/DashboardSharingSettings/index.js +++ b/assets/js/components/dashboard-sharing/DashboardSharingSettings/index.js @@ -39,10 +39,16 @@ import { } from '../../../googlesitekit/datastore/user/constants'; export default function DashboardSharingSettings() { + const hasRecoverableModules = useSelect( + ( select ) => !! select( CORE_MODULES ).getRecoverableModules() + ); + const hasMultipleAdmins = useSelect( ( select ) => select( CORE_SITE ).hasMultipleAdmins() ); + const showManageColumn = hasRecoverableModules || hasMultipleAdmins; + const sortedShareableModules = useSelect( ( select ) => { const userID = select( CORE_USER ).getID(); const shareableModules = select( CORE_MODULES ).getShareableModules(); @@ -79,7 +85,7 @@ export default function DashboardSharingSettings() { 'googlesitekit-dashboard-sharing-settings', { 'googlesitekit-dashboard-sharing-settings--has-multiple-admins': - hasMultipleAdmins, + showManageColumn, } ) } > @@ -91,7 +97,7 @@ export default function DashboardSharingSettings() { { __( 'Who can view', 'google-site-kit' ) } - { hasMultipleAdmins && ( + { showManageColumn && (
{ __( 'Who can manage view access', @@ -102,14 +108,17 @@ export default function DashboardSharingSettings() {
- { sortedShareableModules.map( ( { slug, name, owner } ) => ( - - ) ) } + { sortedShareableModules.map( + ( { slug, name, owner, recoverable } ) => ( + + ) + ) }
); From ac85c89d29c2bf3521c8b4669a5e36ce901ecf02 Mon Sep 17 00:00:00 2001 From: Jimmy Madon Date: Mon, 15 Jul 2024 13:57:13 +0100 Subject: [PATCH 2/8] Show warning notice when managing admin does not exist. --- .../DashboardSharingSettings/Module.js | 53 +++++++++++++++++-- 1 file changed, 50 insertions(+), 3 deletions(-) diff --git a/assets/js/components/dashboard-sharing/DashboardSharingSettings/Module.js b/assets/js/components/dashboard-sharing/DashboardSharingSettings/Module.js index a428230ebe6..8a53653da67 100644 --- a/assets/js/components/dashboard-sharing/DashboardSharingSettings/Module.js +++ b/assets/js/components/dashboard-sharing/DashboardSharingSettings/Module.js @@ -53,6 +53,8 @@ import { PERMISSION_DELEGATE_MODULE_SHARING_MANAGEMENT, PERMISSION_MANAGE_MODULE_SHARING_OPTIONS, } from '../../../googlesitekit/datastore/user/constants'; +import WarningNotice from '../../WarningNotice'; +import Link from '../../Link'; const viewAccessOptions = [ { @@ -65,14 +67,25 @@ const viewAccessOptions = [ }, ]; -export default function Module( { moduleSlug, moduleName, ownerUsername } ) { +export default function Module( { + moduleSlug, + moduleName, + ownerUsername, + recoverable, +} ) { const viewContext = useViewContext(); const moduleRef = useRef(); const [ manageViewAccess, setManageViewAccess ] = useState( undefined ); + + const hasRecoverableModules = useSelect( + ( select ) => !! select( CORE_MODULES ).getRecoverableModules() + ); const hasMultipleAdmins = useSelect( ( select ) => select( CORE_SITE ).hasMultipleAdmins() ); + const showManageColumn = hasRecoverableModules || hasMultipleAdmins; + const management = useSelect( ( select ) => select( CORE_MODULES ).getSharingManagement( moduleSlug ) ?? 'owner' @@ -99,6 +112,12 @@ export default function Module( { moduleSlug, moduleName, ownerUsername } ) { select( CORE_MODULES ).isDoingSubmitSharingChanges() ); + const recoverableModuleSupportLink = useSelect( ( select ) => + select( CORE_SITE ).getDocumentationLinkURL( + 'dashboard-sharing-module-recovery' + ) + ); + const { setSharingManagement } = useDispatch( CORE_MODULES ); const sharedOwnershipModule = @@ -180,7 +199,35 @@ export default function Module( { moduleSlug, moduleName, ownerUsername } ) { /> ) } - { ! hasSharingCapability && ( + { recoverable && ( + + { createInterpolateElement( + sprintf( + /* translators: 1: The warning message. 2: "Learn more" link. */ + __( + '%1$s. %2$s', + 'google-site-kit' + ), + __( + 'Managing user required to manage view access', + 'google-site-kit' + ), + __( 'Learn more', 'google-site-kit' ) + ), + { + Link: ( + + ), + } + ) } + + ) } + + { ! recoverable && ! hasSharingCapability && (

{ __( 'Contact managing user to manage view access', @@ -190,7 +237,7 @@ export default function Module( { moduleSlug, moduleName, ownerUsername } ) { ) } - { hasMultipleAdmins && ( + { showManageColumn && (

{ sharedOwnershipModule && (

From f46c5c4de5bd1b7911cf61c93063d9213c9fdce8 Mon Sep 17 00:00:00 2001 From: Jimmy Madon Date: Mon, 15 Jul 2024 13:57:37 +0100 Subject: [PATCH 3/8] Style the warning component specifically for dashboard sharing settings modal. --- .../global/_googlesitekit-sharing-settings.scss | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/assets/sass/components/global/_googlesitekit-sharing-settings.scss b/assets/sass/components/global/_googlesitekit-sharing-settings.scss index 42f3ca79977..e15c512d37a 100644 --- a/assets/sass/components/global/_googlesitekit-sharing-settings.scss +++ b/assets/sass/components/global/_googlesitekit-sharing-settings.scss @@ -164,6 +164,16 @@ display: flex; flex: 1 1 45%; } + + .googlesitekit-warning-notice { + margin-right: $grid-gap-desktop; + padding: $grid-gap-phone / 2 $grid-gap-phone; + + .googlesitekit-cta-link { + font-weight: $fw-normal; + text-decoration: underline; + } + } } .googlesitekit-dashboard-sharing-settings__row { From e38777b517a925f46f9bd71bb3ad86e023678e5c Mon Sep 17 00:00:00 2001 From: Jimmy Madon Date: Mon, 15 Jul 2024 16:20:12 +0100 Subject: [PATCH 4/8] Reorder variables for clarity. --- .../dashboard-sharing/DashboardSharingSettings/Module.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assets/js/components/dashboard-sharing/DashboardSharingSettings/Module.js b/assets/js/components/dashboard-sharing/DashboardSharingSettings/Module.js index 8a53653da67..8612c32d575 100644 --- a/assets/js/components/dashboard-sharing/DashboardSharingSettings/Module.js +++ b/assets/js/components/dashboard-sharing/DashboardSharingSettings/Module.js @@ -227,7 +227,7 @@ export default function Module( { ) } - { ! recoverable && ! hasSharingCapability && ( + { ! hasSharingCapability && ! recoverable && (

{ __( 'Contact managing user to manage view access', From 21a6e12f5143d84a6bebee18babf642755adbbe4 Mon Sep 17 00:00:00 2001 From: Jimmy Madon Date: Mon, 15 Jul 2024 16:21:36 +0100 Subject: [PATCH 5/8] Fix flaky JS tests. --- .../DashboardSharingSettings/index.test.js | 27 +++++++++++++------ 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/assets/js/components/dashboard-sharing/DashboardSharingSettings/index.test.js b/assets/js/components/dashboard-sharing/DashboardSharingSettings/index.test.js index 1dcf518fc36..74fea94f5b0 100644 --- a/assets/js/components/dashboard-sharing/DashboardSharingSettings/index.test.js +++ b/assets/js/components/dashboard-sharing/DashboardSharingSettings/index.test.js @@ -81,7 +81,7 @@ describe( 'DashboardSharingSettings', () => { expect( container ).toHaveTextContent( 'Search Console' ); } ); - it( 'should render the modules with user role select when the admin owns the modules', () => { + it( 'should render the modules with user role select when the admin owns the modules', async () => { act( () => { provideModules( registry, modules ); provideModuleRegistrations( registry ); @@ -107,9 +107,15 @@ describe( 'DashboardSharingSettings', () => { .dispatch( MODULES_SEARCH_CONSOLE ) .receiveGetSettings( { ownerID: 1 } ); } ); - const { container } = render( , { - registry, - } ); + + const { container, waitForRegistry } = render( + , + { + registry, + } + ); + + await waitForRegistry(); expect( container.querySelector( @@ -118,7 +124,7 @@ describe( 'DashboardSharingSettings', () => { ).toBeInTheDocument(); } ); - it( 'should not render sharing management for a single admin environment', () => { + it( 'should not render sharing management for a single admin environment', async () => { act( () => { provideModules( registry, modules ); provideModuleRegistrations( registry ); @@ -144,9 +150,14 @@ describe( 'DashboardSharingSettings', () => { .dispatch( MODULES_SEARCH_CONSOLE ) .receiveGetSettings( { ownerID: 1 } ); } ); - const { container } = render( , { - registry, - } ); + const { container, waitForRegistry } = render( + , + { + registry, + } + ); + + await waitForRegistry(); expect( container ).not.toHaveTextContent( 'Who can manage view access' From 74d468a6cfd8dc6f337ce2f79f71f1e56c6ca927 Mon Sep 17 00:00:00 2001 From: Jimmy Madon Date: Mon, 15 Jul 2024 18:12:54 +0100 Subject: [PATCH 6/8] Add a reusable selector to check if there are any recoverable modules. --- .../modules/datastore/modules.js | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/assets/js/googlesitekit/modules/datastore/modules.js b/assets/js/googlesitekit/modules/datastore/modules.js index 723dac94330..3a49c415b44 100644 --- a/assets/js/googlesitekit/modules/datastore/modules.js +++ b/assets/js/googlesitekit/modules/datastore/modules.js @@ -1319,6 +1319,25 @@ const baseSelectors = { return calculateRecoverableModules( modules, state.recoverableModules ); } ), + /** + * Checks if there are any recoverable modules for dashboard sharing. + * + * @since n.e.x.t + * + * @param {Object} state Data store's state. + * @return {(boolean|undefined)} `true` if there are recoverable modules. + * `false` if there are none. + * `undefined` if not loaded. + */ + hasRecoverableModules: ( state ) => { + // Return `undefined` if recoverableModules haven't been loaded yet. + if ( state.recoverableModules === undefined ) { + return undefined; + } + + return Object.keys( state.recoverableModules ).length > 0; + }, + /** * Gets the list of shared ownership modules for dashboard sharing. * From 01fd3fd1dc75a9bb5355ec2b4cd92b8fb40d3be1 Mon Sep 17 00:00:00 2001 From: Jimmy Madon Date: Mon, 15 Jul 2024 18:13:27 +0100 Subject: [PATCH 7/8] Refactor code to use the new selector. --- .../dashboard-sharing/DashboardSharingSettings/Module.js | 4 ++-- .../dashboard-sharing/DashboardSharingSettings/index.js | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/assets/js/components/dashboard-sharing/DashboardSharingSettings/Module.js b/assets/js/components/dashboard-sharing/DashboardSharingSettings/Module.js index 8612c32d575..f9f6f526a0a 100644 --- a/assets/js/components/dashboard-sharing/DashboardSharingSettings/Module.js +++ b/assets/js/components/dashboard-sharing/DashboardSharingSettings/Module.js @@ -78,8 +78,8 @@ export default function Module( { const [ manageViewAccess, setManageViewAccess ] = useState( undefined ); - const hasRecoverableModules = useSelect( - ( select ) => !! select( CORE_MODULES ).getRecoverableModules() + const hasRecoverableModules = useSelect( ( select ) => + select( CORE_MODULES ).hasRecoverableModules() ); const hasMultipleAdmins = useSelect( ( select ) => select( CORE_SITE ).hasMultipleAdmins() diff --git a/assets/js/components/dashboard-sharing/DashboardSharingSettings/index.js b/assets/js/components/dashboard-sharing/DashboardSharingSettings/index.js index febbaeff4d4..cd9ba6c432d 100644 --- a/assets/js/components/dashboard-sharing/DashboardSharingSettings/index.js +++ b/assets/js/components/dashboard-sharing/DashboardSharingSettings/index.js @@ -39,8 +39,8 @@ import { } from '../../../googlesitekit/datastore/user/constants'; export default function DashboardSharingSettings() { - const hasRecoverableModules = useSelect( - ( select ) => !! select( CORE_MODULES ).getRecoverableModules() + const hasRecoverableModules = useSelect( ( select ) => + select( CORE_MODULES ).hasRecoverableModules() ); const hasMultipleAdmins = useSelect( ( select ) => From 6979a49ca14dab6859e1b32c49cc4c4ad4dc0f47 Mon Sep 17 00:00:00 2001 From: Jimmy Madon Date: Thu, 25 Jul 2024 14:30:42 +0100 Subject: [PATCH 8/8] Consolidate a translatable string. --- .../DashboardSharingSettings/Module.js | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/assets/js/components/dashboard-sharing/DashboardSharingSettings/Module.js b/assets/js/components/dashboard-sharing/DashboardSharingSettings/Module.js index f9f6f526a0a..d9bdfe1877e 100644 --- a/assets/js/components/dashboard-sharing/DashboardSharingSettings/Module.js +++ b/assets/js/components/dashboard-sharing/DashboardSharingSettings/Module.js @@ -202,17 +202,9 @@ export default function Module( { { recoverable && ( { createInterpolateElement( - sprintf( - /* translators: 1: The warning message. 2: "Learn more" link. */ - __( - '%1$s. %2$s', - 'google-site-kit' - ), - __( - 'Managing user required to manage view access', - 'google-site-kit' - ), - __( 'Learn more', 'google-site-kit' ) + __( + 'Managing user required to manage view access. Learn more', + 'google-site-kit' ), { Link: (