From bddf0fc9c5968f556e59a6d114387f1ed8c35820 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?C=C3=B4me=20Chilliet?= <come@opensides.be>
Date: Wed, 17 Oct 2018 17:12:35 +0200
Subject: [PATCH] :ambulance: fix(attributes) Make sure subattributes are not
 set as required

Attributes which are input for a SetAttribute or equivalent should now
 be set as such, and should not have the "required" html attribute to
 avoid validation problems.
Also subattribute and required status is indicated in the $attributes
 array for future use related to #5858

issue #5910
---
 .../attributes/class_CompositeAttribute.inc   |  9 ++++++
 .../attributes/class_IntAttribute.inc         |  4 ++-
 .../attributes/class_SetAttribute.inc         | 29 +++++++++++++------
 .../attributes/class_StringAttribute.inc      |  6 ++--
 include/simpleplugin/class_Attribute.inc      | 21 +++++++++++---
 5 files changed, 53 insertions(+), 16 deletions(-)

diff --git a/include/simpleplugin/attributes/class_CompositeAttribute.inc b/include/simpleplugin/attributes/class_CompositeAttribute.inc
index 03bae4ca5..445a25c32 100644
--- a/include/simpleplugin/attributes/class_CompositeAttribute.inc
+++ b/include/simpleplugin/attributes/class_CompositeAttribute.inc
@@ -62,6 +62,15 @@ class CompositeAttribute extends Attribute
     unset($attribute);
   }
 
+  function setIsSubAttribute($bool)
+  {
+    parent::setIsSubAttribute($bool);
+    foreach ($this->attributes as &$attribute) {
+      $attribute->setIsSubAttribute($this->isSubAttribute);
+    }
+    unset($attribute);
+  }
+
   function setAcl ($acl)
   {
     parent::setAcl($acl);
diff --git a/include/simpleplugin/attributes/class_IntAttribute.inc b/include/simpleplugin/attributes/class_IntAttribute.inc
index 7ca5e4215..d54cab6bb 100644
--- a/include/simpleplugin/attributes/class_IntAttribute.inc
+++ b/include/simpleplugin/attributes/class_IntAttribute.inc
@@ -108,7 +108,9 @@ class IntAttribute extends Attribute
       $js       = $this->managedAttributesJS();
       $attributes['onChange'] = 'javascript:'.htmlentities($js, ENT_COMPAT, 'UTF-8');
     }
-    if ($this->isRequired()) {
+    if ($this->isSubAttribute) {
+      $attributes['class'] = 'subattribute';
+    } elseif ($this->isRequired()) {
       $attributes['required'] = 'required';
     }
     $display = $this->renderInputField('number', $id, $attributes);
diff --git a/include/simpleplugin/attributes/class_SetAttribute.inc b/include/simpleplugin/attributes/class_SetAttribute.inc
index 90d16938d..322ce22b9 100644
--- a/include/simpleplugin/attributes/class_SetAttribute.inc
+++ b/include/simpleplugin/attributes/class_SetAttribute.inc
@@ -43,10 +43,17 @@ class SetAttribute extends Attribute
       $values
     );
     $this->attribute = $attribute;
-    $this->attribute->setRequired(FALSE);
+    $this->attribute->setRequired(TRUE);
+    $this->attribute->setIsSubAttribute(TRUE);
     $this->valueUnicity = $valueUnicity;
   }
 
+  function setIsSubAttribute($bool)
+  {
+    parent::setIsSubAttribute($bool);
+    $this->attribute->setIsSubAttribute($this->isSubAttribute);
+  }
+
   function setManagedAttributes (array $dontcare)
   {
     trigger_error('method setManagedAttributes is not supported for SetAttributes');
@@ -231,18 +238,22 @@ class SetAttribute extends Attribute
         parent::renderAttribute($attributes, $readOnly);
       } else {
         $attributes[$this->getLdapName()] = array(
-          'htmlid'      => $this->getForHtmlId(),
-          'label'       => '{literal}'.$this->getLabel().'{/literal}'.($this->isRequired() ? '{$must}' : ''),
-          'description' => ($this->isRequired() ? sprintf(_("%s (required)"), $this->getDescription()) : $this->getDescription()),
-          'input'       => $this->renderAcl($this->renderOnlyFormInput()),
+          'htmlid'        => $this->getForHtmlId(),
+          'label'         => '{literal}'.$this->getLabel().'{/literal}'.($this->isRequired() ? '{$must}' : ''),
+          'description'   => ($this->isRequired() ? sprintf(_("%s (required)"), $this->getDescription()) : $this->getDescription()),
+          'input'         => $this->renderAcl($this->renderOnlyFormInput()),
+          'subattribute'  => $this->isSubAttribute,
+          'required'      => $this->isRequired(),
         );
         $this->handleEditingValue();
         $this->attribute->renderAttribute($attributes, $readOnly);
         $attributes[$this->getLdapName().'_buttons'] = array(
-          'htmlid'      => 'add'.$this->getHtmlId(),
-          'label'       => '',
-          'description' => '',
-          'input'       => $this->renderAcl($this->renderButtons()),
+          'htmlid'        => 'add'.$this->getHtmlId(),
+          'label'         => '',
+          'description'   => '',
+          'input'         => $this->renderAcl($this->renderButtons()),
+          'subattribute'  => TRUE,
+          'required'      => FALSE,
         );
       }
     }
