From 0471de41ff4828345ca0afa393f4b8f3bf0098d2 Mon Sep 17 00:00:00 2001
From: Jean-Philippe Lang <jp_lang@yahoo.fr>
Date: Sun, 11 Dec 2011 10:26:12 +0000
Subject: [PATCH] Resourcified enumerations.

git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@8189 e93f8b46-1217-0410-a6f0-8f06a7374b81
---
 app/controllers/enumerations_controller.rb    | 43 ++++++++++---------
 app/models/enumeration.rb                     |  4 ++
 app/views/enumerations/_form.html.erb         | 22 +++-------
 app/views/enumerations/destroy.html.erb       |  4 +-
 app/views/enumerations/edit.html.erb          |  6 +--
 app/views/enumerations/index.html.erb         | 10 ++---
 app/views/enumerations/new.html.erb           |  7 +--
 config/routes.rb                              |  7 +--
 .../enumerations_controller_test.rb           | 19 +++++---
 test/integration/routing_test.rb              |  9 ++++
 10 files changed, 71 insertions(+), 60 deletions(-)

diff --git a/app/controllers/enumerations_controller.rb b/app/controllers/enumerations_controller.rb
index c567b6ae6..f2777c3c6 100644
--- a/app/controllers/enumerations_controller.rb
+++ b/app/controllers/enumerations_controller.rb
@@ -19,28 +19,19 @@ class EnumerationsController < ApplicationController
   layout 'admin'
 
   before_filter :require_admin
+  before_filter :build_new_enumeration, :only => [:new, :create]
+  before_filter :find_enumeration, :only => [:edit, :update, :destroy]
 
   helper :custom_fields
-  include CustomFieldsHelper
 
   def index
   end
 
-  verify :method => :post, :only => [ :destroy, :create, :update ],
-         :redirect_to => { :action => :index }
-
   def new
-    begin
-      @enumeration = params[:type].constantize.new
-    rescue NameError
-      @enumeration = Enumeration.new
-    end
   end
 
   def create
-    @enumeration = Enumeration.new(params[:enumeration])
-    @enumeration.type = params[:enumeration][:type]
-    if @enumeration.save
+    if request.post? && @enumeration.save
       flash[:notice] = l(:notice_successful_create)
       redirect_to :action => 'index', :type => @enumeration.type
     else
@@ -49,13 +40,10 @@ class EnumerationsController < ApplicationController
   end
 
   def edit
-    @enumeration = Enumeration.find(params[:id])
   end
 
   def update
-    @enumeration = Enumeration.find(params[:id])
-    @enumeration.type = params[:enumeration][:type] if params[:enumeration][:type]
-    if @enumeration.update_attributes(params[:enumeration])
+    if request.put? && @enumeration.update_attributes(params[:enumeration])
       flash[:notice] = l(:notice_successful_update)
       redirect_to :action => 'index', :type => @enumeration.type
     else
@@ -63,8 +51,8 @@ class EnumerationsController < ApplicationController
     end
   end
 
+  verify :method => :delete, :only => :destroy, :render => { :nothing => true, :status => :method_not_allowed }
   def destroy
-    @enumeration = Enumeration.find(params[:id])
     if !@enumeration.in_use?
       # No associated objects
       @enumeration.destroy
@@ -77,9 +65,22 @@ class EnumerationsController < ApplicationController
         return
       end
     end
-    @enumerations = @enumeration.class.find(:all) - [@enumeration]
-  #rescue
-  #  flash[:error] = 'Unable to delete enumeration'
-  #  redirect_to :action => 'index'
+    @enumerations = @enumeration.class.all - [@enumeration]
+  end
+
+  private
+
+  def build_new_enumeration
+    class_name = params[:enumeration] && params[:enumeration][:type] || params[:type]
+    @enumeration = Enumeration.new_subclass_instance(class_name, params[:enumeration])
+    if @enumeration.nil?
+      render_404
+    end
+  end
+
+  def find_enumeration
+    @enumeration = Enumeration.find(params[:id])
+  rescue ActiveRecord::RecordNotFound
+    render_404
   end
 end
diff --git a/app/models/enumeration.rb b/app/models/enumeration.rb
index b8cbd84f2..826955cc8 100644
--- a/app/models/enumeration.rb
+++ b/app/models/enumeration.rb
@@ -16,6 +16,8 @@
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
 
 class Enumeration < ActiveRecord::Base
