Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

fix voor validationData #41

Closed
wants to merge 9 commits into from
Closed

fix voor validationData #41

wants to merge 9 commits into from

Conversation

wpdebruin
Copy link
Contributor

@wpdebruin wpdebruin commented Feb 26, 2020

ValidationData always was a string, and so was the IValidionError.getValidationData.
There are already 4 Validators which don't return a string (Unique, ValidateIf, ValidateUnless and Unique)
This caused errors in globalReplacements in ValidationResult.cfc. I replaced it with a more generic if statement, so we don't have to change it for every customValidator or new validator.

This PR adds full globalization for all default validation messages and solves the following issues
https://ortussolutions.atlassian.net/browse/BOX-28
#15
#14
#31 (more or less)

This is fully compatible with old cbvalidation versions, just one potential problem if you are using unknown locales in your app. Which can be adressed. See my note further down.

The changes:

  • In ModuleConfig.cfc cbi18n section is added. We really need a default locale here and a localeStorage or everything will fail. These can be overridden in the main coldbox config.
  • A resourcebundle alias named cbvalidation has been added where we store all language default messages.
  • The alias location is includes/i18n/cbvalidation. This can be overridden in configSettings.validation.cbi18nResourcePath. This opens the possibility to override any default message, and add unknown languages. If you copy the includes/i18n/cbvalidation to your own location you can change any message and add any language you want.
  • all validators have a new dependency ResourceService which is added during init.
    an all validation error messages have been replaced by resourceService.getResource("default.ValidationType@cbvalidation"), in some cases some values are added to the resources.
  • a globalReplacements call is added to addError in ValidationResult. This is necessary so all replacements in locale resources will allways be processed.

I want to add also a smarter way to handle custom messages in Internationalization.
such as {ObjectName}.{Field}.{ConstraintType}}
This can only be added in a DEFAULT resource now, and I want to have them to be available in any resource alias I want. Can easily be solved by adding another setting called resourceAliasName (or something simular) which can default to 'default' which is the current situation, 'cbvalidation' which is were all default messages are, or something new. Makes it very flexible, and keeps your validation messages in one place. Didn't do it yet, want to discuss the rest first.

There's one problem right now: if you add an unknown locale cbvalidation will fail, because the locale is not there. This is because cbi18n ALWAYS needs a valid locale.
I think providing the most used languages shouldn't be a problem, such as en, es, de, fr, nl.
But most locales have country variants such as en_US, en_UK, nl_NL, nl_BE etcetera. If we provide them all it adds up and we still have no default.
By a simple modification in cbi18n this can be solved. Make the locale lookup hierarchical.
So try to find nl_BE. If it is not there try to find nl (so without the country destination) and if still not found try the default locale.
So let's say cbvalidation_nl_BE.properties, followed by cbvalidation_nl.properties, followed by cbvalidatiion.properties. Which is actually how this is meant in java locales, but cbi18n doesn't implement it yet.
We can make this behaviour optional (so no changes to old cbi18n) but is is way more flexible and if we specify lookupParentResource=true (or something simular) our cbvalidation will never fail.
I created a pull for cbi18n as well so this can be fixed as well

@lmajano
Copy link
Contributor

lmajano commented May 22, 2020

@wpdebruin can you revise this, seems lots have changed

@lmajano lmajano closed this May 22, 2020
@wpdebruin
Copy link
Contributor Author

wpdebruin commented May 22, 2020 via email

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.

2 participants