Make messages live-updateable and maybe separate the validation and message creation in ember-cp-validations

Hello

Yesterday I've talked with @offirgolan about that on slack and he recommended me to open an issue here so we can discuss this.

My Issue was that when I change the locale, the translated validation messages don't change. The recommended solution is to make the locale as an dependency of the validators, however this seems not an ideal solution, and is not practical at all when a validator makes an AJAX request.

The following instance-inistializer is a solution that works for me, but it has some caveats:

import Ember from 'ember';
import ValidationError from 'ember-cp-validations/validations/error';

const {get} = Ember;

export function initialize(applicationInstance) {
  const i18n = applicationInstance.lookup('service:i18n');

  ValidationError.reopen({
    i18n,
    formatted: Ember.computed('type', 'attribute', 'i18n.locale', {
      get() {
        const attribute = get(this, 'attribute');
        const type = get(this, 'type');
        return i18n.t(`errors.${type}`, {
          description: i18n.t(`fields.${attribute}`)
        });
      }
    }),
  });
}

export default {
  name: 'ember-cp-validations-i18n',
  initialize
};

The biggest problem is that the descriptionKey is not available. Also its a big ugly that the owner is not injected into the ValidationError. This forces me to use an instance-initializer instead of an initializer.

Also messages and message is not longer useful for me.

Also I think it would be awesome to have a clean and documented way of doing what I want to do.

My thought was that we could look up the ValidationError class from the DI container, create an instance for each error and then fill it will all information about the failed validation:

The type
The field
The value
A validationResult which is currently the message
The options passed to the validator. Here we can look up the descriptionKey and so on.

Next the message could be just a computed property because computed properties are awesome. This would be our implementation for compatibility with the current implementation:

message: Ember.computed('validationResult', {
  get() {
    return get(this, 'validationResult');
  }
});

However the user could always override this by implementing app/validations/error.js and doing something like this:

i18n: Ember.inject.service(),
message: Ember.computed('type', 'field', 'i18n.locale', {
  get() {
    return get(this, 'i18n').t(`errors.${get(this,'type')}`, { description: get(this, 'field') });
  }
});

Next we would change messages to be a CP as well:

