Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Allow layout fallback when using `layout` method #3884

Merged
merged 2 commits into from

3 participants

@sikachu
Collaborator

Rails will now use your default layout (such as "layouts/application") when you specify a layout with :only and :except condition, and those conditions fail.

For example, consider this snippet:

class CarsController
  layout 'single_car', :only => :show
end

Rails will use 'layouts/single_car' when a request comes in :show action, and use 'layouts/application' (or 'layouts/cars', if exists) when a request comes in for any other actions.


Bonus point: I found out that assert_template :layout => 'blahhhhhh' will returns true regardless of layout that got rendered. I've fixed that as well.

sikachu added some commits
@sikachu sikachu Fix bug in assert_template when using only `:layout` option
Currently if you're do this:

    assert_template :layout => "foo"

Regardless of what layout you were using, the test will always pass. This was broken since the introduction of :layout option in [d9375f3].

We have a lot of test cases in actionpack/test/controller/layout_test.rb that use this feature. This will make sure that those test cases are not true negative.
0460b3a
@sikachu sikachu Allow layout fallback when using `layout` method
Rails will now use your default layout (such as "layouts/application") when you specify a layout with `:only` and `:except` condition, and those conditions fail.

For example, consider this snippet:

    class CarsController
      layout 'single_car', :only => :show
    end

Rails will use 'layouts/single_car' when a request comes in `:show` action, and use 'layouts/application' (or 'layouts/cars', if exists) when a request comes in for any other actions.
18ceed2
@josevalim josevalim merged commit 0df2ef3 into rails:master
@carlosantoniodasilva

Thanks! :) :heart:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Dec 7, 2011
  1. @sikachu

    Fix bug in assert_template when using only `:layout` option

    sikachu authored
    Currently if you're do this:
    
        assert_template :layout => "foo"
    
    Regardless of what layout you were using, the test will always pass. This was broken since the introduction of :layout option in [d9375f3].
    
    We have a lot of test cases in actionpack/test/controller/layout_test.rb that use this feature. This will make sure that those test cases are not true negative.
  2. @sikachu

    Allow layout fallback when using `layout` method

    sikachu authored
    Rails will now use your default layout (such as "layouts/application") when you specify a layout with `:only` and `:except` condition, and those conditions fail.
    
    For example, consider this snippet:
    
        class CarsController
          layout 'single_car', :only => :show
        end
    
    Rails will use 'layouts/single_car' when a request comes in `:show` action, and use 'layouts/application' (or 'layouts/cars', if exists) when a request comes in for any other actions.
