Navigation Menu

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

#50 fix undefined method `args' in token.rb when calling to_param but to... #51

Merged
merged 2 commits into from Aug 20, 2014

Conversation

warmwind
Copy link
Contributor

fix undefined method `args' in token.rb when calling to_param but token is nil

@@ -56,6 +56,12 @@ class UntaintedDocument
expect(document.to_param).to_not eq document.token
end

it "should return id when token does not exist when calling `to_param`" do
document_class.send(:token, :override_to_param => true)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use the new Ruby 1.9 hash syntax.

@thetron
Copy link
Owner

thetron commented Aug 20, 2014

@warmwind Thanks heaps! I think we can ignore @houndci's comments, since the rest of the gem uses the pre-1.9 hash syntax.

I'll get this merged in as soon as I get a moment to take a proper look! Thanks! ☕

@warmwind
Copy link
Contributor Author

@thetron Cheers!
I got this problem in our project just now after upgrade, because in some scenario, we need to batch insert data into mongdb without token generated.

@thetron
Copy link
Owner

thetron commented Aug 20, 2014

Having a look over this again, I think passing args in to_param was a mistake in the earlier version. I don't quite know why it's even there.

Do you think you could change it to just be super() (and remove the changes made to override_to_param)? Unless you can see a reason why it would need to be there.

@@ -54,7 +54,7 @@ def set_token_callbacks(options)

def override_to_param(options)
self.send(:define_method, :to_param) do
self.send(options.field_name) || super(*args)
self.send(options.field_name) || super()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Redundant self detected.

@warmwind
Copy link
Contributor Author

@thetron I did NOT have any specific reason to pass the args, but just followed the previous version.
Already removed.

@thetron
Copy link
Owner

thetron commented Aug 20, 2014

That's awesome! Thanks @warmwind!

thetron added a commit that referenced this pull request Aug 20, 2014
#50 fix undefined method `args' in token.rb when calling to_param but to...
@thetron thetron merged commit dd39fb6 into thetron:master Aug 20, 2014
@thetron
Copy link
Owner

thetron commented Aug 21, 2014

Just released 2.1.1, which includes this fix. 🍻

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

3 participants