From 4037e31d8874250e485ca6a27bd792a3beb13f76 Mon Sep 17 00:00:00 2001 From: Ernie Miller Date: Wed, 22 Aug 2012 10:27:42 -0400 Subject: Allow routing concerns to accept a callable This allows us to make alterations to the generated routes based on the scope of the current mapper, and otherwise allows us to move larger blocks of concerns out of the routes file, altogether. --- actionpack/lib/action_dispatch/routing/mapper.rb | 10 +++++--- actionpack/test/abstract_unit.rb | 1 + actionpack/test/dispatch/routing/concerns_test.rb | 28 +++++++++++++++++++++-- 3 files changed, 34 insertions(+), 5 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index b52f66faf1..b1abbbe505 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -1613,8 +1613,8 @@ module ActionDispatch # end # # Any routing helpers can be used inside a concern. - def concern(name, &block) - @concerns[name] = block + def concern(name, callable = nil, &block) + @concerns[name] = callable || block end # Use the named concerns @@ -1631,7 +1631,7 @@ module ActionDispatch def concerns(*names) names.flatten.each do |name| if concern = @concerns[name] - instance_eval(&concern) + concern.call(self) else raise ArgumentError, "No concern named #{name} was found!" end @@ -1645,6 +1645,10 @@ module ActionDispatch @concerns = {} end + def current_scope + @scope + end + include Base include HttpHelpers include Redirection diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb index e5054a9eb8..4f5b2895c9 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -358,6 +358,7 @@ end class ThreadsController < ResourcesController; end class MessagesController < ResourcesController; end class CommentsController < ResourcesController; end +class ReviewsController < ResourcesController; end class AuthorsController < ResourcesController; end class LogosController < ResourcesController; end diff --git a/actionpack/test/dispatch/routing/concerns_test.rb b/actionpack/test/dispatch/routing/concerns_test.rb index 21da3bd77a..8600da32ad 100644 --- a/actionpack/test/dispatch/routing/concerns_test.rb +++ b/actionpack/test/dispatch/routing/concerns_test.rb @@ -1,6 +1,16 @@ require 'abstract_unit' class RoutingConcernsTest < ActionDispatch::IntegrationTest + class Reviewable + def self.call(mapper) + if mapper.current_scope[:controller] == 'posts' + mapper.resources :reviews + elsif mapper.current_scope[:controller] == 'videos' + mapper.resources :reviews, as: :video_reviews + end + end + end + Routes = ActionDispatch::Routing::RouteSet.new.tap do |app| app.draw do concern :commentable do @@ -11,8 +21,10 @@ class RoutingConcernsTest < ActionDispatch::IntegrationTest resources :images, only: :index end - resources :posts, concerns: [:commentable, :image_attachable] do - resource :video, concerns: :commentable + concern :reviewable, Reviewable + + resources :posts, concerns: [:commentable, :image_attachable, :reviewable] do + resource :video, concerns: [:commentable, :reviewable] end resource :picture, concerns: :commentable do @@ -63,6 +75,18 @@ class RoutingConcernsTest < ActionDispatch::IntegrationTest assert_equal "404", @response.code end + def test_accessing_callable_concern_from_resources + get "/posts/1/reviews/1" + assert_equal "200", @response.code + assert_equal "/posts/1/reviews/1", post_review_path(post_id: 1, id: 1) + end + + def test_callable_concern_can_adapt_to_mapper + get "/posts/1/video/reviews/1" + assert_equal "200", @response.code + assert_equal "/posts/1/video/reviews/1", post_video_video_review_path(post_id: 1, id: 1) + end + def test_accessing_concern_from_a_scope get "/videos/comments" assert_equal "200", @response.code -- cgit v1.2.3 From eb43d3d1d94c67b3bf9c0cf576cdae8380f27260 Mon Sep 17 00:00:00 2001 From: Ernie Miller Date: Thu, 23 Aug 2012 09:02:37 -0400 Subject: Fix concerns not executing block in mapper Also, add documentation for alternate usage. --- actionpack/lib/action_dispatch/routing/mapper.rb | 21 +++++++++++++++++++-- actionpack/test/dispatch/routing/concerns_test.rb | 10 ++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index b1abbbe505..8573f4d80b 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -1606,15 +1606,32 @@ module ActionDispatch # concerns :commentable # end module Concerns - # Define a routing concern using a name. + # Define a routing concern using a name. If a second parameter is + # supplied, it should respond to call, which will receive the mapper + # as a parameter, allowing for customized behavior based on the current + # scope. # # concern :commentable do # resources :comments # end # + # # - or - + # + # class Commentable + # def self.call(mapper) + # if mapper.current_scope[:controller] == 'videos' + # mapper.resources :video_comments, as: :comments + # else + # mapper.resources :comments + # end + # end + # end + # + # concern :commentable, Commentable + # # Any routing helpers can be used inside a concern. def concern(name, callable = nil, &block) - @concerns[name] = callable || block + @concerns[name] = callable || lambda { |m| m.instance_eval(&block) } end # Use the named concerns diff --git a/actionpack/test/dispatch/routing/concerns_test.rb b/actionpack/test/dispatch/routing/concerns_test.rb index 8600da32ad..0289f38ba7 100644 --- a/actionpack/test/dispatch/routing/concerns_test.rb +++ b/actionpack/test/dispatch/routing/concerns_test.rb @@ -103,4 +103,14 @@ class RoutingConcernsTest < ActionDispatch::IntegrationTest assert_equal "No concern named foo was found!", e.message end + + def test_concerns_executes_block_in_context_of_current_mapper + mapper = ActionDispatch::Routing::Mapper.new(ActionDispatch::Routing::RouteSet.new) + mapper.concern :test_concern do + resources :things + return self + end + + assert_equal mapper, mapper.concerns(:test_concern) + end end -- cgit v1.2.3 From 05136e5c0b3d7b841bdec53847879321309604d3 Mon Sep 17 00:00:00 2001 From: Ernie Miller Date: Sun, 26 Aug 2012 15:37:23 -0400 Subject: Make enhanced routing Concerns more tell-don't-ask --- actionpack/lib/action_dispatch/routing/mapper.rb | 80 ++++++++++++++++------- actionpack/test/dispatch/routing/concerns_test.rb | 27 ++++---- 2 files changed, 70 insertions(+), 37 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 8573f4d80b..ddb34a2394 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -1585,7 +1585,7 @@ module ActionDispatch end end - # Routing Concerns allows you to declare common routes that can be reused + # Routing Concerns allow you to declare common routes that can be reused # inside others resources and routes. # # concern :commentable do @@ -1606,32 +1606,65 @@ module ActionDispatch # concerns :commentable # end module Concerns - # Define a routing concern using a name. If a second parameter is - # supplied, it should respond to call, which will receive the mapper - # as a parameter, allowing for customized behavior based on the current - # scope. + # Define a routing concern using a name. # - # concern :commentable do - # resources :comments + # Concerns may be defined inline, using a block, or handled by + # another object, by passing that object as the second parameter. + # + # The concern object, if supplied, should respond to call, + # which will receive two parameters: + # + # * The current mapper + # * A hash of options which the concern object may use + # + # Options may also be used by concerns defined in a block by accepting + # a block parameter. So, using a block, you might do something as + # simple as limit the actions available on certain resources, passing + # standard resource options through the concern: + # + # concern :commentable do |options| + # resources :comments, options + # end + # + # resources :posts, concerns: :commentable + # resources :archived_posts do + # # Don't allow comments on archived posts + # concerns :commentable, only: [:index, :show] # end # - # # - or - + # Or, using a callable object, you might implement something more + # specific to your application, which would be out of place in your + # routes file. # - # class Commentable - # def self.call(mapper) - # if mapper.current_scope[:controller] == 'videos' - # mapper.resources :video_comments, as: :comments - # else - # mapper.resources :comments - # end + # # purchasable.rb + # class Purchasable + # def initialize(defaults = {}) + # @defaults = defaults + # end + # + # def call(mapper, options = {}) + # options = @defaults.merge(options) + # mapper.resources :purchases + # mapper.resources :receipts + # mapper.resources :returns if options[:returnable] # end # end # - # concern :commentable, Commentable + # # routes.rb + # concern :purchasable, Purchasable.new(returnable: true) + # + # resources :toys, concerns: :purchasable + # resources :electronics, concerns: :purchasable + # resources :pets do + # concerns :purchasable, returnable: false + # end # - # Any routing helpers can be used inside a concern. + # Any routing helpers can be used inside a concern. If using a + # callable, they're accessible from the Mapper that's passed to + # call. def concern(name, callable = nil, &block) - @concerns[name] = callable || lambda { |m| m.instance_eval(&block) } + callable ||= lambda { |mapper, options| mapper.instance_exec(options, &block) } + @concerns[name] = callable end # Use the named concerns @@ -1645,10 +1678,11 @@ module ActionDispatch # namespace :posts do # concerns :commentable # end - def concerns(*names) - names.flatten.each do |name| + def concerns(*args) + options = args.extract_options! + args.flatten.each do |name| if concern = @concerns[name] - concern.call(self) + concern.call(self, options) else raise ArgumentError, "No concern named #{name} was found!" end @@ -1662,10 +1696,6 @@ module ActionDispatch @concerns = {} end - def current_scope - @scope - end - include Base include HttpHelpers include Redirection diff --git a/actionpack/test/dispatch/routing/concerns_test.rb b/actionpack/test/dispatch/routing/concerns_test.rb index 0289f38ba7..9f37701656 100644 --- a/actionpack/test/dispatch/routing/concerns_test.rb +++ b/actionpack/test/dispatch/routing/concerns_test.rb @@ -2,19 +2,15 @@ require 'abstract_unit' class RoutingConcernsTest < ActionDispatch::IntegrationTest class Reviewable - def self.call(mapper) - if mapper.current_scope[:controller] == 'posts' - mapper.resources :reviews - elsif mapper.current_scope[:controller] == 'videos' - mapper.resources :reviews, as: :video_reviews - end + def self.call(mapper, options = {}) + mapper.resources :reviews, options end end Routes = ActionDispatch::Routing::RouteSet.new.tap do |app| app.draw do - concern :commentable do - resources :comments + concern :commentable do |options| + resources :comments, options end concern :image_attachable do @@ -24,7 +20,9 @@ class RoutingConcernsTest < ActionDispatch::IntegrationTest concern :reviewable, Reviewable resources :posts, concerns: [:commentable, :image_attachable, :reviewable] do - resource :video, concerns: [:commentable, :reviewable] + resource :video, concerns: :commentable do + concerns :reviewable, as: :video_reviews + end end resource :picture, concerns: :commentable do @@ -32,7 +30,7 @@ class RoutingConcernsTest < ActionDispatch::IntegrationTest end scope "/videos" do - concerns :commentable + concerns :commentable, except: :destroy end end end @@ -75,13 +73,13 @@ class RoutingConcernsTest < ActionDispatch::IntegrationTest assert_equal "404", @response.code end - def test_accessing_callable_concern_from_resources + def test_accessing_callable_concern_ get "/posts/1/reviews/1" assert_equal "200", @response.code assert_equal "/posts/1/reviews/1", post_review_path(post_id: 1, id: 1) end - def test_callable_concern_can_adapt_to_mapper + def test_callable_concerns_accept_options get "/posts/1/video/reviews/1" assert_equal "200", @response.code assert_equal "/posts/1/video/reviews/1", post_video_video_review_path(post_id: 1, id: 1) @@ -92,6 +90,11 @@ class RoutingConcernsTest < ActionDispatch::IntegrationTest assert_equal "200", @response.code end + def test_concerns_accept_options + delete "/videos/comments/1" + assert_equal "404", @response.code + end + def test_with_an_invalid_concern_name e = assert_raise ArgumentError do ActionDispatch::Routing::RouteSet.new.tap do |app| -- cgit v1.2.3