0001-Allow-the-current-user-to-log-time-for-other-users.patch

Marius BALTEANU, 2018-06-21 23:16

Download (24.1 KB)

View differences:

app/controllers/timelog_controller.rb
26 26
  before_action :find_optional_issue, :only => [:new, :create]
27 27
  before_action :find_optional_project, :only => [:index, :report]
28 28

  
29
  before_action :authorize_logging_time_for_other_users, :only => [:create, :update]
30

  
29 31
  accept_rss_auth :index
30 32
  accept_api_auth :index, :show, :create, :update, :destroy
31 33

  
......
90 92
  end
91 93

  
92 94
  def new
93
    @time_entry ||= TimeEntry.new(:project => @project, :issue => @issue, :user => User.current, :spent_on => User.current.today)
95
    @time_entry ||= TimeEntry.new(:project => @project, :issue => @issue, :author => User.current, :spent_on => User.current.today)
94 96
    @time_entry.safe_attributes = params[:time_entry]
95 97
  end
96 98

  
97 99
  def create
98
    @time_entry ||= TimeEntry.new(:project => @project, :issue => @issue, :user => User.current, :spent_on => User.current.today)
100
    @time_entry ||= TimeEntry.new(:project => @project, :issue => @issue, :author => User.current, :user => User.current, :spent_on => User.current.today)
99 101
    @time_entry.safe_attributes = params[:time_entry]
100 102
    if @time_entry.project && !User.current.allowed_to?(:log_time, @time_entry.project)
101 103
      render_403
......
144 146

  
145 147
  def update
146 148
    @time_entry.safe_attributes = params[:time_entry]
147

  
148 149
    call_hook(:controller_timelog_edit_before_save, { :params => params, :time_entry => @time_entry })
149 150

  
150 151
    if @time_entry.save
......
243 244
    end
244 245
  end
245 246

  
247
  def authorize_logging_time_for_other_users
248
    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
249
      render_error :message => l(:error_not_allowed_to_log_time_for_other_users), :status => 403
250
      return false
251
    end
252
  end
253

  
246 254
  def find_time_entries
247 255
    @time_entries = TimeEntry.where(:id => params[:id] || params[:ids]).
248 256
      preload(:project => :time_entry_activities).
app/helpers/timelog_helper.rb
42 42
    collection
43 43
  end
44 44

  
45
  def user_collection_for_select_options(time_entry)
46
    collection = time_entry.assignable_users
47
    principals_options_for_select(collection, time_entry.user_id)
48
  end
49

  
45 50
  def select_hours(data, criteria, value)
46 51
    if value.to_s.empty?
47 52
      data.select {|row| row[criteria].blank? }
app/models/time_entry.rb
22 22
  belongs_to :project
23 23
  belongs_to :issue
24 24
  belongs_to :user
25
  belongs_to :author, :class_name => 'User'
25 26
  belongs_to :activity, :class_name => 'TimeEntryActivity'
26 27

  
27 28
  acts_as_customizable
......
39 40
                            :author_key => :user_id,
40 41
                            :scope => joins(:project).preload(:project)
41 42

  
42
  validates_presence_of :user_id, :activity_id, :project_id, :hours, :spent_on
43
  validates_presence_of :author_id, :user_id, :activity_id, :project_id, :hours, :spent_on
43 44
  validates_presence_of :issue_id, :if => lambda { Setting.timelog_required_fields.include?('issue_id') }
44 45
  validates_presence_of :comments, :if => lambda { Setting.timelog_required_fields.include?('comments') }
45 46
  validates_numericality_of :hours, :allow_nil => true, :message => :invalid
46 47
  validates_length_of :comments, :maximum => 1024, :allow_nil => true
47 48
  validates :spent_on, :date => true
48 49
  before_validation :set_project_if_nil
50
  before_validation :set_author_if_nil
49 51
  validate :validate_time_entry
