diff --git a/.changeset/spotty-planets-accept.md b/.changeset/spotty-planets-accept.md new file mode 100644 index 0000000000..7342f89bd1 --- /dev/null +++ b/.changeset/spotty-planets-accept.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-scaffolder': patch +--- + +Cleaned up codebase of RepoUrlPicker 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.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 () => { diff --git a/plugins/scaffolder/src/components/fields/RepoUrlPicker/BitbucketRepoPicker.tsx b/plugins/scaffolder/src/components/fields/RepoUrlPicker/BitbucketRepoPicker.tsx index 10d4a6e6c3..0b7ee49525 100644 --- a/plugins/scaffolder/src/components/fields/RepoUrlPicker/BitbucketRepoPicker.tsx +++ b/plugins/scaffolder/src/components/fields/RepoUrlPicker/BitbucketRepoPicker.tsx @@ -13,11 +13,11 @@ * 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'; -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 = [], @@ -69,93 +68,90 @@ export const BitbucketRepoPicker = (props: { 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 ( <> 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/RepoUrlPicker.test.tsx b/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.test.tsx index fce9d9517c..163f397857 100644 --- a/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.test.tsx +++ b/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.test.tsx @@ -215,13 +215,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'] }, }, }, @@ -266,21 +267,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, }} /> - , ); @@ -325,11 +313,12 @@ describe('RepoUrlPicker', () => { }); it('should call the scmAuthApi with the new host when the host is changed', async () => { + const secretsKey = 'testKey'; + const SecretsComponent = () => { const { secrets } = useTemplateSecrets(); - return ( -
{JSON.stringify({ secrets })}
- ); + const secret = secrets[secretsKey]; + return secret ?
{secret}
: null; }; const allowedHosts = ['github.com', 'gitlab.example.com']; @@ -346,7 +335,7 @@ describe('RepoUrlPicker', () => { ); const secondHost = allowedHosts[1]; - const { getAllByRole, getByTestId } = await renderInTestApp( + const { getAllByRole, getByText } = await renderInTestApp( { 'ui:options': { allowedHosts, requestUserCredentials: { - secretsKey: 'testKey', + secretsKey, }, }, }} @@ -380,12 +369,7 @@ describe('RepoUrlPicker', () => { // need to wait for the debounce to finish to fetch credentials for the first selected host await new Promise(resolve => setTimeout(resolve, 600)); }); - const firstHostSecrets = JSON.parse( - getByTestId('current-secrets').textContent!, - ); - expect(firstHostSecrets).toEqual({ - secrets: { testKey: 'abc123' }, - }); + expect(getByText('abc123')).toBeInTheDocument(); await act(async () => { // Select the second host @@ -395,12 +379,7 @@ describe('RepoUrlPicker', () => { // need to wait for the debounce to finish await new Promise(resolve => setTimeout(resolve, 600)); }); - const secondHostSecrets = JSON.parse( - getByTestId('current-secrets').textContent!, - ); - expect(secondHostSecrets).toEqual({ - secrets: { testKey: 'def456' }, - }); + expect(getByText('def456')).toBeInTheDocument(); expect(mockScmAuthApi.getCredentials).toHaveBeenCalledWith({ url: `https://${secondHost}`, additionalScope: { 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[]; +};