Merge pull request #13470 from strzalek/variants-all-any

Add any/all support for variants

5 files changed, 217 insertions, 27 deletions

diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
index 2490282d6e..24f36e2496 100644
--- a/actionpack/CHANGELOG.md
+++ b/actionpack/CHANGELOG.md
@@ -99,6 +99,24 @@
           format.html.none  { render "trash" }
         end
 
+    Variants also support common `any`/`all` block that formats have.
+
+    It works for both inline:
+
+        respond_to do |format|
+          format.html.any   { render text: "any"   }
+          format.html.phone { render text: "phone" }
+        end
+
+    and block syntax:
+
+        respond_to do |format|
+          format.html do |variant|
+            variant.any(:tablet, :phablet){ render text: "any" }
+            variant.phone { render text: "phone" }
+          end
+        end
+
     *Łukasz Strzałkowski*
 
 *   Fix render of localized templates without an explicit format using wrong
diff --git a/actionpack/lib/action_controller/metal/mime_responds.rb b/actionpack/lib/action_controller/metal/mime_responds.rb
index fbc4024c2d..d5e08b7034 100644
--- a/actionpack/lib/action_controller/metal/mime_responds.rb
+++ b/actionpack/lib/action_controller/metal/mime_responds.rb
@@ -217,6 +217,24 @@ module ActionController #:nodoc:
     #     format.html.phone { redirect_to progress_path }
     #     format.html.none  { render "trash" }
     #   end
+    # 
+    # Variants also support common `any`/`all` block that formats have.
+    #
+    # It works for both inline:
+    #
+    #   respond_to do |format|
+    #     format.html.any   { render text: "any"   }
+    #     format.html.phone { render text: "phone" }
+    #   end
+    #
+    # and block syntax:
+    #
+    #   respond_to do |format|
+    #     format.html do |variant|
+    #       variant.any(:tablet, :phablet){ render text: "any" }
+    #       variant.phone { render text: "phone" }
+    #     end
+    #   end
     #
     # Be sure to check the documentation of +respond_with+ and
     # <tt>ActionController::MimeResponds.respond_to</tt> for more examples.
@@ -224,7 +242,7 @@ module ActionController #:nodoc:
       raise ArgumentError, "respond_to takes either types or a block, never both" if mimes.any? && block_given?
 
       if collector = retrieve_collector_from_mimes(mimes, &block)
-        response = collector.response(request.variant)
+        response = collector.response
         response ? response.call : render({})
       end
     end
@@ -366,7 +384,7 @@ module ActionController #:nodoc:
       if collector = retrieve_collector_from_mimes(&block)
         options = resources.size == 1 ? {} : resources.extract_options!
         options = options.clone
-        options[:default_response] = collector.response(request.variant)
+        options[:default_response] = collector.response
         (options.delete(:responder) || self.class.responder).call(self, resources, options)
       end
     end
@@ -399,7 +417,7 @@ module ActionController #:nodoc:
     # is available.
     def retrieve_collector_from_mimes(mimes=nil, &block) #:nodoc:
       mimes ||= collect_mimes_from_class_level
-      collector = Collector.new(mimes)
+      collector = Collector.new(mimes, request.variant)
       block.call(collector) if block_given?
       format = collector.negotiate_format(request)
 
@@ -437,8 +455,9 @@ module ActionController #:nodoc:
       include AbstractController::Collector
       attr_accessor :format
 
-      def initialize(mimes)
+      def initialize(mimes, variant = nil)
         @responses = {}
+        @variant = variant
 
         mimes.each { |mime| @responses["Mime::#{mime.upcase}".constantize] = nil }
       end
@@ -457,18 +476,20 @@ module ActionController #:nodoc:
         @responses[mime_type] ||= if block_given?
           block
         else
-          VariantCollector.new
+          VariantCollector.new(@variant)
         end
       end
 
-      def response(variant)
+      def response
         response = @responses.fetch(format, @responses[Mime::ALL])
-        if response.is_a?(VariantCollector)
-          response.variant(variant)
-        elsif response.nil? || response.arity == 0
+        if response.is_a?(VariantCollector) # `format.html.phone` - variant inline syntax
+          response.variant
+        elsif response.nil? || response.arity == 0 # `format.html` - just a format, call its block
           response
-        else
-          lambda { response.call VariantFilter.new(variant) }
+        else # `format.html{ |variant| variant.phone }` - variant block syntax
+          variant_collector = VariantCollector.new(@variant)
+          response.call(variant_collector) #call format block with variants collector
+          variant_collector.variant
         end
       end
 
@@ -476,31 +497,37 @@ module ActionController #:nodoc:
         @format = request.negotiate_mime(@responses.keys)
       end
 
-      #Used for inline syntax
       class VariantCollector #:nodoc:
-        def initialize
+        def initialize(variant = nil)
+          @variant = variant
           @variants = {}
         end
 
-        def method_missing(name, *args, &block)
-          @variants[name] = block if block_given?
-        end
-
-        def variant(name)
-          @variants[name.nil? ? :none : name]
+        def any(*args, &block)
+          if block_given?
+            if args.any? && args.none?{ |a| a == @variant }
+              args.each{ |v| @variants[v] = block }
+            else
+              @variants[:any] = block
+            end
+          end
         end
-      end
+        alias :all :any
 
-      #Used for nested block syntax
-      class VariantFilter #:nodoc:
-        def initialize(variant)
-          @variant = variant
+        def method_missing(name, *args, &block)
+          @variants[name] = block if block_given?
         end
 
-        def method_missing(name)
-          if block_given?
-            yield if name == @variant || (name == :none && @variant.nil?)
+        def variant
+          key = if @variant.nil?
+            :none
+          elsif @variants.has_key?(@variant)
+            @variant
+          else
+            :any
           end
