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.

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

CookieJar#deleted? predicate method #4613

Merged
merged 1 commit into from
Jan 23, 2012
Merged

CookieJar#deleted? predicate method #4613

merged 1 commit into from
Jan 23, 2012

Conversation

pda
Copy link
Contributor

@pda pda commented Jan 23, 2012

ActionDispatch::Cookies::CookieJar needs a way to query for deleted cookies.

Here's an example use case from a controller test:

get :destroy
assert cookies.deleted? "auth_token"

Cheers!
Paul

Necessary in controller tests to determine if the CookieJar will delete
the given cookie.
@@ -180,8 +180,7 @@ def recycle!
@env['action_dispatch.request.query_parameters'] = {}
@set_cookies ||= {}
@set_cookies.update(Hash[cookie_jar.instance_variable_get("@set_cookies").map{ |k,o| [k,o[:value]] }])
deleted_cookies = cookie_jar.instance_variable_get("@delete_cookies")
@set_cookies.reject!{ |k,v| deleted_cookies.include?(k) }
@set_cookies.reject!{ |k,v| cookie_jar.deleted?(k) }
Copy link
Contributor

Choose a reason for hiding this comment

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

These are not exactly equivalent. The first one would delete any cookie regardless of the path or the domain, while the second one won't.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. I'll eliminate that commit, leaving just the addition of the #deleted? method.

@pda
Copy link
Contributor Author

pda commented Jan 23, 2012

Removed change to actionpack/lib/action_controller/test_case.rb as @josevalim rightly points out using #deleted? wasn't exactly equivalent to the method using instance_variable_get.

@josevalim
Copy link
Contributor

Another option is to have deleted?(key) check if they key was deleted regardless of the options or add another method for that. Better than the instance variable get check.

@pda
Copy link
Contributor Author

pda commented Jan 23, 2012

I was initially reluctant to have #deleted? disregard the cookie scope options, but I can't really see a problem with def deleted?(key) not taking an options hash.

josevalim added a commit that referenced this pull request Jan 23, 2012
@josevalim josevalim merged commit adb7568 into rails:master Jan 23, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants