Skip to content
New issue

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

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

Already on GitHub? # to your account

Expose ValidationErrors as IEnumerable to Prevent Side Effects #169

Merged
merged 5 commits into from
Mar 12, 2024

Conversation

KyleMcMaster
Copy link
Collaborator

Sample unit test showing behavior

[Fact]
public void DoesNotAddValidationErrorToValidationErrors()
{
    var result = Result.Invalid(new List<ValidationError>());

    var errors = result.ValidationErrors.ToList();
    errors.Add(new ValidationError());

    result.Status.Should().Be(ResultStatus.Invalid);
    result.ValidationErrors.Should().HaveCount(0);
    errors.Should().HaveCount(1);
}

Copy link

github-actions bot commented Mar 9, 2024

Code Coverage

Package Line Rate Branch Rate Complexity Health
Ardalis.Result.Sample.Core 18% 18% 63
Ardalis.Result.FluentValidation 86% 50% 6
Ardalis.Result 21% 0% 82
Summary 23% (95 / 416) 17% (9 / 54) 151

@KyleMcMaster KyleMcMaster marked this pull request as ready for review March 10, 2024 02:04
@ardalis
Copy link
Owner

ardalis commented Mar 12, 2024

Should we have more options for including correlation ids? And if so, rather than having separate methods for all of them, would it make sense to bake it into a single parameter object, like:

public record ErrorList(IEnumerable<string> ErrorMessages, string? CorrelationId);

?

Then the factory methods would just take in (ErrorList errorList) and we wouldn't need any overloads or alternate methods.

@KyleMcMaster
Copy link
Collaborator Author

KyleMcMaster commented Mar 12, 2024

Should we have more options for including correlation ids? And if so, rather than having separate methods for all of them, would it make sense to bake it into a single parameter object, like:

public record ErrorList(IEnumerable<string> ErrorMessages, string? CorrelationId);

?

Then the factory methods would just take in (ErrorList errorList) and we wouldn't need any overloads or alternate methods.

I agree we should try to pass CorrelationId whenever we can, I'll update the methods in a separate PR.

@KyleMcMaster KyleMcMaster merged commit c5966e6 into main Mar 12, 2024
1 check passed
@KyleMcMaster KyleMcMaster deleted the errors-as-readonly-collections branch March 12, 2024 13:37
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change the Status to Failed when any ValidationError is added to the Result.
2 participants