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

rule dependencies #279

Open
heruan opened this issue Aug 9, 2016 · 13 comments
Open

rule dependencies #279

heruan opened this issue Aug 9, 2016 · 13 comments

Comments

@heruan
Copy link
Contributor

heruan commented Aug 9, 2016

The old library had the ability to make rules dependent on others, i.e. re-evaluate a binding when other bindings are updated; now, this won't work:

<input type="password" value.bind="password & validate">
<input type="password" value.bind="passwordConfirm & validate">

Assume passwordConfirm must be equal to password (supported for example in aurelia-validatejs with @equality) the passwordConfirm & validate binding should trigger also when password changes, but the binding behavior doesn't know about the implementation specifics so it won't update/render when password is changed.

@heruan
Copy link
Contributor Author

heruan commented Aug 9, 2016

Some ideas:

  • the validate binding behavior currently accept a rules argument, which is very useful or even necessary in certain cases; so, I would not take the road of another argument to pass to keep a clean signature;
  • I would instead create a ValidationRule interface, to have a dependsOn array of other ValidationRules;
  • then, the ValidationController when triggered to validate a binding would validate also its dependencies.

@plwalters
Copy link
Contributor

This was previously supported because all rules of the object were re-evaluated after any property of that object changed. Perhaps if the code for equality is similar to how it was previously implemented we could use that to trigger a dependency on the target property under the covers.

@heruan
Copy link
Contributor Author

heruan commented Aug 10, 2016

I came up with #280 and aurelia/validatejs#117. I'll publish a GistRun to see it in action!

@heruan
Copy link
Contributor Author

heruan commented Aug 10, 2016

Of course, all that fuss could be avoided if we call validate() to re-evaluate all rules when a binding triggers, instead of evaluate just the rule for its property. Too much overhead? That way dependencies are useless, since all rules would implicitly depend on the others—drawback is that all bindings will render at once.

@jdanyow
Copy link
Contributor

jdanyow commented Aug 27, 2016

I'm hesitant to put UI stuff in the fluent rule api because it's intended to be used server-side. Would it be better to create some sort of binding behavior that you can use to express UI dependencies for rule validation?

<input type="password" value.bind="password & validate">
<input type="password" value.bind="passwordConfirm & validate & dependsOn:'password'">

Today you could do something like this:

<div change.delegate="controller.validate()">
  <input type="password" value.bind="password & validate">
  <input type="password" value.bind="passwordConfirm & validate">
</div>

Or this:

<input type="password" value.bind="password & validate"
       change.delegate="controller.validate()">
<input type="password" value.bind="passwordConfirm & validate">

Or this:

<input type="password" value.bind="password & validate"
       change.delegate="controller.validate({ object: $this, propertyName: 'passwordConfirm' })">
<input type="password" value.bind="passwordConfirm & validate">

Not saying we cannot add .dependsOn(property) to the fluent API- just need to think it through carefully. Maybe it makes sense because it's such a usability win?

@dkent600
Copy link
Contributor

dkent600 commented Sep 28, 2016

It seems to me that any "rule that depends on another rule" should be defined as a single rule using .satisfies(), no? I'm not clear what is the use case where this would not be appropriate or possible.

But using satisfies still presents an issue.

My use case:

I am validating at the object level, thus must manually call ValidationController.validate().

Each object contains object rules (defined by ensureObject().satisfies()). These object rules do not depend on other rules, but they do relate to a set of properties on the object. However, unlike with ensure().satisfies() it is not possible to associate ensureObject().satisfies() with a property name, much less with multiple property names.

Further, I am using neither the validate binding nor a ValidationRenderer. Again, I am manually calling ValidationController.validate(), and manually rendering the error information when validate completes.

A ValidationRenderer could be useful for me in this scenario, but it is not, for the following reasons:

  1. without using the validate binding, it does not provide the element(s) with which a property is associated.
  2. it provides neither property names nor elements for rules define by ensureObject().satisfies()

Thus I propose the following:

Add a parameter propertyNames to ensureObject().satisfies(instance), like this: ensureObject().satisfies(instance, propertyNames?: string[])

Change the type of ValidationError.propertyName from string to string | Array<string> (or have it always be an array).
Perhaps change the name of ValidationError.propertyName to the plural ValidationError.propertyNames

I think this fits fairly nicely into the existing paradigm.

This will help with #351

@dkent600
Copy link
Contributor

dkent600 commented Sep 29, 2016

Alternative suggestion, perhaps even better:

Add a parameter to ensureObject to specify multiple property names.

@rajajhansi
Copy link
Contributor

@jdanyow, I tried to use aurelia-validation on the server side in a nodejs application but it looks like ValidationRules has still dependencies on Element (lib.d.ts aka DOM dependency), which makes it fail. I noticed that the documentation page on "Server Side Validation"(http://aurelia.io/hub.html#/doc/article/aurelia/validation/latest/validation-basics/13) isn't ready yet because I guess it is still work in progress. I've had a great success using aurealia-dependency-injection on the server side without any issues. I'm about to wrap up part 7 of my tutorial in my blog ( http://opensourcedotnetter.blogspot.com/) on Aurelia validation. Please advice if I should wait for a release of aurelia-validation that works on the server side or should I go ahead with using validate-js directly or the deprecated aurelia-validatejs package. Thanks.

@rajajhansi
Copy link
Contributor

I face the same issue using aurelia-validatejs package (although it is deprecated) as well since it has a dependency on aurelia-validation. I'm going to go ahead and use validatejs directly in nodejs and return the validation errors to the client.

@jdanyow
Copy link
Contributor

jdanyow commented Dec 30, 2016

Comment from @dkent600 here: #351 (comment) has gist with a couple of scenarios.

@dkent600
Copy link
Contributor

dkent600 commented Jan 26, 2017

@heruan I'm looking into this, particular at the original use case you provided, and am wondering, is it sufficient, when you set up your rules, to simply require that both password and confirmPassword satisfy a custom rule that you will have created to match the two values? If not, what more is needed? Thanks.

@dkent600
Copy link
Contributor

dkent600 commented Jan 27, 2017

See also this comment, with a gist, demonstrating a potential solution: #351 (comment)

@pettys
Copy link

pettys commented Nov 21, 2017

I realize this is a pretty old thread, but still open, and I'm hit by this currently.

@dkent600, to offer an answer to your question, "is it sufficient..." I think your proposed solution (a custom rule applied to all involved properties) would not work well for the password form of the original question, in terms of the error renderer. Here are steps I'm thinking of:

  1. Enter password = "abc"
  2. Enter confirmPassword = "abc"
  3. Go back and enter password = "def"
    Expect behavior: "passwords must match" error rendered on the confirmPassword field.
    Actual behavior: "passwords must match" error rendered on the password field.
    (Comment: this might be a bit odd, but maybe acceptable.)
  4. Enter confirmPassword = "def"
    Expect behavior: errors all cleared.
    Actual behavior: there is nothing triggered to revalidate the password field; the error doesn't go away.
    (Comment: this behavior seems not ok.)

In summary: although your solution would help get errors on the screen in any case, in a dependent property scenario, errors potentially need to clear when another property changes.

As another example, closer to what I'm dealing with today, a user must enter two numbers, a required positive number, and then another number which must be greater than the first. The domain suggests that this "greater than" rule belongs on the second number, but hard to get it to work right.

My main point is I think a property dependency mechanism of some sort is still warranted.

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

No branches or pull requests

7 participants