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

Already on GitHub? Sign in to your account

DataAnnotations validators don't use the display name in error messages #1469

Closed
xt0rted opened this Issue Mar 1, 2014 · 5 comments

Comments

Projects
None yet
4 participants
Contributor

xt0rted commented Mar 1, 2014

I'm working on a Nancy.Validation.DataAnnotations.Extensions project to add support for the .net 4.5 validation attribtutes and I found that even when I add [DisplayName("Some Field Name")] to my property the error message is still using the property name instead of Some Field Name.

I had to override the Validate function as follows and this gets me the display name if it's available.

public override IEnumerable<ModelValidationError> Validate(object instance, ValidationAttribute attribute, PropertyDescriptor descriptor)
{
    var context = new ValidationContext(instance, null, null)
    {
        DisplayName = descriptor == null ? null : descriptor.DisplayName,
        MemberName = descriptor == null ? null : descriptor.Name
    };

    if (descriptor != null)
    {
        instance = descriptor.GetValue(instance);
    }

    var result = attribute.GetValidationResult(instance, context);

    if (result != null)
    {
        yield return new ModelValidationError(result.MemberNames, attribute.FormatErrorMessage(context.DisplayName));
    }
}

I'm not sure if this is a general enough fix to add to the DataAnnotationsValidatorAdapter. The changes were adding

DisplayName = descriptor == null ? null : descriptor.DisplayName,

and changing

string.Join(" ", result.MemberNames.Select(attribute.FormatErrorMessage))

to

attribute.FormatErrorMessage(context.DisplayName)
Contributor

xt0rted commented Mar 1, 2014

Just realized that the yield can be simplified to this which should make it generic enough to work with all but the Compare attribute.

yield return new ModelValidationError(result.MemberNames, result.ErrorMessage);

The reason this won't work with the Compare attribute is someone had the bright idea to not pass the member names into the ValidationResult object so it's always empty 😠

Ref: here and here

Owner

thecodejunkie commented Mar 1, 2014

Sounds like something we should have in the core of Nancy.Validation.DataAnnotations. If the attribute is there, then we use it, if not then we use the property name. You want to send a pull-request and we can pull this in for the next release?

Thanks

@thecodejunkie thecodejunkie added this to the 0.23 milestone Mar 1, 2014

@thecodejunkie thecodejunkie self-assigned this Mar 1, 2014

Contributor

xt0rted commented Mar 1, 2014

Sure, I'll work on it this weekend and send it in. #1029 should be fixed by this too.

Should these be turned back on now and new tests added?

Also what's the GetRules() function used for on DataAnnotationsValidatorAdapter?

Owner

thecodejunkie commented Mar 1, 2014

Oh yeah, those tests should be brought back ;)

The GetRules() method is used to get the rules that apply, so that we can use those for things like generating client-side (currently being spiked) validation based on model rules

Contributor

xt0rted commented Mar 5, 2014

Until #1473 can be merged I'm manually handling the display name attribute in my adapters. An initial release can be seen here. This also demonstrates the need for the second commit in that PR. Any feedback would be appreciated.

@thecodejunkie thecodejunkie modified the milestones: 0.24, 0.23 Jun 4, 2014

@grumpydev grumpydev modified the milestones: Future, 0.24 Jun 10, 2014

@khellang khellang removed this from the Future milestone Aug 19, 2015

@khellang khellang added this to the 1.4 milestone Oct 6, 2015

@khellang khellang closed this in #1473 Oct 23, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment