Add default spent time activity per role (#29286).

Patch by Marius BALTEANU and Mizuki ISHIKAWA.


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

app/helpers/timelog_helper.rb

@@ -48,6 +48,14 @@ module TimelogHelper
     principals_options_for_select(collection, time_entry.user_id.to_s)
   end
 
+  def default_activity(time_entry)
+    if @project
+      time_entry.activity_id
+    else
+      TimeEntryActivity.default_activity_id(User.current, time_entry.project)
+    end
+  end
+
   def select_hours(data, criteria, value)
     if value.to_s.empty?
       data.select {|row| row[criteria].blank?}

app/models/role.rb

@@ -60,6 +60,8 @@ class Role < ActiveRecord::Base
     where("#{compare} builtin = 0")
   end)
 
+  belongs_to :default_time_entry_activity, :class_name => 'TimeEntryActivity'
+
   before_destroy :check_deletable
   has_many :workflow_rules, :dependent => :delete_all
   has_and_belongs_to_many :custom_fields, :join_table => "#{table_name_prefix}custom_fields_roles#{table_name_suffix}", :foreign_key => "role_id"
@@ -104,7 +106,8 @@ class Role < ActiveRecord::Base
     'managed_role_ids',
     'permissions',
     'permissions_all_trackers',
-    'permissions_tracker_ids'
+    'permissions_tracker_ids',
+    'default_time_entry_activity_id'
   )
 
   # Copies attributes from another role, arg can be an id or a Role

app/models/time_entry.rb

@@ -107,11 +107,7 @@ class TimeEntry < ActiveRecord::Base
   def initialize(attributes=nil, *args)
     super
     if new_record? && self.activity.nil?
-      if default_activity = TimeEntryActivity.default(self.project)
-        self.activity_id = default_activity.id
-      elsif (activities = TimeEntryActivity.available_activities(self.project)) && activities.count == 1
-        self.activity_id = activities.first.id
-      end
+      self.activity_id = TimeEntryActivity.default_activity_id(User.current, self.project)
       self.hours = nil if hours == 0
     end
   end

app/models/time_entry_activity.rb

@@ -32,6 +32,7 @@ class TimeEntryActivity < Enumeration
     project.activities.detect { |activity| activity.parent_id == default_activity.id }
   end
 
+  # Returns the available activities for the time entry
   def self.available_activities(project=nil)
     if project.nil?
       TimeEntryActivity.shared.active
@@ -55,4 +56,38 @@ class TimeEntryActivity < Enumeration
   def transfer_relations(to)
     objects.update_all(:activity_id => to.id)
   end
+
+  def self.default_activity_id(user=nil, project=nil)
+    default_activities = []
+    default_activity = nil
+    available_activities = self.available_activities(project)
+
+    if project && user
+      user_membership = user.membership(project)
+      if user_membership
+        default_activities = user_membership.roles.where.not(:default_time_entry_activity_id => nil).sort.pluck(:default_time_entry_activity_id)
+      end
+
+      project_default_activity = self.default(project)
+      if project_default_activity && !default_activities.include?(project_default_activity.id)
+        default_activities << project_default_activity.id
+      end
+    end
+
+    global_activity = self.default
+    if global_activity && !default_activities.include?(global_activity.id)
+      default_activities << global_activity.id
+    end
+
+    if available_activities.count == 1 && !default_activities.include?(available_activities.first.id)
+      default_activities << available_activities.first.id
+    end
+
+    default_activities.each do |id|
+      default_activity = available_activities.detect{ |a| a.id == id || a.parent_id == id }
+      break unless default_activity.nil?
+    end
+
+    default_activity&.id
+  end
 end

app/views/roles/_form.html.erb

@@ -39,6 +39,10 @@
     </p>
   <% end %>
 
+  <% unless @role.anonymous? %>
+    <p><%= f.select :default_time_entry_activity_id, options_from_collection_for_select(TimeEntryActivity.active.shared, :id, :name, @role.default_time_entry_activity_id), :include_blank => l(:label_none) %></p>
+  <% end %>
+
   <% if @role.new_record? && @roles.any? %>
     <p><label for="copy_workflow_from"><%= l(:label_copy_workflow_from) %></label>
     <%= select_tag(:copy_workflow_from, content_tag("option") + options_from_collection_for_select(@roles, :id, :name, params[:copy_workflow_from] || @copy_from.try(:id))) %></p>

app/views/timelog/new.js.erb

@@ -1,2 +1,2 @@
-$('#time_entry_activity_id').html('<%= escape_javascript options_for_select(activity_collection_for_select_options(@time_entry), @time_entry.activity_id) %>');
+$('#time_entry_activity_id').html('<%= escape_javascript options_for_select(activity_collection_for_select_options(@time_entry), default_activity(@time_entry)) %>');
 $('#time_entry_issue').html('<%= escape_javascript link_to_issue(@time_entry.issue) if @time_entry.issue.try(:visible?) %>');

config/locales/en.yml

@@ -416,6 +416,7 @@ en:
   field_twofa_required: Require two factor authentication
   field_default_issue_query: Default issue query
   field_default_project_query: Default project query
