From 98ae18b68f3eb7bd37046241da8f44cab2decd3c Mon Sep 17 00:00:00 2001 From: Peter Macdonald Date: Sun, 2 Oct 2022 16:28:33 +0200 Subject: [PATCH 1/5] Fixed the allowedOwners from resetting everytime you leave the drop-down or enter stuff in the next input field this should fix #13730 Signed-off-by: Peter Macdonald Fixed the allowedOwners from resetting everytime you leave the drop-down or enter stuff in the next input field this should fix #13730 (added changeset) Signed-off-by: Peter Macdonald Better, improved, test passing fix for the bug Signed-off-by: Peter Macdonald Changed changeset file to ignore allowedOwners with backticks Signed-off-by: Peter Macdonald --- .changeset/old-melons-bathe.md | 5 +++++ .../src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx | 4 ++-- 2 files changed, 7 insertions(+), 2 deletions(-) create mode 100644 .changeset/old-melons-bathe.md diff --git a/.changeset/old-melons-bathe.md b/.changeset/old-melons-bathe.md new file mode 100644 index 0000000000..1a0db03b6e --- /dev/null +++ b/.changeset/old-melons-bathe.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-scaffolder': patch +--- + +Fixed the `allowedOwners` from resetting itself when you make a selection or leave the Allowed Owners drop down and start typing in an input below it diff --git a/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx b/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx index 14fbfcee0b..75f5d998ab 100644 --- a/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx +++ b/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx @@ -106,7 +106,7 @@ export const RepoUrlPicker = ( if (allowedOwners.length > 0) { setState(prevState => ({ ...prevState, - owner: allowedOwners[0], + allowedOwners, })); } }, [setState, allowedOwners]); @@ -174,9 +174,9 @@ export const RepoUrlPicker = ( {hostType === 'github' && ( )} {hostType === 'gitlab' && ( From 0abb5fca3df7f25f0db5e0c3a6ca50ce053996cb Mon Sep 17 00:00:00 2001 From: Peter Macdonald Date: Wed, 5 Oct 2022 17:22:45 +0200 Subject: [PATCH 2/5] Updated useEffect Signed-off-by: Peter Macdonald --- .../fields/RepoUrlPicker/RepoUrlPicker.tsx | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx b/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx index 75f5d998ab..3261295716 100644 --- a/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx +++ b/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx @@ -94,28 +94,31 @@ export const RepoUrlPicker = ( /* we deal with calling the repo setting here instead of in each components for ease */ useEffect(() => { - if (allowedOrganizations.length > 0) { + if (allowedOrganizations.length > 0 && !state.organization) { setState(prevState => ({ ...prevState, organization: allowedOrganizations[0], })); } - }, [setState, allowedOrganizations]); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [setState, allowedOrganizations, state.organization]); useEffect(() => { - if (allowedOwners.length > 0) { + if (allowedOwners.length > 0 && !state.owner) { setState(prevState => ({ ...prevState, - allowedOwners, + owner: allowedOwners[0], })); } - }, [setState, allowedOwners]); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [setState, allowedOwners, state.owner]); useEffect(() => { - if (allowedRepos.length > 0) { + if (allowedRepos.length > 0 && !state.repoName) { setState(prevState => ({ ...prevState, repoName: allowedRepos[0] })); } - }, [setState, allowedRepos]); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [setState, allowedRepos, state.repoName]); const updateLocalState = useCallback( (newState: RepoUrlPickerState) => { From 8f8cc420780c826b23b921b1374cd60ffb61fd83 Mon Sep 17 00:00:00 2001 From: Peter Macdonald Date: Wed, 5 Oct 2022 22:35:25 +0200 Subject: [PATCH 3/5] Updated useEffect to fix #13730 Signed-off-by: Peter Macdonald --- .../fields/RepoUrlPicker/RepoUrlPicker.tsx | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx b/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx index 3261295716..d0c3108976 100644 --- a/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx +++ b/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx @@ -88,37 +88,36 @@ export const RepoUrlPicker = ( [uiSchema], ); + const { owner, organization } = state; + useEffect(() => { onChange(serializeRepoPickerUrl(state)); }, [state, onChange]); /* we deal with calling the repo setting here instead of in each components for ease */ useEffect(() => { - if (allowedOrganizations.length > 0 && !state.organization) { + if (allowedOrganizations.length > 0 && !organization) { setState(prevState => ({ ...prevState, organization: allowedOrganizations[0], })); } - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [setState, allowedOrganizations, state.organization]); + }, [setState, allowedOrganizations, organization]); useEffect(() => { - if (allowedOwners.length > 0 && !state.owner) { + if (allowedOwners.length > 0 && !owner) { setState(prevState => ({ ...prevState, owner: allowedOwners[0], })); } - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [setState, allowedOwners, state.owner]); + }, [setState, allowedOwners, owner]); useEffect(() => { - if (allowedRepos.length > 0 && !state.repoName) { + if (allowedRepos.length > 0) { setState(prevState => ({ ...prevState, repoName: allowedRepos[0] })); } - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [setState, allowedRepos, state.repoName]); + }, [setState, allowedRepos]); const updateLocalState = useCallback( (newState: RepoUrlPickerState) => { @@ -138,7 +137,7 @@ export const RepoUrlPicker = ( return; } - const [host, owner, repoName] = [ + const [encodedHost, encodedOwner, encodedRepoName] = [ state.host, state.owner, state.repoName, @@ -148,7 +147,7 @@ export const RepoUrlPicker = ( // so lets grab them using the scmAuthApi and pass through // any additional scopes from the ui:options const { token } = await scmAuthApi.getCredentials({ - url: `https://${host}/${owner}/${repoName}`, + url: `https://${encodedHost}/${encodedOwner}/${encodedRepoName}`, additionalScope: { repoWrite: true, customScopes: requestUserCredentials.additionalScopes, From dcf632695a1e49837729bdc726e620038da75977 Mon Sep 17 00:00:00 2001 From: blam Date: Thu, 6 Oct 2022 12:52:36 +0200 Subject: [PATCH 4/5] chore: also fix repoName setting when there's no default Signed-off-by: blam --- .../components/fields/RepoUrlPicker/RepoUrlPicker.tsx | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx b/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx index d0c3108976..451df857e5 100644 --- a/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx +++ b/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx @@ -88,7 +88,7 @@ export const RepoUrlPicker = ( [uiSchema], ); - const { owner, organization } = state; + const { owner, organization, repoName } = state; useEffect(() => { onChange(serializeRepoPickerUrl(state)); @@ -114,10 +114,10 @@ export const RepoUrlPicker = ( }, [setState, allowedOwners, owner]); useEffect(() => { - if (allowedRepos.length > 0) { + if (allowedRepos.length > 0 && !repoName) { setState(prevState => ({ ...prevState, repoName: allowedRepos[0] })); } - }, [setState, allowedRepos]); + }, [setState, allowedRepos, repoName]); const updateLocalState = useCallback( (newState: RepoUrlPickerState) => { @@ -216,8 +216,8 @@ export const RepoUrlPicker = ( - setState(prevState => ({ ...prevState, repoName })) + onChange={repo => + setState(prevState => ({ ...prevState, repoName: repo })) } rawErrors={rawErrors} /> From 1eba273b69fd4e6b83a2425307a9c1d69a28117e Mon Sep 17 00:00:00 2001 From: Peter Macdonald Date: Thu, 6 Oct 2022 13:12:55 +0200 Subject: [PATCH 5/5] Fixed a bug where the allowed* values for the RepoUrlPicker would be reset on render. Signed-off-by: Peter Macdonald --- .changeset/old-melons-bathe.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/old-melons-bathe.md b/.changeset/old-melons-bathe.md index 1a0db03b6e..ac21377171 100644 --- a/.changeset/old-melons-bathe.md +++ b/.changeset/old-melons-bathe.md @@ -2,4 +2,4 @@ '@backstage/plugin-scaffolder': patch --- -Fixed the `allowedOwners` from resetting itself when you make a selection or leave the Allowed Owners drop down and start typing in an input below it +Fixed a bug where the `allowed*` values for the `RepoUrlPicker` would be reset on render.