Merge branch...

Merge branch '330887-gitlab-migration-more-fine-grained-status-for-group-bulk-imports' into 'master'

Resolve "GitLab Migration - More fine-grained status for group bulk imports"

See merge request gitlab-org/gitlab!73960

app/assets/javascripts/import_entities/import_groups/components/import_table.vue

@@ -314,9 +314,8 @@ export default {
           variables: { importRequests },
         });
       } catch (error) {
-        const message = error?.networkError?.response?.data?.error ?? i18n.ERROR_IMPORT;
         createFlash({
-          message,
+          message: i18n.ERROR_IMPORT,
           captureError: true,
           error,
         });

app/assets/javascripts/import_entities/import_groups/components/import_target_cell.vue

@@ -32,72 +32,84 @@ export default {
     fullPath() {
       return this.group.importTarget.targetNamespace.fullPath || s__('BulkImport|No parent');
     },
-    invalidNameValidationMessage() {
-      return getInvalidNameValidationMessage(this.group.importTarget);
+    validationMessage() {
+      return (
+        this.group.progress?.message || getInvalidNameValidationMessage(this.group.importTarget)
+      );
+    },
+    validNameState() {
+      // bootstrap-vue requires null for "indifferent" state, if we return true
+      // this will highlight field in green like "passed validation"
+      return this.group.flags.isInvalid && this.group.flags.isAvailableForImport ? false : null;
     },
   },
 };
 </script>
 
 <template>
-  <div class="gl-display-flex gl-align-items-stretch">
-    <import-group-dropdown
-      #default="{ namespaces }"
-      :text="fullPath"
-      :disabled="!group.flags.isAvailableForImport"
-      :namespaces="availableNamespaces"
-      toggle-class="gl-rounded-top-right-none! gl-rounded-bottom-right-none!"
-      class="gl-h-7 gl-flex-grow-1"
-      data-qa-selector="target_namespace_selector_dropdown"
-    >
-      <gl-dropdown-item @click="$emit('update-target-namespace', { fullPath: '', id: null })">{{
-        s__('BulkImport|No parent')
-      }}</gl-dropdown-item>
-      <template v-if="namespaces.length">
-        <gl-dropdown-divider />
-        <gl-dropdown-section-header>
-          {{ s__('BulkImport|Existing groups') }}
-        </gl-dropdown-section-header>
-        <gl-dropdown-item
-          v-for="ns in namespaces"
-          :key="ns.fullPath"
-          data-qa-selector="target_group_dropdown_item"
-          :data-qa-group-name="ns.fullPath"
-          @click="$emit('update-target-namespace', ns)"
-        >
-          {{ ns.fullPath }}
-        </gl-dropdown-item>
-      </template>
-    </import-group-dropdown>
-    <div
-      class="gl-h-7 gl-px-3 gl-display-flex gl-align-items-center gl-border-solid gl-border-0 gl-border-t-1 gl-border-b-1 gl-bg-gray-10"
-      :class="{
-        'gl-text-gray-400 gl-border-gray-100': !group.flags.isAvailableForImport,
-        'gl-border-gray-200': group.flags.isAvailableForImport,
-      }"
-    >
-      /
-    </div>
-    <div class="gl-flex-grow-1">
-      <gl-form-input
-        class="gl-rounded-top-left-none gl-rounded-bottom-left-none"
+  <div>
+    <div class="gl-display-flex gl-align-items-stretch">
+      <import-group-dropdown
+        #default="{ namespaces }"
+        :text="fullPath"
+        :disabled="!group.flags.isAvailableForImport"
+        :namespaces="availableNamespaces"
+        toggle-class="gl-rounded-top-right-none! gl-rounded-bottom-right-none!"
+        class="gl-h-7 gl-flex-grow-1"
+        data-qa-selector="target_namespace_selector_dropdown"
+      >
+        <gl-dropdown-item @click="$emit('update-target-namespace', { fullPath: '', id: null })">{{
+          s__('BulkImport|No parent')
+        }}</gl-dropdown-item>
+        <template v-if="namespaces.length">
+          <gl-dropdown-divider />
+          <gl-dropdown-section-header>
+            {{ s__('BulkImport|Existing groups') }}
+          </gl-dropdown-section-header>
+          <gl-dropdown-item
+            v-for="ns in namespaces"
+            :key="ns.fullPath"
+            data-qa-selector="target_group_dropdown_item"
+            :data-qa-group-name="ns.fullPath"
+            @click="$emit('update-target-namespace', ns)"
+          >
+            {{ ns.fullPath }}
+          </gl-dropdown-item>
+        </template>
+      </import-group-dropdown>
+      <div
+        class="gl-h-7 gl-px-3 gl-display-flex gl-align-items-center gl-border-solid gl-border-0 gl-border-t-1 gl-border-b-1 gl-bg-gray-10"
         :class="{
