Merge pull request #24881 from benjidotsh/fix/scaffolder-request-user-credentials-on-load

fix(scaffolder): remove waiting until all fields are filled in before requesting user credentials

.changeset/hungry-cups-jog.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-scaffolder': patch
+---
+
+Removed waiting for the workspace and repository fields to be filled in before requesting user credentials

plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.test.tsx

@@ -213,7 +213,7 @@ describe('RepoUrlPicker', () => {
           <div data-testid="current-secrets">{JSON.stringify({ secrets })}</div>
         );
       };
-      const { getAllByRole, getByTestId } = await renderInTestApp(
+      const { getByTestId } = await renderInTestApp(
         <TestApiProvider
           apis={[
             [scmIntegrationsApiRef, mockIntegrationsApi],
@@ -243,18 +243,13 @@ describe('RepoUrlPicker', () => {
         </TestApiProvider>,
       );
 
-      const [ownerInput, repoInput] = getAllByRole('textbox');
-
       await act(async () => {
-        fireEvent.change(ownerInput, { target: { value: 'backstage' } });
-        fireEvent.change(repoInput, { target: { value: 'repo123' } });
-
         // need to wait for the debounce to finish
         await new Promise(resolve => setTimeout(resolve, 600));
       });
 
       expect(mockScmAuthApi.getCredentials).toHaveBeenCalledWith({
-        url: 'https://github.com/backstage/repo123',
+        url: 'https://github.com',
         additionalScope: {
           repoWrite: true,
           customScopes: {
@@ -278,7 +273,7 @@ describe('RepoUrlPicker', () => {
           <div data-testid="current-secrets">{JSON.stringify({ secrets })}</div>
         );
       };
-      const { getAllByRole } = await renderInTestApp(
+      await renderInTestApp(
         <TestApiProvider
           apis={[
             [scmIntegrationsApiRef, mockIntegrationsApi],
@@ -308,87 +303,18 @@ describe('RepoUrlPicker', () => {
         </TestApiProvider>,
       );
 
-      const [projectInput, repoInput] = getAllByRole('textbox');
-
       await act(async () => {
-        fireEvent.change(projectInput, {
-          target: { value: 'backstage/mysubgroup' },
-        });
-        fireEvent.change(repoInput, { target: { value: 'repo123' } });
-
         // need to wait for the debounce to finish
         await new Promise(resolve => setTimeout(resolve, 600));
       });
 
       expect(mockScmAuthApi.getCredentials).toHaveBeenCalledWith({
-        url: 'https://gitlab.example.com/backstage/mysubgroup/repo123',
+        url: 'https://gitlab.example.com',
         additionalScope: {
           repoWrite: true,
         },
       });
     });
-    it('should call the scmAuthApi with the correct params if only a project is set', async () => {
-      const SecretsComponent = () => {
-        const { secrets } = useTemplateSecrets();
-        return (
-          <div data-testid="current-secrets">{JSON.stringify({ secrets })}</div>
-        );
-      };
-      const { getAllByRole, getByTestId } = await renderInTestApp(
-        <TestApiProvider
-          apis={[
-            [scmIntegrationsApiRef, mockIntegrationsApi],
-            [scmAuthApiRef, mockScmAuthApi],
-            [scaffolderApiRef, mockScaffolderApi],
-          ]}
-        >
-          <SecretsContextProvider>
-            <Form
-              validator={validator}
-              schema={{ type: 'string' }}
-              uiSchema={{
-                'ui:field': 'RepoUrlPicker',
-                'ui:options': {
-                  allowedHosts: ['server.bitbucket.org'],
-                  requestUserCredentials: {
-                    secretsKey: 'testKey',
-                  },
-                },
-              }}
-              fields={{
-                RepoUrlPicker: RepoUrlPicker as ScaffolderRJSFField<string>,
-              }}
-            />
-            <SecretsComponent />
-          </SecretsContextProvider>
-        </TestApiProvider>,
-      );
-
-      const [projectInput, repoInput] = getAllByRole('textbox');
-
-      await act(async () => {
-        fireEvent.change(projectInput, { target: { value: 'backstage' } });
-        fireEvent.change(repoInput, { target: { value: 'repo123' } });
-
-        // need to wait for the debounce to finish
-        await new Promise(resolve => setTimeout(resolve, 600));
-      });
-
-      expect(mockScmAuthApi.getCredentials).toHaveBeenCalledWith({
-        url: 'https://server.bitbucket.org/backstage/repo123',
-        additionalScope: {
-          repoWrite: true,
-        },
-      });
-
-      const currentSecrets = JSON.parse(
-        getByTestId('current-secrets').textContent!,
-      );
-
-      expect(currentSecrets).toEqual({
-        secrets: { testKey: 'abc123' },
-      });
-    });
 
     it('should not call the scmAuthApi if secret is available in the state', async () => {
       const SecretsComponent = () => {
@@ -397,7 +323,7 @@ describe('RepoUrlPicker', () => {
           <div data-testid="current-secrets">{JSON.stringify({ secrets })}</div>
         );
       };
-      const { getAllByRole, getByTestId } = await renderInTestApp(
+      const { getByTestId } = await renderInTestApp(
         <TestApiProvider
           apis={[
             [scmIntegrationsApiRef, mockIntegrationsApi],
@@ -427,12 +353,7 @@ describe('RepoUrlPicker', () => {
         </TestApiProvider>,
       );
 
-      const [ownerInput, repoInput] = getAllByRole('textbox');
-
       await act(async () => {
-        fireEvent.change(ownerInput, { target: { value: 'backstage' } });
-        fireEvent.change(repoInput, { target: { value: 'repo123' } });
-
         // need to wait for the debounce to finish
         await new Promise(resolve => setTimeout(resolve, 600));
       });

plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx

@@ -124,11 +124,7 @@ export const RepoUrlPicker = (props: RepoUrlPickerProps) => {
     async () => {
       const { requestUserCredentials } = uiSchema?.['ui:options'] ?? {};
 
-      const workspace = state.owner ? state.owner : state.project;
-      if (
-        !requestUserCredentials ||
-        !(state.host && workspace && state.repoName)
-      ) {
+      if (!requestUserCredentials || !state.host) {
         return;
       }
 
@@ -137,19 +133,11 @@ export const RepoUrlPicker = (props: RepoUrlPickerProps) => {
         return;
       }
 
-      // previously, we were encodeURI for state.host, workspace and state.repoName separately.
-      // That created an issue where GitLab workspace can be nested like groupA/subgroupB
-      // when we encodeURi separately and then join, the URL will be malformed and
-      // resulting in 400 request error from GitLab API
-      const [encodedHost, encodedRepoName] = [state.host, state.repoName].map(
-        encodeURIComponent,
-      );
-
       // user has requested that we use the users credentials
       // so lets grab them using the scmAuthApi and pass through
       // any additional scopes from the ui:options
       const { token } = await scmAuthApi.getCredentials({
-        url: `https://${encodedHost}/${workspace}/${encodedRepoName}`,
+        url: `https://${state.host}`,
         additionalScope: {
           repoWrite: true,
           customScopes: requestUserCredentials.additionalScopes,