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

Dis-allow nil confirmations. #9692

Closed

Conversation

steveklabnik
Copy link
Member

It doesn't make sense that if we don't confirm, we allow things
to be saved.

I'm submitting this as a PR because there is one test that implies this behavior was expected, so I'm curious if this was an oversight or if this is expected behavior.

It doesn't make sense that if we don't confirm, we allow things
to be saved.
@steveklabnik
Copy link
Member Author

Apparently @josevalim has something to say about this.

@josevalim
Copy link
Contributor

This behavior is intentional. The goal of validations is too confirm user
input. A form will always send a blank value if the field is there, so the
validation is triggered.

Now, on console and apis, you don't need to set the field, exactly because
it accepts nil.

Therefore this is a feature, there is a post on plataformatec blog, but I'm
on my phone, so you will have to google it. :)

This topic comes once in a while, it would be worthy to add it to the
documentation. If you want to really not accept nil, add a presence check.

José Valim
www.plataformatec.com.br
Skype: jv.ptec
Founder and Lead Developer

@steveklabnik
Copy link
Member Author

I will add some documentation, I think this behavior is pretty terrible, but if that's the way it is, that's the way it is.

@tonycoco
Copy link
Contributor

Agreed. This is a botched way to handle nils from a form submission. Maybe there's a better solution to pop those nils out of the form's resource params instead?

@steveklabnik steveklabnik deleted the has_secure_password_fix branch March 13, 2013 22:14
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