BUG Fix extra blank Group being created when creating a new Group (#9325)

* Fix extra blank Group being created when creating a new Group

* Update tests to reflect expected behavior

* Improved tests
This commit is contained in:
Andre Kiste 2019-11-27 09:32:33 +13:00 committed by Maxime Rainville
parent 91e4aa90f1
commit 6650d81324
2 changed files with 22 additions and 5 deletions

View File

@ -1643,6 +1643,11 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
*/ */
private function skipWriteComponents($recursive, DataObject $target, array &$skip) private function skipWriteComponents($recursive, DataObject $target, array &$skip)
{ {
// skip writing component if it doesn't exist
if (!$target->exists()) {
return true;
}
// We only care about the skip list if our call is meant to be recursive // We only care about the skip list if our call is meant to be recursive
if (!$recursive) { if (!$recursive) {
return false; return false;

View File

@ -1011,6 +1011,12 @@ class DataObjectTest extends SapphireTest
DB::query("SELECT \"CaptainID\" FROM \"DataObjectTest_Team\" WHERE \"ID\" = $team->ID")->value() DB::query("SELECT \"CaptainID\" FROM \"DataObjectTest_Team\" WHERE \"ID\" = $team->ID")->value()
); );
// Can write to component directly
$this->assertEquals(false, $team->Captain()->IsRetired);
$team->Captain()->IsRetired = true;
$team->Captain()->write();
$this->assertEquals(true, $team->Captain()->IsRetired, 'Saves writes to components directly');
/* After giving it a value, you should also be able to set it back to null */ /* After giving it a value, you should also be able to set it back to null */
$team->CaptainID = ''; $team->CaptainID = '';
$team->write(); $team->write();
@ -2169,7 +2175,10 @@ class DataObjectTest extends SapphireTest
// Write object with components // Write object with components
$ceo->Name = 'Edward Scissorhands'; $ceo->Name = 'Edward Scissorhands';
$ceo->write(false, false, false, true); $ceo->write(false, false, false, true);
$this->assertTrue($ceo->Company()->isInDB(), 'write() writes belongs_to components to the database.'); $this->assertFalse(
$ceo->Company()->isInDB(),
'write() does not write belongs_to components to the database that do not already exist.'
);
$newCEO = DataObject::get_by_id(DataObjectTest\CEO::class, $ceo->ID); $newCEO = DataObject::get_by_id(DataObjectTest\CEO::class, $ceo->ID);
$this->assertEquals( $this->assertEquals(
@ -2199,7 +2208,7 @@ class DataObjectTest extends SapphireTest
'belongs_to returns the right results.' 'belongs_to returns the right results.'
); );
// Test automatic creation of class where no assigment exists // Test automatic creation of class where no assignment exists
$ceo = new DataObjectTest\CEO(); $ceo = new DataObjectTest\CEO();
$ceo->write(); $ceo->write();
@ -2208,11 +2217,14 @@ class DataObjectTest extends SapphireTest
'DataObjects across polymorphic belongs_to relations are automatically created.' 'DataObjects across polymorphic belongs_to relations are automatically created.'
); );
$this->assertEquals($ceo->ID, $ceo->CompanyOwned()->OwnerID, 'Remote IDs are automatically set.'); $this->assertEquals($ceo->ID, $ceo->CompanyOwned()->OwnerID, 'Remote IDs are automatically set.');
$this->assertInstanceOf($ceo->CompanyOwned()->OwnerClass, $ceo, 'Remote class is automatically set'); $this->assertInstanceOf($ceo->CompanyOwned()->OwnerClass, $ceo, 'Remote class is automatically set.');
// Write object with components // Skip writing components that do not exist
$ceo->write(false, false, false, true); $ceo->write(false, false, false, true);
$this->assertTrue($ceo->CompanyOwned()->isInDB(), 'write() writes belongs_to components to the database.'); $this->assertFalse(
$ceo->CompanyOwned()->isInDB(),
'write() does not write belongs_to components to the database that do not already exist.'
);
$newCEO = DataObject::get_by_id(DataObjectTest\CEO::class, $ceo->ID); $newCEO = DataObject::get_by_id(DataObjectTest\CEO::class, $ceo->ID);
$this->assertEquals( $this->assertEquals(