From 72206dc2cc95578dcb08b0d7a517096e1235c1d0 Mon Sep 17 00:00:00 2001 From: Guy Sartorelli <36352093+GuySartorelli@users.noreply.github.com> Date: Fri, 16 Aug 2024 11:31:03 +1200 Subject: [PATCH] FIX Use canDelete, not the now-deleted canArchive (#318) --- src/Controllers/LinkFieldController.php | 11 +--- .../Controllers/LinkFieldControllerTest.php | 64 ++++++++----------- .../LinkFieldControllerTest/TestPhoneLink.php | 5 -- 3 files changed, 28 insertions(+), 52 deletions(-) diff --git a/src/Controllers/LinkFieldController.php b/src/Controllers/LinkFieldController.php index 840d5c25..6e1c1f73 100644 --- 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 index 99423736..188b4746 100644 --- 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 index b1f39bfa..c740e1e1 100644 --- 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();