+  include Redmine::SubclassFactory
+
   default_scope :order => "#{Enumeration.table_name}.position ASC"
 
   belongs_to :project
@@ -27,6 +29,8 @@ class Enumeration < ActiveRecord::Base
   before_destroy :check_integrity
   before_save    :check_default
 
+  attr_protected :type
+
   validates_presence_of :name
   validates_uniqueness_of :name, :scope => [:type, :project_id]
   validates_length_of :name, :maximum => 30
diff --git a/app/views/enumerations/_form.html.erb b/app/views/enumerations/_form.html.erb
index 591209da9..8ff774d9d 100644
--- a/app/views/enumerations/_form.html.erb
+++ b/app/views/enumerations/_form.html.erb
@@ -1,19 +1,11 @@
 <%= error_messages_for 'enumeration' %>
-<div class="box">
-<!--[form:optvalue]-->
-<%= hidden_field 'enumeration', 'type'  %>
 
-<p><label for="enumeration_name"><%=l(:field_name)%></label>
-<%= text_field 'enumeration', 'name'  %></p>
+<div class="box tabular">
+  <p><%= f.text_field :name %></p>
+  <p><%= f.check_box :active %></p>
+  <p><%= f.check_box :is_default %></p>
 
-<p><label for="enumeration_active"><%=l(:field_active)%></label>
-<%= check_box 'enumeration', 'active'  %></p>
-
-<p><label for="enumeration_is_default"><%=l(:field_is_default)%></label>
-<%= check_box 'enumeration', 'is_default'  %></p>
-<!--[eoform:optvalue]-->
-
-<% @enumeration.custom_field_values.each do |value| %>
-  <p><%= custom_field_tag_with_label :enumeration, value %></p>
-<% end %>
+  <% @enumeration.custom_field_values.each do |value| %>
+    <p><%= custom_field_tag_with_label :enumeration, value %></p>
+  <% end %>
 </div>
diff --git a/app/views/enumerations/destroy.html.erb b/app/views/enumerations/destroy.html.erb
index b77fff66e..4fca33d4a 100644
--- a/app/views/enumerations/destroy.html.erb
+++ b/app/views/enumerations/destroy.html.erb
@@ -1,6 +1,6 @@
 <h2><%= l(@enumeration.option_name) %>: <%=h @enumeration %></h2>
 
-<% form_tag({}) do %>
+<% form_tag({}, :method => :delete) do %>
 <div class="box">
 <p><strong><%= l(:text_enumeration_destroy_question, @enumeration.objects_count) %></strong></p>
 <p><label for='reassign_to_id'><%= l(:text_enumeration_category_reassign_to) %></label>
@@ -8,5 +8,5 @@
 </div>
 
 <%= submit_tag l(:button_apply) %>
-<%= link_to l(:button_cancel), :controller => 'enumerations', :action => 'index' %>
+<%= link_to l(:button_cancel), enumerations_path %>
 <% end %>
diff --git a/app/views/enumerations/edit.html.erb b/app/views/enumerations/edit.html.erb
index f5caec1e8..c46a7606f 100644
--- a/app/views/enumerations/edit.html.erb
+++ b/app/views/enumerations/edit.html.erb
@@ -1,6 +1,6 @@
-<h2><%= link_to l(@enumeration.option_name), :controller => 'enumerations', :action => 'index' %> &#187; <%=h @enumeration %></h2>
+<h2><%= link_to l(@enumeration.option_name), enumerations_path %> &#187; <%=h @enumeration %></h2>
 
-<% form_tag({:action => 'update', :id => @enumeration}, :class => "tabular") do %>
-  <%= render :partial => 'form' %>
+<% labelled_form_for :enumeration, @enumeration, :url => enumeration_path(@enumeration), :html => {:method => :put} do |f| %>
+  <%= render :partial => 'form', :locals => {:f => f} %>
   <%= submit_tag l(:button_save) %>
 <% end %>
diff --git a/app/views/enumerations/index.html.erb b/app/views/enumerations/index.html.erb
index 80f7ef3dc..d7522bc93 100644
--- a/app/views/enumerations/index.html.erb
+++ b/app/views/enumerations/index.html.erb
@@ -15,13 +15,13 @@
 </tr></thead>
 <% enumerations.each do |enumeration| %>
 <tr class="<%= cycle('odd', 'even') %>">
