diff options
| -rw-r--r-- | app/controllers/projects_controller.rb | 9 | ||||
| -rw-r--r-- | app/models/project.rb | 25 | ||||
| -rw-r--r-- | test/fixtures/time_entries.yml | 2 | ||||
| -rw-r--r-- | test/functional/projects_controller_test.rb | 63 |
4 files changed, 86 insertions, 13 deletions
diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index e36d9bfdf..32326d45d 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -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 diff --git a/app/models/project.rb b/app/models/project.rb index db8c6711e..668238fc8 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -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 diff --git a/test/fixtures/time_entries.yml b/test/fixtures/time_entries.yml index a44c0938f..f56b05aa5 100644 --- a/test/fixtures/time_entries.yml +++ b/test/fixtures/time_entries.yml @@ -55,4 +55,4 @@ time_entries_004: hours: 7.65 user_id: 1 tyear: 2007 -
\ No newline at end of file + diff --git a/test/functional/projects_controller_test.rb b/test/functional/projects_controller_test.rb index 9736ce056..dcbcb77eb 100644 --- a/test/functional/projects_controller_test.rb +++ b/test/functional/projects_controller_test.rb @@ -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) |