From be5fc422f72decd9e168eb59d855191f9cdd1e60 Mon Sep 17 00:00:00 2001 From: blam Date: Fri, 7 Jan 2022 18:24:48 +0100 Subject: [PATCH] chore: reworking some more stuff in the code review, better naming Signed-off-by: blam --- .../fields/RepoUrlPicker/RepoUrlPicker.tsx | 6 +++--- .../fields/RepoUrlPicker/RepoUrlPickerHost.tsx | 2 ++ .../fields/RepoUrlPicker/utils.test.ts | 16 ++++++++-------- .../src/components/fields/RepoUrlPicker/utils.ts | 11 ++++++++--- 4 files changed, 21 insertions(+), 14 deletions(-) diff --git a/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx b/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx index fea96dfe7f..cdce9512a9 100644 --- a/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx +++ b/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx @@ -22,7 +22,7 @@ import { AzureRepoPicker } from './AzureRepoPicker'; import { BitbucketRepoPicker } from './BitbucketRepoPicker'; import { FieldExtensionComponentProps } from '../../../extensions'; import { RepoUrlPickerHost } from './RepoUrlPickerHost'; -import { splitFormData, serializeFormData } from './utils'; +import { parseRepoPickerUrl, serializeRepoPickerUrl } from './utils'; export interface RepoUrlPickerUiOptions { allowedHosts?: string[]; @@ -42,14 +42,14 @@ export const RepoUrlPicker = ({ organization?: string; workspace?: string; project?: string; - }>(splitFormData(formData)); + }>(parseRepoPickerUrl(formData)); const integrationApi = useApi(scmIntegrationsApiRef); const allowedHosts = uiSchema?.['ui:options']?.allowedHosts ?? []; const allowedOwners = uiSchema?.['ui:options']?.allowedOwners ?? []; useEffect(() => { - onChange(serializeFormData(state)); + onChange(serializeRepoPickerUrl(state)); }, [state, onChange]); return ( diff --git a/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPickerHost.tsx b/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPickerHost.tsx index 1e201f1afe..15777fde81 100644 --- a/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPickerHost.tsx +++ b/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPickerHost.tsx @@ -42,6 +42,8 @@ export const RepoUrlPickerHost = ({ useEffect(() => { if (hosts && !host) { + // This is only hear to set the default as the first one in the hosts array + // if the host is not set yet and there is a list of hosts. onChange(hosts[0]); } }, [hosts, host, onChange]); diff --git a/plugins/scaffolder/src/components/fields/RepoUrlPicker/utils.test.ts b/plugins/scaffolder/src/components/fields/RepoUrlPicker/utils.test.ts index e6065ef737..2771d0faa5 100644 --- a/plugins/scaffolder/src/components/fields/RepoUrlPicker/utils.test.ts +++ b/plugins/scaffolder/src/components/fields/RepoUrlPicker/utils.test.ts @@ -13,17 +13,17 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { splitFormData, serializeFormData } from './utils'; +import { parseRepoPickerUrl, serializeRepoPickerUrl } from './utils'; describe('utils', () => { - describe('serializeFormData', () => { + describe('serializeRepoPickerUrl', () => { it('should return undefined when host is not set', () => { - expect(serializeFormData({})).toBeUndefined(); + expect(serializeRepoPickerUrl({})).toBeUndefined(); }); it('should set the correct owner and repo', () => { expect( - serializeFormData({ + serializeRepoPickerUrl({ host: 'github.com', owner: 'owner', repo: 'backstage', @@ -33,7 +33,7 @@ describe('utils', () => { it('should set correct other options', () => { expect( - serializeFormData({ + serializeRepoPickerUrl({ host: 'github.com', organization: 'organization', workspace: 'workspace', @@ -46,7 +46,7 @@ describe('utils', () => { it('should set all correct options', () => { expect( - serializeFormData({ + serializeRepoPickerUrl({ host: 'github.com', owner: 'owner', repo: 'backstage', @@ -60,10 +60,10 @@ describe('utils', () => { }); }); - describe('splitFormData', () => { + describe('parseRepoPickerUrl', () => { it('should parse a complete string', () => { expect( - splitFormData( + parseRepoPickerUrl( 'github.com?owner=owner&repo=backstage&organization=organization&workspace=workspace&project=backstage', ), ).toEqual({ diff --git a/plugins/scaffolder/src/components/fields/RepoUrlPicker/utils.ts b/plugins/scaffolder/src/components/fields/RepoUrlPicker/utils.ts index 8ef109f4c8..66c010ec3d 100644 --- a/plugins/scaffolder/src/components/fields/RepoUrlPicker/utils.ts +++ b/plugins/scaffolder/src/components/fields/RepoUrlPicker/utils.ts @@ -13,14 +13,17 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -export function serializeFormData(data: { + +type RepoUrlPickerOptions = { host?: string; owner?: string; repo?: string; organization?: string; workspace?: string; project?: string; -}) { +}; + +export function serializeRepoPickerUrl(data: RepoUrlPickerOptions) { if (!data.host) { return undefined; } @@ -45,7 +48,9 @@ export function serializeFormData(data: { return `${data.host}?${params.toString()}`; } -export function splitFormData(url: string | undefined) { +export function parseRepoPickerUrl( + url: string | undefined, +): RepoUrlPickerOptions { let host = undefined; let owner = undefined; let repo = undefined;