Blazored / FluentValidation

A library for using FluentValidation with Blazor
https://blazored.github.io/FluentValidation/
MIT License
587 stars 84 forks source link

Expected behaviour for interrelated properties #13

Closed tomRedox closed 4 years ago

tomRedox commented 4 years ago

I've got some validation rules that span multiple properties and what I'm finding is that if there's a validation rule on C that says "A + B must equal C" then when A or B are changed they do not cause the validation message to be updated for C.

By way of a basic example, in the gif below Amount Paid must equal Net Total + Tax Due:

cPDjGtPaLZ

The code for the invoice and its validator looks like this:

    public class InvoiceHeader
    {
        public int? NetTotal { get; set; }
        public int? TaxTotal { get; set; }
        public int? AmountPaid { get; set; }
    }

    public class InvoiceHeaderValidator : AbstractValidator<InvoiceHeader>
    {
        public InvoiceHeaderValidator()
        {
            RuleFor(x => x.AmountPaid).Equal(x => x.NetTotal + x.TaxTotal);
        }
    }

And the code for the Invoice component looks like this

@page "/invoice"
@using ServerSideBlazor.Models
@using Blazored.FluentValidation

<h1>Invoice</h1>

<EditForm Model="_invoiceHeader" OnValidSubmit="HandleValidSubmit" OnInvalidSubmit="HandleInvalidSubmit">
  <FluentValidationValidator />
  <ValidationSummary />

  <div class="form-group">
    <label for="NetTotal" class="control-label  col-md-4">Net Total: </label>
    <div class="col-md-8">

      <InputNumber class="form-control text-right" @bind-Value="_invoiceHeader.NetTotal" />
      <ValidationMessage For="@(() => _invoiceHeader.NetTotal)" />
    </div>
  </div>

  <div class="form-group">
    <label for="TaxTotal" class="control-label  col-md-4">Tax Due: </label>
    <div class="col-md-8">

      <InputNumber class="form-control text-right" @bind-Value="_invoiceHeader.TaxTotal" />
      <ValidationMessage For="@(() => _invoiceHeader.TaxTotal)" />
    </div>
  </div>

  <div class="form-group">
    <label for="AmountPaid" class="control-label  col-md-4">Amount Paid: </label>
    <div class="col-md-8">

      <InputNumber class="form-control text-right" @bind-Value="_invoiceHeader.AmountPaid" />
      <ValidationMessage For="@(() => _invoiceHeader.AmountPaid)" />
    </div>
  </div>

  <button type="submit">Save</button>

</EditForm>
@code {
  private InvoiceHeader _invoiceHeader = new InvoiceHeader();

  private void HandleValidSubmit()
  {
  }

  private void HandleInvalidSubmit()
  {
  }
}

Is the behaviour I'm seeing the expected behaviour, and if so, is there any way to tell the validation to update the validation messages for all the components, not just the one that changed?

