Merge branch...

Merge branch 'kassio/bulkimports-avoid-exception-in-epics-pipelines-when-group-does-not-exist' into 'master'

BulkImports: Avoid exception in EpicsPipeline when there's no Group

See merge request gitlab-org/gitlab!58949

ee/lib/ee/bulk_imports/pipeline/epic_base.rb

@@ -9,13 +9,15 @@ module EE
         def initialize(context)
           super(context)
 
+          return if context.group.blank?
+
           @epic_iids = context.group.epics.order(iid: :desc).pluck(:iid) # rubocop: disable CodeReuse/ActiveRecord
 
           set_next_epic
         end
 
         def run
-          return skip!('Skipping because group has no epics') if current_epic_iid.blank?
+          return skip!(skip_reason) if current_epic_iid.blank?
 
           super
         end
@@ -43,6 +45,14 @@ module EE
         def current_epic_iid
           context.extra[:epic_iid]
         end
+
+        def skip_reason
+          if context.group.blank?
+            'Skipping because bulk import has no group'
+          else
+            'Skipping because group has no epics'
+          end
+        end
       end
     end
   end

ee/spec/lib/ee/bulk_imports/pipeline/epic_base_spec.rb

@@ -3,11 +3,6 @@
 require 'spec_helper'
 
 RSpec.describe EE::BulkImports::Pipeline::EpicBase do
-  let_it_be(:group) { create(:group) }
-  let_it_be(:entity) { create(:bulk_import_entity, group: group) }
-  let_it_be(:pipeline_tracker) { create(:bulk_import_tracker, entity: entity) }
-  let_it_be(:context) { BulkImports::Pipeline::Context.new(pipeline_tracker) }
-
   let(:pipeline_class) do
     Class.new(EE::BulkImports::Pipeline::EpicBase) do
       def extract(_)
@@ -30,16 +25,18 @@ RSpec.describe EE::BulkImports::Pipeline::EpicBase do
     stub_const('MyPipeline', pipeline_class)
   end
 
-  context 'when the group has no epics' do
+  context 'when the group was not imported' do
+    let_it_be(:pipeline_tracker) { create(:bulk_import_tracker) }
+    let_it_be(:context) { BulkImports::Pipeline::Context.new(pipeline_tracker) }
+
     it 'skips the pipeline' do
       expect_next_instance_of(Gitlab::Import::Logger) do |logger|
         expect(logger).to receive(:warn)
           .with(
-            bulk_import_id: entity.bulk_import.id,
-            bulk_import_entity_id: entity.id,
-            bulk_import_entity_type: entity.source_type,
-            context_extra: { epic_iid: nil },
-            message: 'Skipping because group has no epics',
+            bulk_import_id: pipeline_tracker.entity.bulk_import.id,
+            bulk_import_entity_id: pipeline_tracker.entity.id,
+            bulk_import_entity_type: pipeline_tracker.entity.source_type,
+            message: 'Skipping because bulk import has no group',
             pipeline_class: 'MyPipeline'
           )
       end
@@ -49,86 +46,112 @@ RSpec.describe EE::BulkImports::Pipeline::EpicBase do
     end
   end
 
-  context 'when the group has one epic with one page of data' do
-    it 'runs the pipeline for each epic' do
-      create(:epic, group: group)
+  context 'when the group was already imported' do
+    let_it_be(:group) { create(:group) }
+    let_it_be(:entity) { create(:bulk_import_entity, group: group) }
+    let_it_be(:pipeline_tracker) { create(:bulk_import_tracker, entity: entity) }
+    let_it_be(:context) { BulkImports::Pipeline::Context.new(pipeline_tracker) }
+
+    context 'when the group has no epics' do
+      it 'skips the pipeline' do
+        expect_next_instance_of(Gitlab::Import::Logger) do |logger|
+          expect(logger).to receive(:warn)
+            .with(
+              bulk_import_id: entity.bulk_import.id,
+              bulk_import_entity_id: entity.id,
+              bulk_import_entity_type: entity.source_type,
+              context_extra: { epic_iid: nil },
+              message: 'Skipping because group has no epics',
+              pipeline_class: 'MyPipeline'
+            )
+        end
+
+        expect { subject.run }
+          .to change(pipeline_tracker, :status_name).to(:skipped)
+      end
+    end
+
+    context 'when the group has one epic with one page of data' do
+      it 'runs the pipeline for each epic' do
+        create(:epic, group: group)
 
