Skip to content

O
OHR Support
Commit 37d401ac authored by Eric Davis's avatar Eric Davis
Browse files
Options

When a specific TimeEntryActivity are change, associated TimeEntries will be 

re-assigned to the correct record.

* Renamed build to create since the methods now create objects.
* Removed call to Project#save that isn't needed since objects are saved directly now.
* Wrapped the activity creation and updates in a SQL transaction so TimeEntries
  will remain in a consistent state if there is an error.
* When a Project's TimeEntryActivities are destroyed, they are now
  reassigned to the parent TimeEntryActivity.

  #4077

git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@2950 e93f8b46-1217-0410-a6f0-8f06a7374b81
parent 5833ba9f
Hide whitespace changes
Inline Side-by-side
Showing with 86 additions and 13 deletions
app/controllers/projects_controller.rb
View file @ 37d401ac
......@@ -234,10 +234,11 @@ class ProjectsController < ApplicationController
def save_activities
if request.post? && params[:enumerations]
params[:enumerations].each do |id, activity|
@project.update_or_build_time_entry_activity(id, activity)
Project.transaction do
params[:enumerations].each do |id, activity|
@project.update_or_create_time_entry_activity(id, activity)
end
end
@project.save
end
redirect_to :controller => 'projects', :action => 'settings', :tab => 'activities', :id => @project
......@@ -245,7 +246,7 @@ class ProjectsController < ApplicationController
def reset_activities
@project.time_entry_activities.each do |time_entry_activity|
time_entry_activity.destroy
time_entry_activity.destroy(time_entry_activity.parent)
end
redirect_to :controller => 'projects', :action => 'settings', :tab => 'activities', :id => @project
end
......
app/models/project.rb
View file @ 37d401ac
......@@ -170,19 +170,24 @@ class Project < ActiveRecord::Base
end
end
# Will build a new Project specific Activity or update an existing one
def update_or_build_time_entry_activity(id, activity_hash)
# Will create a new Project specific Activity or update an existing one
#
# This will raise a ActiveRecord::Rollback if the TimeEntryActivity
# does not successfully save.
def update_or_create_time_entry_activity(id, activity_hash)
if activity_hash.respond_to?(:has_key?) && activity_hash.has_key?('parent_id')
self.build_time_entry_activity_if_needed(activity_hash)
self.create_time_entry_activity_if_needed(activity_hash)
else
activity = project.time_entry_activities.find_by_id(id.to_i)
activity.update_attributes(activity_hash) if activity
end
end
# Builds new activity
def build_time_entry_activity_if_needed(activity)
# Only new override activities are built
# Create a new TimeEntryActivity if it overrides a system TimeEntryActivity
#
# This will raise a ActiveRecord::Rollback if the TimeEntryActivity
# does not successfully save.
def create_time_entry_activity_if_needed(activity)
if activity['parent_id']
parent_activity = TimeEntryActivity.find(activity['parent_id'])
......@@ -190,7 +195,13 @@ class Project < ActiveRecord::Base
activity['position'] = parent_activity.position
if Enumeration.overridding_change?(activity, parent_activity)
self.time_entry_activities.build(activity)
project_activity = self.time_entry_activities.create(activity)
if project_activity.new_record?
raise ActiveRecord::Rollback, "Overridding TimeEntryActivity was not successfully saved"
else
self.time_entries.update_all("activity_id = #{project_activity.id}", ["activity_id = ?", parent_activity.id])
end
end
end
end
......
test/fixtures/time_entries.yml
View file @ 37d401ac
......@@ -55,4 +55,4 @@ time_entries_004:
hours: 7.65
user_id: 1
tyear: 2007
\ No newline at end of file
test/functional/projects_controller_test.rb
View file @ 37d401ac
......@@ -24,7 +24,7 @@ class ProjectsController; def rescue_action(e) raise e end; end
class ProjectsControllerTest < ActionController::TestCase
fixtures :projects, :versions, :users, :roles, :members, :member_roles, :issues, :journals, :journal_details,
:trackers, :projects_trackers, :issue_statuses, :enabled_modules, :enumerations, :boards, :messages,
:attachments, :custom_fields, :custom_values
:attachments, :custom_fields, :custom_values, :time_entries
def setup
@controller = ProjectsController.new
......@@ -586,6 +586,27 @@ class ProjectsControllerTest < ActionController::TestCase
assert_nil TimeEntryActivity.find_by_id(project_activity_two.id)
end
def test_reset_activities_should_reassign_time_entries_back_to_the_system_activity
@request.session[:user_id] = 2 # manager
project_activity = TimeEntryActivity.new({
:name => 'Project Specific Design',
:parent => TimeEntryActivity.find(9),
:project => Project.find(1),
:active => true
})
assert project_activity.save
assert TimeEntry.update_all("activity_id = '#{project_activity.id}'", ["project_id = ? AND activity_id = ?", 1, 9])
assert 3, TimeEntry.find_all_by_activity_id_and_project_id(project_activity.id, 1).size
delete :reset_activities, :id => 1
assert_response :redirect
assert_redirected_to 'projects/ecookbook/settings/activities'
assert_nil TimeEntryActivity.find_by_id(project_activity.id)
assert_equal 0, TimeEntry.find_all_by_activity_id_and_project_id(project_activity.id, 1).size, "TimeEntries still assigned to project specific activity"
assert_equal 3, TimeEntry.find_all_by_activity_id_and_project_id(9, 1).size, "TimeEntries still assigned to project specific activity"
end
def test_save_activities_routing
assert_routing({:method => :post, :path => 'projects/64/activities/save'},
:controller => 'projects', :action => 'save_activities', :id => '64')
......@@ -683,6 +704,46 @@ class ProjectsControllerTest < ActionController::TestCase
assert !activity_two.active?
end
def test_save_activities_when_creating_new_activities_will_convert_existing_data
assert_equal 3, TimeEntry.find_all_by_activity_id_and_project_id(9, 1).size
@request.session[:user_id] = 2 # manager
post :save_activities, :id => 1, :enumerations => {
"9"=> {"parent_id"=>"9", "custom_field_values"=>{"7" => "1"}, "active"=>"0"} # Design, De-activate
}
assert_response :redirect
# No more TimeEntries using the system activity
assert_equal 0, TimeEntry.find_all_by_activity_id_and_project_id(9, 1).size, "Time Entries still assigned to system activities"
# All TimeEntries using project activity
project_specific_activity = TimeEntryActivity.find_by_parent_id_and_project_id(9, 1)
assert_equal 3, TimeEntry.find_all_by_activity_id_and_project_id(project_specific_activity.id, 1).size, "No Time Entries assigned to the project activity"
end
def test_save_activities_when_creating_new_activities_will_not_convert_existing_data_if_an_exception_is_raised
# TODO: Need to cause an exception on create but these tests
# aren't setup for mocking. Just create a record now so the
# second one is a dupicate
parent = TimeEntryActivity.find(9)
TimeEntryActivity.create!({:name => parent.name, :project_id => 1, :position => parent.position, :active => true})
TimeEntry.create!({:project_id => 1, :hours => 1.0, :user => User.find(1), :issue_id => 3, :activity_id => 10, :spent_on => '2009-01-01'})
assert_equal 3, TimeEntry.find_all_by_activity_id_and_project_id(9, 1).size
assert_equal 1, TimeEntry.find_all_by_activity_id_and_project_id(10, 1).size
@request.session[:user_id] = 2 # manager
post :save_activities, :id => 1, :enumerations => {
"9"=> {"parent_id"=>"9", "custom_field_values"=>{"7" => "1"}, "active"=>"0"}, # Design
"10"=> {"parent_id"=>"10", "custom_field_values"=>{"7"=>"0"}, "active"=>"1"} # Development, Change custom value
}
assert_response :redirect
# TimeEntries shouldn't have been reassigned on the failed record
assert_equal 3, TimeEntry.find_all_by_activity_id_and_project_id(9, 1).size, "Time Entries are not assigned to system activities"
# TimeEntries shouldn't have been reassigned on the saved record either
assert_equal 1, TimeEntry.find_all_by_activity_id_and_project_id(10, 1).size, "Time Entries are not assigned to system activities"
end
# A hook that is manually registered later
class ProjectBasedTemplate < Redmine::Hook::ViewListener
def view_layouts_base_html_head(context)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment