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

Removing Validation Group Issue #1

Open
justinspradlin opened this issue Feb 6, 2013 · 8 comments
Open

Removing Validation Group Issue #1

justinspradlin opened this issue Feb 6, 2013 · 8 comments
Assignees
Labels

Comments

@justinspradlin
Copy link

Hello,

Thank you for your library. I really like it. However, I am stuck with a small problem. I am trying to build a screen to manage network settings. You can choose either DHCP or Static IP addresses using a radaio button. The addressType is bound to the radio buttons. When the user changes the addressType, I have a subscription that will toggle the validation group for the 'static' rules. Basically the static rules group just validate that the IP, Netmask, and Gateway are required and valid IPs. The problem I am seeing is that even after removing the validation group for static, by choosing the "DHCP" radio button I am still seeing 3 validation errors for the validation group that was removed.

I created a fiddle demonstrating this. If you choose the "DHCP" radio button, then click the "Test" button, there should not be any errors since the only required field is addressType and it has a value since the radio-button was checked.

Can you see anything that I am doing wrong?

http://jsfiddle.net/jspradlin/YmjGR/

Thanks in advance,

Justin

@carlschroedl
Copy link
Owner

Hi Justin. Thanks for the fiddle. I will give it a glance this weekend.

@ghost ghost assigned carlschroedl Feb 8, 2013
@justinspradlin
Copy link
Author

I think I have a hack that will work. I'll send it to you tomorrow. The
basic issue was that removing a rule didn't really work in my case.
On Feb 7, 2013 8:10 PM, "Carl Schroedl" notifications@github.com wrote:

Hi Justin. Thanks for the fiddle. I will give it a glance this weekend.


Reply to this email directly or view it on GitHubhttps://github.com//issues/1#issuecomment-13273694.

@carlschroedl
Copy link
Owner

After much debugging, I found two key problems:

  • After the call to ko.applyBindings() and before your function subscribing to addressType is called, extra validation rules are being added to observables. They look like the same ones you had already applied but without message parameters. I can't figure out why extra entries are being put in the model..rules arrays. I need to figure out the root cause of that before proceding.
  • Per your fix, the constraint comparison in removeConstraintFromProperty() is buggy. I'm glad that you have written a workaround! Thanks! I was so hung up on the previous issue that I didn't get a chance to see why the unit tests were failing, but I will take a look tomorrow.

@justinspradlin
Copy link
Author

I saw that same behavior too. I tried debugging but I couldn't locate where
those phantom rules came from. I ended up with a hack by removing that
break statement in the for loop to just remove all rule types. I know that
works in my particular situation, but it probably isn't the cleanest way to
do it generically.

I wonder if using the JSON.stringify may be a more generic solution to
handling the parameter comparison?

On Sat, Feb 9, 2013 at 6:33 PM, Carl Schroedl notifications@github.comwrote:

After much debugging, I found two key problems:

  • After the call to ko.applyBindings() and before your function
    subscribing to addressType is called, extra validation rules are being
    added to observables. They look like the same ones you had already applied
    but without message parameters. I can't figure out why extra entries are
    being put in the model..rules arrays. I need to figure out the root cause
    of that before proceding.

  • Per your fix, the constraint comparison in
    removeConstraintFromProperty() is buggy. I'm glad that you have written a
    workaround! Thanks! I was so hung up on the previous issue that I didn't
    get a chance to see why the unit tests were failing, but I will take a look
    tomorrow.


    Reply to this email directly or view it on GitHubhttps://github.com/Removing Validation Group Issue #1#issuecomment-13341947..

@carlschroedl
Copy link
Owner

Sorry for the delay in response. I did some more debugging and I still can't find the origin of the ghost rules. Thanks for your idea about JSON.stringify -- I like it. It might force us to make our constraint group definitions really verbose in order to trigger equality between the strings though. Do you see any way around that? I suppose we could do this:

var comparisonStringMadeFromConstraintGroup = JSON.stringify(ko.utils.extend({ 
                        all: 'of',
                        the: 'default'
                        rule: 'properties'
                       }, theConstraintGroup)); //set defaults, overwrite values if present in the constraintGroup we're removing
//then iterate through the rules and match stringified rule objects to comparisonStringMadeFromConstraintGroup

If I go this route I would want to closely examine ko.validation's default rule objects for all of the builtin validators. In the words of Jackie Chan Adventures:
http://i.qkme.me/3553mk.jpg

@carlschroedl
Copy link
Owner

I thought of another way. The best way of ensuring exact removal of constraintGroups would be to give every constraint group a unique ID and attach that ID to every rule on every observable involved in the application of a particular constraint group. When you need to remove a constraint group, iterate through all the view model properties involved in the constraint group, iterate through all of the rules for each property, and remove rules where the rule's constraintGroupID matches the ID of the constraintGroup that we have asked to remove.

This way removes the need for expensive JSON.stringifys and the dependence on ko.validation's rule object format, provided we are still able to add a constraintGroupID property.

I'm fiddling with this idea here:

http://jsfiddle.net/carlschroedl/y2MaA/

It's still a work in progress. Hop on if interested.

@carlschroedl
Copy link
Owner

Justin,

I have implemented the group id fix I mentioned above. It relies on a miniscule change to Knockout-Validation. Please voice your support on the pull request I sent to Eric so that the functionality gets incorporated:
https://github.com/ericmbarnard/Knockout-Validation/pull/239

In the meantime, feel free to rely on my patched version of Knockout-Validation and this new branch of ValidatedViewModel that contains the fix.

@crissdev
Copy link

Although this thread is quite old the solution is to add onlyIf to rules.
Updated fiddle http://jsfiddle.net/of0902yk/

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

No branches or pull requests

3 participants