DRY import csv service

- Move import csv service code to a base service
to be able to reuse it for issues and requirements
- Move duplicated spec code to a shared example

app/services/issuable/import_csv/base_service.rb

+# frozen_string_literal: true
+
+module Issuable
+  module ImportCsv
+    class BaseService
+      def initialize(user, project, csv_io)
+        @user = user
+        @project = project
+        @csv_io = csv_io
+        @results = { success: 0, error_lines: [], parse_error: false }
+      end
+
+      def execute
+        process_csv
+        email_results_to_user
+
+        @results
+      end
+
+      private
+
+      def process_csv
+        csv_data = @csv_io.open(&:read).force_encoding(Encoding::UTF_8)
+
+        csv_parsing_params = {
+          col_sep: detect_col_sep(csv_data.lines.first),
+          headers: true,
+          header_converters: :symbol
+        }
+
+        CSV.new(csv_data, csv_parsing_params).each.with_index(2) do |row, line_no|
+          issuable_attributes = {
+            title:       row[:title],
+            description: row[:description]
+          }
+
+          if issuable(issuable_attributes).persisted?
+            @results[:success] += 1
+          else
+            @results[:error_lines].push(line_no)
+          end
+        end
+      rescue ArgumentError, CSV::MalformedCSVError
+        @results[:parse_error] = true
+      end
+
+      def detect_col_sep(header)
+        if header.include?(",")
+          ","
+        elsif header.include?(";")
+          ";"
+        elsif header.include?("\t")
+          "\t"
+        else
+          raise CSV::MalformedCSVError
+        end
+      end
+
+      def email_results_to_user
+        # defined in ImportCsvService
+      end
+
+      private
+
+      def issuable(attributes)
+        create_issuable_class.new(@project, @user, attributes).execute
+      end
+
+      def create_issuable_class
+        # defined in ImportCsvService
+      end
+    end
+  end
+end

app/services/issues/import_csv_service.rb

 # frozen_string_literal: true
 
 module Issues
-  class ImportCsvService
-    def initialize(user, project, csv_io)
-      @user = user
-      @project = project
-      @csv_io = csv_io
-      @results = { success: 0, error_lines: [], parse_error: false }
-    end
-
+  class ImportCsvService < Issuable::ImportCsv::BaseService
     def execute
       record_import_attempt
-      process_csv
-      email_results_to_user
 
-      @results
+      super
     end
 
-    private
-
-    def record_import_attempt
-      Issues::CsvImport.create!(user: @user, project: @project)
+    def email_results_to_user
+      Notify.import_issues_csv_email(@user.id, @project.id, @results).deliver_later
     end
 
-    def process_csv
-      csv_data = @csv_io.open(&:read).force_encoding(Encoding::UTF_8)
-
-      csv_parsing_params = {
-        col_sep: detect_col_sep(csv_data.lines.first),
-        headers: true,
-        header_converters: :symbol
-      }
-
-      CSV.new(csv_data, csv_parsing_params).each.with_index(2) do |row, line_no|
-        issue_attributes = {
-          title:       row[:title],
-          description: row[:description]
-        }
-
-        issue = Issues::CreateService.new(@project, @user, issue_attributes).execute
+    private
 
-        if issue.persisted?
-          @results[:success] += 1
-        else
-          @results[:error_lines].push(line_no)
-        end
-      end
-    rescue ArgumentError, CSV::MalformedCSVError
-      @results[:parse_error] = true
+    def create_issuable_class
+      Issues::CreateService
     end
 
-    def email_results_to_user
-      Notify.import_issues_csv_email(@user.id, @project.id, @results).deliver_later
-    end
-
-    def detect_col_sep(header)
-      if header.include?(",")
-        ","
-      elsif header.include?(";")
-        ";"
-      elsif header.include?("\t")
-        "\t"
-      else
-        raise CSV::MalformedCSVError
-      end
+    def record_import_attempt
+      Issues::CsvImport.create!(user: @user, project: @project)
     end
   end
 end

ee/app/services/requirements_management/import_csv_service.rb

 # frozen_string_literal: true
 
 module RequirementsManagement
