Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIX Archive record from live table on delete #214

Merged
merged 1 commit into from
Feb 13, 2024
Merged

FIX Archive record from live table on delete #214

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 6 additions & 9 deletions src/Controllers/LinkFieldController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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);
Expand Down
42 changes: 37 additions & 5 deletions tests/php/Controllers/LinkFieldControllerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -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();
Expand All @@ -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);
}

/**
Expand Down Expand Up @@ -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);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is unrelated refactoring to remove a pointless assertion

}

public function provideLinkDelete(): array
Expand Down Expand Up @@ -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
*/
Expand Down
5 changes: 5 additions & 0 deletions tests/php/Models/LinkTest/LinkOwner.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -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;
Expand Down
Loading