+
+          @variants[key]
         end
       end
     end
diff --git a/actionpack/test/controller/mime/respond_to_test.rb b/actionpack/test/controller/mime/respond_to_test.rb
index d84eb5790d..84e4936f31 100644
--- a/actionpack/test/controller/mime/respond_to_test.rb
+++ b/actionpack/test/controller/mime/respond_to_test.rb
@@ -191,6 +191,61 @@ class RespondToController < ActionController::Base
     end
   end
 
+  def variant_any
+    respond_to do |format|
+      format.html do |variant|
+        variant.any(:tablet, :phablet){ render text: "any" }
+        variant.phone { render text: "phone" }
+      end
+    end
+  end
+
+  def variant_any_any
+    respond_to do |format|
+      format.html do |variant|
+        variant.any   { render text: "any"   }
+        variant.phone { render text: "phone" }
+      end
+    end
+  end
+
+  def variant_inline_any
+    respond_to do |format|
+      format.html.any(:tablet, :phablet){ render text: "any" }
+      format.html.phone { render text: "phone" }
+    end
+  end
+
+  def variant_inline_any_any
+    respond_to do |format|
+      format.html.phone { render text: "phone" }
+      format.html.any   { render text: "any"   }
+    end
+  end
+
+  def variant_any_implicit_render
+    respond_to do |format|
+      format.html.phone
+      format.html.any(:tablet, :phablet)
+    end
+  end
+
+  def variant_any_with_none
+    respond_to do |format|
+      format.html.any(:none, :phone){ render text: "none or phone" }
+    end
+  end
+
+  def format_any_variant_any
+    respond_to do |format|
+      format.html { render text: "HTML" }
+      format.any(:js, :xml) do |variant|
+        variant.phone{ render text: "phone" }
+        variant.any(:tablet, :phablet){ render text: "tablet" }
+      end
+    end
+  end
+
   protected
     def set_layout
       case action_name
@@ -597,4 +652,92 @@ class RespondToControllerTest < ActionController::TestCase
     assert_equal "text/html", @response.content_type
     assert_equal "phone", @response.body
   end
+
+  def test_variant_any
+    @request.variant = :phone
+    get :variant_any
+    assert_equal "text/html", @response.content_type
+    assert_equal "phone", @response.body
+
+    @request.variant = :tablet
+    get :variant_any
+    assert_equal "text/html", @response.content_type
+    assert_equal "any", @response.body
+
+    @request.variant = :phablet
+    get :variant_any
+    assert_equal "text/html", @response.content_type
+    assert_equal "any", @response.body
+  end
+
+  def test_variant_any_any
+    @request.variant = :phone
+    get :variant_any_any
+    assert_equal "text/html", @response.content_type
+    assert_equal "phone", @response.body
+
+    @request.variant = :yolo
+    get :variant_any_any
+    assert_equal "text/html", @response.content_type
+    assert_equal "any", @response.body
+  end
+
+  def test_variant_inline_any
+    @request.variant = :phone
+    get :variant_any
+    assert_equal "text/html", @response.content_type
+    assert_equal "phone", @response.body
+
+    @request.variant = :tablet
+    get :variant_inline_any
+    assert_equal "text/html", @response.content_type
+    assert_equal "any", @response.body
+
+    @request.variant = :phablet
+    get :variant_inline_any
+    assert_equal "text/html", @response.content_type
+    assert_equal "any", @response.body
+  end
+
+  def test_variant_inline_any_any
+    @request.variant = :phone
+    get :variant_inline_any_any
+    assert_equal "text/html", @response.content_type
+    assert_equal "phone", @response.body
+
+    @request.variant = :yolo
+    get :variant_inline_any_any
+    assert_equal "text/html", @response.content_type
+    assert_equal "any", @response.body
+  end
+
+  def test_variant_any_implicit_render
+    @request.variant = :tablet
+    get :variant_any_implicit_render
+    assert_equal "text/html", @response.content_type
+    assert_equal "tablet", @response.body
+
+    @request.variant = :phablet
+    get :variant_any_implicit_render
+    assert_equal "text/html", @response.content_type
+    assert_equal "phablet", @response.body
+  end
+
+  def test_variant_any_with_none
+    get :variant_any_with_none
+    assert_equal "text/html", @response.content_type
+    assert_equal "none or phone", @response.body
+
+    @request.variant = :phone
+    get :variant_any_with_none
+    assert_equal "text/html", @response.content_type
+    assert_equal "none or phone", @response.body
+  end
+
+  def test_format_any_variant_any
+    @request.variant = :tablet
+    get :format_any_variant_any, format: :js
+    assert_equal "text/javascript", @response.content_type
+    assert_equal "tablet", @response.body
+  end
 end
diff --git a/actionpack/test/fixtures/respond_to/variant_any_implicit_render.html+phablet.erb b/actionpack/test/fixtures/respond_to/variant_any_implicit_render.html+phablet.erb
new file mode 100644
index 0000000000..e905d051bf
--- /dev/null
+++ b/actionpack/test/fixtures/respond_to/variant_any_implicit_render.html+phablet.erb
@@ -0,0 +1 @@
+phablet
\ No newline at end of file
diff --git a/actionpack/test/fixtures/respond_to/variant_any_implicit_render.html+tablet.erb b/actionpack/test/fixtures/respond_to/variant_any_implicit_render.html+tablet.erb
new file mode 100644
index 0000000000..65526af8cf
--- /dev/null
+++ b/actionpack/test/fixtures/respond_to/variant_any_implicit_render.html+tablet.erb
@@ -0,0 +1 @@
+tablet
\ No newline at end of file