From 8b9c8196f12aa666622c6b3ded2ba247acc49cc8 Mon Sep 17 00:00:00 2001
From: Patrik Oldsberg <poldsberg@gmail.com>
Date: Wed, 28 Oct 2020 14:37:21 +0100
Subject: [PATCH] catalog,register-component: always register locations using a
 'url' type

---
 .changeset/fresh-maps-complain.md             |  6 +++
 plugins/catalog/src/api/CatalogClient.ts      | 12 +++++-
 plugins/catalog/src/api/types.ts              |  7 +++-
 .../components/CatalogPage/CatalogPage.tsx    |  2 +-
 .../src/filter/useEntityFilterGroup.test.tsx  |  2 +-
 .../RegisterComponentForm.tsx                 | 39 +++----------------
 .../RegisterComponentPage.test.tsx            |  2 +-
 .../RegisterComponentPage.tsx                 | 15 +------
 .../components/TemplatePage/TemplatePage.tsx  |  9 +++--
 9 files changed, 39 insertions(+), 55 deletions(-)
 create mode 100644 .changeset/fresh-maps-complain.md

diff --git a/.changeset/fresh-maps-complain.md b/.changeset/fresh-maps-complain.md
new file mode 100644
index 0000000000..ef2021df37
--- /dev/null
+++ b/.changeset/fresh-maps-complain.md
@@ -0,0 +1,6 @@
+---
+'@backstage/plugin-catalog': minor
+'@backstage/plugin-register-component': patch
+---
+
+Locations registered through the catalog client now default to the 'url' type. The type selection dropdown in the register-component form has been removed.
diff --git a/plugins/catalog/src/api/CatalogClient.ts b/plugins/catalog/src/api/CatalogClient.ts
index 642183f916..40622da60a 100644
--- a/plugins/catalog/src/api/CatalogClient.ts
+++ b/plugins/catalog/src/api/CatalogClient.ts
@@ -19,7 +19,12 @@ import {
   Location,
   LOCATION_ANNOTATION,
 } from '@backstage/catalog-model';
