From 7fb125ac0741453659df17bcc1f41781c9673b30 Mon Sep 17 00:00:00 2001 From: blam Date: Thu, 13 Jan 2022 17:34:59 +0100 Subject: [PATCH] chore: simplify the contract between the different provider pickers Signed-off-by: blam --- .../fields/RepoUrlPicker/AzureRepoPicker.tsx | 31 ++++---- .../RepoUrlPicker/BitbucketRepoPicker.tsx | 26 +++---- .../RepoUrlPicker/GithubRepoPicker.test.tsx | 42 +++++------ .../fields/RepoUrlPicker/GithubRepoPicker.tsx | 23 +++--- .../fields/RepoUrlPicker/GitlabRepoPicker.tsx | 25 ++++--- .../fields/RepoUrlPicker/RepoUrlPicker.tsx | 72 ++++++------------- .../components/fields/RepoUrlPicker/types.ts | 23 ++++++ .../fields/RepoUrlPicker/utils.test.ts | 6 +- .../components/fields/RepoUrlPicker/utils.ts | 23 +++--- 9 files changed, 121 insertions(+), 150 deletions(-) create mode 100644 plugins/scaffolder/src/components/fields/RepoUrlPicker/types.ts diff --git a/plugins/scaffolder/src/components/fields/RepoUrlPicker/AzureRepoPicker.tsx b/plugins/scaffolder/src/components/fields/RepoUrlPicker/AzureRepoPicker.tsx index df6b105368..1a61dad251 100644 --- a/plugins/scaffolder/src/components/fields/RepoUrlPicker/AzureRepoPicker.tsx +++ b/plugins/scaffolder/src/components/fields/RepoUrlPicker/AzureRepoPicker.tsx @@ -1,5 +1,5 @@ /* - * Copyright 2021 The Backstage Authors + * Copyright 2022 The Backstage Authors * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -13,41 +13,36 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + import React from 'react'; import FormControl from '@material-ui/core/FormControl'; import FormHelperText from '@material-ui/core/FormHelperText'; import Input from '@material-ui/core/Input'; import InputLabel from '@material-ui/core/InputLabel'; +import { RepoUrlPickerState } from './types'; export const AzureRepoPicker = ({ - onOrgChange, - onOwnerChange, - onRepoNameChange, rawErrors, - org, - owner, - repoName, + state, + onChange, }: { - onOrgChange: (org: string) => void; - onOwnerChange: (owner: string) => void; - onRepoNameChange: (name: string) => void; - owner?: string; - org?: string; - repoName?: string; + state: RepoUrlPickerState; + onChange: (state: RepoUrlPickerState) => void; rawErrors: string[]; }) => { + const { organization, repoName, owner } = state; return ( <> 0 && !org} + error={rawErrors?.length > 0 && !organization} > Organization onOrgChange(e.target.value)} - value={org} + onChange={e => onChange({ organization: e.target.value })} + value={organization} /> The organization that this repo will belong to @@ -61,7 +56,7 @@ export const AzureRepoPicker = ({ Owner onOwnerChange(e.target.value)} + onChange={e => onChange({ owner: e.target.value })} value={owner} /> The Owner that this repo will belong to @@ -74,7 +69,7 @@ export const AzureRepoPicker = ({ Repository onRepoNameChange(e.target.value)} + onChange={e => onChange({ repoName: e.target.value })} value={repoName} /> The name of the repository diff --git a/plugins/scaffolder/src/components/fields/RepoUrlPicker/BitbucketRepoPicker.tsx b/plugins/scaffolder/src/components/fields/RepoUrlPicker/BitbucketRepoPicker.tsx index 7712a490b2..1240a4e15b 100644 --- a/plugins/scaffolder/src/components/fields/RepoUrlPicker/BitbucketRepoPicker.tsx +++ b/plugins/scaffolder/src/components/fields/RepoUrlPicker/BitbucketRepoPicker.tsx @@ -18,26 +18,18 @@ import FormControl from '@material-ui/core/FormControl'; import FormHelperText from '@material-ui/core/FormHelperText'; import Input from '@material-ui/core/Input'; import InputLabel from '@material-ui/core/InputLabel'; +import { RepoUrlPickerState } from './types'; export const BitbucketRepoPicker = ({ - onProjectChange, - onWorkspaceChange, - onRepoNameChange, + onChange, rawErrors, - workspace, - project, - host, - repoName, + state, }: { - onProjectChange: (owner: string) => void; - onWorkspaceChange: (name: string) => void; - onRepoNameChange: (name: string) => void; - workspace?: string; - project?: string; - repoName?: string; - host: string; + onChange: (state: RepoUrlPickerState) => void; + state: RepoUrlPickerState; rawErrors: string[]; }) => { + const { host, workspace, project, repoName } = state; return ( <> {host === 'bitbucket.org' && ( @@ -49,7 +41,7 @@ export const BitbucketRepoPicker = ({ Workspace onWorkspaceChange(e.target.value)} + onChange={e => onChange({ workspace: e.target.value })} value={workspace} /> @@ -65,7 +57,7 @@ export const BitbucketRepoPicker = ({ Project onProjectChange(e.target.value)} + onChange={e => onChange({ project: e.target.value })} value={project} /> @@ -80,7 +72,7 @@ export const BitbucketRepoPicker = ({ Repository onRepoNameChange(e.target.value)} + onChange={e => onChange({ repoName: e.target.value })} value={repoName} /> The name of the repository diff --git a/plugins/scaffolder/src/components/fields/RepoUrlPicker/GithubRepoPicker.test.tsx b/plugins/scaffolder/src/components/fields/RepoUrlPicker/GithubRepoPicker.test.tsx index 3f1105badc..4a17016959 100644 --- a/plugins/scaffolder/src/components/fields/RepoUrlPicker/GithubRepoPicker.test.tsx +++ b/plugins/scaffolder/src/components/fields/RepoUrlPicker/GithubRepoPicker.test.tsx @@ -24,10 +24,9 @@ describe('GitubRepoPicker', () => { const allowedOwners = ['owner1', 'owner2']; const { findByText } = render( , ); @@ -36,15 +35,14 @@ describe('GitubRepoPicker', () => { expect(await findByText('owner2')).toBeInTheDocument(); }); - it('calls onOwnerChange when the owner is changed to a different owner', async () => { - const onOwnerChange = jest.fn(); + it('calls onChange when the owner is changed to a different owner', async () => { + const onChange = jest.fn(); const allowedOwners = ['owner1', 'owner2']; const { getByRole } = render( , ); @@ -53,18 +51,17 @@ describe('GitubRepoPicker', () => { target: { value: 'owner2' }, }); - expect(onOwnerChange).toHaveBeenCalledWith('owner2'); + expect(onChange).toHaveBeenCalledWith({ owner: 'owner2' }); }); it('is disabled picked when only one allowed owner', () => { - const onOwnerChange = jest.fn(); + const onChange = jest.fn(); const allowedOwners = ['owner1']; const { getByRole } = render( , ); @@ -73,32 +70,29 @@ describe('GitubRepoPicker', () => { }); it('should display free text if no allowed owners are passed', async () => { - const onOwnerChange = jest.fn(); + const onChange = jest.fn(); const { getAllByRole } = render( , ); - const ownerField = getAllByRole('textbox')[0]; fireEvent.change(ownerField, { target: { value: 'my-mock-owner' } }); - expect(onOwnerChange).toHaveBeenCalledWith('my-mock-owner'); + expect(onChange).toHaveBeenCalledWith({ owner: 'my-mock-owner' }); }); }); describe('repo name', () => { it('should render free text field for input of repo name', () => { - const onRepoNameChange = jest.fn(); + const onChange = jest.fn(); const { getAllByRole } = render( , ); @@ -107,7 +101,7 @@ describe('GitubRepoPicker', () => { target: { value: 'my-mock-repo-name' }, }); - expect(onRepoNameChange).toHaveBeenCalledWith('my-mock-repo-name'); + expect(onChange).toHaveBeenCalledWith({ repoName: 'my-mock-repo-name' }); }); }); }); diff --git a/plugins/scaffolder/src/components/fields/RepoUrlPicker/GithubRepoPicker.tsx b/plugins/scaffolder/src/components/fields/RepoUrlPicker/GithubRepoPicker.tsx index 627027d130..22d13cdcdb 100644 --- a/plugins/scaffolder/src/components/fields/RepoUrlPicker/GithubRepoPicker.tsx +++ b/plugins/scaffolder/src/components/fields/RepoUrlPicker/GithubRepoPicker.tsx @@ -19,26 +19,25 @@ import FormHelperText from '@material-ui/core/FormHelperText'; import Input from '@material-ui/core/Input'; import InputLabel from '@material-ui/core/InputLabel'; import { Select, SelectItem } from '@backstage/core-components'; +import { RepoUrlPickerState } from './types'; export const GithubRepoPicker = ({ - onOwnerChange, - onRepoNameChange, allowedOwners = [], rawErrors, - owner, - repoName, + state, + onChange, }: { - onOwnerChange: (owner: string) => void; - onRepoNameChange: (name: string) => void; allowedOwners?: string[]; - owner?: string; - repoName?: string; rawErrors: string[]; + state: RepoUrlPickerState; + onChange: (state: RepoUrlPickerState) => void; }) => { const ownerItems: SelectItem[] = allowedOwners ? allowedOwners.map(i => ({ label: i, value: i })) : [{ label: 'Loading...', value: 'loading' }]; + const { owner, repoName } = state; + return ( <> onOwnerChange(String(Array.isArray(s) ? s[0] : s))} + onChange={s => + onChange({ owner: String(Array.isArray(s) ? s[0] : s) }) + } disabled={allowedOwners.length === 1} selected={owner} items={ownerItems} @@ -60,7 +61,7 @@ export const GithubRepoPicker = ({ Owner onOwnerChange(e.target.value)} + onChange={e => onChange({ owner: e.target.value })} value={owner} /> @@ -77,7 +78,7 @@ export const GithubRepoPicker = ({ Repository onRepoNameChange(e.target.value)} + onChange={e => onChange({ repoName: e.target.value })} value={repoName} /> The name of the repository diff --git a/plugins/scaffolder/src/components/fields/RepoUrlPicker/GitlabRepoPicker.tsx b/plugins/scaffolder/src/components/fields/RepoUrlPicker/GitlabRepoPicker.tsx index ae76c29555..0d0ad26ee1 100644 --- a/plugins/scaffolder/src/components/fields/RepoUrlPicker/GitlabRepoPicker.tsx +++ b/plugins/scaffolder/src/components/fields/RepoUrlPicker/GitlabRepoPicker.tsx @@ -19,26 +19,25 @@ import FormHelperText from '@material-ui/core/FormHelperText'; import Input from '@material-ui/core/Input'; import InputLabel from '@material-ui/core/InputLabel'; import { Select, SelectItem } from '@backstage/core-components'; +import { RepoUrlPickerState } from './types'; export const GitlabRepoPicker = ({ - onOwnerChange, - onRepoNameChange, allowedOwners = [], rawErrors, - owner, - repoName, + state, + onChange, }: { - onOwnerChange: (owner: string) => void; - onRepoNameChange: (name: string) => void; allowedOwners?: string[]; - owner?: string; - repoName?: string; + state: RepoUrlPickerState; + onChange: (state: RepoUrlPickerState) => void; rawErrors: string[]; }) => { const ownerItems: SelectItem[] = allowedOwners ? allowedOwners.map(i => ({ label: i, value: i })) : [{ label: 'Loading...', value: 'loading' }]; + const { owner, repoName } = state; + return ( <> onOwnerChange(String(Array.isArray(s) ? s[0] : s))} + onChange={selected => + onChange({ + owner: String(Array.isArray(selected) ? selected[0] : selected), + }) + } disabled={allowedOwners.length === 1} selected={owner} items={ownerItems} @@ -60,7 +63,7 @@ export const GitlabRepoPicker = ({ Owner onOwnerChange(e.target.value)} + onChange={e => onChange({ owner: e.target.value })} value={owner} /> @@ -77,7 +80,7 @@ export const GitlabRepoPicker = ({ Repository onRepoNameChange(e.target.value)} + onChange={e => onChange({ repoName: e.target.value })} value={repoName} /> The name of the repository diff --git a/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx b/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx index 68df677274..65b1f8bf61 100644 --- a/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx +++ b/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx @@ -15,7 +15,7 @@ */ import { useApi } from '@backstage/core-plugin-api'; import { scmIntegrationsApiRef } from '@backstage/integration-react'; -import React, { useEffect, useState, useMemo } from 'react'; +import React, { useEffect, useState, useMemo, useCallback } from 'react'; import { GithubRepoPicker } from './GithubRepoPicker'; import { GitlabRepoPicker } from './GitlabRepoPicker'; import { AzureRepoPicker } from './AzureRepoPicker'; @@ -23,6 +23,7 @@ import { BitbucketRepoPicker } from './BitbucketRepoPicker'; import { FieldExtensionComponentProps } from '../../../extensions'; import { RepoUrlPickerHost } from './RepoUrlPickerHost'; import { parseRepoPickerUrl, serializeRepoPickerUrl } from './utils'; +import { RepoUrlPickerState } from './types'; export interface RepoUrlPickerUiOptions { allowedHosts?: string[]; @@ -35,14 +36,9 @@ export const RepoUrlPicker = ({ rawErrors, formData, }: FieldExtensionComponentProps) => { - const [state, setState] = useState<{ - host?: string; - owner?: string; - repo?: string; - organization?: string; - workspace?: string; - project?: string; - }>(parseRepoPickerUrl(formData)); + const [state, setState] = useState( + parseRepoPickerUrl(formData), + ); const integrationApi = useApi(scmIntegrationsApiRef); const allowedHosts = uiSchema?.['ui:options']?.allowedHosts ?? []; @@ -62,76 +58,50 @@ export const RepoUrlPicker = ({ } }, [setState, allowedOwners]); + const updateLocalState = useCallback( + (newState: RepoUrlPickerState) => { + setState(prevState => ({ ...prevState, ...newState })); + }, + [setState], + ); + return ( <> setState({ host })} + onChange={host => setState(prevState => ({ ...prevState, host }))} rawErrors={rawErrors} /> {state.host && integrationApi.byHost(state.host)?.type === 'github' && ( - setState(prevState => ({ ...prevState, repo })) - } - onOwnerChange={owner => - setState(prevState => ({ ...prevState, owner })) - } + state={state} + onChange={updateLocalState} /> )} {state.host && integrationApi.byHost(state.host)?.type === 'gitlab' && ( - setState(prevState => ({ ...prevState, repo })) - } - onOwnerChange={owner => - setState(prevState => ({ ...prevState, owner })) - } + state={state} + onChange={updateLocalState} /> )} {state.host && integrationApi.byHost(state.host)?.type === 'bitbucket' && ( - setState(prevState => ({ ...prevState, repo })) - } - onProjectChange={project => - setState(prevState => ({ ...prevState, project })) - } - onWorkspaceChange={workspace => - setState(prevState => ({ ...prevState, workspace })) - } + state={state} + onChange={updateLocalState} /> )} {state.host && integrationApi.byHost(state.host)?.type === 'azure' && ( - setState(prevState => ({ ...prevState, owner })) - } - onRepoNameChange={repo => - setState(prevState => ({ ...prevState, repo })) - } - onOrgChange={org => - setState(prevState => ({ ...prevState, organization: org })) - } + state={state} + onChange={updateLocalState} /> )} diff --git a/plugins/scaffolder/src/components/fields/RepoUrlPicker/types.ts b/plugins/scaffolder/src/components/fields/RepoUrlPicker/types.ts new file mode 100644 index 0000000000..5e979e789e --- /dev/null +++ b/plugins/scaffolder/src/components/fields/RepoUrlPicker/types.ts @@ -0,0 +1,23 @@ +/* + * Copyright 2022 The Backstage Authors + * + * 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 + * + * http://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. + */ +export interface RepoUrlPickerState { + host?: string; + owner?: string; + repoName?: string; + organization?: string; + workspace?: string; + project?: string; +} diff --git a/plugins/scaffolder/src/components/fields/RepoUrlPicker/utils.test.ts b/plugins/scaffolder/src/components/fields/RepoUrlPicker/utils.test.ts index 2771d0faa5..c90b357cf1 100644 --- a/plugins/scaffolder/src/components/fields/RepoUrlPicker/utils.test.ts +++ b/plugins/scaffolder/src/components/fields/RepoUrlPicker/utils.test.ts @@ -26,7 +26,7 @@ describe('utils', () => { serializeRepoPickerUrl({ host: 'github.com', owner: 'owner', - repo: 'backstage', + repoName: 'backstage', }), ).toBe('github.com?owner=owner&repo=backstage'); }); @@ -49,7 +49,7 @@ describe('utils', () => { serializeRepoPickerUrl({ host: 'github.com', owner: 'owner', - repo: 'backstage', + repoName: 'backstage', organization: 'organization', workspace: 'workspace', project: 'backstage', @@ -69,7 +69,7 @@ describe('utils', () => { ).toEqual({ host: 'github.com', owner: 'owner', - repo: 'backstage', + repoName: 'backstage', organization: 'organization', workspace: 'workspace', project: 'backstage', diff --git a/plugins/scaffolder/src/components/fields/RepoUrlPicker/utils.ts b/plugins/scaffolder/src/components/fields/RepoUrlPicker/utils.ts index 66c010ec3d..d1f2302d3e 100644 --- a/plugins/scaffolder/src/components/fields/RepoUrlPicker/utils.ts +++ b/plugins/scaffolder/src/components/fields/RepoUrlPicker/utils.ts @@ -14,16 +14,9 @@ * limitations under the License. */ -type RepoUrlPickerOptions = { - host?: string; - owner?: string; - repo?: string; - organization?: string; - workspace?: string; - project?: string; -}; +import { RepoUrlPickerState } from './types'; -export function serializeRepoPickerUrl(data: RepoUrlPickerOptions) { +export function serializeRepoPickerUrl(data: RepoUrlPickerState) { if (!data.host) { return undefined; } @@ -32,8 +25,8 @@ export function serializeRepoPickerUrl(data: RepoUrlPickerOptions) { if (data.owner) { params.set('owner', data.owner); } - if (data.repo) { - params.set('repo', data.repo); + if (data.repoName) { + params.set('repo', data.repoName); } if (data.organization) { params.set('organization', data.organization); @@ -50,10 +43,10 @@ export function serializeRepoPickerUrl(data: RepoUrlPickerOptions) { export function parseRepoPickerUrl( url: string | undefined, -): RepoUrlPickerOptions { +): RepoUrlPickerState { let host = undefined; let owner = undefined; - let repo = undefined; + let repoName = undefined; let organization = undefined; let workspace = undefined; let project = undefined; @@ -63,7 +56,7 @@ export function parseRepoPickerUrl( const parsed = new URL(`https://${url}`); host = parsed.host; owner = parsed.searchParams.get('owner') || undefined; - repo = parsed.searchParams.get('repo') || undefined; + repoName = parsed.searchParams.get('repo') || undefined; organization = parsed.searchParams.get('organization') || undefined; workspace = parsed.searchParams.get('workspace') || undefined; project = parsed.searchParams.get('project') || undefined; @@ -72,5 +65,5 @@ export function parseRepoPickerUrl( /* ok */ } - return { host, owner, repo, organization, workspace, project }; + return { host, owner, repoName, organization, workspace, project }; }