-    <td><%= link_to h(enumeration), :action => 'edit', :id => enumeration %></td>
+    <td><%= link_to h(enumeration), edit_enumeration_path(enumeration) %></td>
     <td class="center" style="width:15%;"><%= checked_image enumeration.is_default? %></td>
     <td class="center" style="width:15%;"><%= checked_image enumeration.active? %></td>
-    <td style="width:15%;"><%= reorder_links('enumeration', {:action => 'update', :id => enumeration}) %></td>
+    <td style="width:15%;"><%= reorder_links('enumeration', {:action => 'update', :id => enumeration}, :put) %></td>
     <td class="buttons">
-    <%= link_to l(:button_delete), { :action => 'destroy', :id => enumeration },
-                                   :method => :post,
+    <%= link_to l(:button_delete), enumeration_path(enumeration),
+                                   :method => :delete,
                                    :confirm => l(:text_are_you_sure),
                                    :class => 'icon icon-del' %>
     </td>
@@ -31,7 +31,7 @@
 <% reset_cycle %>
 <% end %>
 
-<p><%= link_to l(:label_enumeration_new), { :action => 'new', :type => klass.name } %></p>
+<p><%= link_to l(:label_enumeration_new), new_enumeration_path(:type => klass.name) %></p>
 <% end %>
 
 <% html_title(l(:label_enumerations)) -%>
diff --git a/app/views/enumerations/new.html.erb b/app/views/enumerations/new.html.erb
index 34882834a..f0e6bd343 100644
--- a/app/views/enumerations/new.html.erb
+++ b/app/views/enumerations/new.html.erb
@@ -1,6 +1,7 @@
-<h2><%= link_to l(@enumeration.option_name), :controller => 'enumerations', :action => 'index' %> &#187; <%=l(:label_enumeration_new)%></h2>
+<h2><%= link_to l(@enumeration.option_name), enumerations_path %> &#187; <%=l(:label_enumeration_new)%></h2>
 
-<% form_tag({:action => 'create'}, :class => "tabular") do %>
-  <%= render :partial => 'form' %>
+<% labelled_form_for :enumeration, @enumeration, :url => enumerations_path do |f| %>
+  <%= f.hidden_field :type  %>
+  <%= render :partial => 'form', :locals => {:f => f} %>
   <%= submit_tag l(:button_create) %>
 <% end %>
diff --git a/config/routes.rb b/config/routes.rb
index 488b366d8..8b40a4fc7 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -212,6 +212,7 @@ ActionController::Routing::Routes.draw do |map|
   map.resources :issue_statuses, :except => :show, :collection => {:update_issue_done_ratio => :post}
   map.resources :custom_fields, :except => :show
   map.resources :roles, :except => :show, :collection => {:permissions => [:get, :post]}
+  map.resources :enumerations, :except => :show
 
   map.connect 'search', :controller => 'search', :action => 'index', :conditions => {:method => :get}
 
@@ -244,12 +245,6 @@ ActionController::Routing::Routes.draw do |map|
   map.connect 'workflows', :controller => 'workflows', :action => 'index', :conditions => {:method => :get}
   map.connect 'workflows/edit', :controller => 'workflows', :action => 'edit', :conditions => {:method => [:get, :post]}
   map.connect 'workflows/copy', :controller => 'workflows', :action => 'copy', :conditions => {:method => [:get, :post]}
-  map.connect 'enumerations', :controller => 'enumerations', :action => 'index', :conditions => {:method => :get}
-  map.connect 'enumerations/new', :controller => 'enumerations', :action => 'new', :conditions => {:method => :get}
-  map.connect 'enumerations/create', :controller => 'enumerations', :action => 'create', :conditions => {:method => :post}
-  map.connect 'enumerations/edit/:id', :controller => 'enumerations', :action => 'edit', :id => /\d+/, :conditions => {:method => :get}
-  map.connect 'enumerations/update/:id', :controller => 'enumerations', :action => 'update', :id => /\d+/, :conditions => {:method => :post}
-  map.connect 'enumerations/destroy/:id', :controller => 'enumerations', :action => 'destroy', :id => /\d+/, :conditions => {:method => :post}
   map.connect 'settings', :controller => 'settings', :action => 'index', :conditions => {:method => :get}
   map.connect 'settings/edit', :controller => 'settings', :action => 'edit', :conditions => {:method => [:get, :post]}
   map.connect 'settings/plugin/:id', :controller => 'settings', :action => 'plugin', :conditions => {:method => [:get, :post]}
