[MGNLUI-6245] Regex field validation has changed for blank or empty values Created: 24/Sep/20  Updated: 20/Jan/22  Resolved: 24/Dec/21

Status: Closed
Project: Magnolia UI
Component/s: None
Affects Version/s: 6.2
Fix Version/s: 6.2.16

Type: Bug Priority: Neutral
Reporter: Richard Gange Assignee: Quach Hao Thien
Resolution: Fixed Votes: 2
Labels: VN-Maintenance, fields, maintenance, new-UI-framework, validation
Remaining Estimate: Not Specified
Time Spent: 7.5h
Original Estimate: Not Specified

Issue Links:
Relates
causality
is causing MGNLUI-7019 Validators don't validate the value w... Closed
documentation
to be documented by MGNLUI-6999 DOC: Field validation for empty values Closed
duplicate
duplicates MGNLUI-6547 EmailFieldValidatorFactory does not a... Closed
relation
Template:
Acceptance criteria:
Empty
Task DoD:
[X]* Doc/release notes changes? Comment present?
[X]* Downstream builds green?
[X]* Solution information and context easily available?
[X]* Tests
[X]* FixVersion filled and not yet released
[ ]  Architecture Decision Record (ADR)
Bug DoR:
[ ]* Steps to reproduce, expected, and actual results filled
[ ]* Affected version filled
Release notes required:
Yes
Documentation update required:
Yes
Date of First Response:
Sprint: UI Maintenance 4
Story Points: 2

 Description   

When migrating from 5.7 to 6.2.3 it has been reported that field validation is being triggered for fields which are empty but not required.

For example, if I had this configuration is Magnolia 5.7:

validators: 
   - name: rellink
     class: info.magnolia.ui.form.validator.definition.RegexpValidatorDefinition
     pattern: ^\/.*

In 6.2 I need to update the pattern to:

validators: 
  - name: rellink
    class: info.magnolia.ui.form.validator.definition.RegexpValidatorDefinition
    pattern: ^(|\/.*)$

Dev note:
validators precede "required: false" property now, this means that conversion of validators should be updated here: info.magnolia.ui.form.field.definition.migration.AbstractConfiguredFieldDefinitionConverter#convertValidators
(also emailValidator should be revisited to have possibility to have unrequired email field in M6 ui)



 Comments   
Comment by Richard Gange [ 24/Sep/20 ]

The problem stems from a change in logic:
Old:

@Override
protected boolean isValidValue(String value) {
    if (value == null || value.isEmpty()) {
        return true;
    }
    if (complete) {
        return getMatcher(value).matches();
    } else {
        return getMatcher(value).find();
    }
}

New:

protected boolean isValid(String value) {
    if (value == null) {
        return true;
    }
    if (complete) {
        return getMatcher(value).matches();
    } else {
        return getMatcher(value).find();
    }
}

The decision was made that the pattern should be responsible for handling the empty values and the old code should not have been checking for empty values.

If empty values are allow --> use the correct pattern (which may require a configuration update)
If empty values are not allowed --> use required=true

Comment by Roman Kovařík [ 10/Mar/21 ]

We agreed there is really a bug with non required field. 

Comment by Ashraf Khamis [ 10/Mar/21 ]

This is a bug that has nothing to do with documentation. Removed myself as an assignee after discussing the issue with Roman.

Comment by Richard Gange [ 10/Mar/21 ]

Great thanks. There was some uncertainty around how we should handle this. It's a pretty quick win to put it back the old way.

Comment by Quach Hao Thien [ 24/Dec/21 ]

For RN:

  • Fix: Ignore configured validators on non-mandatory empty fields

For docu:

Field validators

  • Field which is not required (required: false) will accept it's empty value (depends on field type), then it's empty value won't be validated by configured validators
  • Any using RegexValidator to check empty value should be replaced by using required property, otherwise it will be ignored.
Generated at Mon Feb 12 09:34:33 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.