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.

Sign up for GitHub

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

Jump to bottom

Combine multiple ValidationResults #2125

Closed
leandromoh opened this issue Jul 21, 2023 · 10 comments · Fixed by #2140
Closed

Combine multiple ValidationResults #2125

leandromoh opened this issue Jul 21, 2023 · 10 comments · Fixed by #2140
Milestone
11.7

Comments

@leandromoh
Copy link

leandromoh commented Jul 21, 2023 •
edited
Loading

Is your feature request related to a problem? Please describe.

No, just a useful method that I could not find in the lib.
Is there any "native" way to combine/join multiples ValidationResult's?

Describe the solution you'd like

It is very useful to combine/join mutiples ValidationResults into a new ValidationResults, for example..

  ValidationResult fullValidation = new[]
  {
      ValidationA(obj),
      ValidationB(obj),
      ValidationC(obj),
  }
  .Combine();

ValidationResult ValidationA(Person x) { ... }
ValidationResult ValidationB(Person x) { ... }
ValidationResult ValidationC(Person x) { ... }

Describe alternatives you've considered

In my current system I've added the following extension methods.
If there are interest to make them part of the library (in case of no current native solution) I can submit a PR.

public static ValidationResult Combine(this ValidationResult validationResult, params ValidationResult[] failuresResult)
{
    return failuresResult.Append(validationResult).Combine();
}

public static ValidationResult Combine(this IEnumerable<ValidationResult> failuresResult)
{
    var errors = failuresResult.SelectMany(x => x.Errors);

    return new ValidationResult(errors);
}

Additional Context

No response
@JeremySkinner
Copy link
Member

I'm not keen on a Combine method, but I could add another constructor to ValidationResult that takes an IEnumerable<ValidationResult> if that'd be helpful?

@leandromoh
Copy link
Author

leandromoh commented Jul 25, 2023 •
edited
Loading

I'm not keen on a Combine method, but I could add another constructor to ValidationResult that takes an IEnumerable<ValidationResult> if that'd be helpful?

Sure @JeremySkinner ! It would help a lot.
Any interest in add these extension methods in the lib ? I think it is worth.

@JeremySkinner JeremySkinner changed the title Combine multiple ValidationResult's Combine multiple ValidationResults Aug 1, 2023
@JeremySkinner JeremySkinner added this to the 11.7 milestone Aug 7, 2023
JeremySkinner added a commit that referenced this issue Aug 8, 2023
@JeremySkinner
Add constructor to ValidationResult that can be used to combine other…
1f9102d 
… ValidationResults (#2125)
@JeremySkinner
Copy link
Member

Added the new constructor in 1f9102d. This will go in the 11.7 release.

I'm not keen on including the extension methods in the library, I don't feel they're necessary, but you can continue to use them within your own project.

@leandromoh
Copy link
Author

@JeremySkinner thanks for the improvement (:

@JeremySkinner
Copy link
Member

JeremySkinner commented Aug 11, 2023 •
edited
Loading

11.7.0 is now released with this change

@cremor
Copy link
Contributor

cremor commented Aug 11, 2023

Added the new constructor in 1f9102d. This will go in the 11.7 release.

@JeremySkinner What about the RuleSetsExecuted property? Do you not have to combine those too?

@JeremySkinner
Copy link
Member

@cremor Yes you’re right, I missed that. I’ll try and push out a further update to resolve that when I get some time, or if you’d like to submit a PR that’d be very welcome

@JeremySkinner JeremySkinner reopened this Aug 11, 2023
@cremor
Copy link
Contributor

cremor commented Aug 11, 2023

I'm not really sure what the correct implementation would be. Maybe this?

RuleSetsExecuted = otherResults.Where(x => x.RuleSetsExecuted != null).SelectMany(x => x.RuleSetsExecuted).Distinct().ToArray();

@JeremySkinner
Copy link
Member

Yep I think that should work

@cremor cremor mentioned this issue Aug 12, 2023
Merged
@JeremySkinner
Copy link
Member

I've pushed out 11.7.1 with this fix

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
11.7
Development

Successfully merging a pull request may close this issue.

Also combine RuleSetsExecuted in ValidationResult combine ctor cremor/FluentValidation
3 participants
@JeremySkinner @cremor @leandromoh