From 8d8f3abacec7703167f4cb4fc9f07d6495c9cf7b Mon Sep 17 00:00:00 2001 From: Jeremy Shipman Date: Tue, 7 Jan 2014 17:16:58 +1300 Subject: [PATCH] FIX: ConfirmedPasswordField setName failed to set names of child fields. FIX: ConfirmedPasswordField relied on POST variables. These should instead come from setValue(). Added all important tests for validating the field: valid if passwords match, invalid if passwords differ. --- forms/ConfirmedPasswordField.php | 33 +++++++++++++++++----- tests/forms/ConfirmedPasswordFieldTest.php | 20 +++++++++++++ 2 files changed, 46 insertions(+), 7 deletions(-) diff --git a/forms/ConfirmedPasswordField.php b/forms/ConfirmedPasswordField.php index 35ee2224a..257f3d023 100644 --- a/forms/ConfirmedPasswordField.php +++ b/forms/ConfirmedPasswordField.php @@ -53,6 +53,13 @@ class ConfirmedPasswordField extends FormField { * @param boolean $showOnClick */ protected $showOnClick = false; + + + /** + * A place to temporarly store the confirm password value + * @var string + */ + protected $confirmValue; /** * Title for the link that triggers the visibility of password fields. @@ -247,10 +254,8 @@ class ConfirmedPasswordField extends FormField { $oldValue = $this->value; if(is_array($value)) { - //only set the value if it's valid! - if($this->validate(RequiredFields::create())) { - $this->value = $value['_Password']; - } + $this->value = $value['_Password']; + $this->confirmValue = $value['_ConfirmPassword']; if($this->showOnClick && isset($value['_PasswordFieldVisible'])) { $this->children->fieldByName($this->getName() . '[_PasswordFieldVisible]') @@ -274,6 +279,20 @@ class ConfirmedPasswordField extends FormField { return $this; } + /** + * Update the names of the child fields when updating name of field. + * + * @param string $name new name to give to the field. + */ + public function setName($name) { + $this->children->fieldByName($this->getName() . '[_Password]') + ->setName($name . '[_Password]'); + $this->children->fieldByName($this->getName() . '[_ConfirmPassword]') + ->setName($name . '[_ConfirmPassword]'); + + return parent::setName($name); + } + /** * Determines if the field was actually shown on the client side - if not, * we don't validate or save it. @@ -301,9 +320,9 @@ class ConfirmedPasswordField extends FormField { $passwordField = $this->children->fieldByName($name.'[_Password]'); $passwordConfirmField = $this->children->fieldByName($name.'[_ConfirmPassword]'); - $passwordField->setValue($_POST[$name]['_Password']); - $passwordConfirmField->setValue($_POST[$name]['_ConfirmPassword']); - + $passwordField->setValue($this->value); + $passwordConfirmField->setValue($this->confirmValue); + $value = $passwordField->Value(); // both password-fields should be the same diff --git a/tests/forms/ConfirmedPasswordFieldTest.php b/tests/forms/ConfirmedPasswordFieldTest.php index abe8a9794..ec22fb600 100644 --- a/tests/forms/ConfirmedPasswordFieldTest.php +++ b/tests/forms/ConfirmedPasswordFieldTest.php @@ -4,6 +4,7 @@ * @subpackage tests */ class ConfirmedPasswordFieldTest extends SapphireTest { + public function testSetValue() { $field = new ConfirmedPasswordField('Test', 'Testing', 'valueA'); $this->assertEquals('valueA', $field->Value()); @@ -52,4 +53,23 @@ class ConfirmedPasswordFieldTest extends SapphireTest { $this->assertNotContains("showOnClick", $fieldHTML, "Test class for hiding/showing the form contents is set"); } + + public function testValidation() { + $field = new ConfirmedPasswordField('Test', 'Testing', array( + "_Password" => "abc123", + "_ConfirmPassword" => "abc123" + )); + $validator = new RequiredFields(); + $form = new Form($this, 'Form', new FieldList($field), new FieldList(), $validator); + $this->assertTrue($field->validate($validator)); + $field->setName("TestNew"); //try changing name of field + $this->assertTrue($field->validate($validator)); + //non-matching password should make the field invalid + $field->setValue(array( + "_Password" => "abc123", + "_ConfirmPassword" => "123abc" + )); + $this->assertFalse($field->validate($validator)); + } + }