Merge branch 'georgekoltsov/bulk-import-remove-epic-loader' into 'master'

Remove BulkImports EpicLoader

See merge request gitlab-org/gitlab!55214

ee/lib/ee/bulk_imports/groups/loaders/epics_loader.rb

-# frozen_string_literal: true
-
-module EE
-  module BulkImports
-    module Groups
-      module Loaders
-        class EpicsLoader
-          NotAllowedError = Class.new(StandardError)
-
-          def load(context, data)
-            raise NotAllowedError unless authorized?(context)
-
-            # Use `Epic` directly when creating new epics
-            # instead of `Epics::CreateService` since several
-            # attributes like author_id (which might not be current_user),
-            # group_id, parent, children need to be custom set
-            ::Epic.create!(data)
-          end
-
-          private
-
-          def authorized?(context)
-            Ability.allowed?(context.current_user, :admin_epic, context.group)
-          end
-        end
-      end
-    end
-  end
-end

ee/lib/ee/bulk_imports/groups/pipelines/epics_pipeline.rb

@@ -13,7 +13,11 @@ module EE
           transformer ::BulkImports::Common::Transformers::ProhibitedAttributesTransformer
           transformer EE::BulkImports::Groups::Transformers::EpicAttributesTransformer
 
-          loader EE::BulkImports::Groups::Loaders::EpicsLoader
+          def load(context, data)
+            raise ::BulkImports::Pipeline::NotAllowedError unless authorized?
+
+            context.group.epics.create!(data)
+          end
 
           def after_run(extracted_data)
             context.entity.update_tracker_for(
@@ -26,6 +30,12 @@ module EE
               run
             end
           end
+
+          private
+
+          def authorized?
+            context.current_user.can?(:admin_epic, context.group)
+          end
         end
       end
     end

ee/lib/ee/bulk_imports/groups/transformers/epic_attributes_transformer.rb

@@ -7,7 +7,6 @@ module EE
         class EpicAttributesTransformer
           def transform(context, data)
             data
-              .then { |data| add_group_id(context, data) }
               .then { |data| add_author_id(context, data) }
               .then { |data| add_parent(context, data) }
               .then { |data| add_children(context, data) }
@@ -16,10 +15,6 @@ module EE
 
           private
 
-          def add_group_id(context, data)
-            data.merge('group_id' => context.group.id)
-          end
-
           def add_author_id(context, data)
             user = find_user_by_email(context, data.dig('author', 'public_email'))
             author_id = user&.id || context.current_user.id

ee/spec/lib/ee/bulk_imports/groups/loaders/epics_loader_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe EE::BulkImports::Groups::Loaders::EpicsLoader do
-  describe '#load' do
-    it 'creates the epic' do
-      stub_licensed_features(epics: true)
-
-      user = create(:user)
-      group = create(:group)
-      group.add_owner(user)
-
-      parent_epic = create(:epic, group: group)
-      child_epic = create(:epic, group: group)
-      label = create(:group_label, group: group)
-      bulk_import = create(:bulk_import, user: user)
-      entity = create(:bulk_import_entity, bulk_import: bulk_import, group: group)
-      context = BulkImports::Pipeline::Context.new(entity)
-
-      data = {
-        'title' => 'epic',
-        'state' => 'opened',
-        'confidential' => false,
-        'iid' => 99,
-        'author_id' => user.id,
-        'group_id' => group.id,
-        'parent' => parent_epic,
-        'children' => [child_epic],
-        'labels' => [
-          label
-        ]
-      }
-
-      expect { subject.load(context, data) }.to change(::Epic, :count).by(1)
-
-      epic = group.epics.last
-      expect(epic.group).to eq(group)
-      expect(epic.author).to eq(user)
-      expect(epic.title).to eq('epic')
-      expect(epic.state).to eq('opened')
-      expect(epic.confidential).to eq(false)
-      expect(epic.iid).to eq(99)
-      expect(epic.parent).to eq(parent_epic)
-      expect(epic.children).to contain_exactly(child_epic)
-      expect(epic.labels).to contain_exactly(label)
-    end
-  end
-end

ee/spec/lib/ee/bulk_imports/groups/pipelines/epics_pipeline_spec.rb

@@ -3,9 +3,9 @@
 require 'spec_helper'
 
 RSpec.describe EE::BulkImports::Groups::Pipelines::EpicsPipeline do
-  let(:cursor) { 'cursor' }
-  let(:user) { create(:user) }
-  let(:group) { create(:group) }
+  let_it_be(:cursor) { 'cursor' }
+  let_it_be(:user) { create(:user) }
+  let_it_be(:group) { create(:group) }
   let(:bulk_import) { create(:bulk_import, user: user) }
   let(:entity) do
     create(
@@ -42,6 +42,57 @@ RSpec.describe EE::BulkImports::Groups::Pipelines::EpicsPipeline do
     end
   end
 
+  describe '#load' do
+    context 'when user is authorized to create the epic' do
+      it 'creates the epic' do
+        author = create(:user, email: 'member@email.com')
+        parent_epic = create(:epic, group: group)
+        child_epic = create(:epic, group: group)
+        label = create(:group_label, group: group)
+        group.add_developer(author)
+
+        data = {
+          'id' => 99,
+          'iid' => 99,
+          'title' => 'epic',
+          'state' => 'opened',
+          'confidential' => false,
+          'author_id' => author.id,
+          'parent' => parent_epic,
+          'children' => [child_epic],
+          'labels' => [
+            label
+          ]
+        }
+
+        expect { subject.load(context, data) }.to change(::Epic, :count).by(1)
+
+        epic = group.epics.find_by_iid(99)
+
+        expect(epic.group).to eq(group)
+        expect(epic.author).to eq(author)
+        expect(epic.title).to eq('epic')
+        expect(epic.state).to eq('opened')
+        expect(epic.confidential).to eq(false)
+        expect(epic.parent).to eq(parent_epic)
+        expect(epic.children).to contain_exactly(child_epic)
+        expect(epic.labels).to contain_exactly(label)
+      end
+    end
+
+    context 'when user is not authorized to create the epic' do
+      before do
+        allow(user).to receive(:can?).with(:admin_epic, group).and_return(false)
+      end
+
+      it 'raises NotAllowedError' do
+        data = extractor_data(has_next_page: false)
+
+        expect { subject.load(context, data) }.to raise_error(::BulkImports::Pipeline::NotAllowedError)
+      end
+    end
+  end
+
   describe '#after_run' do
     context 'when extracted data has next page' do
       it 'updates tracker information and runs pipeline again' do
@@ -95,10 +146,6 @@ RSpec.describe EE::BulkImports::Groups::Pipelines::EpicsPipeline do
           { klass: EE::BulkImports::Groups::Transformers::EpicAttributesTransformer, options: nil }
         )
     end
-
-    it 'has loaders' do
-      expect(described_class.get_loader).to eq(klass: EE::BulkImports::Groups::Loaders::EpicsLoader, options: nil)
-    end
   end
 
   def extractor_data(has_next_page:, cursor: nil)

ee/spec/lib/ee/bulk_imports/groups/transformers/epic_attributes_transformer_spec.rb

@@ -26,7 +26,6 @@ RSpec.describe EE::BulkImports::Groups::Transformers::EpicAttributesTransformer
         'due_date_is_fixed' => false,
         'relative_position' => 1073716855,
         'confidential' => false,
-        'group_id' => group.id,
         'author_id' => importer_user.id,
         'parent' => nil,
         'children' => [],

lib/bulk_imports/pipeline.rb

@@ -7,6 +7,8 @@ module BulkImports
     include Gitlab::ClassAttributes
     include Runner
 
+    NotAllowedError = Class.new(StandardError)
+
     def initialize(context)
       @context = context
     end