diff options
Diffstat (limited to 'actionpack')
-rw-r--r-- | actionpack/CHANGELOG | 4 | ||||
-rw-r--r-- | actionpack/lib/action_dispatch/routing/mapper.rb | 20 | ||||
-rw-r--r-- | actionpack/lib/action_dispatch/testing/assertions/response.rb | 10 | ||||
-rw-r--r-- | actionpack/lib/action_view/helpers/form_helper.rb | 10 | ||||
-rw-r--r-- | actionpack/lib/action_view/helpers/form_options_helper.rb | 4 | ||||
-rw-r--r-- | actionpack/lib/action_view/renderer/partial_renderer.rb | 39 | ||||
-rw-r--r-- | actionpack/lib/sprockets/railtie.rb | 80 | ||||
-rw-r--r-- | actionpack/test/dispatch/mapper_test.rb | 7 | ||||
-rw-r--r-- | actionpack/test/template/form_helper_test.rb | 28 | ||||
-rw-r--r-- | actionpack/test/template/render_test.rb | 30 |
10 files changed, 145 insertions, 87 deletions
diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index 9888be07a9..3b323b3899 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -2270,7 +2270,7 @@ superclass' view_paths. [Rick Olson] * Update documentation for erb trim syntax. #5651 [matt@mattmargolis.net] -* Pass :id => nil or :class => nil to error_messages_for to supress that html attribute. #3586 [olivier_ansaldi@yahoo.com, sebastien@goetzilla.info] +* Pass :id => nil or :class => nil to error_messages_for to supress that html attribute. #3586 [olivier_ansaldi@yahoo.com] * Reset @html_document between requests so assert_tag works. #4810 [Jarkko Laine, easleydp@gmail.com] @@ -2867,7 +2867,7 @@ superclass' view_paths. [Rick Olson] * Provide support for decimal columns to form helpers. Closes #5672. [Dave Thomas] -* Pass :id => nil or :class => nil to error_messages_for to supress that html attribute. #3586 [olivier_ansaldi@yahoo.com, sebastien@goetzilla.info] +* Pass :id => nil or :class => nil to error_messages_for to supress that html attribute. #3586 [olivier_ansaldi@yahoo.com] * Reset @html_document between requests so assert_tag works. #4810 [Jarkko Laine, easleydp@gmail.com] diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 003bc1dc2c..53374949ae 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -51,7 +51,7 @@ module ActionDispatch IGNORE_OPTIONS = [:to, :as, :via, :on, :constraints, :defaults, :only, :except, :anchor, :shallow, :shallow_path, :shallow_prefix] ANCHOR_CHARACTERS_REGEX = %r{\A(\\A|\^)|(\\Z|\\z|\$)\Z} SHORTHAND_REGEX = %r{^/[\w/]+$} - WILDCARD_PATH = %r{\*([^/]+)$} + WILDCARD_PATH = %r{\*([^/\)]+)\)?$} def initialize(set, scope, path, options) @set, @scope = set, scope @@ -105,13 +105,13 @@ module ActionDispatch # controllers with default routes like :controller/:action/:id(.:format), e.g: # GET /admin/products/show/1 # => { :controller => 'admin/products', :action => 'show', :id => '1' } - @options.reverse_merge!(:controller => /.+?/) + @options[:controller] ||= /.+?/ end # Add a constraint for wildcard route to make it non-greedy and match the # optional format part of the route by default if path.match(WILDCARD_PATH) && @options[:format] != false - @options.reverse_merge!(:"#{$1}" => /.+?/) + @options[$1.to_sym] ||= /.+?/ end if @options[:format] == false @@ -224,19 +224,11 @@ module ActionDispatch end def default_controller - if @options[:controller] - @options[:controller] - elsif @scope[:controller] - @scope[:controller] - end + @options[:controller] || @scope[:controller] end def default_action - if @options[:action] - @options[:action] - elsif @scope[:action] - @scope[:action] - end + @options[:action] || @scope[:action] end end @@ -264,7 +256,7 @@ module ActionDispatch # because this means it will be matched first. As this is the most popular route # of most Rails applications, this is beneficial. def root(options = {}) - match '/', options.reverse_merge(:as => :root) + match '/', { :as => :root }.merge(options) end # Matches a url pattern to one or more routes. Any symbols in a pattern diff --git a/actionpack/lib/action_dispatch/testing/assertions/response.rb b/actionpack/lib/action_dispatch/testing/assertions/response.rb index a2d639cd56..7381617dd7 100644 --- a/actionpack/lib/action_dispatch/testing/assertions/response.rb +++ b/actionpack/lib/action_dispatch/testing/assertions/response.rb @@ -55,16 +55,14 @@ module ActionDispatch # assert_redirected_to @customer # def assert_redirected_to(options = {}, message=nil) - validate_request! - assert_response(:redirect, message) return true if options == @response.location - redirected_to_after_normalization = normalize_argument_to_redirection(@response.location) - options_after_normalization = normalize_argument_to_redirection(options) + redirect_is = normalize_argument_to_redirection(@response.location) + redirect_expected = normalize_argument_to_redirection(options) - if redirected_to_after_normalization != options_after_normalization - flunk "Expected response to be a redirect to <#{options_after_normalization}> but was a redirect to <#{redirected_to_after_normalization}>" + if redirect_is != redirect_expected + flunk "Expected response to be a redirect to <#{redirect_expected}> but was a redirect to <#{redirect_is}>" end end diff --git a/actionpack/lib/action_view/helpers/form_helper.rb b/actionpack/lib/action_view/helpers/form_helper.rb index 974c963d44..85dea96bbb 100644 --- a/actionpack/lib/action_view/helpers/form_helper.rb +++ b/actionpack/lib/action_view/helpers/form_helper.rb @@ -365,7 +365,7 @@ module ActionView apply_form_for_options!(record, options) end - options[:html][:remote] = options.delete(:remote) + options[:html][:remote] = options.delete(:remote) if options.has_key?(:remote) options[:html][:method] = options.delete(:method) if options.has_key?(:method) options[:html][:authenticity_token] = options.delete(:authenticity_token) @@ -1227,8 +1227,12 @@ module ActionView parent_builder.multipart = multipart if parent_builder end - def self.model_name - @model_name ||= Struct.new(:partial_path).new(name.demodulize.underscore.sub!(/_builder$/, '')) + def self._to_path + @_to_path ||= name.demodulize.underscore.sub!(/_builder$/, '') + end + + def to_path + self.class._to_path end def to_model diff --git a/actionpack/lib/action_view/helpers/form_options_helper.rb b/actionpack/lib/action_view/helpers/form_options_helper.rb index 7c43dc04e0..c677257d60 100644 --- a/actionpack/lib/action_view/helpers/form_options_helper.rb +++ b/actionpack/lib/action_view/helpers/form_options_helper.rb @@ -323,12 +323,12 @@ module ActionView return container if String === container selected, disabled = extract_selected_and_disabled(selected).map do | r | - Array.wrap(r).map(&:to_s) + Array.wrap(r).map { |item| item.to_s } end container.map do |element| html_attributes = option_html_attributes(element) - text, value = option_text_and_value(element).map(&:to_s) + text, value = option_text_and_value(element).map { |item| item.to_s } selected_attribute = ' selected="selected"' if option_value_selected?(value, selected) disabled_attribute = ' disabled="disabled"' if disabled && option_value_selected?(value, disabled) %(<option value="#{ERB::Util.html_escape(value)}"#{selected_attribute}#{disabled_attribute}#{html_attributes}>#{ERB::Util.html_escape(text)}</option>) diff --git a/actionpack/lib/action_view/renderer/partial_renderer.rb b/actionpack/lib/action_view/renderer/partial_renderer.rb index c0ac332c4e..f67388b8cf 100644 --- a/actionpack/lib/action_view/renderer/partial_renderer.rb +++ b/actionpack/lib/action_view/renderer/partial_renderer.rb @@ -206,11 +206,12 @@ module ActionView # <%- end -%> # <% end %> class PartialRenderer < AbstractRenderer #:nodoc: - PARTIAL_NAMES = Hash.new {|h,k| h[k] = {} } + PARTIAL_NAMES = Hash.new { |h,k| h[k] = {} } def initialize(*) super - @partial_names = PARTIAL_NAMES[@lookup_context.prefixes.first] + @context_prefix = @lookup_context.prefixes.first + @partial_names = PARTIAL_NAMES[@context_prefix] end def render(context, options, block) @@ -291,6 +292,7 @@ module ActionView else paths.map! { |path| retrieve_variable(path).unshift(path) } end + if String === partial && @variable.to_s !~ /^[a-z_][a-zA-Z_0-9]*$/ raise ArgumentError.new("The partial name (#{partial}) is not a valid Ruby identifier; " + "make sure your partial name starts with a letter or underscore, " + @@ -360,27 +362,32 @@ module ActionView end def partial_path(object = @object) - @partial_names[object.class.name] ||= begin - object = object.to_model if object.respond_to?(:to_model) - object.class.model_name.partial_path.dup.tap do |partial| - path = @lookup_context.prefixes.first - merge_path_into_partial(path, partial) - end + object = object.to_model if object.respond_to?(:to_model) + + path = if object.respond_to?(:to_path) + object.to_path + else + ActiveSupport::Deprecation.warn "ActiveModel-compatible objects whose classes return a #model_name that responds to #partial_path are deprecated. Please respond to #to_path directly instead." + object.class.model_name.partial_path + end + + @partial_names[path] ||= path.dup.tap do |object_path| + merge_prefix_into_object_path(@context_prefix, object_path) end end - def merge_path_into_partial(path, partial) - if path.include?(?/) && partial.include?(?/) + def merge_prefix_into_object_path(prefix, object_path) + if prefix.include?(?/) && object_path.include?(?/) overlap = [] - path_array = File.dirname(path).split('/') - partial_array = partial.split('/')[0..-3] # skip model dir & partial + prefix_array = File.dirname(prefix).split('/') + object_path_array = object_path.split('/')[0..-3] # skip model dir & partial - path_array.each_with_index do |dir, index| - overlap << dir if dir == partial_array[index] + prefix_array.each_with_index do |dir, index| + overlap << dir if dir == object_path_array[index] end - partial.gsub!(/^#{overlap.join('/')}\//,'') - partial.insert(0, "#{File.dirname(path)}/") + object_path.gsub!(/^#{overlap.join('/')}\//,'') + object_path.insert(0, "#{File.dirname(prefix)}/") end end diff --git a/actionpack/lib/sprockets/railtie.rb b/actionpack/lib/sprockets/railtie.rb index 83799d2b4d..c8438e6043 100644 --- a/actionpack/lib/sprockets/railtie.rb +++ b/actionpack/lib/sprockets/railtie.rb @@ -11,15 +11,20 @@ module Sprockets load "sprockets/assets.rake" end - # We need to configure this after initialization to ensure we collect - # paths from all engines. This hook is invoked exactly before routes - # are compiled, and so that other Railties have an opportunity to - # register compressors. - config.after_initialize do |app| - assets = app.config.assets - next unless assets.enabled + initializer "sprockets.environment" do |app| + config = app.config + next unless config.assets.enabled - app.assets = asset_environment(app) + require 'sprockets' + + app.assets = Sprockets::Environment.new(app.root.to_s) do |env| + env.static_root = File.join(app.root.join('public'), config.assets.prefix) + env.logger = ::Rails.logger + + if config.assets.cache_store != false + env.cache = ActiveSupport::Cache.lookup_store(config.assets.cache_store) || ::Rails.cache + end + end ActiveSupport.on_load(:action_view) do include ::Sprockets::Helpers::RailsHelper @@ -28,53 +33,40 @@ module Sprockets include ::Sprockets::Helpers::RailsHelper end end - - app.routes.prepend do - mount app.assets => assets.prefix - end - - if config.action_controller.perform_caching - app.assets = app.assets.index - end end - protected - def asset_environment(app) - require "sprockets" - - assets = app.config.assets - - env = Sprockets::Environment.new(app.root.to_s) + # We need to configure this after initialization to ensure we collect + # paths from all engines. This hook is invoked exactly before routes + # are compiled, and so that other Railties have an opportunity to + # register compressors. + config.after_initialize do |app| + next unless app.assets + config = app.config - env.static_root = File.join(app.root.join("public"), assets.prefix) + config.assets.paths.each { |path| app.assets.append_path(path) } - if env.respond_to?(:append_path) - assets.paths.each { |path| env.append_path(path) } - else - env.paths.concat assets.paths + if config.assets.compress + # temporarily hardcode default JS compressor to uglify. Soon, it will work + # the same as SCSS, where a default plugin sets the default. + unless config.assets.js_compressor == false + app.assets.js_compressor = LazyCompressor.new { expand_js_compressor(config.assets.js_compressor || :uglifier) } end - env.logger = ::Rails.logger - - if env.respond_to?(:cache) && assets.cache_store != false - env.cache = ActiveSupport::Cache.lookup_store(assets.cache_store) || ::Rails.cache + unless config.assets.css_compressor == false + app.assets.css_compressor = LazyCompressor.new { expand_css_compressor(config.assets.css_compressor) } end + end - if assets.compress - # temporarily hardcode default JS compressor to uglify. Soon, it will work - # the same as SCSS, where a default plugin sets the default. - unless assets.js_compressor == false - env.js_compressor = LazyCompressor.new { expand_js_compressor(assets.js_compressor || :uglifier) } - end - - unless assets.css_compressor == false - env.css_compressor = LazyCompressor.new { expand_css_compressor(assets.css_compressor) } - end - end + app.routes.prepend do + mount app.assets => config.assets.prefix + end - env + if config.action_controller.perform_caching + app.assets = app.assets.index end + end + protected def expand_js_compressor(sym) case sym when :closure diff --git a/actionpack/test/dispatch/mapper_test.rb b/actionpack/test/dispatch/mapper_test.rb index 81f0efb76e..3316dd03aa 100644 --- a/actionpack/test/dispatch/mapper_test.rb +++ b/actionpack/test/dispatch/mapper_test.rb @@ -35,6 +35,13 @@ module ActionDispatch Mapper.new FakeSet.new end + def test_mapping_requirements + options = { :controller => 'foo', :action => 'bar' } + m = Mapper::Mapping.new FakeSet.new, {}, '/store/:name(*rest)', options + _, _, requirements, _ = m.to_route + assert_equal(/.+?/, requirements[:rest]) + end + def test_map_slash fakeset = FakeSet.new mapper = Mapper.new fakeset diff --git a/actionpack/test/template/form_helper_test.rb b/actionpack/test/template/form_helper_test.rb index cc3d2cddf7..71a2c46d92 100644 --- a/actionpack/test/template/form_helper_test.rb +++ b/actionpack/test/template/form_helper_test.rb @@ -791,6 +791,23 @@ class FormHelperTest < ActionView::TestCase assert_dom_equal expected, output_buffer end + def test_form_for_with_remote_in_html + form_for(@post, :url => '/', :html => { :remote => true, :id => 'create-post', :method => :put }) do |f| + concat f.text_field(:title) + concat f.text_area(:body) + concat f.check_box(:secret) + end + + expected = whole_form("/", "create-post", "edit_post", :method => "put", :remote => true) do + "<input name='post[title]' size='30' type='text' id='post_title' value='Hello World' />" + + "<textarea name='post[body]' id='post_body' rows='20' cols='40'>Back to the hill and over it again!</textarea>" + + "<input name='post[secret]' type='hidden' value='0' />" + + "<input name='post[secret]' checked='checked' type='checkbox' id='post_secret' value='1' />" + end + + assert_dom_equal expected, output_buffer + end + def test_form_for_with_remote_without_html @post.persisted = false form_for(@post, :remote => true) do |f| @@ -1874,6 +1891,17 @@ class FormHelperTest < ActionView::TestCase assert_equal LabelledFormBuilder, klass end + def test_form_for_with_labelled_builder_path + path = nil + + form_for(@post, :builder => LabelledFormBuilder) do |f| + path = f.to_path + '' + end + + assert_equal 'labelled_form', path + end + class LabelledFormBuilderSubclass < LabelledFormBuilder; end def test_form_for_with_labelled_builder_with_nested_fields_for_with_custom_builder diff --git a/actionpack/test/template/render_test.rb b/actionpack/test/template/render_test.rb index 68b2ed45d1..0b91e55091 100644 --- a/actionpack/test/template/render_test.rb +++ b/actionpack/test/template/render_test.rb @@ -201,6 +201,36 @@ module RenderTestCases @controller_view.render(customers, :greeting => "Hello") end + class CustomerWithDeprecatedPartialPath + attr_reader :name + + def self.model_name + Struct.new(:partial_path).new("customers/customer") + end + + def initialize(name) + @name = name + end + end + + def test_render_partial_using_object_with_deprecated_partial_path + assert_deprecated(/#model_name.*#partial_path.*#to_path/) do + assert_equal "Hello: nertzy", + @controller_view.render(CustomerWithDeprecatedPartialPath.new("nertzy"), :greeting => "Hello") + end + end + + def test_render_partial_using_collection_with_deprecated_partial_path + assert_deprecated(/#model_name.*#partial_path.*#to_path/) do + customers = [ + CustomerWithDeprecatedPartialPath.new("nertzy"), + CustomerWithDeprecatedPartialPath.new("peeja") + ] + assert_equal "Hello: nertzyHello: peeja", + @controller_view.render(customers, :greeting => "Hello") + end + end + # TODO: The reason for this test is unclear, improve documentation def test_render_partial_and_fallback_to_layout assert_equal "Before (Josh)\n\nAfter", @view.render(:partial => "test/layout_for_partial", :locals => { :name => "Josh" }) |