From 3cb26e9cb3c1f3a069e7654988c650ca12322ede Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Sun, 12 Jun 2005 05:33:23 +0000 Subject: r2495@sedna: jeremy | 2005-05-27 22:07:18 -0700 Branch to have scaffold use new render methods r2496@sedna: jeremy | 2005-05-28 10:50:51 -0700 axe unused process_request method r2497@sedna: jeremy | 2005-05-28 10:51:58 -0700 sanity assertions in process method check that @controller, @request, @response are set r2498@sedna: jeremy | 2005-05-28 10:53:42 -0700 clean up escaping. use instance_variable_get instead of instance_eval. put id in form action instead of hidden input (like the generated scaffolding) r2499@sedna: jeremy | 2005-05-28 10:54:46 -0700 clean up interpolation, remove tabs from stylesheet r2500@sedna: jeremy | 2005-05-28 10:56:40 -0700 modernize dynamic scaffolding. use params[:id] instead of @params[id]. use consolidated render method instead of render_action, render_file, etc. use builtin inflections rather than module methods. r2501@sedna: jeremy | 2005-05-28 11:00:26 -0700 modernize generated scaffolding. use new-style assertions; use params[:id] instead of @params[id]; use consolidated render method instead of render_* r2542@sedna: jeremy | 2005-06-12 02:24:06 -0700 use :id => model instead of :id => model.id. clean up the form helper method. use flash[:notice] instead of flash['notice']. r2543@sedna: jeremy | 2005-06-12 02:33:22 -0700 whitespace r2544@sedna: jeremy | 2005-06-12 02:56:57 -0700 update changelogs r2545@sedna: jeremy | 2005-06-12 03:26:23 -0700 test dynamic form help when @post is not a new record r2546@sedna: jeremy | 2005-06-12 03:44:14 -0700 touch up TestCase#process sanity assertions git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@1404 5ecf4fe2-1ee6-0310-87b1-e25e094e27de --- actionpack/CHANGELOG | 6 +++ actionpack/lib/action_controller/scaffolding.rb | 53 ++++++++++++---------- .../templates/scaffolds/edit.rhtml | 4 +- .../templates/scaffolds/layout.rhtml | 38 ++++++++-------- .../templates/scaffolds/list.rhtml | 6 +-- .../templates/scaffolds/new.rhtml | 4 +- .../templates/scaffolds/show.rhtml | 2 +- actionpack/lib/action_controller/test_process.rb | 5 ++ .../action_view/helpers/active_record_helper.rb | 26 ++++++----- .../test/template/active_record_helper_test.rb | 29 ++++++++---- 10 files changed, 102 insertions(+), 71 deletions(-) (limited to 'actionpack') diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index 081abbd11a..e1a62c8713 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,5 +1,11 @@ *SVN* +* Modernize scaffolding to match the generator: use the new render method and change style from the warty @params["id"] to the sleek params[:id]. #1367 + +* Include :id in the action generated by the form helper method. Then, for example, the controller can do Model.find(params[:id]) for both edit and update actions. Updated scaffolding to take advantage. #1367 + +* Add assertions with friendly messages to TestCase#process to ensure that @controller, @request, and @response are set. #1367 + * Arrays sent via multipart posts are converted to strings #1032 [dj@omelia.org] * render(:layout => true) is a synonym for render(:layout => nil) diff --git a/actionpack/lib/action_controller/scaffolding.rb b/actionpack/lib/action_controller/scaffolding.rb index 4faa23cdf3..3435ce5108 100644 --- a/actionpack/lib/action_controller/scaffolding.rb +++ b/actionpack/lib/action_controller/scaffolding.rb @@ -27,12 +27,12 @@ module ActionController # end # # def show - # @entry = Entry.find(@params["id"]) + # @entry = Entry.find(params[:id]) # render_scaffold # end # # def destroy - # Entry.find(@params["id"]).destroy + # Entry.find(params[:id]).destroy # redirect_to :action => "list" # end # @@ -42,9 +42,9 @@ module ActionController # end # # def create - # @entry = Entry.new(@params["entry"]) + # @entry = Entry.new(params[:entry]) # if @entry.save - # flash["notice"] = "Entry was successfully created" + # flash[:notice] = "Entry was successfully created" # redirect_to :action => "list" # else # render_scaffold('new') @@ -52,17 +52,17 @@ module ActionController # end # # def edit - # @entry = Entry.find(@params["id"]) + # @entry = Entry.find(params[:id]) # render_scaffold # end # # def update - # @entry = Entry.find(@params["entry"]["id"]) - # @entry.attributes = @params["entry"] + # @entry = Entry.find(params[:id]) + # @entry.attributes = params[:entry] # # if @entry.save - # flash["notice"] = "Entry was successfully updated" - # redirect_to :action => "show/" + @entry.id.to_s + # flash[:notice] = "Entry was successfully updated" + # redirect_to :action => "show", :id => @entry # else # render_scaffold('edit') # end @@ -84,9 +84,9 @@ module ActionController def scaffold(model_id, options = {}) validate_options([ :class_name, :suffix ], options.keys) - singular_name = model_id.id2name - class_name = options[:class_name] || Inflector.camelize(singular_name) - plural_name = Inflector.pluralize(singular_name) + singular_name = model_id.to_s + class_name = options[:class_name] || singular_name.camelize + plural_name = singular_name.pluralize suffix = options[:suffix] ? "_#{singular_name}" : "" unless options[:suffix] @@ -104,12 +104,12 @@ module ActionController end def show#{suffix} - @#{singular_name} = #{class_name}.find(@params["id"]) + @#{singular_name} = #{class_name}.find(params[:id]) render#{suffix}_scaffold end def destroy#{suffix} - #{class_name}.find(@params["id"]).destroy + #{class_name}.find(params[:id]).destroy redirect_to :action => "list#{suffix}" end @@ -119,9 +119,9 @@ module ActionController end def create#{suffix} - @#{singular_name} = #{class_name}.new(@params["#{singular_name}"]) + @#{singular_name} = #{class_name}.new(params[:#{singular_name}]) if @#{singular_name}.save - flash["notice"] = "#{class_name} was successfully created" + flash[:notice] = "#{class_name} was successfully created" redirect_to :action => "list#{suffix}" else render#{suffix}_scaffold('new') @@ -129,17 +129,17 @@ module ActionController end def edit#{suffix} - @#{singular_name} = #{class_name}.find(@params["id"]) + @#{singular_name} = #{class_name}.find(params[:id]) render#{suffix}_scaffold end def update#{suffix} - @#{singular_name} = #{class_name}.find(@params["#{singular_name}"]["id"]) - @#{singular_name}.attributes = @params["#{singular_name}"] + @#{singular_name} = #{class_name}.find(params[:id]) + @#{singular_name}.attributes = params[:#{singular_name}] if @#{singular_name}.save - flash["notice"] = "#{class_name} was successfully updated" - redirect_to :action => "show#{suffix}", :id => @#{singular_name}.id.to_s + flash[:notice] = "#{class_name} was successfully updated" + redirect_to :action => "show#{suffix}", :id => @#{singular_name} else render#{suffix}_scaffold('edit') end @@ -148,7 +148,7 @@ module ActionController private def render#{suffix}_scaffold(action = caller_method_name(caller)) if template_exists?("\#{self.class.controller_path}/\#{action}") - render_action(action) + render(:action => action) else @scaffold_class = #{class_name} @scaffold_singular_name, @scaffold_plural_name = "#{singular_name}", "#{plural_name}" @@ -156,10 +156,15 @@ module ActionController add_instance_variables_to_assigns @content_for_layout = @template.render_file(scaffold_path(action.sub(/#{suffix}$/, "")), false) - self.active_layout ? render_file(self.active_layout, "200 OK", true) : render_file(scaffold_path("layout")) + + if active_layout? + render :file => active_layout, :use_full_path => true + else + render :file => scaffold_path("layout") + end end end - + def scaffold_path(template_name) File.dirname(__FILE__) + "/templates/scaffolds/" + template_name + ".rhtml" end diff --git a/actionpack/lib/action_controller/templates/scaffolds/edit.rhtml b/actionpack/lib/action_controller/templates/scaffolds/edit.rhtml index 31e15f7501..63dff602a1 100644 --- a/actionpack/lib/action_controller/templates/scaffolds/edit.rhtml +++ b/actionpack/lib/action_controller/templates/scaffolds/edit.rhtml @@ -1,7 +1,7 @@

