From 88191bb54ebf9e118f3c5fbbd2a7ab7b1e669bd6 Mon Sep 17 00:00:00 2001 From: NIKUNJ LALITKUMAR HUDKA Date: Wed, 17 Apr 2024 17:39:41 -0300 Subject: [PATCH 1/8] bugfix: Proper error thrown in plugin-azure-devops-backend when gitRepository is not found. Signed-off-by: NIKUNJ LALITKUMAR HUDKA --- .changeset/three-sheep-remember.md | 5 +++ .../src/api/AzureDevOpsApi.test.ts | 37 +++++++++++++++++++ .../src/api/AzureDevOpsApi.ts | 20 ++++++++++ 3 files changed, 62 insertions(+) create mode 100644 .changeset/three-sheep-remember.md diff --git a/.changeset/three-sheep-remember.md b/.changeset/three-sheep-remember.md new file mode 100644 index 0000000000..51ffc8e3db --- /dev/null +++ b/.changeset/three-sheep-remember.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-azure-devops-backend': minor +--- + +Fixed bug in plugin-azure-devops-backend where proper error was not thrown when gitRepository was not found. diff --git a/plugins/azure-devops-backend/src/api/AzureDevOpsApi.test.ts b/plugins/azure-devops-backend/src/api/AzureDevOpsApi.test.ts index 174c5ff6cd..a056beee02 100644 --- a/plugins/azure-devops-backend/src/api/AzureDevOpsApi.test.ts +++ b/plugins/azure-devops-backend/src/api/AzureDevOpsApi.test.ts @@ -465,6 +465,43 @@ describe('AzureDevOpsApi', () => { ]); }); + it('should throw error when gitRepository is undefined', async () => { + const mockApi = { + getGitApi: jest.fn().mockReturnValue({}), + serverUrl: 'serverUrl', + }; + + (WebApi as unknown as jest.Mock).mockImplementation(() => mockApi); + + const api = AzureDevOpsApi.fromConfig(mockConfig, { + logger: mockLogger, + urlReader: mockUrlReader, + }); + + const pullRequestOptions: PullRequestOptions = { + top: 10, + status: PullRequestStatus.Active, + }; + + api.getGitRepository = jest.fn().mockResolvedValue(undefined); + + const temp = async () => { + try { + await api.getPullRequests('project', 'repo', pullRequestOptions); + return null; + } catch (error) { + return error; + } + }; + + const error = await temp(); + + expect(error).toHaveProperty( + 'message', + 'No repository found for Project "project" with Repository "repo" on host "undefined" under organization "undefined".', + ); + }); + it('should get build definitions', async () => { const mockBuilds: Build[] = [ { diff --git a/plugins/azure-devops-backend/src/api/AzureDevOpsApi.ts b/plugins/azure-devops-backend/src/api/AzureDevOpsApi.ts index 4ed54e63eb..468d6a9d88 100644 --- a/plugins/azure-devops-backend/src/api/AzureDevOpsApi.ts +++ b/plugins/azure-devops-backend/src/api/AzureDevOpsApi.ts @@ -231,6 +231,11 @@ export class AzureDevOpsApi { host, org, ); + if (!gitRepository) { + throw new Error( + `No repository found for Project "${projectName}" with Repository "${repoName}" on host "${host}" under organization "${org}".`, + ); + } const buildList = await this.getBuildList( projectName, gitRepository.id as string, @@ -262,6 +267,11 @@ export class AzureDevOpsApi { host, org, ); + if (!gitRepository) { + throw new Error( + `No repository found for Project "${projectName}" with Repository "${repoName}" on host "${host}" under organization "${org}".`, + ); + } const webApi = await this.getWebApi(host, org); const client = await webApi.getGitApi(); const tagRefs: GitRef[] = await client.getRefs( @@ -304,6 +314,11 @@ export class AzureDevOpsApi { host, org, ); + if (!gitRepository) { + throw new Error( + `No repository found for Project "${projectName}" with Repository "${repoName}" on host "${host}" under organization "${org}".`, + ); + } const webApi = await this.getWebApi(host, org); const client = await webApi.getGitApi(); const searchCriteria: GitPullRequestSearchCriteria = { @@ -514,6 +529,11 @@ export class AzureDevOpsApi { host, org, ); + if (!gitRepository) { + throw new Error( + `No repository found for Project "${projectName}" with Repository "${repoName}" on host "${host}" under organization "${org}".`, + ); + } repoId = gitRepository.id; } From dbde8c0d6fa8d5937faad7bb347c4ab5440143b0 Mon Sep 17 00:00:00 2001 From: NIKUNJ LALITKUMAR HUDKA Date: Fri, 19 Apr 2024 09:46:44 -0300 Subject: [PATCH 2/8] feat: EntityListComponent now uses Catalog Presentation API Signed-off-by: NIKUNJ LALITKUMAR HUDKA --- plugins/catalog-import/package.json | 1 + .../EntityListComponent.tsx | 43 ++++-- .../StepPrepareSelectLocations.test.tsx | 126 ++++++++++++------ 3 files changed, 118 insertions(+), 52 deletions(-) diff --git a/plugins/catalog-import/package.json b/plugins/catalog-import/package.json index 67215f6a74..8a882206a3 100644 --- a/plugins/catalog-import/package.json +++ b/plugins/catalog-import/package.json @@ -78,6 +78,7 @@ "@backstage/cli": "workspace:^", "@backstage/core-app-api": "workspace:^", "@backstage/dev-utils": "workspace:^", + "@backstage/plugin-catalog": "workspace:^", "@backstage/test-utils": "workspace:^", "@testing-library/dom": "^10.0.0", "@testing-library/jest-dom": "^6.0.0", diff --git a/plugins/catalog-import/src/components/EntityListComponent/EntityListComponent.tsx b/plugins/catalog-import/src/components/EntityListComponent/EntityListComponent.tsx index ef57fad204..9c4af562e5 100644 --- a/plugins/catalog-import/src/components/EntityListComponent/EntityListComponent.tsx +++ b/plugins/catalog-import/src/components/EntityListComponent/EntityListComponent.tsx @@ -14,11 +14,15 @@ * limitations under the License. */ -import { Entity, CompoundEntityRef } from '@backstage/catalog-model'; -import { useApp } from '@backstage/core-plugin-api'; +import { + Entity, + CompoundEntityRef, + stringifyEntityRef, +} from '@backstage/catalog-model'; +import { useApi, useApp } from '@backstage/core-plugin-api'; import { EntityRefLink, - humanizeEntityRef, + entityPresentationApiRef, } from '@backstage/plugin-catalog-react'; import Collapse from '@material-ui/core/Collapse'; import IconButton from '@material-ui/core/IconButton'; @@ -38,12 +42,6 @@ const useStyles = makeStyles(theme => ({ }, })); -function sortEntities(entities: Array) { - return entities.sort((a, b) => - humanizeEntityRef(a).localeCompare(humanizeEntityRef(b)), - ); -} - /** * Props for {@link EntityListComponent}. * @@ -78,7 +76,7 @@ export const EntityListComponent = (props: EntityListComponentProps) => { const app = useApp(); const classes = useStyles(); - + const entityPresentationApi = useApi(entityPresentationApiRef); const [expandedUrls, setExpandedUrls] = useState([]); const handleClick = (url: string) => { @@ -87,6 +85,17 @@ export const EntityListComponent = (props: EntityListComponentProps) => { ); }; + function sortEntities(entities: Array) { + return entities.sort((a, b) => + entityPresentationApi + .forEntity(stringifyEntityRef(a)) + .snapshot.entityRef.localeCompare( + entityPresentationApi.forEntity(stringifyEntityRef(b)).snapshot + .entityRef, + ), + ); + } + return ( {firstListItem} @@ -129,7 +138,11 @@ export const EntityListComponent = (props: EntityListComponentProps) => { ); return ( { : {})} > {Icon && } - + ); })} diff --git a/plugins/catalog-import/src/components/StepPrepareSelectLocations/StepPrepareSelectLocations.test.tsx b/plugins/catalog-import/src/components/StepPrepareSelectLocations/StepPrepareSelectLocations.test.tsx index b5efd3fbbf..e5a4174859 100644 --- a/plugins/catalog-import/src/components/StepPrepareSelectLocations/StepPrepareSelectLocations.test.tsx +++ b/plugins/catalog-import/src/components/StepPrepareSelectLocations/StepPrepareSelectLocations.test.tsx @@ -14,14 +14,22 @@ * limitations under the License. */ -import { renderInTestApp } from '@backstage/test-utils'; +import { TestApiProvider, renderInTestApp } from '@backstage/test-utils'; import { act, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import React from 'react'; import { AnalyzeResult } from '../../api'; import { StepPrepareSelectLocations } from './StepPrepareSelectLocations'; +import { + CatalogApi, + catalogApiRef, + entityPresentationApiRef, +} from '@backstage/plugin-catalog-react'; +import { DefaultEntityPresentationApi } from '@backstage/plugin-catalog'; +import { Entity } from '@backstage/catalog-model'; describe('', () => { + let entities: Entity[]; const analyzeResult = { type: 'locations', locations: [ @@ -53,17 +61,43 @@ describe('', () => { ], } as Extract; + const catalogApi: jest.Mocked = { + getLocationById: jest.fn(), + getEntityByName: jest.fn(), + getEntities: jest.fn(async () => ({ items: entities })), + addLocation: jest.fn(), + getLocationByRef: jest.fn(), + removeEntityByUid: jest.fn(), + } as any; + let Wrapper: React.ComponentType>; + beforeEach(() => { jest.resetAllMocks(); + catalogApi.getEntities.mockResolvedValue({ items: entities }); + Wrapper = ({ children }: { children?: React.ReactNode }) => ( + + {children} + + ); }); it('renders display locations to be added', async () => { await renderInTestApp( - undefined} - onGoBack={() => undefined} - />, + + undefined} + onGoBack={() => undefined} + /> + , ); expect(screen.getByText('url-1')).toBeInTheDocument(); @@ -96,11 +130,13 @@ describe('', () => { } as Extract; await renderInTestApp( - undefined} - onGoBack={() => undefined} - />, + + undefined} + onGoBack={() => undefined} + /> + , ); expect(screen.getByText(/my-target/)).toBeInTheDocument(); @@ -112,11 +148,13 @@ describe('', () => { it('should select and deselect all', async () => { await renderInTestApp( - undefined} - onGoBack={() => undefined} - />, + + undefined} + onGoBack={() => undefined} + /> + , ); const checkboxes = screen.getAllByRole('checkbox'); @@ -144,15 +182,17 @@ describe('', () => { it('should preselect prepared locations', async () => { await renderInTestApp( - undefined} - onGoBack={() => undefined} - />, + + undefined} + onGoBack={() => undefined} + /> + , ); const checkboxes = screen.getAllByRole('checkbox'); @@ -164,11 +204,13 @@ describe('', () => { it('should select items', async () => { await renderInTestApp( - undefined} - onGoBack={() => undefined} - />, + + undefined} + onGoBack={() => undefined} + /> + , ); const checkboxes = screen.getAllByRole('checkbox'); @@ -193,11 +235,13 @@ describe('', () => { const onGoBack = jest.fn(); await renderInTestApp( - undefined} - onGoBack={onGoBack} - />, + + undefined} + onGoBack={onGoBack} + /> + , ); await act(async () => { @@ -211,11 +255,13 @@ describe('', () => { const onPrepare = jest.fn(); await renderInTestApp( - undefined} - />, + + undefined} + /> + , ); const checkboxes = screen.getAllByRole('checkbox'); From 82214e72c9a8abc6c39613841fc48f30210e39eb Mon Sep 17 00:00:00 2001 From: NIKUNJ LALITKUMAR HUDKA Date: Sat, 20 Apr 2024 14:18:23 -0300 Subject: [PATCH 3/8] Revert "bugfix: Proper error thrown in plugin-azure-devops-backend when gitRepository is not found." This reverts commit 88191bb54ebf9e118f3c5fbbd2a7ab7b1e669bd6. Signed-off-by: NIKUNJ LALITKUMAR HUDKA --- .changeset/three-sheep-remember.md | 5 --- .../src/api/AzureDevOpsApi.test.ts | 37 ------------------- .../src/api/AzureDevOpsApi.ts | 20 ---------- 3 files changed, 62 deletions(-) delete mode 100644 .changeset/three-sheep-remember.md diff --git a/.changeset/three-sheep-remember.md b/.changeset/three-sheep-remember.md deleted file mode 100644 index 51ffc8e3db..0000000000 --- a/.changeset/three-sheep-remember.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'@backstage/plugin-azure-devops-backend': minor ---- - -Fixed bug in plugin-azure-devops-backend where proper error was not thrown when gitRepository was not found. diff --git a/plugins/azure-devops-backend/src/api/AzureDevOpsApi.test.ts b/plugins/azure-devops-backend/src/api/AzureDevOpsApi.test.ts index a056beee02..174c5ff6cd 100644 --- a/plugins/azure-devops-backend/src/api/AzureDevOpsApi.test.ts +++ b/plugins/azure-devops-backend/src/api/AzureDevOpsApi.test.ts @@ -465,43 +465,6 @@ describe('AzureDevOpsApi', () => { ]); }); - it('should throw error when gitRepository is undefined', async () => { - const mockApi = { - getGitApi: jest.fn().mockReturnValue({}), - serverUrl: 'serverUrl', - }; - - (WebApi as unknown as jest.Mock).mockImplementation(() => mockApi); - - const api = AzureDevOpsApi.fromConfig(mockConfig, { - logger: mockLogger, - urlReader: mockUrlReader, - }); - - const pullRequestOptions: PullRequestOptions = { - top: 10, - status: PullRequestStatus.Active, - }; - - api.getGitRepository = jest.fn().mockResolvedValue(undefined); - - const temp = async () => { - try { - await api.getPullRequests('project', 'repo', pullRequestOptions); - return null; - } catch (error) { - return error; - } - }; - - const error = await temp(); - - expect(error).toHaveProperty( - 'message', - 'No repository found for Project "project" with Repository "repo" on host "undefined" under organization "undefined".', - ); - }); - it('should get build definitions', async () => { const mockBuilds: Build[] = [ { diff --git a/plugins/azure-devops-backend/src/api/AzureDevOpsApi.ts b/plugins/azure-devops-backend/src/api/AzureDevOpsApi.ts index 468d6a9d88..4ed54e63eb 100644 --- a/plugins/azure-devops-backend/src/api/AzureDevOpsApi.ts +++ b/plugins/azure-devops-backend/src/api/AzureDevOpsApi.ts @@ -231,11 +231,6 @@ export class AzureDevOpsApi { host, org, ); - if (!gitRepository) { - throw new Error( - `No repository found for Project "${projectName}" with Repository "${repoName}" on host "${host}" under organization "${org}".`, - ); - } const buildList = await this.getBuildList( projectName, gitRepository.id as string, @@ -267,11 +262,6 @@ export class AzureDevOpsApi { host, org, ); - if (!gitRepository) { - throw new Error( - `No repository found for Project "${projectName}" with Repository "${repoName}" on host "${host}" under organization "${org}".`, - ); - } const webApi = await this.getWebApi(host, org); const client = await webApi.getGitApi(); const tagRefs: GitRef[] = await client.getRefs( @@ -314,11 +304,6 @@ export class AzureDevOpsApi { host, org, ); - if (!gitRepository) { - throw new Error( - `No repository found for Project "${projectName}" with Repository "${repoName}" on host "${host}" under organization "${org}".`, - ); - } const webApi = await this.getWebApi(host, org); const client = await webApi.getGitApi(); const searchCriteria: GitPullRequestSearchCriteria = { @@ -529,11 +514,6 @@ export class AzureDevOpsApi { host, org, ); - if (!gitRepository) { - throw new Error( - `No repository found for Project "${projectName}" with Repository "${repoName}" on host "${host}" under organization "${org}".`, - ); - } repoId = gitRepository.id; } From e1174b01b20e4a29fc81b07f25798aebb2595729 Mon Sep 17 00:00:00 2001 From: NIKUNJ LALITKUMAR HUDKA Date: Sat, 20 Apr 2024 15:28:46 -0300 Subject: [PATCH 4/8] chore: changeset added for this feature Signed-off-by: NIKUNJ LALITKUMAR HUDKA --- .changeset/five-cows-crash.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/five-cows-crash.md diff --git a/.changeset/five-cows-crash.md b/.changeset/five-cows-crash.md new file mode 100644 index 0000000000..2c298dbad3 --- /dev/null +++ b/.changeset/five-cows-crash.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-catalog-import': minor +--- + +`EntityListComponent` uses `entityPresentationApi` instead of `humanizeEntityRef` to display Entity From 4946ed88513e70ed5787c4503096810097f994c3 Mon Sep 17 00:00:00 2001 From: NIKUNJ LALITKUMAR HUDKA Date: Sat, 20 Apr 2024 15:45:50 -0300 Subject: [PATCH 5/8] chore: yarn.lock updated with new dependencies Signed-off-by: NIKUNJ LALITKUMAR HUDKA --- yarn.lock | 1 + 1 file changed, 1 insertion(+) diff --git a/yarn.lock b/yarn.lock index 0a27eecdea..5b5aef8874 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5868,6 +5868,7 @@ __metadata: "@backstage/frontend-plugin-api": "workspace:^" "@backstage/integration": "workspace:^" "@backstage/integration-react": "workspace:^" + "@backstage/plugin-catalog": "workspace:^" "@backstage/plugin-catalog-common": "workspace:^" "@backstage/plugin-catalog-react": "workspace:^" "@backstage/test-utils": "workspace:^" From 163be6316365538dd8edd17be44123919dd36169 Mon Sep 17 00:00:00 2001 From: NIKUNJ LALITKUMAR HUDKA Date: Sun, 5 May 2024 12:20:51 -0300 Subject: [PATCH 6/8] chore: requested changes done Signed-off-by: NIKUNJ LALITKUMAR HUDKA --- .../EntityListComponent/EntityListComponent.tsx | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/plugins/catalog-import/src/components/EntityListComponent/EntityListComponent.tsx b/plugins/catalog-import/src/components/EntityListComponent/EntityListComponent.tsx index 9c4af562e5..58b541795d 100644 --- a/plugins/catalog-import/src/components/EntityListComponent/EntityListComponent.tsx +++ b/plugins/catalog-import/src/components/EntityListComponent/EntityListComponent.tsx @@ -21,6 +21,7 @@ import { } from '@backstage/catalog-model'; import { useApi, useApp } from '@backstage/core-plugin-api'; import { + EntityDisplayName, EntityRefLink, entityPresentationApiRef, } from '@backstage/plugin-catalog-react'; @@ -138,11 +139,7 @@ export const EntityListComponent = (props: EntityListComponentProps) => { ); return ( { {Icon && } } /> From f57e0314c835cd89b1d2d6a727242ff93ba4db89 Mon Sep 17 00:00:00 2001 From: NIKUNJ LALITKUMAR HUDKA Date: Wed, 8 May 2024 09:07:30 -0300 Subject: [PATCH 7/8] chore: render icons from EntityDisplayName Signed-off-by: NIKUNJ LALITKUMAR HUDKA --- .../components/EntityListComponent/EntityListComponent.tsx | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/plugins/catalog-import/src/components/EntityListComponent/EntityListComponent.tsx b/plugins/catalog-import/src/components/EntityListComponent/EntityListComponent.tsx index 58b541795d..12c7376f9a 100644 --- a/plugins/catalog-import/src/components/EntityListComponent/EntityListComponent.tsx +++ b/plugins/catalog-import/src/components/EntityListComponent/EntityListComponent.tsx @@ -149,11 +149,8 @@ export const EntityListComponent = (props: EntityListComponentProps) => { } : {})} > - {Icon && } - } + primary={} /> ); From f0b2a070844edf48c63ac8cf4c4209e02a2ecfc2 Mon Sep 17 00:00:00 2001 From: NIKUNJ LALITKUMAR HUDKA Date: Wed, 8 May 2024 09:32:38 -0300 Subject: [PATCH 8/8] chore: lint error resolved Signed-off-by: NIKUNJ LALITKUMAR HUDKA --- .../components/EntityListComponent/EntityListComponent.tsx | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/plugins/catalog-import/src/components/EntityListComponent/EntityListComponent.tsx b/plugins/catalog-import/src/components/EntityListComponent/EntityListComponent.tsx index 12c7376f9a..80a2d90fcc 100644 --- a/plugins/catalog-import/src/components/EntityListComponent/EntityListComponent.tsx +++ b/plugins/catalog-import/src/components/EntityListComponent/EntityListComponent.tsx @@ -19,7 +19,7 @@ import { CompoundEntityRef, stringifyEntityRef, } from '@backstage/catalog-model'; -import { useApi, useApp } from '@backstage/core-plugin-api'; +import { useApi } from '@backstage/core-plugin-api'; import { EntityDisplayName, EntityRefLink, @@ -75,7 +75,6 @@ export const EntityListComponent = (props: EntityListComponentProps) => { withLinks = false, } = props; - const app = useApp(); const classes = useStyles(); const entityPresentationApi = useApi(entityPresentationApiRef); const [expandedUrls, setExpandedUrls] = useState([]); @@ -134,9 +133,6 @@ export const EntityListComponent = (props: EntityListComponentProps) => { > {sortEntities(r.entities).map(entity => { - const Icon = app.getSystemIcon( - `kind:${entity.kind.toLocaleLowerCase('en-US')}`, - ); return (