diff --git a/src/ORM/DataObject.php b/src/ORM/DataObject.php index a47034381..028025551 100644 --- a/src/ORM/DataObject.php +++ b/src/ORM/DataObject.php @@ -166,6 +166,11 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity */ const CHANGE_NONE = 0; + /** + * Represents a field that has had a change forced + */ + const CHANGE_FORCED = 0.5; + /** * Represents a field that has changed type, although not the loosely defined value. * (before !== after && before == after) @@ -1126,7 +1131,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity foreach ($fieldNames as $fieldName) { if (!isset($this->changed[$fieldName])) { - $this->changed[$fieldName] = self::CHANGE_STRICT; + $this->changed[$fieldName] = self::CHANGE_FORCED; } // Populate the null values in record so that they actually get written if (!isset($this->record[$fieldName])) { @@ -1520,7 +1525,11 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity // If there's any relations that couldn't be saved before, save them now (we have an ID here) $this->writeRelations(); $this->onAfterWrite(); - $this->changed = array(); + + // Reset isChanged data + // DBComposites properly bound to the parent record will also have their isChanged value reset + $this->changed = []; + $this->original = $this->record; } else { if ($showDebug) { Debug::message("no changes for DataObject"); @@ -2487,7 +2496,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity } /** - * Return the fields that have changed. + * Return the fields that have changed since the last write. * * The change level affects what the functions defines as "changed": * - Level CHANGE_STRICT (integer 1) will return strict changes, even !== ones. @@ -2628,22 +2637,25 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity } // if a field is not existing or has strictly changed - if (!isset($this->record[$fieldName]) || $this->record[$fieldName] !== $val) { + if (!isset($this->original[$fieldName]) || $this->original[$fieldName] !== $val) { // TODO Add check for php-level defaults which are not set in the db // TODO Add check for hidden input-fields (readonly) which are not set in the db // At the very least, the type has changed $this->changed[$fieldName] = self::CHANGE_STRICT; - if ((!isset($this->record[$fieldName]) && $val) - || (isset($this->record[$fieldName]) && $this->record[$fieldName] != $val) + if ((!isset($this->original[$fieldName]) && $val) + || (isset($this->original[$fieldName]) && $this->original[$fieldName] != $val) ) { // Value has changed as well, not just the type $this->changed[$fieldName] = self::CHANGE_VALUE; } - - // Value is always saved back when strict check succeeds. - $this->record[$fieldName] = $val; + // Value has been restored to its original, remove any record of the change + } elseif (isset($this->changed[$fieldName]) && $this->changed[$fieldName] !== self::CHANGE_FORCED) { + unset($this->changed[$fieldName]); } + + // Value is saved regardless, since the change detection relates to the last write + $this->record[$fieldName] = $val; } return $this; } diff --git a/src/ORM/FieldType/DBComposite.php b/src/ORM/FieldType/DBComposite.php index 781cd200c..844b00afa 100644 --- a/src/ORM/FieldType/DBComposite.php +++ b/src/ORM/FieldType/DBComposite.php @@ -36,6 +36,12 @@ abstract class DBComposite extends DBField */ private static $composite_db = array(); + /** + * Marker as to whether this record has changed + * Only used when deference to the parent object isn't possible + */ + protected $isChanged = false; + /** * Either the parent dataobject link, or a record of saved values for each field * @@ -113,6 +119,10 @@ abstract class DBComposite extends DBField } + /** + * Returns true if this composite field has changed. + * For fields bound to a DataObject, this will be cleared when the DataObject is written. + */ public function isChanged() { // When unbound, use the local changed flag diff --git a/tests/php/ORM/DataObjectTest.php b/tests/php/ORM/DataObjectTest.php index aee0b8d1c..d34373b63 100644 --- a/tests/php/ORM/DataObjectTest.php +++ b/tests/php/ORM/DataObjectTest.php @@ -765,6 +765,62 @@ class DataObjectTest extends SapphireTest ); } + public function testChangedFieldsWhenRestoringData() + { + $obj = $this->objFromFixture(DataObjectTest\Player::class, 'captain1'); + $obj->FirstName = 'Captain-changed'; + $obj->FirstName = 'Captain'; + + $this->assertEquals( + [], + $obj->getChangedFields(true, DataObject::CHANGE_STRICT) + ); + } + + public function testChangedFieldsAfterWrite() + { + $obj = $this->objFromFixture(DataObjectTest\Player::class, 'captain1'); + $obj->FirstName = 'Captain-changed'; + $obj->write(); + $obj->FirstName = 'Captain'; + + $this->assertEquals( + array( + 'FirstName' => array( + 'before' => 'Captain-changed', + 'after' => 'Captain', + 'level' => DataObject::CHANGE_VALUE, + ), + ), + $obj->getChangedFields(true, DataObject::CHANGE_VALUE) + ); + } + + public function testForceChangeCantBeCancelledUntilWrite() + { + $obj = $this->objFromFixture(DataObjectTest\Player::class, 'captain1'); + $this->assertFalse($obj->isChanged('FirstName')); + $this->assertFalse($obj->isChanged('Surname')); + + $obj->forceChange(); + $this->assertTrue($obj->isChanged('FirstName')); + $this->assertTrue($obj->isChanged('Surname')); + + $this->assertFalse($obj->isChanged('FirstName', DataObject::CHANGE_VALUE)); + $this->assertFalse($obj->isChanged('Surname', DataObject::CHANGE_VALUE)); + + $obj->FirstName = 'Captain'; + $this->assertTrue($obj->isChanged('FirstName')); + $this->assertTrue($obj->isChanged('Surname')); + + $obj->write(); + $this->assertFalse($obj->isChanged('FirstName')); + $this->assertFalse($obj->isChanged('Surname')); + + $obj->FirstName = 'Captain'; + $this->assertFalse($obj->isChanged('FirstName')); + } + /** * @skipUpgrade */ diff --git a/tests/php/Security/MemberTest.php b/tests/php/Security/MemberTest.php index bdf664c1f..4b5341622 100644 --- a/tests/php/Security/MemberTest.php +++ b/tests/php/Security/MemberTest.php @@ -655,10 +655,14 @@ class MemberTest extends FunctionalTest $member = $this->objFromFixture(Member::class, 'test'); $member->setName('Test Some User'); $this->assertEquals('Test Some User', $member->getName()); + $this->assertEquals('Test Some', $member->FirstName); + $this->assertEquals('User', $member->Surname); $member->setName('Test'); $this->assertEquals('Test', $member->getName()); $member->FirstName = 'Test'; $member->Surname = ''; + $this->assertEquals('Test', $member->FirstName); + $this->assertEquals('', $member->Surname); $this->assertEquals('Test', $member->getName()); }