Merge pull request #11099 from creative-commoners/pulls/4.13/confirmedpasswordfield-required

FIX Correctly mark ConfirmedPasswordField children as required

src/Forms/ConfirmedPasswordField.php

@@ -4,6 +4,7 @@ namespace SilverStripe\Forms;
 
 use SilverStripe\ORM\DataObject;
 use SilverStripe\ORM\DataObjectInterface;
+use SilverStripe\ORM\FieldType\DBField;
 use SilverStripe\Security\Authenticator;
 use SilverStripe\Security\Security;
 use SilverStripe\View\HTML;
@@ -200,7 +201,7 @@ class ConfirmedPasswordField extends FormField
                 }
             }
 
-            $fieldContent .= $field->FieldHolder();
+            $fieldContent .= $field->FieldHolder(['AttributesHTML' => $this->getAttributesHTMLForChild($field)]);
         }
 
         if (!$this->showOnClick) {
@@ -233,6 +234,19 @@ class ConfirmedPasswordField extends FormField
         );
     }
 
+    public function Required()
+    {
+        return !$this->canBeEmpty || parent::Required();
+    }
+
+    public function setForm($form)
+    {
+        foreach ($this->getChildren() as $field) {
+            $field->setForm($form);
+        }
+        return parent::setForm($form);
+    }
+
     /**
      * Returns the children of this field for use in templating.
      * @return FieldList
@@ -694,4 +708,18 @@ class ConfirmedPasswordField extends FormField
     {
         return $this->requireStrongPassword;
     }
+
+    /**
+     * Get the AttributesHTML for a child field.
+     * Includes extra information the child isn't aware of on its own, such as whether
+     * it's required due to this field as a whole being required.
+     */
+    private function getAttributesHTMLForChild(FormField $child): DBField
+    {
+        $attributes = $child->getAttributesHTML();
+        if (strpos($attributes, 'required="required"') === false && $this->Required()) {
+            $attributes .= ' required="required" aria-required="true"';
+        }
+        return DBField::create_field('HTMLFragment', $attributes);
+    }
 }

tests/php/Forms/ConfirmedPasswordFieldTest.php

@@ -383,4 +383,83 @@ class ConfirmedPasswordFieldTest extends SapphireTest
         $field->setRequireExistingPassword(false);
         $this->assertCount(2, $field->getChildren(), 'Current password field should not be removed');
     }
+
+    public function provideRequired()
+    {
+        return [
+            'can be empty' => [true],
+            'cannot be empty' => [false],
+        ];
+    }
+
+    /**
+     * @dataProvider provideRequired
+     */
+    public function testRequired(bool $canBeEmpty)
+    {
+        $field = new ConfirmedPasswordField('Test');
+        $field->setCanBeEmpty($canBeEmpty);
+        $this->assertSame(!$canBeEmpty, $field->Required());
+    }
+
+    public function provideChildFieldsAreRequired()
+    {
+        return [
+            'not required' => [
+                'canBeEmpty' => true,
+                'required' => false,
+                'childrenRequired' => false,
+                'expectRequired' => false,
+            ],
+            'required via validator' => [
+                'canBeEmpty' => true,
+                'required' => true,
+                'childrenRequired' => false,
+                'expectRequired' => true,
+            ],
+            'children required directly' => [
+                'canBeEmpty' => true,
+                'required' => false,
+                'childrenRequired' => true,
+                'expectRequired' => true,
+            ],
+            'required because cannot be empty' => [
+                'canBeEmpty' => false,
+                'required' => false,
+                'childrenRequired' => false,
+                'expectRequired' => true,
+            ],
+        ];
+    }
+
+    /**
+     * @dataProvider provideChildFieldsAreRequired
+     */
+    public function testChildFieldsAreRequired(bool $canBeEmpty, bool $required, bool $childrenRequired, bool $expectRequired)
+    {
+        $form = new Form();
+        $field = new ConfirmedPasswordField('Test');
+        $field->setForm($form);
+        $field->setCanBeEmpty($canBeEmpty);
+        $requiredFields = [];
+        if ($required) {
+            $requiredFields[] = 'Test';
+        }
+        if ($childrenRequired) {
+            $requiredFields[] = 'Test[_Password]';
+            $requiredFields[] = 'Test[_ConfirmPassword]';
+        }
+        $form->setValidator(new RequiredFields($requiredFields));
+
+        $rendered = $field->Field();
+        $fieldOneRegex = '<input\s+type="password"\s+name="Test\[_Password\]"\s[^>]*?required="required"\s+aria-required="true"\s[^>]*\/>';
+        $fieldTwoRegex = '<input\s+type="password"\s+name="Test\[_ConfirmPassword\]"\s[^>]*?required="required"\s+aria-required="true"\s[^>]*\/>';
+        $regex = '/' . $fieldOneRegex . '.*' . $fieldTwoRegex . '/isu';
+
+        if ($expectRequired) {
+            $this->assertMatchesRegularExpression($regex, $rendered);
+        } else {
+            $this->assertDoesNotMatchRegularExpression($regex, $rendered);
+        }
+    }
 }