-import { CatalogApi, EntityCompoundName } from './types';
+import {
+  AddLocationRequest,
+  AddLocationResponse,
+  CatalogApi,
+  EntityCompoundName,
+} from './types';
 import { DiscoveryApi } from '@backstage/core';
 
 export class CatalogClient implements CatalogApi {
@@ -87,7 +92,10 @@ export class CatalogClient implements CatalogApi {
     return this.getOptional(`/entities/by-name/${kind}/${namespace}/${name}`);
   }
 
-  async addLocation(type: string, target: string) {
+  async addLocation({
+    type = 'url',
+    target,
+  }: AddLocationRequest): Promise<AddLocationResponse> {
     const response = await fetch(
       `${await this.discoveryApi.getBaseUrl('catalog')}/locations`,
       {
diff --git a/plugins/catalog/src/api/types.ts b/plugins/catalog/src/api/types.ts
index 36c86f2e96..324a8d43fd 100644
--- a/plugins/catalog/src/api/types.ts
+++ b/plugins/catalog/src/api/types.ts
@@ -35,11 +35,16 @@ export interface CatalogApi {
     compoundName: EntityCompoundName,
   ): Promise<Entity | undefined>;
   getEntities(filter?: Record<string, string | string[]>): Promise<Entity[]>;
-  addLocation(type: string, target: string): Promise<AddLocationResponse>;
+  addLocation(location: AddLocationRequest): Promise<AddLocationResponse>;
   getLocationByEntity(entity: Entity): Promise<Location | undefined>;
   removeEntityByUid(uid: string): Promise<void>;
 }
 
+export type AddLocationRequest = {
+  type?: string;
+  target: string;
+};
+
 export type AddLocationResponse = {
   location: Location;
   entities: Entity[];
diff --git a/plugins/catalog/src/components/CatalogPage/CatalogPage.tsx b/plugins/catalog/src/components/CatalogPage/CatalogPage.tsx
index 00be785754..1a9a723f16 100644
--- a/plugins/catalog/src/components/CatalogPage/CatalogPage.tsx
+++ b/plugins/catalog/src/components/CatalogPage/CatalogPage.tsx
@@ -76,7 +76,7 @@ const CatalogPageContents = () => {
       const root = configApi.getConfig('catalog.exampleEntityLocations');
       for (const type of root.keys()) {
         for (const target of root.getStringArray(type)) {
-          promises.push(catalogApi.addLocation(type, target));
+          promises.push(catalogApi.addLocation({ target }));
         }
       }
       await Promise.all(promises);
diff --git a/plugins/catalog/src/filter/useEntityFilterGroup.test.tsx b/plugins/catalog/src/filter/useEntityFilterGroup.test.tsx
index acf4790ebf..3d11ffe7a9 100644
--- a/plugins/catalog/src/filter/useEntityFilterGroup.test.tsx
+++ b/plugins/catalog/src/filter/useEntityFilterGroup.test.tsx
@@ -30,7 +30,7 @@ describe('useEntityFilterGroup', () => {
   beforeEach(() => {
     catalogApi = {
       /* eslint-disable-next-line @typescript-eslint/no-unused-vars */
-      addLocation: jest.fn((_a, _b) => new Promise(() => {})),
+      addLocation: jest.fn(_a => new Promise(() => {})),
       getEntities: jest.fn(),
       getLocationByEntity: jest.fn(),
       getLocationById: jest.fn(),
diff --git a/plugins/register-component/src/components/RegisterComponentForm/RegisterComponentForm.tsx b/plugins/register-component/src/components/RegisterComponentForm/RegisterComponentForm.tsx
index bc433e1c36..56dd50ea67 100644
--- a/plugins/register-component/src/components/RegisterComponentForm/RegisterComponentForm.tsx
+++ b/plugins/register-component/src/components/RegisterComponentForm/RegisterComponentForm.tsx
@@ -19,15 +19,12 @@ import {
   Button,
   FormControl,
   FormHelperText,
-  InputLabel,
   LinearProgress,
-  MenuItem,
-  Select,
   TextField,
 } from '@material-ui/core';
 import { makeStyles } from '@material-ui/core/styles';
 import React from 'react';
-import { Controller, useForm } from 'react-hook-form';
+import { useForm } from 'react-hook-form';
 import { ComponentIdValidators } from '../../util/validate';
 
 const useStyles = makeStyles<BackstageTheme>(theme => ({
@@ -50,11 +47,11 @@ export type Props = {
 };
 
 export const RegisterComponentForm = ({ onSubmit, submitting }: Props) => {
-  const { control, register, handleSubmit, errors, formState } = useForm({
+  const { register, handleSubmit, errors, formState } = useForm({
     mode: 'onChange',
   });
   const classes = useStyles();
-  const hasErrors = !!errors.componentLocation;
+  const hasErrors = !!errors.entityLocation;
   const dirty = formState?.isDirty;
 
   return submitting ? (
@@ -71,10 +68,9 @@ export const RegisterComponentForm = ({ onSubmit, submitting }: Props) => {
           id="registerComponentInput"
           variant="outlined"
           label="Entity file URL"
-          data-testid="componentLocationInput"
           error={hasErrors}
           placeholder="https://example.com/user/some-service/blob/master/catalog-info.yaml"
-          name="componentLocation"
+          name="entityLocation"
           required
           margin="normal"
           helperText="Enter the full path to the catalog-info.yaml file in GitHub, GitLab, Bitbucket or Azure to start tracking your component."
@@ -84,36 +80,13 @@ export const RegisterComponentForm = ({ onSubmit, submitting }: Props) => {
           })}
         />
 
-        {errors.componentLocation && (
+        {errors.entityLocation && (
           <FormHelperText error={hasErrors} id="register-component-helper-text">
-            {errors.componentLocation.message}
+            {errors.entityLocation.message}
           </FormHelperText>
         )}
       </FormControl>
 
-      <FormControl variant="outlined" className={classes.select}>
-        <InputLabel id="scmLabel">Host type</InputLabel>
-        <Controller
-          control={control}
-          name="scmType"
-          defaultValue="AUTO"
-          render={({ onChange, onBlur, value }) => (
-            <Select
-              labelId="scmLabel"
-              id="scmSelect"
-              label="scmLabel"
-              value={value}
-              onChange={onChange}
-              onBlur={onBlur}
-            >
-              <MenuItem value="AUTO">Auto-detect</MenuItem>
-              <MenuItem value="gitlab">GitLab</MenuItem>
-              <MenuItem value="bitbucket/api">Bitbucket</MenuItem>
-              <MenuItem value="azure/api">Azure</MenuItem>
-            </Select>
-          )}
-        />
-      </FormControl>
       <Button
         variant="contained"
         color="primary"
diff --git a/plugins/register-component/src/components/RegisterComponentPage/RegisterComponentPage.test.tsx b/plugins/register-component/src/components/RegisterComponentPage/RegisterComponentPage.test.tsx
index e5496d1ea5..6b097085e9 100644
--- a/plugins/register-component/src/components/RegisterComponentPage/RegisterComponentPage.test.tsx
+++ b/plugins/register-component/src/components/RegisterComponentPage/RegisterComponentPage.test.tsx
@@ -35,7 +35,7 @@ const errorApi: jest.Mocked<typeof errorApiRef.T> = {
 
 const catalogApi: jest.Mocked<typeof catalogApiRef.T> = {
   /* eslint-disable-next-line @typescript-eslint/no-unused-vars */
-  addLocation: jest.fn((_a, _b) => new Promise(() => {})),
+  addLocation: jest.fn(_a => new Promise(() => {})),
   getEntities: jest.fn(),
   getLocationByEntity: jest.fn(),
   getLocationById: jest.fn(),
diff --git a/plugins/register-component/src/components/RegisterComponentPage/RegisterComponentPage.tsx b/plugins/register-component/src/components/RegisterComponentPage/RegisterComponentPage.tsx
index cd3eb1adbb..c7f94e2060 100644
--- a/plugins/register-component/src/components/RegisterComponentPage/RegisterComponentPage.tsx
+++ b/plugins/register-component/src/components/RegisterComponentPage/RegisterComponentPage.tsx
@@ -82,20 +82,9 @@ export const RegisterComponentPage = ({
 
   const handleSubmit = async (formData: Record<string, string>) => {
     setFormState(FormStates.Submitting);
-    const { scmType, componentLocation: target } = formData;
+    const { entityLocation: target } = formData;
     try {
-      const typeMapping = [
-        { url: /^https:\/\/gitlab\.com\/.*/, type: 'gitlab/api' },
-        { url: /^https:\/\/bitbucket\.org\/.*/, type: 'bitbucket/api' },
-        { url: /^https:\/\/dev\.azure\.com\/.*/, type: 'azure/api' },
-        { url: /.*/, type: 'github' },
-      ];
-
-      const type =
-        scmType === 'AUTO'
-          ? typeMapping.filter(item => item.url.test(target))[0].type
-          : scmType;
-      const data = await catalogApi.addLocation(type, target);
+      const data = await catalogApi.addLocation({ target });
 
       if (!isMounted()) return;
 
diff --git a/plugins/scaffolder/src/components/TemplatePage/TemplatePage.tsx b/plugins/scaffolder/src/components/TemplatePage/TemplatePage.tsx
index 7cd1ce0243..f99df26264 100644
--- a/plugins/scaffolder/src/components/TemplatePage/TemplatePage.tsx
+++ b/plugins/scaffolder/src/components/TemplatePage/TemplatePage.tsx
@@ -108,12 +108,15 @@ export const TemplatePage = () => {
   );
 
   const handleCreateComplete = async (job: Job) => {
-    const componentYaml = job.metadata.remoteUrl?.replace(
+    const target = job.metadata.remoteUrl?.replace(
       /\.git$/,
+      // TODO(Rugvip): This is not the location we want. As part of scaffodler v2 we
+      //               want this to be more flexible, but before that we might want
+      //               to update all templates to use catalog-info.yaml instead.
       '/blob/master/component-info.yaml',
     );
 
-    if (!componentYaml) {
+    if (!target) {
       errorApi.post(
         new Error(
           `Failed to find component-info.yaml file in ${job.metadata.remoteUrl}.`,
@@ -124,7 +127,7 @@ export const TemplatePage = () => {
 
     const {
       entities: [createdEntity],
-    } = await catalogApi.addLocation('github', componentYaml);
+    } = await catalogApi.addLocation({ target });
 
     setEntity((createdEntity as any) as TemplateEntityV1alpha1);
   };