diff --git a/app/services/import_csv/base_service.rb b/app/services/import_csv/base_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..fd75a251dd2e81ca3cc643b22aa63c3b9e544cc9 --- /dev/null +++ b/app/services/import_csv/base_service.rb @@ -0,0 +1,99 @@ +# frozen_string_literal: true + +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 + + def email_results_to_user + raise NotImplementedError + end + + private + + attr_reader :user, :project, :csv_io, :results + + def attributes_for(row) + raise NotImplementedError + end + + def validate_headers_presence!(headers) + raise NotImplementedError + end + + def create_object_class + raise NotImplementedError + end + + def process_csv + with_csv_lines.each do |row, line_no| + attributes = attributes_for(row) + + if create_object(attributes)&.persisted? + results[:success] += 1 + else + results[:error_lines].push(line_no) + end + end + rescue ArgumentError, CSV::MalformedCSVError + results[:parse_error] = true + end + + def with_csv_lines + csv_data = @csv_io.open(&:read).force_encoding(Encoding::UTF_8) + validate_headers_presence!(csv_data.lines.first) + + CSV.new( + csv_data, + col_sep: detect_col_sep(csv_data.lines.first), + headers: true, + header_converters: :symbol + ).each.with_index(2) + end + + def detect_col_sep(header) + if header.include?(",") + "," + elsif header.include?(";") + ";" + elsif header.include?("\t") + "\t" + else + raise CSV::MalformedCSVError.new('Invalid CSV format', 1) + end + end + + def create_object(attributes) + # NOTE: CSV imports are performed by workers, so we do not have a request context in order + # to create a SpamParams object to pass to the issuable create service. + spam_params = nil + + # default_params can be extracted into a method if we need + # to support creation of objects that belongs to groups. + default_params = { project: project, + current_user: user, + params: attributes, + spam_params: spam_params } + + create_service = create_object_class.new(**default_params.merge(extra_create_service_params)) + + create_service.execute_without_rate_limiting + end + + # Overidden in subclasses to support specific parameters + def extra_create_service_params + {} + end + end +end diff --git a/app/services/issuable/import_csv/base_service.rb b/app/services/issuable/import_csv/base_service.rb index e84d1032e41144a57aaf4b8c4d6fa50647b968b9..83cf5a6745364ef6df4766ba5d8453759ca48a5c 100644 --- a/app/services/issuable/import_csv/base_service.rb +++ b/app/services/issuable/import_csv/base_service.rb @@ -2,38 +2,13 @@ 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 + class BaseService < ::ImportCsv::BaseService + extend ::Gitlab::Utils::Override private - def process_csv - with_csv_lines.each do |row, line_no| - attributes = issuable_attributes_for(row) - - if create_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 issuable_attributes_for(row) + override :attributes_for + def attributes_for(row) { title: row[:title], description: row[:description], @@ -41,58 +16,13 @@ def issuable_attributes_for(row) } end - def with_csv_lines - csv_data = @csv_io.open(&:read).force_encoding(Encoding::UTF_8) - validate_headers_presence!(csv_data.lines.first) - - CSV.new( - csv_data, - col_sep: detect_col_sep(csv_data.lines.first), - headers: true, - header_converters: :symbol - ).each.with_index(2) - end - + override :validate_headers_presence! def validate_headers_presence!(headers) headers.downcase! if headers return if headers && headers.include?('title') && headers.include?('description') raise CSV::MalformedCSVError end - - def detect_col_sep(header) - if header.include?(",") - "," - elsif header.include?(";") - ";" - elsif header.include?("\t") - "\t" - else - raise CSV::MalformedCSVError - end - end - - def create_issuable(attributes) - # NOTE: CSV imports are performed by workers, so we do not have a request context in order - # to create a SpamParams object to pass to the issuable create service. - spam_params = nil - create_service = create_issuable_class.new(project: @project, current_user: @user, params: attributes, spam_params: spam_params) - - # For now, if create_issuable_class prepends RateLimitedService let's bypass rate limiting - if create_issuable_class < RateLimitedService - create_service.execute_without_rate_limiting - else - create_service.execute - end - end - - def email_results_to_user - # defined in ImportCsvService - end - - def create_issuable_class - # defined in ImportCsvService - end end end end diff --git a/app/services/issues/import_csv_service.rb b/app/services/issues/import_csv_service.rb index 83e550583f60e54fdfcce2cbba7292fdb53f514b..c3d6af952b445362447fe497967e22f5aafaa22b 100644 --- a/app/services/issues/import_csv_service.rb +++ b/app/services/issues/import_csv_service.rb @@ -9,21 +9,21 @@ def execute end def email_results_to_user - Notify.import_issues_csv_email(@user.id, @project.id, @results).deliver_later + Notify.import_issues_csv_email(user.id, project.id, results).deliver_later end private - def create_issuable(attributes) + def create_object(attributes) super[:issue] end - def create_issuable_class + def create_object_class Issues::CreateService end def record_import_attempt - Issues::CsvImport.create!(user: @user, project: @project) + Issues::CsvImport.create!(user: user, project: project) end end end diff --git a/ee/app/services/requirements_management/import_csv_service.rb b/ee/app/services/requirements_management/import_csv_service.rb index 07d3162a033dc510e057eb8f78971bff586f5f09..1311575b738a68c305fe2429cddd3843071ed984 100644 --- a/ee/app/services/requirements_management/import_csv_service.rb +++ b/ee/app/services/requirements_management/import_csv_service.rb @@ -3,26 +3,26 @@ module RequirementsManagement class ImportCsvService < ::Issuable::ImportCsv::BaseService def execute - raise Gitlab::Access::AccessDeniedError unless Ability.allowed?(@user, :create_requirement, @project) + raise Gitlab::Access::AccessDeniedError unless Ability.allowed?(user, :create_requirement, project) super end private - def create_issuable(attributes) + def create_object(attributes) super[:issue] end - def create_issuable_class + def create_object_class ::Issues::CreateService end def email_results_to_user - Notify.import_requirements_csv_email(@user.id, @project.id, @results).deliver_later + Notify.import_requirements_csv_email(user.id, project.id, results).deliver_later end - def issuable_attributes_for(row) + def attributes_for(row) super.merge(issue_type: 'requirement') end end diff --git a/ee/spec/services/requirements_management/import_csv_service_spec.rb b/ee/spec/services/requirements_management/import_csv_service_spec.rb index 18c1c9e5dd474c0148248039b34c285bfa514cc3..c666b212c5626b636b1862265e047f031aa8a4b6 100644 --- a/ee/spec/services/requirements_management/import_csv_service_spec.rb +++ b/ee/spec/services/requirements_management/import_csv_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe RequirementsManagement::ImportCsvService do +RSpec.describe RequirementsManagement::ImportCsvService, feature_category: :requirements_management do let_it_be_with_refind(:project) { create(:project) } let_it_be(:user) { create(:user) } diff --git a/spec/services/import_csv/base_service_spec.rb b/spec/services/import_csv/base_service_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..0c0ed40ff4d21da6c173cb0f110034b65cb26bbb --- /dev/null +++ b/spec/services/import_csv/base_service_spec.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ImportCsv::BaseService, feature_category: :importers do + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project) } + let_it_be(:csv_io) { double } + + subject { described_class.new(user, project, csv_io) } + + shared_examples 'abstract method' do |method, args| + it "raises NotImplemented error when #{method} is called" do + if args + expect { subject.send(method, args) }.to raise_error(NotImplementedError) + else + expect { subject.send(method) }.to raise_error(NotImplementedError) + end + end + end + + it_behaves_like 'abstract method', :email_results_to_user + it_behaves_like 'abstract method', :attributes_for, "any" + it_behaves_like 'abstract method', :validate_headers_presence!, "any" + it_behaves_like 'abstract method', :create_object_class + + describe '#detect_col_sep' do + context 'when header contains invalid separators' do + it 'raises error' do + header = 'Name&email' + + expect { subject.send(:detect_col_sep, header) }.to raise_error(CSV::MalformedCSVError) + end + end + + context 'when header is valid' do + shared_examples 'header with valid separators' do + let(:header) { "Name#{separator}email" } + + it 'returns separator value' do + expect(subject.send(:detect_col_sep, header)).to eq(separator) + end + end + + context 'with ; as separator' do + let(:separator) { ';' } + + it_behaves_like 'header with valid separators' + end + + context 'with \t as separator' do + let(:separator) { "\t" } + + it_behaves_like 'header with valid separators' + end + + context 'with , as separator' do + let(:separator) { ',' } + + it_behaves_like 'header with valid separators' + end + end + end +end diff --git a/spec/services/issues/import_csv_service_spec.rb b/spec/services/issues/import_csv_service_spec.rb index 9ad1d7dba9f9417ae2c9311200e60f60dfabd9ba..90e360f9cf15d23cfbebe4a5271790f0426b91f7 100644 --- a/spec/services/issues/import_csv_service_spec.rb +++ b/spec/services/issues/import_csv_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Issues::ImportCsvService do +RSpec.describe Issues::ImportCsvService, feature_category: :team_planning do let(:project) { create(:project) } let(:user) { create(:user) } let(:assignee) { create(:user, username: 'csv_assignee') }