messages: Ember.computed(‚errors.@each.message`, {
  get() {
    return get(this, ‚errors').map(err => get(err, ‚message');
  }
});

In the next step we could think about shifting the validators from returning a human-readable error to returning a machine-readable error that we can use in the ValidationError for formatting and internationalization. However this would be a change of public APIs.

What do you guys think about this approach?

The recommended solution is to make the locale as an dependency of the validators, however this seems not an ideal solution, and is not practical at all when a validator makes an AJAX request.

That really depends on what you're validating and if what you're validating is dependent on the locale. So, I would say perhaps making it an option to revalidate on a dependent key change versus recomputing a message needs to be decoupled.

Perhaps something like:

validator('date-range', {
  dependentKeys: ['startDate', 'endDate'],
  messageDependentKeys: ['i18n.locale'] // we can do this internally for ember-i18n-cp-validations
});

In any case, I think changes on both ends would be needed to support this

Also its a big ugly that the owner is not injected into the ValidationError

This isn't a public API, so expectations should be limited on how you think it should behave versus what it was intended to solve.

Also messages and message is not longer useful for me.

Unsure what you mean by this.

In the next step we could think about shifting the validators from returning a human-readable error to returning a machine-readable error that we can use in the ValidationError for formatting and internationalization. However this would be a change of public APIs.

I'm not sure I understand this either.

That really depends on what you're validating and if what you're validating is dependent on the locale.

Do you mean like for example for phone numbers, where you want to allow local numbers depending on the locale? Well my suggestion wouldn't break this, because this is existing functionality. But for most validators this is not required.

This isn't a public API

Thats exactly my suggestion. I think it should be public API.

Also messages and message is not longer useful for me.

Well, with my current solution (the snippet above) doing

{{#each model.validations.messages as |m|}}
  {{m}}
{{/each}}

will no longer produce the same as

{{#each model.validations.errors as |e|}}
  {{e.message}}
{{/each}}

which would be desirable.

In the next step we could think about shifting the validators from returning a human-readable error to returning a machine-readable error that we can use in the ValidationError for formatting and internationalization. However this would be a change of public APIs.

Assume we have a length validator with minLength and maxLength. It should produce validation errors like Name should be at least 5 characters long. or Name should not be longer then 10 characters..

Now we would like to have translations like this:

length: {
  tooShort: "Name should be at least {{minLength}} characters long.",
  tooLong: "Name should not be longer then {{maxLength}} characters.",
}

Now this doesn't work with my simple initializer above because the error object has not the required information to decide which message to print. However this can be returned by the validator. So instead of returning the message, we could return the options/validatorResult required by the ValidationError to create the message. Something like this:

{
  "type": "tooShort",
  "interpolations": { minLength : 5 }
}

However this can be implemented in userland. You just can't use the built-in validators then.

The important thing we need in ember-cp-validations for this to work is just the ability to pass anything from the validate() function to the ValidationError object.

I hope I was able to explain it a bit what I mean.

I'll likely need time to investigate this further, but I'm not against the general idea of exposing an API for messages to be recomputed versus the entire validator. The implementation is just something we will need to coordinate with @offirgolan

@offirgolan what are your thoughts?

I think the reasoning here is pretty solid, it's just that being able to support something like this might be a bit tough. Tbh, I'm still having trouble wrapping my head around how exactly you guys want this to look like.

@luxferresum can you give an example of what sort of API you expect / what you want the end goal to look like?

I've just had some time to dig into this deeper. Later I will publish a WIP-PR on ember-cp-validations where we can discuss some ideas.

To be honest in my personal opinion the and goal would be a breaking change:

specifying the validations

I think the existing API is pretty good here. Lets make a shot example with a simple length validator:

let Validator = buildValidations({
    name: validator('length', {
        description: 'Name',
        descriptionKey: 'name',
        min: 4,
        max: 8
    })
});

the validator

Currently I could build a validator like this:

validate(value, options) {
    if(value.length > options.max) {
        return options.description + " is to long";
    }

    if(value.length < options.min) {
        return options.description + " is to short";
    }

    return true;
}

But better I do it like this:

validate(value, options) {
    if(value.length > options.max) {
        return this.createErrorMessage('toLong', value, options);
    }

    if(value.length < options.min) {
        return this.createErrorMessage('toShort', value, options);
    }

    return true;
}

However this has the problem that the error message is calculated during the validation process, and the type sent to createErrorMessage is not avalibale later. What I propose is this:

    validate(value, options) {
    if(value.length > options.max) {
        return {type: 'toLong'};
    }

    if(value.length < options.min) {
        return {type: 'toShort'};
    }

    return true;
}

We could change the default implementation of createErrorMessage to support old validators:

createErrorMessage(type) {
    return {type};
}

Now I would propose that the user can just create an file validations/error with the following content:

import Ember from 'ember';
import Base from 'ember-cp-validations/validations/error';

const {get} = Ember;

export default Base.extend({
    i18n: Ember.inject.service(),
    message: Ember.computed('result.type', 'options.description', {
        let descriptionKey = get(this, 'options.description');
        let type = get(this,'result.type');

        // here we could rebuild the current behavior of `createErrorMessage`
        // or for i18n:

        let i18n = get(this, 'i18n');
        let description = i18n.t(`fields.${descriptionKey}`);
        return i18n.t(`errors.${type}`, { field: description });
    });
});

If the user doesnt override the Error class we could implement the current behavior of createErrorMessage.

jasonmit / ember-i18n-cp-validations

Make messages live-updateable and maybe separate the validation and message creation in ember-cp-validations #24

specifying the validations

the validator