git-svn-id: http://svn.redmine.org/redmine/branches/4.1-stable@19679 e93f8b46-1217-0410-a6f0-8f06a7374b81tags/4.1.1
@@ -577,6 +577,7 @@ class IssuesController < ApplicationController | |||
time_entry = @time_entry || TimeEntry.new | |||
time_entry.project = @issue.project | |||
time_entry.issue = @issue | |||
time_entry.author = User.current | |||
time_entry.user = User.current | |||
time_entry.spent_on = User.current.today | |||
time_entry.safe_attributes = params[:time_entry] |
@@ -28,8 +28,6 @@ class TimelogController < ApplicationController | |||
before_action :find_optional_issue, :only => [:new, :create] | |||
before_action :find_optional_project, :only => [:index, :report] | |||
before_action :authorize_logging_time_for_other_users, :only => [:create, :update] | |||
accept_rss_auth :index | |||
accept_api_auth :index, :show, :create, :update, :destroy | |||
@@ -258,13 +256,6 @@ class TimelogController < ApplicationController | |||
end | |||
end | |||
def authorize_logging_time_for_other_users | |||
if !User.current.allowed_to?(:log_time_for_other_users, @project) && params['time_entry'].present? && params['time_entry']['user_id'].present? && params['time_entry']['user_id'].to_i != User.current.id | |||
render_error :message => l(:error_not_allowed_to_log_time_for_other_users), :status => 403 | |||
return false | |||
end | |||
end | |||
def find_time_entries | |||
@time_entries = TimeEntry.where(:id => params[:id] || params[:ids]). | |||
preload(:project => :time_entry_activities). |
@@ -50,6 +50,7 @@ class TimeEntry < ActiveRecord::Base | |||
validates_length_of :comments, :maximum => 1024, :allow_nil => true | |||
validates :spent_on, :date => true | |||
before_validation :set_project_if_nil | |||
#TODO: remove this, author should be always explicitly set | |||
before_validation :set_author_if_nil | |||
validate :validate_time_entry | |||
@@ -116,6 +117,11 @@ class TimeEntry < ActiveRecord::Base | |||
@invalid_issue_id = issue_id | |||
end | |||
end | |||
if user_id_changed? && user_id != author_id && !user.allowed_to?(:log_time_for_other_users, project) | |||
@invalid_user_id = user_id | |||
else | |||
@invalid_user_id = nil | |||
end | |||
end | |||
attrs | |||
end | |||
@@ -146,7 +152,7 @@ class TimeEntry < ActiveRecord::Base | |||
end | |||
end | |||
errors.add :project_id, :invalid if project.nil? | |||
if user_id_changed? && user_id != author_id && !self.assignable_users.map(&:id).include?(user_id) | |||
if @invalid_user_id || (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 |
@@ -85,7 +85,7 @@ class TimeEntryImport < Import | |||
end | |||
user_id = nil | |||
if User.current.allowed_to?(:log_time_for_other_users, project) | |||
if user.allowed_to?(:log_time_for_other_users, project) | |||
if user_value | |||
user_id = user_value | |||
elsif user_name = row_value(row, 'user_id') | |||
@@ -98,6 +98,7 @@ class TimeEntryImport < Import | |||
attributes = { | |||
:project_id => project.id, | |||
:activity_id => activity_id, | |||
:author_id => user.id, | |||
:user_id => user_id, | |||
:issue_id => row_value(row, 'issue_id'), |
@@ -383,7 +383,7 @@ class TimelogControllerTest < Redmine::ControllerTest | |||
assert_equal 2, t.author_id | |||
end | |||
def test_create_for_other_user_should_deny_for_user_without_permission | |||
def test_create_for_other_user_should_fail_without_permission | |||
Role.find_by_name('Manager').remove_permission! :log_time_for_other_users | |||
@request.session[:user_id] = 2 | |||
@@ -399,8 +399,8 @@ class TimelogControllerTest < Redmine::ControllerTest | |||
} | |||
} | |||
assert_response 403 | |||
assert_select 'p[id=?]', 'errorExplanation', :text => I18n.t(:error_not_allowed_to_log_time_for_other_users) | |||
assert_response :success | |||
assert_select_error /User is invalid/ | |||
end | |||
def test_create_and_continue_at_project_level | |||
@@ -668,7 +668,7 @@ class TimelogControllerTest < Redmine::ControllerTest | |||
assert_select_error /Issue is invalid/ | |||
end | |||
def test_update_should_deny_changing_user_for_user_without_permission | |||
def test_update_should_fail_when_changing_user_without_permission | |||
Role.find_by_name('Manager').remove_permission! :log_time_for_other_users | |||
@request.session[:user_id] = 2 | |||
@@ -679,8 +679,8 @@ class TimelogControllerTest < Redmine::ControllerTest | |||
} | |||
} | |||
assert_response 403 | |||
assert_select 'p[id=?]', 'errorExplanation', :text => I18n.t(:error_not_allowed_to_log_time_for_other_users) | |||
assert_response :success | |||
assert_select_error /User is invalid/ | |||
end | |||
def test_update_should_allow_updating_existing_entry_logged_on_a_locked_user |
@@ -144,6 +144,40 @@ class Redmine::ApiTest::TimeEntriesTest < Redmine::ApiTest::Base | |||
assert_select 'errors error', :text => "Hours cannot be blank" | |||
end | |||
test "POST /time_entries.xml with :project_id for other user" do | |||
Role.find_by_name('Manager').add_permission! :log_time_for_other_users | |||
entry = new_record(TimeEntry) do | |||
post( | |||
'/time_entries.xml', | |||
:params => | |||
{:time_entry => | |||
{:project_id => '1', :spent_on => '2010-12-02', :user_id => '3', | |||
:hours => '3.5', :activity_id => '11'}}, | |||
:headers => credentials('jsmith')) | |||
end | |||
assert_response :created | |||
assert_equal 3, entry.user_id | |||
assert_equal 2, entry.author_id | |||
end | |||
test "POST /time_entries.xml with :issue_id for other user" do | |||
Role.find_by_name('Manager').add_permission! :log_time_for_other_users | |||
entry = new_record(TimeEntry) do | |||
post( | |||
'/time_entries.xml', | |||
:params => | |||
{:time_entry => | |||
{:issue_id => '1', :spent_on => '2010-12-02', :user_id => '3', | |||
:hours => '3.5', :activity_id => '11'}}, | |||
:headers => credentials('jsmith')) | |||
end | |||
assert_response :created | |||
assert_equal 3, entry.user_id | |||
assert_equal 2, entry.author_id | |||
end | |||
test "PUT /time_entries/:id.xml with valid parameters should update time entry" do | |||
assert_no_difference 'TimeEntry.count' do | |||
put( |
@@ -127,7 +127,8 @@ class TimeEntryImportTest < ActiveSupport::TestCase | |||
end | |||
def test_maps_user_id_for_user_with_permissions | |||
User.current = User.find(1) | |||
Role.find_by_name('Manager').add_permission! :log_time_for_other_users | |||
import = generate_import_with_mapping | |||
first, second, third, fourth = new_records(TimeEntry, 4) { import.run } | |||
@@ -138,16 +139,17 @@ class TimeEntryImportTest < ActiveSupport::TestCase | |||
end | |||
def test_maps_user_to_column_value | |||
User.current = User.find(1) | |||
Role.find_by_name('Manager').add_permission! :log_time_for_other_users | |||
import = generate_import_with_mapping | |||
import.mapping.merge!('user_id' => 'value:1') | |||
import.mapping.merge!('user_id' => 'value:3') | |||
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 | |||
assert_equal 3, first.user_id | |||
assert_equal 3, second.user_id | |||
assert_equal 3, third.user_id | |||
assert_equal 3, fourth.user_id | |||
end | |||
def test_maps_user_id_for_user_without_permissions |