From 8ae2c34ba6e25c5e655c38d4ef103648f31579ce Mon Sep 17 00:00:00 2001 From: sashadoes Date: Tue, 6 Dec 2022 19:39:44 +0000 Subject: [PATCH 1/5] Alter text for view-only users is ReportError. --- assets/js/components/ReportError.js | 63 ++++++++++++++------- assets/js/components/ReportError.stories.js | 22 ++++++- 2 files changed, 62 insertions(+), 23 deletions(-) diff --git a/assets/js/components/ReportError.js b/assets/js/components/ReportError.js index b9b77540c78..3dedadc9f21 100644 --- a/assets/js/components/ReportError.js +++ b/assets/js/components/ReportError.js @@ -44,10 +44,11 @@ import ErrorText from '../components/ErrorText'; import CTA from './notifications/CTA'; import Link from './Link'; import { CORE_SITE } from '../googlesitekit/datastore/site/constants'; - +import useViewOnly from '../hooks/useViewOnly'; const { useSelect, useDispatch } = Data; export default function ReportError( { moduleSlug, error } ) { + const isViewOnly = useViewOnly(); const module = useSelect( ( select ) => select( CORE_MODULES ).getModule( moduleSlug ) ); @@ -87,6 +88,21 @@ export default function ReportError( { moduleSlug, error } ) { let title; const getMessage = ( err ) => { + if ( isViewOnly ) { + title = sprintf( + /* translators: %s: module name */ + __( 'Access lost to %s', 'google-site-kit' ), + module?.name + ); + return sprintf( + /* translators: %s: module name */ + __( + 'The administrator sharing this module with you has lost access to the %s service, so you won’t be able to see stats from it on the Site Kit dashboard. You can contact them or another administrator to restore access.', + 'google-site-kit' + ), + module?.name + ); + } if ( isInsufficientPermissionsError( err ) ) { title = sprintf( /* translators: %s: module name */ @@ -166,30 +182,35 @@ export default function ReportError( { moduleSlug, error } ) { return ( -
- { showRequestAccessURL && ( - - ) } - { showRetry ? ( - - - - { __( 'Retry didn’t work?', 'google-site-kit' ) }{ ' ' } - + ) } + { showRetry ? ( + + + + { __( + 'Retry didn’t work?', + 'google-site-kit' + ) }{ ' ' } + + + { __( 'Get help', 'google-site-kit' ) } + + + ) : ( { __( 'Get help', 'google-site-kit' ) } - - ) : ( - - { __( 'Get help', 'google-site-kit' ) } - - ) } -
+ ) } + + ) } ); } diff --git a/assets/js/components/ReportError.stories.js b/assets/js/components/ReportError.stories.js index f4d13f4a675..4278052e28f 100644 --- a/assets/js/components/ReportError.stories.js +++ b/assets/js/components/ReportError.stories.js @@ -27,12 +27,21 @@ import { provideModuleRegistrations, } from '../../../tests/js/utils'; import WithRegistrySetup from '../../../tests/js/WithRegistrySetup'; +import { Provider as ViewContextProvider } from './Root/ViewContextContext'; import { ERROR_REASON_INSUFFICIENT_PERMISSIONS } from '../util/errors'; import { MODULES_ANALYTICS } from '../modules/analytics/datastore/constants'; +import { + VIEW_CONTEXT_MAIN_DASHBOARD, + VIEW_CONTEXT_MAIN_DASHBOARD_VIEW_ONLY, +} from '../googlesitekit/constants'; -const Template = ( { setupRegistry = () => {}, ...args } ) => ( +const Template = ( { setupRegistry = () => {}, viewContext, ...args } ) => ( - + + + ); @@ -243,6 +252,15 @@ MultipleUniqueReportErrorsWithRetryButtonWith.args = { ], }; +export const ReportErrorViewOnlyMode = Template.bind( {} ); +ReportErrorViewOnlyMode.storyName = 'ReportError with ViewOnly Mode'; +ReportErrorViewOnlyMode.args = { + error: { + code: 'test-error-code', + }, + viewContext: VIEW_CONTEXT_MAIN_DASHBOARD_VIEW_ONLY, +}; + export default { title: 'Components/ReportError', component: ReportError, From c96a13f54ae64158af2153d90cdf5a235147347c Mon Sep 17 00:00:00 2001 From: sashadoes Date: Fri, 9 Dec 2022 10:10:29 +0000 Subject: [PATCH 2/5] Update retry button render. --- assets/js/components/ReportError.js | 47 +++++++++++++---------------- 1 file changed, 21 insertions(+), 26 deletions(-) diff --git a/assets/js/components/ReportError.js b/assets/js/components/ReportError.js index 3dedadc9f21..e3dfdd48552 100644 --- a/assets/js/components/ReportError.js +++ b/assets/js/components/ReportError.js @@ -69,7 +69,7 @@ export default function ReportError( { moduleSlug, error } ) { err.selectorData.name === 'getReport' ); - const showRetry = !! retryableErrors.length; + const showRetry = !! retryableErrors.length && ! isViewOnly; const errorTroubleshootingLinkURL = useSelect( ( select ) => { const err = { @@ -182,35 +182,30 @@ export default function ReportError( { moduleSlug, error } ) { return ( - { ! isViewOnly && ( -
- { showRequestAccessURL && ( - + ) } + { showRetry ? ( + + - ) } - { showRetry ? ( - - - - { __( - 'Retry didn’t work?', - 'google-site-kit' - ) }{ ' ' } - - - { __( 'Get help', 'google-site-kit' ) } - - - ) : ( + + { __( 'Retry didn’t work?', 'google-site-kit' ) }{ ' ' } + { __( 'Get help', 'google-site-kit' ) } - ) } -
- ) } + + ) : ( + + { __( 'Get help', 'google-site-kit' ) } + + ) } + ); } From d531f2ade216c4bf4ff996d912551eee0227c726 Mon Sep 17 00:00:00 2001 From: sashadoes Date: Sun, 11 Dec 2022 17:38:04 +0000 Subject: [PATCH 3/5] Fix render logic. --- assets/js/components/ReportError.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assets/js/components/ReportError.js b/assets/js/components/ReportError.js index e3dfdd48552..25fb8071288 100644 --- a/assets/js/components/ReportError.js +++ b/assets/js/components/ReportError.js @@ -178,7 +178,7 @@ export default function ReportError( { moduleSlug, error } ) { }, [ dispatch, retryableErrors ] ); const showRequestAccessURL = - requestAccessURL && hasInsufficientPermissionsError; + requestAccessURL && hasInsufficientPermissionsError && ! isViewOnly; return ( From ee808c399b2e05df62d92a1371eed3a223ef2788 Mon Sep 17 00:00:00 2001 From: sashadoes Date: Mon, 12 Dec 2022 13:30:01 +0000 Subject: [PATCH 4/5] Add Unit test. --- assets/js/components/ReportError.test.js | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/assets/js/components/ReportError.test.js b/assets/js/components/ReportError.test.js index c6541deda02..b53853028a3 100644 --- a/assets/js/components/ReportError.test.js +++ b/assets/js/components/ReportError.test.js @@ -33,6 +33,7 @@ import { import { fireEvent, render } from '../../../tests/js/test-utils'; import ReportError from './ReportError'; import { MODULES_ANALYTICS } from '../modules/analytics/datastore/constants'; +import { VIEW_CONTEXT_MAIN_DASHBOARD_VIEW_ONLY } from '../googlesitekit/constants'; describe( 'ReportError', () => { let registry; @@ -337,6 +338,27 @@ describe( 'ReportError', () => { expect( queryByText( /retry/i ) ).not.toBeInTheDocument(); } ); + it( 'should not render the `Retry` if user is in view only mode', () => { + const { queryByText } = render( + , + { + registry, + }, + { + viewContext: VIEW_CONTEXT_MAIN_DASHBOARD_VIEW_ONLY, + } + ); + + expect( queryByText( /retry/i ) ).not.toBeInTheDocument(); + } ); + it( 'should render the `Retry` button if the error selector name is `getReport`', () => { const { getByRole } = render( Date: Tue, 13 Dec 2022 18:25:28 +0600 Subject: [PATCH 5/5] Update unit test. --- assets/js/components/ReportError.test.js | 40 +++++++++++++++--------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/assets/js/components/ReportError.test.js b/assets/js/components/ReportError.test.js index b53853028a3..d786b6f41c4 100644 --- a/assets/js/components/ReportError.test.js +++ b/assets/js/components/ReportError.test.js @@ -338,37 +338,46 @@ describe( 'ReportError', () => { expect( queryByText( /retry/i ) ).not.toBeInTheDocument(); } ); - it( 'should not render the `Retry` if user is in view only mode', () => { - const { queryByText } = render( + it( 'should render the `Retry` button if the error selector name is `getReport`', () => { + const { getByRole } = render( , { registry, - }, - { - viewContext: VIEW_CONTEXT_MAIN_DASHBOARD_VIEW_ONLY, } ); - expect( queryByText( /retry/i ) ).not.toBeInTheDocument(); + expect( getByRole( 'button', { name: /retry/i } ) ).toBeInTheDocument(); } ); - it( 'should render the `Retry` button if the error selector name is `getReport`', () => { - const { getByRole } = render( + it( 'should not render the `Retry` button for a view-only user', () => { + const { queryByText } = render( { />, { registry, + viewContext: VIEW_CONTEXT_MAIN_DASHBOARD_VIEW_ONLY, } ); - expect( getByRole( 'button', { name: /retry/i } ) ).toBeInTheDocument(); + expect( queryByText( /retry/i ) ).not.toBeInTheDocument(); } ); it( 'should dispatch the `invalidateResolution` action for each retry-able error', () => {