Editing <%= @scaffold_singular_name %>

<%= error_messages_for(@scaffold_singular_name) %> -<%= form(@scaffold_singular_name, :action => "update" + @scaffold_suffix) %> +<%= form(@scaffold_singular_name, :action => "update#{@scaffold_suffix}") %> -<%= link_to "Show", :action => "show#{@scaffold_suffix}", :id => instance_variable_get("@#{@scaffold_singular_name}").id %> | +<%= link_to "Show", :action => "show#{@scaffold_suffix}", :id => instance_variable_get("@#{@scaffold_singular_name}") %> | <%= link_to "Back", :action => "list#{@scaffold_suffix}" %> diff --git a/actionpack/lib/action_controller/templates/scaffolds/layout.rhtml b/actionpack/lib/action_controller/templates/scaffolds/layout.rhtml index 300aa1d77a..80996985d2 100644 --- a/actionpack/lib/action_controller/templates/scaffolds/layout.rhtml +++ b/actionpack/lib/action_controller/templates/scaffolds/layout.rhtml @@ -28,40 +28,40 @@ #ErrorExplanation { width: 400px; - border: 2px solid 'red'; - padding: 7px; - padding-bottom: 12px; - margin-bottom: 20px; - background-color: #f0f0f0; + border: 2px solid 'red'; + padding: 7px; + padding-bottom: 12px; + margin-bottom: 20px; + background-color: #f0f0f0; } #ErrorExplanation h2 { - text-align: left; - font-weight: bold; - padding: 5px 5px 5px 15px; - font-size: 12px; - margin: -7px; - background-color: #c00; - color: #fff; + text-align: left; + font-weight: bold; + padding: 5px 5px 5px 15px; + font-size: 12px; + margin: -7px; + background-color: #c00; + color: #fff; } #ErrorExplanation p { - color: #333; - margin-bottom: 0; - padding: 5px; + color: #333; + margin-bottom: 0; + padding: 5px; } #ErrorExplanation ul li { - font-size: 12px; - list-style: square; + font-size: 12px; + list-style: square; } -

