Merge branch 'kassio/remove-unnecessary-transform-from-pipeline' into 'master'

BulkImports: Remove unnecessary transform from GroupAvatar

See merge request gitlab-org/gitlab!65578

lib/bulk_imports/groups/pipelines/group_avatar_pipeline.rb

@@ -24,10 +24,6 @@ module BulkImports
           BulkImports::Pipeline::ExtractedData.new(data: { filepath: filepath })
         end
 
-        def transform(_, data)
-          data
-        end
-
         def load(context, data)
           return if data.blank?
 
@@ -44,7 +40,7 @@ module BulkImports
           end
         end
 
-        def after_run(context, _)
+        def after_run(_)
           FileUtils.remove_entry(context.extra[:tmpdir]) if context.extra[:tmpdir].present?
         end
       end

spec/lib/bulk_imports/groups/pipelines/group_avatar_pipeline_spec.rb

@@ -23,47 +23,55 @@ RSpec.describe BulkImports::Groups::Pipelines::GroupAvatarPipeline do
 
   subject { described_class.new(context) }
 
-  describe '#extract' do
-    it 'downloads the group avatar' do
-      expect_next_instance_of(
-        BulkImports::FileDownloadService,
+  describe '#run' do
+    it 'updates the group avatar' do
+      avatar_path = 'spec/fixtures/dk.png'
+      stub_file_download(
+        avatar_path,
         configuration: context.configuration,
         relative_url: "/groups/source%2Ffull%2Fpath/avatar",
         dir: an_instance_of(String),
         file_size_limit: Avatarable::MAXIMUM_FILE_SIZE,
         allowed_content_types: described_class::ALLOWED_AVATAR_DOWNLOAD_TYPES
-      ) do |downloader|
-        expect(downloader).to receive(:execute)
-      end
+      )
 
-      subject.run
-    end
-  end
+      expect { subject.run }.to change(context.group, :avatar)
 
-  describe '#transform' do
-    it 'returns the given data' do
-      expect(subject.transform(nil, :value)).to eq(:value)
+      expect(context.group.avatar.filename).to eq(File.basename(avatar_path))
     end
-  end
 
-  describe '#load' do
-    it 'updates the group avatar' do
-      avatar_path = 'spec/fixtures/dk.png'
-      data = { filepath: fixture_file_upload(avatar_path) }
+    it 'raises an error when the avatar upload fails' do
+      avatar_path = 'spec/fixtures/aosp_manifest.xml'
+      stub_file_download(
+        avatar_path,
+        configuration: context.configuration,
+        relative_url: "/groups/source%2Ffull%2Fpath/avatar",
+        dir: an_instance_of(String),
+        file_size_limit: Avatarable::MAXIMUM_FILE_SIZE,
+        allowed_content_types: described_class::ALLOWED_AVATAR_DOWNLOAD_TYPES
+      )
 
-      expect { subject.load(context, data) }.to change(group, :avatar)
+      expect_next_instance_of(Gitlab::Import::Logger) do |logger|
+        expect(logger).to receive(:error)
+          .with(
+            bulk_import_id: context.bulk_import.id,
+            bulk_import_entity_id: context.entity.id,
+            bulk_import_entity_type: context.entity.source_type,
+            context_extra: context.extra,
+            exception_class: "BulkImports::Groups::Pipelines::GroupAvatarPipeline::GroupAvatarLoadingError",
+            exception_message: "Avatar file format is not supported. Please try one of the following supported formats: image/png, image/jpeg, image/gif, image/bmp, image/tiff, image/vnd.microsoft.icon",
+            pipeline_class: "BulkImports::Groups::Pipelines::GroupAvatarPipeline",
+            pipeline_step: :loader
+          )
+      end
 
-      expect(FileUtils.identical?(avatar_path, group.avatar.file.file)).to eq(true)
+      expect { subject.run }.to change(BulkImports::Failure, :count)
     end
+  end
 
-    it 'raises an error when the avatar upload fails' do
-      avatar_path = 'spec/fixtures/aosp_manifest.xml'
-      data = { filepath: fixture_file_upload(avatar_path) }
-
-      expect { subject.load(context, data) }.to raise_error(
-        described_class::GroupAvatarLoadingError,
-        "Avatar file format is not supported. Please try one of the following supported formats: image/png, image/jpeg, image/gif, image/bmp, image/tiff, image/vnd.microsoft.icon"
-      )
+  def stub_file_download(filepath = 'file/path.png', **params)
+    expect_next_instance_of(BulkImports::FileDownloadService, params.presence) do |downloader|
+      expect(downloader).to receive(:execute).and_return(filepath)
     end
   end
 end