From 3c8d922551411dcaeb02b5a3ea4dabfa6f5d57b0 Mon Sep 17 00:00:00 2001 From: Arafat Zahan Date: Wed, 1 Jun 2022 15:46:32 +0600 Subject: [PATCH 1/5] Show limited version of SearchFunnelWidget in Shared Dashboard when only search-console is shared. --- .../datastore/user/permissions.js | 32 +++++- .../dashboard/SearchFunnelWidget/Overview.js | 44 +++++--- .../dashboard/SearchFunnelWidget/index.js | 101 ++++++++++-------- assets/js/modules/search-console/index.js | 2 +- 4 files changed, 122 insertions(+), 57 deletions(-) diff --git a/assets/js/googlesitekit/datastore/user/permissions.js b/assets/js/googlesitekit/datastore/user/permissions.js index 5dbbb30e571..55426913021 100644 --- a/assets/js/googlesitekit/datastore/user/permissions.js +++ b/assets/js/googlesitekit/datastore/user/permissions.js @@ -187,7 +187,7 @@ export const selectors = { } // Return an array of module slugs for modules that are - // sharable and the user has the "read shared module data" + // shareable and the user has the "read shared module data" // capability for. return Object.values( modules ).reduce( ( moduleSlugs, module ) => { const hasCapability = select( CORE_USER ).hasCapability( @@ -231,6 +231,36 @@ export const selectors = { return undefined; } ), + + /** + * Checks if the specified module is shareable and viewable by the current user. + * + * @since n.e.x.t + * + * @param {Object} state Data store's state. + * @param {string} moduleSlug Module slug to check. + * @return {(boolean|undefined)} `true` if the module is shareable and viewable by the current user. + * `false` if the module does not exist, is not shareable or not viewable by the current user. + * `undefined` if state is not loaded yet. + */ + canViewSharedModule: createRegistrySelector( + ( select ) => ( state, moduleSlug ) => { + const module = select( CORE_MODULES ).getModule( moduleSlug ); + + if ( module === undefined ) { + return undefined; + } + + if ( module === null || ! module.shareable ) { + return false; + } + + return select( CORE_USER ).hasCapability( + PERMISSION_READ_SHARED_MODULE_DATA, + module.slug + ); + } + ), }; export default { diff --git a/assets/js/modules/search-console/components/dashboard/SearchFunnelWidget/Overview.js b/assets/js/modules/search-console/components/dashboard/SearchFunnelWidget/Overview.js index 4bd756b6404..0daf123396d 100644 --- a/assets/js/modules/search-console/components/dashboard/SearchFunnelWidget/Overview.js +++ b/assets/js/modules/search-console/components/dashboard/SearchFunnelWidget/Overview.js @@ -57,6 +57,8 @@ import { BREAKPOINT_SMALL, useBreakpoint, } from '../../../../../hooks/useBreakpoint'; +import useViewOnly from '../../../../../hooks/useViewOnly'; +import { CORE_USER } from '../../../../../googlesitekit/datastore/user/constants'; const { useSelect, useInViewSelect } = Data; function getDatapointAndChange( [ report ], selectedStat, divider = 1 ) { @@ -85,6 +87,16 @@ const Overview = ( { const zeroDataStatesEnabled = useFeature( 'zeroDataStates' ); const breakpoint = useBreakpoint(); + const viewOnly = useViewOnly(); + + const canViewSharedAnalytics = useSelect( ( select ) => { + if ( ! viewOnly ) { + return true; + } + + return select( CORE_USER ).canViewSharedModule( 'analytics' ); + } ); + const analyticsModuleConnected = useSelect( ( select ) => select( CORE_MODULES ).isModuleConnected( 'analytics' ) ); @@ -151,11 +163,15 @@ const Overview = ( { } const showAnalytics = - ( analyticsModuleConnected && + ( canViewSharedAnalytics && + analyticsModuleConnected && ! isAnalyticsGatheringData && ! zeroDataStatesEnabled && ! error ) || - ( analyticsModuleConnected && zeroDataStatesEnabled && ! error ); + ( canViewSharedAnalytics && + analyticsModuleConnected && + zeroDataStatesEnabled && + ! error ); const showGoalsCTA = showAnalytics && @@ -236,7 +252,8 @@ const Overview = ( { ) } - { ( ! analyticsModuleConnected || ! analyticsModuleActive ) && + { canViewSharedAnalytics && + ( ! analyticsModuleConnected || ! analyticsModuleActive ) && ! isNavigatingToReauthURL && ( { zeroDataStatesEnabled && @@ -252,16 +269,19 @@ const Overview = ( { ) } - { analyticsModuleActiveAndConnected && error && ( - - - - ) } + { canViewSharedAnalytics && + analyticsModuleActiveAndConnected && + error && ( + + + + ) } - { isAnalyticsGatheringData && + { canViewSharedAnalytics && + isAnalyticsGatheringData && ! error && ! zeroDataStatesEnabled && ( diff --git a/assets/js/modules/search-console/components/dashboard/SearchFunnelWidget/index.js b/assets/js/modules/search-console/components/dashboard/SearchFunnelWidget/index.js index cfb1e463ffa..4ffa4f21a55 100644 --- a/assets/js/modules/search-console/components/dashboard/SearchFunnelWidget/index.js +++ b/assets/js/modules/search-console/components/dashboard/SearchFunnelWidget/index.js @@ -60,6 +60,7 @@ import { BREAKPOINT_SMALL, useBreakpoint, } from '../../../../../hooks/useBreakpoint'; +import useViewOnly from '../../../../../hooks/useViewOnly'; const { useSelect, useInViewSelect } = Data; const SearchFunnelWidget = ( { @@ -73,6 +74,16 @@ const SearchFunnelWidget = ( { const breakpoint = useBreakpoint(); + const viewOnly = useViewOnly(); + + const canViewSharedAnalytics = useSelect( ( select ) => { + if ( ! viewOnly ) { + return true; + } + + return select( CORE_USER ).canViewSharedModule( 'analytics' ); + } ); + const isAnalyticsConnected = useSelect( ( select ) => select( CORE_MODULES ).isModuleConnected( 'analytics' ) ); @@ -317,23 +328,25 @@ const SearchFunnelWidget = ( { ) } - { ( ! isAnalyticsConnected || ! isAnalyticsActive ) && ( - - - - - - - { ! isAnalyticsActive && ( - - ) } - - { isAnalyticsActive && ! isAnalyticsConnected && ( - - ) } - - - ) } + { canViewSharedAnalytics && + ( ! isAnalyticsConnected || ! isAnalyticsActive ) && ( + + + + + + + { ! isAnalyticsActive && ( + + ) } + + { isAnalyticsActive && + ! isAnalyticsConnected && ( + + ) } + + + ) } ); } @@ -368,6 +381,7 @@ const SearchFunnelWidget = ( { ) } { zeroDataStatesEnabled && + canViewSharedAnalytics && ( ! isAnalyticsActive || ! isAnalyticsConnected ) && BREAKPOINT_SMALL === breakpoint && ( @@ -398,32 +412,33 @@ const SearchFunnelWidget = ( { /> ) } - { ( selectedStats === 3 || selectedStats === 4 ) && ( - parseFloat( x ).toLocaleString(), - ( x ) => - numFmt( x / 100, { - style: 'percent', - signDisplay: 'never', - maximumFractionDigits: 2, - } ), - ] } - statsColor={ - SearchFunnelWidget.metrics[ selectedStats ].color - } - gatheringData={ isAnalyticsGatheringData } - /> - ) } + { canViewSharedAnalytics && + ( selectedStats === 3 || selectedStats === 4 ) && ( + parseFloat( x ).toLocaleString(), + ( x ) => + numFmt( x / 100, { + style: 'percent', + signDisplay: 'never', + maximumFractionDigits: 2, + } ), + ] } + statsColor={ + SearchFunnelWidget.metrics[ selectedStats ].color + } + gatheringData={ isAnalyticsGatheringData } + /> + ) } ); }; diff --git a/assets/js/modules/search-console/index.js b/assets/js/modules/search-console/index.js index 43d240dbee7..0401c845715 100644 --- a/assets/js/modules/search-console/index.js +++ b/assets/js/modules/search-console/index.js @@ -159,7 +159,7 @@ export const registerWidgets = ( widgets ) => { width: [ widgets.WIDGET_WIDTHS.FULL ], priority: 3, wrapWidget: false, - modules: [ 'search-console', 'analytics' ], + modules: [ 'search-console' ], }, [ AREA_MAIN_DASHBOARD_TRAFFIC_PRIMARY, From 0a7c061746789093cda3d3849685fd78303c597d Mon Sep 17 00:00:00 2001 From: "Matthew Riley MacPherson (tofumatt)" Date: Wed, 1 Jun 2022 23:57:29 +0100 Subject: [PATCH 2/5] Fix spread error in Dashboard Sharing settings modal. --- assets/js/components/dashboard-sharing/UserRoleSelect.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/assets/js/components/dashboard-sharing/UserRoleSelect.js b/assets/js/components/dashboard-sharing/UserRoleSelect.js index a823cb69295..d110bbb04b4 100644 --- a/assets/js/components/dashboard-sharing/UserRoleSelect.js +++ b/assets/js/components/dashboard-sharing/UserRoleSelect.js @@ -81,7 +81,12 @@ export default function UserRoleSelect( { moduleSlug, isLocked = false } ) { const toggleEditMode = useCallback( () => { if ( editMode ) { - if ( ! isEqual( [ ...sharedRoles ].sort(), initialSharedRoles ) ) { + if ( + ! isEqual( + [ ...( sharedRoles || [] ) ].sort(), + initialSharedRoles + ) + ) { trackEvent( `${ viewContext }_sharing`, 'change_shared_roles', @@ -89,7 +94,7 @@ export default function UserRoleSelect( { moduleSlug, isLocked = false } ) { ); } } else { - setInitialSharedRoles( [ ...sharedRoles ].sort() ); + setInitialSharedRoles( [ ...( sharedRoles || [] ) ].sort() ); } setEditMode( ! editMode ); From d3e6f4d1da61a0f48fc406e0b2a09244f5799efe Mon Sep 17 00:00:00 2001 From: Arafat Zahan Date: Thu, 2 Jun 2022 12:51:13 +0600 Subject: [PATCH 3/5] Add tests for `canViewSharedModule` selector. --- .../datastore/user/permissions.test.js | 101 ++++++++++++++++++ 1 file changed, 101 insertions(+) diff --git a/assets/js/googlesitekit/datastore/user/permissions.test.js b/assets/js/googlesitekit/datastore/user/permissions.test.js index 727580ed0b7..3dd561f13eb 100644 --- a/assets/js/googlesitekit/datastore/user/permissions.test.js +++ b/assets/js/googlesitekit/datastore/user/permissions.test.js @@ -25,6 +25,7 @@ import { } from '../../../../../tests/js/utils'; import { CORE_USER, PERMISSION_MANAGE_OPTIONS } from './constants'; import FIXTURES from '../../modules/datastore/__fixtures__'; +import { CORE_MODULES } from '../../modules/datastore/constants'; describe( 'core/user authentication', () => { const capabilitiesBaseVar = '_googlesitekitUserData'; @@ -251,5 +252,105 @@ describe( 'core/user authentication', () => { ] ); } ); } ); + + describe( 'canViewSharedModule', () => { + it( 'should return undefined if modules are not loaded', () => { + fetchMock.getOnce( + /^\/google-site-kit\/v1\/core\/modules\/data\/list/, + { body: FIXTURES, status: 200 } + ); + + const canViewSharedModule = registry + .select( CORE_USER ) + .canViewSharedModule( 'search-console' ); + + expect( canViewSharedModule ).toBeUndefined(); + } ); + + it( 'should return FALSE if the module does not exist', () => { + registry + .dispatch( CORE_MODULES ) + .receiveGetModules( [ + { slug: 'search-console', name: 'Search Console' }, + ] ); + + const canViewSharedModule = registry + .select( CORE_USER ) + .canViewSharedModule( 'invalid-module' ); + + expect( canViewSharedModule ).toBe( false ); + } ); + + it( 'should return FALSE if the module is not shared', () => { + registry.dispatch( CORE_MODULES ).receiveGetModules( [ + { + slug: 'search-console', + name: 'Search Console', + shareable: false, + }, + ] ); + + const canViewSharedModule = registry + .select( CORE_USER ) + .canViewSharedModule( 'search-console' ); + + expect( canViewSharedModule ).toBe( false ); + } ); + + it( 'should return undefined if the capabilities are not loaded', () => { + registry.dispatch( CORE_MODULES ).receiveGetModules( [ + { + slug: 'search-console', + name: 'Search Console', + shareable: true, + }, + ] ); + global[ capabilitiesBaseVar ] = undefined; + + const canViewSharedModule = registry + .select( CORE_USER ) + .canViewSharedModule( 'search-console' ); + + expect( console ).toHaveErroredWith( + 'Could not load core/user permissions.' + ); + + expect( canViewSharedModule ).toBeUndefined(); + } ); + + it( 'should return FALSE if the module is shared but the user does not have the view permission', () => { + registry.dispatch( CORE_MODULES ).receiveGetModules( [ + { + slug: 'search-console', + name: 'Search Console', + shareable: true, + }, + ] ); + global[ capabilitiesBaseVar ] = capabilities; + + const canViewSharedModule = registry + .select( CORE_USER ) + .canViewSharedModule( 'search-console' ); + + expect( canViewSharedModule ).toBe( false ); + } ); + + it( 'should return TRUE if the module is shared and the user has the view permission', () => { + registry.dispatch( CORE_MODULES ).receiveGetModules( [ + { + slug: 'search-console', + name: 'Search Console', + shareable: true, + }, + ] ); + global[ capabilitiesBaseVar ] = capabilitiesWithPermission; + + const canViewSharedModule = registry + .select( CORE_USER ) + .canViewSharedModule( 'search-console' ); + + expect( canViewSharedModule ).toBe( true ); + } ); + } ); } ); } ); From aaf07ebd5094deb1e6a8a3df8f0c35ab32f753a7 Mon Sep 17 00:00:00 2001 From: Arafat Zahan Date: Thu, 2 Jun 2022 15:23:58 +0600 Subject: [PATCH 4/5] Add a story for viewOnly Search Console Only Search Funnel Widget. --- .../SearchFunnelWidget/index.stories.js | 136 +++++++++++------- 1 file changed, 88 insertions(+), 48 deletions(-) diff --git a/assets/js/modules/search-console/components/dashboard/SearchFunnelWidget/index.stories.js b/assets/js/modules/search-console/components/dashboard/SearchFunnelWidget/index.stories.js index 5c21bb85095..8cd9568dd53 100644 --- a/assets/js/modules/search-console/components/dashboard/SearchFunnelWidget/index.stories.js +++ b/assets/js/modules/search-console/components/dashboard/SearchFunnelWidget/index.stories.js @@ -19,25 +19,33 @@ /** * Internal dependencies */ -import { CORE_USER } from '../../../../../googlesitekit/datastore/user/constants'; import { + createTestRegistry, + provideModuleRegistrations, provideModules, provideSiteInfo, - provideModuleRegistrations, + provideUserCapabilities, + WithTestRegistry, } from '../../../../../../../tests/js/utils'; -import { MODULES_SEARCH_CONSOLE } from '../../../datastore/constants'; -import { MODULES_ANALYTICS } from '../../../../analytics/datastore/constants'; -import { withWidgetComponentProps } from '../../../../../googlesitekit/widgets/util'; -import { provideSearchConsoleMockReport } from '../../../util/data-mock'; -import { provideAnalyticsMockReport } from '../../../../analytics/util/data-mock'; -import { goals } from '../../../../analytics/datastore/__fixtures__'; +import WithRegistrySetup from '../../../../../../../tests/js/WithRegistrySetup'; +import { Provider as ViewContextProvider } from '../../../../../components/Root/ViewContextContext'; import { VIEW_CONTEXT_DASHBOARD, + VIEW_CONTEXT_DASHBOARD_VIEW_ONLY, VIEW_CONTEXT_PAGE_DASHBOARD, } from '../../../../../googlesitekit/constants'; -import WithRegistrySetup from '../../../../../../../tests/js/WithRegistrySetup'; +import { + CORE_USER, + PERMISSION_READ_SHARED_MODULE_DATA, +} from '../../../../../googlesitekit/datastore/user/constants'; +import { getMetaCapabilityPropertyName } from '../../../../../googlesitekit/datastore/util/permissions'; +import { withWidgetComponentProps } from '../../../../../googlesitekit/widgets/util'; +import { MODULES_ANALYTICS } from '../../../../analytics/datastore/constants'; +import { goals } from '../../../../analytics/datastore/__fixtures__'; +import { provideAnalyticsMockReport } from '../../../../analytics/util/data-mock'; +import { MODULES_SEARCH_CONSOLE } from '../../../datastore/constants'; +import { provideSearchConsoleMockReport } from '../../../util/data-mock'; import SearchFunnelWidget from './index'; -import { Provider } from '../../../../../components/Root/ViewContextContext'; const searchConsoleArgs = { startDate: '2021-08-18', @@ -116,9 +124,11 @@ const WidgetWithComponentProps = withWidgetComponentProps( 'widget-slug' )( SearchFunnelWidget ); -const Template = ( { setupRegistry, ...args } ) => ( +const Template = ( { setupRegistry = () => {}, viewContext, ...args } ) => ( - + + + ); @@ -360,16 +370,49 @@ ReadyEntityDashboard.args = { provideAnalyticsMockReport( registry, options ); } }, + viewContext: VIEW_CONTEXT_PAGE_DASHBOARD, }; -ReadyEntityDashboard.decorators = [ - ( Story ) => { - return ( - - - - ); + +export const ViewOnlySearchConsoleOnlyReady = Template.bind( {} ); +ViewOnlySearchConsoleOnlyReady.storyName = + 'ViewOnly - Only Search Console Shared - Ready'; +ViewOnlySearchConsoleOnlyReady.args = { + setupRegistry: ( registry ) => { + provideModules( registry, [ + { + active: true, + connected: true, + slug: 'search-console', + shareable: true, + }, + { + active: true, + connected: true, + slug: 'analytics', + shareable: false, + }, + ] ); + provideUserCapabilities( registry, { + [ getMetaCapabilityPropertyName( + PERMISSION_READ_SHARED_MODULE_DATA, + 'search-console' + ) ]: true, + [ getMetaCapabilityPropertyName( + PERMISSION_READ_SHARED_MODULE_DATA, + 'analytics' + ) ]: false, + } ); + provideSearchConsoleMockReport( registry, searchConsoleArgs ); + for ( const options of analyticsArgs ) { + provideAnalyticsMockReport( registry, options ); + } + registry.dispatch( MODULES_ANALYTICS ).receiveGetGoals( goals ); }, -]; + viewContext: VIEW_CONTEXT_DASHBOARD_VIEW_ONLY, +}; +ViewOnlySearchConsoleOnlyReady.parameters = { + features: [ 'dashboardSharing' ], +}; export default { title: 'Modules/SearchConsole/Widgets/SearchFunnelWidget', @@ -381,37 +424,34 @@ export default { ), - ( Story, { args } ) => { - const setupRegistry = ( registry ) => { - provideSiteInfo( registry ); - registry.dispatch( CORE_USER ).setReferenceDate( '2021-10-13' ); - registry.dispatch( CORE_USER ).receiveGetAuthentication( { - needsReauthentication: false, - } ); - - provideModules( registry, [ - { - active: true, - connected: true, - slug: 'search-console', - }, - { - active: true, - connected: true, - slug: 'analytics', - }, - ] ); + ( Story, { parameters } ) => { + const registry = createTestRegistry(); + provideSiteInfo( registry ); + registry.dispatch( CORE_USER ).setReferenceDate( '2021-10-13' ); + registry.dispatch( CORE_USER ).receiveGetAuthentication( { + needsReauthentication: false, + } ); - // Call story-specific setup. - args.setupRegistry( registry ); - }; + provideModules( registry, [ + { + active: true, + connected: true, + slug: 'search-console', + }, + { + active: true, + connected: true, + slug: 'analytics', + }, + ] ); return ( - - - - - + + + ); }, ], From 133286da2e1c1f8680a83a62a3944351fdb81311 Mon Sep 17 00:00:00 2001 From: "Matthew Riley MacPherson (tofumatt)" Date: Thu, 2 Jun 2022 20:54:19 +0100 Subject: [PATCH 5/5] Adjust alignment of JSDoc. --- assets/js/googlesitekit/datastore/user/permissions.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/assets/js/googlesitekit/datastore/user/permissions.js b/assets/js/googlesitekit/datastore/user/permissions.js index 55426913021..395f99ff182 100644 --- a/assets/js/googlesitekit/datastore/user/permissions.js +++ b/assets/js/googlesitekit/datastore/user/permissions.js @@ -239,9 +239,7 @@ export const selectors = { * * @param {Object} state Data store's state. * @param {string} moduleSlug Module slug to check. - * @return {(boolean|undefined)} `true` if the module is shareable and viewable by the current user. - * `false` if the module does not exist, is not shareable or not viewable by the current user. - * `undefined` if state is not loaded yet. + * @return {(boolean|undefined)} `true` if the module is shareable and viewable by the current user. `false` if the module does not exist, is not shareable or not viewable by the current user. `undefined` if state is not loaded yet. */ canViewSharedModule: createRegistrySelector( ( select ) => ( state, moduleSlug ) => {