Aspnetcore: Ensure it's possible to validate based on the fields in the UI

Created on 25 Sep 2019  路  8Comments  路  Source: dotnet/aspnetcore

The out-of-box DataAnnotationsValidator follows Data Annotations conventions of doing model-based, not UI-based, validation.

Sometimes people may want UI-based validation instead (i.e., we validate just the fields that have a corresponding editor on the screen right now).

If we're missing APIs to enable customizations for this, add them.

affected-medium area-blazor enhancement severity-minor

Most helpful comment

Hi all

I've put together some changes for you to consider. They aren't in a format that I can submit a pull-request. What I've done is to copy the various form classes into a folder called Modified (into a namespace with the same name) - from there I have made a branch, made the changes, and created a PR into my master branch so you can see the changes, which are accompanied by comments in the PR explaining the purpose of the changes.

This code should be backward compatible, but give the option for the user to specify Mode = DataAnnotationsValidatorMode.Recursive to get a full tree walk when validating.

I've done it this way so you can download the branch, and then run it to see it in action.

Benefits are

  1. No breaking changes
  2. Recursive validation is optional
  3. Doesn't require any additional attributes on the model being edited
  4. Will validate inputs regardless of how deep the @bind-Value expression is
  5. When not doing a Recursive validation it will only validate the form inputs, making it possible to edit a large object in a wizard-style UI.
  6. The tree walking code can be shared by other custom validation libraries (such as Blazor-Validation plug)

The recursive attribute on the model doesn't sit right with my gut. It seems UI decisions are being imposed on the model being edited, but the model is imposing an approach on the UI. If an app wanted two ways of editing the same object (Expert=one form / New user=wizard UI) then a recursive attribute would prevent this by forcing one approach.

PS: Ignore any changes with the text FormX or NewInputField. Those were corrections of earlier mistakes.

https://github.com/mrpmorris/_BlazorValidationChanges_/pull/1/files

All 8 comments

Some thoughts about this from @mrpmorris: https://github.com/aspnet/AspNetCore/issues/10526#issuecomment-533926474

Hi all

I've put together some changes for you to consider. They aren't in a format that I can submit a pull-request. What I've done is to copy the various form classes into a folder called Modified (into a namespace with the same name) - from there I have made a branch, made the changes, and created a PR into my master branch so you can see the changes, which are accompanied by comments in the PR explaining the purpose of the changes.

This code should be backward compatible, but give the option for the user to specify Mode = DataAnnotationsValidatorMode.Recursive to get a full tree walk when validating.

I've done it this way so you can download the branch, and then run it to see it in action.

Benefits are

  1. No breaking changes
  2. Recursive validation is optional
  3. Doesn't require any additional attributes on the model being edited
  4. Will validate inputs regardless of how deep the @bind-Value expression is
  5. When not doing a Recursive validation it will only validate the form inputs, making it possible to edit a large object in a wizard-style UI.
  6. The tree walking code can be shared by other custom validation libraries (such as Blazor-Validation plug)

The recursive attribute on the model doesn't sit right with my gut. It seems UI decisions are being imposed on the model being edited, but the model is imposing an approach on the UI. If an app wanted two ways of editing the same object (Expert=one form / New user=wizard UI) then a recursive attribute would prevent this by forcing one approach.

PS: Ignore any changes with the text FormX or NewInputField. Those were corrections of earlier mistakes.

https://github.com/mrpmorris/_BlazorValidationChanges_/pull/1/files

Any thoughts on my suggested approach? If it has failings then I'd like to know so I can rethink.

It's not something we're focusing on at the moment, as it's on the backlog and we have lots of urgent 3.1 work to get through. We'll consider possible designs in the future when we have capacity. Hope that's OK!

Sure, thanks for your reply.

I just wanted to make sure if it needed additional mental work that I was aware so I could do it!

@SteveSandersonMS @danroth27 @pranavkm @rynowak

I see this has been added to Blazor 3.1 -> https://github.com/aspnet/AspNetCore/blob/release/3.1/src/Components/Blazor/Validation/src/ObjectGraphDataAnnotationsValidator.cs

I feel it is just introducing a different problem, where we end up with two solutions and neither of them solve all three validation scenarios. I want to ensure you've seen my comments on this:
https://github.com/aspnet/AspNetCore/pull/14972#issuecomment-546251922

I did propose a solution that would deal with all three validation scenarios (the two you have now + wizard UI validating only visible components) here https://github.com/aspnet/AspNetCore/issues/14426#issuecomment-535238974

I'm not sure what you thought of my approach, but regardless of how you ultimately proceed I feel it would be better to leave this class out until a more complete solution has been identified.

@mrpmorris Did you get any new feedback/comment for your solution or/and your suggestion to add the following code in #14972:

if (value.GetType().Assembly == typeof(object).Assembly)
  return;

@mrpmorris Did you get any new feedback/comment for your solution or/and your suggestion to add the following code in #14972:

if (value.GetType().Assembly == typeof(object).Assembly)
  return;

The suggestion was wrong, because lists and arrays live there, but it should skip String though.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

aurokk picture aurokk  路  3Comments

githubgitgit picture githubgitgit  路  3Comments

rbanks54 picture rbanks54  路  3Comments

markrendle picture markrendle  路  3Comments

ermithun picture ermithun  路  3Comments