search

carlschroedl / validatedviewmodel

Make validation on your KnockoutJS view model classes scale to multiple views by applying validation constraints on an instance-basis.
3 stars 1 forks source link

Removing Validation Group Issue #1

Open justinspradlin opened 11 years ago

justinspradlin commented 11 years ago

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 commented 11 years ago

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

justinspradlin commented 11 years ago

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/carlschroedl/validatedviewmodel/issues/1#issuecomment-13273694.

carlschroedl commented 11 years ago

After much debugging, I found two key problems:

justinspradlin commented 11 years ago

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/carlschroedl/validatedviewmodel/issues/1#issuecomment-13341947..

carlschroedl commented 11 years ago

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 commented 11 years ago

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 commented 11 years ago

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 commented 9 years ago

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