-  class ImportCsvService
-    def initialize(user, project, csv_io)
-      @user = user
-      @project = project
-      @csv_io = csv_io
-      @results = { success: 0, error_lines: [], parse_error: false }
-    end
-
-    def execute
-      process_csv
-      email_results_to_user
-
-      @results
-    end
-
-    private
-
-    def process_csv
-      csv_data = @csv_io.open(&:read).force_encoding(Encoding::UTF_8)
-
-      csv_parsing_params = {
-        col_sep: detect_col_sep(csv_data.lines.first),
-        headers: true,
-        header_converters: :symbol
-      }
-
-      CSV.new(csv_data, csv_parsing_params).each.with_index(2) do |row, line_no|
-        requirement_attributes = {
-          title:       row[:title],
-          description: row[:description]
-        }
-
-        requirement = RequirementsManagement::CreateRequirementService.new(@project, @user, requirement_attributes).execute
-
-        if requirement.persisted?
-          @results[:success] += 1
-        else
-          @results[:error_lines].push(line_no)
-        end
-      end
-    rescue ArgumentError, CSV::MalformedCSVError
-      @results[:parse_error] = true
-    end
-
+  class ImportCsvService < ::Issuable::ImportCsv::BaseService
     def email_results_to_user
       Notify.import_requirements_csv_email(@user.id, @project.id, @results).deliver_later
     end
 
-    def detect_col_sep(header)
-      if header.include?(",")
-        ","
-      elsif header.include?(";")
-        ";"
-      elsif header.include?("\t")
-        "\t"
-      else
-        raise CSV::MalformedCSVError
-      end
+    private
+
+    def create_issuable_class
+      RequirementsManagement::CreateRequirementService
     end
   end
 end

ee/spec/services/requirements_management/import_csv_service_spec.rb

@@ -6,101 +6,30 @@ RSpec.describe RequirementsManagement::ImportCsvService do
   let(:project) { create(:project) }
   let(:user) { create(:user) }
 
-  subject do
+  let(:service) do
     uploader = FileUploader.new(project)
     uploader.store!(file)
 
     described_class.new(user, project, uploader).execute
   end
 
-  describe '#execute' do
-    context 'when user can create requirements' do
-      before do
-        project.add_reporter(user)
-        stub_licensed_features(requirements: true)
-      end
-
-      context 'invalid file' do
-        let(:file) { fixture_file_upload('spec/fixtures/banana_sample.gif') }
-
-        it 'returns invalid file error' do
-          expect(Notify).to receive_message_chain(:import_requirements_csv_email, :deliver_later)
-
-          expect(subject[:success]).to eq(0)
-          expect(subject[:parse_error]).to eq(true)
-        end
-      end
-
-      context 'comma delimited file' do
-        let(:file) { fixture_file_upload('spec/fixtures/csv_comma.csv') }
-
-        it 'imports CSV without errors' do
-          expect(Notify).to receive_message_chain(:import_requirements_csv_email, :deliver_later)
-
-          expect(subject[:success]).to eq(3)
-          expect(subject[:error_lines]).to eq([])
-          expect(subject[:parse_error]).to eq(false)
-        end
-
-        it 'correctly sets the requirement attributes' do
-          expect { subject }.to change { project.requirements.count }.by 3
-
-          expect(project.requirements.reload.last).to have_attributes(
-            title: 'Title with quote"',
-            description: 'Description'
-          )
-        end
-      end
-
-      context 'tab delimited file with error row' do
-        let(:file) { fixture_file_upload('spec/fixtures/csv_tab.csv') }
-
-        it 'imports CSV with some error rows' do
-          expect(Notify).to receive_message_chain(:import_requirements_csv_email, :deliver_later)
-
-          expect(subject[:success]).to eq(2)
-          expect(subject[:error_lines]).to eq([3])
-          expect(subject[:parse_error]).to eq(false)
-        end
-
-        it 'correctly sets the requirement attributes' do
-          expect { subject }.to change { project.requirements.count }.by 2
-
-          expect(project.requirements.reload.last).to have_attributes(
-            title: 'Hello',
-            description: 'World'
-          )
-        end
-      end
-
-      context 'semicolon delimited file with CRLF' do
-        let(:file) { fixture_file_upload('spec/fixtures/csv_semicolon.csv') }
-
-        it 'imports CSV with a blank row' do
-          expect(Notify).to receive_message_chain(:import_requirements_csv_email, :deliver_later)
-
-          expect(subject[:success]).to eq(3)
-          expect(subject[:error_lines]).to eq([4])
-          expect(subject[:parse_error]).to eq(false)
-        end
-
-        it 'correctly sets the requirement attributes' do
-          expect { subject }.to change { project.requirements.count }.by 3
+  context 'when user can create requirements' do
+    before do
+      project.add_reporter(user)
+      stub_licensed_features(requirements: true)
+    end
 
