Add administration setting to allow time logging on closed issues. By default, the setting is enabled (#13596).

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

10 files changed, 90 insertions, 3 deletions

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 @@
         </div>
         </fieldset>
     <% end %>
-    <% if User.current.allowed_to?(:log_time, @issue.project) %>
+    <% if @issue.time_loggable? %>
         <fieldset class="tabular" id="log_time"><legend><%= l(:button_log_time) %></legend>
         <%= labelled_fields_for :time_entry, @time_entry do |time_entry| %>
         <div class="splitcontent">
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 @@
 <p><%= setting_check_box :timelog_accept_0_hours %></p>
 
 <p><%= setting_check_box :timelog_accept_future_dates %></p>
+
+<p><%= setting_check_box :timelog_accept_closed_issues %></p>
 </div>
 
 <fieldset class="box">
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 <strong>%{name}</strong> 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)