-      expect(subject)
-        .to receive(:run)
-        .once
-        .and_call_original
+        expect(subject)
+          .to receive(:run)
+          .once
+          .and_call_original
 
-      subject.run
+        subject.run
+      end
     end
-  end
 
-  context 'when the group has one epic with multiple page of data' do
-    it 'runs the pipeline for page' do
-      create(:epic, group: group)
+    context 'when the group has one epic with multiple page of data' do
+      it 'runs the pipeline for page' do
+        create(:epic, group: group)
 
-      first_page = ::BulkImports::Pipeline::ExtractedData.new(page_info: {
-        'has_next_page' => true,
-        'next_page' => 'page'
-      })
-      last_page = ::BulkImports::Pipeline::ExtractedData.new(page_info: {
-        'has_next_page' => false
-      })
+        first_page = ::BulkImports::Pipeline::ExtractedData.new(page_info: {
+          'has_next_page' => true,
+          'next_page' => 'page'
+        })
+        last_page = ::BulkImports::Pipeline::ExtractedData.new(page_info: {
+          'has_next_page' => false
+        })
 
-      expect(subject)
-        .to receive(:extract)
-        .and_return(first_page, last_page)
+        expect(subject)
+          .to receive(:extract)
+          .and_return(first_page, last_page)
 
-      expect(subject)
-        .to receive(:run)
-        .twice
-        .and_call_original
+        expect(subject)
+          .to receive(:run)
+          .twice
+          .and_call_original
 
-      subject.run
+        subject.run
+      end
     end
-  end
 
-  context 'when the group has multiple epics with one page of data' do
-    it 'runs the pipeline once for each epic' do
-      create(:epic, group: group)
-      create(:epic, group: group)
+    context 'when the group has multiple epics with one page of data' do
+      it 'runs the pipeline once for each epic' do
+        create(:epic, group: group)
+        create(:epic, group: group)
 
-      expect(subject)
-        .to receive(:run)
-        .twice
-        .and_call_original
+        expect(subject)
+          .to receive(:run)
+          .twice
+          .and_call_original
 
-      subject.run
+        subject.run
+      end
     end
-  end
 
-  context 'when the group has multiple epics with multiple pages of data' do
-    it 'runs the pipeline once for each page of each epic' do
-      create(:epic, group: group)
-      create(:epic, group: group)
-
-      first_page = ::BulkImports::Pipeline::ExtractedData.new(page_info: {
-        'has_next_page' => true,
-        'next_page' => 'page'
-      })
-      last_page = ::BulkImports::Pipeline::ExtractedData.new(page_info: {
-        'has_next_page' => false
-      })
-
-      expect(subject)
-        .to receive(:extract)
-        .and_return(
-          first_page, # first epic
-          last_page,  # first epic
-          first_page, # last epic
-          last_page   # last epic
-        )
-
-      expect(subject)
-        .to receive(:run)
-        .exactly(4).times
-        .and_call_original
-
-      subject.run
+    context 'when the group has multiple epics with multiple pages of data' do
+      it 'runs the pipeline once for each page of each epic' do
+        create(:epic, group: group)
+        create(:epic, group: group)
+
+        first_page = ::BulkImports::Pipeline::ExtractedData.new(page_info: {
+          'has_next_page' => true,
+          'next_page' => 'page'
+        })
+        last_page = ::BulkImports::Pipeline::ExtractedData.new(page_info: {
+          'has_next_page' => false
+        })
+
+        expect(subject)
+          .to receive(:extract)
+          .and_return(
+            first_page, # first epic
+            last_page,  # first epic
+            first_page, # last epic
+            last_page   # last epic
+          )
+
+        expect(subject)
+          .to receive(:run)
+          .exactly(4).times
+          .and_call_original
+
+        subject.run
+      end
     end
   end
 end