(Apologies, this may well be how Blazor validation is expected to work, I'm trying to work out if I've set my rules up badly, need to do something to change Blazor's in-built validation behaviour or if this behaviour comes from the Blazored.FluentValidation library)

chrissainty commented 4 years ago

Hi Tom, this is expected behaviour. When you change a field validation is triggered for that field. When you submit a form, if using @OnValidSubmit, then all fields on the form are validated.

Blazored FluentValidation mimics the behaviour of the default data annotations validator in this regard. To get the functionality you're looking for, you would need to re-validate the whole form on every input change. I don't think is something I'd like this component to do as there could be potential performance issue in large forms.

As a suggestion, you could look at triggering the re-validation of the whole form yourself via the EditContext. The events are public (you can see them here: https://github.com/aspnet/AspNetCore/blob/909f1d368f1e195e9fa2368e12d07b5b5584f8e8/src/Components/Forms/src/EditContext.cs#L39). Hope this helps.

tomRedox commented 4 years ago

Hi Chris, many thanks for the quick response yesterday, that really helped and stopped me spending the day on a wild goose chase!

Completely understand re not wanting to revalidate the whole form by default. I'll have a proper dig into to EditContext and see that that offers.

We've used the CSLA framework a lot which has the concept of dependent validation properties, so you can say A depends on B (and vice-versa, or not), so if A changes, revalidate B too. That's no doubt what's influencing my thinking here. The difference in CSLA however is that the broken rules collection is a property of the model object itself, so at any point an object knows its validation state, unlike in ASP.NET.

It seems like dependent properties like this must be a fairly common requirement, I'm going to try and cobble together a library that allows a user to set up some sort of list of relationships and then listen for update notifications from the EditContext and trigger validation of those related properties.

tomRedox commented 4 years ago

Hi Chris,

Thanks for the pointer on EditContext, I went away and did some reading about that and came up with this code that gets things working as required for my scenario (based on this code:

@page "/invoice"
@using ServerSideBlazor.Models
@using Blazored.FluentValidation

<h1>Invoice</h1>

<EditForm  EditContext="@EditContext" OnValidSubmit="HandleValidSubmit" OnInvalidSubmit="HandleInvalidSubmit">
  <FluentValidationValidator />
  <ValidationSummary />

  <div class="form-group">
    <label for="NetTotal" class="control-label  col-md-4">Net Total: </label>
    <div class="col-md-8">

      <InputNumber class="form-control text-right" @bind-Value="_invoiceHeader.NetTotal" />
      <ValidationMessage For="@(() => _invoiceHeader.NetTotal)" />
    </div>
  </div>

  <div class="form-group">
    <label for="TaxTotal" class="control-label  col-md-4">Tax Due: </label>
    <div class="col-md-8">

      <InputNumber class="form-control text-right" @bind-Value="_invoiceHeader.TaxTotal" />
      <ValidationMessage For="@(() => _invoiceHeader.TaxTotal)" />
    </div>
  </div>

  <div class="form-group">
    <label for="AmountPaid" class="control-label  col-md-4">Amount Paid: </label>
    <div class="col-md-8">

      <InputNumber class="form-control text-right" @bind-Value="_invoiceHeader.AmountPaid" />
      <ValidationMessage For="@(() => _invoiceHeader.AmountPaid)" />
    </div>
  </div>

  <button type="submit">Save</button>

</EditForm>
@code {
  private InvoiceHeader _invoiceHeader;
  private EditContext EditContext { get; set; }

  protected override void OnInitialized()
  {
    base.OnInitialized();

    _invoiceHeader = new InvoiceHeader();

    EditContext = new EditContext(_invoiceHeader);

    EditContext.OnFieldChanged += EditContext_OnFieldChanged;
  }

  private void EditContext_OnFieldChanged(object sender, FieldChangedEventArgs e)
  {
    EditContext.Validate();
  }

  private void HandleValidSubmit()  {}
  private void HandleInvalidSubmit()  {}
}

The component now works like this:

t7xO5nfoIB

When I look at the components in my app I can foresee a lot of repetition of that boilerplate for creating the EditContext and subscribing/responding to its events, which makes me think this probably isn't the optimal solution and that this functionality doesn't really belong here. I think for the current project I'm working on most of the forms would want this revalidation of all the fields to be the default behaviour as there's a lot of cross property validation, and the validations aren't hitting the database in most cases so I'm not so concerned about the performance issues.

I wonder if there would be any value to having an optional parameter for the FluentValidationValidator component that allowed for the user to choose for the whole form to be validated when a field changes? Steve Sanderson revalidates the whole form in his basic example here, but does also note in the related article that this was just done for simplicity and that it's not efficient. I'd be happy to give adding that feature a go if you thought it was a useful option?

Essentially, it seems to me that when a field changes and the form validator runs, the expected behaviour at the end of that validation would be that all the displayed fields are now showing their correct validation state as per the validation rules defined for the model? In the DataAnnotations world the validations only applied to a single property so revalidating a single field when it changed makes sense, but the interrelated property validations mean that after the validation runs the form may no longer be showing the true state of the model's validation, so in that scenario it seems like either the whole form needs to be revalidated, or interrelated properties need to be aware of each other. What do you think?

chrissainty commented 4 years ago

I think adding this as an option would be a good edition to the control. We can add a parameter on the validator and if true we could wire up the event handlers a bit differently. Something like this:

if (FullValidate)
{
    editContext.OnFieldChanged +=
                (sender, eventArgs) => ValidateModel((EditContext)sender, messages, serviceProvider, validator);
}
else
{
    editContext.OnFieldChanged +=
                (sender, eventArgs) => ValidateField(editContext, messages, eventArgs.FieldIdentifier, serviceProvider, validator);
}

The rest of the existing code should be able to stay as is. If you can think of a better name than FullValidate that would be good! 😄

tomRedox commented 4 years ago

Hi Chris, great stuff, I'm on it, I'll stick a PR in shortly. You may regret asking the world's most verbose variable namer to come up with a new name for something..

chrissainty commented 4 years ago

Hi @tomRedox, I don't think this is something that is going to get added as there has been no progress for 9+ months :) I'm having a clean up and I'm going to close this issue. If this is still something you think is worthwhile please start up the discussion again and we can always reopen.