Blazored / FluentValidation

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

[Bug] Validations not working as expected. #104

Closed Sharaf-Mansour closed 2 years ago

Sharaf-Mansour commented 2 years ago

This is a screenshot from your demo at https://blazored.github.io/FluentValidation/ For Complex Types, Inputs only shows Validation Messages on submit (not by binding) image Expected Behavior should be this image I have installed your latest Version and have issue myself here image When it should say image My Repo can be found here https://github.com/Sharaf-Mansour/CVBuilder.

kylepope-ge commented 2 years ago

I ran into this same problem recently and (incorrectly) assumed it was due to the dotnet 6 upgrade of our blazor wasm app that I was doing at the same time. It looks like this is a new issue introduced in version 2.0.3 from fix in PR#91. I'm not exactly sure why that fix causes this new problem, but I can also repoduce this strange behavior where nested properties aren't validated onchange correctly.

Downgrading to 2.0.1 fixed the issue for me.

Sharaf-Mansour commented 2 years ago

Thanks for the help, but I am pretty sure that this issue needed to be addressed so it get fixes in the upcoming releases.

leonibr commented 2 years ago

I ran into this same problem recently and (incorrectly) assumed it was due to the dotnet 6 upgrade of our blazor wasm app that I was doing at the same time. It looks like this is a new issue introduced in version 2.0.3 from fix in PR#91. I'm not exactly sure why that fix causes this new problem, but I can also repoduce this strange behavior where nested properties aren't validated onchange correctly.

Downgrading to 2.0.1 fixed the issue for me.

Thanks for sharing, also worked for me

dubluxas commented 2 years ago

Same problem. Do not show messages in child components.

cbordeman commented 2 years ago

This solution only partially works. With components 2 or more levels deep, removing validation messages works with 2.0.1, but not adding them.

Sharaf-Mansour commented 2 years ago

@cbordeman How do you go levels deep ?

Option 1

RuleFor(x => x.Country).SetValidator(new CountryValidator());

Option 2

RuleFor(x => x.Country).Null().When(x=>x.Country.Id is 0).WithMessage("Country required.");

Option 3

RuleFor(x => x.Country.Id).SetValidator(new CountryIDValidator());

Option 4

 RuleFor(x => x.Country.Id).NotEqual(0).WithMessage("Country required.");
cbordeman commented 2 years ago

On both objects I'm doing something like: RuleFor(p => p.Street) .Cascade(CascadeMode.Stop) .MaximumLength(70).WithMessage("Too long") .NotEmpty().WithMessage("Required");

Sharaf-Mansour commented 2 years ago

Try Splitting it into 2 rules and not chain them.

 RuleFor(p => p.Street).MaximumLength(70).WithMessage("Too long");
 RuleFor(p => p.Street).NotEmpty().WithMessage("Required");

But I have a code like yours that work with 2.0.1

 RuleFor(x => x!.Value)
            .NotEmpty().When(x => x!.IsLink).WithMessage("Please enter Data")
            .MaximumLength(100).WithMessage("Maximum Length is 100 letter");

I have not tried

.Cascade(CascadeMode.Stop);

But I hope this helps...

cbordeman commented 2 years ago

I have other fields that are very simple and don't work:

RuleFor(p => p.FirstName).MaximumLength(25).WithMessage("Too long");

I removed the Cascade and broke down the others and it doesn't change anything.

Sharaf-Mansour commented 2 years ago

@cbordeman are you sending the parent model or the child model to your EditForm component ? Are you using static model or dependency injection?

cbordeman commented 2 years ago

The EditForm component is on the page, not on any subcomponent. The subcomponents just contain some divs with InputTexts and such on them for code reuse. I'm using the Model="app" approach, not EditContext='yada'.

I'm using WASM and letting FluentValidation scan the assemblies, if that's what you're asking about the static/di.

leonibr commented 2 years ago

Hey, @cbordeman, there is a running sample here: https://blazorrepl.telerik.com/QmaQbQle59JWatqq11 and it is working with 2 levels deep:

Person.Address.District 

Just hit 'Run' and then 'Save' to see validation working, if you have a different setup maybe you can reproduce it on blazorrepl and share here Screenshot - 2022-02-12T120415 754

cbordeman commented 2 years ago

@leonibr That's not 2 blazor components deep, that's a single binding in the top level page. I mean you have 1 page, which uses a component, and that component uses a second component.

leonibr commented 2 years ago

@cbordeman , ok then, I refactored to use 2 nested blazor components and it is working properly, maybe you are not passing the top-level EditContext down to the components Check it out: https://blazorrepl.telerik.com/mmEmPcPh34yKeTKU4 image