<%= flash['notice'] %>

+

<%= flash[:notice] %>

<%= @content_for_layout %> - \ No newline at end of file + diff --git a/actionpack/lib/action_controller/templates/scaffolds/list.rhtml b/actionpack/lib/action_controller/templates/scaffolds/list.rhtml index 233e9da87d..d1196a1599 100644 --- a/actionpack/lib/action_controller/templates/scaffolds/list.rhtml +++ b/actionpack/lib/action_controller/templates/scaffolds/list.rhtml @@ -12,9 +12,9 @@ <% for column in @scaffold_class.content_columns %> <%= entry.send(column.name) %> <% end %> - <%= link_to "Show", :action => "show#{@scaffold_suffix}", :id => entry.id %> - <%= link_to "Edit", :action => "edit#{@scaffold_suffix}", :id => entry.id %> - <%= link_to "Destroy", :action => "destroy#{@scaffold_suffix}", :id => entry.id, :confirm => "Are you sure?" %> + <%= link_to "Show", :action => "show#{@scaffold_suffix}", :id => entry %> + <%= link_to "Edit", :action => "edit#{@scaffold_suffix}", :id => entry %> + <%= link_to "Destroy", :action => "destroy#{@scaffold_suffix}", :id => entry, :confirm => "Are you sure?" %> <% end %> diff --git a/actionpack/lib/action_controller/templates/scaffolds/new.rhtml b/actionpack/lib/action_controller/templates/scaffolds/new.rhtml index 74db8468bd..66f6626f4e 100644 --- a/actionpack/lib/action_controller/templates/scaffolds/new.rhtml +++ b/actionpack/lib/action_controller/templates/scaffolds/new.rhtml @@ -1,6 +1,6 @@

