Defect #43904 » 0001-Update-time-entry-activities-when-moving-issues-or-t.patch
| app/models/issue.rb | ||
|---|---|---|
| 1752 | 1752 |
end |
| 1753 | 1753 | |
| 1754 | 1754 |
def after_project_change |
| 1755 |
# Update project_id on related time entries |
|
| 1756 |
TimeEntry.where({:issue_id => id}).update_all(["project_id = ?", project_id])
|
|
| 1755 |
# Update project_id and activity_id on related time entries |
|
| 1756 |
TimeEntry.where(:issue_id => id).distinct.pluck(:activity_id).each do |activity_id| |
|
| 1757 |
TimeEntry.where(:issue_id => id, :activity_id => activity_id).update_all( |
|
| 1758 |
:project_id => project_id, |
|
| 1759 |
:activity_id => TimeEntryActivity.activity_id_for_project(activity_id, project) |
|
| 1760 |
) |
|
| 1761 |
end |
|
| 1757 | 1762 | |
| 1758 | 1763 |
# Delete issue relations |
| 1759 | 1764 |
unless Setting.cross_project_issue_relations? |
| app/models/time_entry.rb | ||
|---|---|---|
| 57 | 57 |
before_validation :set_project_if_nil |
| 58 | 58 |
# TODO: remove this, author should be always explicitly set |
| 59 | 59 |
before_validation :set_author_if_nil |
| 60 |
before_validation :set_activity_for_project, :if => lambda {persisted? && project_id_changed? && !activity_id_changed?}
|
|
| 60 | 61 |
validate :validate_time_entry |
| 61 | 62 | |
| 62 | 63 |
scope :visible, (lambda do |*args| |
| ... | ... | |
| 168 | 169 |
self.author = User.current if author.nil? |
| 169 | 170 |
end |
| 170 | 171 | |
| 172 |
# Replaces the activity with the matching one of the new project |
|
| 173 |
# when the time entry is moved to another project |
|
| 174 |
def set_activity_for_project |
|
| 175 |
self.activity_id = TimeEntryActivity.activity_id_for_project(activity_id, project) |
|
| 176 |
end |
|
| 177 | ||
| 171 | 178 |
def validate_time_entry |
| 172 | 179 |
if hours |
| 173 | 180 |
errors.add :hours, :invalid if hours < 0 |
| app/models/time_entry_activity.rb | ||
|---|---|---|
| 90 | 90 | |
| 91 | 91 |
nil |
| 92 | 92 |
end |
| 93 | ||
| 94 |
# Returns the id of the activity to use in the given project for the given |
|
| 95 |
# activity: the project override of the shared activity if any, the shared |
|
| 96 |
# activity otherwise |
|
| 97 |
def self.activity_id_for_project(activity_id, project) |
|
| 98 |
activity = find_by(:id => activity_id) |
|
| 99 |
return activity_id if activity.nil? |
|
| 100 | ||
| 101 |
shared_activity_id = activity.parent_id || activity.id |
|
| 102 |
override_id = project.time_entry_activities.where(:parent_id => shared_activity_id).pick(:id) if project |
|
| 103 |
override_id || shared_activity_id |
|
| 104 |
end |
|
| 93 | 105 |
end |
| db/migrate/20261013120000_update_time_entries_activity_for_project.rb | ||
|---|---|---|
| 1 |
class UpdateTimeEntriesActivityForProject < ActiveRecord::Migration[8.1] |
|
| 2 |
def up |
|
| 3 |
overrides = TimeEntryActivity.where.not(project_id: nil).where.not(parent_id: nil).pluck(:id, :project_id, :parent_id) |
|
| 4 |
return if overrides.empty? |
|
| 5 | ||
| 6 |
shared_activity_ids = overrides.to_h {|id, _project_id, parent_id| [id, parent_id]}
|
|
| 7 |
override_ids = overrides.to_h {|id, project_id, parent_id| [[project_id, parent_id], id]}
|
|
| 8 | ||
| 9 |
TimeEntry.distinct.pluck(:project_id, :activity_id).each do |project_id, activity_id| |
|
| 10 |
shared_activity_id = shared_activity_ids[activity_id] || activity_id |
|
| 11 |
new_activity_id = override_ids[[project_id, shared_activity_id]] || shared_activity_id |
|
| 12 |
next if new_activity_id == activity_id |
|
| 13 | ||
| 14 |
TimeEntry.where(project_id: project_id, activity_id: activity_id).update_all(activity_id: new_activity_id) |
|
| 15 |
end |
|
| 16 |
end |
|
| 17 | ||
| 18 |
def down |
|
| 19 |
# no-op |
|
| 20 |
end |
|
| 21 |
end |
|
| test/functional/timelog_controller_test.rb | ||
|---|---|---|
| 823 | 823 |
assert_equal [9, 9, 9], TimeEntry.where(:id => [1, 2, 4]).collect {|i| i.activity_id}
|
| 824 | 824 |
end |
| 825 | 825 | |
| 826 |
def test_bulk_update_to_another_project_should_update_activity |
|
| 827 |
override = TimeEntryActivity.create!(:name => 'Design', :parent_id => 9, :project_id => 3) |
|
| 828 |
entry = TimeEntry.generate!(:project_id => 1, :issue_id => nil, :activity_id => 9) |
|
| 829 |
@request.session[:user_id] = 1 |
|
| 830 | ||
| 831 |
post :bulk_update, :params => {:ids => [entry.id], :time_entry => {:project_id => 3, :activity_id => ''}}
|
|
| 832 | ||
| 833 |
assert_response :found |
|
| 834 |
entry.reload |
|
| 835 |
assert_equal 3, entry.project_id |
|
| 836 |
assert_equal override.id, entry.activity_id |
|
| 837 |
end |
|
| 838 | ||
| 826 | 839 |
def test_bulk_update_on_different_projects_without_rights |
| 827 | 840 |
@request.session[:user_id] = 3 |
| 828 | 841 |
user = User.find(3) |
| test/unit/issue_test.rb | ||
|---|---|---|
| 2097 | 2097 |
parent.errors[:base]) |
| 2098 | 2098 |
end |
| 2099 | 2099 | |
| 2100 |
def test_move_to_another_project_should_update_time_entries_activity |
|
| 2101 |
override = TimeEntryActivity.create!(:name => 'Design', :parent_id => 9, :project_id => 2) |
|
| 2102 |
issue = Issue.find(1) |
|
| 2103 |
assert_equal [9, 9], issue.time_entries.order(:id).pluck(:activity_id) |
|
| 2104 | ||
| 2105 |
issue.project = Project.find(2) |
|
| 2106 |
assert issue.save |
|
| 2107 |
assert_equal [[2, override.id]] * 2, issue.time_entries.order(:id).pluck(:project_id, :activity_id) |
|
| 2108 | ||
| 2109 |
issue.project = Project.find(1) |
|
| 2110 |
assert issue.save |
|
| 2111 |
assert_equal [[1, 9]] * 2, issue.time_entries.order(:id).pluck(:project_id, :activity_id) |
|
| 2112 |
end |
|
| 2113 | ||
| 2114 |
def test_move_to_another_project_should_update_subtasks_time_entries_activity |
|
| 2115 |
override = TimeEntryActivity.create!(:name => 'Design', :parent_id => 9, :project_id => 2) |
|
| 2116 |
parent = Issue.generate!(:project_id => 1) |
|
| 2117 |
child = Issue.generate!(:project_id => 1, :parent_issue_id => parent.id) |
|
| 2118 |
time_entry = TimeEntry.generate!(:issue => child, :activity_id => 9) |
|
| 2119 | ||
| 2120 |
parent.reload |
|
| 2121 |
parent.project = Project.find(2) |
|
| 2122 |
assert parent.save |
|
| 2123 |
time_entry.reload |
|
| 2124 |
assert_equal 2, time_entry.project_id |
|
| 2125 |
assert_equal override.id, time_entry.activity_id |
|
| 2126 |
end |
|
| 2127 | ||
| 2100 | 2128 |
def test_copy_to_the_same_project |
| 2101 | 2129 |
issue = Issue.find(1) |
| 2102 | 2130 |
copy = issue.copy |
| test/unit/time_entry_activity_test.rb | ||
|---|---|---|
| 303 | 303 |
[TimeEntryActivity.find(10), TimeEntryActivity.find(11)].each {|a| a.update(active: false)}
|
| 304 | 304 |
assert_equal 9, TimeEntryActivity.default_activity_id(User.find(2), project) |
| 305 | 305 |
end |
| 306 | ||
| 307 |
def test_activity_id_for_project_should_return_project_override_of_shared_activity |
|
| 308 |
override = TimeEntryActivity.create!(:name => 'Design', :parent_id => 9, :project_id => 2) |
|
| 309 | ||
| 310 |
assert_equal override.id, TimeEntryActivity.activity_id_for_project(9, Project.find(2)) |
|
| 311 |
end |
|
| 312 | ||
| 313 |
def test_activity_id_for_project_should_return_shared_activity_without_project_override |
|
| 314 |
assert_equal 9, TimeEntryActivity.activity_id_for_project(9, Project.find(2)) |
|
| 315 |
end |
|
| 316 | ||
| 317 |
def test_activity_id_for_project_should_map_override_to_override_of_other_project |
|
| 318 |
override1 = TimeEntryActivity.create!(:name => 'Design', :parent_id => 9, :project_id => 1) |
|
| 319 |
override2 = TimeEntryActivity.create!(:name => 'Design', :parent_id => 9, :project_id => 2) |
|
| 320 | ||
| 321 |
assert_equal override2.id, TimeEntryActivity.activity_id_for_project(override1.id, Project.find(2)) |
|
| 322 |
end |
|
| 323 | ||
| 324 |
def test_activity_id_for_project_should_map_override_to_shared_activity_without_project_override |
|
| 325 |
override = TimeEntryActivity.create!(:name => 'Design', :parent_id => 9, :project_id => 1) |
|
| 326 | ||
| 327 |
assert_equal 9, TimeEntryActivity.activity_id_for_project(override.id, Project.find(2)) |
|
| 328 |
assert_equal 9, TimeEntryActivity.activity_id_for_project(override.id, nil) |
|
| 329 |
end |
|
| 330 | ||
| 331 |
def test_activity_id_for_project_should_return_given_id_for_unknown_activity |
|
| 332 |
assert_nil TimeEntryActivity.activity_id_for_project(nil, Project.find(2)) |
|
| 333 |
assert_equal 999, TimeEntryActivity.activity_id_for_project(999, Project.find(2)) |
|
| 334 |
end |
|
| 306 | 335 |
end |
| test/unit/time_entry_test.rb | ||
|---|---|---|
| 270 | 270 |
assert_include I18n.translate('activerecord.errors.messages.inclusion'), entry.errors[:activity_id]
|
| 271 | 271 |
end |
| 272 | 272 | |
| 273 |
def test_move_to_another_project_should_update_activity |
|
| 274 |
override = TimeEntryActivity.create!(:name => 'Design', :parent_id => 9, :project_id => 2) |
|
| 275 |
entry = TimeEntry.generate!(:project_id => 1, :activity_id => 9) |
|
| 276 | ||
| 277 |
entry.project_id = 2 |
|
| 278 |
assert entry.save |
|
| 279 |
assert_equal override.id, entry.reload.activity_id |
|
| 280 | ||
| 281 |
entry.project_id = 1 |
|
| 282 |
assert entry.save |
|
| 283 |
assert_equal 9, entry.reload.activity_id |
|
| 284 |
end |
|
| 285 | ||
| 286 |
def test_move_to_another_project_should_keep_activity_explicitly_set |
|
| 287 |
TimeEntryActivity.create!(:name => 'Design', :parent_id => 9, :project_id => 2) |
|
| 288 |
entry = TimeEntry.generate!(:project_id => 1, :activity_id => 9) |
|
| 289 | ||
| 290 |
entry.project_id = 2 |
|
| 291 |
entry.activity_id = 10 |
|
| 292 |
assert entry.save |
|
| 293 |
assert_equal 10, entry.reload.activity_id |
|
| 294 |
end |
|
| 295 | ||
| 273 | 296 |
def test_spent_on_with_2_digits_year_should_not_be_valid |
| 274 | 297 |
entry = TimeEntry.new(:project => Project.find(1), :user => User.find(1), :activity => TimeEntryActivity.first, :hours => 1) |
| 275 | 298 |
entry.spent_on = "09-02-04" |