-          expect(project.requirements.reload.last).to have_attributes(
-            title: 'Hello',
-            description: 'World'
-          )
-        end
-      end
+    include_examples 'issuable import csv service', 'requirement' do
+      let(:issuables) { project.requirements }
+      let(:email_method) { :import_requirements_csv_email }
     end
+  end
 
-    context 'when user cannot create requirements' do
-      let(:file) { fixture_file_upload('spec/fixtures/csv_comma.csv') }
+  context 'when user cannot create requirements' do
+    let(:file) { fixture_file_upload('spec/fixtures/csv_comma.csv') }
 
-      it 'raises an exception' do
-        expect { subject }.to raise_error(Gitlab::Access::AccessDeniedError)
-      end
+    it 'raises an exception' do
+      expect { service }.to raise_error(Gitlab::Access::AccessDeniedError)
     end
   end
 end

spec/services/issues/import_csv_service_spec.rb

@@ -5,126 +5,15 @@ require 'spec_helper'
 RSpec.describe Issues::ImportCsvService do
   let(:project) { create(:project) }
   let(:user) { create(:user) }
-
-  subject do
+  let(:service) do
     uploader = FileUploader.new(project)
     uploader.store!(file)
 
     described_class.new(user, project, uploader).execute
   end
 
-  shared_examples_for 'an issue importer' do
-    it 'records the import attempt' do
-      expect { subject }
-        .to change { Issues::CsvImport.where(project: project, user: user).count }
-        .by 1
-    end
-  end
-
-  describe '#execute' do
-    context 'invalid file' do
-      let(:file) { fixture_file_upload('spec/fixtures/banana_sample.gif') }
-
-      it 'returns invalid file error' do
-        expect(Notify).to receive_message_chain(:import_issues_csv_email, :deliver_later)
-
-        expect(subject[:success]).to eq(0)
-        expect(subject[:parse_error]).to eq(true)
-      end
-
-      it_behaves_like 'an issue importer'
-    end
-
-    context 'with a file generated by Gitlab CSV export' do
-      let(:file) { fixture_file_upload('spec/fixtures/csv_gitlab_export.csv') }
-
-      it 'imports the CSV without errors' do
-        expect(Notify).to receive_message_chain(:import_issues_csv_email, :deliver_later)
-
-        expect(subject[:success]).to eq(4)
-        expect(subject[:error_lines]).to eq([])
-        expect(subject[:parse_error]).to eq(false)
-      end
-
-      it 'correctly sets the issue attributes' do
-        expect { subject }.to change { project.issues.count }.by 4
-
-        expect(project.issues.reload.last).to have_attributes(
-          title: 'Test Title',
-          description: 'Test Description'
-        )
-      end
-
-      it_behaves_like 'an issue importer'
-    end
-
-    context 'comma delimited file' do
-      let(:file) { fixture_file_upload('spec/fixtures/csv_comma.csv') }
-
-      it 'imports CSV without errors' do
-        expect(Notify).to receive_message_chain(:import_issues_csv_email, :deliver_later)
-
-        expect(subject[:success]).to eq(3)
-        expect(subject[:error_lines]).to eq([])
-        expect(subject[:parse_error]).to eq(false)
-      end
-
-      it 'correctly sets the issue attributes' do
-        expect { subject }.to change { project.issues.count }.by 3
-
-        expect(project.issues.reload.last).to have_attributes(
-          title: 'Title with quote"',
-          description: 'Description'
-        )
-      end
-
-      it_behaves_like 'an issue importer'
-    end
-
-    context 'tab delimited file with error row' do
-      let(:file) { fixture_file_upload('spec/fixtures/csv_tab.csv') }
-
-      it 'imports CSV with some error rows' do
-        expect(Notify).to receive_message_chain(:import_issues_csv_email, :deliver_later)
-
-        expect(subject[:success]).to eq(2)
-        expect(subject[:error_lines]).to eq([3])
-        expect(subject[:parse_error]).to eq(false)
-      end
-
-      it 'correctly sets the issue attributes' do
-        expect { subject }.to change { project.issues.count }.by 2
-
-        expect(project.issues.reload.last).to have_attributes(
-          title: 'Hello',
-          description: 'World'
-        )
-      end
-
-      it_behaves_like 'an issue importer'
-    end
-
-    context 'semicolon delimited file with CRLF' do
-      let(:file) { fixture_file_upload('spec/fixtures/csv_semicolon.csv') }
-
-      it 'imports CSV with a blank row' do
-        expect(Notify).to receive_message_chain(:import_issues_csv_email, :deliver_later)
-
-        expect(subject[:success]).to eq(3)
-        expect(subject[:error_lines]).to eq([4])
-        expect(subject[:parse_error]).to eq(false)
-      end
-
-      it 'correctly sets the issue attributes' do
-        expect { subject }.to change { project.issues.count }.by 3
-
-        expect(project.issues.reload.last).to have_attributes(
-          title: 'Hello',
-          description: 'World'
-        )
-      end
-
-      it_behaves_like 'an issue importer'
-    end
+  include_examples 'issuable import csv service', 'issue' do
+    let(:issuables) { project.issues }
+    let(:email_method) { :import_issues_csv_email }
   end
 end

