Allow import time entries for other users (#32196).

Patch by Marius BALTEANU.

git-svn-id: http://svn.redmine.org/redmine/trunk@18890 e93f8b46-1217-0410-a6f0-8f06a7374b81

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

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'),

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]} %>
 </p>
 
-
 <div class="splitcontent">
 <div class="splitcontentleft">
+<% if User.current.allowed_to?(:log_time_for_other_users, @import.project) %>
+  <p>
+    <label for="import_mapping_user_id"><%= l(:field_user) %></label>
+    <%= 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}" %>
+  </p>
+<% end %>
 <p>
   <label for="import_mapping_issue_id"><%= l(:field_issue) %></label>
   <%= mapping_select_tag @import, 'issue_id' %>

app/views/imports/_time_entries_saved_objects.html.erb

@@ -2,6 +2,7 @@
   <thead>
   <tr>
     <th><%= t(:field_project) %></th>
+    <th><%= t(:field_user) %></th>
     <th><%= t(:field_activity) %></th>
     <th><%= t(:field_issue) %></th>
     <th><%= t(:field_spent_on) %></th>
@@ -13,6 +14,7 @@
   <% saved_objects.each do |time_entry| %>
   <tr>
     <td><%= link_to_project(time_entry.project, :jump => 'time_entries') if time_entry.project %></td>
+    <td><%= link_to_user time_entry.user %></td>
     <td><%= time_entry.activity.name if time_entry.activity %></td>
     <td><%= link_to_issue time_entry.issue if time_entry.issue %></td>
     <td><%= format_date(time_entry.spent_on) %></td>

test/fixtures/files/import_time_entries.csv

@@ -1,5 +1,5 @@
-row;issue_id;date;hours;comment;activity;overtime
+row;issue_id;date;hours;comment;activity;overtime;user_id
-1;;2020-01-01;1;Some Design;Design;yes
+1;;2020-01-01;1;Some Design;Design;yes;2
-2;;2020-01-02;2;Some Development;Development;yes
+2;;2020-01-02;2;Some Development;Development;yes;2
-3;1;2020-01-03;3;Some QA;QA;no
+3;1;2020-01-03;3;Some QA;QA;no;3
-4;2;2020-01-04;4;Some Inactivity;Inactive Activity;no
+4;2;2020-01-04;4;Some Inactivity;Inactive Activity;no;2

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
 

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

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!