-          'gl-inset-border-1-gray-200!': group.flags.isAvailableForImport,
-          'gl-inset-border-1-gray-100!': !group.flags.isAvailableForImport,
-          'is-invalid': group.flags.isInvalid && group.flags.isAvailableForImport,
+          'gl-text-gray-400 gl-border-gray-100': !group.flags.isAvailableForImport,
+          'gl-border-gray-200': group.flags.isAvailableForImport,
         }"
-        debounce="500"
-        :disabled="!group.flags.isAvailableForImport"
-        :value="group.importTarget.newName"
-        :aria-label="__('New name')"
-        @input="$emit('update-new-name', $event)"
-      />
-      <p
-        v-if="group.flags.isAvailableForImport && group.flags.isInvalid"
-        class="gl-text-red-500 gl-m-0 gl-mt-2"
       >
-        {{ invalidNameValidationMessage }}
-      </p>
+        /
+      </div>
+      <div class="gl-flex-grow-1">
+        <gl-form-input
+          class="gl-rounded-top-left-none gl-rounded-bottom-left-none"
+          :class="{
+            'gl-inset-border-1-gray-200!':
+              group.flags.isAvailableForImport && !group.flags.isInvalid,
+            'gl-inset-border-1-gray-100!':
+              !group.flags.isAvailableForImport && !group.flags.isInvalid,
+          }"
+          debounce="500"
+          :disabled="!group.flags.isAvailableForImport"
+          :value="group.importTarget.newName"
+          :aria-label="__('New name')"
+          :state="validNameState"
+          @input="$emit('update-new-name', $event)"
+        />
+      </div>
+    </div>
+    <div
+      v-if="group.flags.isAvailableForImport && (group.flags.isInvalid || validationMessage)"
+      class="gl-text-red-500 gl-m-0 gl-mt-2"
+      role="alert"
+    >
+      {{ validationMessage }}
     </div>
   </div>
 </template>

app/assets/javascripts/import_entities/import_groups/graphql/client_factory.js

@@ -142,9 +142,7 @@ export function createResolvers({ endpoints }) {
           };
         });
 
