From fdc1603771c4052a85554da51dcaa8059e18edcd Mon Sep 17 00:00:00 2001 From: Eric Lippmann Date: Mon, 24 Oct 2022 22:38:13 +0200 Subject: [PATCH 1/8] Test password element --- tests/FormElement/PasswordElementTest.php | 66 +++++++++++++++++++++++ tests/TestDummy/ObscurePassword.php | 13 +++++ 2 files changed, 79 insertions(+) create mode 100644 tests/FormElement/PasswordElementTest.php create mode 100644 tests/TestDummy/ObscurePassword.php diff --git a/tests/FormElement/PasswordElementTest.php b/tests/FormElement/PasswordElementTest.php new file mode 100644 index 00000000..b931a4ff --- /dev/null +++ b/tests/FormElement/PasswordElementTest.php @@ -0,0 +1,66 @@ +addElement('password', 'password') + ->populate(['password' => 'secret']); + + $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()) + ->addElement('password', 'password') + ->populate(['password' => 'secret']); + + $password = $form->getElement('password'); + $this->assertEquals($password->getValue(), 'secret'); + $this->assertTrue($password->hasValue()); + } + + public function testGetValueReturnsNewPassword() + { + $form = (new Form()) + ->addElement('password', 'password') + ->populate(['password' => 'secret']) + ->populate(['password' => 'topsecret']); + + $password = $form->getElement('password'); + $this->assertEquals($password->getValue(), 'topsecret'); + $this->assertTrue($password->hasValue()); + } + + public function testGetValueReturnsNullIfPasswordReset() + { + $form = (new Form()) + ->addElement('password', 'password') + ->populate(['password' => 'secret']) + ->populate(['password' => '']); + + $password = $form->getElement('password'); + $this->assertNull($password->getValue()); + $this->assertFalse($password->hasValue()); + } +} diff --git a/tests/TestDummy/ObscurePassword.php b/tests/TestDummy/ObscurePassword.php new file mode 100644 index 00000000..fa6e0185 --- /dev/null +++ b/tests/TestDummy/ObscurePassword.php @@ -0,0 +1,13 @@ + Date: Mon, 24 Oct 2022 22:44:36 +0200 Subject: [PATCH 2/8] Revert "Introduce PasswordElement class" This reverts commit ec2648257dbdced0155c561a111918d6f124ed05. --- src/FormElement/PasswordElement.php | 55 ----------------------------- 1 file changed, 55 deletions(-) delete mode 100644 src/FormElement/PasswordElement.php diff --git a/src/FormElement/PasswordElement.php b/src/FormElement/PasswordElement.php deleted file mode 100644 index dfa6d8c5..00000000 --- a/src/FormElement/PasswordElement.php +++ /dev/null @@ -1,55 +0,0 @@ -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; - } - ); - } - - public function onRegistered(Form $form) - { - $form->on(Form::ON_VALIDATE, function ($form) { - $this->isFormValid = $form->isValid(); - }); - } - - public function getValue() - { - $value = parent::getValue(); - $candidates = $this->getValueCandidates(); - while ($value === self::DUMMYPASSWORD) { - $value = array_pop($candidates); - } - - return $value; - } -} From 87b46392472e41fad99682d3996416c665e50447 Mon Sep 17 00:00:00 2001 From: Eric Lippmann Date: Mon, 24 Oct 2022 22:44:49 +0200 Subject: [PATCH 3/8] Revert "Introduce ON_VALIDATE form event" This reverts commit 9c82c99cd87ddea109a93c87d240618601203f1b. --- src/Form.php | 3 +-- src/FormElement/FormElements.php | 3 --- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/src/Form.php b/src/Form.php index 93bb6d98..2151f750 100644 --- a/src/Form.php +++ b/src/Form.php @@ -12,7 +12,7 @@ class Form extends BaseHtmlElement { use FormElements { - FormElements::remove as private removeElement; + remove as private removeElement; } use Messages; @@ -21,7 +21,6 @@ class Form extends BaseHtmlElement public const ON_REQUEST = 'request'; public const ON_SUCCESS = 'success'; public const ON_SENT = 'sent'; - public const ON_VALIDATE = 'validate'; /** @var string Form submission URL */ protected $action; diff --git a/src/FormElement/FormElements.php b/src/FormElement/FormElements.php index 2f8e4e5a..2da39d56 100644 --- a/src/FormElement/FormElements.php +++ b/src/FormElement/FormElements.php @@ -481,8 +481,6 @@ public function isValid() { if ($this->isValid === null) { $this->validate(); - - $this->emit(Form::ON_VALIDATE, [$this]); } return $this->isValid; @@ -539,7 +537,6 @@ public function isValidEvent($event) Form::ON_SENT, Form::ON_ERROR, Form::ON_REQUEST, - Form::ON_VALIDATE, Form::ON_ELEMENT_REGISTERED, ]); } From 0fdaa8ff58307122218aea0904183a55c4ca6655 Mon Sep 17 00:00:00 2001 From: Eric Lippmann Date: Mon, 24 Oct 2022 22:44:58 +0200 Subject: [PATCH 4/8] Revert "Implement ValueCandidates interface" This reverts commit e2576c6a5ae7332331baf89e1c10f433556fd8c5. --- src/FormElement/BaseFormElement.php | 18 +----------------- src/FormElement/FormElements.php | 11 +++-------- 2 files changed, 4 insertions(+), 25 deletions(-) diff --git a/src/FormElement/BaseFormElement.php b/src/FormElement/BaseFormElement.php index a234a41c..22bc70dd 100644 --- a/src/FormElement/BaseFormElement.php +++ b/src/FormElement/BaseFormElement.php @@ -6,13 +6,12 @@ use ipl\Html\Attributes; use ipl\Html\BaseHtmlElement; use ipl\Html\Contract\FormElement; -use ipl\Html\Contract\ValueCandidates; use ipl\Html\Form; use ipl\Stdlib\Messages; use ipl\Validator\ValidatorChain; use ReflectionProperty; -abstract class BaseFormElement extends BaseHtmlElement implements FormElement, ValueCandidates +abstract class BaseFormElement extends BaseHtmlElement implements FormElement { use Messages; @@ -37,9 +36,6 @@ abstract class BaseFormElement extends BaseHtmlElement implements FormElement, V /** @var mixed Value of the element */ protected $value; - /** @var array Value candidates of the element */ - protected $valueCandidates = []; - /** * @deprecated STOP: Do not use this property!! * @@ -255,18 +251,6 @@ public function setValue($value) return $this; } - public function getValueCandidates() - { - return $this->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 2da39d56..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; } From 346b983fc29c7d66b8494e1cedd09d3de6cc7b9d Mon Sep 17 00:00:00 2001 From: Eric Lippmann Date: Mon, 24 Oct 2022 22:45:04 +0200 Subject: [PATCH 5/8] Revert "Introduce ValueCandidates interface" This reverts commit ad6f4c9b5a007c7c858e3cf01921cf9cdc0b3f10. --- src/Contract/ValueCandidates.php | 22 ---------------------- 1 file changed, 22 deletions(-) delete mode 100644 src/Contract/ValueCandidates.php 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 @@ - Date: Mon, 24 Oct 2022 22:46:12 +0200 Subject: [PATCH 6/8] Add password element --- src/FormElement/PasswordElement.php | 35 +++++++++++++++++++++++++++++ tests/TestDummy/ObscurePassword.php | 2 +- 2 files changed, 36 insertions(+), 1 deletion(-) create mode 100644 src/FormElement/PasswordElement.php diff --git a/src/FormElement/PasswordElement.php b/src/FormElement/PasswordElement.php new file mode 100644 index 00000000..6010c154 --- /dev/null +++ b/src/FormElement/PasswordElement.php @@ -0,0 +1,35 @@ +password; + } + + public function setValue($value) + { + parent::setValue($value); + // Consider any changes to the password made by the parent setValue() call. + $value = parent::getValue(); + + if ($value !== static::OBSCURE_PASSWORD) { + $this->password = $value; + } + + return $this; + } + + public function getValueAttribute() + { + return $this->hasValue() ? static::OBSCURE_PASSWORD : null; + } +} diff --git a/tests/TestDummy/ObscurePassword.php b/tests/TestDummy/ObscurePassword.php index fa6e0185..80012c73 100644 --- a/tests/TestDummy/ObscurePassword.php +++ b/tests/TestDummy/ObscurePassword.php @@ -8,6 +8,6 @@ class ObscurePassword extends PasswordElement { public static function get(): string { - return static::DUMMYPASSWORD; + return static::OBSCURE_PASSWORD; } } From f1bc4b5ac66c3eff0a4acd22a4c63e50d7818ccf Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Wed, 9 Nov 2022 13:37:44 +0100 Subject: [PATCH 7/8] PasswordElementTest: Add additional cases, improve others I've adjusted the other tests as well. They resemble the chronological order better that is usually in effect. This also makes the first test run without the changes in this PR. --- tests/FormElement/PasswordElementTest.php | 76 ++++++++++++++++++++--- 1 file changed, 68 insertions(+), 8 deletions(-) diff --git a/tests/FormElement/PasswordElementTest.php b/tests/FormElement/PasswordElementTest.php index b931a4ff..62548089 100644 --- a/tests/FormElement/PasswordElementTest.php +++ b/tests/FormElement/PasswordElementTest.php @@ -11,8 +11,8 @@ class PasswordElementTest extends TestCase public function testPasswordValueRenderedObscured() { $form = (new Form()) - ->addElement('password', 'password') - ->populate(['password' => 'secret']); + ->populate(['password' => 'secret']) + ->addElement('password', 'password'); $html = <<<'HTML'
@@ -32,8 +32,8 @@ public function testGetValueReturnsNullIfNoPasswordSet() public function testGetValueReturnsPassword() { $form = (new Form()) - ->addElement('password', 'password') - ->populate(['password' => 'secret']); + ->populate(['password' => 'secret']) + ->addElement('password', 'password'); $password = $form->getElement('password'); $this->assertEquals($password->getValue(), 'secret'); @@ -43,9 +43,9 @@ public function testGetValueReturnsPassword() public function testGetValueReturnsNewPassword() { $form = (new Form()) - ->addElement('password', 'password') ->populate(['password' => 'secret']) - ->populate(['password' => 'topsecret']); + ->populate(['password' => 'topsecret']) + ->addElement('password', 'password'); $password = $form->getElement('password'); $this->assertEquals($password->getValue(), 'topsecret'); @@ -55,12 +55,72 @@ public function testGetValueReturnsNewPassword() public function testGetValueReturnsNullIfPasswordReset() { $form = (new Form()) - ->addElement('password', 'password') ->populate(['password' => 'secret']) - ->populate(['password' => '']); + ->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()); + } } From f9cc24512cce37efe1ee5dedc675730db9723b0e Mon Sep 17 00:00:00 2001 From: raviks789 <33730024+raviks789@users.noreply.github.com> Date: Fri, 24 Feb 2023 09:51:28 +0100 Subject: [PATCH 8/8] PasswordElementTest: Test case with autosubmit element in Fieldset --- tests/FormElement/PasswordElementTest.php | 31 +++++++++++++ .../ElementWithAutosubmitInFieldsetForm.php | 45 +++++++++++++++++++ 2 files changed, 76 insertions(+) create mode 100644 tests/Lib/ElementWithAutosubmitInFieldsetForm.php diff --git a/tests/FormElement/PasswordElementTest.php b/tests/FormElement/PasswordElementTest.php index 62548089..a5880e53 100644 --- a/tests/FormElement/PasswordElementTest.php +++ b/tests/FormElement/PasswordElementTest.php @@ -4,6 +4,7 @@ use ipl\Html\Form; use ipl\Html\FormElement\PasswordElement; +use ipl\Tests\Html\Lib\ElementWithAutosubmitInFieldsetForm; use ipl\Tests\Html\TestDummy\ObscurePassword; class PasswordElementTest extends TestCase @@ -123,4 +124,34 @@ public function testOriginalPasswordMustBePreserved() $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' + ] + ); + } +}