diff --git a/app/helpers/imports_helper.rb b/app/helpers/imports_helper.rb index ea5a6e7ee..315973fcb 100644 --- a/app/helpers/imports_helper.rb +++ b/app/helpers/imports_helper.rb @@ -33,7 +33,7 @@ module ImportsHelper tags << options_for_select(import.columns_options, import.mapping[field]) if values = options[:values] tags << content_tag('option', '--', :disabled => true) - tags << options_for_select(values.map {|text, value| [text, "value:#{value}"]}, import.mapping[field]) + tags << options_for_select(values.map {|text, value| [text, "value:#{value}"]}, import.mapping[field] || options[:default_value]) end tags end diff --git a/app/models/time_entry_import.rb b/app/models/time_entry_import.rb index 66b762f74..49c6de375 100644 --- a/app/models/time_entry_import.rb +++ b/app/models/time_entry_import.rb @@ -43,6 +43,16 @@ class TimeEntryImport < Import project.activities end + def allowed_target_users + users = [] + if project + users = project.members.active.preload(:user) + users = users.map(&:user).select{ |u| u.allowed_to?(:log_time, project) } + end + users << User.current if User.current.logged? && !users.include?(User.current) + users + end + def project project_id = mapping['project_id'].to_i allowed_target_projects.find_by_id(project_id) || allowed_target_projects.first @@ -55,11 +65,17 @@ class TimeEntryImport < Import end end + def user_value + if mapping['user_id'].to_s =~ /\Avalue:(\d+)\z/ + $1.to_i + end + end + private def build_object(row, item) object = TimeEntry.new - object.user = user + object.author = user activity_id = nil if activity @@ -68,9 +84,17 @@ class TimeEntryImport < Import activity_id = allowed_target_activities.named(activity_name).first.try(:id) end + user_id = nil + if User.current.allowed_to?(:log_time_for_other_users, project) + user_id = user_value || row_value(row, 'user_id') + else + user_id = user.id + end + attributes = { :project_id => project.id, :activity_id => activity_id, + :user_id => user_id, :issue_id => row_value(row, 'issue_id'), :spent_on => row_date(row, 'spent_on'), diff --git a/app/views/imports/_time_entries_fields_mapping.html.erb b/app/views/imports/_time_entries_fields_mapping.html.erb index f480fae68..1b4f7a0e4 100644 --- a/app/views/imports/_time_entries_fields_mapping.html.erb +++ b/app/views/imports/_time_entries_fields_mapping.html.erb @@ -10,9 +10,15 @@ :values => @import.allowed_target_activities.sorted.map {|t| [t.name, t.id]} %>
-+ + <%= mapping_select_tag @import, 'user_id', :required => true, + :values => @import.allowed_target_users.map {|u| [u.name, u.id]}, :default_value => "value:#{User.current.id}" %> +
+<% end %>
<%= mapping_select_tag @import, 'issue_id' %>
diff --git a/app/views/imports/_time_entries_saved_objects.html.erb b/app/views/imports/_time_entries_saved_objects.html.erb
index 0579536d7..002401546 100644
--- a/app/views/imports/_time_entries_saved_objects.html.erb
+++ b/app/views/imports/_time_entries_saved_objects.html.erb
@@ -2,6 +2,7 @@
<%= t(:field_project) %>
+ <%= t(:field_user) %>
<%= t(:field_activity) %>
<%= t(:field_issue) %>
<%= t(:field_spent_on) %>
@@ -13,6 +14,7 @@
<% saved_objects.each do |time_entry| %>
<%= link_to_project(time_entry.project, :jump => 'time_entries') if time_entry.project %>
+ <%= link_to_user time_entry.user %>
<%= time_entry.activity.name if time_entry.activity %>
<%= link_to_issue time_entry.issue if time_entry.issue %>
<%= format_date(time_entry.spent_on) %>
diff --git a/test/fixtures/files/import_time_entries.csv b/test/fixtures/files/import_time_entries.csv
index 1ff8075ef..0628e5203 100644
--- a/test/fixtures/files/import_time_entries.csv
+++ b/test/fixtures/files/import_time_entries.csv
@@ -1,5 +1,5 @@
-row;issue_id;date;hours;comment;activity;overtime
-1;;2020-01-01;1;Some Design;Design;yes
-2;;2020-01-02;2;Some Development;Development;yes
-3;1;2020-01-03;3;Some QA;QA;no
-4;2;2020-01-04;4;Some Inactivity;Inactive Activity;no
+row;issue_id;date;hours;comment;activity;overtime;user_id
+1;;2020-01-01;1;Some Design;Design;yes;2
+2;;2020-01-02;2;Some Development;Development;yes;2
+3;1;2020-01-03;3;Some QA;QA;no;3
+4;2;2020-01-04;4;Some Inactivity;Inactive Activity;no;2
diff --git a/test/functional/imports_controller_test.rb b/test/functional/imports_controller_test.rb
index ed9024306..77bc6723f 100644
--- a/test/functional/imports_controller_test.rb
+++ b/test/functional/imports_controller_test.rb
@@ -187,6 +187,41 @@ class ImportsControllerTest < Redmine::ControllerTest
assert_equal '0', mapping['subject']
end
+ def test_get_mapping_time_entry
+ Role.find(1).add_permission! :log_time_for_other_users
+ import = generate_time_entry_import
+ import.settings = {'separator' => ";", 'wrapper' => '"', 'encoding' => "ISO-8859-1"}
+ import.save!
+
+ get :mapping, :params => {
+ :id => import.to_param
+ }
+
+ assert_response :success
+
+ # 'user_id' field should be available because User#2 has both
+ # 'import_time_entries' and 'log_time_for_other_users' permissions
+ assert_select 'select[name=?]', 'import_settings[mapping][user_id]' do
+ # Current user should be the default value
+ assert_select 'option[value="value:2"][selected]', :text => User.find(2).name
+ assert_select 'option[value="value:3"]', :text => User.find(3).name
+ end
+ end
+
+ def test_get_mapping_time_entry_for_user_without_log_time_for_other_users_permission
+ import = generate_time_entry_import
+ import.settings = {'separator' => ";", 'wrapper' => '"', 'encoding' => "ISO-8859-1"}
+ import.save!
+
+ get :mapping, :params => {
+ :id => import.to_param
+ }
+
+ assert_response :success
+
+ assert_select 'select[name=?]', 'import_settings[mapping][user_id]', 0
+ end
+
def test_get_run
import = generate_import_with_mapping
diff --git a/test/object_helpers.rb b/test/object_helpers.rb
index 4f1f4b486..e89e1bd05 100644
--- a/test/object_helpers.rb
+++ b/test/object_helpers.rb
@@ -259,6 +259,14 @@ module ObjectHelpers
import.save!
import
end
+
+ def generate_time_entry_import(fixture_name='import_time_entries.csv')
+ import = TimeEntryImport.new
+ import.user_id = 2
+ import.file = uploaded_test_file(fixture_name, 'text/csv')
+ import.save!
+ import
+ end
end
module TrackerObjectHelpers
diff --git a/test/unit/time_entry_import_test.rb b/test/unit/time_entry_import_test.rb
index cc732f0d7..d84de22f4 100644
--- a/test/unit/time_entry_import_test.rb
+++ b/test/unit/time_entry_import_test.rb
@@ -36,6 +36,7 @@ class TimeEntryImportTest < ActiveSupport::TestCase
def setup
set_language_if_valid 'en'
+ User.current = nil
end
def test_authorized
@@ -125,6 +126,43 @@ class TimeEntryImportTest < ActiveSupport::TestCase
assert_equal '0', fourth.custom_field_value(overtime_cf)
end
+ def test_maps_user_id_for_user_with_permissions
+ User.current = User.find(1)
+ import = generate_import_with_mapping
+ first, second, third, fourth = new_records(TimeEntry, 4) { import.run }
+
+ assert_equal 2, first.user_id
+ assert_equal 2, second.user_id
+ assert_equal 3, third.user_id
+ assert_equal 2, fourth.user_id
+ end
+
+ def test_maps_user_to_column_value
+ User.current = User.find(1)
+ import = generate_import_with_mapping
+ import.mapping.merge!('user_id' => 'value:1')
+ import.save!
+ first, second, third, fourth = new_records(TimeEntry, 4) { import.run }
+
+ assert_equal 1, first.user_id
+ assert_equal 1, second.user_id
+ assert_equal 1, third.user_id
+ assert_equal 1, fourth.user_id
+ end
+
+ def test_maps_user_id_for_user_without_permissions
+ # User 2 doesn't have log_time_for_other_users permission
+ User.current = User.find(2)
+ import = generate_import_with_mapping
+ first, second, third, fourth = new_records(TimeEntry, 4) { import.run }
+
+ assert_equal 2, first.user_id
+ assert_equal 2, second.user_id
+ # user_id value from CSV should be ignored
+ assert_equal 2, third.user_id
+ assert_equal 2, fourth.user_id
+ end
+
protected
def generate_import(fixture_name='import_time_entries.csv')
@@ -146,7 +184,8 @@ class TimeEntryImportTest < ActiveSupport::TestCase
'issue_id' => '1',
'spent_on' => '2',
'hours' => '3',
- 'comments' => '4'
+ 'comments' => '4',
+ 'user_id' => '7'
}
}
import.save!