From 2ae54c905ad9b34a23e3ed36ba3cb80728af54a3 Mon Sep 17 00:00:00 2001 From: Sabina Talipova Date: Tue, 24 Oct 2023 16:29:39 +1300 Subject: [PATCH] FIX Remove implicitly added item --- src/ChangeSet.php | 16 ++++++++ tests/php/ChangeSetTest.php | 75 ++++++++++++++++++++++++++++++++++--- tests/php/ChangeSetTest.yml | 4 ++ 3 files changed, 90 insertions(+), 5 deletions(-) diff --git a/src/ChangeSet.php b/src/ChangeSet.php index feeaef67..53801b68 100644 --- a/src/ChangeSet.php +++ b/src/ChangeSet.php @@ -216,6 +216,22 @@ public function removeObject(DataObject $object) // TODO: Handle case of implicit added item being removed. $item->delete(); + + // Get the implicitly included items for this ChangeSet + $implicit = $this->calculateImplicit(); + + foreach ($this->Changes()->filter(['Added' => ChangeSetItem::IMPLICITLY]) as $changeSetItem) { + $objectKey = $this->implicitKey($changeSetItem); + + // If a ChangeSetItem exists, but isn't in $implicit, it's no longer required, so delete it + if (!array_key_exists($objectKey, $implicit ?? [])) { + $changeSetItem->delete(); + } else { + // Otherwise it is required, so update ReferencedBy and remove from $implicit + $changeSetItem->ReferencedBy()->setByIDList($implicit[$objectKey]['ReferencedBy']); + unset($implicit[$objectKey]); + } + } } $this->sync(); diff --git a/tests/php/ChangeSetTest.php b/tests/php/ChangeSetTest.php index 768e70b9..5bf09833 100644 --- a/tests/php/ChangeSetTest.php +++ b/tests/php/ChangeSetTest.php @@ -334,7 +334,7 @@ public function testCanPublish() $base = $this->objFromFixture(ChangeSetTest\BaseObject::class, 'base'); $changeSet->addObject($base); $changeSet->sync(); - $this->assertEquals(5, $changeSet->Changes()->count()); + $this->assertEquals(6, $changeSet->Changes()->count()); // Test un-authenticated user cannot publish $this->logOut(); @@ -378,7 +378,7 @@ public function testCanPublishNested() $changeSet->addObject($base); $changeSet->addObject($mid1); $changeSet->sync(); - $this->assertEquals(5, $changeSet->Changes()->count()); + $this->assertEquals(6, $changeSet->Changes()->count()); // With model publish permissions only publish is allowed $this->assertTrue($changeSet->canPublish()); @@ -434,7 +434,7 @@ public function testCanEdit() $base = $this->objFromFixture(ChangeSetTest\BaseObject::class, 'base'); $changeSet->addObject($base); $changeSet->sync(); - $this->assertEquals(5, $changeSet->Changes()->count()); + $this->assertEquals(6, $changeSet->Changes()->count()); // Check canEdit $this->logOut(); @@ -465,7 +465,7 @@ public function testCanDelete() $base = $this->objFromFixture(ChangeSetTest\BaseObject::class, 'base'); $changeSet->addObject($base); $changeSet->sync(); - $this->assertEquals(5, $changeSet->Changes()->count()); + $this->assertEquals(6, $changeSet->Changes()->count()); // Check canDelete $this->logOut(); @@ -485,7 +485,7 @@ public function testCanView() $base = $this->objFromFixture(ChangeSetTest\BaseObject::class, 'base'); $changeSet->addObject($base); $changeSet->sync(); - $this->assertEquals(5, $changeSet->Changes()->count()); + $this->assertEquals(6, $changeSet->Changes()->count()); // Check canView $this->logOut(); @@ -735,4 +735,69 @@ public function testIsSyncedCanBeSkipped() $this->assertFalse($changeset->isSyncCalled, 'isSynced is skipped when providing truthy argument to publish'); } + + public function testRemoveObject() + { + $this->publishAllFixtures(); + + $mid1 = $this->objFromFixture(ChangeSetTest\MidObject::class, 'mid1'); // Item mid1 reference Item end1 + $mid2 = $this->objFromFixture(ChangeSetTest\MidObject::class, 'mid2'); // Item mid2 reference Item end2 + $mid3 = $this->objFromFixture(ChangeSetTest\MidObject::class, 'mid4'); // Item mid4 reference Item end2 + + $end1 = $this->objFromFixture(ChangeSetTest\EndObject::class, 'end1'); // Item end1 + + $changeset = new ChangeSet(); + $changeset->write(); + $changeset->addObject($mid1); + $changeset->addObject($mid2); + $changeset->addObject($mid3); + $changeset->addObject($end1); // Item end1 explisitly added to ChangeSet + $changeset->publish(); + + $this->assertChangeSetLooksLike( + $changeset, + [ + ChangeSetTest\MidObject::class . '.mid1' => ChangeSetItem::EXPLICITLY, + ChangeSetTest\EndObject::class . '.end1' => ChangeSetItem::IMPLICITLY, + ChangeSetTest\MidObject::class . '.mid2' => ChangeSetItem::EXPLICITLY, + ChangeSetTest\EndObject::class . '.end2' => ChangeSetItem::IMPLICITLY, + ChangeSetTest\MidObject::class . '.mid4' => ChangeSetItem::EXPLICITLY, + ChangeSetTest\EndObject::class . '.end2' => ChangeSetItem::IMPLICITLY, + ChangeSetTest\EndObject::class . '.end1' => ChangeSetItem::EXPLICITLY + ] + ); + + $changeset->removeObject($mid1); // Remove mid1 + + $this->assertChangeSetLooksLike( + $changeset, + [ + ChangeSetTest\MidObject::class . '.mid2' => ChangeSetItem::EXPLICITLY, + ChangeSetTest\EndObject::class . '.end2' => ChangeSetItem::IMPLICITLY, + ChangeSetTest\MidObject::class . '.mid4' => ChangeSetItem::EXPLICITLY, + ChangeSetTest\EndObject::class . '.end2' => ChangeSetItem::IMPLICITLY, + ChangeSetTest\EndObject::class . '.end1' => ChangeSetItem::EXPLICITLY // Item end1 is still in ChangeSet + ] + ); + + $changeset->removeObject($mid2); // Remove mid2 + + $this->assertChangeSetLooksLike( + $changeset, + [ + ChangeSetTest\MidObject::class . '.mid4' => ChangeSetItem::EXPLICITLY, + ChangeSetTest\EndObject::class . '.end2' => ChangeSetItem::IMPLICITLY, // Item end2 is still in ChangeSet + ChangeSetTest\EndObject::class . '.end1' => ChangeSetItem::EXPLICITLY + ] + ); + + $changeset->removeObject($mid3); // Remove mid3 + + $this->assertChangeSetLooksLike( + $changeset, + [ + ChangeSetTest\EndObject::class . '.end1' => ChangeSetItem::EXPLICITLY // Item end1 is still in ChangeSet + ] + ); + } } diff --git a/tests/php/ChangeSetTest.yml b/tests/php/ChangeSetTest.yml index ae4ad8a0..1bc7454f 100644 --- a/tests/php/ChangeSetTest.yml +++ b/tests/php/ChangeSetTest.yml @@ -23,6 +23,10 @@ SilverStripe\Versioned\Tests\ChangeSetTest\MidObject: End: =>SilverStripe\Versioned\Tests\ChangeSetTest\EndObject.end2 mid3: Base: =>SilverStripe\Versioned\Tests\ChangeSetTest\BaseObject.base2 + mid4: + Bar: 3 + Base: =>SilverStripe\Versioned\Tests\ChangeSetTest\BaseObject.base + End: =>SilverStripe\Versioned\Tests\ChangeSetTest\EndObject.end2 SilverStripe\Versioned\Tests\ChangeSetTest\UnversionedObject: unversioned1: Title: 'object'