50 52

  
51 53
  scope :visible, lambda {|*args|
......
60 62
    where("#{Issue.table_name}.root_id = #{issue.root_id} AND #{Issue.table_name}.lft >= #{issue.lft} AND #{Issue.table_name}.rgt <= #{issue.rgt}")
61 63
  }
62 64

  
63
  safe_attributes 'hours', 'comments', 'project_id', 'issue_id', 'activity_id', 'spent_on', 'custom_field_values', 'custom_fields'
65
  safe_attributes 'user_id', 'hours', 'comments', 'project_id', 'issue_id', 'activity_id', 'spent_on', 'custom_field_values', 'custom_fields'
64 66

  
65 67
  # Returns a SQL conditions string used to find all time entries visible by the specified user
66 68
  def self.visible_condition(user, options={})
......
119 121
    self.project = issue.project if issue && project.nil?
120 122
  end
121 123

  
124
  def set_author_if_nil
125
    self.author = User.current if author.nil?
126
  end
127

  
122 128
  def validate_time_entry
123 129
    if hours
124 130
      errors.add :hours, :invalid if hours < 0
......
134 140
      end
135 141
    end
136 142
    errors.add :project_id, :invalid if project.nil?
143
    errors.add :user_id, :invalid if (user_id != author_id && !self.assignable_users.map(&:id).include?(user_id))
137 144
    errors.add :issue_id, :invalid if (issue_id && !issue) || (issue && project!=issue.project) || @invalid_issue_id
138 145
    errors.add :activity_id, :inclusion if activity_id_changed? && project && !project.activities.include?(activity)
139 146
  end
......
177 184
    editable_custom_field_values(user).map(&:custom_field).uniq
178 185
  end
179 186

  
187
  def assignable_users
188
    users = []
189
    if project
190
      users = project.members.active.preload(:user)
191
      users = users.map(&:user).select{ |u| u.allowed_to?(:log_time, project) }
192
    end
193
    users << User.current if User.current.logged? && !users.include?(User.current)
194
    users
195
  end
196

  
180 197
  private
181 198

  
182 199
  # Returns the hours that were logged in other time entries for the same user and the same day
app/models/time_entry_query.rb
25 25
    QueryColumn.new(:spent_on, :sortable => ["#{TimeEntry.table_name}.spent_on", "#{TimeEntry.table_name}.created_on"], :default_order => 'desc', :groupable => true),
26 26
    QueryColumn.new(:created_on, :sortable => "#{TimeEntry.table_name}.created_on", :default_order => 'desc'),
27 27
    QueryColumn.new(:tweek, :sortable => ["#{TimeEntry.table_name}.spent_on", "#{TimeEntry.table_name}.created_on"], :caption => :label_week),
28
    QueryColumn.new(:author, :sortable => lambda {User.fields_for_order_statement}),
28 29
    QueryColumn.new(:user, :sortable => lambda {User.fields_for_order_statement}, :groupable => true),
29 30
    QueryColumn.new(:activity, :sortable => "#{TimeEntryActivity.table_name}.position", :groupable => true),
30 31
    QueryColumn.new(:issue, :sortable => "#{Issue.table_name}.id"),
......
75 76
      :type => :list_optional, :values => lambda { author_values }
76 77
    )
77 78

  
79
    add_available_filter("author_id",
80
      :type => :list_optional, :values => lambda { author_values }
81
    )
82

  
78 83
    activities = (project ? project.activities : TimeEntryActivity.shared)
