Uploaded image for project: 'Magnolia UI'
  1. Magnolia UI
  2. MGNLUI-3227

Improve validation capabilities of the multi fields

    XMLWordPrintable

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Neutral
    • Resolution: Incomplete
    • Affects Version/s: 5.2.10, 5.3.4
    • Fix Version/s: 5.3.6
    • Component/s: forms
    • Labels:
      None
    • Patch included:
      Yes

      Description

      Initial issue title: Field Validation: Don't ignore 'required' property on the CompositeField itself.
      Setting on the CompositeFieldDefinition the required property it is ignored.
      So if one needs to ensure that all fields in the composite need to be filled out, one must set to all sub fields a required property.

      Much easier would be if the required could be set on the composite itself.

      Use case definition:
      If the Composite field sets required=true, every sub field in the composite needs a value.

      Problem:
      CompositeFieldDefinition does not implement isValid(), and the super's isValid is only checking the validation of the sub fields.

      This code solves the problem, I included a patch.

      @Override
          public boolean isValid() {
              boolean isValid = super.isValid();
              
              if (this.isRequired()) {
                  if (isEmpty()) {
                     isValid = false;
                  }
              }
              return isValid;
          }
      

      UPD:

      • This looks like a part of more fundamental problem: the validate() method of com.vaadin.ui.AbstractField is not invoked at all.
        • If it was invoked then the standard isRequired()/isEmpty() check would be handled.
        • It is supposed to be called within com.vaadin.ui.AbstractField#isValid but AbstractCustomMultiField impl doesn't call super method.
        • As another side effect the value validators of the AbstractCustomMultiField wouldn't be triggered since that also happens in the validate() method.

      Proposed solution:

      • Simply call super.isValid() in AbstractCustomMultiField#isValid instead of assigning true as a default value.

        Checklists

        Acceptance criteria

          Attachments

            Issue Links

              Activity

                People

                Assignee:
                apchelintcev Aleksandr Pchelintcev
                Reporter:
                cringele Christian Ringele
                Votes:
                0 Vote for this issue
                Watchers:
                8 Start watching this issue

                  Dates

                  Created:
                  Updated:
                  Resolved:
                  Date of First Response:

                    Checklists

                    DoD