New <%= @scaffold_singular_name %>

<%= error_messages_for(@scaffold_singular_name) %> -<%= form(@scaffold_singular_name, :action => "create" + @scaffold_suffix) %> +<%= form(@scaffold_singular_name, :action => "create#{@scaffold_suffix}") %> -<%= link_to "Back", :action => "list#{@scaffold_suffix}" %> \ No newline at end of file +<%= link_to "Back", :action => "list#{@scaffold_suffix}" %> diff --git a/actionpack/lib/action_controller/templates/scaffolds/show.rhtml b/actionpack/lib/action_controller/templates/scaffolds/show.rhtml index 10c46342fd..46cdfdb493 100644 --- a/actionpack/lib/action_controller/templates/scaffolds/show.rhtml +++ b/actionpack/lib/action_controller/templates/scaffolds/show.rhtml @@ -5,5 +5,5 @@

<% end %> -<%= link_to "Edit", :action => "edit#{@scaffold_suffix}", :id => instance_variable_get("@#{@scaffold_singular_name}").id %> | +<%= link_to "Edit", :action => "edit#{@scaffold_suffix}", :id => instance_variable_get("@#{@scaffold_singular_name}") %> | <%= link_to "Back", :action => "list#{@scaffold_suffix}" %> diff --git a/actionpack/lib/action_controller/test_process.rb b/actionpack/lib/action_controller/test_process.rb index 25ad540468..20022aab91 100644 --- a/actionpack/lib/action_controller/test_process.rb +++ b/actionpack/lib/action_controller/test_process.rb @@ -247,6 +247,11 @@ module Test private # execute the request and set/volley the response def process(action, parameters = nil, session = nil, flash = nil) + # Sanity check for required instance variables so we can give an understandable error message. + %w(controller request response).each do |iv_name| + assert_not_nil instance_variable_get("@#{iv_name}"), "@#{iv_name} is nil: make sure you set it in your test's setup method." + end + @html_document = nil @request.env['REQUEST_METHOD'] ||= "GET" @request.action = action.to_s diff --git a/actionpack/lib/action_view/helpers/active_record_helper.rb b/actionpack/lib/action_view/helpers/active_record_helper.rb index f8a003e948..deb3effb1c 100644 --- a/actionpack/lib/action_view/helpers/active_record_helper.rb +++ b/actionpack/lib/action_view/helpers/active_record_helper.rb @@ -56,20 +56,22 @@ module ActionView # form << content_tag("b", "Department") # form << collection_select("department", "id", @departments, "id", "name") # end - def form(record_name, options = nil) - options = (options || {}).symbolize_keys - record = instance_eval("@#{record_name}") + def form(record_name, options = {}) + record = instance_variable_get("@#{record_name}") + options = options.symbolize_keys options[:action] ||= record.new_record? ? "create" : "update" - action = url_for(:action => options[:action]) + action = url_for(:action => options[:action], :id => record) submit_value = options[:submit_value] || options[:action].gsub(/[^\w]/, '').capitalize - id_field = record.new_record? ? "" : InstanceTag.new(record_name, "id", self).to_input_field_tag("hidden") + contents = '' + contents << hidden_field(record_name, :id) unless record.new_record? + contents << all_input_tags(record, record_name, options) + yield contents if block_given? + contents << submit_tag(submit_value) - formtag = %(
#{id_field}) + all_input_tags(record, record_name, options) - yield formtag if block_given? - formtag + %(
) + content_tag('form', contents, :action => action, :method => 'post') end # Returns a string containing the error message attached to the +method+ on the +object+, if one exists. @@ -83,8 +85,8 @@ module ActionView # <%= error_message_on "post", "title", "Title simply ", " (or it won't work)", "inputError" %> => #
Title simply can't be empty (or it won't work)
def error_message_on(object, method, prepend_text = "", append_text = "", css_class = "formError") - if errors = instance_eval("@#{object}").errors.on(method) - "
#{prepend_text + (errors.is_a?(Array) ? errors.first : errors) + append_text}
" + if errors = instance_variable_get("@#{object}").errors.on(method) + content_tag("div", "#{prepend_text}#{errors.is_a?(Array) ? errors.first : errors}#{append_text}", :class => css_class) end end @@ -96,7 +98,7 @@ module ActionView # * class - The class of the error div (default: errorExplanation) def error_messages_for(object_name, options = {}) options = options.symbolize_keys - object = instance_eval "@#{object_name}" + object = instance_variable_get("@#{object_name}") unless object.errors.empty? content_tag("div", content_tag( @@ -117,7 +119,7 @@ module ActionView end def default_input_block - Proc.new { |record, column| "