diff --git a/include/simpleplugin/attributes/class_StringAttribute.inc b/include/simpleplugin/attributes/class_StringAttribute.inc
index 3ea86764b..a51f64068 100644
--- a/include/simpleplugin/attributes/class_StringAttribute.inc
+++ b/include/simpleplugin/attributes/class_StringAttribute.inc
@@ -82,7 +82,9 @@ class StringAttribute extends Attribute
     if ($this->html5pattern !== NULL) {
       $attributes['pattern'] = '{literal}'.htmlentities($this->html5pattern, ENT_COMPAT, 'UTF-8').'{/literal}';
     }
-    if ($this->isRequired()) {
+    if ($this->isSubAttribute) {
+      $attributes['class'] = 'subattribute';
+    } elseif ($this->isRequired()) {
       $attributes['required'] = 'required';
     }
     $display  = $this->renderInputField($this->inputType, $id, $attributes);
@@ -159,7 +161,7 @@ class TextAreaAttribute extends StringAttribute
     $id = $this->getHtmlId();
     $display  = '<textarea name="'.$id.'" id="'.$id.'"'.
                 ($this->disabled ? ' disabled="disabled"' : '').
-                ($this->isRequired() ? ' required="required"' : '').
+                ($this->isSubAttribute ? ' class="subattribute"' : ($this->isRequired() ? ' required="required"' : '')).
                 '>'.
                 '{literal}'.htmlentities($this->getValue(), ENT_COMPAT, 'UTF-8').'{/literal}</textarea>';
     return $this->renderAcl($display);
diff --git a/include/simpleplugin/class_Attribute.inc b/include/simpleplugin/class_Attribute.inc
index 213415a46..337a24f1c 100644
--- a/include/simpleplugin/class_Attribute.inc
+++ b/include/simpleplugin/class_Attribute.inc
@@ -82,6 +82,12 @@ class Attribute
   /* \bried Array of booleans telling for each managing attributes if he's disabling us */
   protected $managingAttributesOrders = array();
 
+  /* \bried If this is TRUE it means this attribute is not directly submitted with the form
+   * but is part of a multivalue attribute.
+   * It means it should not be set as required in the HTML form for instance.
+   */
+  protected $isSubAttribute = FALSE;
+
   /*! \brief The constructor of Attribute
    *
    *  \param string $label The label to show for this attribute
@@ -114,6 +120,11 @@ class Attribute
     $this->manageAttributes($this->getValue());
   }
 
+  function setIsSubAttribute($bool)
+  {
+    $this->isSubAttribute = $bool;
+  }
+
   function setInLdap ($inLdap)
   {
     $this->inLdap = $inLdap;
@@ -589,10 +600,12 @@ class Attribute
         $input = $this->renderFormInput();
       }
       $attributes[$this->getLdapName()] = array(
-        'htmlid'      => $this->getForHtmlId(),
-        'label'       => '{literal}'.$this->getLabel().'{/literal}'.($this->isRequired() ? '{$must}' : ''),
-        'description' => ($this->isRequired() ? sprintf(_("%s (required)"), $this->getDescription()) : $this->getDescription()),
-        'input'       => $input,
+        'htmlid'        => $this->getForHtmlId(),
+        'label'         => '{literal}'.$this->getLabel().'{/literal}'.($this->isRequired() ? '{$must}' : ''),
+        'description'   => ($this->isRequired() ? sprintf(_("%s (required)"), $this->getDescription()) : $this->getDescription()),
+        'input'         => $input,
+        'subattribute'  => $this->isSubAttribute,
+        'required'      => $this->isRequired(),
       );
     }
   }
-- 
GitLab