From 9b779a3da96e8f8175862992326a3e31a3349fc0 Mon Sep 17 00:00:00 2001 From: Arafat Zahan Date: Tue, 12 Apr 2022 17:11:54 +0600 Subject: [PATCH 1/7] Update ErrorNotifications component. --- assets/js/components/notifications/ErrorNotifications.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/assets/js/components/notifications/ErrorNotifications.js b/assets/js/components/notifications/ErrorNotifications.js index 255a31ca979..1ef7bae82bd 100644 --- a/assets/js/components/notifications/ErrorNotifications.js +++ b/assets/js/components/notifications/ErrorNotifications.js @@ -24,16 +24,22 @@ import { Fragment } from '@wordpress/element'; /** * Internal dependencies */ +import Data from 'googlesitekit-data'; import AuthError from './AuthError'; import UnsatisfiedScopesAlert from './UnsatisfiedScopesAlert'; import InternalServerError from './InternalServerError'; +import { CORE_USER } from '../../googlesitekit/datastore/user/constants'; +const { useSelect } = Data; export default function ErrorNotifications() { + const isAuthenticated = useSelect( ( select ) => + select( CORE_USER ).isAuthenticated() + ); return ( - + { isAuthenticated && } ); } From 37101d9a4f729a006ab99a8ae4157732041fa8d7 Mon Sep 17 00:00:00 2001 From: Arafat Zahan Date: Tue, 12 Apr 2022 17:26:05 +0600 Subject: [PATCH 2/7] Update PermissionsModal component. --- assets/js/components/PermissionsModal/index.js | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/assets/js/components/PermissionsModal/index.js b/assets/js/components/PermissionsModal/index.js index f3e0bec8250..20ddefc3653 100644 --- a/assets/js/components/PermissionsModal/index.js +++ b/assets/js/components/PermissionsModal/index.js @@ -33,7 +33,7 @@ import Dialog from '../Dialog'; import Portal from '../Portal'; const { useSelect, useDispatch, useRegistry } = Data; -const PermissionsModal = () => { +const AuthenticatedPermissionsModal = () => { const registry = useRegistry(); const permissionsError = useSelect( ( select ) => select( CORE_USER ).getPermissionScopeError() @@ -109,4 +109,16 @@ const PermissionsModal = () => { ); }; +const PermissionsModal = () => { + const isAuthenticated = useSelect( ( select ) => + select( CORE_USER ).isAuthenticated() + ); + + if ( isAuthenticated ) { + return ; + } + + return null; +}; + export default PermissionsModal; From fa5b15659fb0fb71ac205ea83d33e9d66ed1c1fb Mon Sep 17 00:00:00 2001 From: Arafat Zahan Date: Tue, 12 Apr 2022 17:54:00 +0600 Subject: [PATCH 3/7] Split component to separate file. --- .../AuthenticatedPermissionsModal.js | 112 ++++++++++++++++++ .../js/components/PermissionsModal/index.js | 89 +------------- 2 files changed, 114 insertions(+), 87 deletions(-) create mode 100644 assets/js/components/PermissionsModal/AuthenticatedPermissionsModal.js diff --git a/assets/js/components/PermissionsModal/AuthenticatedPermissionsModal.js b/assets/js/components/PermissionsModal/AuthenticatedPermissionsModal.js new file mode 100644 index 00000000000..12314288d18 --- /dev/null +++ b/assets/js/components/PermissionsModal/AuthenticatedPermissionsModal.js @@ -0,0 +1,112 @@ +/** + * AuthenticatedPermissionsModal component. + * + * Site Kit by Google, Copyright 2021 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. + */ + +/** + * WordPress dependencies + */ +import { __ } from '@wordpress/i18n'; +import { useEffect, useCallback } from '@wordpress/element'; + +/** + * Internal dependencies + */ +import Data from 'googlesitekit-data'; +import { CORE_USER } from '../../googlesitekit/datastore/user/constants'; +import { CORE_LOCATION } from '../../googlesitekit/datastore/location/constants'; +import { snapshotAllStores } from '../../googlesitekit/data/create-snapshot-store'; +import Dialog from '../Dialog'; +import Portal from '../Portal'; +const { useSelect, useDispatch, useRegistry } = Data; + +const AuthenticatedPermissionsModal = () => { + const registry = useRegistry(); + const permissionsError = useSelect( ( select ) => + select( CORE_USER ).getPermissionScopeError() + ); + const connectURL = useSelect( ( select ) => + select( CORE_USER ).getConnectURL( { + additionalScopes: permissionsError?.data?.scopes, + redirectURL: global.location.href, + } ) + ); + + const { clearPermissionScopeError } = useDispatch( CORE_USER ); + const { navigateTo } = useDispatch( CORE_LOCATION ); + + const onCancel = useCallback( () => { + clearPermissionScopeError(); + }, [ clearPermissionScopeError ] ); + + const onConfirm = useCallback( async () => { + // If we have a datastores to snapshot before navigating away to the + // authorization page, do that first. + await snapshotAllStores( registry ); + navigateTo( connectURL ); + }, [ registry, connectURL, navigateTo ] ); + + useEffect( () => { + // If error has flag to skip the modal, redirect to the authorization + // page immediately without prompting the user, essentially short- + // circuiting to the confirm step. + const confirmIfSkipModal = async () => { + if ( + permissionsError?.data?.skipModal && + permissionsError?.data?.scopes?.length + ) { + await onConfirm(); + } + }; + confirmIfSkipModal(); + }, [ onConfirm, permissionsError ] ); + + if ( ! permissionsError ) { + return null; + } + + // If there aren't any scopes for us to request, there's no reason to show + // the modal. Log a console warning if this happens and return `null`. + if ( ! permissionsError?.data?.scopes?.length ) { + global.console.warn( + 'permissionsError lacks scopes array to use for redirect, so not showing the PermissionsModal. permissionsError was:', + permissionsError + ); + return null; + } + + if ( permissionsError?.data?.skipModal ) { + return null; + } + + return ( + + + + ); +}; + +export default AuthenticatedPermissionsModal; diff --git a/assets/js/components/PermissionsModal/index.js b/assets/js/components/PermissionsModal/index.js index 20ddefc3653..ac7ac2afe3b 100644 --- a/assets/js/components/PermissionsModal/index.js +++ b/assets/js/components/PermissionsModal/index.js @@ -16,98 +16,13 @@ * limitations under the License. */ -/** - * WordPress dependencies - */ -import { __ } from '@wordpress/i18n'; -import { useEffect, useCallback } from '@wordpress/element'; - /** * Internal dependencies */ import Data from 'googlesitekit-data'; +import AuthenticatedPermissionsModal from './AuthenticatedPermissionsModal'; import { CORE_USER } from '../../googlesitekit/datastore/user/constants'; -import { CORE_LOCATION } from '../../googlesitekit/datastore/location/constants'; -import { snapshotAllStores } from '../../googlesitekit/data/create-snapshot-store'; -import Dialog from '../Dialog'; -import Portal from '../Portal'; -const { useSelect, useDispatch, useRegistry } = Data; - -const AuthenticatedPermissionsModal = () => { - const registry = useRegistry(); - const permissionsError = useSelect( ( select ) => - select( CORE_USER ).getPermissionScopeError() - ); - const connectURL = useSelect( ( select ) => - select( CORE_USER ).getConnectURL( { - additionalScopes: permissionsError?.data?.scopes, - redirectURL: global.location.href, - } ) - ); - - const { clearPermissionScopeError } = useDispatch( CORE_USER ); - const { navigateTo } = useDispatch( CORE_LOCATION ); - - const onCancel = useCallback( () => { - clearPermissionScopeError(); - }, [ clearPermissionScopeError ] ); - - const onConfirm = useCallback( async () => { - // If we have a datastores to snapshot before navigating away to the - // authorization page, do that first. - await snapshotAllStores( registry ); - navigateTo( connectURL ); - }, [ registry, connectURL, navigateTo ] ); - - useEffect( () => { - // If error has flag to skip the modal, redirect to the authorization - // page immediately without prompting the user, essentially short- - // circuiting to the confirm step. - const confirmIfSkipModal = async () => { - if ( - permissionsError?.data?.skipModal && - permissionsError?.data?.scopes?.length - ) { - await onConfirm(); - } - }; - confirmIfSkipModal(); - }, [ onConfirm, permissionsError ] ); - - if ( ! permissionsError ) { - return null; - } - - // If there aren't any scopes for us to request, there's no reason to show - // the modal. Log a console warning if this happens and return `null`. - if ( ! permissionsError?.data?.scopes?.length ) { - global.console.warn( - 'permissionsError lacks scopes array to use for redirect, so not showing the PermissionsModal. permissionsError was:', - permissionsError - ); - return null; - } - - if ( permissionsError?.data?.skipModal ) { - return null; - } - - return ( - - - - ); -}; +const { useSelect } = Data; const PermissionsModal = () => { const isAuthenticated = useSelect( ( select ) => From 706694ba1f530de4519400ded02a49b2dd5da634 Mon Sep 17 00:00:00 2001 From: Arafat Zahan Date: Tue, 12 Apr 2022 18:45:27 +0600 Subject: [PATCH 4/7] Add PermissionsModal component test. --- .../components/PermissionsModal/index.test.js | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 assets/js/components/PermissionsModal/index.test.js diff --git a/assets/js/components/PermissionsModal/index.test.js b/assets/js/components/PermissionsModal/index.test.js new file mode 100644 index 00000000000..9b22fae4f48 --- /dev/null +++ b/assets/js/components/PermissionsModal/index.test.js @@ -0,0 +1,58 @@ +/** + * PermissionsModal component tests. + * + * 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. + */ + +/** + * Internal dependencies + */ +import PermissionsModal from './'; +import { + render, + createTestRegistry, + provideUserAuthentication, +} from '../../../../tests/js/test-utils'; +import { CORE_USER } from '../../googlesitekit/datastore/user/constants'; + +describe( 'PermissionsModal', () => { + let registry; + + beforeEach( () => { + registry = createTestRegistry(); + registry.dispatch( CORE_USER ).receiveConnectURL( 'test-url' ); + registry.dispatch( CORE_USER ).setPermissionScopeError( { + status: 500, + message: 'Bad', + data: { + scopes: [ + 'https://www.googleapis.com/auth/analytics.readonly', + ], + }, + } ); + } ); + + it( 'Does not render AuthenticatedPermissionsModal when user is not authenticated', () => { + provideUserAuthentication( registry, { authenticated: false } ); + const { baseElement } = render( , { registry } ); + expect( baseElement.childElementCount ).toBe( 1 ); + } ); + + it( 'Renders AuthenticatedPermissionsModal when user is authenticated', () => { + provideUserAuthentication( registry ); + const { baseElement } = render( , { registry } ); + expect( baseElement.childElementCount ).toBe( 2 ); + } ); +} ); From c38bf8194ddbf7e8ddd58f35e6363267d05a0633 Mon Sep 17 00:00:00 2001 From: Arafat Zahan Date: Tue, 12 Apr 2022 18:57:51 +0600 Subject: [PATCH 5/7] Add ErrorNotifications component test. --- .../notifications/ErrorNotifications.test.js | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 assets/js/components/notifications/ErrorNotifications.test.js diff --git a/assets/js/components/notifications/ErrorNotifications.test.js b/assets/js/components/notifications/ErrorNotifications.test.js new file mode 100644 index 00000000000..cfb01f30360 --- /dev/null +++ b/assets/js/components/notifications/ErrorNotifications.test.js @@ -0,0 +1,64 @@ +/** + * ErrorNotifications component tests. + * + * 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. + */ + +/** + * Internal dependencies + */ +import ErrorNotifications from './ErrorNotifications'; +import { + render, + createTestRegistry, + provideUserAuthentication, + provideModules, +} from '../../../../tests/js/test-utils'; +import { CORE_USER } from '../../googlesitekit/datastore/user/constants'; + +describe( 'ErrorNotifications', () => { + let registry; + + beforeEach( () => { + registry = createTestRegistry(); + provideModules( registry ); + registry.dispatch( CORE_USER ).receiveConnectURL( 'test-url' ); + } ); + + it( 'Does not render UnsatisfiedScopesAlert when user is not authenticated', () => { + provideUserAuthentication( registry, { + authenticated: false, + unsatisfiedScopes: [ + 'https://www.googleapis.com/auth/analytics.readonly', + ], + } ); + const { container } = render( , { + registry, + } ); + expect( container.childElementCount ).toBe( 0 ); + } ); + + it( 'Renders UnsatisfiedScopesAlert when user is authenticated', () => { + provideUserAuthentication( registry, { + unsatisfiedScopes: [ + 'https://www.googleapis.com/auth/analytics.readonly', + ], + } ); + const { container } = render( , { + registry, + } ); + expect( container.childElementCount ).toBe( 1 ); + } ); +} ); From adeb8bbd7d9ce55e36e25902ade1db6bbe0a7a0d Mon Sep 17 00:00:00 2001 From: "Matthew Riley MacPherson (tofumatt)" Date: Tue, 12 Apr 2022 17:07:56 +0100 Subject: [PATCH 6/7] Update the JSDoc for getPermissionScopeError. --- assets/js/googlesitekit/datastore/user/permissions.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assets/js/googlesitekit/datastore/user/permissions.js b/assets/js/googlesitekit/datastore/user/permissions.js index b12984b996e..1e164e6fab3 100644 --- a/assets/js/googlesitekit/datastore/user/permissions.js +++ b/assets/js/googlesitekit/datastore/user/permissions.js @@ -151,7 +151,7 @@ export const selectors = { * @private * * @param {Object} state Data store's state. - * @return {(Object|undefined)} Permission scope errors. Returns `null` if no error exists. + * @return {(Object|null)} Permission scope errors. Returns `null` if no error exists. */ getPermissionScopeError( state ) { const { permissionError } = state; From d6dfcfbad5909ab797d0c954de4d3048c95af7f9 Mon Sep 17 00:00:00 2001 From: "Matthew Riley MacPherson (tofumatt)" Date: Tue, 12 Apr 2022 19:46:53 +0100 Subject: [PATCH 7/7] Test for text content instead of element count. --- .../js/components/PermissionsModal/index.test.js | 14 ++++++++++---- .../notifications/ErrorNotifications.test.js | 9 ++++++--- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/assets/js/components/PermissionsModal/index.test.js b/assets/js/components/PermissionsModal/index.test.js index 9b22fae4f48..ef26accde24 100644 --- a/assets/js/components/PermissionsModal/index.test.js +++ b/assets/js/components/PermissionsModal/index.test.js @@ -44,15 +44,21 @@ describe( 'PermissionsModal', () => { } ); } ); - it( 'Does not render AuthenticatedPermissionsModal when user is not authenticated', () => { + it( 'does not render AuthenticatedPermissionsModal when user is not authenticated', () => { provideUserAuthentication( registry, { authenticated: false } ); const { baseElement } = render( , { registry } ); - expect( baseElement.childElementCount ).toBe( 1 ); + + expect( baseElement ).not.toHaveTextContent( + 'Additional Permissions Required' + ); } ); - it( 'Renders AuthenticatedPermissionsModal when user is authenticated', () => { + it( 'renders AuthenticatedPermissionsModal when user is authenticated', () => { provideUserAuthentication( registry ); const { baseElement } = render( , { registry } ); - expect( baseElement.childElementCount ).toBe( 2 ); + + expect( baseElement ).toHaveTextContent( + 'Additional Permissions Required' + ); } ); } ); diff --git a/assets/js/components/notifications/ErrorNotifications.test.js b/assets/js/components/notifications/ErrorNotifications.test.js index cfb01f30360..11eba05677e 100644 --- a/assets/js/components/notifications/ErrorNotifications.test.js +++ b/assets/js/components/notifications/ErrorNotifications.test.js @@ -37,7 +37,7 @@ describe( 'ErrorNotifications', () => { registry.dispatch( CORE_USER ).receiveConnectURL( 'test-url' ); } ); - it( 'Does not render UnsatisfiedScopesAlert when user is not authenticated', () => { + it( 'does not render UnsatisfiedScopesAlert when user is not authenticated', () => { provideUserAuthentication( registry, { authenticated: false, unsatisfiedScopes: [ @@ -50,7 +50,7 @@ describe( 'ErrorNotifications', () => { expect( container.childElementCount ).toBe( 0 ); } ); - it( 'Renders UnsatisfiedScopesAlert when user is authenticated', () => { + it( 'renders UnsatisfiedScopesAlert when user is authenticated', () => { provideUserAuthentication( registry, { unsatisfiedScopes: [ 'https://www.googleapis.com/auth/analytics.readonly', @@ -59,6 +59,9 @@ describe( 'ErrorNotifications', () => { const { container } = render( , { registry, } ); - expect( container.childElementCount ).toBe( 1 ); + + expect( container ).toHaveTextContent( + 'Site Kit can’t access necessary data' + ); } ); } );