jwaliszko / ExpressiveAnnotations

Annotation-based conditional validation library.
MIT License
351 stars 123 forks source link

Validating using array indexed values from Model hiccup and maybe fix. #149

Closed rluiten closed 7 years ago

rluiten commented 7 years ago

Firstly thanks for your library it has already saved me pain and is pretty nice. I was not looking forward to having to fully understand extending unobstrusive validation I was happy your lib was approachable and appears quite powerful.

I have a model with a list (or array) of selected days of week. Its currently rendered out as a set of checkboxes one for each day. There names end up being SelectedDays[0] through SelectedDays[6] in the DOM.

public class MyModel {

  public IList<bool> SelectedDays { get; set; }

  [Required]
  [AssertThat("ValidDays(StartDate, EndDate, "
              + "SelectedDays[0], SelectedDays[1], SelectedDays[2], "
              + "SelectedDays[3], SelectedDays[4], SelectedDays[5], SelectedDays[6])",
      ErrorMessage = @"At least one day between Start and End date needs to be selected.")]
  public   DateTime? StartDate { get; set;}

  public DateTime? EndDate { get; set;}
  // rest
}

I wish to validate that at least one selected day in the range of StartDate to EndDate is selected. On the client side this fails because when it gets to executing the ValidDays as follows.

AssertThat expression of StartDate field:
ValidDays(StartDate, EndDate, SelectedDays[0], SelectedDays[1], SelectedDays[2], SelectedDays[3], SelectedDays[4], SelectedDays[5], SelectedDays[6])
will be executed within following context (methods hidden):
{
    "StartDate": 1481673600000,
    "EndDate": null,
    "SelectedDays[0]": false,
    "SelectedDays[1]": false,
    "SelectedDays[2]": false,
    "SelectedDays[3]": false,
    "SelectedDays[4]": false,
    "SelectedDays[5]": false,
    "SelectedDays[6]": false
}

I get the error.

expressive.annotations.validate.js:93 ReferenceError: SelectedDays is not defined(…)

I've had a look and it appears (without deep understanding) that a modification to deserializeObject() sub function buildField() may be all that is required to produce a model of form.

AssertThat expression of StartDate field:
ValidDays(StartDate, EndDate, SelectedDays[0], SelectedDays[1], SelectedDays[2], SelectedDays[3], SelectedDays[4], SelectedDays[5], SelectedDays[6])
will be executed within following context (methods hidden):
{
    "StartDate": 1481673600000,
    "EndDate": null,
    "SelectedDays": [
        false,
        false,
        false,
        false,
        false,
        false,
        false
    ]
}

Which runs appears to run fine for me at the moment for calling ValidDays(StartDate, EndDate, SelectedDays[0], SelectedDays[1], SelectedDays[2], SelectedDays[3], SelectedDays[4], SelectedDays[5], SelectedDays[6])

Here is what my modified buildField() looks like. I moved the array pattern match to variable arrayPat. Then use it at the end of function when creating values against parent object so I create an array if the end of fieldName is of array form.

            function buildField(fieldName, fieldValue, object) {
                var props, parent, i, match, arridx;
                props = fieldName.split('.');
                parent = object;
                for (i = 0; i < props.length - 1; i++) {
                    fieldName = props[i];

                    match = arrayPat.exec(fieldName); // check for array element access
                    if (match) {
                        fieldName = match[1];
                        arridx = match[2];
                        if (!parent.hasOwnProperty(fieldName)) {
                            parent[fieldName] = {};
                        }
                        parent[fieldName][arridx] = {};
                        parent = parent[fieldName][arridx];
                        continue;
                    }

                    if (!parent.hasOwnProperty(fieldName)) {
                        parent[fieldName] = {};
                    }
                    parent = parent[fieldName];
                }
                fieldName = props[props.length - 1];

                var endMatch = arrayPat.exec(fieldName);
                if (endMatch) { // our fieldName is array pattern eg. SelectedDays[0]
                    var arrayName = endMatch[1];
                    var arrayIndex = endMatch[2];
                    parent[arrayName] = parent[arrayName] || []; // create it if needed.
                    parent[arrayName][arrayIndex] = fieldValue;
                } else {
                    parent[fieldName] = fieldValue;
                }
            }

Is what I have done naive in some way and I am going to be eaten by bears ? Maybe I have missed some other way to achieve the same result ?

At the moment the other way that comes to mind is just create 7 boolean fields one for each week day and just use that for model, then I expect things will work out simpler, but I have to ask this question :). I think the array solution suits me better at moment, but I may be just a misguided soul..

Thanks for any responses.

jwaliszko commented 7 years ago

Hi, due to the fact my laptop has crashed and I haven't get new one yet, I'm currently unable to verify your changes. Nevertheless thank you for your effort, and proposing a solution, which I appreciate very much.

If you want to be sure your change is compatible with legacy behaviour, fork the repository and run the tests (i.e. rebuild in release mode, and then execute ./test.ps1 script - you'll need firefox 46.0.1 for proper execution of current Selenium UI tests though...). If everything passes, all the usage cases I've been thinking of so far are covered as expected, so you should be safe with your modification.

What's more, although I don't know whether is applies into your case, there is a generic way of solving custom serialization cases, mainly ValueParser attribute - particularly take a look here (sample usage) and here (sample definition). Mabye it can be helpful for you.

rluiten commented 7 years ago

Thanks for response :). Oh, shame about your laptop, hope you get something nice soon.

I did find ValueParser, but it didn't appear to cover my use case, though maybe I just misunderstood it.

I have output my array of booleans as 7 separate checkbox inputs, one for each day of week. So there is no single field called "SelectedDays" in the form, this means I think that I can't use the ValueParser I think (please correct me if I am wrong) The names that exist for me as fields are "SelectedDays[0]" though to "SelectedDays[6]".

I will look at getting the tests running, not sure I will get to it this week though.