DisplayNameKey or Metadata requirement?

[jsobell]

A situation came up last night where @mroseboom wanted the following code:

.ensure((p: Person) => p.firstName).displayName('common.labels.firstname').required()

The display name is translated, but it's clearly not using the property name as a key, and it's standard to have messages looked up by a key, so should we have the same thing for the DisplayName?
It currently only performs translation if a property name is supplied, but displayName is left null:
https://github.com/aurelia/validation/blob/master/src/implementation/standard-validator.ts#L30

If we had a .metadata('') that allowed arbitrary data to be associated with the rule.property then I suppose people could leave the displayName blank and use that if they chose, and it would give lots of flexibility, but it's not very elegant.

I considered .displayNameKey() in the same way we have a messageKey, but messageKeys are centralised, whereas the displayNameKey is simply a function intercept for multilingual purposes.

Perhaps the easiest thing is to simply say

    if (displayName !== null || propertyName !== null) {
      displayName = this.messageProvider.getDisplayName(displayName || propertyName);
    }

[mroseboom]

The above solution that @jsobell mentioned works for me. Sorry for the late response!

if (displayName !== null || propertyName !== null) {
  displayName = this.messageProvider.getDisplayName(displayName || propertyName);
}

[jdanyow]

How about .displayKey(key: string):

ValidationRules
  .ensure('fname').displayKey('common.labels.firstName')
    .required()
    .minLength(3)
  ...

Then we'd add a second argument to the message provider's getDisplayName method:

getDisplayName(propertyName: string, displayKey: string = null)

Thoughts?

[jsobell]

We could, although I suspect that displayName and displayKey are mutually exclusive, and if we're providing a hook to allow the dev to choose whether or not to translate then we might as well leave it as displayName. People will either use displayName in a format they can use for translation, or in common format as a non-translatable field.
The main difference between DisplayName and Message is that Messages can contain expressions.
If we are saying that DisplayName must be resolved to the final string, which seems reasonable to me, I'd say we simply make the code change I suggested so people can apply translation of any DisplayName if they choose, and we don't create yet another property against each field.

I can see a common situation being that people have a DisplayName of 'Surname', and in their mutation code they look up a translation, and if one doesn't exists they leave the string unchanged.
Since adding a displayKey simply says "You must look up this as a translation", then if they really want to enforce it they simply throw an error if there is no translation to the provided DisplayName/Property.

[jdanyow]

although I suspect that displayName and displayKey are mutually exclusive

right- that's what I was thinking

---

I don't want to do displayName = this.messageProvider.getDisplayName(displayName || propertyName);

This makes it impossible for getDisplayName to determine whether it's dealing with a displayName or a propertyName.

---

My thinking was .displayName('some name') would continue to enable circumventing the message provider's display name logic. When it's not used, or a .displayKey(key) hint is used, the message provider determines the property's display name.

[jsobell]

This makes it impossible for getDisplayName to determine whether it's dealing with a displayName or a propertyName.

Does this matter? Can you think of a use-case where the user might need to know?
I'm just thinking that if the dev has created the rules, he would have decided if he wants to use translations or not. If so, he can choose to do everything by property name by never specifying the displayName, or can override some properties by using displayName as an 'override' or alias for the propertyname.
e.g. Translations exist for 'email', some properties are 'email', while some are called 'emailaddress' so the dev adds .displayName('email')
The only time they might care is if they for some reason have a display name that is not wanted in a translation, but I can't think why or how that might happen. Even if they have no control over the object, they must have control of the validation rules so they will simply add .displayName to have a standard set of keys for translation.
It certainly does no harm to add the displayKey, but I suspect it may be superfluous.

If we really want to provide flexibility and support all possibilities, we should have a .settings(s: any) in the same way as routes, so the dev can associate anything they like with the ruleset. Of course this means things like getDisplayName would need to have the ruleset so they could read the settings, and not just the name...

[jdanyow]

I think ...getDisplayName(displayName || propertyName) leads to unnecessary ambiguity in the implementations (is it a property name or a display name?) and removes the ability to slam in a .displayName('my custom name') to completely circumvent the getDisplayName logic. It seems to me displayKey(key) and getDisplayName(propertyName: string, displayKey: string = null) preserves the explicitness without making any extra work for anyone.

[jsobell]

OK, I don't think it's a huge issue.
As I said, I'm not sure why people would ever want to circumvent the getDisplayName, and it does mean anyone adding translations will have to replace all their displayName('First name') with displayName('First name').displayKey('firstname').
Re the ambiguity, definitely, if people use First name as their key for translation it will be confusing, but if they user person.firstname or @firstname (using a convention) it wouldn't be an issue. Perhaps this is more of a 'best practise' problem than a technical one?

Why not getDisplayName(propertyName: string, displayName: string = null, displayKey: string = null)?
I can see everyone's code containing

return Translate(displayKey || displayName || propertyName)

:)

[valichek]

I have almost the same issue. Having .displayKey() combined with getDisplayName(propertyName: string, displayName: string = null, displayKey: string = null) or getDisplayName(propertyName: string, displayKey: string = null) is good option. I would add here some generator for displayKey, so you have option not only to set it explicitly, but to generate it using propertyName. Here is my possible use case:

ValidationRules
  .ensure('fname').displayName('First Name')
  .on(target)
  .getDisplayKey((propertyName, displayKey) => {
      return displayKey || `common.customer-form.labels.${propertyName}`
  })

[valichek]

Also I suggest getDisplayName(propertyName: string, displayName: string = null, displayKey: string = null) is better option, because it makes possible to override translation locally setting displayName or use displayName when translation is not available.

[valichek]

And maybe having access to the object in getDisplayName() would be useful too, so one can use own meta implementation.

[fabioluz]

Any update here? I've just created a resourceName() that is pretty much the displayKey() aforementioned. I was going to open a new issue to discuss about this enhancement then I found this one!

[jdanyow]

@fabioluz we're definitely going to implement this- did the solution discussed here cover your need?

[fabioluz]

Yeah, it did cover my need. My solution's code is a bit different the but result is the same.

[jdanyow]

Added this in 4120e25

Went with @jsobell's approach of passing displayName to the message provider's getDisplayName method.