79 84
    add_available_filter("activity_id",
80 85
      :type => :list, :values => activities.map {|a| [a.name, a.id.to_s]}
app/views/timelog/_form.html.erb
16 16
      <%=  link_to_issue(@time_entry.issue) if @time_entry.issue.try(:visible?) %>
17 17
    </span>
18 18
  </p>
19
  <% if User.current.allowed_to?(:log_time_for_other_users, @project) %>
20
    <p><%= f.select :user_id, user_collection_for_select_options(@time_entry), :required => true %></p>
21
  <% end %>
19 22
  <p><%= f.date_field :spent_on, :size => 10, :required => true %><%= calendar_for('time_entry_spent_on') %></p>
20 23
  <p><%= f.hours_field :hours, :size => 6, :required => true %></p>
21 24
  <p><%= f.text_field :comments, :size => 100, :maxlength => 1024, :required => Setting.timelog_required_fields.include?('comments') %></p>
config/locales/en.yml
222 222
  warning_fields_cleared_on_bulk_edit: "Changes will result in the automatic deletion of values from one or more fields on the selected objects"
223 223
  error_exceeds_maximum_hours_per_day: "Cannot log more than %{max_hours} hours on the same day (%{logged_hours} hours have already been logged)"
224 224
  error_can_not_delete_auth_source: "This authentication mode is in use and cannot be deleted."
225
  error_not_allowed_to_log_time_for_other_users: "Your role is not allowed to log time for other users"
225 226

  
226 227
  mail_subject_lost_password: "Your %{value} password"
227 228
  mail_body_lost_password: 'To change your password, click on the following link:'
......
534 535
  permission_manage_subtasks: Manage subtasks
535 536
  permission_manage_related_issues: Manage related issues
536 537
  permission_import_issues: Import issues
538
  permission_log_foreign_time: Log spent time for other users
537 539

  
538 540
  project_module_issue_tracking: Issue tracking
539 541
  project_module_time_tracking: Time tracking
db/migrate/20180501132547_add_author_id_to_time_entries.rb
1
class AddAuthorIdToTimeEntries < ActiveRecord::Migration[5.1]
2
  def up
3
    add_column :time_entries, :author_id, :integer, :default => nil, :after => :project_id
4
    # Copy existing user_id to author_id
5
    TimeEntry.update_all('author_id = user_id')
6
  end
7

  
8
  def down
9
    remove_column :time_entries, :author_id
10
  end
11
end
lib/redmine.rb
126 126
    map.permission :edit_time_entries, {:timelog => [:edit, :update, :destroy, :bulk_edit, :bulk_update]}, :require => :member
127 127
    map.permission :edit_own_time_entries, {:timelog => [:edit, :update, :destroy,:bulk_edit, :bulk_update]}, :require => :loggedin
128 128
    map.permission :manage_project_activities, {:projects => :settings, :project_enumerations => [:update, :destroy]}, :require => :member
129
    map.permission :log_time_for_other_users, :require => :member
129 130
  end
130 131

  
131 132
  map.project_module :news do |map|
test/fixtures/time_entries.yml
1
--- 
2
time_entries_001: 
1
---
2
time_entries_001:
3 3
  created_on: 2007-03-23 12:54:18 +01:00
4 4
  tweek: 12
5 5
  tmonth: 3
......
12 12
  id: 1
13 13
  hours: 4.25
14 14
  user_id: 2
15
  author_id: 2
15 16
  tyear: 2007
16
time_entries_002: 
17
time_entries_002:
17 18
  created_on: 2007-03-23 14:11:04 +01:00
18 19
  tweek: 11
19 20
  tmonth: 3
......
26 27
  id: 2
27 28
  hours: 150.0
28 29
  user_id: 1
30
  author_id: 1
29 31
  tyear: 2007
30
time_entries_003: 
32
time_entries_003:
31 33
  created_on: 2007-04-21 12:20:48 +02:00
32 34
  tweek: 16
33 35
  tmonth: 4
......
40 42
  id: 3
41 43
  hours: 1.0
42 44
  user_id: 1
45
  author_id: 1
43 46
  tyear: 2007
44
time_entries_004: 
47
time_entries_004:
45 48
  created_on: 2007-04-22 12:20:48 +02:00
46 49
  tweek: 16
47 50
  tmonth: 4
......
50 53
  updated_on: 2007-04-22 12:20:48 +02:00
51 54
  activity_id: 10
52 55
  spent_on: 2007-04-22
53
  issue_id: 
56
  issue_id:
54 57
  id: 4
55 58
  hours: 7.65
56 59
  user_id: 1
60
  author_id: 1
57 61
  tyear: 2007
58
time_entries_005: 
62
time_entries_005:
59 63
  created_on: 2011-03-22 12:20:48 +02:00
60 64
  tweek: 12
61 65
  tmonth: 3
......
64 68
  updated_on: 2011-03-22 12:20:48 +02:00
65 69
  activity_id: 10
66 70
  spent_on: 2011-03-22
67
  issue_id: 
71
  issue_id:
68 72
  id: 5
69 73
  hours: 7.65
70 74
  user_id: 1
75
  author_id: 1
71 76
  tyear: 2011
72
  
77

  
test/functional/project_enumerations_controller_test.rb
143 143
        :enumerations => {
144 144
          "9"=> {
145 145
            "parent_id"=>"9", "custom_field_values"=> {
146
            "7" => "1"}, "active"=>"0"} # Design, De-activate      
147
            
146
            "7" => "1"}, "active"=>"0"} # Design, De-activate
