FIX Use canDelete, not the now-deleted canArchive (#318)

silverstripe · Aug 15, 2024 · 72206dc · 72206dc
1 parent 5904235
commit 72206dc
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 52 deletions.
diff --git a/src/Controllers/LinkFieldController.php b/src/Controllers/LinkFieldController.php
@@ -16,7 +16,6 @@
 use SilverStripe\Control\Controller;
 use SilverStripe\Control\HTTPRequest;
 use SilverStripe\Core\Injector\Injector;
-use SilverStripe\Dev\Deprecation;
 use SilverStripe\Forms\HiddenField;
 use SilverStripe\LinkField\Services\LinkTypeService;
 use SilverStripe\ORM\DataList;
@@ -133,16 +132,12 @@ public function linkDelete(): HTTPResponse
             $this->jsonError(400);
         }
         $link = $this->linkFromRequest();
+        if (!$link->canDelete()) {
+            $this->jsonError(403);
+        }
         if ($link->hasExtension(Versioned::class)) {
-            $canArchive = Deprecation::withNoReplacement(fn() => $link->canArchive());
-            if (!$canArchive) {
-                $this->jsonError(403);
-            }
             $link->doArchive();
         } else {
-            if (!$link->canDelete()) {
-                $this->jsonError(403);
-            }
             $link->delete();
         }
         // Send response

diff --git a/tests/php/Controllers/LinkFieldControllerTest.php b/tests/php/Controllers/LinkFieldControllerTest.php
@@ -538,41 +538,32 @@ public function testLinkArchive(
         int $expectedCode
     ): void {
         TestPhoneLink::$fail = $fail;
-        if ($fail === 'can-delete') {
-            // Remove the Versioned extension because there's logic in LinkFieldController
-            // to use either canArchive() or canDelete() based on the presence of the Versioned extension
-            // Note that you must remove the extension on the base class rather than a TestOnly subclass
-            Link::remove_extension(Versioned::class);
+        $owner = $this->getFixtureLinkOwner();
+        $ownerID = $owner->ID;
+        $ownerClass = urlencode($owner->ClassName);
+        $ownerRelation = 'Link';
+        $ownerLinkID = $owner->LinkID;
+        $id = $this->getID($idType);
+        $fixtureID = $this->getFixtureLink()->ID;
+        if ($id === -1) {
+            $url = "/admin/linkfield/delete?ownerID=$ownerID&ownerClass=$ownerClass&ownerRelation=$ownerRelation";
+        } else {
+            $url = "/admin/linkfield/delete/$id?ownerID=$ownerID&ownerClass=$ownerClass&ownerRelation=$ownerRelation";
         }
-        try {
-            $owner = $this->getFixtureLinkOwner();
-            $ownerID = $owner->ID;
-            $ownerClass = urlencode($owner->ClassName);
-            $ownerRelation = 'Link';
-            $ownerLinkID = $owner->LinkID;
-            $id = $this->getID($idType);
-            $fixtureID = $this->getFixtureLink()->ID;
-            if ($id === -1) {
-                $url = "/admin/linkfield/delete?ownerID=$ownerID&ownerClass=$ownerClass&ownerRelation=$ownerRelation";
-            } else {
-                $url = "/admin/linkfield/delete/$id?ownerID=$ownerID&ownerClass=$ownerClass&ownerRelation=$ownerRelation";
-            }
-            $headers = [];
-            if ($fail !== 'csrf-token') {
-                $headers = array_merge($headers, $this->csrfTokenheader());
-            }
-            $response = $this->mainSession->sendRequest('DELETE', $url, [], $headers);
-            $this->assertSame('application/json', $response->getHeader('Content-type'));
-            $this->assertSame($expectedCode, $response->getStatusCode());
-            if ($expectedCode >= 400) {
-                $this->assertNotNull(TestPhoneLink::get()->byID($fixtureID));
-            } else {
-                $this->assertNull(TestPhoneLink::get()->byID($fixtureID));
-            }
-        } finally {
-            if ($fail === 'can-delete') {
-                Link::add_extension(Versioned::class);
-            }
+        $headers = [];
+        if ($fail !== 'csrf-token') {
+            $headers = array_merge($headers, $this->csrfTokenheader());
+        }
+        $response = $this->mainSession->sendRequest('DELETE', $url, [], $headers);
+        $this->assertSame('application/json', $response->getHeader('Content-type'));
+        $this->assertSame($expectedCode, $response->getStatusCode());
+        if ($expectedCode >= 400) {
+            $this->assertNotNull(TestPhoneLink::get()->byID($fixtureID));
+        } else {
+            $this->assertNull(TestPhoneLink::get()->byID($fixtureID));
+        }
+        if ($fail == 'can-delete') {
+            Link::add_extension(Versioned::class);
         }
     }
 
@@ -584,11 +575,6 @@ public function provideLinkArchive(): array
                 'fail' => '',
                 'expectedCode' => 204,
             ],
-            'Reject fail canArchive()' => [
-                'idType' => 'existing',
-                'fail' => 'can-archive',
-                'expectedCode' => 403,
-            ],
             'Reject fail canDelete()' => [
                 'idType' => 'existing',
                 'fail' => 'can-delete',

diff --git a/tests/php/Controllers/LinkFieldControllerTest/TestPhoneLink.php b/tests/php/Controllers/LinkFieldControllerTest/TestPhoneLink.php
@@ -43,11 +43,6 @@ public function canDelete($member = null)
         return TestPhoneLink::$fail !== 'can-delete';
     }
 
-    public function canArchive($member = null)
-    {
-        return TestPhoneLink::$fail !== 'can-archive';
-    }
-
     public function validate(): ValidationResult
     {
         $validationResult = parent::validate();