Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhancement/9621 update siwg client id url #9689

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions assets/js/modules/service-urls.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,11 @@ it( 'ensures all serviceURLs are properly constructed', () => {
'https://search.google.com/search-console/foo-path',
'https://search.google.com/search-console/foo-path?bar=baz',
],
'modules/sign-in-with-google': [
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

'https://developers.google.com/identity/site-kit',
'https://developers.google.com/identity/site-kit#/foo-path',
'https://developers.google.com/identity/site-kit?bar=baz#/foo-path',
],
'modules/tagmanager': [
'https://tagmanager.google.com/',
'https://tagmanager.google.com/#/foo-path',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import { __ } from '@wordpress/i18n';
import { useSelect } from 'googlesitekit-data';
import StoreErrorNotices from '../../../../components/StoreErrorNotices';
import { MODULES_SIGN_IN_WITH_GOOGLE } from '../../datastore/constants';
import { CORE_SITE } from '../../../../googlesitekit/datastore/site/constants';
import ClientIDTextField from '../common/ClientIDTextField';
import { Button } from 'googlesitekit-components';
import SupportLink from '../../../../components/SupportLink';
Expand All @@ -40,10 +39,9 @@ const LazyGraphicSVG = lazy( () =>
);

export default function SetupForm() {
const siteName = useSelect( ( select ) =>
select( CORE_SITE ).getSiteName()
const clientIDURL = useSelect( ( select ) =>
select( MODULES_SIGN_IN_WITH_GOOGLE ).getClientIDURL()
);
const homeURL = useSelect( ( select ) => select( CORE_SITE ).getHomeURL() );

return (
<div className="googlesitekit-sign-in-with-google-setup__form">
Expand Down Expand Up @@ -74,9 +72,7 @@ export default function SetupForm() {
</div>
<Button
className="googlesitekit-sign-in-with-google-client-id-cta"
href={ `https://developers.google.com/web/site-kit?sitename=${ encodeURIComponent(
siteName
) }&siteurl=${ encodeURIComponent( homeURL ) }` }
href={ clientIDURL }
target="_blank"
trailingIcon={ <ExternalIcon width="15" height="15" /> }
inverse
Expand Down
3 changes: 2 additions & 1 deletion assets/js/modules/sign-in-with-google/datastore/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,9 @@
import { combineStores } from 'googlesitekit-data';
import { MODULES_SIGN_IN_WITH_GOOGLE } from './constants';
import baseModuleStore from './base';
import service from './service';

const store = combineStores( baseModuleStore );
const store = combineStores( baseModuleStore, service );

export const initialState = store.initialState;
export const actions = store.actions;
Expand Down
98 changes: 98 additions & 0 deletions assets/js/modules/sign-in-with-google/datastore/service.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
/**
* `modules/sign-in-with-google` data store: service.
*
* Site Kit by Google, Copyright 2024 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 { addQueryArgs } from '@wordpress/url';

/**
* Internal dependencies
*/
import { createRegistrySelector } from 'googlesitekit-data';
import { CORE_USER } from '../../../googlesitekit/datastore/user/constants';
import { CORE_SITE } from '../../../googlesitekit/datastore/site/constants';
import { MODULES_SIGN_IN_WITH_GOOGLE } from './constants';
import { untrailingslashit } from '../../../util';

export const selectors = {
/**
* Gets a URL to the service.
*
* @since n.e.x.t
*
* @param {Object} state Data store's state.
* @param {Object} [args] Object containing optional path and query args.
* @param {string} [args.path] A path to append to the base url.
* @param {Object} [args.query] Object of query params to be added to the URL.
* @return {(string|undefined)} The URL to the service, or `undefined` if not loaded.
*/
getServiceURL: createRegistrySelector(
( select ) =>
( state, { path, query } = {} ) => {
let serviceURL =
'https://developers.google.com/identity/site-kit';

if ( query ) {
serviceURL = addQueryArgs( serviceURL, query );
}

if ( path ) {
const sanitizedPath = `/${ path.replace( /^\//, '' ) }`;
serviceURL = `${ serviceURL }#${ sanitizedPath }`;
}

const accountChooserBaseURI =
select( CORE_USER ).getAccountChooserURL( serviceURL );

if ( accountChooserBaseURI === undefined ) {
return undefined;
}

return accountChooserBaseURI;
}
),
/**
* Gets a URL to fetch the client ID.
*
* @since n.e.x.t
*
* @param {Object} state Data store's state.
* @return {(string|undefined)} The URL to the clientID, or `undefined` if not loaded.
*/
getClientIDURL: createRegistrySelector( ( select ) => () => {
Comment on lines +71 to +78
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a bad idea to give this a dedicated selector but we generally name these using a getService*URL to indicate the URL is based on the serviceURL. Maybe getServiceClientIDProvisioningURL while a bit long would be much more clear :) WDYT?

const siteName = select( CORE_SITE ).getSiteName();
const homeURL = untrailingslashit( select( CORE_SITE ).getHomeURL() );
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The trailing slash is irrelevant now given we only need the origin which wouldn't include any part of the path.

const supportEmail = select( CORE_USER ).getEmail();

const query = {
appname: siteName,
sitename: siteName,
siteurl: homeURL,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the change here, but we need to pass the origin here instead. Essentially:

Suggested change
siteurl: homeURL,
siteorigin: new URL( homeURL ).origin,

supportemail: supportEmail,
};

return select( MODULES_SIGN_IN_WITH_GOOGLE ).getServiceURL( { query } );
} ),
};

const store = {
selectors,
};

export default store;
110 changes: 110 additions & 0 deletions assets/js/modules/sign-in-with-google/datastore/service.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
/**
* `modules/sign-in-with-google` data store: service tests.
*
* Site Kit by Google, Copyright 2024 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 { decodeServiceURL } from '../../../../../tests/js/mock-accountChooserURL-utils';
import {
createTestRegistry,
provideSiteInfo,
provideUserInfo,
} from '../../../../../tests/js/utils';
import { MODULES_SIGN_IN_WITH_GOOGLE } from './constants';

describe( 'module/sign-in-with-google service store', () => {
const baseURI = 'https://developers.google.com/identity/site-kit';

const userData = {
id: 1,
email: '[email protected]',
name: 'admin',
picture: 'https://path/to/image',
};

let registry;

beforeEach( () => {
registry = createTestRegistry();
provideUserInfo( registry, userData );
provideSiteInfo( registry );
} );

describe( 'selectors', () => {
describe( 'getServiceURL', () => {
it( 'retrieves the correct URL with no arguments', () => {
const serviceURL = registry
.select( MODULES_SIGN_IN_WITH_GOOGLE )
.getServiceURL();

expect( serviceURL ).toMatchInlineSnapshot(
'"https://accounts.google.com/accountchooser?continue=https%3A%2F%2Fdevelopers.google.com%2Fidentity%2Fsite-kit&Email=admin%40example.com"'
);
} );

it( 'adds the path parameter', () => {
const serviceURLNoSlashes = registry
.select( MODULES_SIGN_IN_WITH_GOOGLE )
.getServiceURL( { path: 'test/path/to/deeplink' } );

expect( serviceURLNoSlashes ).toMatchInlineSnapshot(
'"https://accounts.google.com/accountchooser?continue=https%3A%2F%2Fdevelopers.google.com%2Fidentity%2Fsite-kit%23%2Ftest%2Fpath%2Fto%2Fdeeplink&Email=admin%40example.com"'
);

const serviceURLWithLeadingSlash = registry
.select( MODULES_SIGN_IN_WITH_GOOGLE )
.getServiceURL( { path: '/test/path/to/deeplink' } );

expect( serviceURLWithLeadingSlash ).toMatchInlineSnapshot(
'"https://accounts.google.com/accountchooser?continue=https%3A%2F%2Fdevelopers.google.com%2Fidentity%2Fsite-kit%23%2Ftest%2Fpath%2Fto%2Fdeeplink&Email=admin%40example.com"'
);
} );

it( 'adds query args', () => {
const path = '/test/path/to/deeplink';
const query = {
authuser: userData.email,
param1: '1',
param2: '2',
};
const serviceURL = registry
.select( MODULES_SIGN_IN_WITH_GOOGLE )
.getServiceURL( { path, query } );
const decodedServiceURL = decodeServiceURL( serviceURL );

expect( decodedServiceURL.startsWith( baseURI ) ).toBe( true );
expect( decodedServiceURL.endsWith( `#${ path }` ) ).toBe(
true
);
expect( decodedServiceURL ).toMatchQueryParameters( query );
} );
} );
describe( 'getClientIDURL', () => {
it( 'retrieves the correct Client ID URL', () => {
const clientIDURL = registry
.select( MODULES_SIGN_IN_WITH_GOOGLE )
.getClientIDURL();

expect( clientIDURL ).toMatchInlineSnapshot(
'"https://accounts.google.com/accountchooser?continue=https%3A%2F%2Fdevelopers.google.com%2Fidentity%2Fsite-kit%3Fappname%3DMy%2520Site%2520Name%26sitename%3DMy%2520Site%2520Name%26siteurl%3Dhttp%253A%252F%252Fexample.com%26supportemail%3Dadmin%2540example.com&Email=admin%40example.com"'
);
} );
} );
} );
} );