Merge branch 'jh-group_import_status' into 'master'

Display import status page after Group import

See merge request gitlab-org/gitlab!31731

app/assets/stylesheets/pages/groups.scss

@@ -49,6 +49,12 @@
   }
 }
 
+.save-group-loader {
+  margin-top: $gl-padding-50;
+  margin-bottom: $gl-padding-50;
+  color: $gl-gray-700;
+}
+
 .group-nav-container .nav-controls {
   .group-filter-form {
     flex: 1 1 auto;

app/controllers/groups/imports_controller.rb

+# frozen_string_literal: true
+
+class Groups::ImportsController < Groups::ApplicationController
+  include ContinueParams
+
+  def show
+    if @group.import_state.nil? || @group.import_state.finished?
+      if continue_params[:to]
+        redirect_to continue_params[:to], notice: continue_params[:notice]
+      else
+        redirect_to group_path(@group), notice: s_('GroupImport|The group was successfully imported.')
+      end
+    elsif @group.import_state.failed?
+      redirect_to new_group_path(@group), alert: s_('GroupImport|Failed to import group.')
+    else
+      flash.now[:notice] = continue_params[:notice_now]
+    end
+  end
+end

app/controllers/groups_controller.rb

@@ -74,8 +74,12 @@ class GroupsController < Groups::ApplicationController
   def show
     respond_to do |format|
       format.html do
+        if @group.import_state&.in_progress?
+          redirect_to group_import_path(@group)
+        else
           render_show_html
         end
+      end
 
       format.atom do
         render_details_view_atom

app/controllers/import/gitlab_groups_controller.rb

@@ -8,7 +8,7 @@ class Import::GitlabGroupsController < ApplicationController
 
   def create
     unless file_is_valid?(group_params[:file])
-      return redirect_back_or_default(options: { alert: _('Unable to process group import file') })
+      return redirect_back_or_default(options: { alert: s_('GroupImport|Unable to process group import file') })
     end
 
     group_data = group_params.except(:file).merge(
@@ -19,15 +19,17 @@ class Import::GitlabGroupsController < ApplicationController
     group = ::Groups::CreateService.new(current_user, group_data).execute
 
     if group.persisted?
-      Groups::ImportExport::ImportService.new(group: group, user: current_user).async_execute
-
+      if Groups::ImportExport::ImportService.new(group: group, user: current_user).async_execute
         redirect_to(
           group_path(group),
-        notice: _("Group '%{group_name}' is being imported.") % { group_name: group.name }
+          notice: s_("GroupImport|Group '%{group_name}' is being imported.") % { group_name: group.name }
         )
+      else
+        redirect_to group_path(group), alert: _("Group import could not be scheduled")
+      end
     else
       redirect_back_or_default(
-        options: { alert: _("Group could not be imported: %{errors}") % { errors: group.errors.full_messages.to_sentence } }
+        options: { alert: s_("GroupImport|Group could not be imported: %{errors}") % { errors: group.errors.full_messages.to_sentence } }
       )
     end
   end

app/models/group_import_state.rb

@@ -32,4 +32,8 @@ class GroupImportState < ApplicationRecord
       state.update_column(:last_error, last_error) if last_error
     end
   end
+
+  def in_progress?
+    created? || started?
+  end
 end

app/services/groups/import_export/import_service.rb

@@ -13,7 +13,16 @@ module Groups
       end
 
       def async_execute
-        GroupImportWorker.perform_async(current_user.id, group.id)
+        group_import_state = GroupImportState.safe_find_or_create_by!(group: group)
+        jid = GroupImportWorker.perform_async(current_user.id, group.id)
+
+        if jid.present?
+          group_import_state.update!(jid: jid)
+        else
+          group_import_state.fail_op('Failed to schedule import job')
+
+          false
+        end
       end
 
       def execute

app/views/groups/imports/show.html.haml

+- page_title _('Import in progress')
+- @content_class = "limit-container-width" unless fluid_layout
+
+.save-group-loader
+  .center
+    %h2
+      %i.loading.spinner.spinner-sm
+      = page_title
+    %p
+      = s_('GroupImport|Please wait while we import the group for you. Refresh at will.')

app/workers/group_import_worker.rb

@@ -9,15 +9,16 @@ class GroupImportWorker # rubocop:disable Scalability/IdempotentWorker
   def perform(user_id, group_id)
     current_user = User.find(user_id)
     group = Group.find(group_id)
-    group_import = group.build_import_state(jid: self.jid)
+    group_import_state = group.import_state || group.build_import_state
 
-    group_import.start!
+    group_import_state.jid = self.jid
+    group_import_state.start!
 
     ::Groups::ImportExport::ImportService.new(group: group, user: current_user).execute
 
-    group_import.finish!
+    group_import_state.finish!
   rescue StandardError => e
-    group_import&.fail_op(e.message)
+    group_import_state&.fail_op(e.message)
 
     raise e
   end

changelogs/unreleased/jh-group_import_status.yml

+---
+title: Show import in progress screen for group imports
+merge_request: 31731
+author:
+type: added

config/routes/group.rb

@@ -70,6 +70,7 @@ constraints(::Constraints::GroupUrlConstrainer.new) do
     end
 
     resource :avatar, only: [:destroy]
+    resource :import, only: [:show]
 
     concerns :clusterable
 

locale/gitlab.pot

@@ -10928,9 +10928,6 @@ msgstr ""
 msgid "Group %{group_name} was successfully created."
 msgstr ""
 
-msgid "Group '%{group_name}' is being imported."
-msgstr ""
-
 msgid "Group Audit Events"
 msgstr ""
 
@@ -10967,9 +10964,6 @@ msgstr ""
 msgid "Group by:"
 msgstr ""
 
-msgid "Group could not be imported: %{errors}"
-msgstr ""
-
 msgid "Group description"
 msgstr ""
 
@@ -10997,6 +10991,9 @@ msgstr ""
 msgid "Group has not been marked for deletion"
 msgstr ""
 
+msgid "Group import could not be scheduled"
+msgstr ""
+
 msgid "Group info:"
 msgstr ""
 
@@ -11063,6 +11060,24 @@ msgstr ""
 msgid "GroupActivyMetrics|Recent activity (last 90 days)"
 msgstr ""
 
+msgid "GroupImport|Failed to import group."
+msgstr ""
+
+msgid "GroupImport|Group '%{group_name}' is being imported."
+msgstr ""
+
+msgid "GroupImport|Group could not be imported: %{errors}"
+msgstr ""
+
+msgid "GroupImport|Please wait while we import the group for you. Refresh at will."
+msgstr ""
+
+msgid "GroupImport|The group was successfully imported."
+msgstr ""
+
+msgid "GroupImport|Unable to process group import file"
+msgstr ""
+
 msgid "GroupRoadmap|%{dateWord} – No end date"
 msgstr ""
 
@@ -23870,9 +23885,6 @@ msgstr ""
 msgid "Unable to load the merge request widget. Try reloading the page."
 msgstr ""
 
-msgid "Unable to process group import file"
-msgstr ""
-
 msgid "Unable to resolve"
 msgstr ""
 

spec/controllers/groups/imports_controller_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Groups::ImportsController do
+  describe 'GET #show' do
+    let_it_be(:user) { create(:user) }
+    let_it_be(:group) { create(:group, :private) }
+
+    context 'when the user has permission to view the group' do
+      before do
+        sign_in(user)
+        group.add_maintainer(user)
+      end
+
+      context 'when the import is in progress' do
+        before do
+          create(:group_import_state, group: group)
+        end
+
+        it 'renders the show template' do
+          get :show, params: { group_id: group }
+
+          expect(response).to render_template :show
+        end
+
+        it 'sets the flash notice' do
+          get :show, params: { group_id: group, continue: { to: '/', notice_now: 'In progress' } }
+
+          expect(flash.now[:notice]).to eq 'In progress'
+        end
+      end
+
+      context 'when the import has failed' do
+        before do
+          create(:group_import_state, :failed, group: group)
+        end
+
+        it 'redirects to the new group path' do
+          get :show, params: { group_id: group }
+
+          expect(response).to redirect_to new_group_path(group)
+        end
+
+        it 'sets a flash error' do
+          get :show, params: { group_id: group }
+
+          expect(flash[:alert]).to eq 'Failed to import group.'
+        end
+      end
+
+      context 'when the import has finished' do
+        before do
+          create(:group_import_state, :finished, group: group)
+        end
+
+        it 'redirects to the group page' do
+          get :show, params: { group_id: group }
+
+          expect(response).to redirect_to group_path(group)
+        end
+      end
+
+      context 'when there is no import state' do
+        it 'redirects to the group page' do
+          get :show, params: { group_id: group }
+
+          expect(response).to redirect_to group_path(group)
+        end
+      end
+    end
+
+    context 'when the user does not have permission to view the group' do
+      before do
+        sign_in(user)
+      end
+
+      it 'returns a 404' do
+        get :show, params: { group_id: group }
+
+        expect(response).to have_gitlab_http_status :not_found
+      end
+    end
+  end
+end

spec/controllers/groups_controller_spec.rb

@@ -66,9 +66,21 @@ RSpec.describe GroupsController do
 
     subject { get :show, params: { id: group.to_param }, format: format }
 
+    context 'when the group is not importing' do
       it_behaves_like 'details view'
     end
 
+    context 'when the group is importing' do
+      before do
+        create(:group_import_state, group: group)
+      end
+
+      it 'redirects to the import status page' do
+        expect(subject).to redirect_to group_import_path(group)
+      end
+    end
+  end
+
   describe 'GET #details' do
     before do
       sign_in(user)

spec/models/group_import_state_spec.rb

@@ -35,4 +35,38 @@ describe GroupImportState do
       expect(import_state).to be_valid
     end
   end
+
+  describe '#in_progress?' do
+    context "when the import is 'created'" do
+      it "returns true" do
+        group_import_state = build(:group_import_state, :created)
+
+        expect(group_import_state.in_progress?).to eq true
+      end
+    end
+
+    context "when the import is 'started'" do
+      it "returns true" do
+        group_import_state = build(:group_import_state, :started)
+
+        expect(group_import_state.in_progress?).to eq true
+      end
+    end
+
+    context "when the import is 'finished'" do
+      it "returns false" do
+        group_import_state = build(:group_import_state, :finished)
+
+        expect(group_import_state.in_progress?).to eq false
+      end
+    end
+
+    context "when the import is 'failed'" do
+      it "returns false" do
+        group_import_state = build(:group_import_state, :failed)
+
+        expect(group_import_state.in_progress?).to eq false
+      end
+    end
+  end
 end

spec/services/groups/import_export/import_service_spec.rb

@@ -16,6 +16,12 @@ describe Groups::ImportExport::ImportService do
         import_service.async_execute
       end
 
+      it 'marks the group import as in progress' do
+        import_service.async_execute
+
+        expect(group.import_state.in_progress?).to eq true
+      end
+
       it 'returns truthy' do
         expect(import_service.async_execute).to be_truthy
       end
@@ -31,6 +37,10 @@ describe Groups::ImportExport::ImportService do
       it 'returns falsey' do
         expect(import_service.async_execute).to be_falsey
       end
+
+      it 'does not mark the group import as created' do
+        expect { import_service.async_execute }.not_to change { group.import_state }
+      end
     end
   end
 

spec/workers/group_import_worker_spec.rb

@@ -26,7 +26,7 @@ describe GroupImportWorker do
         subject.perform(user.id, group.id)
       end
 
-      context 'import state' do
+      context 'when the import state does not exist' do
         it 'creates group import' do
           expect(group.import_state).to be_nil
 
@@ -54,6 +54,17 @@ describe GroupImportWorker do
           subject.perform(user.id, group.id)
         end
       end
+
+      context 'when the import state already exists' do
+        it 'updates the existing state' do
+          existing_state = create(:group_import_state, group: group)
+
+          expect { subject.perform(user.id, group.id) }
+            .not_to change { GroupImportState.count }
+
+          expect(existing_state.reload).to be_finished
+        end
+      end
     end
 
     context 'when it fails' do