[PATCH] validates_confirmation_of doesn't fail when confirmation not present

I submitted patch #348 which was committed in revision [241] to only check to see if field_confirmation matched field if field_confirmation was non-nil. This allows one to write scripts outside the web world when confirmation isn't really necessary (for instance, automatically adding users when you know the password already and it's not going to change from u.password= to u.password_confirmation=). Perhaps this needs another look. I definitely think the script-friendly functionality must be maintained.

Yea, good point Dan. Perhaps we should require it to be present if new_record?

Whoa this ticket is old. I'm expiring it because I don't care any more, but I will say that writing

u.password = u.password_confirmation = ENVPASSWORD? is not really that painful.

Hey there. I think

u.password = u.password_confirmation = ENV[PASSWORD]

is missing the point. Yes, you can do that to get around the password_confirmation requirement in a script. But that's not the issue here. The issue is that this behavior is unexpected, at best, and probably incorrect.

The attached patch requires a confirmation of the attribute, unless the attribute is nil, regardless of whether the confirmation is nil. Test updated to match.

...probably incorrect...

Heh. Well, unexpected for sure.

Seems ok, but I get a validation test failure with the patch. Could you update it?

I just ran it again with rake test_mysql from the rails/activerecord directory (and it worked). Should I be doing something different?

I think a reasonable thing to do here would be to support allow_nil, like most other validators. (actually I didn't check if it works)

So when you have validates_confirmation_of :foo, :allow_nil => true it'll ignore the confirmation when it's nil, otherwise validate it.

Eminently reasonable. Anybody else?

I can probably get to it in a few days, unless somebody else wants to slap it together.

(In [4917]) validates_confirmation_of only kicks in when the attribute, rather than its confirmation, is present. Closes #785.

I find this change very annoying. Things like this don't work anymore:

class User < ActiveRecord::Base
  validates_confirmation_of :email
end

user = User.find(1) # already exists
user.something_not_email = 'hi'
user.save #=> false, email not confirmed

Why should email be confirmed if it wasn't changed? This goes directly against my first comment to this ticket about ease of use in scripts. Yeah, I could use :if to only make the confirmation fire if email_confirmation is present, but...that's how it used to work.

I understand web interaction is the common case here but when this breaks simple one-attribute changes I think it goes too far.

I've got to agree with Dan here. This breaks current functionality _and_ adds stupid hurdles to simple changes.

Hi Dan and Kevin,

Would using the :on => :create option for validates_confirmation_of ease the pain? I ask because you both refer to changes.

It would, yes, but it still has problems. If I have an edit user page and the email is able to be changed I'll want that change confirmed. But if I've got a pre-set list of email changes and I'm processing them with a script I don't care so much about confirmation.

How about this for a solution: revert validates_confirmation_of to how it was before. If the presence of confirmation is required, use validates_presence_of on the dynamic attribute. Example at http://pastie.caboo.se/11943. Documentation can then be added to validates_confirmation_of explaining what to do if the confirmation must always (or selectively) be present.

I think this is a good solution. The previous functionality is restored and what's being done (when confirmation is required) is more verbose which I think it should be.

Sounds good Nick. +1

Dan, I see the rub.

Anonymous, that flip-flops the problem. It seems like validates_confirmation_of might be a subtle violation of MVC, since the _confirmation field isn't really part of the data model (I think this is the only validates_* that uses a dummy field?). Is this something the controller could take care of? It seems like it's pertinent only when gathering input from a view.

validates_acceptance_of uses a virtual field as well

Until a "proper" solution comes around, the patch should be reverted. Not sure how to handle both contexts, the web context which is what confirmation_of and acceptance_of are designed for, and the ability to work with the model outside of a web context.

+1 on reverting the patch too. Broke an app I have when upgrading from 1.1.6 to edge in preparation for 1.2 and I bet I won't be the only one this happens to.

In the long term, this behavior doesn't really belong in the model. I'd like to remove it in or before Rails 2.0.

Reassigning to bitsweat since as is hinted above, I don't use this.

(In [5067]) Rollback [4917]. Closes #785.

Probably going to hate me for reopening this, but currently in r7190 for example the main part of the validates_confirmation_of has this

record.errors.add(attr_name, configuration[:message]) unless 
  record.send("#{attr_name}_confirmation").nil? or 
    value == record.send("#{attr_name}_confirmation")

yet surely you would still want the validation to fire if and add an error if the say password_confirmation were missing (i.e. .nil?).

Does this sit with what everyone else expects or is it expected that if password_confirmation were missing the validation would not fire at all.

essentially I am echo'ing what Caio Chassot said earlier

Adds :allow_nil option to validates_confirmation_of and tests