+  field_default_time_entry_activity: Default spent time activity
 
   setting_app_title: Application title
   setting_welcome_text: Welcome text

db/migrate/20220806215628_add_default_time_entry_activity_to_roles.rb

@@ -0,0 +1,5 @@
+class AddDefaultTimeEntryActivityToRoles < ActiveRecord::Migration[6.1]
+  def change
+    add_column :roles, :default_time_entry_activity_id, :int
+  end
+end

test/functional/timelog_controller_test.rb

@@ -89,7 +89,20 @@ class TimelogControllerTest < Redmine::ControllerTest
     assert_response 403
   end
 
-  def test_new_should_select_default_activity
+  def test_new_should_select_default_role_activity
+    developer = Role.find(2)
+    developer.default_time_entry_activity_id = 9
+    developer.save!
+
+    @request.session[:user_id] = 3
+    get :new, :params => {:project_id => 1}
+    assert_response :success
+    assert_select 'select[name=?]', 'time_entry[activity_id]' do
+      assert_select 'option[selected=selected]', :text => 'Design'
+    end
+  end
+
+  def test_new_should_select_default_global_activity_for_user_roles_without_default_activities
     @request.session[:user_id] = 3
     get :new, :params => {:project_id => 1}
     assert_response :success

test/unit/time_entry_activity_test.rb

@@ -231,4 +231,86 @@ class TimeEntryActivityTest < ActiveSupport::TestCase
     assert_not_equal TimeEntryActivity.default(project).id, 10
     assert_equal TimeEntryActivity.default(project).id, project_specific_default_activity.id
   end
+
+  def test_default_activity_id_without_user_and_project_should_return_global_default_activity
+    assert_equal 10, TimeEntryActivity.default_activity_id
+  end
+
+  def test_default_activity_id_with_user_and_project_should_return_role_default_activity
+    # set a default activity for Manager role
+    manager = Role.find(1)
+    manager.default_time_entry_activity_id = 9
+    manager.save
+
+    assert_equal 9, TimeEntryActivity.default_activity_id(User.find(2), Project.find(1))
+  end
+
+  def test_default_activity_id_with_user_and_project_should_consider_role_position
+    project = Project.find(1)
+    user = User.find(2)
+
+    # set a default activity for Manager role
+    manager = Role.find(1)
+    manager.default_time_entry_activity_id = 9
+    manager.save!
+
+    # set a default activity for Developer role
+    # and set the role position first
+    developer = Role.find(2)
+    developer.default_time_entry_activity_id = 11
+    developer.position = 1
+    developer.save!
+
+    member = Member.find_or_initialize_by(:project_id => project.id, :user_id => user.id)
+    member.role_ids = [1, 2]
+    member.save!
+
+    assert_equal 11, TimeEntryActivity.default_activity_id(user, project)
+  end
+
+  def test_default_activity_id_should_include_only_available_activities
+    # set a default activity for Manager role
+    manager = Role.find(1)
+    manager.default_time_entry_activity_id = 9
+    manager.save!
+
+    project = Project.find(1)
+
+    # disable role default activity
+    disable_activity = TimeEntryActivity.new({:name => "QA", :project => project, :parent => TimeEntryActivity.find(9), :active => false})
+    disable_activity.save!
+
+    assert_equal 10, TimeEntryActivity.default_activity_id(User.find(2), project)
+  end
+
+  def test_default_activity_id_should_selected_from_highest_priority_of_multiple_default_activity_candidates
+    project = Project.find(1)
+
+    manager = Role.find(1)
+    manager.default_time_entry_activity_id = 9
+    manager.save
+
+    # Returns the role_default_activity with the highest priority
+    assert_equal 9, TimeEntryActivity.default_activity_id(User.find(2), project)
+
+    # Returns the child activity of role_default_activity if there is an activity that has the id of role_default_activity as parent_id
+    design_project_activity = TimeEntryActivity.create!(name: 'Design', parent_id: 9, project_id: project.id, is_default: false)
+    development_project_activity = TimeEntryActivity.create!(name: 'Development', parent_id: 10, project_id: project.id, is_default: true)
+    qa_project_activity = TimeEntryActivity.create!(name: 'QA', parent_id: 11, project_id: project.id, is_default: false)
+    assert_equal design_project_activity.id, TimeEntryActivity.default_activity_id(User.find(2), project)
+
+    # Returns default project activity if role_default_activity is not present
+    manager.default_time_entry_activity_id = nil
+    manager.save
+    assert_equal development_project_activity.id, TimeEntryActivity.default_activity_id(User.find(2), project)
+
+    # Returns global default activity if role_default_activity and project activities are not present
+    [design_project_activity, development_project_activity, qa_project_activity].each {|activity| activity.destroy}
+    TimeEntryActivity.find(11).update(is_default: true)
+    assert_equal 11, TimeEntryActivity.default_activity_id(User.find(2), project)
+
+    # If there is only one activity available, it returns that activity.
+    [TimeEntryActivity.find(10), TimeEntryActivity.find(11)].each {|a| a.update(active: false)}
+    assert_equal 9, TimeEntryActivity.default_activity_id(User.find(2), project)
+  end
 end