summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorHeinrich Lee Yu <hleeyu@gmail.com>2018-12-31 16:39:43 +0800
committerHeinrich Lee Yu <hleeyu@gmail.com>2019-01-07 11:16:58 +0800
commite2698d5d7455d91fa94f9bbf1fc838f8cb142700 (patch)
tree900a7abd2983a5a31843c25a13ce06898c1e02d6
parent63e9969ca3ac57839b78d9cc44bcf32bc9a45248 (diff)
downloadgitlab-ce-e2698d5d7455d91fa94f9bbf1fc838f8cb142700.tar.gz
Improve email messages
Also refactored cleanup view to use the same localized string
-rw-r--r--app/controllers/concerns/uploads_actions.rb4
-rw-r--r--app/policies/project_policy.rb3
-rw-r--r--app/services/issues/import_csv_service.rb6
-rw-r--r--app/views/notify/import_issues_csv_email.html.haml10
-rw-r--r--app/views/notify/import_issues_csv_email.text.erb10
-rw-r--r--app/views/projects/cleanup/_show.html.haml2
-rw-r--r--app/views/projects/issues/import_csv/_modal.html.haml5
-rw-r--r--locale/gitlab.pot3
-rw-r--r--spec/services/issues/import_csv_service_spec.rb14
-rw-r--r--spec/services/upload_service_spec.rb2
10 files changed, 28 insertions, 31 deletions
diff --git a/app/controllers/concerns/uploads_actions.rb b/app/controllers/concerns/uploads_actions.rb
index 5992ad5f1e1..3771c97fdc8 100644
--- a/app/controllers/concerns/uploads_actions.rb
+++ b/app/controllers/concerns/uploads_actions.rb
@@ -7,12 +7,12 @@ module UploadsActions
UPLOAD_MOUNTS = %w(avatar attachment file logo header_logo favicon).freeze
def create
- link_to_file = UploadService.new(model, params[:file], uploader_class).execute.to_h
+ link_to_file = UploadService.new(model, params[:file], uploader_class).execute
respond_to do |format|
if link_to_file
format.json do
- render json: { link: link_to_file }
+ render json: { link: link_to_file.to_h }
end
else
format.json do
diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb
index 0026a88b972..8e256b766df 100644
--- a/app/policies/project_policy.rb
+++ b/app/policies/project_policy.rb
@@ -222,8 +222,9 @@ class ProjectPolicy < BasePolicy
rule { owner | admin | guest | group_member }.prevent :request_access
rule { ~request_access_enabled }.prevent :request_access
+ rule { developer & can?(:create_issue) }.enable :import_issues
+
rule { can?(:developer_access) }.policy do
- enable :import_issues
enable :admin_merge_request
enable :admin_milestone
enable :update_merge_request
diff --git a/app/services/issues/import_csv_service.rb b/app/services/issues/import_csv_service.rb
index 411000a7066..7fa2ecc3afd 100644
--- a/app/services/issues/import_csv_service.rb
+++ b/app/services/issues/import_csv_service.rb
@@ -6,7 +6,7 @@ module Issues
@user = user
@project = project
@uploader = upload.build_uploader
- @results = { success: 0, errors: [], valid_file: true }
+ @results = { success: 0, error_lines: [], parse_error: false }
end
def execute
@@ -30,11 +30,11 @@ module Issues
if issue.persisted?
@results[:success] += 1
else
- @results[:errors].push(line_no)
+ @results[:error_lines].push(line_no)
end
end
rescue ArgumentError, CSV::MalformedCSVError
- @results[:valid_file] = false
+ @results[:parse_error] = true
end
def email_results_to_user
diff --git a/app/views/notify/import_issues_csv_email.html.haml b/app/views/notify/import_issues_csv_email.html.haml
index a6a797a5fb7..f30d2b5f078 100644
--- a/app/views/notify/import_issues_csv_email.html.haml
+++ b/app/views/notify/import_issues_csv_email.html.haml
@@ -7,12 +7,12 @@
has been completed.
%p{ style: text_style }
- #{pluralize(@results[:success], 'issue')} imported successfully.
+ #{pluralize(@results[:success], 'issue')} imported.
-- if @results[:errors].present?
+- if @results[:error_lines].present?
%p{ style: text_style }
- Errors found on line #{'number'.pluralize(@results[:errors].size)}: #{@results[:errors].join(', ')}.
+ Errors found on line #{'number'.pluralize(@results[:error_lines].size)}: #{@results[:error_lines].join(', ')}. Please check if these lines have an issue title.
-- unless @results[:valid_file]
+- if @results[:parse_error]
%p{ style: text_style }
- Error parsing CSV file.
+ Error parsing CSV file. Please make sure it has the correct format: a delimited text file that uses a comma to separate values.
diff --git a/app/views/notify/import_issues_csv_email.text.erb b/app/views/notify/import_issues_csv_email.text.erb
index 54a1762c4ec..1117f90714d 100644
--- a/app/views/notify/import_issues_csv_email.text.erb
+++ b/app/views/notify/import_issues_csv_email.text.erb
@@ -1,11 +1,11 @@
Your CSV import for project <%= @project.full_name %> (<%= project_url(@project) %>) has been completed.
-<%= pluralize(@results[:success], 'issue') %> imported successfully.
+<%= pluralize(@results[:success], 'issue') %> imported.
-<% if @results[:errors].present? %>
-Errors found on line <%= 'number'.pluralize(@results[:errors].size) %>: <%= @results[:errors].join(', ') %>.
+<% 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.
<% end %>
-<% unless @results[:valid_file] %>
-Error parsing CSV file.
+<% if @results[:parse_error] %>
+Error parsing CSV file. Please make sure it has the correct format: a delimited text file that uses a comma to separate values.
<% end %>
diff --git a/app/views/projects/cleanup/_show.html.haml b/app/views/projects/cleanup/_show.html.haml
index cecc139b183..888be4ee282 100644
--- a/app/views/projects/cleanup/_show.html.haml
+++ b/app/views/projects/cleanup/_show.html.haml
@@ -24,6 +24,6 @@
= _("No file selected")
= f.file_field :bfg_object_map, accept: 'text/plain', class: "hidden js-object-map-input", required: true
.form-text.text-muted
- = _("The maximum file size allowed is %{max_attachment_size}mb") % { max_attachment_size: Gitlab::CurrentSettings.max_attachment_size }
+ = _("The maximum file size allowed is %{size}.") % { size: number_to_human_size(Gitlab::CurrentSettings.max_attachment_size.megabytes) }
= f.submit _('Start cleanup'), class: 'btn btn-success'
diff --git a/app/views/projects/issues/import_csv/_modal.html.haml b/app/views/projects/issues/import_csv/_modal.html.haml
index 03a6c79ff74..bef34c93e63 100644
--- a/app/views/projects/issues/import_csv/_modal.html.haml
+++ b/app/views/projects/issues/import_csv/_modal.html.haml
@@ -1,7 +1,7 @@
.issues-import-modal.modal
.modal-dialog
.modal-content
- = form_tag [:import_csv, @project.namespace.becomes(Namespace), @project, :issues], multipart: true do
+ = form_tag import_csv_namespace_project_issues_path, multipart: true do
.modal-header
%h3
= _('Import issues')
@@ -18,8 +18,7 @@
= file_field_tag :file, accept: 'text/csv', required: true
%p.text-secondary
= _('It must have a header row and at least two columns: the first column is the issue title and the second column is the issue description. The separator is automatically detected.')
- %p.text-secondary
- = _('The maximum file size allowed is %{size}.') % {size: number_to_human_size(Gitlab::CurrentSettings.max_attachment_size.megabytes)}
+ = _('The maximum file size allowed is %{size}.') % { size: number_to_human_size(Gitlab::CurrentSettings.max_attachment_size.megabytes) }
.modal-footer
%button{ type: 'submit', class: 'btn btn-success', title: _('Import issues') }
= _('Import issues')
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 4a77e09bed9..3be3779d7b6 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -6620,9 +6620,6 @@ msgstr ""
msgid "The issue stage shows the time it takes from creating an issue to assigning the issue to a milestone, or add the issue to a list on your Issue Board. Begin creating issues to see data for this stage."
msgstr ""
-msgid "The maximum file size allowed is %{max_attachment_size}mb"
-msgstr ""
-
msgid "The maximum file size allowed is %{size}."
msgstr ""
diff --git a/spec/services/issues/import_csv_service_spec.rb b/spec/services/issues/import_csv_service_spec.rb
index f5e58bc9a9e..848dc2c2cd2 100644
--- a/spec/services/issues/import_csv_service_spec.rb
+++ b/spec/services/issues/import_csv_service_spec.rb
@@ -21,7 +21,7 @@ describe Issues::ImportCsvService do
expect_any_instance_of(Notify).to receive(:import_issues_csv_email)
expect(subject[:success]).to eq(0)
- expect(subject[:valid_file]).to eq(false)
+ expect(subject[:parse_error]).to eq(true)
end
end
@@ -32,8 +32,8 @@ describe Issues::ImportCsvService do
expect_any_instance_of(Notify).to receive(:import_issues_csv_email)
expect(subject[:success]).to eq(3)
- expect(subject[:errors]).to eq([])
- expect(subject[:valid_file]).to eq(true)
+ expect(subject[:error_lines]).to eq([])
+ expect(subject[:parse_error]).to eq(false)
end
end
@@ -44,8 +44,8 @@ describe Issues::ImportCsvService do
expect_any_instance_of(Notify).to receive(:import_issues_csv_email)
expect(subject[:success]).to eq(2)
- expect(subject[:errors]).to eq([3])
- expect(subject[:valid_file]).to eq(true)
+ expect(subject[:error_lines]).to eq([3])
+ expect(subject[:parse_error]).to eq(false)
end
end
@@ -56,8 +56,8 @@ describe Issues::ImportCsvService do
expect_any_instance_of(Notify).to receive(:import_issues_csv_email)
expect(subject[:success]).to eq(3)
- expect(subject[:errors]).to eq([4])
- expect(subject[:valid_file]).to eq(true)
+ expect(subject[:error_lines]).to eq([4])
+ expect(subject[:parse_error]).to eq(false)
end
end
end
diff --git a/spec/services/upload_service_spec.rb b/spec/services/upload_service_spec.rb
index 57382c4bc8d..4a809d5bf18 100644
--- a/spec/services/upload_service_spec.rb
+++ b/spec/services/upload_service_spec.rb
@@ -63,7 +63,7 @@ describe UploadService do
@link_to_file = upload_file(@project, txt)
end
- it { expect(@link_to_file).to eq(nil) }
+ it { expect(@link_to_file).to eq({}) }
end
end