MudBlazor / MudBlazor

Blazor Component Library based on Material design with an emphasis on ease of use. Mainly written in C# with Javascript kept to a bare minimum it empowers .NET developers to easily debug it if needed.
http://mudblazor.com
MIT License
7.51k stars 1.21k forks source link

MudFormComponent:ValidateWithAttribute ValidationContext prioritization #9215

Open SKSniperSK opened 2 weeks ago

SKSniperSK commented 2 weeks ago

Bug type

Component

Component name

MudFormComponent

What happened?

Hi, while using MudBlazor with some more complex validations we experienced an issue when it comes to selecting the correct ValidationContext. Basically it comes down to the following line in the source code: https://github.com/MudBlazor/MudBlazor/blob/94d3a9ebce9df460b94f0dd8e2a1c009db3d5cdb/src/MudBlazor/Base/MudFormComponent.cs#L413

The question here is: should not the more specific _fieldIdentifier.Model of the input component be prioritized before the more general EditContext?.Model of the containing form when it comes to selecting the correct context? So switching to this:

var validationContextSubject = _fieldIdentifier.Model ?? EditContext?.Model ?? this;

Because when trying to create custom validations with nested models and based on other properties of the validated object, you might get into issues because by default you don't get the correct model and therefore can't access its properties.

We created a small sample that should demonstrate the issue as it only happens with the MudTextField and not with the default InputText.

https://try.mudblazor.com/snippet/QEwyYUPtJhjBbHOB

You can see that the exception is only thrown when updating the MudTextField (to "really" see it you either have to copy the source into your IDE and debug it or temporarily comment out the <DataAnnotationsValidator /> to see the exception) In the end the sample is still working as it seems to do multiple calls with different models but the logic still leads to issues within our more complex application and we can't really see the reason of the prioritization here.

Expected behavior

Prioritize the more specific validation context model of the input component over the more general one of the surrounding form.

Reproduction link

https://try.mudblazor.com/snippet/QEwyYUPtJhjBbHOB

Reproduction steps

Either: Copy the source code provided in the reproduction link into a sample page in your IDE and debug it (enter values in both input text fields and see that only the MudTextField will trigger the exception from the custom validator)

Or: Comment out the <DataAnnotationsValidator /> in the provided reproduction link, this will no longer show the validation notifications but you will get the exception when entering a short text in the MudTextField while the default InputText keeps working as expected.

Relevant log output

No response

Version (bug)

6.19.1

Version (working)

No response

What browsers are you seeing the problem on?

Chrome

On which operating systems are you experiencing the issue?

Windows

Pull Request

Code of Conduct

ScarletKuro commented 2 weeks ago

Hi.

Do you maybe want to create a PR?

SKSniperSK commented 2 weeks ago

Thanks for the quick reaction.

The technical change would be really a small thing (changing the one line I already mentioned) but I am not sure why this was done this way in the first place or if it would introduce new/other issues within the framework. This is why I wanted to leave this to more experienced people here. But if you would rather do the discussion within the PR instead of here then yes it would be possible to create the PR.