From: Jean-Philippe Lang <jp_lang@yahoo.fr>
Date: Sun, 5 Apr 2020 09:47:00 +0000 (+0000)
Subject: Merged r19651 and r19652 to 4.1-stable (#32973).
X-Git-Tag: 4.1.1~10
X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=4b0ac7a78b31a409f51814303d6b40ecf24801a3;p=redmine.git

Merged r19651 and r19652 to 4.1-stable (#32973).

git-svn-id: http://svn.redmine.org/redmine/branches/4.1-stable@19657 e93f8b46-1217-0410-a6f0-8f06a7374b81
---

diff --git a/app/helpers/timelog_helper.rb b/app/helpers/timelog_helper.rb
index e0cc51d27..f8f36fbdd 100644
--- a/app/helpers/timelog_helper.rb
+++ b/app/helpers/timelog_helper.rb
@@ -44,6 +44,7 @@ module TimelogHelper
 
   def user_collection_for_select_options(time_entry)
     collection = time_entry.assignable_users
+    collection << time_entry.user unless time_entry.user.nil? && !collection.include?(time_entry.user)
     principals_options_for_select(collection, time_entry.user_id.to_s)
   end
 
diff --git a/app/models/time_entry.rb b/app/models/time_entry.rb
index 38359294d..943613a82 100644
--- a/app/models/time_entry.rb
+++ b/app/models/time_entry.rb
@@ -146,7 +146,9 @@ class TimeEntry < ActiveRecord::Base
       end
     end
     errors.add :project_id, :invalid if project.nil?
-    errors.add :user_id, :invalid if user_id != author_id && !self.assignable_users.map(&:id).include?(user_id)
+    if user_id_changed? && user_id != author_id && !self.assignable_users.map(&:id).include?(user_id)
+      errors.add :user_id, :invalid
+    end
     errors.add :issue_id, :invalid if (issue_id && !issue) || (issue && project!=issue.project) || @invalid_issue_id
     errors.add :activity_id, :inclusion if activity_id_changed? && project && !project.activities.include?(activity)
     if spent_on_changed? && user
diff --git a/test/functional/timelog_controller_test.rb b/test/functional/timelog_controller_test.rb
index 700362f18..8b701432b 100644
--- a/test/functional/timelog_controller_test.rb
+++ b/test/functional/timelog_controller_test.rb
@@ -186,6 +186,43 @@ class TimelogControllerTest < Redmine::ControllerTest
     assert_select 'a[href=?]', '/projects/ecookbook/time_entries', {:text => 'Cancel'}
   end
 
+  def test_get_edit_with_an_existing_time_entry_with_locked_user
+    user = User.find(3)
+    entry = TimeEntry.generate!(:user_id => user.id, :comments => "Time entry on a future locked user")
+    entry.save!
+
+    user.status = User::STATUS_LOCKED
+    user.save!
+    Role.find_by_name('Manager').add_permission! :log_time_for_other_users
+    @request.session[:user_id] = 2
+
+    get :edit, :params => {
+      :id => entry.id
+    }
+
+    assert_response :success
+
+    assert_select 'select[name=?]', 'time_entry[user_id]' do
+      # User with id 3 should be selected even if it's locked
+      assert_select 'option[value="3"][selected=selected]'
+    end
+  end
+
+  def test_get_edit_for_other_user
+    Role.find_by_name('Manager').add_permission! :log_time_for_other_users
+    @request.session[:user_id] = 2
+
+    get :edit, :params => {
+      :id => 1
+    }
+
+    assert_response :success
+
+    assert_select 'select[name=?]', 'time_entry[user_id]' do
+      assert_select 'option[value="2"][selected=selected]'
+    end
+  end
+
   def test_post_create
     @request.session[:user_id] = 3
     assert_difference 'TimeEntry.count' do
@@ -639,6 +676,26 @@ class TimelogControllerTest < Redmine::ControllerTest
     assert_select 'p[id=?]', 'errorExplanation', :text => I18n.t(:error_not_allowed_to_log_time_for_other_users)
   end
 
+  def test_update_should_allow_updating_existing_entry_logged_on_a_locked_user
+    entry = TimeEntry.generate!(:user_id => 2, :hours => 4, :comments => "Time entry on a future locked user")
+    Role.find_by_name('Manager').add_permission! :log_time_for_other_users
+    @request.session[:user_id] = 2
+
+    put :update, :params => {
+      :id => entry.id,
+      :time_entry => {
+        :hours => '6'
+      }
+    }
+
+    assert_response :redirect
+
+    entry.reload
+    # Ensure user didn't change
+    assert_equal 2, entry.user_id
+    assert_equal 6.0, entry.hours
+  end
+
   def test_get_bulk_edit
     @request.session[:user_id] = 2
 
@@ -1459,19 +1516,4 @@ class TimelogControllerTest < Redmine::ControllerTest
     assert_response :success
     assert_select "td.issue_cf_#{field.id}", :text => 'This is a long text'
   end
-
-  def test_edit_for_other_user
-    Role.find_by_name('Manager').add_permission! :log_time_for_other_users
-    @request.session[:user_id] = 2
-
-    get :edit, :params => {
-      :id => 1
-    }
-
-    assert_response :success
-
-    assert_select 'select[name=?]', 'time_entry[user_id]' do
-      assert_select 'option[value="2"][selected=selected]'
-    end
-  end
 end