diff --git a/app/mailers/emails/issues.rb b/app/mailers/emails/issues.rb index 6a5680c080bd363d1a48f2567014d4e8a467d971..7227a284e523292aac845f21f79b907ea6636940 100644 --- a/app/mailers/emails/issues.rb +++ b/app/mailers/emails/issues.rb @@ -99,6 +99,16 @@ def import_issues_csv_email(user_id, project_id, results) subject: subject('Imported issues')) end + def import_work_items_csv_email(user_id, project_id, results) + @user = User.find(user_id) + @project = Project.find(project_id) + @results = results + + email_with_layout( + to: @user.notification_email_for(@project.group), + subject: subject('Imported work items')) + end + def issues_csv_email(user, project, csv_data, export_status) @project = project @count = export_status.fetch(:rows_expected) diff --git a/app/services/import_csv/base_service.rb b/app/services/import_csv/base_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..5f5883a586033612ac0a9b9fb3e41f54e2d66c2b --- /dev/null +++ b/app/services/import_csv/base_service.rb @@ -0,0 +1,109 @@ +# 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 + + private + + 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 attributes_for(row) + raise NotImplementedError + end + + def headers_valid?(headers) + required_headers.all? { |header| headers.include?(header) } + end + + def required_headers + NotImplementedError + 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 validate_headers_presence!(headers) + headers.downcase! if headers + return if headers && headers_valid?(headers) + + 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_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 = { project: @project, + current_user: @user, + params: attributes, + spam_params: spam_params } + + create_service = create_service_class.new(**default_params.merge(extra_create_service_params)) + + # For now, if create_service_class prepends RateLimitedService let's bypass rate limiting + if create_service_class < RateLimitedService + create_service.execute_without_rate_limiting + else + create_service.execute + end + end + + def extra_create_service_params + {} + end + + def email_results_to_user + # defined in ImportCsvService + end + + def create_service_class + # defined in ImportCsvService + 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..30a622621ca86737951cc8ecdb27060437e0970c 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,57 +16,9 @@ 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 - - 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 + override :required_headers + def required_headers + %w[title description].freeze end end end diff --git a/app/services/issues/import_csv_service.rb b/app/services/issues/import_csv_service.rb index 83e550583f60e54fdfcce2cbba7292fdb53f514b..aa0bc5d1fe080db7b7e5c18fb1a96842bd75b941 100644 --- a/app/services/issues/import_csv_service.rb +++ b/app/services/issues/import_csv_service.rb @@ -14,11 +14,11 @@ def email_results_to_user private - def create_issuable(attributes) + def create_object(attributes) super[:issue] end - def create_issuable_class + def create_service_class Issues::CreateService end diff --git a/app/services/work_items/import_csv_service.rb b/app/services/work_items/import_csv_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..4313d49f8a456f3c05abce9b6324a1c739527d7c --- /dev/null +++ b/app/services/work_items/import_csv_service.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +module WorkItems + class ImportCsvService < ::ImportCsv::BaseService + extend ::Gitlab::Utils::Override + + def execute + raise Gitlab::Access::AccessDeniedError unless Ability.allowed?(@user, :create_work_item, @project) + + super + end + + private + + override :attributes_for + def attributes_for(row) + { + # depends on work item type + # map key -> value + } + end + + override :extra_create_service_params + def extra_create_service_params + { widget_params: {} } + end + + override :required_headers + def required_headers + # depends on work item type + end + + def create_service_class + ::WorkItems::CreateService + end + + def email_results_to_user + Notify.import_work_items_csv_email(@user.id, @project.id, @results).deliver_later + end + end +end diff --git a/app/views/notify/import_issues_csv_email.html.haml b/app/views/notify/import_issues_csv_email.html.haml index 0008085025bd9fbc813c6a2bd82ebeec6d36b583..5613d08142f39164d15cfd74ffbda4ea5520bb4a 100644 --- a/app/views/notify/import_issues_csv_email.html.haml +++ b/app/views/notify/import_issues_csv_email.html.haml @@ -5,12 +5,12 @@ = s_('Notify|Your CSV import for project %{project_link} has been completed.').html_safe % { project_link: project_link } %p{ style: text_style } - - issues = n_('%d issue', '%d issues', @results[:success]) % @results[:success] - = s_('Notify|%{issues} imported.') % { issues: issues } + - work_items = n_('%d work item', '%d work items', @results[:success]) % @results[:success] + = s_('Notify|%{work_items} imported.') % { work_items: work_items } - if @results[:error_lines].present? %p{ style: text_style } - = s_('Notify|Errors found on %{singular_or_plural_line}: %{error_lines}. Please check if these lines have an issue title.') % { singular_or_plural_line: n_('line', 'lines', @results[:error_lines].size), + = s_('Notify|Errors found on %{singular_or_plural_line}: %{error_lines}. Please check if these lines have an work item title.') % { singular_or_plural_line: n_('line', 'lines', @results[:error_lines].size), error_lines: @results[:error_lines].join(', ') } - if @results[:parse_error] diff --git a/app/views/notify/import_issues_csv_email.text.erb b/app/views/notify/import_issues_csv_email.text.erb index 1117f90714dc890823707ed000c1aff1166df91d..454441e36b95344f58ff2be660769a94e708fd54 100644 --- a/app/views/notify/import_issues_csv_email.text.erb +++ b/app/views/notify/import_issues_csv_email.text.erb @@ -1,9 +1,9 @@ Your CSV import for project <%= @project.full_name %> (<%= project_url(@project) %>) has been completed. -<%= pluralize(@results[:success], 'issue') %> imported. +<%= pluralize(@results[:success], 'work_items') %> imported. <% if @results[:error_lines].present? %> -Errors found on line <%= 'number'.pluralize(@results[:error_lines].size) %>: <%= @results[:error_lines].join(', ') %>. Please check if these lines have an issue title. +Errors found on line <%= 'number'.pluralize(@results[:error_lines].size) %>: <%= @results[:error_lines].join(', ') %>. Please check if these lines have an work items title. <% end %> <% if @results[:parse_error] %> diff --git a/app/workers/work_items/import_work_items_csv_worker.rb b/app/workers/work_items/import_work_items_csv_worker.rb new file mode 100644 index 0000000000000000000000000000000000000000..b9e3196d69127fac50137de0b0bf3cb88f959929 --- /dev/null +++ b/app/workers/work_items/import_work_items_csv_worker.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +module WorkItems + class ImportWorkItemsCsvWorker + include ApplicationWorker + + data_consistency :always + + sidekiq_options retry: 3 + include Gitlab::Utils::StrongMemoize + + idempotent! + feature_category :work_items + + sidekiq_retries_exhausted do |job| + Upload.find(job['args'][2]).destroy + end + + def perform(current_user_id, project_id, upload_id) + upload = Upload.find(upload_id) + user = User.find(current_user_id) + parent_link = ::WorkItems::ParentLink.find(project_id) + + WorkItems::ImportCsvService.new(user, parent_link, upload.retrieve_uploader).execute + upload.destroy! + rescue ActiveRecord::RecordNotFound + # Resources have been removed, job should not be retried + end + end +end diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index 548e643c7feb17ebf93603f6cfa45797764e30d3..49d5c6eaac85bcb18ab682a30af7b8fac7d17047 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -517,5 +517,7 @@ - 1 - - wikis_git_garbage_collect - 1 +- - work_items_import_work_items_csv + - 1 - - x509_certificate_revoke - 1 diff --git a/ee/app/services/requirements_management/import_csv_service.rb b/ee/app/services/requirements_management/import_csv_service.rb index 07d3162a033dc510e057eb8f78971bff586f5f09..406a425352214e3cf27729874dc875185a8473e2 100644 --- a/ee/app/services/requirements_management/import_csv_service.rb +++ b/ee/app/services/requirements_management/import_csv_service.rb @@ -10,11 +10,11 @@ def execute private - def create_issuable(attributes) + def create_object(attributes) super[:issue] end - def create_issuable_class + def create_service_class ::Issues::CreateService end @@ -22,7 +22,7 @@ def email_results_to_user 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/locale/gitlab.pot b/locale/gitlab.pot index a4ac7b42231a7e6c773c34274dbd9a10ea8b22f6..aa0da50229a25db7a95326dfed3f315d1fa73fe0 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -468,6 +468,11 @@ msgid_plural "%d warnings found:" msgstr[0] "" msgstr[1] "" +msgid "%d work item" +msgid_plural "%d work items" +msgstr[0] "" +msgstr[1] "" + msgid "%s additional commit has been omitted to prevent performance issues." msgid_plural "%s additional commits have been omitted to prevent performance issues." msgstr[0] "" @@ -27484,9 +27489,6 @@ msgstr "" msgid "Notify|%{invited_user} has %{highlight_start}declined%{highlight_end} your invitation to join the %{target_link} %{target_name}." msgstr "" -msgid "Notify|%{issues} imported." -msgstr "" - msgid "Notify|%{member_link} requested %{member_role} access to the %{target_source_link} %{target_type}." msgstr "" @@ -27514,6 +27516,9 @@ msgstr "" msgid "Notify|%{updated_by_user_name} pushed new commits to merge request %{mr_link}" msgstr "" +msgid "Notify|%{work_items} imported." +msgstr "" + msgid "Notify|A new GPG key was added to your account:" msgstr "" @@ -27559,7 +27564,7 @@ msgstr "" msgid "Notify|Error parsing CSV file. Please make sure it has the correct format: a delimited text file that uses a comma to separate values." msgstr "" -msgid "Notify|Errors found on %{singular_or_plural_line}: %{error_lines}. Please check if these lines have an issue title." +msgid "Notify|Errors found on %{singular_or_plural_line}: %{error_lines}. Please check if these lines have an work item title." msgstr "" msgid "Notify|Fingerprint: %{fingerprint}" diff --git a/spec/views/notify/import_issues_csv_email.html.haml_spec.rb b/spec/views/notify/import_issues_csv_email.html.haml_spec.rb index 43dfab87ac9abc1bc91e1df5733f9d69eb6cd68e..9135bd7a3c407aaee85fc4fd0c74bce074df168f 100644 --- a/spec/views/notify/import_issues_csv_email.html.haml_spec.rb +++ b/spec/views/notify/import_issues_csv_email.html.haml_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'notify/import_issues_csv_email.html.haml' do +RSpec.describe 'notify/import_work_items_csv_email.html.haml' do let(:user) { create(:user) } let(:project) { create(:project) } let(:correct_results) { { success: 3, valid_file: true } } @@ -23,7 +23,7 @@ render expect(rendered).to have_link(project.full_name, href: project_url(project)) - expect(rendered).to have_content("3 issues imported") + expect(rendered).to have_content("3 work items imported") expect(rendered).not_to have_content("Errors found on line") expect(rendered).not_to have_content( "Error parsing CSV file. Please make sure it has the correct format: \ @@ -40,7 +40,7 @@ render expect(rendered).to have_content("Errors found on lines: #{errored_results[:error_lines].join(", ")}. \ -Please check if these lines have an issue title.") +Please check if these lines have an work item title.") expect(rendered).not_to have_content("Error parsing CSV file. Please make sure it has the correct format: \ a delimited text file that uses a comma to separate values.") end