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

require_from_group, skip_or_fill_minimum disables other rules #412

Closed
johnthebiker opened this issue May 7, 2012 · 26 comments · Fixed by #873
Closed

require_from_group, skip_or_fill_minimum disables other rules #412

johnthebiker opened this issue May 7, 2012 · 26 comments · Fixed by #873

Comments

@johnthebiker
Copy link

Whenever i use require_from_group it disables all other validations (rules). Any ideas why?

EDIT: check ryleyb's examples in post below.

@ryleyb
Copy link
Contributor

ryleyb commented May 8, 2012

Here is a simplified example: http://jsfiddle.net/ryleyb/MH3ra/ (with groups)

Without groups: http://jsfiddle.net/ryleyb/MH3ra/1/

@madmuffin1
Copy link

I have found out it would only disable fields that are above the group, if that is of any help.

@barbnatali
Copy link

I'm having the same problem. Any progress toward a solution?

@ghost
Copy link

ghost commented Oct 2, 2012

I have the same problem (actually with a custom rule which is very similar to require_from_group, but I have tested with require_from_group too).

I have even tried putting a submitHandler with if( $(form).valid() ) in it, mainly so that i could set a breakpoint in firebug and check it, but it claims the form is valid. I have an example with both with and without groups in the same page (using the same valdiation options) if it's any use. The example is slightly more complex in that I added error placement and highlighting to show that fields after the group are validate coirrectly while those before the group are not. (http://jsfiddle.net/qXscC/)

I'd rate it a fairly serious bug in a validation plugin that it thinks an invlaid form is valid, when using only the supplied rules. Is there any way to prioritise up this problem?

@ghost
Copy link

ghost commented Oct 2, 2012

Sorry to SPAM the comments here, but I have a workaround for the time being I think. I have also set this as a question at Stack Overflow, and the workaound is detailed there (and also demonstrated in the jsfiddle in my first comment). The question can be found here. The updated jsfiddle here

The workaround is just that, a workaround not a solution, in that it involves validering some of the elements twice.

The StackOverflow question now has a simplified version of the problem and workaround

@jzaefferer
Copy link
Collaborator

The problem is in that custom method - it uses .valid(), which goes through the same lifecycle of initialization validation and showing errors as everything else, and that shouldn't happen within a validation method.

May need some refactoring within the plugin to make it easier to run validation on other fields. Or maybe the approach for that group method is just wrong, and we need something else. Not sure yet.

@smilyanp
Copy link

Try this example, it seems to work: http://jsfiddle.net/hfLwB/4/

I found it here: http://stackoverflow.com/questions/11070245/jquery-validator-group-how-to-use-it

This is the custom method:

jQuery.validator.addMethod("require_from_group", function(value, element, options) {
  var numberRequired = options[0];
  var selector = options[1];
  var fields = $(selector, element.form);
  var filled_fields = fields.filter(function() {
    // it's more clear to compare with empty string
    return $(this).val() != ""; 
  });
  var empty_fields = fields.not(filled_fields);
  // we will mark only first empty field as invalid
  if (filled_fields.length < numberRequired && empty_fields[0] == element) {
    return false;
  }
  return true;
// {0} below is the 0th item in the options field
}, jQuery.format("Please fill out at least {0} of these fields."));

@sonjz
Copy link

sonjz commented Oct 29, 2012

Thanks @smilyanp, the fix is good.

@jzaefferer
Copy link
Collaborator

@Synchro @nschonni since you helped out with a few things recently, is this issue something you could look into? The goal is not to use .valid(), as that causes trouble. Testing the solution mentioned by @smilyanp is probably the first step, but it also needs tests to avoid further regressions.

@smilyanp
Copy link

smilyanp commented Dec 1, 2012

@sonjz I'm happy it works for you as well.

@jzaefferer
Copy link
Collaborator

skip_or_fill_minimum has the same issue, see #578

@jzaefferer
Copy link
Collaborator

Right now all I can recommend is using the method suggested by @smilyanp. It won't pass against the existing unit tests, and I couldn't yet figure out why.

@ghost
Copy link

ghost commented Feb 4, 2013

The problem with using .check instead of .valid is that the marking of errors does not update. For example, if i have three fields which I require one of them to be filled in and I leave all three blank and try and continue, so are all three marked as errors. If I then fill in one of them it loses its error status (because after the first submit attempt it validates on change and the field is now valid) but the other two fields are still marked as errors (since they have not been updated) even though they are now valid.