cbordeman commented 2 years ago

That fiddle is empty.

Sharaf-Mansour commented 2 years ago

@cbordeman I have an app that goes 3 or maybe 4 levels deep with validations here Source code here It has a custom Address component with custom validation It might be good of use It is for sure not the simplest sample but it is what I have available and working as expected ATM Note: Validations are inside ModelValidators Folder and FluentValidationValidator can be found inside CVForm.Razor

cbordeman commented 2 years ago

The problem is that validations remove w/o issue, but don't add (or don't display visually) per-field. When you tab out of the field (such as removing the text from a .NotEmpty() field. It only happens on InputTexts on the third level of depth (or page + component + component). That source doesn't do it, either.

leonibr commented 2 years ago

@cbordeman sorry about that: https://blazorrepl.telerik.com/QmOmPQFt37aha3th17

Sharaf-Mansour commented 2 years ago

@cbordeman You mean that you want the validation start on input focus not binding ? Or you want it to be removed on blur ?

cbordeman commented 2 years ago

Just standard behavior for .NotEmpty(). If there's NOT a validation error showing, then when you remove the value in the input and press TAB, it should appear. The opposite already works fine (type a value and it disappears). This works properly on the parent component and directly on the page.

Sharaf-Mansour commented 2 years ago

@cbordeman Tabbing from an input does not trigger the validation because it validates on binding or submitting. If you want it to trigger on Tab or On Focus / On Blur then you will have to create some sort of code that triggers the binding (Shows a single error message) or the Form Submit. (Show all errors messages)

Sharaf-Mansour commented 2 years ago

@cbordeman after doing a little research, You can trigger

editContext.Validate();

Docs can be found here

cbordeman commented 2 years ago

@Sharaf-Mansour I don't mean an actual html input tag, I mean a component, so I don't need to manually call .Validate(). My code seems to be identical in structure to the fiddle @leonibr posted. I don't know why hers doesn't exhibit this behavior! @leonibr what version of Blazored.FluentValidation are you using? I'm using 2.0.1 since 2.0.3 introduced a bug.

I added a ValidationSummary to mine and that seems to work as expected, both removing and re-adding errors as expected. IT's just the <ValidationMessage below the that doesn't show the message onblur, (unless you manually invoke .Validate() or click the Submit button ofc).

cbordeman commented 2 years ago

I've figured it out. I subclassed both my validator classes. If I get rid of the subclassing, all works well.

For example, I have AddressValidatorBase and subclasses AddressValidator and AddressValidatorWithRequiredCounty. Easy enough to fix, but it is a bug I think. I tried to do this with rulesets earlier, but had no way to change the call with this package. I also tried passing as a ctor param but FluentValidations couldn't construct it.

Sharaf-Mansour commented 2 years ago

I found a bug as well in my own project. I made 2 models one inherit the other. then made 2 validators one for the parent and other for the chilled. In the parent I set rule for name to print (Please enter Parent Name) And for the child name I set (Please enter child name). When I submit empty text in child I get please enter child name as expected But if I type something and then remove it.... It says Please enter Parent name.. But get fixed on invalid submit to Please enter child name... as a easy fix I just changed the msg to just (Please enter name) But I think the issue is with the assembly scanning tech not really in the validators.

cbordeman commented 2 years ago

Possibly.

cbordeman commented 2 years ago

In both cases of my Address and Person validator subclass, I'm setting them explicitly in every instance they're needed, using .SetValidator(new...). So it's not selecting the wrong classes in my bug, seems like something else.

andrewaggb commented 2 years ago

I'm also having this problem. the issue for me seems to be in this function. In my case the field is part of a subobject. So something like Person.Address.Street but I'm using a single validator for the whole thing. The full model validation works but the ValidateField doesn't work as-is. If I change the fieldIdentifier.FieldName to be "Address." + fieldIdentifier.FieldName it works as expected. I think fieldIdentifier has the object name and I know the overall model name so maybe I can do something with that. I think I'll look at the other two blazor fluent validation projects and see how they implemented ValidateField.

private static async void ValidateField(EditContext editContext, ValidationMessageStore messages, FieldIdentifier fieldIdentifier, IServiceProvider serviceProvider, bool disableAssemblyScanning, IValidator validator = null) { **var properties = new[] { fieldIdentifier.FieldName };**

andrewaggb commented 2 years ago

Well it's probably not the best/most elegant way but this is what I came up with for my particular problem. Basically if the fieldIdentifier.Model isn't the same object as the editContext.Model I look through properties of the editContext.Model for ones of the same type and see if they are the same object and build a properyname based on that. I also added a second pass to go one level deeper so I can handle (Model.Class1.Class2.Property). In my case I wanted it all in a single validator and not a separate validator for class1 and a separate one for class2 as the business rules between it all is quite complex. I could have reorganized it into separate validators that took the parent(s) or other objects into the separate validators (so I have the required information for the validation rules) and that may have worked without this change. I also modified this line messages.Add(fieldIdentifier, validationResults.Errors.Where(x => properties.Contains(x.PropertyName)).Select(error => error.ErrorMessage)); to check that the PropertyName was one I was expecting. In my case I had a single validation rule for the root object that was also returned and I didn't want that message displayed on this property.

`private static ConcurrentDictionary<Type, System.Reflection.PropertyInfo[]> TypeProperties = new ConcurrentDictionary<Type, System.Reflection.PropertyInfo[]>(); private static async void ValidateField(EditContext editContext, ValidationMessageStore messages, FieldIdentifier fieldIdentifier, IServiceProvider serviceProvider, bool disableAssemblyScanning, IValidator validator = null) { string[] properties = new[] { fieldIdentifier.FieldName }; if (fieldIdentifier.Model != editContext.Model) { var editContextType = editContext.Model.GetType(); if (!TypeProperties.TryGetValue(editContextType, out var editContextProperties)) { editContextProperties = editContextType.GetProperties(); TypeProperties.TryAdd(editContextType, editContextProperties); } var fieldType = fieldIdentifier.Model.GetType(); bool found = false; foreach (var editContextProperty in editContextProperties.Where(x => x.PropertyType == fieldType)) { try { if (editContextProperty.GetValue(editContext.Model) == fieldIdentifier.Model) { properties = new[] { $"{editContextProperty.Name}.{fieldIdentifier.FieldName}" }; found = true; break; } } catch (System.Reflection.TargetException) { } catch (Exception ex) { Console.WriteLine($"{editContextProperty.Name} {ex}"); } } if (!found) { foreach (var editContextProperty in editContextProperties.Where(x => x.PropertyType.IsClass)) {

                    if (!TypeProperties.TryGetValue(editContextProperty.PropertyType, out var subClass1Properties))
                    {
                        subClass1Properties = editContextProperty.PropertyType.GetProperties();
                        TypeProperties.TryAdd(editContextProperty.PropertyType, subClass1Properties);
                    }
                    foreach (var subClass1Property in subClass1Properties.Where(x => x.PropertyType == fieldType))
                    {
                        try
                        {
                            if (subClass1Property.GetValue(editContextProperty.GetValue(editContext.Model)) == fieldIdentifier.Model)
                            {
                                properties = new[] { $"{editContextProperty.Name}.{subClass1Property.Name}.{fieldIdentifier.FieldName}" };
                                found = true;
                                break;
                            }
                        }
                        catch (System.Reflection.TargetException) { }
                        catch (Exception ex)
                        {
                            Console.WriteLine($"{editContextProperty.Name}.{subClass1Property.Name} {ex}");
                        }
                    }
                    if (found) break;
                }
            }
        }
        var context = new ValidationContext<object>(editContext.Model, new PropertyChain(), new MemberNameValidatorSelector(properties));

        validator ??= GetValidatorForModel(serviceProvider, editContext.Model, disableAssemblyScanning);

        if (validator is object)
        {
            var validationResults = await validator.ValidateAsync(context);

            messages.Clear(fieldIdentifier);
            messages.Add(fieldIdentifier, validationResults.Errors.Where(x => properties.Contains(x.PropertyName)).Select(error => error.ErrorMessage));

            editContext.NotifyValidationStateChanged();
        }
    }`
andrewaggb commented 2 years ago

I ran into an unexpected issue with my code above. the model passed by the fieldIdentifier wasn't always the one from my edit content. for fieldIdentifiers I created in my razor components and manually trigger it looks fine but for others it isn't. If I use the ToFieldIdentifier code , eg var fieldIdentifier = ToFieldIdentifier(editContext, properties[0]); then adding the messages works but then I can't use the object matching technique I was using before (assuming that fieldIdentifier.Model would be part of the editContext) to build the property name. I'm not sure if I'll get around to looking at it further.

ThumbGen commented 2 years ago

I'm using 2.0.1 since 2.0.3 introduced a bug.

Is the latest preview fixing the bug introduced by 2.0.3? After going back to 2.0.1 nested validation works for me again (was broken in 2.0.3)

leonibr commented 2 years ago

yes, as mention by @chrissainty, it was solved on PR #132 , this was on release v2.1.0-preview.1