From e6e293ca30256d8d7641f5c9ff27e29b76f149de Mon Sep 17 00:00:00 2001 From: Benjamin Janssens Date: Mon, 15 Jul 2024 10:22:21 +0200 Subject: [PATCH 1/6] chore: extract BaseRepoUrlPickerProps Signed-off-by: Benjamin Janssens --- .../fields/RepoUrlPicker/AzureRepoPicker.tsx | 15 +++++++-------- .../RepoUrlPicker/BitbucketRepoPicker.tsx | 17 ++++++++--------- .../fields/RepoUrlPicker/GerritRepoPicker.tsx | 8 ++------ .../fields/RepoUrlPicker/GiteaRepoPicker.tsx | 15 +++++++-------- .../fields/RepoUrlPicker/GithubRepoPicker.tsx | 13 ++++++------- .../fields/RepoUrlPicker/GitlabRepoPicker.tsx | 15 +++++++-------- .../components/fields/RepoUrlPicker/types.ts | 6 ++++++ 7 files changed, 43 insertions(+), 46 deletions(-) diff --git a/plugins/scaffolder/src/components/fields/RepoUrlPicker/AzureRepoPicker.tsx b/plugins/scaffolder/src/components/fields/RepoUrlPicker/AzureRepoPicker.tsx index 833a0849fb..48fcb4f6af 100644 --- a/plugins/scaffolder/src/components/fields/RepoUrlPicker/AzureRepoPicker.tsx +++ b/plugins/scaffolder/src/components/fields/RepoUrlPicker/AzureRepoPicker.tsx @@ -19,16 +19,15 @@ 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'; +import { BaseRepoUrlPickerProps } from './types'; import { Select, SelectItem } from '@backstage/core-components'; -export const AzureRepoPicker = (props: { - allowedOrganizations?: string[]; - allowedProject?: string[]; - rawErrors: string[]; - state: RepoUrlPickerState; - onChange: (state: RepoUrlPickerState) => void; -}) => { +export const AzureRepoPicker = ( + props: BaseRepoUrlPickerProps<{ + allowedOrganizations?: string[]; + allowedProject?: string[]; + }>, +) => { const { allowedOrganizations = [], allowedProject = [], diff --git a/plugins/scaffolder/src/components/fields/RepoUrlPicker/BitbucketRepoPicker.tsx b/plugins/scaffolder/src/components/fields/RepoUrlPicker/BitbucketRepoPicker.tsx index 10d4a6e6c3..5d627e9512 100644 --- a/plugins/scaffolder/src/components/fields/RepoUrlPicker/BitbucketRepoPicker.tsx +++ b/plugins/scaffolder/src/components/fields/RepoUrlPicker/BitbucketRepoPicker.tsx @@ -17,7 +17,7 @@ import React, { useEffect, useState } from 'react'; import FormControl from '@material-ui/core/FormControl'; import FormHelperText from '@material-ui/core/FormHelperText'; import { Select, SelectItem } from '@backstage/core-components'; -import { RepoUrlPickerState } from './types'; +import { BaseRepoUrlPickerProps } from './types'; import Autocomplete from '@material-ui/lab/Autocomplete'; import TextField from '@material-ui/core/TextField'; import useDebounce from 'react-use/esm/useDebounce'; @@ -33,14 +33,13 @@ import { scaffolderApiRef } from '@backstage/plugin-scaffolder-react'; * @param allowedProjects - Allowed projects for the Bitbucket cloud repository * */ -export const BitbucketRepoPicker = (props: { - allowedOwners?: string[]; - allowedProjects?: string[]; - onChange: (state: RepoUrlPickerState) => void; - state: RepoUrlPickerState; - rawErrors: string[]; - accessToken?: string; -}) => { +export const BitbucketRepoPicker = ( + props: BaseRepoUrlPickerProps<{ + allowedOwners?: string[]; + allowedProjects?: string[]; + accessToken?: string; + }>, +) => { const { allowedOwners = [], allowedProjects = [], diff --git a/plugins/scaffolder/src/components/fields/RepoUrlPicker/GerritRepoPicker.tsx b/plugins/scaffolder/src/components/fields/RepoUrlPicker/GerritRepoPicker.tsx index 13af966c25..1b13c5eb29 100644 --- a/plugins/scaffolder/src/components/fields/RepoUrlPicker/GerritRepoPicker.tsx +++ b/plugins/scaffolder/src/components/fields/RepoUrlPicker/GerritRepoPicker.tsx @@ -18,13 +18,9 @@ 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'; +import { BaseRepoUrlPickerProps } from './types'; -export const GerritRepoPicker = (props: { - onChange: (state: RepoUrlPickerState) => void; - state: RepoUrlPickerState; - rawErrors: string[]; -}) => { +export const GerritRepoPicker = (props: BaseRepoUrlPickerProps) => { const { onChange, rawErrors, state } = props; const { workspace, owner } = state; return ( diff --git a/plugins/scaffolder/src/components/fields/RepoUrlPicker/GiteaRepoPicker.tsx b/plugins/scaffolder/src/components/fields/RepoUrlPicker/GiteaRepoPicker.tsx index 566a2732e3..9fb3af211f 100644 --- a/plugins/scaffolder/src/components/fields/RepoUrlPicker/GiteaRepoPicker.tsx +++ b/plugins/scaffolder/src/components/fields/RepoUrlPicker/GiteaRepoPicker.tsx @@ -19,15 +19,14 @@ 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'; +import { BaseRepoUrlPickerProps } from './types'; -export const GiteaRepoPicker = (props: { - allowedOwners?: string[]; - allowedRepos?: string[]; - state: RepoUrlPickerState; - onChange: (state: RepoUrlPickerState) => void; - rawErrors: string[]; -}) => { +export const GiteaRepoPicker = ( + props: BaseRepoUrlPickerProps<{ + allowedOwners?: string[]; + allowedRepos?: string[]; + }>, +) => { const { allowedOwners = [], state, onChange, rawErrors } = props; const ownerItems: SelectItem[] = allowedOwners ? allowedOwners.map(i => ({ label: i, value: i })) diff --git a/plugins/scaffolder/src/components/fields/RepoUrlPicker/GithubRepoPicker.tsx b/plugins/scaffolder/src/components/fields/RepoUrlPicker/GithubRepoPicker.tsx index ec59d89ef8..5d46dc4333 100644 --- a/plugins/scaffolder/src/components/fields/RepoUrlPicker/GithubRepoPicker.tsx +++ b/plugins/scaffolder/src/components/fields/RepoUrlPicker/GithubRepoPicker.tsx @@ -19,14 +19,13 @@ 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'; +import { BaseRepoUrlPickerProps } from './types'; -export const GithubRepoPicker = (props: { - allowedOwners?: string[]; - rawErrors: string[]; - state: RepoUrlPickerState; - onChange: (state: RepoUrlPickerState) => void; -}) => { +export const GithubRepoPicker = ( + props: BaseRepoUrlPickerProps<{ + allowedOwners?: string[]; + }>, +) => { const { allowedOwners = [], rawErrors, state, onChange } = props; const ownerItems: SelectItem[] = allowedOwners ? allowedOwners.map(i => ({ label: i, value: i })) diff --git a/plugins/scaffolder/src/components/fields/RepoUrlPicker/GitlabRepoPicker.tsx b/plugins/scaffolder/src/components/fields/RepoUrlPicker/GitlabRepoPicker.tsx index fbcc13c846..12ae37bf15 100644 --- a/plugins/scaffolder/src/components/fields/RepoUrlPicker/GitlabRepoPicker.tsx +++ b/plugins/scaffolder/src/components/fields/RepoUrlPicker/GitlabRepoPicker.tsx @@ -19,15 +19,14 @@ 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'; +import { BaseRepoUrlPickerProps } from './types'; -export const GitlabRepoPicker = (props: { - allowedOwners?: string[]; - allowedRepos?: string[]; - state: RepoUrlPickerState; - onChange: (state: RepoUrlPickerState) => void; - rawErrors: string[]; -}) => { +export const GitlabRepoPicker = ( + props: BaseRepoUrlPickerProps<{ + allowedOwners?: string[]; + allowedRepos?: string[]; + }>, +) => { const { allowedOwners = [], state, onChange, rawErrors } = props; const ownerItems: SelectItem[] = allowedOwners ? allowedOwners.map(i => ({ label: i, value: i })) diff --git a/plugins/scaffolder/src/components/fields/RepoUrlPicker/types.ts b/plugins/scaffolder/src/components/fields/RepoUrlPicker/types.ts index 1430b10b84..9489901d61 100644 --- a/plugins/scaffolder/src/components/fields/RepoUrlPicker/types.ts +++ b/plugins/scaffolder/src/components/fields/RepoUrlPicker/types.ts @@ -22,3 +22,9 @@ export interface RepoUrlPickerState { project?: string; availableRepos?: string[]; } + +export type BaseRepoUrlPickerProps = T & { + onChange: (state: RepoUrlPickerState) => void; + state: RepoUrlPickerState; + rawErrors: string[]; +}; From 6b8bab93beaee731a7db18b818cb13af21e848c3 Mon Sep 17 00:00:00 2001 From: Benjamin Janssens Date: Mon, 15 Jul 2024 10:40:19 +0200 Subject: [PATCH 2/6] refactor: use useCallback for autocompletion updates in BitbucketRepoPicker Signed-off-by: Benjamin Janssens --- .../RepoUrlPicker/BitbucketRepoPicker.tsx | 153 +++++++++--------- 1 file changed, 75 insertions(+), 78 deletions(-) diff --git a/plugins/scaffolder/src/components/fields/RepoUrlPicker/BitbucketRepoPicker.tsx b/plugins/scaffolder/src/components/fields/RepoUrlPicker/BitbucketRepoPicker.tsx index 5d627e9512..0b7ee49525 100644 --- a/plugins/scaffolder/src/components/fields/RepoUrlPicker/BitbucketRepoPicker.tsx +++ b/plugins/scaffolder/src/components/fields/RepoUrlPicker/BitbucketRepoPicker.tsx @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import React, { useEffect, useState } from 'react'; +import React, { useCallback, useEffect, useState } from 'react'; import FormControl from '@material-ui/core/FormControl'; import FormHelperText from '@material-ui/core/FormHelperText'; import { Select, SelectItem } from '@backstage/core-components'; @@ -68,93 +68,90 @@ export const BitbucketRepoPicker = ( const [availableProjects, setAvailableProjects] = useState([]); // Update available workspaces when client is available - useDebounce( - () => { - const updateAvailableWorkspaces = async () => { - if ( - host === 'bitbucket.org' && - accessToken && - scaffolderApi.autocomplete - ) { - const { results } = await scaffolderApi.autocomplete({ - token: accessToken, - resource: 'workspaces', - context: {}, - provider: 'bitbucket-cloud', - }); + const updateAvailableWorkspaces = useCallback(() => { + if ( + !scaffolderApi.autocomplete || + !accessToken || + host !== 'bitbucket.org' + ) { + setAvailableWorkspaces([]); + return; + } - setAvailableWorkspaces(results.map(r => r.title)); - } else { - setAvailableWorkspaces([]); - } - }; + scaffolderApi + .autocomplete({ + token: accessToken, + resource: 'workspaces', + provider: 'bitbucket-cloud', + }) + .then(({ results }) => { + setAvailableWorkspaces(results.map(r => r.title)); + }) + .catch(() => { + setAvailableWorkspaces([]); + }); + }, [scaffolderApi, accessToken, host]); - updateAvailableWorkspaces().catch(() => setAvailableWorkspaces([])); - }, - 500, - [host, accessToken], - ); + useDebounce(updateAvailableWorkspaces, 500, [updateAvailableWorkspaces]); // Update available projects when client is available and workspace changes - useDebounce( - () => { - const updateAvailableProjects = async () => { - if ( - host === 'bitbucket.org' && - accessToken && - workspace && - scaffolderApi.autocomplete - ) { - const { results } = await scaffolderApi.autocomplete({ - token: accessToken, - resource: 'projects', - context: { workspace }, - provider: 'bitbucket-cloud', - }); + const updateAvailableProjects = useCallback(() => { + if ( + !scaffolderApi.autocomplete || + !accessToken || + host !== 'bitbucket.org' || + !workspace + ) { + setAvailableProjects([]); + return; + } - setAvailableProjects(results.map(r => r.title)); - } else { - setAvailableProjects([]); - } - }; + scaffolderApi + .autocomplete({ + token: accessToken, + resource: 'projects', + context: { workspace }, + provider: 'bitbucket-cloud', + }) + .then(({ results }) => { + setAvailableProjects(results.map(r => r.title)); + }) + .catch(() => { + setAvailableProjects([]); + }); + }, [scaffolderApi, accessToken, host, workspace]); - updateAvailableProjects().catch(() => setAvailableProjects([])); - }, - 500, - [host, accessToken, workspace], - ); + useDebounce(updateAvailableProjects, 500, [updateAvailableProjects]); // Update available repositories when client is available and workspace or project changes - useDebounce( - () => { - const updateAvailableRepositories = async () => { - if ( - host === 'bitbucket.org' && - accessToken && - workspace && - project && - scaffolderApi.autocomplete - ) { - const { results } = await scaffolderApi.autocomplete({ - token: accessToken, - resource: 'repositories', - context: { workspace, project }, - provider: 'bitbucket-cloud', - }); + const updateAvailableRepositories = useCallback(() => { + if ( + !scaffolderApi.autocomplete || + !accessToken || + host !== 'bitbucket.org' || + !workspace || + !project + ) { + onChange({ availableRepos: [] }); + return; + } - onChange({ availableRepos: results.map(r => r.title) }); - } else { - onChange({ availableRepos: [] }); - } - }; + scaffolderApi + .autocomplete({ + token: accessToken, + resource: 'repositories', + context: { workspace, project }, + provider: 'bitbucket-cloud', + }) + .then(({ results }) => { + onChange({ availableRepos: results.map(r => r.title) }); + }) + .catch(() => { + onChange({ availableRepos: [] }); + }); + }, [scaffolderApi, accessToken, host, workspace, project, onChange]); - updateAvailableRepositories().catch(() => - onChange({ availableRepos: [] }), - ); - }, - 500, - [host, accessToken, workspace, project], - ); + useDebounce(updateAvailableRepositories, 500, [updateAvailableRepositories]); return ( <> From 7f153f94c1c3d1849e418109e097b03914941113 Mon Sep 17 00:00:00 2001 From: Benjamin Janssens Date: Mon, 15 Jul 2024 10:44:22 +0200 Subject: [PATCH 3/6] test: simplify checking for the secret in the document; remove SecretsComponent from test that didn't use it Signed-off-by: Benjamin Janssens --- .../RepoUrlPicker/RepoUrlPicker.test.tsx | 47 ++++++------------- 1 file changed, 15 insertions(+), 32 deletions(-) diff --git a/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.test.tsx b/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.test.tsx index 002652cfab..f0e47a2504 100644 --- a/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.test.tsx +++ b/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.test.tsx @@ -214,13 +214,14 @@ describe('RepoUrlPicker', () => { describe('requestUserCredentials', () => { it('should call the scmAuthApi with the correct params', async () => { + const secretsKey = 'testKey'; + const SecretsComponent = () => { const { secrets } = useTemplateSecrets(); - return ( -
{JSON.stringify({ secrets })}
- ); + const secret = secrets[secretsKey]; + return secret ?
{secret}
: null; }; - const { getByTestId } = await renderInTestApp( + const { getByText } = await renderInTestApp( { 'ui:field': 'RepoUrlPicker', 'ui:options': { requestUserCredentials: { - secretsKey: 'testKey', + secretsKey, additionalScopes: { github: ['workflow'] }, }, }, @@ -265,21 +266,9 @@ describe('RepoUrlPicker', () => { }, }); - const currentSecrets = JSON.parse( - getByTestId('current-secrets').textContent!, - ); - - expect(currentSecrets).toEqual({ - secrets: { testKey: 'abc123' }, - }); + expect(getByText('abc123')).toBeInTheDocument(); }); it('should call the scmAuthApi with the correct params if workspace is nested', async () => { - const SecretsComponent = () => { - const { secrets } = useTemplateSecrets(); - return ( -
{JSON.stringify({ secrets })}
- ); - }; await renderInTestApp( { RepoUrlPicker: RepoUrlPicker as ScaffolderRJSFField, }} /> - , ); @@ -324,13 +312,14 @@ describe('RepoUrlPicker', () => { }); it('should not call the scmAuthApi if secret is available in the state', async () => { + const secretsKey = 'testKey'; + const SecretsComponent = () => { const { secrets } = useTemplateSecrets(); - return ( -
{JSON.stringify({ secrets })}
- ); + const secret = secrets[secretsKey]; + return secret ?
{secret}
: null; }; - const { getByTestId } = await renderInTestApp( + const { getByText } = await renderInTestApp( { [scaffolderApiRef, mockScaffolderApi], ]} > - +
{ 'ui:field': 'RepoUrlPicker', 'ui:options': { requestUserCredentials: { - secretsKey: 'testKey', + secretsKey, additionalScopes: { github: ['workflow'] }, }, }, @@ -368,13 +357,7 @@ describe('RepoUrlPicker', () => { // as we already have a secret in the state, getCredentials should not be called again. expect(mockScmAuthApi.getCredentials).toHaveBeenCalledTimes(0); - const currentSecrets = JSON.parse( - getByTestId('current-secrets').textContent!, - ); - - expect(currentSecrets).toEqual({ - secrets: { testKey: 'abc123' }, - }); + expect(getByText('abc123')).toBeInTheDocument(); }); }); }); From bbd9f56c764b3fc22c79a279bca48f70d6c83dea Mon Sep 17 00:00:00 2001 From: Benjamin Janssens Date: Mon, 15 Jul 2024 10:49:56 +0200 Subject: [PATCH 4/6] chore: add changeset Signed-off-by: Benjamin Janssens --- .changeset/spotty-planets-accept.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/spotty-planets-accept.md diff --git a/.changeset/spotty-planets-accept.md b/.changeset/spotty-planets-accept.md new file mode 100644 index 0000000000..db262dd8b5 --- /dev/null +++ b/.changeset/spotty-planets-accept.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-scaffolder': patch +--- + +Cleaned up codebase From 9a5e7f1c36934b6eca39c8c13fbc35bcc43fefb2 Mon Sep 17 00:00:00 2001 From: Benjamin Janssens Date: Mon, 15 Jul 2024 10:51:24 +0200 Subject: [PATCH 5/6] chore: update changeset Signed-off-by: Benjamin Janssens --- .changeset/spotty-planets-accept.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/spotty-planets-accept.md b/.changeset/spotty-planets-accept.md index db262dd8b5..7342f89bd1 100644 --- a/.changeset/spotty-planets-accept.md +++ b/.changeset/spotty-planets-accept.md @@ -2,4 +2,4 @@ '@backstage/plugin-scaffolder': patch --- -Cleaned up codebase +Cleaned up codebase of RepoUrlPicker From c4a4a69912e004dbfa622d9f2107410af8fffd34 Mon Sep 17 00:00:00 2001 From: Benjamin Janssens Date: Mon, 15 Jul 2024 11:15:21 +0200 Subject: [PATCH 6/6] test: fix BitbucketRepoPicker tests Signed-off-by: Benjamin Janssens --- .../fields/RepoUrlPicker/BitbucketRepoPicker.test.tsx | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/plugins/scaffolder/src/components/fields/RepoUrlPicker/BitbucketRepoPicker.test.tsx b/plugins/scaffolder/src/components/fields/RepoUrlPicker/BitbucketRepoPicker.test.tsx index 2ae89233ff..570c5bd664 100644 --- a/plugins/scaffolder/src/components/fields/RepoUrlPicker/BitbucketRepoPicker.test.tsx +++ b/plugins/scaffolder/src/components/fields/RepoUrlPicker/BitbucketRepoPicker.test.tsx @@ -27,9 +27,11 @@ import { act } from 'react-dom/test-utils'; describe('BitbucketRepoPicker', () => { const scaffolderApiMock: Partial = { - autocomplete: jest.fn().mockImplementation(opts => ({ - results: [{ title: `${opts.resource}_example` }], - })), + autocomplete: jest.fn().mockImplementation(opts => + Promise.resolve({ + results: [{ title: `${opts.resource}_example` }], + }), + ), }; it('renders a select if there is a list of allowed owners', async () => {