Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make full validation result details available (also per field) #212

Merged
merged 2 commits into from
Feb 18, 2024
Merged

Conversation

luetm
Copy link
Contributor

@luetm luetm commented Feb 14, 2024

It looks like this was once possible (see #75 ). I think the simplest way to add this is to just make the ValidationFailures available, so if anyone needs anything else from it (not just Severity) they can.

The approach is easy, I just save the latest validation run for a FluentValidationValidator and made a method to easily access it (GetFailuresFromLastValidation).

I tried to follow the style as well as I could, and I added a documentation paragraph about it, which I can remove if that's not desired.

Closes #75

@luetm luetm mentioned this pull request Feb 17, 2024
Copy link
Member

@chrissainty chrissainty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @luetm this looks good to me. If you wanted bonus points it would be great to add some tests--it's been on my todo for far too long. 😉

@chrissainty chrissainty added the Feature New feature that will be added to the project label Feb 18, 2024
@chrissainty chrissainty merged commit a6291d2 into Blazored:main Feb 18, 2024
1 check passed
@luetm
Copy link
Contributor Author

luetm commented Feb 18, 2024

Sure, I'll look into it!

@luetm luetm mentioned this pull request Feb 20, 2024
@Liero
Copy link

Liero commented Feb 26, 2024

@luetm : I just had a chance to look at the PR only now and would suggest to store the last FluentValidation's ValidationResult in EditContext instead UI component.

EditContext.Properties are made exactly for such cases

The reason is that EditContext can be used in component-free UI layer like ViewModels. I wouldn't recommend to keep reference to FluentValidator component in a viewmodel.

Also lifetime of EditContext might be different than UI Components. I used EditContext like this:

class ViewModel 
{
     public ViewModel()
     {
         EditContext = new EditContext(this);
         //subscribe for EditContext's events, maybe perform initial validation, etc...
     }
     
     public EditContext EditContext {get;}
}
<EditForm Context="_viewModel.EditContext">

I also recommend you to add option to format different severity levels differently. Without it, the UX will be quite poor.
For example add [Warning] prefix

@luetm
Copy link
Contributor Author

luetm commented Feb 26, 2024

@Liero OK, should I do a PR that undoes this PR and implements it in the other way? I like the idea!
Edit: Is there a discord or another community hub we can discuss this?

@Liero
Copy link

Liero commented Feb 26, 2024

@luetm: Not my choice, I am not even contributor here. I made my own fluentvalidator long time ago, since I needed special features back then, but I'm happy they are being ported here. I'm just sharing my own experience

@luetm
Copy link
Contributor Author

luetm commented Feb 26, 2024

@Liero I'm not sure what you mean by this

I also recommend you to add option to format different severity levels differently. Without it, the UX will be quite poor.
For example add [Warning] prefix

Can you explain further please?

@chrissainty
Copy link
Member

@Liero would you be able to create an issue covering the points you made?

@luetm If Liero can do that you could raise a PR against that new issue.

@Liero
Copy link

Liero commented Feb 26, 2024

@chrissainty:

I'm not sure what you mean by this
For example, try to display warnings with orange text color so that user knows it is a warning.

Check demo and source code: https://github.com/Liero/vNext.BlazorComponents.FluentValidation
Feel free to use it.

It's OT, but if I may: I think the entire EditContextExtensions class lost it's point.

It only makes sense when it its independent of the FluentValidator UI component, but that is not the case here. The original extension allowed to add fluent validations without FluentValidator component.

Either create something like FluentValidatorOptions property that would be a parameter to EditContextExtensions, or move all the code to FluentValidator UI component.

I'm mentioning FluentValidatorOptions because it is also a good way of accessing last ValidationFailure or asynchronous Task<ValidationFailure> that can be awaited

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New feature that will be added to the project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request] Severity
3 participants