spec/support/services/issuable_import_csv_service_shared_examples.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.shared_examples 'issuable import csv service' do |issuable_type|
+  let(:project) { create(:project) }
+  let(:user) { create(:user) }
+
+  subject { service }
+
+  shared_examples_for 'an issuable importer' do
+    it 'records the import attempt if resource is an issue' do
+      if issuable_type == 'issue'
+        expect { subject }
+          .to change { Issues::CsvImport.where(project: project, user: user).count }
+          .by 1
+      end
+    end
+  end
+
+  describe '#execute' do
+    context 'invalid file' do
+      let(:file) { fixture_file_upload('spec/fixtures/banana_sample.gif') }
+
+      it 'returns invalid file error' do
+        expect(Notify).to receive_message_chain(email_method, :deliver_later)
+
+        expect(subject[:success]).to eq(0)
+        expect(subject[:parse_error]).to eq(true)
+      end
+
+      it_behaves_like 'an issuable importer'
+    end
+
+    context 'with a file generated by Gitlab CSV export' do
+      let(:file) { fixture_file_upload('spec/fixtures/csv_gitlab_export.csv') }
+
+      it 'imports the CSV without errors' do
+        expect(Notify).to receive_message_chain(email_method, :deliver_later)
+
+        expect(subject[:success]).to eq(4)
+        expect(subject[:error_lines]).to eq([])
+        expect(subject[:parse_error]).to eq(false)
+      end
+
+      it 'correctly sets the issuable attributes' do
+        expect { subject }.to change { issuables.count }.by 4
+
+        expect(issuables.reload.last).to have_attributes(
+          title: 'Test Title',
+          description: 'Test Description'
+        )
+      end
+
+      it_behaves_like 'an issuable importer'
+    end
+
+    context 'comma delimited file' do
+      let(:file) { fixture_file_upload('spec/fixtures/csv_comma.csv') }
+
+      it 'imports CSV without errors' do
+        expect(Notify).to receive_message_chain(email_method, :deliver_later)
+
+        expect(subject[:success]).to eq(3)
+        expect(subject[:error_lines]).to eq([])
+        expect(subject[:parse_error]).to eq(false)
+      end
+
+      it 'correctly sets the issuable attributes' do
+        expect { subject }.to change { issuables.count }.by 3
+
+        expect(issuables.reload.last).to have_attributes(
+          title: 'Title with quote"',
+          description: 'Description'
+        )
+      end
+
+      it_behaves_like 'an issuable importer'
+    end
+
+    context 'tab delimited file with error row' do
+      let(:file) { fixture_file_upload('spec/fixtures/csv_tab.csv') }
+
+      it 'imports CSV with some error rows' do
+        expect(Notify).to receive_message_chain(email_method, :deliver_later)
+
+        expect(subject[:success]).to eq(2)
+        expect(subject[:error_lines]).to eq([3])
+        expect(subject[:parse_error]).to eq(false)
+      end
+
+      it 'correctly sets the issuable attributes' do
+        expect { subject }.to change { issuables.count }.by 2
+
+        expect(issuables.reload.last).to have_attributes(
+          title: 'Hello',
+          description: 'World'
+        )
+      end
+
+      it_behaves_like 'an issuable importer'
+    end
+
+    context 'semicolon delimited file with CRLF' do
+      let(:file) { fixture_file_upload('spec/fixtures/csv_semicolon.csv') }
+
+      it 'imports CSV with a blank row' do
+        expect(Notify).to receive_message_chain(email_method, :deliver_later)
+
+        expect(subject[:success]).to eq(3)
+        expect(subject[:error_lines]).to eq([4])
+        expect(subject[:parse_error]).to eq(false)
+      end
+
+      it 'correctly sets the issuable attributes' do
+        expect { subject }.to change { issuables.count }.by 3
+
+        expect(issuables.reload.last).to have_attributes(
+          title: 'Hello',
+          description: 'World'
+        )
+      end
+
+      it_behaves_like 'an issuable importer'
+    end
+  end
+end