diff --git a/security/Group.php b/security/Group.php
index c45823d51..f9c661efd 100755
--- a/security/Group.php
+++ b/security/Group.php
@@ -115,7 +115,7 @@ class Group extends DataObject {
$config->getComponentByType('GridFieldAddExistingAutocompleter')
->setResultsFormat('$Title ($Email)')->setSearchFields(array('FirstName', 'Surname', 'Email'));
$config->getComponentByType('GridFieldDetailForm')
- ->setValidator(new Member_Validator())
+ ->setValidator(Member_Validator::create())
->setItemEditFormCallback(function($form, $component) use($group) {
$record = $form->getRecord();
$groupsField = $form->Fields()->dataFieldByName('DirectGroups');
diff --git a/security/Member.php b/security/Member.php
index e0195153a..ab8f4b3e7 100644
--- a/security/Member.php
+++ b/security/Member.php
@@ -766,6 +766,7 @@ class Member extends DataObject implements TemplateGlobalProvider {
*/
public function getValidator() {
$validator = Injector::inst()->create('Member_Validator');
+ $validator->setForMember($this);
$this->extend('updateValidator', $validator);
return $validator;
@@ -1801,21 +1802,39 @@ class Member_ForgotPasswordEmail extends Email {
* Member Validator
*
* Custom validation for the Member object can be achieved either through an
- * {@link DataExtension} on the Member object or, by specifying a subclass of
+ * {@link DataExtension} on the Member_Validator object or, by specifying a subclass of
* {@link Member_Validator} through the {@link Injector} API.
- *
+ * The Validator can also be modified by adding an Extension to Member and implement the
+ * updateValidator hook.
* {@see Member::getValidator()}
*
+ * Additional required fields can also be set via config API, eg.
+ *
+ * Member_Validator:
+ * customRequired:
+ * - Surname
+ *
+ *
* @package framework
* @subpackage security
*/
-class Member_Validator extends RequiredFields {
-
+class Member_Validator extends RequiredFields
+{
+ /**
+ * Fields that are required by this validator
+ * @config
+ * @var array
+ */
protected $customRequired = array(
'FirstName',
'Email'
);
+ /**
+ * Determine what member this validator is meant for
+ * @var Member
+ */
+ protected $forMember = null;
/**
* Constructor
@@ -1829,7 +1848,33 @@ class Member_Validator extends RequiredFields {
$required = array_merge($required, $this->customRequired);
- parent::__construct($required);
+ // check for config API values and merge them in
+ $config = $this->config()->customRequired;
+ if(is_array($config)){
+ $required = array_merge($required, $config);
+ }
+
+ parent::__construct(array_unique($required));
+ }
+
+ /**
+ * Get the member this validator applies to.
+ * @return Member
+ */
+ public function getForMember()
+ {
+ return $this->forMember;
+ }
+
+ /**
+ * Set the Member this validator applies to.
+ * @param Member $value
+ * @return $this
+ */
+ public function setForMember(Member $value)
+ {
+ $this->forMember = $value;
+ return $this;
}
/**
@@ -1842,47 +1887,54 @@ class Member_Validator extends RequiredFields {
* @return bool Returns TRUE if the submitted data is valid, otherwise
* FALSE.
*/
- public function php($data) {
+ public function php($data)
+ {
$valid = parent::php($data);
- $identifierField = Member::config()->unique_identifier_field;
- $member = DataObject::get_one('Member', array(
- "\"$identifierField\"" => $data[$identifierField]
- ));
+ $identifierField = (string)Member::config()->unique_identifier_field;
- // if we are in a complex table field popup, use ctf[childID], else use ID
- if(isset($_REQUEST['ctf']['childID'])) {
- $id = $_REQUEST['ctf']['childID'];
- } elseif(isset($_REQUEST['ID'])) {
- $id = $_REQUEST['ID'];
- } else {
- $id = null;
- }
-
- if($id && is_object($member) && $member->ID != $id) {
- $uniqueField = $this->form->Fields()->dataFieldByName($identifierField);
- $this->validationError(
- $uniqueField->id(),
- _t(
- 'Member.VALIDATIONMEMBEREXISTS',
- 'A member already exists with the same %s',
- array('identifier' => strtolower($identifierField))
- ),
- 'required'
- );
- $valid = false;
- }
-
- // Execute the validators on the extensions
- if($this->extension_instances) {
- foreach($this->extension_instances as $extension) {
- if(method_exists($extension, 'hasMethod') && $extension->hasMethod('updatePHP')) {
- $valid &= $extension->updatePHP($data, $this->form);
+ // Only validate identifier field if it's actually set. This could be the case if
+ // somebody removes `Email` from the list of required fields.
+ if(isset($data[$identifierField])){
+ $id = isset($data['ID']) ? (int)$data['ID'] : 0;
+ if(!$id && ($ctrl = $this->form->getController())){
+ // get the record when within GridField (Member editing page in CMS)
+ if($ctrl instanceof GridFieldDetailForm_ItemRequest && $record = $ctrl->getRecord()){
+ $id = $record->ID;
}
}
+
+ // If there's no ID passed via controller or form-data, use the assigned member (if available)
+ if(!$id && ($member = $this->getForMember())){
+ $id = $member->exists() ? $member->ID : 0;
+ }
+
+ // set the found ID to the data array, so that extensions can also use it
+ $data['ID'] = $id;
+
+ $members = Member::get()->filter($identifierField, $data[$identifierField]);
+ if($id) {
+ $members = $members->exclude('ID', $id);
+ }
+
+ if($members->count() > 0) {
+ $this->validationError(
+ $identifierField,
+ _t(
+ 'Member.VALIDATIONMEMBEREXISTS',
+ 'A member already exists with the same {identifier}',
+ array('identifier' => Member::singleton()->fieldLabel($identifierField))
+ ),
+ 'required'
+ );
+ $valid = false;
+ }
}
- return $valid;
- }
+ // Execute the validators on the extensions
+ $results = $this->extend('updatePHP', $data, $this->form);
+ $results[] = $valid;
+ return min($results);
+ }
}
diff --git a/tests/security/MemberTest.php b/tests/security/MemberTest.php
index 4a8e66b4c..474dfd89a 100644
--- a/tests/security/MemberTest.php
+++ b/tests/security/MemberTest.php
@@ -678,7 +678,7 @@ class MemberTest extends FunctionalTest {
$newAdminGroup = new Group(array('Title' => 'newadmin'));
$newAdminGroup->write();
Permission::grant($newAdminGroup->ID, 'ADMIN');
-
+
// Test staff member can't be added to admin groups
$this->assertFalse($staffMember->inGroup($newAdminGroup));
$staffMember->Groups()->setByIDList(array($newAdminGroup->ID));
@@ -842,7 +842,135 @@ class MemberTest extends FunctionalTest {
}
}
- public function testCustomMemberValidator() {
+ public function testMemberValidator()
+ {
+ // clear custom requirements for this test
+ Config::inst()->update('Member_Validator', 'customRequired', null);
+ $memberA = $this->objFromFixture('Member', 'admin');
+ $memberB = $this->objFromFixture('Member', 'test');
+
+ // create a blank form
+ $form = new MemberTest_ValidatorForm();
+
+ $validator = new Member_Validator();
+ $validator->setForm($form);
+
+ // Simulate creation of a new member via form, but use an existing member identifier
+ $fail = $validator->php(array(
+ 'FirstName' => 'Test',
+ 'Email' => $memberA->Email
+ ));
+
+ $this->assertFalse(
+ $fail,
+ 'Member_Validator must fail when trying to create new Member with existing Email.'
+ );
+
+ // populate the form with values from another member
+ $form->loadDataFrom($memberB);
+
+ // Assign the validator to an existing member
+ // (this is basically the same as passing the member ID with the form data)
+ $validator->setForMember($memberB);
+
+ // Simulate update of a member via form and use an existing member Email
+ $fail = $validator->php(array(
+ 'FirstName' => 'Test',
+ 'Email' => $memberA->Email
+ ));
+
+ // Simulate update to a new Email address
+ $pass1 = $validator->php(array(
+ 'FirstName' => 'Test',
+ 'Email' => 'membervalidatortest@testing.com'
+ ));
+
+ // Pass in the same Email address that the member already has. Ensure that case is valid
+ $pass2 = $validator->php(array(
+ 'FirstName' => 'Test',
+ 'Surname' => 'User',
+ 'Email' => $memberB->Email
+ ));
+
+ $this->assertFalse(
+ $fail,
+ 'Member_Validator must fail when trying to update existing member with existing Email.'
+ );
+
+ $this->assertTrue(
+ $pass1,
+ 'Member_Validator must pass when Email is updated to a value that\'s not in use.'
+ );
+
+ $this->assertTrue(
+ $pass2,
+ 'Member_Validator must pass when Member updates his own Email to the already existing value.'
+ );
+ }
+
+ public function testMemberValidatorWithExtensions()
+ {
+ // clear custom requirements for this test
+ Config::inst()->update('Member_Validator', 'customRequired', null);
+
+ // create a blank form
+ $form = new MemberTest_ValidatorForm();
+
+ // Test extensions
+ Member_Validator::add_extension('MemberTest_MemberValidator_SurnameMustMatchFirstNameExtension');
+ $validator = new Member_Validator();
+ $validator->setForm($form);
+
+ // This test should fail, since the extension enforces FirstName == Surname
+ $fail = $validator->php(array(
+ 'FirstName' => 'Test',
+ 'Surname' => 'User',
+ 'Email' => 'test-member-validator-extension@testing.com'
+ ));
+
+ $pass = $validator->php(array(
+ 'FirstName' => 'Test',
+ 'Surname' => 'Test',
+ 'Email' => 'test-member-validator-extension@testing.com'
+ ));
+
+ $this->assertFalse(
+ $fail,
+ 'Member_Validator must fail because of added extension.'
+ );
+
+ $this->assertTrue(
+ $pass,
+ 'Member_Validator must succeed, since it meets all requirements.'
+ );
+
+ // Add another extension that always fails. This ensures that all extensions are considered in the validation
+ Member_Validator::add_extension('MemberTest_MemberValidator_AlwaysFailsExtension');
+ $validator = new Member_Validator();
+ $validator->setForm($form);
+
+ // Even though the data is valid, This test should still fail, since one extension always returns false
+ $fail = $validator->php(array(
+ 'FirstName' => 'Test',
+ 'Surname' => 'Test',
+ 'Email' => 'test-member-validator-extension@testing.com'
+ ));
+
+ $this->assertFalse(
+ $fail,
+ 'Member_Validator must fail because of added extensions.'
+ );
+
+ // Remove added extensions
+ Member_Validator::remove_extension('MemberTest_MemberValidator_AlwaysFailsExtension');
+ Member_Validator::remove_extension('MemberTest_MemberValidator_SurnameMustMatchFirstNameExtension');
+ }
+
+ public function testCustomMemberValidator()
+ {
+ // clear custom requirements for this test
+ Config::inst()->update('Member_Validator', 'customRequired', null);
+
$member = $this->objFromFixture('Member', 'admin');
$form = new MemberTest_ValidatorForm();
@@ -861,7 +989,7 @@ class MemberTest extends FunctionalTest {
'Surname' => ''
));
- $this->assertTrue($pass, 'Validator requires on FirstName and Email');
+ $this->assertTrue($pass, 'Validator requires a FirstName and Email');
$this->assertFalse($fail, 'Missing FirstName');
$ext = new MemberTest_ValidatorExtension();
@@ -919,6 +1047,30 @@ class MemberTest_ValidatorExtension extends DataExtension implements TestOnly {
}
}
+/**
+ * Extension that adds additional validation criteria
+ * @package framework
+ * @subpackage tests
+ */
+class MemberTest_MemberValidator_SurnameMustMatchFirstNameExtension extends DataExtension implements TestOnly
+{
+ public function updatePHP($data, $form) {
+ return $data['FirstName'] == $data['Surname'];
+ }
+}
+
+/**
+ * Extension that adds additional validation criteria
+ * @package framework
+ * @subpackage tests
+ */
+class MemberTest_MemberValidator_AlwaysFailsExtension extends DataExtension implements TestOnly
+{
+ public function updatePHP($data, $form) {
+ return false;
+ }
+}
+
/**
* @package framework
* @subpackage tests