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

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 725bde788a6745811c8ecd1458852f97ffdc0de0..32bb528a31217db30595a5d8c17fd8c42eeaa261 100644 (file)
--- 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 d63e2364168024676754a29005fdc8d380037929..0b15887fa391cb05cc085128136bb2057b470b41 100644 (file)
--- 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 5909ac64ccc007cde3e38485265c61d862e99cc3..726e6240cbe37016b1dd0c5c77ccd8bc930b5307 100644 (file)
--- 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 %>\r
+<% form_tag(project_project_enumerations_path(@project), :method => :put, :class => "tabular") do %>\r
 \r
 <table class="list">\r
   <thead><tr>\r
@@ -32,7 +32,7 @@
 </table>\r
 \r
 <div class="contextual">\r
-<%= link_to(l(:button_reset), {:controller => 'project_enumerations', :action => 'destroy', :id => @project},\r
+<%= link_to(l(:button_reset), project_project_enumerations_path(@project),\r
                                                :method => :delete,\r
                                                :confirm => l(:text_are_you_sure),\r
             :class => 'icon icon-del') %>\r

diff --git a/config/routes.rb b/config/routes.rb
index 9f12cd454166c042ab68b20687faf058106bcd51..129c3b4560292d632298e4c581d9234b670adb3b 100644 (file)
--- 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 534f8587597fe815a9bd7232c44f7085c47cb476..526b83e65bb2d58c04a134f0f30269b4a49c84c0 100644 (file)
--- 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 c03be04fb7d42702217a1261cc60c703567550ad..942399a96cb1862f40f790c060f7623adf67274a 100644 (file)
--- 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 5cd0b2d391f78f9ce7083db6e5a07b8a0200fce3..ac75d4d7cf057f4b72b06c723049c786903d6469 100644 (file)
--- 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