From c347b35042c2ed164e7b685785f0fd7dd169865f Mon Sep 17 00:00:00 2001
From: Steve Boyd <emteknetnz@gmail.com>
Date: Tue, 13 Feb 2024 10:08:49 +1300
Subject: [PATCH] FIX Archive record from live table on delete

---
 src/Controllers/LinkFieldController.php       | 15 +++----
 .../Controllers/LinkFieldControllerTest.php   | 42 ++++++++++++++++---
 tests/php/Models/LinkTest/LinkOwner.php       |  5 +++
 3 files changed, 48 insertions(+), 14 deletions(-)

diff --git a/src/Controllers/LinkFieldController.php b/src/Controllers/LinkFieldController.php
index 9e0f263a..96da66af 100644
--- a/src/Controllers/LinkFieldController.php
+++ b/src/Controllers/LinkFieldController.php
@@ -22,6 +22,8 @@
 use SilverStripe\ORM\DataObject;
 use SilverStripe\LinkField\Form\LinkField;
 use SilverStripe\LinkField\Form\MultiLinkField;
+use SilverStripe\ORM\Queries\SQLUpdate;
+use SilverStripe\Versioned\Versioned;
 
 class LinkFieldController extends LeftAndMain
 {
@@ -136,15 +138,10 @@ public function linkDelete(): HTTPResponse
         if (!SecurityToken::inst()->checkRequest($this->getRequest())) {
             $this->jsonError(400);
         }
-        // delete() will also delete any published version immediately
-        $link->delete();
-        // Update owner object if this Link is on a has_one relation on the owner
-        $owner = $this->getOwnerFromRequest();
-        $ownerRelation = $this->getOwnerRelationFromRequest();
-        $hasOne = Injector::inst()->get($owner->ClassName)->hasOne();
-        if (array_key_exists($ownerRelation, $hasOne) && $owner->canEdit()) {
-            $owner->$ownerRelation = null;
-            $owner->write();
+        if ($link->hasExtension(Versioned::class)) {
+            $link->doArchive();
+        } else {
+            $link->delete();
         }
         // Send response
         return $this->jsonSuccess(204);
diff --git a/tests/php/Controllers/LinkFieldControllerTest.php b/tests/php/Controllers/LinkFieldControllerTest.php
index 5c679507..8af2f22c 100644
--- a/tests/php/Controllers/LinkFieldControllerTest.php
+++ b/tests/php/Controllers/LinkFieldControllerTest.php
@@ -9,6 +9,7 @@
 use SilverStripe\Security\SecurityToken;
 use SilverStripe\Control\HTTPRequest;
 use SilverStripe\LinkField\Tests\Models\LinkTest\LinkOwner;
+use SilverStripe\Versioned\Versioned;
 
 class LinkFieldControllerTest extends FunctionalTest
 {
@@ -24,6 +25,15 @@ class LinkFieldControllerTest extends FunctionalTest
     protected function setUp(): void
     {
         parent::setUp();
+        // The Versioned extension is added to LinkOwner, though it's only used for the
+        // unit tests speicifically to test Versioned functionality. It interferes with
+        // other units tests that were written before the Versioned extension was added
+        // The extenion needs to be added on the class rather than dynmically so that the
+        // relevant database tables are created. The extension is added back in on unit
+        // tests that requrie it. There's a call to add the extension back in the
+        // tearDown method
+        LinkOwner::remove_extension(Versioned::class);
+
         $this->logInWithPermission('ADMIN');
         // CSRF token check is normally disabled for unit-tests
         $this->securityTokenWasEnabled = SecurityToken::is_enabled();
@@ -45,6 +55,8 @@ protected function tearDown(): void
         if (!$this->securityTokenWasEnabled) {
             SecurityToken::disable();
         }
+        // Reset the extension that was removed in the setUp() method
+        LinkOwner::add_extension(Versioned::class);
     }
 
     /**
@@ -493,14 +505,9 @@ public function testLinkDelete(
         $this->assertSame($expectedCode, $response->getStatusCode());
         if ($expectedCode >= 400) {
             $this->assertNotNull(TestPhoneLink::get()->byID($fixtureID));
-            $owner = $this->getFixtureLinkOwner();
-            $this->assertSame($ownerLinkID, $owner->LinkID);
         } else {
             $this->assertNull(TestPhoneLink::get()->byID($fixtureID));
-            $owner = $this->getFixtureLinkOwner();
-            $this->assertSame(0, $owner->LinkID);
         }
-        $this->assertTrue(true);
     }
 
     public function provideLinkDelete(): array
@@ -544,6 +551,31 @@ public function provideLinkDelete(): array
         ];
     }
 
+    public function testLinkDeleteVersions(): void
+    {
+        LinkOwner::add_extension(Versioned::class);
+        // Need to re-link rejoin $link to $owner as the fixture data for whatever reason
+        // because doing weird stuff with versioning and adding extensions
+        $link = $this->getFixtureLink();
+        $owner = $this->getFixtureLinkOwner();
+        $owner->Link = $link;
+        $link->publishSingle();
+        $owner->publishSingle();
+        $linkID = $link->ID;
+        $ownerID = $owner->ID;
+        $ownerClass = urlencode($owner->ClassName);
+        $ownerRelation = 'Link';
+        $url = "/admin/linkfield/delete/$linkID?ownerID=$ownerID&ownerClass=$ownerClass&ownerRelation=$ownerRelation";
+        $headers = $this->csrfTokenheader();
+        $liveLink = Versioned::get_by_stage($link::class, Versioned::LIVE)->byID($linkID);
+        $this->assertNotNull($liveLink);
+        $this->mainSession->sendRequest('DELETE', $url, [], $headers);
+        $draftLink = Versioned::get_by_stage($link::class, Versioned::DRAFT)->byID($linkID);
+        $this->assertNull($draftLink);
+        $liveLink = Versioned::get_by_stage($link::class, Versioned::LIVE)->byID($linkID);
+        $this->assertNull($liveLink);
+    }
+
     /**
      * @dataProvider provideLinkSort
      */
diff --git a/tests/php/Models/LinkTest/LinkOwner.php b/tests/php/Models/LinkTest/LinkOwner.php
index bd61af73..08dd7996 100644
--- a/tests/php/Models/LinkTest/LinkOwner.php
+++ b/tests/php/Models/LinkTest/LinkOwner.php
@@ -5,6 +5,7 @@
 use SilverStripe\Dev\TestOnly;
 use SilverStripe\LinkField\Models\Link;
 use SilverStripe\ORM\DataObject;
+use SilverStripe\Versioned\Versioned;
 
 class LinkOwner extends DataObject implements TestOnly
 {
@@ -17,6 +18,10 @@ class LinkOwner extends DataObject implements TestOnly
         'LinkList2' => Link::class . '.Owner',
     ];
 
+    private static array $extensions = [
+        Versioned::class,
+    ];
+
     // Allows us to toggle permissions easily within a unit test
     public bool $canView = true;
     public bool $canEdit = true;