[MGNLFORM-79] Cannot (easily) provide custom error message with custom validator Created: 08/Feb/11  Updated: 12/Sep/11  Resolved: 09/Sep/11

Status: Closed
Project: Magnolia Form Module
Component/s: None
Affects Version/s: 1.1.4
Fix Version/s: 1.2.4, 1.3

Type: Improvement Priority: Major
Reporter: Felix Rabe Assignee: Ondrej Chytil
Resolution: Fixed Votes: 1
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Attachments: Text File 0001-MGNLFORM-79-Backport-of-r45141-and-also-applied-API-.patch     Text File 0001-MGNLFORM-79-Change-r45141-according-to-my-issue-comm.patch     PNG File Screen shot 2011-02-08 at 11.31.08 AM.png    
Issue Links:
dependency
relation
is related to MGNLFORM-50 i18n-problem with custom validation-e... Closed
Template:
Acceptance criteria:
Empty
Task DoD:
[ ]* Doc/release notes changes? Comment present?
[ ]* Downstream builds green?
[ ]* Solution information and context easily available?
[ ]* Tests
[ ]* FixVersion filled and not yet released
[ ]  Architecture Decision Record (ADR)
Date of First Response:

 Description   

I've created a new module (magnolia-module-enterprise-validation) in which I've implemented a new validator. (This validator is to be shipped separately from the Form module.)

At the moment, there is no straightforward way to show a custom error message upon failed validation for this validator - the default "invalid input" is not appropriate for this validator. I.e., it is not possible for the validator to show a custom error message in this setup:

  • Add the validator to config:/modules/form/config/validators as 'enterpriseEmail' (using a bootstrap file)
  • Use the validator for a form field by selecting it there.

Showing a custom error message would require unneccessarily invasive changes to the form as a whole.

A workaround is to show the default error message, but add a prominent hint next to the validated field like: "Addresses from free email providers will not be accepted."



 Comments   
Comment by Tobias Mattsson [ 16/May/11 ]

The error message for a validator comes from the resource bundle in the form module. The key for it is in the form form.user.errorMessage.<validatorName>. If none is found there the text 'invalid input' is shown. The resource bundle to use is taken from the form paragraph.

It is therefor possible to have a custom error message for a custom validator by changing the resource bundle on the paragraph. The new resource bundle should be a copy of the existing bundle from the form module.

Agreed, this is not that easy. A better solution could be to let the validator have a i18nBasename of its own and let its validate() method return a key into that bundle. This would be more flexible and still allow error messages in multiple languages.

Proposed changes:

boolean Validator.validate(String) => String Validator.validate(String)

If validate() returns null it was successful.

and additions:

String Validator.getI18nBasename();

If it returns null it means functionality as before, i.e. use i18nBasename from paragraph, fallback to 'invalid input'.

Comment by Tobias Mattsson [ 16/May/11 ]

Changes applied with a modification of the proposal above. The validate() method returns a ValidationResult instead.

  • A validator that simply returns false will get an error message same as today.
  • A validator that returns a message key but doesnt have i18nBasename set will use that message key in the default bundle.
  • A validator that returns a message key and has i18nBasename set will get its error message from a lookup in that bundle.
Comment by Felix Rabe [ 17/May/11 ]

I like the new validate method, but I have an idea for a change that might make this work without API breakage:

  • Rename the new validate method to validateWithResult.
  • Restore the old validate method. Also revert r45141 for the Validator subclasses NoHTMLValidator, PasswordValidator, and ValidateExpression.
  • Change Validator.validationWithResult to:
        public ValidationResult validateWithResult(String value) {
            return new ValidationResult(validate(value));
        }
      
Comment by Felix Rabe [ 19/May/11 ]

Patch that implements my proposal. (Untested.)

Comment by Felix Rabe [ 24/May/11 ]

Second patch that applies to head of branch 1.2. This combines r45141 with my previous patch. (Still to be tested.)

Comment by Felix Rabe [ 31/May/11 ]

Both patches have now been applied to the respective branches. Also I've tested the change on branch 1.2 with the current enterprise email validation module snapshot + corp.website snapshot.

Comment by Boris Kraft [ 29/Aug/11 ]

ITs ok if we try not to break the API's but ultimately we need to ask ourselves if keeping around a method that is more or less useless in its current for is worth no breaking the API. I suggest to at least deprecate the existing validate() method, and get rid of it entirely with 5.

Generated at Mon Feb 12 05:36:39 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.