Skip to content

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);
}

@github-actions
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
@KyleMcMaster KyleMcMaster deleted the errors-as-readonly-collections branch March 12, 2024 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in 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.

3 participants