diff options
| -rw-r--r-- | app/controllers/custom_fields_controller.rb | 38 | ||||
| -rw-r--r-- | app/models/custom_field.rb | 16 | ||||
| -rw-r--r-- | app/views/custom_fields/_form.html.erb | 2 | ||||
| -rw-r--r-- | app/views/custom_fields/_index.html.erb | 8 | ||||
| -rw-r--r-- | app/views/custom_fields/edit.html.erb | 2 | ||||
| -rw-r--r-- | app/views/custom_fields/new.html.erb | 2 | ||||
| -rw-r--r-- | config/routes.rb | 1 | ||||
| -rw-r--r-- | test/functional/custom_fields_controller_test.rb | 39 | ||||
| -rw-r--r-- | test/integration/routing_test.rb | 9 | ||||
| -rw-r--r-- | test/unit/custom_field_test.rb | 19 |
10 files changed, 108 insertions, 28 deletions
diff --git a/app/controllers/custom_fields_controller.rb b/app/controllers/custom_fields_controller.rb index 2b848b3df..5121ebc4a 100644 --- a/app/controllers/custom_fields_controller.rb +++ b/app/controllers/custom_fields_controller.rb @@ -19,6 +19,8 @@ class CustomFieldsController < ApplicationController layout 'admin' before_filter :require_admin + before_filter :build_new_custom_field, :only => [:new, :create] + before_filter :find_custom_field, :only => [:edit, :update, :destroy] def index @custom_fields_by_type = CustomField.find(:all).group_by {|f| f.class.name } @@ -26,39 +28,51 @@ class CustomFieldsController < ApplicationController end def new - @custom_field = begin - if params[:type].to_s.match(/.+CustomField$/) - params[:type].to_s.constantize.new(params[:custom_field]) - end - rescue - end - (redirect_to(:action => 'index'); return) unless @custom_field.is_a?(CustomField) + end + def create if request.post? and @custom_field.save flash[:notice] = l(:notice_successful_create) call_hook(:controller_custom_fields_new_after_save, :params => params, :custom_field => @custom_field) redirect_to :action => 'index', :tab => @custom_field.class.name else - @trackers = Tracker.find(:all, :order => 'position') + render :action => 'new' end end def edit - @custom_field = CustomField.find(params[:id]) - if request.post? and @custom_field.update_attributes(params[:custom_field]) + end + + def update + if request.put? and @custom_field.update_attributes(params[:custom_field]) flash[:notice] = l(:notice_successful_update) call_hook(:controller_custom_fields_edit_after_save, :params => params, :custom_field => @custom_field) redirect_to :action => 'index', :tab => @custom_field.class.name else - @trackers = Tracker.find(:all, :order => 'position') + render :action => 'edit' end end def destroy - @custom_field = CustomField.find(params[:id]).destroy + @custom_field.destroy redirect_to :action => 'index', :tab => @custom_field.class.name rescue flash[:error] = l(:error_can_not_delete_custom_field) redirect_to :action => 'index' end + + private + + def build_new_custom_field + @custom_field = CustomField.new_subclass_instance(params[:type], params[:custom_field]) + if @custom_field.nil? + render_404 + end + end + + def find_custom_field + @custom_field = CustomField.find(params[:id]) + rescue ActiveRecord::RecordNotFound + render_404 + end end diff --git a/app/models/custom_field.rb b/app/models/custom_field.rb index 0ad8b4ce0..93e662785 100644 --- a/app/models/custom_field.rb +++ b/app/models/custom_field.rb @@ -155,6 +155,22 @@ class CustomField < ActiveRecord::Base find(:all, :conditions => ["is_for_all=?", true], :order => 'position') end + # Returns an instance of the given subclass name + def self.new_subclass_instance(class_name, *args) + klass = nil + begin + klass = class_name.to_s.classify.constantize + rescue + # invalid class name + end + unless subclasses.include? klass + klass = nil + end + if klass + klass.new(*args) + end + end + def type_name nil end diff --git a/app/views/custom_fields/_form.html.erb b/app/views/custom_fields/_form.html.erb index 7432a426f..a5abb2031 100644 --- a/app/views/custom_fields/_form.html.erb +++ b/app/views/custom_fields/_form.html.erb @@ -81,7 +81,7 @@ function toggle_custom_field_format() { when "IssueCustomField" %> <fieldset><legend><%=l(:label_tracker_plural)%></legend> - <% for tracker in @trackers %> + <% Tracker.all.each do |tracker| %> <%= check_box_tag "custom_field[tracker_ids][]", tracker.id, (@custom_field.trackers.include? tracker), diff --git a/app/views/custom_fields/_index.html.erb b/app/views/custom_fields/_index.html.erb index ccbacde1d..1adbcaa08 100644 --- a/app/views/custom_fields/_index.html.erb +++ b/app/views/custom_fields/_index.html.erb @@ -13,7 +13,7 @@ <tbody> <% (@custom_fields_by_type[tab[:name]] || []).sort.each do |custom_field| -%> <tr class="<%= cycle("odd", "even") %>"> - <td><%= link_to h(custom_field.name), :action => 'edit', :id => custom_field %></td> + <td><%= link_to h(custom_field.name), edit_custom_field_path(custom_field) %></td> <td align="center"><%= l(Redmine::CustomFieldFormat.label_for(custom_field.field_format)) %></td> <td align="center"><%= checked_image custom_field.is_required? %></td> <% if tab[:name] == 'IssueCustomField' %> @@ -22,8 +22,8 @@ <% end %> <td align="center" style="width:15%;"><%= reorder_links('custom_field', {:action => 'edit', :id => custom_field}) %></td> <td class="buttons"> - <%= link_to(l(:button_delete), { :action => 'destroy', :id => custom_field }, - :method => :post, + <%= link_to(l(:button_delete), custom_field_path(custom_field), + :method => :delete, :confirm => l(:text_are_you_sure), :class => 'icon icon-del') %> </td> @@ -32,4 +32,4 @@ </tbody> </table> -<p><%= link_to l(:label_custom_field_new), {:action => 'new', :type => tab[:name]}, :class => 'icon icon-add' %></p> +<p><%= link_to l(:label_custom_field_new), new_custom_field_path(:type => tab[:name]), :class => 'icon icon-add' %></p> diff --git a/app/views/custom_fields/edit.html.erb b/app/views/custom_fields/edit.html.erb index 51ac84fcf..62672ab20 100644 --- a/app/views/custom_fields/edit.html.erb +++ b/app/views/custom_fields/edit.html.erb @@ -2,7 +2,7 @@ » <%= link_to l(@custom_field.type_name), :controller => 'custom_fields', :action => 'index', :tab => @custom_field.class.name %> » <%=h @custom_field.name %></h2> -<% labelled_form_for :custom_field, @custom_field, :url => { :action => "edit", :id => @custom_field } do |f| %> +<% labelled_form_for :custom_field, @custom_field, :url => custom_field_path(@custom_field), :html => {:method => :put} do |f| %> <%= render :partial => 'form', :locals => { :f => f } %> <%= submit_tag l(:button_save) %> <% end %> diff --git a/app/views/custom_fields/new.html.erb b/app/views/custom_fields/new.html.erb index a68cef5dd..4fcd160e4 100644 --- a/app/views/custom_fields/new.html.erb +++ b/app/views/custom_fields/new.html.erb @@ -2,7 +2,7 @@ » <%= link_to l(@custom_field.type_name), :controller => 'custom_fields', :action => 'index', :tab => @custom_field.class.name %> » <%= l(:label_custom_field_new) %></h2> -<% labelled_form_for :custom_field, @custom_field, :url => { :action => "new" } do |f| %> +<% labelled_form_for :custom_field, @custom_field, :url => custom_fields_path do |f| %> <%= render :partial => 'form', :locals => { :f => f } %> <%= hidden_field_tag 'type', @custom_field.type %> <%= submit_tag l(:button_save) %> diff --git a/config/routes.rb b/config/routes.rb index 33e53ba00..f17ad6f00 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -184,6 +184,7 @@ ActionController::Routing::Routes.draw do |map| map.resources :trackers, :except => :show map.resources :issue_statuses, :except => :show, :collection => {:update_issue_done_ratio => :post} + map.resources :custom_fields, :except => :show #left old routes at the bottom for backwards compat map.connect 'boards/:board_id/topics/:action/:id', :controller => 'messages' diff --git a/test/functional/custom_fields_controller_test.rb b/test/functional/custom_fields_controller_test.rb index ce20b2b89..42e57b91a 100644 --- a/test/functional/custom_fields_controller_test.rb +++ b/test/functional/custom_fields_controller_test.rb @@ -37,10 +37,11 @@ class CustomFieldsControllerTest < ActionController::TestCase assert_template 'index' end - def test_get_new_issue_custom_field + def test_new_issue_custom_field get :new, :type => 'IssueCustomField' assert_response :success assert_template 'new' + assert_tag :input, :attributes => {:name => 'custom_field[name]'} assert_tag :select, :attributes => {:name => 'custom_field[field_format]'}, :child => { @@ -55,16 +56,17 @@ class CustomFieldsControllerTest < ActionController::TestCase :attributes => {:value => 'version'}, :content => 'Version' } + assert_tag :input, :attributes => {:name => 'type', :value => 'IssueCustomField'} end - def test_get_new_with_invalid_custom_field_class_should_redirect_to_list + def test_new_with_invalid_custom_field_class_should_render_404 get :new, :type => 'UnknownCustomField' - assert_redirected_to '/custom_fields' + assert_response 404 end - def test_post_new_list_custom_field + def test_create_list_custom_field assert_difference 'CustomField.count' do - post :new, :type => "IssueCustomField", + post :create, :type => "IssueCustomField", :custom_field => {:name => "test_post_new_list", :default_value => "", :min_length => "0", @@ -85,28 +87,47 @@ class CustomFieldsControllerTest < ActionController::TestCase assert_equal 1, field.trackers.size end - def test_get_edit + def test_create_with_failure + assert_no_difference 'CustomField.count' do + post :create, :type => "IssueCustomField", :custom_field => {:name => ''} + end + assert_response :success + assert_template 'new' + end + + def test_edit get :edit, :id => 1 assert_response :success assert_template 'edit' assert_tag 'input', :attributes => {:name => 'custom_field[name]', :value => 'Database'} end - def test_post_edit - post :edit, :id => 1, :custom_field => {:name => 'New name'} + def test_edit_invalid_custom_field_should_render_404 + get :edit, :id => 99 + assert_response 404 + end + + def test_update + put :update, :id => 1, :custom_field => {:name => 'New name'} assert_redirected_to '/custom_fields?tab=IssueCustomField' field = CustomField.find(1) assert_equal 'New name', field.name end + def test_update_with_failure + put :update, :id => 1, :custom_field => {:name => ''} + assert_response :success + assert_template 'edit' + end + def test_destroy custom_values_count = CustomValue.count(:conditions => {:custom_field_id => 1}) assert custom_values_count > 0 assert_difference 'CustomField.count', -1 do assert_difference 'CustomValue.count', - custom_values_count do - post :destroy, :id => 1 + delete :destroy, :id => 1 end end diff --git a/test/integration/routing_test.rb b/test/integration/routing_test.rb index bcb35b33b..d196c0873 100644 --- a/test/integration/routing_test.rb +++ b/test/integration/routing_test.rb @@ -45,6 +45,15 @@ class RoutingTest < ActionController::IntegrationTest end + context "custom_fields" do + should_route :get, "/custom_fields", :controller => 'custom_fields', :action => 'index' + should_route :get, "/custom_fields/new", :controller => 'custom_fields', :action => 'new' + should_route :post, "/custom_fields", :controller => 'custom_fields', :action => 'create' + should_route :get, "/custom_fields/2/edit", :controller => 'custom_fields', :action => 'edit', :id => 2 + should_route :put, "/custom_fields/2", :controller => 'custom_fields', :action => 'update', :id => 2 + should_route :delete, "/custom_fields/2", :controller => 'custom_fields', :action => 'destroy', :id => 2 + end + context "documents" do should_route :get, "/projects/567/documents", :controller => 'documents', :action => 'index', :project_id => '567' should_route :get, "/projects/567/documents/new", :controller => 'documents', :action => 'new', :project_id => '567' diff --git a/test/unit/custom_field_test.rb b/test/unit/custom_field_test.rb index 580e268ec..f3564e383 100644 --- a/test/unit/custom_field_test.rb +++ b/test/unit/custom_field_test.rb @@ -56,4 +56,23 @@ class CustomFieldTest < ActiveSupport::TestCase field = CustomField.find(1) assert field.destroy end + + def test_new_subclass_instance_should_return_an_instance + f = CustomField.new_subclass_instance('IssueCustomField') + assert_kind_of IssueCustomField, f + end + + def test_new_subclass_instance_should_set_attributes + f = CustomField.new_subclass_instance('IssueCustomField', :name => 'Test') + assert_kind_of IssueCustomField, f + assert_equal 'Test', f.name + end + + def test_new_subclass_instance_with_invalid_class_name_should_return_nil + assert_nil CustomField.new_subclass_instance('WrongClassName') + end + + def test_new_subclass_instance_with_non_subclass_name_should_return_nil + assert_nil CustomField.new_subclass_instance('Project') + end end |