From 18571288a8f1b254c374766164dd2dec15ef1ff5 Mon Sep 17 00:00:00 2001 From: Oliver Sand Date: Thu, 16 Sep 2021 16:05:11 +0200 Subject: [PATCH] Make preparePullRequest async Signed-off-by: Oliver Sand --- .../src/api/CatalogImportApi.ts | 6 +- .../src/api/CatalogImportClient.test.ts | 15 +- .../src/api/CatalogImportClient.ts | 4 +- .../ImportInfoCard/ImportInfoCard.test.tsx | 4 +- .../src/components/ImportStepper/defaults.tsx | 5 - .../StepPrepareCreatePullRequest.test.tsx | 53 +++--- .../StepPrepareCreatePullRequest.tsx | 169 ++++++++++-------- 7 files changed, 139 insertions(+), 117 deletions(-) diff --git a/plugins/catalog-import/src/api/CatalogImportApi.ts b/plugins/catalog-import/src/api/CatalogImportApi.ts index fae4e3d3b3..a4645dc7ef 100644 --- a/plugins/catalog-import/src/api/CatalogImportApi.ts +++ b/plugins/catalog-import/src/api/CatalogImportApi.ts @@ -15,8 +15,8 @@ */ import { EntityName } from '@backstage/catalog-model'; -import { PartialEntity } from '../types'; import { createApiRef } from '@backstage/core-plugin-api'; +import { PartialEntity } from '../types'; export const catalogImportApiRef = createApiRef({ id: 'plugin.catalog-import.service', @@ -42,10 +42,10 @@ export type AnalyzeResult = export interface CatalogImportApi { analyzeUrl(url: string): Promise; - preparePullRequest?(): { + preparePullRequest?(): Promise<{ title: string; body: string; - }; + }>; submitPullRequest(options: { repositoryUrl: string; fileContent: string; diff --git a/plugins/catalog-import/src/api/CatalogImportClient.test.ts b/plugins/catalog-import/src/api/CatalogImportClient.test.ts index 99928727fb..0782b44147 100644 --- a/plugins/catalog-import/src/api/CatalogImportClient.test.ts +++ b/plugins/catalog-import/src/api/CatalogImportClient.test.ts @@ -115,7 +115,11 @@ describe('CatalogImportClient', () => { scmIntegrationsApi, identityApi, catalogApi, - configApi: new ConfigReader({}), + configApi: new ConfigReader({ + app: { + baseUrl: 'https://demo.backstage.io/', + }, + }), }); }); @@ -444,4 +448,13 @@ describe('CatalogImportClient', () => { }); }); }); + + describe('preparePullRequest', () => { + test('should prepare pull request details', async () => { + await expect(catalogImportClient.preparePullRequest()).resolves.toEqual({ + title: 'Add catalog-info.yaml config file', + body: expect.any(String), + }); + }); + }); }); diff --git a/plugins/catalog-import/src/api/CatalogImportClient.ts b/plugins/catalog-import/src/api/CatalogImportClient.ts index 12aabf2b7f..242f8440d1 100644 --- a/plugins/catalog-import/src/api/CatalogImportClient.ts +++ b/plugins/catalog-import/src/api/CatalogImportClient.ts @@ -118,10 +118,10 @@ export class CatalogImportClient implements CatalogImportApi { }; } - preparePullRequest(): { + async preparePullRequest(): Promise<{ title: string; body: string; - } { + }> { const appTitle = this.configApi.getOptionalString('app.title') ?? 'Backstage'; const appBaseUrl = this.configApi.getString('app.baseUrl'); diff --git a/plugins/catalog-import/src/components/ImportInfoCard/ImportInfoCard.test.tsx b/plugins/catalog-import/src/components/ImportInfoCard/ImportInfoCard.test.tsx index f03d6b28e8..b33e3c4d36 100644 --- a/plugins/catalog-import/src/components/ImportInfoCard/ImportInfoCard.test.tsx +++ b/plugins/catalog-import/src/components/ImportInfoCard/ImportInfoCard.test.tsx @@ -66,7 +66,7 @@ describe('', () => { }); it('renders section on GitHub discovery if supported', async () => { - catalogImportApi.preparePullRequest = () => ({ title: '', body: '' }); + catalogImportApi.preparePullRequest = async () => ({ title: '', body: '' }); await act(async () => { const { getByText } = render( @@ -82,7 +82,7 @@ describe('', () => { }); it('renders section on pull requests if supported', async () => { - catalogImportApi.preparePullRequest = () => ({ title: '', body: '' }); + catalogImportApi.preparePullRequest = async () => ({ title: '', body: '' }); await act(async () => { const { getByText } = render( diff --git a/plugins/catalog-import/src/components/ImportStepper/defaults.tsx b/plugins/catalog-import/src/components/ImportStepper/defaults.tsx index 72f946af6c..357ed64e14 100644 --- a/plugins/catalog-import/src/components/ImportStepper/defaults.tsx +++ b/plugins/catalog-import/src/components/ImportStepper/defaults.tsx @@ -136,9 +136,6 @@ export function defaultGenerateStepper( return defaults.prepare(state, opts); } - const { title, body } = - opts.apis.catalogImportApi.preparePullRequest!(); - return { stepLabel: Create Pull Request, content: ( @@ -146,8 +143,6 @@ export function defaultGenerateStepper( analyzeResult={state.analyzeResult} onPrepare={state.onPrepare} onGoBack={state.onGoBack} - defaultTitle={title} - defaultBody={body} renderFormFields={({ values, setValue, diff --git a/plugins/catalog-import/src/components/StepPrepareCreatePullRequest/StepPrepareCreatePullRequest.test.tsx b/plugins/catalog-import/src/components/StepPrepareCreatePullRequest/StepPrepareCreatePullRequest.test.tsx index 08b8177291..e697b54b19 100644 --- a/plugins/catalog-import/src/components/StepPrepareCreatePullRequest/StepPrepareCreatePullRequest.test.tsx +++ b/plugins/catalog-import/src/components/StepPrepareCreatePullRequest/StepPrepareCreatePullRequest.test.tsx @@ -14,23 +14,25 @@ * limitations under the License. */ +import { ApiProvider, ApiRegistry } from '@backstage/core-app-api'; import { catalogApiRef } from '@backstage/plugin-catalog-react'; import { TextField } from '@material-ui/core'; import { act, render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import React from 'react'; +import { errorApiRef } from '../../../../../packages/core-plugin-api/src'; import { AnalyzeResult, catalogImportApiRef } from '../../api'; import { asInputRef } from '../helpers'; import { generateEntities, StepPrepareCreatePullRequest, } from './StepPrepareCreatePullRequest'; -import { ApiProvider, ApiRegistry } from '@backstage/core-app-api'; describe('', () => { const catalogImportApi: jest.Mocked = { analyzeUrl: jest.fn(), submitPullRequest: jest.fn(), + preparePullRequest: jest.fn(), }; const catalogApi: jest.Mocked = { @@ -44,12 +46,16 @@ describe('', () => { removeEntityByUid: jest.fn(), }; + const errorApi: jest.Mocked = { + error$: jest.fn(), + post: jest.fn(), + }; + const Wrapper = ({ children }: { children?: React.ReactNode }) => ( {children} @@ -77,16 +83,19 @@ describe('', () => { beforeEach(() => { jest.resetAllMocks(); + + (catalogImportApi.preparePullRequest! as jest.Mock).mockResolvedValue({ + title: 'My title', + body: 'My **body**', + }); }); it('renders without exploding', async () => { catalogApi.getEntities.mockReturnValue(Promise.resolve({ items: [] })); await act(async () => { - const { getByText } = render( + const { findByText } = render( { @@ -105,8 +114,8 @@ describe('', () => { }, ); - const title = getByText('My title'); - const description = getByText('body', { selector: 'strong' }); + const title = await findByText('My title'); + const description = await findByText('body', { selector: 'strong' }); expect(title).toBeInTheDocument(); expect(title).toBeVisible(); expect(description).toBeInTheDocument(); @@ -124,10 +133,8 @@ describe('', () => { ); await act(async () => { - await render( + render( { @@ -154,11 +161,9 @@ describe('', () => { }, ); - await userEvent.type(await screen.getByLabelText('name'), '-changed'); - await userEvent.type(await screen.getByLabelText('owner'), '-changed'); - await userEvent.click( - await screen.getByRole('button', { name: /Create PR/i }), - ); + userEvent.type(await screen.findByLabelText('name'), '-changed'); + userEvent.type(await screen.findByLabelText('owner'), '-changed'); + userEvent.click(screen.getByRole('button', { name: /Create PR/i })); }); expect(catalogImportApi.submitPullRequest).toBeCalledTimes(1); @@ -212,10 +217,8 @@ spec: ); await act(async () => { - await render( + render( { @@ -234,8 +237,8 @@ spec: }, ); - await userEvent.click( - await screen.getByRole('button', { name: /Create PR/i }), + userEvent.click( + await screen.findByRole('button', { name: /Create PR/i }), ); }); @@ -261,10 +264,8 @@ spec: ); await act(async () => { - await render( + render( void; onGoBack?: () => void; - defaultTitle: string; - defaultBody: string; - renderFormFields: ( props: Pick< UseFormReturn, @@ -99,16 +96,30 @@ export const StepPrepareCreatePullRequest = ({ onPrepare, onGoBack, renderFormFields, - defaultTitle, - defaultBody, }: Props) => { const classes = useStyles(); const catalogApi = useApi(catalogApiRef); - const catalogInfoApi = useApi(catalogImportApiRef); + const catalogImportApi = useApi(catalogImportApiRef); + const errorApi = useApi(errorApiRef); const [submitted, setSubmitted] = useState(false); const [error, setError] = useState(); + const { + loading: prDefaultsLoading, + value: prDefaults, + error: prDefaultsError, + } = useAsync( + () => catalogImportApi.preparePullRequest!(), + [catalogImportApi.preparePullRequest], + ); + + useEffect(() => { + if (prDefaultsError) { + errorApi.post(prDefaultsError); + } + }, [prDefaultsError, errorApi]); + const { loading: groupsLoading, value: groups } = useAsync(async () => { const groupEntities = await catalogApi.getEntities({ filter: { kind: 'group' }, @@ -124,7 +135,7 @@ export const StepPrepareCreatePullRequest = ({ setSubmitted(true); try { - const pr = await catalogInfoApi.submitPullRequest({ + const pr = await catalogImportApi.submitPullRequest({ repositoryUrl: analyzeResult.url, title: data.title, body: data.body, @@ -171,7 +182,7 @@ export const StepPrepareCreatePullRequest = ({ analyzeResult.generatedEntities, analyzeResult.integrationType, analyzeResult.url, - catalogInfoApi, + catalogImportApi, onPrepare, ], ); @@ -184,79 +195,81 @@ export const StepPrepareCreatePullRequest = ({ a Pull Request that creates one. - - onSubmit={handleResult} - defaultValues={{ - title: defaultTitle, - body: defaultBody, - owner: - (analyzeResult.generatedEntities[0]?.spec?.owner as string) || '', - componentName: - analyzeResult.generatedEntities[0]?.metadata?.name || '', - useCodeowners: false, - }} - render={({ values, formState, register, setValue }) => ( - <> - {renderFormFields({ - values, - formState, - register, - setValue, - groups: groups ?? [], - groupsLoading, - })} + {!prDefaultsLoading && ( + + onSubmit={handleResult} + defaultValues={{ + title: prDefaults?.title ?? '', + body: prDefaults?.body ?? '', + owner: + (analyzeResult.generatedEntities[0]?.spec?.owner as string) || '', + componentName: + analyzeResult.generatedEntities[0]?.metadata?.name || '', + useCodeowners: false, + }} + render={({ values, formState, register, setValue }) => ( + <> + {renderFormFields({ + values, + formState, + register, + setValue, + groups: groups ?? [], + groupsLoading, + })} - - Preview Pull Request - + + Preview Pull Request + - + - - Preview Entities - + + Preview Entities + - - - {error && {error}} - - - {onGoBack && ( - - )} - - Create PR - - - - )} - /> + repositoryUrl={analyzeResult.url} + classes={{ + card: classes.previewCard, + cardContent: classes.previewCardContent, + }} + /> + + {error && {error}} + + + {onGoBack && ( + + )} + + Create PR + + + + )} + /> + )} ); };