From eb1b63434d74aed4e5f4c7ec8ec8bc0feb4ab485 Mon Sep 17 00:00:00 2001 From: Debsmita Santra Date: Thu, 12 Sep 2024 21:18:13 +0530 Subject: [PATCH] fix(bulk-import): bulk import fixes (#2156) fix(bulk-import): bulk import bug fixes --- plugins/bulk-import/README.md | 27 +- .../AddRepositoriesFormFooter.tsx | 1 + .../AddRepositories/AddRepositoriesPage.tsx | 157 ++++----- .../src/components/BulkImportPage.tsx | 47 +-- .../PreviewFile/KeyValueTextField.tsx | 2 +- .../components/PreviewFile/PreviewFile.tsx | 2 +- .../PreviewFile/PreviewFileSidebar.tsx | 314 ++++++------------ .../PreviewFileSidebarDrawerContent.tsx | 204 ++++++++++++ .../PreviewFile/PreviewPullRequest.test.tsx | 117 ++++--- .../PreviewFile/PreviewPullRequest.tsx | 58 ++-- .../PreviewPullRequestForm.test.tsx | 3 + .../PreviewFile/PreviewPullRequests.test.tsx | 5 + .../Repositories/CatalogInfoAction.tsx | 66 +++- .../Repositories/EditCatalogInfo.tsx | 8 +- .../Repositories/RepositoriesList.test.tsx | 31 ++ .../Repositories/RepositoriesList.tsx | 1 + .../Repositories/RepositoriesListToolbar.tsx | 20 ++ .../src/hooks/useAddedRepositories.ts | 1 + .../bulk-import/src/hooks/useRepositories.ts | 1 + plugins/bulk-import/src/types/types.ts | 2 +- .../src/utils/repository-utils.test.tsx | 34 ++ .../src/utils/repository-utils.tsx | 45 ++- 22 files changed, 708 insertions(+), 438 deletions(-) create mode 100644 plugins/bulk-import/src/components/PreviewFile/PreviewFileSidebarDrawerContent.tsx diff --git a/plugins/bulk-import/README.md b/plugins/bulk-import/README.md index 6d0d6dd61c..b098e27474 100644 --- a/plugins/bulk-import/README.md +++ b/plugins/bulk-import/README.md @@ -10,8 +10,8 @@ This plugin allows bulk import of multiple catalog entities into the catalog. The sections below are relevant for static plugins. If the plugin is expected to be installed as a dynamic one: -- follow https://github.com/janus-idp/backstage-showcase/blob/main/showcase-docs/dynamic-plugins.md#installing-a-dynamic-plugin-package-in-the-showcase -- add content of `app-config.janus-idp.yaml` into `app-config.local.yaml`. +- Follow https://github.com/janus-idp/backstage-showcase/blob/main/showcase-docs/dynamic-plugins.md#installing-a-dynamic-plugin-package-in-the-showcase +- Add content of `app-config.janus-idp.yaml` into `app-config.local.yaml`. #### Prerequisites @@ -23,29 +23,6 @@ The sections below are relevant for static plugins. If the plugin is expected to **NOTE** -- When RBAC permission framework is enabled, for non-admin users wanting to access bulk import UI, the role associated with your user should have the following permission policies associated with it. Add the following in your permission policies configuration file: - -```CSV -p, role:default/team_a, catalog-entity.read, read, allow -p, role:default/team_a, catalog-entity.create, create, allow -g, user:default/, role:default/team_a -``` - -#### Installing as a dynamic plugin? - -The sections below are relevant for static plugins. If the plugin is expected to be installed as a dynamic one: - -- follow https://github.com/janus-idp/backstage-showcase/blob/main/showcase-docs/dynamic-plugins.md#installing-a-dynamic-plugin-package-in-the-showcase -- add content of `app-config.janus-idp.yaml` into `app-config.local.yaml`. - -#### Prerequisites - -Follow the Bulk import backend plugin [README](https://github.com/janus-idp/backstage-plugins/blob/main/plugins/bulk-import-backend/README.md) to integrate bulk import in your Backstage instance. - ---- - -**NOTE** - - When RBAC permission framework is enabled, for non-admin users to access bulk import UI, the role associated with your user should have the following permission policies associated with it. Add the following in your permission policies configuration file: ```CSV diff --git a/plugins/bulk-import/src/components/AddRepositories/AddRepositoriesFormFooter.tsx b/plugins/bulk-import/src/components/AddRepositories/AddRepositoriesFormFooter.tsx index 62d80d7883..89320c48b7 100644 --- a/plugins/bulk-import/src/components/AddRepositories/AddRepositoriesFormFooter.tsx +++ b/plugins/bulk-import/src/components/AddRepositories/AddRepositoriesFormFooter.tsx @@ -39,6 +39,7 @@ const useStyles = makeStyles(theme => ({ width: '100%', borderTopStyle: 'groove', border: theme.palette.divider, + zIndex: 1, }, })); diff --git a/plugins/bulk-import/src/components/AddRepositories/AddRepositoriesPage.tsx b/plugins/bulk-import/src/components/AddRepositories/AddRepositoriesPage.tsx index 6cf391fbe7..a1a5c26aec 100644 --- a/plugins/bulk-import/src/components/AddRepositories/AddRepositoriesPage.tsx +++ b/plugins/bulk-import/src/components/AddRepositories/AddRepositoriesPage.tsx @@ -47,6 +47,7 @@ const useStyles = makeStyles(() => ({ export const AddRepositoriesPage = () => { const theme = useTheme(); const classes = useStyles(); + const bulkImportApi = useApi(bulkImportApiRef); const navigate = useNavigate(); const [generalSubmitError, setGeneralSubmitError] = React.useState<{ message: string; @@ -59,7 +60,6 @@ export const AddRepositoriesPage = () => { approvalTool: ApprovalTool.Git, }; - const bulkImportApi = useApi(bulkImportApiRef); const bulkImportViewPermissionResult = usePermission({ permission: bulkImportPermission, resourceRef: bulkImportPermission.resourceType, @@ -102,7 +102,7 @@ export const AddRepositoriesPage = () => { formikHelpers.setStatus(createJobErrors); formikHelpers.setSubmitting(false); } else { - navigate(`../bulk-import/repositories`); + navigate(`..`); } } } @@ -115,25 +115,25 @@ export const AddRepositoriesPage = () => { } }; - return ( - -
- - {bulkImportViewPermissionResult.loading && } - {bulkImportViewPermissionResult.allowed ? ( - <> -
- - } - id="add-repository-summary" - > - - Add repositories to Red Hat Developer Hub in 4 steps - - - - {/* { + if (bulkImportViewPermissionResult.loading) { + return ; + } + if (bulkImportViewPermissionResult.allowed) { + return ( + <> +
+ + } + id="add-repository-summary" + > + + Add repositories to Red Hat Developer Hub in 4 steps + + + + {/* { } iconText="Choose approval tool (git/ServiceNow) for PR/ticket creation" /> */} - - - - - - -
- - - - - - - ) : ( -
- - Permission required - To add repositories, contact your administrator to give you the - `bulk.import` permission. - + + + + + +
- )} - + + + + + + + ); + } + return ( +
+ + Permission required + To add repositories, contact your administrator to give you the + `bulk.import` permission. + +
+ ); + }; + + return ( + +
+ {showContent()} ); }; diff --git a/plugins/bulk-import/src/components/BulkImportPage.tsx b/plugins/bulk-import/src/components/BulkImportPage.tsx index 46bf3dd131..18970457e4 100644 --- a/plugins/bulk-import/src/components/BulkImportPage.tsx +++ b/plugins/bulk-import/src/components/BulkImportPage.tsx @@ -33,32 +33,39 @@ export const BulkImportPage = () => { resourceRef: bulkImportPermission.resourceType, }); + const showContent = () => { + if (bulkImportViewPermissionResult.loading) { + return ; + } + if (bulkImportViewPermissionResult.allowed) { + return ( + {}} + > + + + + + ); + } + return ( + + Permission required + To view the added repositories, contact your administrator to give you + the `bulk.import` permission. + + ); + }; + return (
-
- {bulkImportViewPermissionResult.loading && } - {bulkImportViewPermissionResult.allowed ? ( - {}} - > - - - - - ) : ( - - Permission required - To view the added repositories, contact your administrator to - give you the `bulk.import` permission. - - )} -
+
{showContent()}
diff --git a/plugins/bulk-import/src/components/PreviewFile/KeyValueTextField.tsx b/plugins/bulk-import/src/components/PreviewFile/KeyValueTextField.tsx index a3ad79e03b..0729b83db1 100644 --- a/plugins/bulk-import/src/components/PreviewFile/KeyValueTextField.tsx +++ b/plugins/bulk-import/src/components/PreviewFile/KeyValueTextField.tsx @@ -55,7 +55,7 @@ const KeyValueTextField: React.FC = ({ return { ...formErrors, [repoId]: { - ...formErrors[repoId], + ...(formErrors?.[repoId] || {}), [fieldName]: validationError, }, }; diff --git a/plugins/bulk-import/src/components/PreviewFile/PreviewFile.tsx b/plugins/bulk-import/src/components/PreviewFile/PreviewFile.tsx index 19739db274..fcb2438540 100644 --- a/plugins/bulk-import/src/components/PreviewFile/PreviewFile.tsx +++ b/plugins/bulk-import/src/components/PreviewFile/PreviewFile.tsx @@ -60,7 +60,7 @@ export const PreviewFile = ({ data }: { data: AddRepositoryData }) => { - errorMessage.showRepositoryLink ? null : setOpenDrawer(true) + errorMessage.showRepositoryLink ? null : openDrawer(data) } data-testid="edit-pull-request" > diff --git a/plugins/bulk-import/src/components/PreviewFile/PreviewFileSidebar.tsx b/plugins/bulk-import/src/components/PreviewFile/PreviewFileSidebar.tsx index 2897d6d222..d9bd3affe3 100644 --- a/plugins/bulk-import/src/components/PreviewFile/PreviewFileSidebar.tsx +++ b/plugins/bulk-import/src/components/PreviewFile/PreviewFileSidebar.tsx @@ -1,31 +1,29 @@ import * as React from 'react'; -import { Link } from '@backstage/core-components'; -import { useApi } from '@backstage/core-plugin-api'; +import { + configApiRef, + identityApiRef, + useApi, +} from '@backstage/core-plugin-api'; -import { Button, Drawer, makeStyles } from '@material-ui/core'; -import { Skeleton } from '@material-ui/lab'; -import CloseIcon from '@mui/icons-material/Close'; -import OpenInNewIcon from '@mui/icons-material/OpenInNew'; -import Box from '@mui/material/Box'; -import CircularProgress from '@mui/material/CircularProgress'; -import IconButton from '@mui/material/IconButton'; -import Stack from '@mui/material/Stack'; -import Typography from '@mui/material/Typography'; +import { Drawer, makeStyles } from '@material-ui/core'; +import { useFormikContext } from 'formik'; import { bulkImportApiRef } from '../../api/BulkImportBackendClient'; import { + AddRepositoriesFormValues, AddRepositoryData, ImportJobStatus, PullRequestPreview, PullRequestPreviewData, - RepositorySelection, RepositoryStatus, RepositoryType, } from '../../types'; -import { evaluatePRTemplate, urlHelper } from '../../utils/repository-utils'; -import { PreviewPullRequest } from './PreviewPullRequest'; -import { PreviewPullRequests } from './PreviewPullRequests'; +import { + evaluatePRTemplate, + getPRTemplate, +} from '../../utils/repository-utils'; +import { PreviewFileSidebarDrawerContent } from './PreviewFileSidebarDrawerContent'; const useDrawerStyles = makeStyles(theme => ({ paper: { @@ -37,178 +35,6 @@ const useDrawerStyles = makeStyles(theme => ({ }, })); -const useDrawerContentStyles = makeStyles(theme => ({ - createButton: { - marginRight: theme.spacing(1), - }, - header: { - display: 'flex', - flexDirection: 'row', - justifyContent: 'space-between', - alignItems: 'baseline', - padding: theme.spacing(2.5), - }, - body: { - padding: theme.spacing(2.5), - paddingTop: theme.spacing(1), - paddingBottom: theme.spacing(1), - marginBottom: '100px', - flexGrow: 1, - }, - footer: { - flexDirection: 'row', - gap: '14px', - paddingTop: theme.spacing(2.5), - display: 'flex', - justifyContent: 'left', - position: 'fixed', - bottom: 0, - paddingLeft: '24px', - paddingBottom: '24px', - backgroundColor: theme.palette.background.paper, - width: '100%', - borderTopStyle: 'groove', - border: theme.palette.divider, - zIndex: 1, - }, -})); - -const DrawerContent = ({ - repositoryType, - onCancel, - isLoading, - isSubmitting, - data, - pullRequest, - onSave, - setPullRequest, -}: { - repositoryType: string; - onCancel: () => void; - isLoading: boolean; - isSubmitting: boolean | undefined; - data: AddRepositoryData; - pullRequest: PullRequestPreviewData; - onSave: (pullRequest: PullRequestPreviewData, _event: any) => void; - - setPullRequest: (pullRequest: PullRequestPreviewData) => void; -}) => { - const [formErrors, setFormErrors] = React.useState(); - const classes = useDrawerStyles(); - const contentClasses = useDrawerContentStyles(); - - if (isLoading) { - return ( - - - - - - - - - - - ); - } - return ( - <> - - -
- {repositoryType === RepositorySelection.Repository ? ( - <> - - {`${data.orgName ?? data.organizationUrl}/${data.repoName}`} - - - {urlHelper(data.repoUrl || '')} - - - - ) : ( - <> - {`${data.orgName}`} - - {urlHelper(data.organizationUrl || '')} - - - - )} -
- - - - -
- - - {repositoryType === RepositorySelection.Repository && ( - - )} - {repositoryType === RepositorySelection.Organization && ( - - )} - -
-
- - - - -
- - ); -}; - export const PreviewFileSidebar = ({ open, onClose, @@ -224,52 +50,116 @@ export const PreviewFileSidebar = ({ handleSave: (pullRequest: PullRequestPreviewData, _event: any) => void; isSubmitting?: boolean; }) => { + const { setStatus, status } = useFormikContext(); const classes = useDrawerStyles(); const bulkImportApi = useApi(bulkImportApiRef); + const identityApi = useApi(identityApiRef); + const configApi = useApi(configApiRef); const [pullRequest, setPullRequest] = React.useState( {}, ); const [isInitialized, setIsInitialized] = React.useState(false); + const fetchPullRequestData = async ( + id: string, + repoName: string, + orgName: string, + url: string, + branch: string, + repoPrTemplate: PullRequestPreview, + ) => { + const result = await bulkImportApi.getImportAction( + url || '', + branch || 'main', + ); + if ((result as Response)?.statusText) { + setStatus({ + ...status, + errors: { + ...(status?.errors || {}), + [data.id]: { + error: { + title: (result as Response)?.statusText, + message: [ + `Failed to fetch the pull request. A new YAML has been generated below.`, + ], + }, + }, + }, + }); + return repoPrTemplate; + } else if ( + (result as ImportJobStatus)?.status === RepositoryStatus.WAIT_PR_APPROVAL + ) { + const importJobResult = result as ImportJobStatus; + const evaluatedPRTemplate = evaluatePRTemplate(importJobResult); + let pullReqPreview = { ...evaluatedPRTemplate.pullReqPreview }; + + if (evaluatedPRTemplate.isInvalidEntity) { + const identityRef = await identityApi.getBackstageIdentity(); + const baseUrl = configApi.getString('app.baseUrl'); + const prTemp = getPRTemplate( + repoName, + orgName, + identityRef.userEntityRef || 'user:default/guest', + baseUrl as string, + url, + branch, + ); + delete prTemp.prDescription; + delete prTemp.prTitle; + + setStatus({ + ...status, + infos: { + ...(status?.infos || {}), + [id]: { + error: { + message: [ + `The entity YAML in your pull request is invalid (empty file or missing apiVersion, kind, or metadata.name). A new YAML has been generated below.`, + ], + }, + }, + }, + }); + pullReqPreview = { + ...prTemp, + prDescription: pullReqPreview.prDescription || '', + prTitle: pullReqPreview.prTitle || '', + }; + } + return pullReqPreview; + } + return repoPrTemplate; + }; + const initializePullRequest = React.useCallback(async () => { const newPullRequestData: PullRequestPreviewData = {}; if (Object.keys(data?.selectedRepositories || [])?.length > 0) { for (const repo of Object.values(data?.selectedRepositories || [])) { - const result = await bulkImportApi.getImportAction( + newPullRequestData[repo.id] = await fetchPullRequestData( + repo.id, + repo.repoName || '', + repo.orgName || '', repo.repoUrl || '', repo.defaultBranch || 'main', + repo.catalogInfoYaml?.prTemplate as PullRequestPreview, ); - if ( - (result as ImportJobStatus)?.status === - RepositoryStatus.WAIT_PR_APPROVAL - ) { - const prTemplate = evaluatePRTemplate(result as ImportJobStatus); - newPullRequestData[repo.id] = prTemplate; - } else { - newPullRequestData[repo.id] = repo.catalogInfoYaml - ?.prTemplate as PullRequestPreview; - } } } else { - const result = await bulkImportApi.getImportAction( + newPullRequestData[data.id] = await fetchPullRequestData( + data.id, + data.repoName || '', + data.orgName || '', data.repoUrl || '', data.defaultBranch || 'main', + data.catalogInfoYaml?.prTemplate as PullRequestPreview, ); - if ( - (result as ImportJobStatus)?.status === - RepositoryStatus.WAIT_PR_APPROVAL - ) { - const prTemplate = evaluatePRTemplate(result as ImportJobStatus); - newPullRequestData[data.id] = prTemplate; - } else { - newPullRequestData[data.id] = data.catalogInfoYaml - ?.prTemplate as PullRequestPreview; - } } - setPullRequest(newPullRequestData); setIsInitialized(true); - }, [data, bulkImportApi]); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [data, bulkImportApi, setStatus, status]); React.useEffect(() => { if (!isInitialized && data?.id) { @@ -296,7 +186,7 @@ export const PreviewFileSidebar = ({ paper: classes.paper, }} > - ({ + paper: { + width: '40%', + gap: '3%', + }, + body: { + padding: theme.spacing(2.5), + }, +})); + +const useDrawerContentStyles = makeStyles(theme => ({ + createButton: { + marginRight: theme.spacing(1), + }, + header: { + display: 'flex', + flexDirection: 'row', + justifyContent: 'space-between', + alignItems: 'baseline', + padding: theme.spacing(2.5), + }, + body: { + padding: theme.spacing(2.5), + paddingTop: theme.spacing(1), + paddingBottom: theme.spacing(1), + marginBottom: '100px', + flexGrow: 1, + }, + footer: { + flexDirection: 'row', + gap: '14px', + paddingTop: theme.spacing(2.5), + display: 'flex', + justifyContent: 'left', + position: 'fixed', + bottom: 0, + paddingLeft: '24px', + paddingBottom: '24px', + backgroundColor: theme.palette.background.paper, + width: '100%', + borderTopStyle: 'groove', + border: theme.palette.divider, + zIndex: 1, + }, +})); + +export const PreviewFileSidebarDrawerContent = ({ + repositoryType, + onCancel, + isLoading, + isSubmitting, + data, + pullRequest, + onSave, + setPullRequest, +}: { + repositoryType: string; + onCancel: () => void; + isLoading: boolean; + isSubmitting: boolean | undefined; + data: AddRepositoryData; + pullRequest: PullRequestPreviewData; + onSave: (pullRequest: PullRequestPreviewData, _event: any) => void; + + setPullRequest: (pullRequest: PullRequestPreviewData) => void; +}) => { + const [formErrors, setFormErrors] = React.useState(); + const classes = useDrawerStyles(); + const contentClasses = useDrawerContentStyles(); + + if (isLoading) { + return ( + + + + + + + + + + + ); + } + return ( + <> + + +
+ {repositoryType === RepositorySelection.Repository ? ( + <> + + {`${data.orgName ?? data.organizationUrl}/${data.repoName}`} + + + {urlHelper(data.repoUrl || '')} + + + + ) : ( + <> + {`${data.orgName}`} + + {urlHelper(data.organizationUrl || '')} + + + + )} +
+ + + + +
+ + + {repositoryType === RepositorySelection.Repository && ( + + )} + {repositoryType === RepositorySelection.Organization && ( + + )} + +
+
+ + + + +
+ + ); +}; diff --git a/plugins/bulk-import/src/components/PreviewFile/PreviewPullRequest.test.tsx b/plugins/bulk-import/src/components/PreviewFile/PreviewPullRequest.test.tsx index 310d0c2318..9bcc3e30ac 100644 --- a/plugins/bulk-import/src/components/PreviewFile/PreviewPullRequest.test.tsx +++ b/plugins/bulk-import/src/components/PreviewFile/PreviewPullRequest.test.tsx @@ -1,5 +1,4 @@ import React, { useState } from 'react'; -import { useAsync } from 'react-use'; import { configApiRef } from '@backstage/core-plugin-api'; import { CatalogApi, catalogApiRef } from '@backstage/plugin-catalog-react'; @@ -8,10 +7,9 @@ import { MockConfigApi, TestApiProvider } from '@backstage/test-utils'; import { render } from '@testing-library/react'; import { useFormikContext } from 'formik'; -import { bulkImportApiRef } from '../../api/BulkImportBackendClient'; -import { mockGetImportJobs, mockGetRepositories } from '../../mocks/mockData'; +import { mockGetRepositories } from '../../mocks/mockData'; import { mockEntities } from '../../mocks/mockEntities'; -import { ApprovalTool, ImportJobStatus, RepositoryStatus } from '../../types'; +import { ApprovalTool, RepositoryStatus } from '../../types'; import { getPRTemplate } from '../../utils/repository-utils'; import { PreviewPullRequest } from './PreviewPullRequest'; @@ -25,30 +23,12 @@ jest.mock('formik', () => ({ useFormikContext: jest.fn(), })); -jest.mock('react-use', () => ({ - ...jest.requireActual('react-use'), - useAsync: jest.fn().mockReturnValue({ loading: false }), -})); - -class MockBulkImportApi { - async getImportAction( - repo: string, - _defaultBranch: string, - ): Promise { - return mockGetImportJobs.find( - i => i.repository.url === repo, - ) as ImportJobStatus; - } -} - const setState = jest.fn(); beforeEach(() => { (useState as jest.Mock).mockImplementation(initial => [initial, setState]); }); -const mockBulkImportApi = new MockBulkImportApi(); - const mockCatalogApi: Partial = { getEntities: jest.fn().mockReturnValue(mockEntities), }; @@ -93,7 +73,6 @@ describe('Preview Pull Request', () => { }), ], [catalogApiRef, mockCatalogApi], - [bulkImportApiRef, mockBulkImportApi], ]} > { 'user:default/guest', 'https://localhost:3001', 'https://github.com/org/dessert/cupcake', + 'main', ), }} setFormErrors={() => jest.fn()} @@ -162,7 +142,6 @@ describe('Preview Pull Request', () => { }), ], [catalogApiRef, mockCatalogApi], - [bulkImportApiRef, mockBulkImportApi], ]} > { 'user:default/guest', 'https://localhost:3001', 'https://github.com/org/dessert/cupcake', + 'main', ), }} setFormErrors={() => jest.fn()} @@ -192,19 +172,69 @@ describe('Preview Pull Request', () => { }); it('should show PR link in the info if the job has a PR waiting for approval', async () => { - (useAsync as jest.Mock).mockReturnValue({ - loading: false, - value: { - github: { - pullRequest: { - url: 'https://github.com/org/dessert/cupcake', - }, + (useFormikContext as jest.Mock).mockReturnValue({ + errors: {}, + status: {}, + values: { + repositories: { + 'org/dessert/cupcake': mockGetRepositories.repositories[0], }, + approvalTool: ApprovalTool.Git, }, }); + + const { getByTestId } = render( + + jest.fn()} + formErrors={{}} + setPullRequest={() => jest.fn()} + /> + , + ); + expect(getByTestId('pull-request-info')).toBeInTheDocument(); + }); + + it('should show an info if entity YAML is invalid', async () => { (useFormikContext as jest.Mock).mockReturnValue({ errors: {}, - status: {}, + status: { + infos: { + ['org/dessert/cupcake']: { + error: { message: ['invalid entity YAML'] }, + }, + }, + }, values: { repositories: { 'org/dessert/cupcake': mockGetRepositories.repositories[0], @@ -213,7 +243,7 @@ describe('Preview Pull Request', () => { }, }); - const { getByTestId } = render( + const { getByTestId, getByText } = render( { }), ], [catalogApiRef, mockCatalogApi], - [bulkImportApiRef, mockBulkImportApi], ]} > { repoUrl="https://github.com/org/dessert/cupcake" repoBranch="main" pullRequest={{ - 'org/dessert/cupcake': getPRTemplate( - 'org/dessert/cupcake', - 'org/dessert', - 'user:default/guest', - 'https://localhost:3001', - 'https://github.com/org/dessert/cupcake', - ), + 'org/dessert/cupcake': { + ...getPRTemplate( + 'org/dessert/cupcake', + 'org/dessert', + 'user:default/guest', + 'https://localhost:3001', + 'https://github.com/org/dessert/cupcake', + 'main', + ), + pullRequestUrl: 'https://github.com/org/dessert/cupcake/pulls/9', + }, }} setFormErrors={() => jest.fn()} formErrors={{}} @@ -249,6 +282,8 @@ describe('Preview Pull Request', () => { /> , ); + expect(getByTestId('other-info')).toBeInTheDocument(); + expect(getByText('invalid entity YAML')).toBeInTheDocument(); expect(getByTestId('pull-request-info')).toBeInTheDocument(); }); }); diff --git a/plugins/bulk-import/src/components/PreviewFile/PreviewPullRequest.tsx b/plugins/bulk-import/src/components/PreviewFile/PreviewPullRequest.tsx index cd12253ef6..56478111d5 100644 --- a/plugins/bulk-import/src/components/PreviewFile/PreviewPullRequest.tsx +++ b/plugins/bulk-import/src/components/PreviewFile/PreviewPullRequest.tsx @@ -1,19 +1,16 @@ import * as React from 'react'; -import { useAsync } from 'react-use'; import { Link } from '@backstage/core-components'; -import { useApi } from '@backstage/core-plugin-api'; import { Alert, AlertTitle } from '@material-ui/lab'; import OpenInNewIcon from '@mui/icons-material/OpenInNew'; import Box from '@mui/material/Box'; import { useFormikContext } from 'formik'; -import { bulkImportApiRef } from '../../api/BulkImportBackendClient'; import { AddRepositoriesFormValues, - ImportJobStatus, PullRequestPreviewData, + RepositoryStatus, } from '../../types'; import { getCustomisedErrorMessage } from '../../utils/repository-utils'; import { PreviewPullRequestForm } from './PreviewPullRequestForm'; @@ -21,7 +18,6 @@ import { PreviewPullRequestForm } from './PreviewPullRequestForm'; export const PreviewPullRequest = ({ repoId, repoUrl, - repoBranch, pullRequest, setPullRequest, formErrors, @@ -40,21 +36,18 @@ export const PreviewPullRequest = ({ setFormErrors: (pullRequest: PullRequestPreviewData) => void; }) => { const { status } = useFormikContext(); - const bulkImportApi = useApi(bulkImportApiRef); - const { value: repoStatus } = useAsync(async () => { - const result = await bulkImportApi.getImportAction( - repoUrl || '', - repoBranch || 'main', - ); - return result; - }); const [entityOwner, setEntityOwner] = React.useState(''); const error = status?.errors?.[repoId]; const info = status?.infos?.[repoId]; - if (info && !error) { - // prioritize error over info + if ( + info?.error?.message.includes( + RepositoryStatus.CATALOG_INFO_FILE_EXISTS_IN_REPO, + ) && + !error + ) { + // hide preview pull request form for this status return ( @@ -69,24 +62,37 @@ export const PreviewPullRequest = ({ {error && ( - Failed to create PR + + {error.error?.title ? error.error?.title : 'Failed to create PR'} + {getCustomisedErrorMessage(error.error.message).message}{' '} - - View repository - - + {error?.repository?.organization && error?.repository?.name && ( + + View repository + + + )} + + {!others?.addPaddingTop &&
} +
+ )} + {info && ( + + + {getCustomisedErrorMessage(info.error.message).message}{' '} + {!others?.addPaddingTop &&
}
)} - {(repoStatus as ImportJobStatus)?.github?.pullRequest?.url && ( + {pullRequest[repoId]?.pullRequestUrl && ( The{' '} - + pull request {' '} is pending approval diff --git a/plugins/bulk-import/src/components/PreviewFile/PreviewPullRequestForm.test.tsx b/plugins/bulk-import/src/components/PreviewFile/PreviewPullRequestForm.test.tsx index ecfa3f62a7..be133a4a5c 100644 --- a/plugins/bulk-import/src/components/PreviewFile/PreviewPullRequestForm.test.tsx +++ b/plugins/bulk-import/src/components/PreviewFile/PreviewPullRequestForm.test.tsx @@ -105,6 +105,7 @@ describe('Preview Pull Request Form', () => { 'user:default/guest', 'https://localhost:3001', 'https://github.com/org/dessert/cupcake', + 'main', ), }} setFormErrors={() => jest.fn()} @@ -159,6 +160,7 @@ describe('Preview Pull Request Form', () => { 'user:default/guest', 'https://localhost:3001', 'https://github.com/org/dessert/cupcake', + 'main', ), }} setFormErrors={() => jest.fn()} @@ -215,6 +217,7 @@ describe('Preview Pull Request Form', () => { 'user:default/guest', 'https://localhost:3001', 'https://github.com/org/dessert/cupcake', + 'main', ), }} setFormErrors={setFormErrors} diff --git a/plugins/bulk-import/src/components/PreviewFile/PreviewPullRequests.test.tsx b/plugins/bulk-import/src/components/PreviewFile/PreviewPullRequests.test.tsx index d0b400031f..7ca7835857 100644 --- a/plugins/bulk-import/src/components/PreviewFile/PreviewPullRequests.test.tsx +++ b/plugins/bulk-import/src/components/PreviewFile/PreviewPullRequests.test.tsx @@ -100,6 +100,7 @@ describe('Preview Pull Requests', () => { 'user:default/guest', 'https://localhost:3001', 'https://github.com/org/dessert/cupcake', + 'main', ), }} setFormErrors={() => jest.fn()} @@ -156,6 +157,7 @@ describe('Preview Pull Requests', () => { 'user:default/guest', 'https://localhost:3001', 'https://github.com/org/dessert/cupcake', + 'main', ), 'org/dessert/donut': getPRTemplate( 'org/dessert/donut', @@ -163,6 +165,7 @@ describe('Preview Pull Requests', () => { 'user:default/guest', 'https://localhost:3001', 'https://github.com/org/dessert/donut', + 'main', ), }} setFormErrors={() => jest.fn()} @@ -247,6 +250,7 @@ describe('Preview Pull Requests', () => { 'user:default/guest', 'https://localhost:3001', 'https://github.com/org/dessert/cupcake', + 'main', ), 'org/dessert/donut': getPRTemplate( 'org/dessert/donut', @@ -254,6 +258,7 @@ describe('Preview Pull Requests', () => { 'user:default/guest', 'https://localhost:3001', 'https://github.com/org/dessert/donut', + 'main', ), }} setFormErrors={() => jest.fn()} diff --git a/plugins/bulk-import/src/components/Repositories/CatalogInfoAction.tsx b/plugins/bulk-import/src/components/Repositories/CatalogInfoAction.tsx index 42400ccec6..ea699939c7 100644 --- a/plugins/bulk-import/src/components/Repositories/CatalogInfoAction.tsx +++ b/plugins/bulk-import/src/components/Repositories/CatalogInfoAction.tsx @@ -23,33 +23,39 @@ import { const CatalogInfoAction = ({ data }: { data: AddRepositoryData }) => { const { setDrawerData, setOpenDrawer, drawerData } = useDrawer(); + const { setStatus } = useFormikContext(); const { values } = useFormikContext(); const navigate = useNavigate(); const location = useLocation(); const searchParams = new URLSearchParams(location.search); const bulkImportApi = useApi(bulkImportApiRef); + const repoUrl = searchParams.get('repository'); + const defaultBranch = searchParams.get('defaultBranch'); + const { allowed } = usePermission({ permission: bulkImportPermission, resourceRef: bulkImportPermission.resourceType, }); - const { value } = useAsync( - async () => - await bulkImportApi.getImportAction( - data?.repoUrl || '', - data?.defaultBranch || 'main', - ), - [setOpenDrawer], - ); + const { value, loading } = useAsync(async () => { + if (repoUrl) { + return await bulkImportApi.getImportAction( + repoUrl, + defaultBranch || 'main', + ); + } + return null; + }, [repoUrl, defaultBranch]); - const openDrawer = (status: ImportJobStatus) => { + const handleOpenDrawer = (importStatus: ImportJobStatus) => { searchParams.set('repository', data.repoUrl || ''); + searchParams.set('defaultBranch', data.defaultBranch || 'main'); navigate({ pathname: location.pathname, search: `?${searchParams.toString()}`, }); setOpenDrawer(true); - setDrawerData(status); + setDrawerData(importStatus); }; const hasPermissionToEdit = @@ -57,16 +63,39 @@ const CatalogInfoAction = ({ data }: { data: AddRepositoryData }) => { values.repositories[data.id]?.catalogInfoYaml?.status === RepositoryStatus.WAIT_PR_APPROVAL; + const removeQueryParams = () => { + searchParams.delete('repository'); + searchParams.delete('defaultBranch'); + navigate({ + pathname: location.pathname, + search: `?${searchParams.toString()}`, + }); + }; + React.useEffect(() => { - const shouldOpenPanel = searchParams.get('repository'); - if (shouldOpenPanel) { - setOpenDrawer(true); - if (Object.keys(drawerData || {}).length === 0) { - setDrawerData(value as ImportJobStatus); + if (!loading && repoUrl && defaultBranch) { + const shouldOpenPanel = + value?.status === RepositoryStatus.WAIT_PR_APPROVAL && + values.repositories[(value as ImportJobStatus)?.repository?.id]; + + if ((value as Response)?.statusText) { + setOpenDrawer(false); + setStatus({ + title: (value as Response)?.statusText, + url: (value as Response)?.url, + }); + removeQueryParams(); + } else if (shouldOpenPanel) { + setOpenDrawer(true); + if (Object.keys(drawerData || {}).length === 0) { + setDrawerData(value as ImportJobStatus); + } + } else { + removeQueryParams(); } } // eslint-disable-next-line react-hooks/exhaustive-deps - }, [value]); + }, [repoUrl, defaultBranch, value?.status, values?.repositories, loading]); return ( { color="inherit" aria-label="Update" data-testid="update" - onClick={() => openDrawer(value as ImportJobStatus)} + onClick={() => handleOpenDrawer(value as ImportJobStatus)} > @@ -94,7 +123,8 @@ const CatalogInfoAction = ({ data }: { data: AddRepositoryData }) => { (); - const yamlContent = yaml.load( + const yamlContent = yaml.loadAll( importStatus?.github?.pullRequest?.catalogInfoContent, - ) as Entity; - const catalogEntityName = yamlContent.metadata?.name; - const entityOwner = yamlContent.spec?.owner as string; + )[0] as Entity; + const catalogEntityName = yamlContent?.metadata?.name; + const entityOwner = yamlContent?.spec?.owner as string; const previewData: AddRepositoryData = { id: importStatus?.repository?.id, diff --git a/plugins/bulk-import/src/components/Repositories/RepositoriesList.test.tsx b/plugins/bulk-import/src/components/Repositories/RepositoriesList.test.tsx index 2392778a22..2db43fbd27 100644 --- a/plugins/bulk-import/src/components/Repositories/RepositoriesList.test.tsx +++ b/plugins/bulk-import/src/components/Repositories/RepositoriesList.test.tsx @@ -5,6 +5,7 @@ import { identityApiRef } from '@backstage/core-plugin-api'; import { TestApiProvider } from '@backstage/test-utils'; import { render, screen } from '@testing-library/react'; +import { useFormikContext } from 'formik'; import { useAddedRepositories } from '../../hooks'; import { mockGetImportJobs } from '../../mocks/mockData'; @@ -101,6 +102,10 @@ describe('RepositoriesList', () => { }); it('should have an add button and an Added repositories table', async () => { + (useFormikContext as jest.Mock).mockReturnValue({ + status: null, + setFieldValue: jest.fn(), + }); mockUseAddedRepositories.mockReturnValue(mockAsyncData); render( @@ -123,6 +128,10 @@ describe('RepositoriesList', () => { }); it('should render the component and display empty content when no data', async () => { + (useFormikContext as jest.Mock).mockReturnValue({ + status: null, + setFieldValue: jest.fn(), + }); mockUseAddedRepositories.mockReturnValue({ ...mockAsyncData, data: [] }); render( @@ -139,4 +148,26 @@ describe('RepositoriesList', () => { expect(emptyMessage).toBeInTheDocument(); expect(emptyMessage).toHaveTextContent('No records found'); }); + + it('should display an alert if get import job api fails', async () => { + (useFormikContext as jest.Mock).mockReturnValue({ + status: { + title: 'Not found', + url: 'https://xyz', + }, + setFieldValue: jest.fn(), + }); + mockUseAddedRepositories.mockReturnValue({ ...mockAsyncData, data: [] }); + render( + + + + + , + ); + + expect( + screen.getByText('Not found https://xyz', { exact: false }), + ).toBeInTheDocument(); + }); }); diff --git a/plugins/bulk-import/src/components/Repositories/RepositoriesList.tsx b/plugins/bulk-import/src/components/Repositories/RepositoriesList.tsx index 9ecef6de7b..c1293e032c 100644 --- a/plugins/bulk-import/src/components/Repositories/RepositoriesList.tsx +++ b/plugins/bulk-import/src/components/Repositories/RepositoriesList.tsx @@ -47,6 +47,7 @@ export const RepositoriesList = () => { const closeDrawer = () => { searchParams.delete('repository'); + searchParams.delete('defaultBranch'); navigate({ pathname: location.pathname, search: `?${searchParams.toString()}`, diff --git a/plugins/bulk-import/src/components/Repositories/RepositoriesListToolbar.tsx b/plugins/bulk-import/src/components/Repositories/RepositoriesListToolbar.tsx index 80fd1a8368..5af5665d68 100644 --- a/plugins/bulk-import/src/components/Repositories/RepositoriesListToolbar.tsx +++ b/plugins/bulk-import/src/components/Repositories/RepositoriesListToolbar.tsx @@ -3,6 +3,10 @@ import React from 'react'; import { LinkButton } from '@backstage/core-components'; import { makeStyles } from '@material-ui/core'; +import { Alert, AlertTitle } from '@material-ui/lab'; +import { useFormikContext } from 'formik'; + +import { AddRepositoriesFormValues } from '../../types'; const useStyles = makeStyles(theme => ({ toolbar: { @@ -19,9 +23,25 @@ const useStyles = makeStyles(theme => ({ })); export const RepositoriesListToolbar = () => { + const { status, setStatus } = useFormikContext(); const classes = useStyles(); + + const handleCloseAlert = () => { + setStatus(null); + }; return (
+ {status && ( + <> + handleCloseAlert()}> + + Error occured while fetching the pull request + + {`${status?.title} ${status?.url}`} + +
+ + )} { expect(componentNameRegex.test('s-cat12')).toBe(true); expect(componentNameRegex.test('s-cat@')).toBe(false); }); + + it('should load catalog info content and evaluate PR template', () => { + const importJobStatus = { + approvalTool: 'GIT', + github: { + pullRequest: { + number: 105, + url: 'https://github.com/che-electron/client/pull/105', + title: 'Add catalog-info.yaml config file', + body: 'This pull request adds a **Backstage entity metadata file**\nto this repository so that the component can\nbe added to the [software catalog](http://localhost:3000/catalog).\nAfter this pull request is merged, the component will become available.\nFor more information, read an [overview of the Backstage software catalog](https://backstage.io/docs/features/software-catalog/).\nView the import job in your app [here](http://localhost:3000/bulk-import/repositories?repository=https://github.com/che-electron/client&defaultBranch=master).', + catalogInfoContent: + 'apiVersion: backstage.io/v1alpha1\nkind: Component\nmetadata:\n name: client\n annotations:\n github.com/project-slug: che-electron/client\nspec:\n type: other\n lifecycle: unknown\n owner: user:default/debsmita1\n', + }, + }, + id: 'https://github.com/che-electron/client', + lastUpdate: '2024-09-11T18:37:28Z', + repository: { + url: 'https://github.com/che-electron/client', + name: 'client', + organization: 'che-electron', + id: 'che-electron/client', + defaultBranch: 'main', + }, + status: 'WAIT_PR_APPROVAL', + }; + expect(evaluatePRTemplate(importJobStatus).isInvalidEntity).toBeFalsy(); + + importJobStatus.github.pullRequest.catalogInfoContent = '---\n'; + expect(evaluatePRTemplate(importJobStatus).isInvalidEntity).toBeTruthy(); + importJobStatus.github.pullRequest.catalogInfoContent = + 'kind: Component\nmetadata:\n name: client\n annotations:\n github.com/project-slug: che-electron/client\nspec:\n type: other\n lifecycle: unknown\n owner: user:default/debsmita1\n'; + expect(evaluatePRTemplate(importJobStatus).isInvalidEntity).toBeTruthy(); + }); }); diff --git a/plugins/bulk-import/src/utils/repository-utils.tsx b/plugins/bulk-import/src/utils/repository-utils.tsx index ae53efc8f8..5cbd03c5e1 100644 --- a/plugins/bulk-import/src/utils/repository-utils.tsx +++ b/plugins/bulk-import/src/utils/repository-utils.tsx @@ -20,6 +20,7 @@ import { ImportStatus, JobErrors, Order, + PullRequestPreview, RepositorySelection, RepositoryStatus, } from '../types'; @@ -90,9 +91,10 @@ export const getPRTemplate = ( entityOwner: string, baseUrl: string, repositoryUrl: string, -) => { + defaultBranch: string, +): PullRequestPreview => { const importJobUrl = repositoryUrl - ? `${baseUrl}/bulk-import/repositories?repository=${repositoryUrl}` + ? `${baseUrl}/bulk-import/repositories?repository=${repositoryUrl}&defaultBranch=${defaultBranch}` : `${baseUrl}/bulk-import/repositories`; return { componentName, @@ -453,6 +455,9 @@ export const getCustomisedErrorMessage = ( ); } }); + if (!message) { + message = status?.join('\n') || ''; + } return { message, showRepositoryLink }; }; @@ -485,20 +490,32 @@ export const calculateLastUpdated = (dateString: string) => { return `${diffInSeconds} ${diffInSeconds > 1 ? 'seconds' : 'second'} ago`; }; -export const evaluatePRTemplate = (repositoryStatus: ImportJobStatus) => { - const entity = jsyaml.load( +export const evaluatePRTemplate = ( + repositoryStatus: ImportJobStatus, +): { pullReqPreview: PullRequestPreview; isInvalidEntity: boolean } => { + const entity = jsyaml.loadAll( repositoryStatus.github.pullRequest.catalogInfoContent, - ) as Entity; + )[0] as Entity; + const isInvalid = + !entity?.metadata?.name || !entity?.apiVersion || !entity?.kind; return { - prTitle: repositoryStatus.github.pullRequest.title, - prDescription: repositoryStatus.github.pullRequest.body, - prAnnotations: convertKeyValuePairsToString(entity.metadata.annotations), - prLabels: convertKeyValuePairsToString(entity.metadata.labels), - prSpec: convertKeyValuePairsToString(entity.spec as Record), - componentName: entity.metadata.name, - entityOwner: entity.spec?.owner as string, - useCodeOwnersFile: !entity.spec?.owner, - yaml: entity, + pullReqPreview: { + pullRequestUrl: repositoryStatus.github.pullRequest.url, + prTitle: repositoryStatus.github.pullRequest.title, + prDescription: repositoryStatus.github.pullRequest.body, + prAnnotations: convertKeyValuePairsToString( + entity?.metadata?.annotations, + ), + prLabels: convertKeyValuePairsToString(entity?.metadata?.labels), + prSpec: convertKeyValuePairsToString( + entity?.spec as Record, + ), + componentName: entity?.metadata?.name, + entityOwner: entity?.spec?.owner as string, + useCodeOwnersFile: !entity?.spec?.owner, + yaml: entity, + }, + isInvalidEntity: isInvalid, }; };