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 sequences are processed differently when stored in metadata vs. on controller #482

Open
fkleuver opened this issue Apr 7, 2018 · 5 comments
Labels

Comments

@fkleuver
Copy link
Member

fkleuver commented Apr 7, 2018

I discovered this by accident when I was answering this SO question. To quote the relevant part of my answer:

When you call .validate() on a controller, it will first look for "local" rules stored via .addObject() and pass those to the validator via .validateObject().
The rules are processed in one pass, and the sequence behaves as it should.

Then it goes through the registered bindings (added by the binding behaviors) and for each individual property it will call .validateProperty() on the validator if the object is not stored on the controller.

Rather than the sequence being run once on the object, it's run once per individual property, effectively bypassing the behavior of sequences to some extent (they still apply on property level, just not on object level).

TL;DR - 2 issues

1: Rule sequences don't behave as they are documented. Are the docs wrong or is the code wrong?

They are documented to simply run sequentially (rather than in parallel). What they are really doing is: one rule sequence evaluates to invalid -> following rule sequences are not evaluated at all.

2: Rule sequences behave differently if they are defined globally vs. on a controller. Is the global behavior correct or is the controller behavior correct?

2.1 Globally via the fluent terminator .on(obj), they are evaluated per property. Meaning if you have a single rule for each property in a sequence, each rule will be evaluated and result in a validation message since non-matching sequences are skipped.

2.1 On the controller via .addObject(obj, rules), they are evaluated per object. So the exactly same rules as above will result in only one property being evaluated, rather than all of them.

Addendum

If the behavior as described in .1 is indended, then .2.2 more or less represents the way it should be: one rule evaluates to invalid, no further rules are processed. If .1 is not intended, then .2.1 is more like how it should be (though not entirely)

This is not an urgent issue, but it's a very subtle bug/inconsistency that should be addressed at some point.

A decision needs to be made on what the intended behavior is, and then I could potentially help with a PR to make the code behave that way consistently in the different scenarios.

@Alexander-Taran
Copy link

@fkleuver please reformat your issue somehow.. so it is more actionable..
Can't figure from glancing.. Is it a question or an issue?

@fkleuver
Copy link
Member Author

fkleuver commented Apr 7, 2018

I updated the question. Your observation is however on point; it is first and foremost a question to help decide what the real issue is, and having an answer on that question would turn this into an issue :)

@jdanyow
Copy link
Contributor

jdanyow commented Apr 29, 2018

I'd rather folks used the API to define rules than attempting to construct the internal data structure which may change in the future.

Here's a way to create rules from data using the existing API.
#363 (comment)

I'd love to have this as part of the public API if we can reach consensus on the rulesDefinitions data structure shown in that post. Inspired by your work in the keyof PR:

interface IRuleDefinition<T> {
  propertyName: keyof T;
  displayName: string;
  rules: ???[];
}

thoughts?

@fkleuver
Copy link
Member Author

@jdanyow I'm a bit confused. Did you mean to respond to a different issue? This is a question about rule sequences being processed differently based on whether the rules are defined on the controller vs. globally. Both scenarios pertain to defining rules via the public API..

@MaximBalaganskiy
Copy link

MaximBalaganskiy commented Aug 29, 2019

Will this be looked at?
The inconsistent behaviour is demonstrated here https://gist.run/?id=53fd9f7b05e8c878079bb87a650460b8
Try entering something except “A” into the first field - it will correctly display an error.
Then click “Validate” the error will be gone until the second field is valid.

Ideally, even when the whole object is explicitly added for validation, rules should be grouped by properties and only then by sequence. Otherwise properties are falsely reported as valid.

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

4 participants