DavyJonesLocker / client_side_validations

Client Side Validations made easy for Ruby on Rails
MIT License
2.69k stars 403 forks source link

Password confirmation error shows on password field instead of password_confirmation field #630

Closed tkbky closed 7 years ago

tkbky commented 9 years ago

Version of client side validation: master branch Version of rails: 4.2.3

Model validation

class User < ActiveRecord::Base
  validates :password_confirmation, presence: true, on: :create
  validates :password, confirmation: true, presence: true,
                       length: { minimum: 8 }, on: :create
end

Form snippet

  = f.password_field :password
  = f.password_field :password_confirmation

Script tag snippet

"user[password]":{"confirmation":[{"message":"please confirm your password"}],"presence":[{"message":"can't be blank"}],"length":[{"messages":{"minimum":"is too short (minimum is 8 characters)"},"minimum":8}],"password":[{"message":"must be at least 8 alphanumeric characters"}]},"user[password_confirmation]":{"presence":[{"message":"can't be blank"}]}}}
lcguida commented 9 years ago

I've experienced this too. Using master branch and simple_form plugin.

tagliala commented 8 years ago

I do not have a solution for this, feel free to submit a PR with proper tests

tagliala commented 8 years ago

Refers to #53, read throughout that thread, didn't understand what is going on. :)

tagliala commented 8 years ago

The problem is that CSV has no javascript support for this feature.

Brian's code at https://github.com/DavyJonesLocker/client_side_validations/issues/53#issuecomment-8312139 doesn't work.

This code

    # Inputs for confirmations
    $input.filter('[id$=_confirmation]').each ->
      confirmationElement = $(@)
      element = $form.find("##{@id.match(/(.+)_confirmation/)[1]}:input")
      if element[0]
        $("##{confirmationElement.attr('id')}").on(event, binding) for event, binding of {
          'focusout.ClientSideValidations': -> element.data('changed', true).isValid(form.ClientSideValidations.settings.validators)
          'keyup.ClientSideValidations'   : -> element.data('changed', true).isValid(form.ClientSideValidations.settings.validators)
        }

https://github.com/DavyJonesLocker/client_side_validations/blob/master/coffeescript/rails.validations.coffee#L198

runs all validations against the non-confirmation field

Please also note that this should work for each confirmation validation, not only for passwords

I don't know if I'm able to fix this but I will try to

PR with proper unit tests are very welcome.

kyleramirez commented 8 years ago

I replaced ClientSideValidations.validators.local.confirmation with my own. By checking if the user has even tried to put in a confirmation password, you can avoid getting a premature error message.

This works by keeping track of when the user has blurred the confirmation field at least once, and then it manually fires the validation for its relevant field.

$confirmationField =
$element =
handleBlur = ->
  $confirmationField.data "blurredOnce", true
  $element.data "changed", true
  $element.isValid $element[0].form.ClientSideValidations.settings.validators

ClientSideValidations.validators.local.confirmation = (element, options) ->
  $element = element
  $confirmationField = $("#" + (element.attr('id')) + "_confirmation")
  $confirmationField.off("blur",handleBlur).on("blur",handleBlur)
  if ($confirmationField.data("blurredOnce") && $element.val() != $confirmationField.val())
    return options.message
  return
jujudellago commented 8 years ago

kyleramirez, this seems great, but I can't figure out how to use it.... Is it meant to be added to the original coffee version, or added as a separate script ? (I tried both)

kyleramirez commented 8 years ago

@jujudellago, I somewhat followed the gem's documentation and created a custom file at /app/assets/javascripts/rails.validations.customValidatiors.js.coffee ... and then required the file in the asset pipeline in the application.js file. By creating the function ClientSideValidations.validators.local.confirmation ... you essentially override the gem's built-in local confirmation validator. Are you not observing that when you include this snippet?

jujudellago commented 8 years ago

@kyleramirez, all right that make sense now, after a few more tests it does perform better than the original - the validation is performed when the password_confirmed is blurred - but the message is still displayed under the password, I thought it'll come out under the confirmation.

But that will be all right, the real frustration was to have the message "password do not match" before even entering the confirmation.

Thanks a lot for that snipped and explanation !

RavWar commented 8 years ago

If anyone is still looking for a complete solution to both of these problems, you can find it here: https://github.com/RavWar/client_side_validations/commit/cfb149a0441362d9c6b313c93c7eb4ebcdf86ebb.

I'm a bit hesitant to submit PR. There was a need for rails monkey-patching (see https://github.com/rails/rails/pull/8123) which could break in Rails 5 (i have not tested it yet). @tagliala do you think it's worth submitting?

