From 78f8e388374b4ca7f8c1fb81c935e3e14acd5d67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marius=20B=C4=82LTEANU?= Date: Sat, 11 Apr 2026 19:09:57 +0300 Subject: [PATCH] Update time entry activities when moving issues or time entries to another project (#43904). --- app/models/issue.rb | 9 ++++-- app/models/time_entry.rb | 7 +++++ app/models/time_entry_activity.rb | 12 ++++++++ ...pdate_time_entries_activity_for_project.rb | 21 ++++++++++++++ test/functional/timelog_controller_test.rb | 13 +++++++++ test/unit/issue_test.rb | 28 ++++++++++++++++++ test/unit/time_entry_activity_test.rb | 29 +++++++++++++++++++ test/unit/time_entry_test.rb | 23 +++++++++++++++ 8 files changed, 140 insertions(+), 2 deletions(-) create mode 100644 db/migrate/20261013120000_update_time_entries_activity_for_project.rb diff --git a/app/models/issue.rb b/app/models/issue.rb index 0a7cc5ed3..4a57423ea 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -1752,8 +1752,13 @@ class Issue < ApplicationRecord end def after_project_change - # Update project_id on related time entries - TimeEntry.where({:issue_id => id}).update_all(["project_id = ?", project_id]) + # Update project_id and activity_id on related time entries + TimeEntry.where(:issue_id => id).distinct.pluck(:activity_id).each do |activity_id| + TimeEntry.where(:issue_id => id, :activity_id => activity_id).update_all( + :project_id => project_id, + :activity_id => TimeEntryActivity.activity_id_for_project(activity_id, project) + ) + end # Delete issue relations unless Setting.cross_project_issue_relations? diff --git a/app/models/time_entry.rb b/app/models/time_entry.rb index 4c997d81e..cbf52467d 100644 --- a/app/models/time_entry.rb +++ b/app/models/time_entry.rb @@ -57,6 +57,7 @@ class TimeEntry < ApplicationRecord before_validation :set_project_if_nil # TODO: remove this, author should be always explicitly set before_validation :set_author_if_nil + before_validation :set_activity_for_project, :if => lambda {persisted? && project_id_changed? && !activity_id_changed?} validate :validate_time_entry scope :visible, (lambda do |*args| @@ -168,6 +169,12 @@ class TimeEntry < ApplicationRecord self.author = User.current if author.nil? end + # Replaces the activity with the matching one of the new project + # when the time entry is moved to another project + def set_activity_for_project + self.activity_id = TimeEntryActivity.activity_id_for_project(activity_id, project) + end + def validate_time_entry if hours errors.add :hours, :invalid if hours < 0 diff --git a/app/models/time_entry_activity.rb b/app/models/time_entry_activity.rb index d279359fb..58dd062c2 100644 --- a/app/models/time_entry_activity.rb +++ b/app/models/time_entry_activity.rb @@ -90,4 +90,16 @@ class TimeEntryActivity < Enumeration nil end + + # Returns the id of the activity to use in the given project for the given + # activity: the project override of the shared activity if any, the shared + # activity otherwise + def self.activity_id_for_project(activity_id, project) + activity = find_by(:id => activity_id) + return activity_id if activity.nil? + + shared_activity_id = activity.parent_id || activity.id + override_id = project.time_entry_activities.where(:parent_id => shared_activity_id).pick(:id) if project + override_id || shared_activity_id + end end diff --git a/db/migrate/20261013120000_update_time_entries_activity_for_project.rb b/db/migrate/20261013120000_update_time_entries_activity_for_project.rb new file mode 100644 index 000000000..a8da85197 --- /dev/null +++ b/db/migrate/20261013120000_update_time_entries_activity_for_project.rb @@ -0,0 +1,21 @@ +class UpdateTimeEntriesActivityForProject < ActiveRecord::Migration[8.1] + def up + overrides = TimeEntryActivity.where.not(project_id: nil).where.not(parent_id: nil).pluck(:id, :project_id, :parent_id) + return if overrides.empty? + + shared_activity_ids = overrides.to_h {|id, _project_id, parent_id| [id, parent_id]} + override_ids = overrides.to_h {|id, project_id, parent_id| [[project_id, parent_id], id]} + + TimeEntry.distinct.pluck(:project_id, :activity_id).each do |project_id, activity_id| + shared_activity_id = shared_activity_ids[activity_id] || activity_id + new_activity_id = override_ids[[project_id, shared_activity_id]] || shared_activity_id + next if new_activity_id == activity_id + + TimeEntry.where(project_id: project_id, activity_id: activity_id).update_all(activity_id: new_activity_id) + end + end + + def down + # no-op + end +end diff --git a/test/functional/timelog_controller_test.rb b/test/functional/timelog_controller_test.rb index 8ceaf6533..875adeb2b 100644 --- a/test/functional/timelog_controller_test.rb +++ b/test/functional/timelog_controller_test.rb @@ -823,6 +823,19 @@ class TimelogControllerTest < Redmine::ControllerTest assert_equal [9, 9, 9], TimeEntry.where(:id => [1, 2, 4]).collect {|i| i.activity_id} end + def test_bulk_update_to_another_project_should_update_activity + override = TimeEntryActivity.create!(:name => 'Design', :parent_id => 9, :project_id => 3) + entry = TimeEntry.generate!(:project_id => 1, :issue_id => nil, :activity_id => 9) + @request.session[:user_id] = 1 + + post :bulk_update, :params => {:ids => [entry.id], :time_entry => {:project_id => 3, :activity_id => ''}} + + assert_response :found + entry.reload + assert_equal 3, entry.project_id + assert_equal override.id, entry.activity_id + end + def test_bulk_update_on_different_projects_without_rights @request.session[:user_id] = 3 user = User.find(3) diff --git a/test/unit/issue_test.rb b/test/unit/issue_test.rb index 852686473..8b7eaf75a 100644 --- a/test/unit/issue_test.rb +++ b/test/unit/issue_test.rb @@ -2097,6 +2097,34 @@ class IssueTest < ActiveSupport::TestCase parent.errors[:base]) end + def test_move_to_another_project_should_update_time_entries_activity + override = TimeEntryActivity.create!(:name => 'Design', :parent_id => 9, :project_id => 2) + issue = Issue.find(1) + assert_equal [9, 9], issue.time_entries.order(:id).pluck(:activity_id) + + issue.project = Project.find(2) + assert issue.save + assert_equal [[2, override.id]] * 2, issue.time_entries.order(:id).pluck(:project_id, :activity_id) + + issue.project = Project.find(1) + assert issue.save + assert_equal [[1, 9]] * 2, issue.time_entries.order(:id).pluck(:project_id, :activity_id) + end + + def test_move_to_another_project_should_update_subtasks_time_entries_activity + override = TimeEntryActivity.create!(:name => 'Design', :parent_id => 9, :project_id => 2) + parent = Issue.generate!(:project_id => 1) + child = Issue.generate!(:project_id => 1, :parent_issue_id => parent.id) + time_entry = TimeEntry.generate!(:issue => child, :activity_id => 9) + + parent.reload + parent.project = Project.find(2) + assert parent.save + time_entry.reload + assert_equal 2, time_entry.project_id + assert_equal override.id, time_entry.activity_id + end + def test_copy_to_the_same_project issue = Issue.find(1) copy = issue.copy diff --git a/test/unit/time_entry_activity_test.rb b/test/unit/time_entry_activity_test.rb index 3d367d9b6..9505fcae7 100644 --- a/test/unit/time_entry_activity_test.rb +++ b/test/unit/time_entry_activity_test.rb @@ -303,4 +303,33 @@ class TimeEntryActivityTest < ActiveSupport::TestCase [TimeEntryActivity.find(10), TimeEntryActivity.find(11)].each {|a| a.update(active: false)} assert_equal 9, TimeEntryActivity.default_activity_id(User.find(2), project) end + + def test_activity_id_for_project_should_return_project_override_of_shared_activity + override = TimeEntryActivity.create!(:name => 'Design', :parent_id => 9, :project_id => 2) + + assert_equal override.id, TimeEntryActivity.activity_id_for_project(9, Project.find(2)) + end + + def test_activity_id_for_project_should_return_shared_activity_without_project_override + assert_equal 9, TimeEntryActivity.activity_id_for_project(9, Project.find(2)) + end + + def test_activity_id_for_project_should_map_override_to_override_of_other_project + override1 = TimeEntryActivity.create!(:name => 'Design', :parent_id => 9, :project_id => 1) + override2 = TimeEntryActivity.create!(:name => 'Design', :parent_id => 9, :project_id => 2) + + assert_equal override2.id, TimeEntryActivity.activity_id_for_project(override1.id, Project.find(2)) + end + + def test_activity_id_for_project_should_map_override_to_shared_activity_without_project_override + override = TimeEntryActivity.create!(:name => 'Design', :parent_id => 9, :project_id => 1) + + assert_equal 9, TimeEntryActivity.activity_id_for_project(override.id, Project.find(2)) + assert_equal 9, TimeEntryActivity.activity_id_for_project(override.id, nil) + end + + def test_activity_id_for_project_should_return_given_id_for_unknown_activity + assert_nil TimeEntryActivity.activity_id_for_project(nil, Project.find(2)) + assert_equal 999, TimeEntryActivity.activity_id_for_project(999, Project.find(2)) + end end diff --git a/test/unit/time_entry_test.rb b/test/unit/time_entry_test.rb index e6c39ed04..8fac677e2 100644 --- a/test/unit/time_entry_test.rb +++ b/test/unit/time_entry_test.rb @@ -270,6 +270,29 @@ class TimeEntryTest < ActiveSupport::TestCase assert_include I18n.translate('activerecord.errors.messages.inclusion'), entry.errors[:activity_id] end + def test_move_to_another_project_should_update_activity + override = TimeEntryActivity.create!(:name => 'Design', :parent_id => 9, :project_id => 2) + entry = TimeEntry.generate!(:project_id => 1, :activity_id => 9) + + entry.project_id = 2 + assert entry.save + assert_equal override.id, entry.reload.activity_id + + entry.project_id = 1 + assert entry.save + assert_equal 9, entry.reload.activity_id + end + + def test_move_to_another_project_should_keep_activity_explicitly_set + TimeEntryActivity.create!(:name => 'Design', :parent_id => 9, :project_id => 2) + entry = TimeEntry.generate!(:project_id => 1, :activity_id => 9) + + entry.project_id = 2 + entry.activity_id = 10 + assert entry.save + assert_equal 10, entry.reload.activity_id + end + def test_spent_on_with_2_digits_year_should_not_be_valid entry = TimeEntry.new(:project => Project.find(1), :user => User.find(1), :activity => TimeEntryActivity.first, :hours => 1) entry.spent_on = "09-02-04" -- 2.54.0 (Apple Git-157)