@jzaefferer
Copy link
Collaborator

Yeah, using check doesn't help at all. It has the same recursion issue and, as you point out, skips the message display.

@ghost
Copy link

ghost commented Feb 26, 2013

Just came across this issue myself (details on Stack Overflow). Is anyone aware of any fixes or workarounds to this at all (asking in case there's anything floating around outside of this issue thread that I've not came across yet)? Thanks.

@sonjz
Copy link

sonjz commented Feb 26, 2013

@zest I've reiterated smileyanp's post of @Tats_innit solution on stackoverflow - http://stackoverflow.com/a/13127475/740575

@ghost
Copy link

ghost commented Feb 26, 2013

@sonjz Thanks for pointing this out. I was using version 1.11.0 (where the require_from_group rules were working correctly, but other rules were not) rather than version 1.10.0, but have just gone back to 1.10.0 for both jQuery Validate and the Additional Methods. After applying the fix, I found thst whilst all the other rules are working again, the require_from_group rules appear to be buggy. The rule I have to require 1 out of 3 checkboxes doesn't work at all now, and the other rule I have to require 1 out of 4 text fields only highlights the first of the four fields, though the message disappears correctly when one of the other fields are filled in (but the 1st field remains highlighted as if the field was marked as required).

@sonjz
Copy link

sonjz commented Feb 26, 2013

@zest hmm ... i had no problems with the require_from_group for 1.10... did you update the function in additional-method-1.10.js or override it by placing your function after additional-method-1.10.js loaded?

@ghost
Copy link

ghost commented Feb 26, 2013

@sonjz I updated the code within additional-method-1.10.js with the patch from Stack Overflow (completely replacing lines 346-360 with the patch). I should add that require_from_group also worked fine for me in 1.10.0, albeit, with that issue of it disabling other rules. I'll try and put together a simplified example with jsfiddle to demonstrate. My last jsfiddle was using 1.11.0 without any patches, so I'll need to update that.

@ghost
Copy link

ghost commented Mar 6, 2013

@sonjz I've just updated my jsfiddle to show this issue: http://jsfiddle.net/ya7Px/4/ This is using jQuery Validate 1.10.0, with a patched version of additional-methods.js 1.10.0 which includes the fix you linked to earlier in this issue. On this new jsfiddle, you can see how the text field now validates correctly (before adding the patch, its required rule was being ignored), but the require_from_group checkboxes aren't validated now.

@kshade2001
Copy link

I've submitted a patch, and example is at http://jsfiddle.net/f887W/11/
Not the neatest solution, I'm sure it can be improved on substantially..

@lleiiell
Copy link

Thanks @smilyanp .

patheard added a commit to patheard/jquery-validation that referenced this issue Aug 27, 2013
Stops the require_from_group and skip_or_fill_minimum validation
methods from clearing previous validation rules on a form.

It does this by creating a cloned copy of the form's validator for
each group of conditionally required fields.  This validator is then
used to validate field group and is stored for re-use (fixes jquery-validation#412).
@sfreytag
Copy link

The problem for me with smilyanp's fix is that I do want the message against all fields in the group (I think this is nicer for the end user).

patheard's fix works for me.

Also, the following worked. Inspired by the stack overflow discussion, I do both fields.valid() and $(element.form).valid(). This is a one-liner change over the current version.

jQuery.validator.addMethod("require_from_group", function(value, element, options) {
    var validator = this;
    var selector = options[1];
    var validOrNot = $(selector, element.form).filter(function() {
        return validator.elementValue(this);
    }).length >= options[0];

    if(!$(element).data('being_validated')) {
        var fields = $(selector, element.form);
        fields.data('being_validated', true);
        fields.valid();
        $(element.form).valid();
        fields.data('being_validated', false);
    }
    return validOrNot;
}, jQuery.format("Please fill at least {0} of these fields."));

@creativetim
Copy link

+1 @sfreytag

@mvanroon
Copy link

Had to reimplement this code in order to fix it again. Tested on 1.11.1 and 1.12.0

@smaudet
Copy link

smaudet commented Sep 21, 2016

Seems like this regressed on 1.15.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet