From 3a2efb47572d198c91a09b1bf92e717b843980ed Mon Sep 17 00:00:00 2001 From: Eric Davis Date: Fri, 10 Sep 2010 16:00:49 +0000 Subject: [PATCH] Refactor: convert ProjectEnumerations to a resource on a project. git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@4075 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/controllers/application_controller.rb | 7 ++++++ .../project_enumerations_controller.rb | 6 ++--- app/views/projects/settings/_activities.rhtml | 4 ++-- config/routes.rb | 10 +++------ lib/redmine.rb | 2 +- .../project_enumerations_controller_test.rb | 22 +++++++++---------- test/integration/routing_test.rb | 4 ++-- 7 files changed, 29 insertions(+), 26 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 725bde788..32bb528a3 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -169,6 +169,13 @@ class ApplicationController < ActionController::Base render_404 end + # Find project of id params[:project_id] + def find_project_by_project_id + @project = Project.find(params[:project_id]) + rescue ActiveRecord::RecordNotFound + render_404 + end + # Find a project based on params[:project_id] # TODO: some subclasses override this, see about merging their logic def find_optional_project diff --git a/app/controllers/project_enumerations_controller.rb b/app/controllers/project_enumerations_controller.rb index d63e23641..0b15887fa 100644 --- a/app/controllers/project_enumerations_controller.rb +++ b/app/controllers/project_enumerations_controller.rb @@ -1,9 +1,9 @@ class ProjectEnumerationsController < ApplicationController - before_filter :find_project + before_filter :find_project_by_project_id before_filter :authorize - def save - if request.post? && params[:enumerations] + def update + if request.put? && params[:enumerations] Project.transaction do params[:enumerations].each do |id, activity| @project.update_or_create_time_entry_activity(id, activity) diff --git a/app/views/projects/settings/_activities.rhtml b/app/views/projects/settings/_activities.rhtml index 5909ac64c..726e6240c 100644 --- a/app/views/projects/settings/_activities.rhtml +++ b/app/views/projects/settings/_activities.rhtml @@ -1,4 +1,4 @@ -<% form_tag({:controller => 'project_enumerations', :action => 'save', :id => @project}, :class => "tabular") do %> +<% form_tag(project_project_enumerations_path(@project), :method => :put, :class => "tabular") do %> @@ -32,7 +32,7 @@
-<%= link_to(l(:button_reset), {:controller => 'project_enumerations', :action => 'destroy', :id => @project}, +<%= link_to(l(:button_reset), project_project_enumerations_path(@project), :method => :delete, :confirm => l(:text_are_you_sure), :class => 'icon icon-del') %> diff --git a/config/routes.rb b/config/routes.rb index 9f12cd454..129c3b456 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -179,7 +179,9 @@ ActionController::Routing::Routes.draw do |map| :modules => :post, :archive => :post, :unarchive => :post - } + } do |project| + project.resource :project_enumerations, :as => 'enumerations', :only => [:update, :destroy] + end # Destroy uses a get request to prompt the user before the actual DELETE request map.project_destroy_confirm 'projects/:id/destroy', :controller => 'projects', :action => 'destroy', :conditions => {:method => :get} @@ -195,13 +197,7 @@ ActionController::Routing::Routes.draw do |map| project_mapper.with_options :conditions => {:method => :post} do |project_actions| project_actions.connect 'projects/:id/files/new', :controller => 'files', :action => 'new' - project_actions.connect 'projects/:id/activities/save', :controller => 'project_enumerations', :action => 'save' - end - - project_mapper.with_options :conditions => {:method => :delete} do |project_actions| - project_actions.conditions 'projects/:id/reset_activities', :controller => 'project_enumerations', :action => 'destroy' end - end map.with_options :controller => 'activities', :action => 'index', :conditions => {:method => :get} do |activity| diff --git a/lib/redmine.rb b/lib/redmine.rb index 534f85875..526b83e65 100644 --- a/lib/redmine.rb +++ b/lib/redmine.rb @@ -87,7 +87,7 @@ Redmine::AccessControl.map do |map| map.permission :view_time_entries, :timelog => [:details, :report] map.permission :edit_time_entries, {:timelog => [:edit, :destroy]}, :require => :member map.permission :edit_own_time_entries, {:timelog => [:edit, :destroy]}, :require => :loggedin - map.permission :manage_project_activities, {:project_enumerations => [:save, :destroy]}, :require => :member + map.permission :manage_project_activities, {:project_enumerations => [:update, :destroy]}, :require => :member end map.project_module :news do |map| diff --git a/test/functional/project_enumerations_controller_test.rb b/test/functional/project_enumerations_controller_test.rb index c03be04fb..942399a96 100644 --- a/test/functional/project_enumerations_controller_test.rb +++ b/test/functional/project_enumerations_controller_test.rb @@ -8,11 +8,11 @@ class ProjectEnumerationsControllerTest < ActionController::TestCase Setting.default_language = 'en' end - def test_save_to_override_system_activities + def test_update_to_override_system_activities @request.session[:user_id] = 2 # manager billable_field = TimeEntryActivityCustomField.find_by_name("Billable") - post :save, :id => 1, :enumerations => { + put :update, :project_id => 1, :enumerations => { "9"=> {"parent_id"=>"9", "custom_field_values"=>{"7" => "1"}, "active"=>"0"}, # Design, De-activate "10"=> {"parent_id"=>"10", "custom_field_values"=>{"7"=>"0"}, "active"=>"1"}, # Development, Change custom value "14"=>{"parent_id"=>"14", "custom_field_values"=>{"7"=>"1"}, "active"=>"1"}, # Inactive Activity, Activate with custom value @@ -58,7 +58,7 @@ class ProjectEnumerationsControllerTest < ActionController::TestCase assert_equal nil, project.time_entry_activities.find_by_name("QA"), "Custom QA activity created when it wasn't modified" end - def test_save_will_update_project_specific_activities + def test_update_will_update_project_specific_activities @request.session[:user_id] = 2 # manager project_activity = TimeEntryActivity.new({ @@ -77,7 +77,7 @@ class ProjectEnumerationsControllerTest < ActionController::TestCase assert project_activity_two.save - post :save, :id => 1, :enumerations => { + put :update, :project_id => 1, :enumerations => { project_activity.id => {"custom_field_values"=>{"7" => "1"}, "active"=>"0"}, # De-activate project_activity_two.id => {"custom_field_values"=>{"7" => "1"}, "active"=>"0"} # De-activate } @@ -100,11 +100,11 @@ class ProjectEnumerationsControllerTest < ActionController::TestCase assert !activity_two.active? end - def test_save_when_creating_new_activities_will_convert_existing_data + def test_update_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, :id => 1, :enumerations => { + put :update, :project_id => 1, :enumerations => { "9"=> {"parent_id"=>"9", "custom_field_values"=>{"7" => "1"}, "active"=>"0"} # Design, De-activate } assert_response :redirect @@ -116,7 +116,7 @@ class ProjectEnumerationsControllerTest < ActionController::TestCase 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_when_creating_new_activities_will_not_convert_existing_data_if_an_exception_is_raised + def test_update_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 @@ -128,7 +128,7 @@ class ProjectEnumerationsControllerTest < ActionController::TestCase assert_equal 1, TimeEntry.find_all_by_activity_id_and_project_id(10, 1).size @request.session[:user_id] = 2 # manager - post :save, :id => 1, :enumerations => { + put :update, :project_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 } @@ -140,7 +140,7 @@ class ProjectEnumerationsControllerTest < ActionController::TestCase assert_equal 1, TimeEntry.find_all_by_activity_id_and_project_id(10, 1).size, "Time Entries are not assigned to system activities" end - def test_destroy + def test_destroy @request.session[:user_id] = 2 # manager project_activity = TimeEntryActivity.new({ :name => 'Project Specific', @@ -157,7 +157,7 @@ class ProjectEnumerationsControllerTest < ActionController::TestCase }) assert project_activity_two.save - delete :destroy, :id => 1 + delete :destroy, :project_id => 1 assert_response :redirect assert_redirected_to 'projects/ecookbook/settings/activities' @@ -177,7 +177,7 @@ class ProjectEnumerationsControllerTest < ActionController::TestCase 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 :destroy, :id => 1 + delete :destroy, :project_id => 1 assert_response :redirect assert_redirected_to 'projects/ecookbook/settings/activities' diff --git a/test/integration/routing_test.rb b/test/integration/routing_test.rb index 5cd0b2d39..ac75d4d7c 100644 --- a/test/integration/routing_test.rb +++ b/test/integration/routing_test.rb @@ -182,14 +182,14 @@ class RoutingTest < ActionController::IntegrationTest should_route :post, "/projects/33/files/new", :controller => 'files', :action => 'new', :id => '33' should_route :post, "/projects/64/archive", :controller => 'projects', :action => 'archive', :id => '64' should_route :post, "/projects/64/unarchive", :controller => 'projects', :action => 'unarchive', :id => '64' - should_route :post, "/projects/64/activities/save", :controller => 'project_enumerations', :action => 'save', :id => '64' + should_route :put, "/projects/64/enumerations", :controller => 'project_enumerations', :action => 'update', :project_id => '64' should_route :put, "/projects/4223", :controller => 'projects', :action => 'update', :id => '4223' should_route :put, "/projects/1.xml", :controller => 'projects', :action => 'update', :id => '1', :format => 'xml' should_route :delete, "/projects/64", :controller => 'projects', :action => 'destroy', :id => '64' should_route :delete, "/projects/1.xml", :controller => 'projects', :action => 'destroy', :id => '1', :format => 'xml' - should_route :delete, "/projects/64/reset_activities", :controller => 'project_enumerations', :action => 'destroy', :id => '64' + should_route :delete, "/projects/64/enumerations", :controller => 'project_enumerations', :action => 'destroy', :project_id => '64' end context "repositories" do -- 2.39.5