From 737f79a53b000d382849994a98a9149f638d1ac2 Mon Sep 17 00:00:00 2001 From: Marek Calus Date: Mon, 9 Nov 2020 20:03:08 +0100 Subject: [PATCH] Apply catalog-import code review comments --- plugins/catalog-import/package.json | 1 + .../src/api/CatalogImportApi.ts | 11 +++----- .../src/api/CatalogImportClient.ts | 25 +++++++++---------- .../src/components/ComponentConfigDisplay.tsx | 10 ++++---- .../src/components/ImportComponentForm.tsx | 5 ++-- .../src/components/ImportFinished.tsx | 2 +- .../catalog-import/src/util/useGithubRepos.ts | 16 +++++------- plugins/catalog-import/tsconfig.json | 5 ---- 8 files changed, 31 insertions(+), 44 deletions(-) delete mode 100644 plugins/catalog-import/tsconfig.json diff --git a/plugins/catalog-import/package.json b/plugins/catalog-import/package.json index 7013ec12b5..d7f34584b4 100644 --- a/plugins/catalog-import/package.json +++ b/plugins/catalog-import/package.json @@ -30,6 +30,7 @@ "@material-ui/icons": "^4.9.1", "@material-ui/lab": "4.0.0-alpha.45", "@octokit/rest": "^18.0.6", + "git-url-parse": "^11.4.0", "react": "^16.13.1", "react-dom": "^16.13.1", "react-hook-form": "^6.6.0", diff --git a/plugins/catalog-import/src/api/CatalogImportApi.ts b/plugins/catalog-import/src/api/CatalogImportApi.ts index aba921b883..7ecb4a543b 100644 --- a/plugins/catalog-import/src/api/CatalogImportApi.ts +++ b/plugins/catalog-import/src/api/CatalogImportApi.ts @@ -23,16 +23,13 @@ export const catalogImportApiRef = createApiRef({ }); export interface CatalogImportApi { - submitPRToRepo(options: { - token: string; + submitPrToRepo(options: { + oAuthToken: string; owner: string; repo: string; fileContent: string; - }): Promise<{ link: string }>; - createRepositoryLocation(options: { - owner: string; - repo: string; - }): Promise; + }): Promise<{ link: string; location: string }>; + createRepositoryLocation(options: { location: string }): Promise; generateEntityDefinitions(options: { repo: string; }): Promise; diff --git a/plugins/catalog-import/src/api/CatalogImportClient.ts b/plugins/catalog-import/src/api/CatalogImportClient.ts index 7300db45dd..cef4b2da0e 100644 --- a/plugins/catalog-import/src/api/CatalogImportClient.ts +++ b/plugins/catalog-import/src/api/CatalogImportClient.ts @@ -20,8 +20,6 @@ import { CatalogImportApi } from './CatalogImportApi'; import { AnalyzeLocationResponse } from '@backstage/plugin-catalog-backend'; import { PartialEntity } from '../util/types'; -export const API_BASE_URL = '/api/catalog/locations'; - export class CatalogImportClient implements CatalogImportApi { private readonly discoveryApi: DiscoveryApi; @@ -59,11 +57,9 @@ export class CatalogImportClient implements CatalogImportApi { } async createRepositoryLocation({ - owner, - repo, + location, }: { - owner: string; - repo: string; + location: string; }): Promise { const response = await fetch( `${await this.discoveryApi.getBaseUrl('catalog')}/locations`, @@ -74,7 +70,7 @@ export class CatalogImportClient implements CatalogImportApi { method: 'POST', body: JSON.stringify({ type: 'github', - target: `https://github.com/${owner}/${repo}/blob/master/catalog-info.yaml`, + target: location, presence: 'optional', }), }, @@ -86,19 +82,19 @@ export class CatalogImportClient implements CatalogImportApi { } } - async submitPRToRepo({ - token, + async submitPrToRepo({ + oAuthToken, owner, repo, fileContent, }: { - token: string; + oAuthToken: string; owner: string; repo: string; fileContent: string; - }): Promise<{ link: string }> { + }): Promise<{ link: string; location: string }> { const octo = new Octokit({ - auth: token, + auth: oAuthToken, }); const branchName = 'backstage-integration'; @@ -176,7 +172,10 @@ export class CatalogImportClient implements CatalogImportApi { ); }); - return { link: pullRequestRespone.data.html_url }; + return { + link: pullRequestRespone.data.html_url, + location: `https://github.com/${owner}/${repo}/blob/${repoData.data.default_branch}/${fileName}`, + }; } } diff --git a/plugins/catalog-import/src/components/ComponentConfigDisplay.tsx b/plugins/catalog-import/src/components/ComponentConfigDisplay.tsx index a9a7ef118e..0265440fa6 100644 --- a/plugins/catalog-import/src/components/ComponentConfigDisplay.tsx +++ b/plugins/catalog-import/src/components/ComponentConfigDisplay.tsx @@ -27,18 +27,18 @@ type Props = { savePRLink: (PRLink: string) => void; }; -const ComponentConfigDisplay: React.FC = ({ +const ComponentConfigDisplay = ({ nextStep, configFile, savePRLink, -}) => { +}: Props) => { const [submitting, setSubmitting] = useState(false); const errorApi = useApi(errorApiRef); - const { submitPRToRepo } = useGithubRepos(); + const { submitPrToRepo } = useGithubRepos(); const onNext = useCallback(async () => { try { setSubmitting(true); - const result = await submitPRToRepo(configFile); + const result = await submitPrToRepo(configFile); savePRLink(result.link); setSubmitting(false); nextStep(); @@ -46,7 +46,7 @@ const ComponentConfigDisplay: React.FC = ({ setSubmitting(false); errorApi.post(e); } - }, [submitPRToRepo, configFile, nextStep, savePRLink, errorApi]); + }, [submitPrToRepo, configFile, nextStep, savePRLink, errorApi]); return ( diff --git a/plugins/catalog-import/src/components/ImportComponentForm.tsx b/plugins/catalog-import/src/components/ImportComponentForm.tsx index 5b26c27d09..da078e10f0 100644 --- a/plugins/catalog-import/src/components/ImportComponentForm.tsx +++ b/plugins/catalog-import/src/components/ImportComponentForm.tsx @@ -63,10 +63,9 @@ export const RegisterComponentForm = ({ nextStep, saveConfig }: Props) => { try { if (!isMounted()) return; - const repo = target.split('/').slice(-2).join('/'); - const config = await generateEntityDefinitions(repo); + const config = await generateEntityDefinitions(target); saveConfig({ - repo, + repo: target, config, }); nextStep(); diff --git a/plugins/catalog-import/src/components/ImportFinished.tsx b/plugins/catalog-import/src/components/ImportFinished.tsx index eee0ad61df..e7680dbf90 100644 --- a/plugins/catalog-import/src/components/ImportFinished.tsx +++ b/plugins/catalog-import/src/components/ImportFinished.tsx @@ -38,7 +38,7 @@ type Props = { PRLink: string; }; -export const ImportFinished: React.FC = ({ nextStep, PRLink }) => { +export const ImportFinished = ({ nextStep, PRLink }: Props) => { const classes = useStyles(); return ( diff --git a/plugins/catalog-import/src/util/useGithubRepos.ts b/plugins/catalog-import/src/util/useGithubRepos.ts index 9290d5e993..3ec4f56c68 100644 --- a/plugins/catalog-import/src/util/useGithubRepos.ts +++ b/plugins/catalog-import/src/util/useGithubRepos.ts @@ -23,16 +23,13 @@ export function useGithubRepos() { const api = useApi(catalogImportApiRef); const auth = useApi(githubAuthApiRef); - const submitPRToRepo = async (selectedRepo: ConfigSpec) => { + const submitPrToRepo = async (selectedRepo: ConfigSpec) => { const token = await auth.getAccessToken(['repo']); - const [ownerName, repoName] = [ - selectedRepo.repo.split('/')[0], - selectedRepo.repo.split('/')[1], - ]; + const [ownerName, repoName] = selectedRepo.repo.split('/').slice(-2); const submitPRResponse = await api - .submitPRToRepo({ - token, + .submitPrToRepo({ + oAuthToken: token, owner: ownerName, repo: repoName, fileContent: selectedRepo.config @@ -45,8 +42,7 @@ export function useGithubRepos() { await api .createRepositoryLocation({ - owner: selectedRepo.repo.split('/')[0], - repo: selectedRepo.repo.split('/')[1], + location: submitPRResponse.location, }) .catch(e => { throw new Error(`Failed to create repository location:\n${e.message}`); @@ -56,7 +52,7 @@ export function useGithubRepos() { }; return { - submitPRToRepo, + submitPrToRepo, generateEntityDefinitions: (repo: string) => api.generateEntityDefinitions({ repo }), }; diff --git a/plugins/catalog-import/tsconfig.json b/plugins/catalog-import/tsconfig.json deleted file mode 100644 index b663b01fa2..0000000000 --- a/plugins/catalog-import/tsconfig.json +++ /dev/null @@ -1,5 +0,0 @@ -{ - "extends": "../../tsconfig.json", - "include": ["src", "dev"], - "compilerOptions": {} -}