#{input(record, column.name)}

" } + Proc.new { |record, column| %(


#{input(record, column.name)}

) } end end diff --git a/actionpack/test/template/active_record_helper_test.rb b/actionpack/test/template/active_record_helper_test.rb index 5ee7221913..8a4e45b65f 100644 --- a/actionpack/test/template/active_record_helper_test.rb +++ b/actionpack/test/template/active_record_helper_test.rb @@ -4,6 +4,7 @@ require File.dirname(__FILE__) + '/../../lib/action_view/helpers/form_helper' require File.dirname(__FILE__) + '/../../lib/action_view/helpers/text_helper' require File.dirname(__FILE__) + '/../../lib/action_view/helpers/tag_helper' require File.dirname(__FILE__) + '/../../lib/action_view/helpers/url_helper' +require File.dirname(__FILE__) + '/../../lib/action_view/helpers/form_tag_helper' # require File.dirname(__FILE__) + '/../../lib/action_view/helpers/active_record_helper' class ActiveRecordHelperTest < Test::Unit::TestCase @@ -12,6 +13,7 @@ class ActiveRecordHelperTest < Test::Unit::TestCase include ActionView::Helpers::TextHelper include ActionView::Helpers::TagHelper include ActionView::Helpers::UrlHelper + include ActionView::Helpers::FormTagHelper Post = Struct.new("Post", :title, :author_name, :body, :secret, :written_on) Post.class_eval do @@ -33,6 +35,7 @@ class ActiveRecordHelperTest < Test::Unit::TestCase end def @post.new_record?() true end + def @post.to_param() nil end def @post.column_for_attribute(attr_name) Post.content_columns.select { |column| column.name == attr_name }.first @@ -46,12 +49,12 @@ class ActiveRecordHelperTest < Test::Unit::TestCase @post.secret = 1 @post.written_on = Date.new(2004, 6, 15) - @controller = Class.new do - def url_for(options, *parameters_for_method_reference) - options[:action] || options["action"] - end + @controller = Object.new + def @controller.url_for(options, *parameters_for_method_reference) + options = options.symbolize_keys + + [options[:action], options[:id].to_param].compact.join('/') end - @controller = @controller.new end def test_generic_input_tag @@ -76,7 +79,17 @@ class ActiveRecordHelperTest < Test::Unit::TestCase def test_form_with_string assert_equal( - %(


\n


), + %(


\n


), + form("post") + ) + + class << @post + def new_record?() false end + def to_param() id end + def id() 1 end + end + assert_equal( + %(


\n


), form("post") ) end @@ -85,7 +98,7 @@ class ActiveRecordHelperTest < Test::Unit::TestCase def Post.content_columns() [ Column.new(:date, "written_on", "Written on") ] end assert_equal( - %(


\n\n\n

), + %(


\n\n\n

), form("post") ) end @@ -95,7 +108,7 @@ class ActiveRecordHelperTest < Test::Unit::TestCase @post.written_on = Time.gm(2004, 6, 15, 16, 30) assert_equal( - %(


\n\n\n — \n : \n

), + %(


\n\n\n — \n : \n

), form("post") ) end -- cgit v1.2.3