diff --git a/src/Contract/ValueCandidates.php b/src/Contract/ValueCandidates.php deleted file mode 100644 index 0271500e..00000000 --- a/src/Contract/ValueCandidates.php +++ /dev/null @@ -1,22 +0,0 @@ -valueCandidates; - } - - public function setValueCandidates(array $values) - { - $this->valueCandidates = $values; - - return $this; - } - public function onRegistered(Form $form) { } diff --git a/src/FormElement/FormElements.php b/src/FormElement/FormElements.php index 2f8e4e5a..68268726 100644 --- a/src/FormElement/FormElements.php +++ b/src/FormElement/FormElements.php @@ -5,7 +5,6 @@ use InvalidArgumentException; use ipl\Html\Contract\FormElement; use ipl\Html\Contract\FormElementDecorator; -use ipl\Html\Contract\ValueCandidates; use ipl\Html\Form; use ipl\Html\FormDecorator\DecoratorInterface; use ipl\Html\ValidHtml; @@ -194,11 +193,7 @@ public function registerElement(FormElement $element) $this->elements[$name] = $element; if (array_key_exists($name, $this->populatedValues)) { - $element->setValue($this->populatedValues[$name][count($this->populatedValues[$name]) - 1]); - - if ($element instanceof ValueCandidates) { - $element->setValueCandidates($this->populatedValues[$name]); - } + $element->setValue($this->populatedValues[$name]); } $this->onElementRegistered($element); @@ -316,7 +311,7 @@ public function getValues() public function populate($values) { foreach ($values as $name => $value) { - $this->populatedValues[$name][] = $value; + $this->populatedValues[$name] = $value; if ($this->hasElement($name)) { $this->getElement($name)->setValue($value); } @@ -338,7 +333,7 @@ public function populate($values) public function getPopulatedValue($name, $default = null) { return isset($this->populatedValues[$name]) - ? $this->populatedValues[$name][count($this->populatedValues[$name]) - 1] + ? $this->populatedValues[$name] : $default; } @@ -481,8 +476,6 @@ public function isValid() { if ($this->isValid === null) { $this->validate(); - - $this->emit(Form::ON_VALIDATE, [$this]); } return $this->isValid; @@ -539,7 +532,6 @@ public function isValidEvent($event) Form::ON_SENT, Form::ON_ERROR, Form::ON_REQUEST, - Form::ON_VALIDATE, Form::ON_ELEMENT_REGISTERED, ]); } diff --git a/src/FormElement/PasswordElement.php b/src/FormElement/PasswordElement.php index dfa6d8c5..6010c154 100644 --- a/src/FormElement/PasswordElement.php +++ b/src/FormElement/PasswordElement.php @@ -2,54 +2,34 @@ namespace ipl\Html\FormElement; -use ipl\Html\Attributes; -use ipl\Html\Form; - class PasswordElement extends InputElement { - /** @var string Dummy passwd of this element to be rendered */ - public const DUMMYPASSWORD = '_ipl_form_5847ed1b5b8ca'; - - protected $type = 'password'; + protected const OBSCURE_PASSWORD = '_ipl_form_5847ed1b5b8ca'; - /** @var bool Status of the form */ - protected $isFormValid = true; + protected $password; - protected function registerAttributeCallbacks(Attributes $attributes) - { - parent::registerAttributeCallbacks($attributes); - - $attributes->registerAttributeCallback( - 'value', - function () { - if ($this->hasValue() && count($this->getValueCandidates()) === 1 && $this->isFormValid) { - return self::DUMMYPASSWORD; - } - - if (parent::getValue() === self::DUMMYPASSWORD) { - return self::DUMMYPASSWORD; - } - - return null; - } - ); - } + protected $type = 'password'; - public function onRegistered(Form $form) + public function getValue() { - $form->on(Form::ON_VALIDATE, function ($form) { - $this->isFormValid = $form->isValid(); - }); + return $this->password; } - public function getValue() + public function setValue($value) { + parent::setValue($value); + // Consider any changes to the password made by the parent setValue() call. $value = parent::getValue(); - $candidates = $this->getValueCandidates(); - while ($value === self::DUMMYPASSWORD) { - $value = array_pop($candidates); + + if ($value !== static::OBSCURE_PASSWORD) { + $this->password = $value; } - return $value; + return $this; + } + + public function getValueAttribute() + { + return $this->hasValue() ? static::OBSCURE_PASSWORD : null; } } diff --git a/tests/FormElement/PasswordElementTest.php b/tests/FormElement/PasswordElementTest.php new file mode 100644 index 00000000..a5880e53 --- /dev/null +++ b/tests/FormElement/PasswordElementTest.php @@ -0,0 +1,157 @@ +populate(['password' => 'secret']) + ->addElement('password', 'password'); + + $html = <<<'HTML' +
+HTML; + $this->assertHtml(sprintf($html, ObscurePassword::get()), $form); + } + + public function testGetValueReturnsNullIfNoPasswordSet() + { + $password = new PasswordElement('password'); + $this->assertNull($password->getValue()); + $this->assertFalse($password->hasValue()); + } + + public function testGetValueReturnsPassword() + { + $form = (new Form()) + ->populate(['password' => 'secret']) + ->addElement('password', 'password'); + + $password = $form->getElement('password'); + $this->assertEquals($password->getValue(), 'secret'); + $this->assertTrue($password->hasValue()); + } + + public function testGetValueReturnsNewPassword() + { + $form = (new Form()) + ->populate(['password' => 'secret']) + ->populate(['password' => 'topsecret']) + ->addElement('password', 'password'); + + $password = $form->getElement('password'); + $this->assertEquals($password->getValue(), 'topsecret'); + $this->assertTrue($password->hasValue()); + } + + public function testGetValueReturnsNullIfPasswordReset() + { + $form = (new Form()) + ->populate(['password' => 'secret']) + ->populate(['password' => '']) + ->addElement('password', 'password'); + + $password = $form->getElement('password'); + $this->assertNull($password->getValue()); + $this->assertFalse($password->hasValue()); + } + + public function testGetValueReturnsNullIfNotChanged() + { + $form = (new Form()) + ->populate(['password' => ObscurePassword::get()]) + ->addElement('password', 'password'); + + $password = $form->getElement('password'); + $this->assertNull($password->getValue()); + } + + /** + * Represents a form that requires a password to be set as confirmation. + * If another element is invalid, the password element should have no + * value, as otherwise the user thinks the password is still set, although + * it's the obscured password. + * + * @return void + */ + public function testObscuredValueNotVisibleAfterFormValidationFailed() + { + $form = (new Form()) + ->populate(['password' => 'secret']) + ->addElement('password', 'password') + ->addElement('text', 'username', ['required' => true]); + + $this->assertFalse($form->isValid()); + + $html = <<<'HTML' + +HTML; + $this->assertHtml($html, $form); + } + + /** + * Represents a controller action that populates saved data and + * allows the user to change it. If the password isn't changed, + * the saved password must be preserved. + * + * @return void + */ + public function testOriginalPasswordMustBePreserved() + { + $form = (new Form()); + + // The action populates always first + $form->populate(['password' => 'secret']); + + // handleRequest() then another time + $form->populate(['password' => ObscurePassword::get()]); + + // assemble() then registers the element + $form->addElement('password', 'password'); + + $password = $form->getElement('password'); + $this->assertEquals('secret', $password->getValue()); + } + + /** + * Represents a controller action that populates saved data and + * allows the user to change it. If the password isn't changed, + * the password must persist when the value of an element + * with autosubmit class is changed. + * + * @return void + */ + public function testOriginalPasswordMustPersistOnAutoSubmit() + { + $form = (new ElementWithAutosubmitInFieldsetForm()); + + // The action populates always first + $form->populate([ + 'foo1' => ['select' => 'option1'], + 'foo2' => ['password' => 'secret'] + ]); + + // handleRequest() then another time + $form->populate([ + 'foo1' => ['select' => 'option2'], + 'foo2' => ['password' => ObscurePassword::get()] + ]); + + $form->ensureAssembled(); + + $password = $form->getElement('foo2')->getValue('password'); + $this->assertEquals('secret', $password); + } +} diff --git a/tests/Lib/ElementWithAutosubmitInFieldsetForm.php b/tests/Lib/ElementWithAutosubmitInFieldsetForm.php new file mode 100644 index 00000000..337bd1c1 --- /dev/null +++ b/tests/Lib/ElementWithAutosubmitInFieldsetForm.php @@ -0,0 +1,45 @@ + 'foo1' + ])); + + $this->addElement($foo1); + + $foo1->addElement( + 'select', + 'select', + [ + 'label' => 'Select', + 'class' => 'autosubmit', + 'options' => [ + 'option1' => 'option1', + 'option2' => 'option2' + ] + ] + ); + + $foo2 = (new FieldsetElement('foo2', [ + 'label' => 'foo2' + ])); + + $this->addElement($foo2); + + $foo2->addElement( + 'password', + 'password', + [ + 'label' => 'Password' + ] + ); + } +} diff --git a/tests/TestDummy/ObscurePassword.php b/tests/TestDummy/ObscurePassword.php new file mode 100644 index 00000000..80012c73 --- /dev/null +++ b/tests/TestDummy/ObscurePassword.php @@ -0,0 +1,13 @@ +