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

Provide a convenient way to create a Result when using fluentvalidation #16

Closed
SimonGeering opened this issue May 15, 2020 · 8 comments
Closed
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed

Comments

@SimonGeering
Copy link
Contributor

Create an extension method or some other mechanism that provides an easy way to convert from a FluentValidation.ValidationResult to an Ardalis.Result

See https://github.com/FluentValidation

This will be implemented with a new project to prevent anyone from needing to take an implicit dependency on the FluentValidation package if they only want to use the main Ardalis.Result package

@ardalis ardalis added enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed labels May 15, 2020
@SimonGeering
Copy link
Contributor Author

My initial investigations in converting the ValidationResult from FluentValidation into an Ardalis.Result have shown some key differences in design which need some clarification and direction to proceed.

Their ValidationResult exposes an IList<ValidationFailure> as the equivalent of where Ardalis.Result currently uses Dictionary<string, string>.

The trouble here is they provide a much richer model in their ValidationFailure class, primarily due to the constraints imposed by the need to convert an Ardalis.Result to an IActionResponse

I guess this raised some fundamental questions about what ValidationError on a result should be for and fundamentally if the intent of the project is to provide a Result type intended for REST API compatibility or for more general use within a Domain model.

In either case, consideration would also need to be given to the following:

  1. Their ValidationFailure exposes a Severity property as an enum for Error, Warning and Informational failures.

    Do we differentiate between Validation Errors, Validation Warnings and Validation Informational Messages? These are all things that make sense once you consider the UX around presenting validation Result back to a user.
    One possible solution here might be to provide additional ValidationWarnings and ValidationMessages properties of Dictionary<string, string> but it is not obvious how this might be handled by Ardalis.Result.AspNetCore.ResultExtensions, as it is making use of ModelState.AddModelError.

    While some form of "encoding" of messages by convention, such as a prefix to the message string passed to ModelState.AddModelError may be an option, it's not one which I'm a fan of as it does not answer the fundamental question about Result and WebAPI; namely, should we be trying to remain restful and ovoid any solution that turns Ardalis.Result into an implicit Envelope wrapper.

  2. Do we care about the difference between form vs field-level validation? In the case of the later, there is another consideration for validation rules that apply across fields or to the whole entity such that the reality is both form and field-level validation on the same entity might apply.

    E.g validating a TODO item might have a field level rule saying a description is a required field. A multi-field rule saying its start date must be before its end date and vice versa, and an entity level validation that an associated prerequisite entity should have an earlier due date.

    This is viable in FlientValidation's ValidationFailure as it exposes an optional PropertyName property which can be populated for FieldLevel validation messages.

At the end of the day, this all comes back to if the design of Result should be limited by a requirement to be compatible with WebAPI and if it should or shouldn't become an envelope in that context especially when considered in the context of a RESTFUL approach.

@ardalis
Copy link
Owner

ardalis commented Aug 20, 2020

Revisiting this. For now, I think it's helpful to implement multiple string-based messages in a set of validation errors. I'll accept a PR with just that shortly (I think #24 is out of date so I'll probably take a new one but the end result should be the same).

I think the action item from this issue (16) should be to create new samples:

  1. Using Ardalis.Result with FluentValidation in a domain model (regardless of UI)
  2. Using Ardalis.Result with FluentValidation and web APIs (including the translation filter).

I think the course of implementing these two samples we will uncover specific problems/incompatibilities as you've described above and we'll address them in that context.

@SimonGeering
Copy link
Contributor Author

Sorry I got distracted by other work and let this go asside. Agree with the above and will wait for that before looking at this any further.

@SimonGeering
Copy link
Contributor Author

SimonGeering commented Aug 24, 2020

FYI - I have integrated the latest package into my project without issue (see this commit
da11beb
)

Until Ardalis.Result.FluentValidation is available as a NuGet package I have cloned FluentValidationResultExtensions.cs in my project.

This has saved about 10 lines of code per API call! thanks 👍

@ardalis
Copy link
Owner

ardalis commented Aug 25, 2020

Awesome, glad to hear it!

Why did you need to clone the extensions vs. using them from the package, here:
https://github.com/ardalis/Result/blob/master/src/Ardalis.Result.FluentValidation/FluentValidationResultExtensions.cs
?

I assume you have some changes you need to make that are specific to your project?

@SimonGeering
Copy link
Contributor Author

I thought it was a separate NuGet package? Couldn't find it listed as such.

@ardalis
Copy link
Owner

ardalis commented Aug 25, 2020

Oh right, sorry. It's waiting on me to fix the deploy by updating my nuget.org key. I'll try to knock that out today and will confirm here when the package is published.

@ardalis
Copy link
Owner

ardalis commented Aug 25, 2020

Published here now:
https://www.nuget.org/packages/Ardalis.Result.FluentValidation/

Should be available momentarily.

@ardalis ardalis closed this as completed Dec 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants