Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Allow setting a symbol as path in scope on routes #8114

Merged
merged 1 commit into from Nov 22, 2012
Merged

Allow setting a symbol as path in scope on routes #8114

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 22 additions & 0 deletions actionpack/CHANGELOG.md
View file
Open in desktop
@@ -1,5 +1,27 @@
## Rails 4.0.0 (unreleased) ##

* Allow setting a symbol as path in scope on routes. This is now allowed:

scope :api do
resources :users
end

also is possible pass multiple symbols to scope to shorten multiple nested scopes:

scope :api do
scope :v1 do
resources :users
end
end

can be rewritten as:

scope :api, :v1 do
resources :users
end

*Guillermo Iguaran*

* Fix error when using a non-hash query argument named "params" in `url_for`.

Before:
Expand Down
2 changes: 1 addition & 1 deletion actionpack/lib/action_dispatch/routing/mapper.rb
View file
Open in desktop
Expand Up @@ -644,7 +644,7 @@ def scope(*args)
options = args.extract_options!
options = options.dup

options[:path] = args.first if args.first.is_a?(String)
options[:path] = args.flatten.join('/') if args.any?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we expect people to use scope [:v3, :admin]? Otherwise I'd say flatten would not be necessary.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just being defensive - it's not a performance sensitive section of code so it won't do any harm. I was thinking of situations like this:

API.scope = :api # or it could be an array, e.g: [:api, :v1]

scope API.scope do
  # stuff
end

Without the flatten you'd have to splat the arguments, e.g:

scope *Array(API.scope) do
  # stuff
end

But if you want to take it out I'm okay with that 😄

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No problem :). I remember @tenderlove arguing against it somewhere else, because it'd accept hashes like { api: :v1 }.flatten # => [:api, :v1] making it a wider api.

recover = {}

options[:constraints] ||= {}
Expand Down
20 changes: 20 additions & 0 deletions actionpack/test/dispatch/routing_test.rb
View file
Open in desktop
Expand Up @@ -370,6 +370,14 @@ def self.call(params, request)
scope :path => 'api' do
resource :me
get '/' => 'mes#index'
scope :v2 do
resource :me, as: 'v2_me'
get '/' => 'mes#index'
end

scope :v3, :admin do
resource :me, as: 'v3_me'
end
end

get "(/:username)/followers" => "followers#index"
Expand Down Expand Up @@ -1467,6 +1475,18 @@ def test_path_scope
assert_equal 'mes#index', @response.body
end

def test_symbol_scope
get '/api/v2/me'
assert_equal 'mes#show', @response.body
assert_equal '/api/v2/me', v2_me_path

get '/api/v2'
assert_equal 'mes#index', @response.body

get '/api/v3/admin/me'
assert_equal 'mes#show', @response.body
end

def test_url_generator_for_generic_route
get 'whatever/foo/bar'
assert_equal 'foo#bar', @response.body
Expand Down