-        const {
-          data: { id: jobId },
-        } = await axios.post(endpoints.createBulkImport, {
+        const { data: originalResponse } = await axios.post(endpoints.createBulkImport, {
           bulk_import: importOperations.map((op) => ({
             source_type: 'group_entity',
             source_full_path: op.group.fullPath,
@@ -153,15 +151,21 @@ export function createResolvers({ endpoints }) {
           })),
         });
 
-        return importOperations.map((op) => {
+        const responses = Array.isArray(originalResponse)
+          ? originalResponse
+          : [{ success: true, id: originalResponse.id }];
+
+        return importOperations.map((op, idx) => {
+          const response = responses[idx];
           const lastImportTarget = {
             targetNamespace: op.targetNamespace,
             newName: op.newName,
           };
 
           const progress = {
-            id: jobId,
-            status: STATUSES.CREATED,
+            id: response.id || `local-${Date.now()}-${idx}`,
+            status: response.success ? STATUSES.CREATED : STATUSES.FAILED,
+            message: response.message || null,
           };
 
           localStorageCache.set(op.group.webUrl, { progress, lastImportTarget });

app/assets/javascripts/import_entities/import_groups/graphql/fragments/bulk_import_source_group_progress.fragment.graphql

 fragment BulkImportSourceGroupProgress on ClientBulkImportProgress {
   id
   status
+  message
 }

app/assets/javascripts/import_entities/import_groups/graphql/mutations/import_groups.mutation.graphql

@@ -9,6 +9,7 @@ mutation importGroups($importRequests: [ImportGroupInput!]!) {
     progress {
       id
       status
+      message
     }
   }
 }

app/assets/javascripts/import_entities/import_groups/graphql/services/local_storage_cache.js

@@ -22,7 +22,14 @@ export class LocalStorageCache {
 
   loadCacheFromStorage() {
     try {
-      return JSON.parse(this.storage.getItem(KEY)) ?? {};
+      const storage = JSON.parse(this.storage.getItem(KEY)) ?? {};
+      Object.values(storage).forEach((entry) => {
+        if (entry.progress && !('message' in entry.progress)) {
+          // eslint-disable-next-line no-param-reassign
+          entry.progress.message = '';
+        }
+      });
+      return storage;
     } catch {
       return {};
     }

app/assets/javascripts/import_entities/import_groups/graphql/typedefs.graphql

@@ -16,6 +16,7 @@ type ClientBulkImportSourceGroupConnection {
 type ClientBulkImportProgress {
   id: ID!
   status: String!
+  message: String
 }
 
 type ClientBulkImportValidationError {

app/controllers/import/bulk_imports_controller.rb

@@ -40,13 +40,9 @@ class Import::BulkImportsController < ApplicationController
   end
 
   def create
-    response = ::BulkImports::CreateService.new(current_user, create_params, credentials).execute
+    responses = create_params.map { |entry| ::BulkImports::CreateService.new(current_user, entry, credentials).execute }
 
-    if response.success?
-      render json: response.payload.to_json(only: [:id])
-    else
-      render json: { error: response.message }, status: response.http_status
-    end
+    render json: responses.map { |response| { success: response.success?, id: response.payload[:id], message: response.message } }
   end
 
   def realtime_changes

app/services/bulk_imports/create_service.rb

@@ -59,7 +59,7 @@ module BulkImports
         )
         bulk_import.create_configuration!(credentials.slice(:url, :access_token))
 
-        params.each do |entity|
+        Array.wrap(params).each do |entity|
           BulkImports::Entity.create!(
             bulk_import: bulk_import,
             source_type: entity[:source_type],

spec/controllers/import/bulk_imports_controller_spec.rb

@@ -215,9 +215,13 @@ RSpec.describe Import::BulkImportsController do
         let(:pat) { "fake-pat" }
         let(:bulk_import_params) do
           [{ "source_type" => "group_entity",
-            "source_full_path" => "full_path",
-            "destination_name" => "destination_name",
-            "destination_namespace" => "root" }]
+             "source_full_path" => "full_path",
+             "destination_name" => "destination_name",
+             "destination_namespace" => "root" },
+           { "source_type" => "group_entity2",
+             "source_full_path" => "full_path2",
+             "destination_name" => "destination_name2",
+             "destination_namespace" => "root" }]
         end
 
         before do
@@ -225,29 +229,23 @@ RSpec.describe Import::BulkImportsController do
           session[:bulk_import_gitlab_url] = instance_url
         end
 
-        it 'executes BulkImpors::CreatetService' do
+        it 'executes BulkImpors::CreateService' do
+          error_response = ServiceResponse.error(message: 'Record invalid', http_status: :unprocessable_entity)
+
           expect_next_instance_of(
-            ::BulkImports::CreateService, user, bulk_import_params, { url: instance_url, access_token: pat }) do |service|
+            ::BulkImports::CreateService, user, bulk_import_params[0], { url: instance_url, access_token: pat }) do |service|
             allow(service).to receive(:execute).and_return(ServiceResponse.success(payload: bulk_import))
           end
-
-          post :create, params: { bulk_import: bulk_import_params }
-
-          expect(response).to have_gitlab_http_status(:ok)
-          expect(response.body).to eq({ id: bulk_import.id }.to_json)
-        end
-
-        it 'returns error when validation fails' do
-          error_response = ServiceResponse.error(message: 'Record invalid', http_status: :unprocessable_entity)
           expect_next_instance_of(
-            ::BulkImports::CreateService, user, bulk_import_params, { url: instance_url, access_token: pat }) do |service|
+            ::BulkImports::CreateService, user, bulk_import_params[1], { url: instance_url, access_token: pat }) do |service|
             allow(service).to receive(:execute).and_return(error_response)
           end
 
           post :create, params: { bulk_import: bulk_import_params }
 
-          expect(response).to have_gitlab_http_status(:unprocessable_entity)
-          expect(response.body).to eq({ error: 'Record invalid' }.to_json)
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(json_response).to eq([{ "success" => true, "id" => bulk_import.id, "message" => nil },
+                                       { "success" => false, "id" => nil, "message" => "Record invalid" }])
         end
       end
     end

spec/frontend/import_entities/import_groups/components/import_target_cell_spec.js

@@ -123,13 +123,22 @@ describe('import target cell', () => {
   });
 
   describe('when entity is available for import', () => {
+    const FAKE_PROGRESS_MESSAGE = 'progress message';
     beforeEach(() => {
-      group = generateFakeTableEntry({ id: 1, flags: { isAvailableForImport: true } });
+      group = generateFakeTableEntry({
+        id: 1,
+        flags: { isAvailableForImport: true },
+        progress: { message: FAKE_PROGRESS_MESSAGE },
+      });
       createComponent({ group });
     });
 
     it('renders namespace dropdown as enabled', () => {
       expect(findNamespaceDropdown().attributes('disabled')).toBe(undefined);
     });
+
+    it('renders progress message as error if it exists', () => {
+      expect(wrapper.find('[role=alert]').text()).toBe(FAKE_PROGRESS_MESSAGE);
+    });
   });
 });

spec/frontend/import_entities/import_groups/graphql/client_factory_spec.js

@@ -163,12 +163,34 @@ describe('Bulk import resolvers', () => {
   });
 
   describe('mutations', () => {
-    beforeEach(() => {
-      axiosMockAdapter.onPost(FAKE_ENDPOINTS.createBulkImport).reply(httpStatus.OK, { id: 1 });
-    });
+    beforeEach(() => {});
 
     describe('importGroup', () => {
-      it('sets import status to CREATED when request completes', async () => {
+      it('sets import status to CREATED for successful groups when request completes', async () => {
+        axiosMockAdapter
+          .onPost(FAKE_ENDPOINTS.createBulkImport)
+          .reply(httpStatus.OK, [{ success: true, id: 1 }]);
+
+        await client.mutate({
+          mutation: importGroupsMutation,
+          variables: {
+            importRequests: [
+              {
+                sourceGroupId: statusEndpointFixture.importable_data[0].id,
+                newName: 'test',
+                targetNamespace: 'root',
+              },
+            ],
+          },
+        });
+
+        await axios.waitForAll();
+        expect(results[0].progress.status).toBe(STATUSES.CREATED);
+      });
+
+      it('sets import status to CREATED for successful groups when request completes with legacy response', async () => {
+        axiosMockAdapter.onPost(FAKE_ENDPOINTS.createBulkImport).reply(httpStatus.OK, { id: 1 });
+
         await client.mutate({
           mutation: importGroupsMutation,
           variables: {
@@ -185,9 +207,37 @@ describe('Bulk import resolvers', () => {
         await axios.waitForAll();
         expect(results[0].progress.status).toBe(STATUSES.CREATED);
       });
+
+      it('sets import status to FAILED and sets progress message for failed groups when request completes', async () => {
+        const FAKE_ERROR_MESSAGE = 'foo';
+        axiosMockAdapter
+          .onPost(FAKE_ENDPOINTS.createBulkImport)
+          .reply(httpStatus.OK, [{ success: false, id: 1, message: FAKE_ERROR_MESSAGE }]);
+
+        await client.mutate({
+          mutation: importGroupsMutation,
+          variables: {
+            importRequests: [
+              {
+                sourceGroupId: statusEndpointFixture.importable_data[0].id,
+                newName: 'test',
+                targetNamespace: 'root',
+              },
+            ],
+          },
+        });
+
+        await axios.waitForAll();
+        expect(results[0].progress.status).toBe(STATUSES.FAILED);
+        expect(results[0].progress.message).toBe(FAKE_ERROR_MESSAGE);
+      });
     });
 
     it('updateImportStatus updates status', async () => {
+      axiosMockAdapter
+        .onPost(FAKE_ENDPOINTS.createBulkImport)
+        .reply(httpStatus.OK, [{ success: true, id: 1 }]);
+
       const NEW_STATUS = 'dummy';
       await client.mutate({
         mutation: importGroupsMutation,
@@ -216,6 +266,7 @@ describe('Bulk import resolvers', () => {
       expect(statusInResponse).toStrictEqual({
         __typename: clientTypenames.BulkImportProgress,
         id,
+        message: null,
         status: NEW_STATUS,
       });
     });

spec/frontend/import_entities/import_groups/graphql/fixtures.js

 import { STATUSES } from '~/import_entities/constants';
 import { clientTypenames } from '~/import_entities/import_groups/graphql/client_factory';
 
-export const generateFakeEntry = ({ id, status, ...rest }) => ({
+export const generateFakeEntry = ({ id, status, message, ...rest }) => ({
   __typename: clientTypenames.BulkImportSourceGroup,
   webUrl: `https://fake.host/${id}`,
   fullPath: `fake_group_${id}`,
@@ -18,6 +18,7 @@ export const generateFakeEntry = ({ id, status, ...rest }) => ({
       : {
           id,
           status,
+          message: message || '',
         },
   ...rest,
 });