diff --git a/app/controllers/context_menus_controller.rb b/app/controllers/context_menus_controller.rb index 4c7305873..1e37f623b 100644 --- a/app/controllers/context_menus_controller.rb +++ b/app/controllers/context_menus_controller.rb @@ -33,7 +33,7 @@ class ContextMenusController < ApplicationController @can = { :edit => @issues.all?(&:attributes_editable?), - :log_time => (@project && User.current.allowed_to?(:log_time, @project)), + :log_time => @issue&.time_loggable?, :copy => User.current.allowed_to?(:copy_issues, @projects) && Issue.allowed_target_projects.any?, :add_watchers => User.current.allowed_to?(:add_issue_watchers, @projects), :delete => @issues.all?(&:deletable?), diff --git a/app/models/issue.rb b/app/models/issue.rb index 980bd56f0..7b261cf67 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -188,6 +188,11 @@ class Issue < ApplicationRecord end end + # Returns true if user or current user is allowed to log time on the issue + def time_loggable?(user=User.current) + user.allowed_to?(:log_time, project) && (!Setting.timelog_accept_future_dates? || !closed?) + end + # Returns true if user or current user is allowed to edit or add notes to the issue def editable?(user=User.current) attributes_editable?(user) || notes_addable?(user) diff --git a/app/models/time_entry.rb b/app/models/time_entry.rb index abbbe4441..b97d47b5d 100644 --- a/app/models/time_entry.rb +++ b/app/models/time_entry.rb @@ -182,6 +182,9 @@ class TimeEntry < ApplicationRecord if spent_on && spent_on_changed? && user errors.add :base, I18n.t(:error_spent_on_future_date) if !Setting.timelog_accept_future_dates? && (spent_on > user.today) end + if !Setting.timelog_accept_closed_issues? && issue&.closed? && issue&.was_closed? + errors.add :base, I18n.t(:error_spent_on_closed_issue) + end end def hours=(h) diff --git a/app/views/issues/_action_menu.html.erb b/app/views/issues/_action_menu.html.erb index 6009df89e..8b5d3021d 100644 --- a/app/views/issues/_action_menu.html.erb +++ b/app/views/issues/_action_menu.html.erb @@ -3,7 +3,7 @@ :onclick => 'showAndScrollTo("update", "issue_notes"); return false;', :class => 'icon icon-edit ', :accesskey => accesskey(:edit) if @issue.editable? %> <%= link_to sprite_icon('time-add', l(:button_log_time)), new_issue_time_entry_path(@issue), - :class => 'icon icon-time-add ' if User.current.allowed_to?(:log_time, @project) %> + :class => 'icon icon-time-add ' if @issue.time_loggable? %> <%= watcher_link(@issue, User.current) %> <%= link_to sprite_icon('copy', l(:button_copy)), project_copy_issue_path(@project, @issue), :class => 'icon icon-copy ' if User.current.allowed_to?(:copy_issues, @project) && Issue.allowed_target_projects.any? %> diff --git a/app/views/issues/_edit.html.erb b/app/views/issues/_edit.html.erb index f00b65750..5692eb80b 100644 --- a/app/views/issues/_edit.html.erb +++ b/app/views/issues/_edit.html.erb @@ -9,7 +9,7 @@ <% end %> - <% if User.current.allowed_to?(:log_time, @issue.project) %> + <% if @issue.time_loggable? %>
<%= l(:button_log_time) %> <%= labelled_fields_for :time_entry, @time_entry do |time_entry| %>
diff --git a/app/views/settings/_timelog.html.erb b/app/views/settings/_timelog.html.erb index c6efd966d..0e860ffd1 100644 --- a/app/views/settings/_timelog.html.erb +++ b/app/views/settings/_timelog.html.erb @@ -9,6 +9,8 @@

<%= setting_check_box :timelog_accept_0_hours %>

<%= setting_check_box :timelog_accept_future_dates %>

+ +

<%= setting_check_box :timelog_accept_closed_issues %>

diff --git a/config/locales/en.yml b/config/locales/en.yml index aac5afe6b..6fd9a8e45 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -235,6 +235,7 @@ en: error_exceeds_maximum_hours_per_day: "Cannot log more than %{max_hours} hours on the same day (%{logged_hours} hours have already been logged)" error_can_not_delete_auth_source: "This authentication mode is in use and cannot be deleted." error_spent_on_future_date: "Cannot log time on a future date" + error_spent_on_closed_issue: "Cannot log time on a closed issue" error_not_allowed_to_log_time_for_other_users: "You are not allowed to log time for other users" error_can_not_execute_macro_html: "Error executing the %{name} macro (%{error})" error_macro_does_not_accept_block: "This macro does not accept a block of text" @@ -519,6 +520,7 @@ en: setting_timelog_accept_0_hours: Accept time logs with 0 hours setting_timelog_max_hours_per_day: Maximum hours that can be logged per day and user setting_timelog_accept_future_dates: Accept time logs on future dates + setting_timelog_accept_closed_issues: Accept time logs on closed issues setting_show_status_changes_in_mail_subject: Show status changes in issue mail notifications subject setting_project_list_defaults: Projects list defaults setting_twofa: Two-factor authentication diff --git a/config/settings.yml b/config/settings.yml index 48da91a10..0aa8f0a44 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -347,6 +347,8 @@ timelog_max_hours_per_day: default: 999 timelog_accept_future_dates: default: 1 +timelog_accept_closed_issues: + default: 1 show_status_changes_in_mail_subject: default: 1 wiki_tablesort_enabled: diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb index 5b2ac0d8c..976dd0e11 100644 --- a/test/functional/issues_controller_test.rb +++ b/test/functional/issues_controller_test.rb @@ -5917,6 +5917,16 @@ class IssuesControllerTest < Redmine::ControllerTest assert_select 'input[name=?]', 'time_entry[hours]', 0 end + def test_get_edit_should_not_display_the_time_entry_form_on_closed_issue + with_settings :timelog_accept_closed_issues => '0' do + @request.session[:user_id] = 2 + issue = Issue.find(1) + issue.update :status => IssueStatus.find(5) + get(:edit, :params => {:id => 1}) + assert_select 'input[name=?]', 'time_entry[hours]', 0 + end + end + def test_get_edit_with_params @request.session[:user_id] = 2 get( @@ -6373,6 +6383,57 @@ class IssuesControllerTest < Redmine::ControllerTest assert mail.subject.include?("(#{IssueStatus.find(2).name})") end + def test_update_should_accept_time_entry_when_closing_issue + with_settings :timelog_accept_closed_issues => '0' do + issue = Issue.find(1) + assert_equal 1, issue.status_id + @request.session[:user_id] = 2 + assert_difference('TimeEntry.count', 1) do + put( + :update, + :params => { + :id => 1, + :issue => { + :status_id => 5, + }, + :time_entry => { + :hours => '2', + :comments => '', + :activity_id => TimeEntryActivity.first + } + } + ) + end + assert_redirected_to :action => 'show', :id => '1' + issue.reload + assert issue.closed? + end + end + + def test_update_should_not_accept_time_entry_on_closed_issue + with_settings :timelog_accept_closed_issues => '0' do + issue = Issue.find(1) + issue.update :status => IssueStatus.find(5) + @request.session[:user_id] = 2 + assert_no_difference('TimeEntry.count') do + put( + :update, + :params => { + :id => 1, + :issue => { + }, + :time_entry => { + :hours => '2', + :comments => '', + :activity_id => TimeEntryActivity.first + } + } + ) + end + assert_response :success + end + end + def test_put_update_with_note_only notes = 'Note added by IssuesControllerTest#test_update_with_note_only' diff --git a/test/unit/time_entry_test.rb b/test/unit/time_entry_test.rb index 6d04619e8..19b1ba2a4 100644 --- a/test/unit/time_entry_test.rb +++ b/test/unit/time_entry_test.rb @@ -175,6 +175,18 @@ class TimeEntryTest < ActiveSupport::TestCase end end + def test_should_not_accept_closed_issue + with_settings :timelog_accept_closed_issues => '0' do + project = Project.find(1) + entry = TimeEntry.generate project: project + issue = project.issues.to_a.detect(&:closed?) + entry.issue = issue + assert !entry.save + assert entry.errors[:base].present? + assert_equal 'Cannot log time on a closed issue', entry.errors[:base].first + end + end + def test_should_require_spent_on with_settings :timelog_accept_future_dates => '0' do entry = TimeEntry.find(1)