147

  
148 148
          }
149 149
      }
150 150
    assert_response :redirect
......
163 163
    # TODO: Need to cause an exception on create but these tests
164 164
    # aren't setup for mocking.  Just create a record now so the
165 165
    # second one is a dupicate
166
    user = User.find(1)
166 167
    parent = TimeEntryActivity.find(9)
167 168
    TimeEntryActivity.create!({:name => parent.name, :project_id => 1,
168 169
                               :position => parent.position, :active => true, :parent_id => 9})
169
    TimeEntry.create!({:project_id => 1, :hours => 1.0, :user => User.find(1),
170
    TimeEntry.create!({:project_id => 1, :hours => 1.0, :user => user, :author => user,
170 171
                       :issue_id => 3, :activity_id => 10, :spent_on => '2009-01-01'})
171 172
    assert_equal 3, TimeEntry.where(:activity_id => 9, :project_id => 1).count
172 173
    assert_equal 1, TimeEntry.where(:activity_id => 10, :project_id => 1).count
test/functional/timelog_controller_test.rb
102 102
    assert_select 'option', :text => 'Inactive Activity', :count => 0
103 103
  end
104 104

  
105
  def test_new_should_show_user_select_if_user_has_permission
106
    Role.find_by_name('Manager').add_permission! :log_time_for_other_users
107
    @request.session[:user_id] = 2
108

  
109
    get :new, :params => {:project_id => 1}
110
    assert_response :success
111
    assert_select 'select[name=?]', 'time_entry[user_id]' do
112
      assert_select 'option', 3
113
      assert_select 'option[value=?]', '2', 2
114
      assert_select 'option[value=?]', '3', 1
115
      # locked members should not be available
116
      assert_select 'option[value=?]', '4', 0
117
    end
118
  end
119

  
120
  def test_new_user_select_should_include_current_user_if_is_logged
121
    @request.session[:user_id] = 1
122

  
123
    get :new, :params => {:project_id => 1}
124
    assert_response :success
125
    assert_select 'select[name=?]', 'time_entry[user_id]' do
126
      assert_select 'option[value=?]', '1', :text => '<< me >>'
127
      assert_select 'option[value=?]', '1', :text => 'Redmine Admin'
128
    end
129
  end
130

  
131
  def test_new_should_not_show_user_select_if_user_does_not_have_permission
132
    @request.session[:user_id] = 2
133

  
134
    get :new, :params => {:project_id => 1}
135
    assert_response :success
136
    assert_select 'select[name=?]', 'time_entry[user_id]', 0
137
  end
138

  
105 139
  def test_post_new_as_js_should_update_activity_options
106 140
    @request.session[:user_id] = 3
107 141
    post :new, :params => {:time_entry => {:project_id => 1}, :format => 'js'}
......
268 302
    assert !response.body.include?('issue_that_is_not_visible')
269 303
  end
270 304

  
305
  def test_create_for_other_user
306
    Role.find_by_name('Manager').add_permission! :log_time_for_other_users
307
    @request.session[:user_id] = 2
308

  
309
    post :create, :params => {
310
      :project_id => 1,
311
      :time_entry => {:comments => 'Some work on TimelogControllerTest',
312
        # Not the default activity
313
        :activity_id => '11',
314
        :spent_on => '2008-03-14',
315
        :issue_id => '1',
316
        :hours => '7.3',
317
        :user_id => '3'
318
      }
319
    }
320

  
321
    assert_redirected_to '/projects/ecookbook/time_entries'
322

  
323
    t = TimeEntry.last
324
    assert_equal 3, t.user_id
325
    assert_equal 2, t.author_id
326
  end
327

  
328
  def test_create_for_other_user_should_deny_for_user_without_permission
329
    Role.find_by_name('Manager').remove_permission! :log_time_for_other_users
330
    @request.session[:user_id] = 2
331

  
332
    post :create, :params => {
333
      :project_id => 1,
334
      :time_entry => {:comments => 'Some work on TimelogControllerTest',
335
        # Not the default activity
336
        :activity_id => '11',
337
        :spent_on => '2008-03-14',
338
        :issue_id => '1',
339
        :hours => '7.3',
340
        :user_id => '3'
341
      }
342
    }
343

  
344
    assert_response 403
345
    assert_select 'p[id=?]', 'errorExplanation', :text => 'Your role is not allowed to log time for other users'
346
  end
347

  
271 348
  def test_create_and_continue_at_project_level
272 349
    @request.session[:user_id] = 2
273 350
    assert_difference 'TimeEntry.count' do
......
533 610
    assert_select_error /Issue is invalid/
534 611
  end
535 612

  
613
  def test_update_should_deny_changing_user_for_user_without_permission
614
    Role.find_by_name('Manager').remove_permission! :log_time_for_other_users
615
    @request.session[:user_id] = 2
616

  
617
    put :update, :params => {
618
      :id => 3,
619
      :time_entry => {
620
        :user_id => '3'
621
      }
622
    }
623

  
624
    assert_response 403
625
    assert_select 'p[id=?]', 'errorExplanation', :text => 'Your role is not allowed to log time for other users'
626
  end
627

  
536 628
  def test_get_bulk_edit
537 629
    @request.session[:user_id] = 2
538 630

  
......
899 991
  end
900 992

  
901 993
  def test_index_should_sort_by_spent_on_and_created_on
902
    t1 = TimeEntry.create!(:user => User.find(1), :project => Project.find(1), :hours => 1, :spent_on => '2012-06-16', :created_on => '2012-06-16 20:00:00', :activity_id => 10)
903
    t2 = TimeEntry.create!(:user => User.find(1), :project => Project.find(1), :hours => 1, :spent_on => '2012-06-16', :created_on => '2012-06-16 20:05:00', :activity_id => 10)
904
    t3 = TimeEntry.create!(:user => User.find(1), :project => Project.find(1), :hours => 1, :spent_on => '2012-06-15', :created_on => '2012-06-16 20:10:00', :activity_id => 10)
994
    t1 = TimeEntry.create!(:author => User.find(1), :user => User.find(1), :project => Project.find(1), :hours => 1, :spent_on => '2012-06-16', :created_on => '2012-06-16 20:00:00', :activity_id => 10)
995
    t2 = TimeEntry.create!(:author => User.find(1), :user => User.find(1), :project => Project.find(1), :hours => 1, :spent_on => '2012-06-16', :created_on => '2012-06-16 20:05:00', :activity_id => 10)
996
    t3 = TimeEntry.create!(:author => User.find(1), :user => User.find(1), :project => Project.find(1), :hours => 1, :spent_on => '2012-06-15', :created_on => '2012-06-16 20:10:00', :activity_id => 10)
905 997

  
906 998
    get :index, :params => {
907 999
      :project_id => 1,
......
1046 1138
    assert_select 'td.issue-category', :text => 'Printing'
1047 1139
  end
1048 1140

  
1141
  def test_index_with_author_filter
1142
    get :index, :params => {
1143
      :project_id => 'ecookbook',
1144
      :f => ['author_id'],
1145
      :op => {'author_id' => '='},
1146
      :v => {'author_id' => ['2']}
1147
    }
1148
    assert_response :success
1149
    assert_equal ['1'], css_select('input[name="ids[]"]').map {|e| e.attr('value')}
1150
  end
1151

  
1152
  def test_index_with_author_column
1153
    get :index, :params => {
1154
      :project_id => 'ecookbook',
1155
      :c => %w(project spent_on issue comments hours author)
1156
    }
1157

  
1158
    assert_response :success
1159
    assert_select 'td.author', :text => 'Redmine Admin'
1160
  end
1161

  
1049 1162
  def test_index_with_issue_category_sort
1050 1163
    issue = Issue.find(3)
1051 1164
    issue.category_id = 2
test/object_helpers.rb
142 142
  def TimeEntry.generate(attributes={})
143 143
    entry = TimeEntry.new(attributes)
144 144
    entry.user ||= User.find(2)
145
    entry.author ||= entry.user
145 146
    entry.issue ||= Issue.find(1) unless entry.project
146 147
    entry.project ||= entry.issue.project
147 148
    entry.activity ||= TimeEntryActivity.first
test/unit/lib/redmine/export/pdf/issues_pdf_test.rb
27 27
    query = IssueQuery.new(:project => Project.find(1), :name => '_')
28 28
    query.column_names = [:subject, :spent_hours]
29 29
    issue = Issue.find(2)
30
    TimeEntry.create(:spent_on => Date.today, :hours => 4.3432, :user => User.find(1),
30
    user = User.find(1)
31
    time_entry = TimeEntry.create!(:spent_on => Date.today, :hours => 4.3432, :user => user, :author => user,
31 32
                     :project_id => 1, :issue => issue, :activity => TimeEntryActivity.first)
33

  
32 34
    results = fetch_row_values(issue, query, 0)
33 35
    assert_equal ["2", "Add ingredients categories", "4.34"], results
34 36
  end
test/unit/time_entry_test.rb
168 168
                          :issue    => issue,
169 169
                          :project  => project,
170 170
                          :user     => anon,
171
                          :author     => anon,
171 172
                          :activity => activity)
172 173
    assert_equal 1, te.errors.count
173 174
  end
......
206 207
  def test_create_with_required_issue_id_and_comment_should_be_validated
207 208
    set_language_if_valid 'en'
208 209
    with_settings :timelog_required_fields => ['issue_id' , 'comments'] do
209
      entry = TimeEntry.new(:project => Project.find(1), :spent_on => Date.today, :user => User.find(1), :activity => TimeEntryActivity.first, :hours => 1)
210
      entry = TimeEntry.new(:project => Project.find(1), :spent_on => Date.today, :author => User.find(1), :user => User.find(1), :activity => TimeEntryActivity.first, :hours => 1)
210 211

  
211 212
      assert !entry.save
212 213
      assert_equal ["Comment cannot be blank", "Issue cannot be blank"], entry.errors.full_messages.sort
213 214
    end
214 215
  end
216

  
217
  def test_create_should_validate_user_id
218
    entry = TimeEntry.new(:spent_on => '2010-01-01',
219
                          :hours    => 10,
220
                          :project_id => 1,
221
                          :user_id    => 4)
222

  
223
    assert !entry.save
224
    assert_equal ["User is invalid"], entry.errors.full_messages.sort
225
  end
226

  
227
  def test_assignable_users_should_include_active_project_members_with_log_time_permission
228
    Role.find(2).remove_permission! :log_time
229
    time_entry = TimeEntry.find(1)
230

  
231
    assert_equal [2], time_entry.assignable_users.map(&:id)
232
  end
215 233
end
216
-