tagliala commented 8 years ago

@RavWar thanks for sharing your solution. We could add a wiki page but I'm against a PR which involves a monkey patch to rails

tagliala commented 7 years ago

Won't fix this, sorry. It is too hard

PR are welcome

vedran commented 6 years ago

Hello.

I have a hacky method that solves this problem for me. I don't have a PR because I think this is too hacky for a general solution.

I included the following javascript into the page that contains my form. Essentially it just checks if the Client Side Validation function is about to add an error to the password field, and adds it to the password_confirmation field instead.

I spent a lot of time trying to get custom validators to solve the issue, but I could not get the correct validation to fire for the confirmation input. Anyway, I'm sure that was my fault, but the following worked for me.

var originalAdd = window.ClientSideValidations.formBuilders['ActionView::Helpers::FormBuilder']['add'];
var originalRemove = window.ClientSideValidations.formBuilders['ActionView::Helpers::FormBuilder']['remove'];

window.ClientSideValidations.formBuilders['ActionView::Helpers::FormBuilder'] = {
    add: function(element, settings, message) {
        // Non i18n friendly hack for indicating non-matching password confirmation 
        var isPasswordField = $(element[0]).attr('id') === 'user_password';
        var isMatchingError = /.*match.*/i.test(message);

        if(isPasswordField && isMatchingError) {
          originalAdd($("#user_password_confirmation"), settings, message)
          message = '' // Clear the message for the password field
        }

        return originalAdd(element, settings, message)
    },

    remove: function(element, settings) {
        // Non i18n friendly hack for indicating non-matching password confirmation 
        var isPasswordField = $(element[0]).attr('id') === 'user_password';

        if(isPasswordField) {
          originalRemove($("#user_password_confirmation"), settings)
        }

        return originalRemove(element, settings)
    }
}

I'm not familiar enough with the library to properly handle the duplicate deletion error in the console, but it doesn't appear to break anything.

IvanRomanovski commented 6 years ago

Building on top of kyleramirez solution for displaying error only after confirmation field has been blurred, here is my solution which displays confirmation feedback messages after confirmation field. Include this code after you import 'rails.validations':

ClientSideValidations.callbacks.element.fail = function(element, message, addError, eventData) {
  // if confirmation field is invalid only show error under confirmation field
  if (message == 'skip'){
    return element[0].form.ClientSideValidations.removeError(element);
  } else {
    // otherwise force rendering of any other errors under field that requires confirmation
    if ($(element[0].form).find("label.message[for='" + (element.attr('id')) + "']")[0] == null) {
      element.data('valid', true);
    }
    return addError();
  }
}

ClientSideValidations.validators.local.confirmation = function(element, options) {
  var $form, $element, $confirmationField;
  $element = element;
  $form = $(element[0].form);
  $confirmationField = $(`#${element.attr('id')}_confirmation`);
  // render error under confirmation field if confirmation field has been blurred
  if ($confirmationField.data("blurredOnce") && ($element.val() !== $confirmationField.val())) {
    if ($confirmationField.data('validConfirmation') !== false) {
      $confirmationField.data('validConfirmation',false);
      $form[0].ClientSideValidations.addError($confirmationField, options.message);
      $confirmationField.focus();
    }
    // do not render an error under password field
    return 'skip';
  } else if ($confirmationField.data('validConfirmation') === false ){
    $confirmationField.data('validConfirmation',true);
    // remove error under confirmation field
    $form[0].ClientSideValidations.removeError($confirmationField);
    $confirmationField.focus();
 }
};

var handleBlur = function() {
  var $confirmationField, $element;
  $confirmationField = $(this);
  $element = $("#" + this.id.replace('_confirmation',''));
  $confirmationField.data("blurredOnce", true);
  $element.data("changed", true);
  return $element.isValid($element[0].form.ClientSideValidations.settings.validators);
}

var attachBlurEventListenersToConfirmations = function() {
  var $forms = $(ClientSideValidations.selectors.forms);
  var $inputs = $forms.find(ClientSideValidations.selectors.inputs);
  var $confirmations = $inputs.filter('[id$=_confirmation]');
  $confirmations.each(function() {
    $(this).on("blur",handleBlur);
  });
}

// attach blur event listeners to confirmation fields
if ((window.Turbolinks != null) && window.Turbolinks.supported) {
  var initializeOnEvent = window.Turbolinks.EVENTS != null ? 'page:change' : 'turbolinks:load';
  $(document).on(initializeOnEvent, function() {
    attachBlurEventListenersToConfirmations();
  });
} else {
  $(function() {
    attachBlurEventListenersToConfirmations();
  });
}