diff --git a/test/functional/enumerations_controller_test.rb b/test/functional/enumerations_controller_test.rb
index 05e438b96..506fda186 100644
--- a/test/functional/enumerations_controller_test.rb
+++ b/test/functional/enumerations_controller_test.rb
@@ -35,6 +35,8 @@ class EnumerationsControllerTest < ActionController::TestCase
     assert_response :success
     assert_template 'new'
     assert_kind_of IssuePriority, assigns(:enumeration)
+    assert_tag 'input', :attributes => {:name => 'enumeration[type]', :value => 'IssuePriority'}
+    assert_tag 'input', :attributes => {:name => 'enumeration[name]'}
   end
 
   def test_create
@@ -58,11 +60,12 @@ class EnumerationsControllerTest < ActionController::TestCase
     get :edit, :id => 6
     assert_response :success
     assert_template 'edit'
+    assert_tag 'input', :attributes => {:name => 'enumeration[name]', :value => 'High'}
   end
 
   def test_update
     assert_no_difference 'IssuePriority.count' do
-      post :update, :id => 6, :enumeration => {:type => 'IssuePriority', :name => 'New name'}
+      put :update, :id => 6, :enumeration => {:type => 'IssuePriority', :name => 'New name'}
     end
     assert_redirected_to '/enumerations?type=IssuePriority'
     e = IssuePriority.find(6)
@@ -71,20 +74,24 @@ class EnumerationsControllerTest < ActionController::TestCase
 
   def test_update_with_failure
     assert_no_difference 'IssuePriority.count' do
-      post :update, :id => 6, :enumeration => {:type => 'IssuePriority', :name => ''}
+      put :update, :id => 6, :enumeration => {:type => 'IssuePriority', :name => ''}
     end
     assert_response :success
     assert_template 'edit'
   end
 
   def test_destroy_enumeration_not_in_use
-    post :destroy, :id => 7
+    assert_difference 'IssuePriority.count', -1 do
+      delete :destroy, :id => 7
+    end
     assert_redirected_to :controller => 'enumerations', :action => 'index'
     assert_nil Enumeration.find_by_id(7)
   end
 
   def test_destroy_enumeration_in_use
-    post :destroy, :id => 4
+    assert_no_difference 'IssuePriority.count' do
+      delete :destroy, :id => 4
+    end
     assert_response :success
     assert_template 'destroy'
     assert_not_nil Enumeration.find_by_id(4)
@@ -92,7 +99,9 @@ class EnumerationsControllerTest < ActionController::TestCase
 
   def test_destroy_enumeration_in_use_with_reassignment
     issue = Issue.find(:first, :conditions => {:priority_id => 4})
-    post :destroy, :id => 4, :reassign_to_id => 6
+    assert_difference 'IssuePriority.count', -1 do
+      delete :destroy, :id => 4, :reassign_to_id => 6
+    end
     assert_redirected_to :controller => 'enumerations', :action => 'index'
     assert_nil Enumeration.find_by_id(4)
     # check that the issue was reassign
diff --git a/test/integration/routing_test.rb b/test/integration/routing_test.rb
index 205f3b9c5..2119c5bc2 100644
--- a/test/integration/routing_test.rb
+++ b/test/integration/routing_test.rb
@@ -66,6 +66,15 @@ class RoutingTest < ActionController::IntegrationTest
 
     should_route :post, "/documents/22/add_attachment", :controller => 'documents', :action => 'add_attachment', :id => '22'
   end
+
+  context "roles" do
+    should_route :get, "/enumerations", :controller => 'enumerations', :action => 'index'
+    should_route :get, "/enumerations/new", :controller => 'enumerations', :action => 'new'
+    should_route :post, "/enumerations", :controller => 'enumerations', :action => 'create'
+    should_route :get, "/enumerations/2/edit", :controller => 'enumerations', :action => 'edit', :id => 2
+    should_route :put, "/enumerations/2", :controller => 'enumerations', :action => 'update', :id => 2
+    should_route :delete, "/enumerations/2", :controller => 'enumerations', :action => 'destroy', :id => 2
+  end
   
   context "groups" do
     should_route :post,   "/groups/567/users", :controller => 'groups', :action => 'add_users', :id => '567'
-- 
2.39.5