Ensuring validation rules, messages, and appliesIf can access data properly about treacherous HOT 4 CLOSED

I dont see options and PRs as hacks, the options is just a placeholder to pass whatever you want (or dont want) into a validator, so a maxLength rule may want a numeric value representing the max length, a require validator may want a boolean specifying if it is required, then there may be really complex rules where you need to pass in multiple values, but its down to the rule consumer to specify what it requires, like something that did an async ajax request and wanted to know how much to throttle etc may need a json object passed in rather than a singular value.

Anyway getting to the crux of this, I dont see the model resolver adding anything that you don't already have access to. So lets take a theoretical matches rule where one property must match another:

import {PropertyResolver} from "property-resolver/index";
import {IValidationRule} from "./ivalidation-rule";

export class MatchesValidationRule implements IValidationRule
{
    public ruleName = "equal";

    constructor(private propertyResolver: PropertyResolver){}

    public validate(model, value, fieldToMatch: string): Promise<boolean>
    {
        if (fieldToMatch === undefined || fieldToMatch === null)
        { return Promise.resolve(false); }

        var matchingFieldValue = this.propertyResolver.resolveProperty(model, fieldToMatch);
        var result = matchingFieldValue === value;
        return Promise.resolve(result);
    }

    public getMessage(model, value, fieldToMatch: string) {
        var matchingFieldValue = this.propertyResolver.resolveProperty(model, fieldToMatch);
        return `This field is ${value} but should match ${matchingFieldValue}`;
    }
}

This assumes we are passing the model to rules, which is basically the same as your approach above but you have a wrapper around the model to assist with accessing it. So here we adhere to IoC and pass in the property resolver (as our rule option is going to be a property as a string), then we basically consume that via the property resolver.

matches is the only kind of rule type I can envision which would require a property as a string in common use-cases, so in most other cases where you want to alter validation logic based upon some state within the model you would generally just ask the model directly for the values you care about, like model.isAdmin or model.currentOptionsChosen.length or whatever.

The addition of model to the rules does complicate things more than I would like however it does keep it self contained and also allows complex use cases where you need to alter the underlying rule logic based upon composite state in the model.

The developer in me likes the idea of a model resolver which basically pre-packages the model and the property resolver, but in most cases its just an abstraction on something that doesnt need to be abstracted from my current perspective.

As always I am happy to discuss further if there are some strong points for this.

from treacherous.

I know what you're saying, and most of this makes sense, although the idea of having devs new up PropertyResolvers when we've just used one doesn't sound right.
Also, as I said, why would we resolve the value before calling the rule, then resolve other properties within the rule? We might as well be passing in the property name in the same way that we pass in other properties as names in the options.
Field matching is probably the least-used of the property lookups, as the main one is the conditional appliesTo, because any rule used in that function will almost certainly be using other fields. As such, we can't reference the model directly because it could change (such as selecting another record in a list), so every message, appliesTo and options has to go through the PR.
I'll knock up a test version to see how it works and how it looks.

from treacherous.

Given that we now have this notion of model resolver can this be closed?

from treacherous.

I am closing this, you can re-open if you want.

from treacherous.