FIX Remove implicitly added item

silverstripe · Oct 27, 2023 · 2ae54c9 · 2ae54c9
1 parent 302aa4c
commit 2ae54c9
Show file tree

Hide file tree

Showing 3 changed files with 90 additions and 5 deletions.
diff --git 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
@@ -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
@@ -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'