This page is out of date. Refresh to see the latest.
View
10 actionpack/CHANGELOG.md
@@ -1,5 +1,15 @@
## Rails 3.2.0 (unreleased) ##
+* Rails will now use your default layout (such as "layouts/application") when you specify a layout with `:only` and `:except` condition, and those conditions fail. *Prem Sichanugrist*
+
+ For example, consider this snippet:
+
+ class CarsController
+ layout 'single_car', :only => :show
+ end
+
+ Rails will use 'layouts/single_car' when a request comes in `:show` action, and use 'layouts/application' (or 'layouts/cars', if exists) when a request comes in for any other actions.
+
* form_for with +:as+ option uses "#{action}_#{as}" as css class and id:
Before:
View
59 actionpack/lib/abstract_controller/layouts.rb
@@ -244,42 +244,51 @@ def _implied_layout_name
def _write_layout_method
remove_possible_method(:_layout)
- case defined?(@_layout) ? @_layout : nil
- when String
- self.class_eval %{def _layout; #{@_layout.inspect} end}, __FILE__, __LINE__
- when Symbol
- self.class_eval <<-ruby_eval, __FILE__, __LINE__ + 1
- def _layout
+ prefixes = _implied_layout_name =~ /\blayouts/ ? [] : ["layouts"]
+ layout_definition = case defined?(@_layout) ? @_layout : nil
+ when String
+ @_layout.inspect
+ when Symbol
+ <<-RUBY
#{@_layout}.tap do |layout|
unless layout.is_a?(String) || !layout
raise ArgumentError, "Your layout method :#{@_layout} returned \#{layout}. It " \
"should have returned a String, false, or nil"
end
end
- end
- ruby_eval
- when Proc
- define_method :_layout_from_proc, &@_layout
- self.class_eval %{def _layout; _layout_from_proc(self) end}, __FILE__, __LINE__
- when false
- self.class_eval %{def _layout; end}, __FILE__, __LINE__
- when true
- raise ArgumentError, "Layouts must be specified as a String, Symbol, false, or nil"
- when nil
- if name
- _prefixes = _implied_layout_name =~ /\blayouts/ ? [] : ["layouts"]
-
- self.class_eval <<-RUBY, __FILE__, __LINE__ + 1
- def _layout
- if template_exists?("#{_implied_layout_name}", #{_prefixes.inspect})
+ RUBY
+ when Proc
+ define_method :_layout_from_proc, &@_layout
+ "_layout_from_proc(self)"
+ when false
+ nil
+ when true
+ raise ArgumentError, "Layouts must be specified as a String, Symbol, false, or nil"
+ when nil
+ if name
+ <<-RUBY
+ if template_exists?("#{_implied_layout_name}", #{prefixes.inspect})
"#{_implied_layout_name}"
else
super
end
+ RUBY
+ end
+ end
+
+ self.class_eval <<-RUBY, __FILE__, __LINE__ + 1
+ def _layout
+ if action_has_layout?
+ #{layout_definition}
+ elsif self.class.name
+ if template_exists?("#{_implied_layout_name}", #{prefixes.inspect})
+ "#{_implied_layout_name}"
+ else
+ super
end
- RUBY
+ end
end
- end
+ RUBY
self.class_eval { private :_layout }
end
end
@@ -337,7 +346,7 @@ def _layout_for_option(name)
# * <tt>template</tt> - The template object for the default layout (or nil)
def _default_layout(require_layout = false)
begin
- layout_name = _layout if action_has_layout?
+ layout_name = _layout
rescue NameError => e
raise e, "Could not render layout: #{e.message}"
end
View
28 actionpack/lib/action_controller/test_case.rb
@@ -86,6 +86,21 @@ def assert_template(options = {}, message = nil)
end
end
when Hash
+ if expected_layout = options[:layout]
+ msg = build_message(message,
+ "expecting layout <?> but action rendered <?>",
+ expected_layout, @layouts.keys)
+
+ case expected_layout
+ when String
+ assert(@layouts.keys.include?(expected_layout), msg)
+ when Regexp
+ assert(@layouts.keys.any? {|l| l =~ expected_layout }, msg)
+ when nil
+ assert(@layouts.empty?, msg)
+ end
+ end
+
if expected_partial = options[:partial]
if expected_locals = options[:locals]
actual_locals = @locals[expected_partial.to_s.sub(/^_/,'')]
@@ -98,19 +113,6 @@ def assert_template(options = {}, message = nil)
"expecting ? to be rendered ? time(s) but rendered ? time(s)",
expected_partial, expected_count, actual_count)
assert(actual_count == expected_count.to_i, msg)
- elsif options.key?(:layout)
- msg = build_message(message,
- "expecting layout <?> but action rendered <?>",
- expected_layout, @layouts.keys)
-
- case layout = options[:layout]
- when String
- assert(@layouts.include?(expected_layout), msg)
- when Regexp
- assert(@layouts.any? {|l| l =~ layout }, msg)
- when nil
- assert(@layouts.empty?, msg)
- end
else
msg = build_message(message,
"expecting partial <?> but action rendered <?>",
View
50 actionpack/test/abstract/layouts_test.rb
@@ -141,6 +141,30 @@ def index
end
end
+ class WithOnlyConditional < WithStringImpliedChild
+ layout "overwrite", :only => :show
+
+ def index
+ render :template => ActionView::Template::Text.new("Hello index!")
+ end
+
+ def show
+ render :template => ActionView::Template::Text.new("Hello show!")
+ end
+ end
+
+ class WithExceptConditional < WithStringImpliedChild
+ layout "overwrite", :except => :show
+
+ def index
+ render :template => ActionView::Template::Text.new("Hello index!")
+ end
+
+ def show
+ render :template => ActionView::Template::Text.new("Hello show!")
+ end
+ end
+
class TestBase < ActiveSupport::TestCase
test "when no layout is specified, and no default is available, render without a layout" do
controller = Blank.new
@@ -260,6 +284,30 @@ class ::BadFailLayout < AbstractControllerTests::Layouts::Base
end
end
end
+
+ test "when specify an :only option which match current action name" do
+ controller = WithOnlyConditional.new
+ controller.process(:show)
+ assert_equal "Overwrite Hello show!", controller.response_body
+ end
+
+ test "when specify an :only option which does not match current action name" do
+ controller = WithOnlyConditional.new
+ controller.process(:index)
+ assert_equal "With Implied Hello index!", controller.response_body
+ end
+
+ test "when specify an :except option which match current action name" do
+ controller = WithExceptConditional.new
+ controller.process(:show)
+ assert_equal "With Implied Hello show!", controller.response_body
+ end
+
+ test "when specify an :except option which does not match current action name" do
+ controller = WithExceptConditional.new
+ controller.process(:index)
+ assert_equal "Overwrite Hello index!", controller.response_body
+ end
end
end
-end
+end
View
16 actionpack/test/controller/action_pack_assertions_test.rb
@@ -71,6 +71,10 @@ def render_text_with_custom_content_type
render :text => "Hello!", :content_type => Mime::RSS
end
+ def render_with_layout
+ render "test/hello_world", :layout => "layouts/standard"
+ end
+
def session_stuffing
session['xmas'] = 'turkey'
render :text => "ho ho ho"
@@ -471,6 +475,18 @@ def test_fails_with_incorrect_symbol
end
end
+ def test_fails_with_wrong_layout
+ get :render_with_layout
+ assert_raise(ActiveSupport::TestCase::Assertion) do
+ assert_template :layout => "application"
+ end
+ end
+
+ def test_passes_with_correct_layout
+ get :render_with_layout
+ assert_template :layout => "layouts/standard"
+ end
+
def test_assert_template_reset_between_requests
get :hello_world
assert_template 'test/hello_world'
View
2  actionpack/test/controller/layout_test.rb
@@ -167,7 +167,7 @@ def test_layout_is_not_set_when_none_rendered
def test_layout_is_picked_from_the_controller_instances_view_path
@controller = PrependsViewPathController.new
get :hello
- assert_template :layout => /layouts\/alt\.\w+/
+ assert_template :layout => /layouts\/alt/
end
def test_absolute_pathed_layout
Something went wrong with that request. Please try again.