Skip to content

Loading…

resource and resources do no longer modify passed options #7789

Merged
merged 1 commit into from

3 participants

@senny
Ruby on Rails member

this is a patch for #7777. I could not modify the shared apply_common_behavior_for method because the outer methods rely on the fact, that those keys are deleted. Adding the dup call inside apply_common_behavior_for resultated in 8 error when running the tests.

I added two test cases to make sure the options are not modified.

@senny
Ruby on Rails member

@schneems, @rafaelfranca I was not sure if I need to add a Changelog entry for this subtle fix. Please tell me if I should add one.

@steveklabnik
Ruby on Rails member

Bugfixes generally don't get a CHANGELOG entry, so you're good.

@rafaelfranca
Ruby on Rails member

@steveklabnik we changed the policy to CHANGELOG entries and bugfixes get an entry too.

@rafaelfranca
Ruby on Rails member

Seems good. Could you add a CHANGELOG entry?

@senny
Ruby on Rails member

@rafaelfranca thanks for the explanation. I added the entry.

@rafaelfranca rafaelfranca commented on an outdated diff
actionpack/CHANGELOG.md
@@ -1,5 +1,9 @@
## Rails 4.0.0 (unreleased) ##
+* Fix #7777, `resource` and `resources` don't modify the passed options hash
@rafaelfranca Ruby on Rails member

Change the entry to:

`resource` and `resources` don't modify the passed options hash.
Fix #7777
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@rafaelfranca
Ruby on Rails member

We need a rebase

@senny
Ruby on Rails member
@rafaelfranca rafaelfranca merged commit d0ad97b into rails:master
@rafaelfranca
Ruby on Rails member

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Oct 1, 2012
  1. @senny

    resource and resources do no longer modify passed options

    senny committed with senny
    this is a patch for #7777.
View
5 actionpack/CHANGELOG.md
@@ -1,5 +1,10 @@
## Rails 4.0.0 (unreleased) ##
+* `resource` and `resources` don't modify the passed options hash
+ Fix #7777
+
+ *Yves Senn*
+
* Precompiled assets include aliases from foo.js to foo/index.js and vice versa.
# Precompiles phone-<digest>.css and aliases phone/index.css to phone.css.
View
4 actionpack/lib/action_dispatch/routing/mapper.rb
@@ -1038,7 +1038,7 @@ def resources_path_names(options)
# === Options
# Takes same options as +resources+.
def resource(*resources, &block)
- options = resources.extract_options!
+ options = resources.extract_options!.dup
if apply_common_behavior_for(:resource, resources, options, &block)
return self
@@ -1204,7 +1204,7 @@ def resource(*resources, &block)
# # resource actions are at /admin/posts.
# resources :posts, :path => "admin/posts"
def resources(*resources, &block)
- options = resources.extract_options!
+ options = resources.extract_options!.dup
if apply_common_behavior_for(:resources, resources, options, &block)
return self
View
20 actionpack/test/dispatch/routing_test.rb
@@ -1124,6 +1124,26 @@ def test_resources_for_uncountable_names
assert_equal '/sheep/1/_it', _it_sheep_path(1)
end
+ def test_resource_does_not_modify_passed_options
+ options = {:id => /.+?/, :format => /json|xml/}
+ self.class.stub_controllers do |routes|
+ routes.draw do
+ resource :user, options
+ end
+ end
+ assert_equal({:id => /.+?/, :format => /json|xml/}, options)
+ end
+
+ def test_resources_does_not_modify_passed_options
+ options = {:id => /.+?/, :format => /json|xml/}
+ self.class.stub_controllers do |routes|
+ routes.draw do
+ resources :users, options
+ end
+ end
+ assert_equal({:id => /.+?/, :format => /json|xml/}, options)
+ end
+
def test_path_names
get '/pt/projetos'
assert_equal 'projects#index', @response.body
Something went wrong with that request. Please try again.