From c02549cf30833c94c87aa77bfc3aafcede0718f9 Mon Sep 17 00:00:00 2001 From: Guy Sartorelli Date: Wed, 4 May 2022 12:43:23 +1200 Subject: [PATCH 1/3] FIX Allow empty PreviewURLs for CMSPreviewable objects An empty PreviewURL will result in the "no preview available" message displaying instead of a 404 error. --- code/Controllers/SilverStripeNavigatorItem_ArchiveLink.php | 7 +++---- code/Controllers/SilverStripeNavigatorItem_LiveLink.php | 4 +++- code/Controllers/SilverStripeNavigatorItem_StageLink.php | 7 ++++++- code/Controllers/SilverStripeNavigatorItem_Unversioned.php | 3 ++- 4 files changed, 14 insertions(+), 7 deletions(-) diff --git a/code/Controllers/SilverStripeNavigatorItem_ArchiveLink.php b/code/Controllers/SilverStripeNavigatorItem_ArchiveLink.php index 00dab137..52627557 100644 --- a/code/Controllers/SilverStripeNavigatorItem_ArchiveLink.php +++ b/code/Controllers/SilverStripeNavigatorItem_ArchiveLink.php @@ -46,10 +46,8 @@ class SilverStripeNavigatorItem_ArchiveLink extends SilverStripeNavigatorItem public function getLink() { - return Controller::join_links( - $this->record->PreviewLink(), - '?archiveDate=' . urlencode($this->record->LastEdited ?? '') - ); + $link = $this->record->PreviewLink(); + return $link ? Controller::join_links($link, '?archiveDate=' . urlencode($this->record->LastEdited ?? '')) : ''; } public function canView($member = null) @@ -62,6 +60,7 @@ class SilverStripeNavigatorItem_ArchiveLink extends SilverStripeNavigatorItem && $this->isArchived() // Don't follow redirects in preview, they break the CMS editing form && !($record instanceof RedirectorPage) + && $this->getLink() ); } diff --git a/code/Controllers/SilverStripeNavigatorItem_LiveLink.php b/code/Controllers/SilverStripeNavigatorItem_LiveLink.php index 0dd68258..32d72029 100644 --- a/code/Controllers/SilverStripeNavigatorItem_LiveLink.php +++ b/code/Controllers/SilverStripeNavigatorItem_LiveLink.php @@ -48,7 +48,8 @@ class SilverStripeNavigatorItem_LiveLink extends SilverStripeNavigatorItem public function getLink() { - return Controller::join_links($this->getLivePage()->PreviewLink(), '?stage=Live'); + $link = $this->getLivePage()->PreviewLink(); + return $link ? Controller::join_links($link, '?stage=Live') : ''; } public function canView($member = null) @@ -60,6 +61,7 @@ class SilverStripeNavigatorItem_LiveLink extends SilverStripeNavigatorItem && $this->showLiveLink() && $record->hasStages() && $this->getLivePage() + && $this->getLink() ); } diff --git a/code/Controllers/SilverStripeNavigatorItem_StageLink.php b/code/Controllers/SilverStripeNavigatorItem_StageLink.php index 2c623c01..280f8cd9 100644 --- a/code/Controllers/SilverStripeNavigatorItem_StageLink.php +++ b/code/Controllers/SilverStripeNavigatorItem_StageLink.php @@ -50,8 +50,12 @@ class SilverStripeNavigatorItem_StageLink extends SilverStripeNavigatorItem public function getLink() { $date = Versioned::current_archived_date(); + $link = $this->record->PreviewLink(); + if (!$link) { + return ''; + } return Controller::join_links( - $this->record->PreviewLink(), + $link, '?stage=Stage', $date ? '?archiveDate=' . $date : null ); @@ -66,6 +70,7 @@ class SilverStripeNavigatorItem_StageLink extends SilverStripeNavigatorItem && $this->showStageLink() && $record->hasStages() && $this->getDraftPage() + && $this->getLink() ); } diff --git a/code/Controllers/SilverStripeNavigatorItem_Unversioned.php b/code/Controllers/SilverStripeNavigatorItem_Unversioned.php index ac489827..bc532efc 100644 --- a/code/Controllers/SilverStripeNavigatorItem_Unversioned.php +++ b/code/Controllers/SilverStripeNavigatorItem_Unversioned.php @@ -19,7 +19,7 @@ class SilverStripeNavigatorItem_Unversioned extends SilverStripeNavigatorItem public function getLink() { - return $this->getRecord()->PreviewLink(); + return $this->getRecord()->PreviewLink() ?? ''; } public function getTitle() @@ -42,6 +42,7 @@ class SilverStripeNavigatorItem_Unversioned extends SilverStripeNavigatorItem return ( !$this->getRecord()->hasExtension(Versioned::class) && $this->showUnversionedLink() + && $this->getLink() ); } From 3a990c95f5d14961d870d9ad006ae0d7d4962eff Mon Sep 17 00:00:00 2001 From: Guy Sartorelli Date: Fri, 6 May 2022 14:29:22 +1200 Subject: [PATCH 2/3] MNT Correct test SilverStripeNavigatorItems. --- .../Controllers/SilverStripeNavigatorTest.php | 232 +++++++++++++++--- .../UnstagedRecord.php | 12 +- .../UnversionedRecord.php | 36 +++ .../VersionedRecord.php | 42 ++++ ...rStripeNavigatorTest_ProtectedTestItem.php | 30 --- 5 files changed, 290 insertions(+), 62 deletions(-) create mode 100644 tests/php/Controllers/SilverStripeNavigatorTest/UnversionedRecord.php create mode 100644 tests/php/Controllers/SilverStripeNavigatorTest/VersionedRecord.php delete mode 100644 tests/php/Controllers/SilverStripeNavigatorTest_ProtectedTestItem.php diff --git a/tests/php/Controllers/SilverStripeNavigatorTest.php b/tests/php/Controllers/SilverStripeNavigatorTest.php index 18fd5832..6e59bde3 100644 --- a/tests/php/Controllers/SilverStripeNavigatorTest.php +++ b/tests/php/Controllers/SilverStripeNavigatorTest.php @@ -6,6 +6,7 @@ use SilverStripe\CMS\Controllers\SilverStripeNavigator; use SilverStripe\CMS\Controllers\SilverStripeNavigatorItem_ArchiveLink; use SilverStripe\CMS\Controllers\SilverStripeNavigatorItem_LiveLink; use SilverStripe\CMS\Controllers\SilverStripeNavigatorItem_StageLink; +use SilverStripe\CMS\Controllers\SilverStripeNavigatorItem_Unversioned; use SilverStripe\Dev\SapphireTest; use SilverStripe\Security\Member; @@ -15,53 +16,224 @@ class SilverStripeNavigatorTest extends SapphireTest protected static $extra_dataobjects = [ SilverStripeNavigatorTest\UnstagedRecord::class, + SilverStripeNavigatorTest\UnversionedRecord::class, + SilverStripeNavigatorTest\VersionedRecord::class, ]; - public function testGetItems() + public function testGetItemsAutoDiscovery(): void { - $page = $this->objFromFixture('Page', 'page1'); - $navigator = new SilverStripeNavigator($page); - - $items = $navigator->getItems(); - $classes = array_map('get_class', $items->toArray() ?? []); - $this->assertContains( - SilverStripeNavigatorItem_StageLink::class, - $classes, - 'Adds default classes' - ); + $record = new SilverStripeNavigatorTest\VersionedRecord(); + $record->PreviewLinkTestProperty = 'some-value'; + $record->write(); + $navigator = new SilverStripeNavigator($record); + $classes = array_map('get_class', $navigator->getItems()->toArray()); $this->assertContains( SilverStripeNavigatorTest_TestItem::class, $classes, 'Autodiscovers new classes' ); - - // Non-versioned items don't have stage / live } - public function testCanView() + public function testGetItemsPublished(): void { - $page = $this->objFromFixture('Page', 'page1'); - $admin = $this->objFromFixture(Member::class, 'admin'); - $navigator = new SilverStripeNavigator($page); + $record = new SilverStripeNavigatorTest\VersionedRecord(); + $record->PreviewLinkTestProperty = 'some-value'; + $record->write(); + $record->publishRecursive(); + $navigator = new SilverStripeNavigator($record); + $classes = array_map('get_class', $navigator->getItems()->toArray()); - // TODO Shouldn't be necessary but SapphireTest logs in as ADMIN by default - $this->logInWithPermission('CMS_ACCESS_CMSMain'); - $items = $navigator->getItems(); - $classes = array_map('get_class', $items->toArray() ?? []); - $this->assertNotContains(SilverStripeNavigatorTest_ProtectedTestItem::class, $classes); + // Has the live and staged links + $this->assertContains(SilverStripeNavigatorItem_LiveLink::class, $classes); + $this->assertContains(SilverStripeNavigatorItem_StageLink::class, $classes); - $this->logInWithPermission('ADMIN'); - $items = $navigator->getItems(); - $classes = array_map('get_class', $items->toArray() ?? []); - $this->assertContains(SilverStripeNavigatorTest_ProtectedTestItem::class, $classes); + // Does not have the other links + $this->assertNotContains(SilverStripeNavigatorItem_ArchiveLink::class, $classes); + $this->assertNotContains(SilverStripeNavigatorItem_Unversioned::class, $classes); + } - // Unversioned record shouldn't be viewable in stage / live specific views - $unversioned = new SilverStripeNavigatorTest\UnstagedRecord(); - $navigator2 = new SilverStripeNavigator($unversioned); - $classes = array_map('get_class', $navigator2->getItems()->toArray() ?? []); + public function testGetItemsStaged(): void + { + $record = new SilverStripeNavigatorTest\VersionedRecord(); + $record->PreviewLinkTestProperty = 'some-value'; + $record->write(); + $navigator = new SilverStripeNavigator($record); + $classes = array_map('get_class', $navigator->getItems()->toArray()); + + // Has the stage link + $this->assertContains(SilverStripeNavigatorItem_StageLink::class, $classes); + + // Does not have the other links + $this->assertNotContains(SilverStripeNavigatorItem_ArchiveLink::class, $classes); + $this->assertNotContains(SilverStripeNavigatorItem_LiveLink::class, $classes); + $this->assertNotContains(SilverStripeNavigatorItem_Unversioned::class, $classes); + } + + public function testGetItemsArchived(): void + { + $record = new SilverStripeNavigatorTest\VersionedRecord(); + $record->PreviewLinkTestProperty = 'some-value'; + $record->write(); + $record->doArchive(); + $navigator = new SilverStripeNavigator($record); + $classes = array_map('get_class', $navigator->getItems()->toArray()); + + // Has the archived link + $this->assertContains(SilverStripeNavigatorItem_ArchiveLink::class, $classes); + + // Does not have the other links $this->assertNotContains(SilverStripeNavigatorItem_LiveLink::class, $classes); $this->assertNotContains(SilverStripeNavigatorItem_StageLink::class, $classes); + $this->assertNotContains(SilverStripeNavigatorItem_UnversionedLink::class, $classes); + } + + public function testGetItemsUnstaged(): void + { + $record = new SilverStripeNavigatorTest\UnstagedRecord(); + $record->previewLinkTestProperty = 'some-value'; + $record->write(); + $navigator = new SilverStripeNavigator($record); + $classes = array_map('get_class', $navigator->getItems()->toArray()); + + // Has the unversioned link + $this->assertContains(SilverStripeNavigatorItem_Unversioned::class, $classes); + + // Does not have the other links $this->assertNotContains(SilverStripeNavigatorItem_ArchiveLink::class, $classes); + $this->assertNotContains(SilverStripeNavigatorItem_LiveLink::class, $classes); + $this->assertNotContains(SilverStripeNavigatorItem_StageLink::class, $classes); + } + + public function testGetItemsUnversioned(): void + { + $record = new SilverStripeNavigatorTest\UnversionedRecord(); + $record->previewLinkTestProperty = 'some-value'; + $record->write(); + $navigator = new SilverStripeNavigator($record); + $classes = array_map('get_class', $navigator->getItems()->toArray()); + + // Has the unversioned link + $this->assertContains(SilverStripeNavigatorItem_Unversioned::class, $classes); + + // Does not have the other links + $this->assertNotContains(SilverStripeNavigatorItem_ArchiveLink::class, $classes); + $this->assertNotContains(SilverStripeNavigatorItem_LiveLink::class, $classes); + $this->assertNotContains(SilverStripeNavigatorItem_StageLink::class, $classes); + } + + public function testCanViewPublished(): void + { + $record = new SilverStripeNavigatorTest\VersionedRecord(); + $record->write(); + $record->publishRecursive(); + $liveLinkItem = new SilverStripeNavigatorItem_LiveLink($record); + $stagedLinkItem = new SilverStripeNavigatorItem_StageLink($record); + $archivedLinkItem = new SilverStripeNavigatorItem_ArchiveLink($record); + $unversionedLinkItem = new SilverStripeNavigatorItem_Unversioned($record); + + // Cannot view staged and live links when there's no preview link + $this->assertFalse($liveLinkItem->canView()); + $this->assertFalse($stagedLinkItem->canView()); + + $record->PreviewLinkTestProperty = 'some-value'; + $record->write(); + $record->publishRecursive(); + + // Can view staged and live links + $this->assertTrue($liveLinkItem->canView()); + $this->assertTrue($stagedLinkItem->canView()); + // Cannot view the other links + $this->assertFalse($archivedLinkItem->canView()); + $this->assertFalse($unversionedLinkItem->canView()); + } + + public function testCanViewStaged(): void + { + $record = new SilverStripeNavigatorTest\VersionedRecord(); + $record->write(); + $liveLinkItem = new SilverStripeNavigatorItem_LiveLink($record); + $stagedLinkItem = new SilverStripeNavigatorItem_StageLink($record); + $archivedLinkItem = new SilverStripeNavigatorItem_ArchiveLink($record); + $unversionedLinkItem = new SilverStripeNavigatorItem_Unversioned($record); + + // Cannot view staged link when there's no preview link + $this->assertFalse($stagedLinkItem->canView()); + + $record->PreviewLinkTestProperty = 'some-value'; + + // Can view staged link + $this->assertTrue($stagedLinkItem->canView()); + // Cannot view the other links + $this->assertFalse($liveLinkItem->canView()); + $this->assertFalse($archivedLinkItem->canView()); + $this->assertFalse($unversionedLinkItem->canView()); + } + + public function testCanViewArchived(): void + { + $record = new SilverStripeNavigatorTest\VersionedRecord(); + $record->write(); + $record->doArchive(); + $liveLinkItem = new SilverStripeNavigatorItem_LiveLink($record); + $stagedLinkItem = new SilverStripeNavigatorItem_StageLink($record); + $archivedLinkItem = new SilverStripeNavigatorItem_ArchiveLink($record); + $unversionedLinkItem = new SilverStripeNavigatorItem_Unversioned($record); + + // Cannot view archived link when there's no preview link + $this->assertFalse($archivedLinkItem->canView()); + + $record->PreviewLinkTestProperty = 'some-value'; + + // Can view archived link + $this->assertTrue($archivedLinkItem->canView()); + // Cannot view the other links + $this->assertFalse($liveLinkItem->canView()); + $this->assertFalse($stagedLinkItem->canView()); + $this->assertFalse($unversionedLinkItem->canView()); + } + + public function testCanViewUnstaged(): void + { + $record = new SilverStripeNavigatorTest\UnstagedRecord(); + $record->write(); + $liveLinkItem = new SilverStripeNavigatorItem_LiveLink($record); + $stagedLinkItem = new SilverStripeNavigatorItem_StageLink($record); + $archivedLinkItem = new SilverStripeNavigatorItem_ArchiveLink($record); + $unversionedLinkItem = new SilverStripeNavigatorItem_Unversioned($record); + + // Cannot view unversioned link when there's no preview link + $this->assertFalse($unversionedLinkItem->canView()); + + $record->previewLinkTestProperty = 'some-value'; + + // Can view unversioned link + $this->assertTrue($unversionedLinkItem->canView()); + // Cannot view the other links + $this->assertFalse($liveLinkItem->canView()); + $this->assertFalse($stagedLinkItem->canView()); + $this->assertFalse($archivedLinkItem->canView()); + } + + public function testCanViewUnversioned(): void + { + $record = new SilverStripeNavigatorTest\UnversionedRecord(); + $record->write(); + $liveLinkItem = new SilverStripeNavigatorItem_LiveLink($record); + $stagedLinkItem = new SilverStripeNavigatorItem_StageLink($record); + $archivedLinkItem = new SilverStripeNavigatorItem_ArchiveLink($record); + $unversionedLinkItem = new SilverStripeNavigatorItem_Unversioned($record); + + // Cannot view unversioned link when there's no preview link + $this->assertFalse($unversionedLinkItem->canView()); + + $record->previewLinkTestProperty = 'some-value'; + + // Can view unversioned link + $this->assertTrue($unversionedLinkItem->canView()); + // Cannot view the other links + $this->assertFalse($liveLinkItem->canView()); + $this->assertFalse($stagedLinkItem->canView()); + $this->assertFalse($archivedLinkItem->canView()); } } diff --git a/tests/php/Controllers/SilverStripeNavigatorTest/UnstagedRecord.php b/tests/php/Controllers/SilverStripeNavigatorTest/UnstagedRecord.php index 6af3e221..c85889b1 100644 --- a/tests/php/Controllers/SilverStripeNavigatorTest/UnstagedRecord.php +++ b/tests/php/Controllers/SilverStripeNavigatorTest/UnstagedRecord.php @@ -12,15 +12,23 @@ use SilverStripe\Versioned\Versioned; */ class UnstagedRecord extends DataObject implements TestOnly, CMSPreviewable { - private static $table_name = 'SilverStripeNavigatorTest_UnversionedRecord'; + private static $table_name = 'SilverStripeNavigatorTest_UnstagedRecord'; + + private static $show_stage_link = true; + + private static $show_live_link = true; + + private static $show_unversioned_preview_link = true; private static $extensions = [ Versioned::class . '.versioned', ]; + public $previewLinkTestProperty = null; + public function PreviewLink($action = null) { - return null; + return $this->previewLinkTestProperty; } /** diff --git a/tests/php/Controllers/SilverStripeNavigatorTest/UnversionedRecord.php b/tests/php/Controllers/SilverStripeNavigatorTest/UnversionedRecord.php new file mode 100644 index 00000000..9198b89c --- /dev/null +++ b/tests/php/Controllers/SilverStripeNavigatorTest/UnversionedRecord.php @@ -0,0 +1,36 @@ +previewLinkTestProperty; + } + + public function getMimeType() + { + return 'text/html'; + } + + public function CMSEditLink() + { + return null; + } +} diff --git a/tests/php/Controllers/SilverStripeNavigatorTest/VersionedRecord.php b/tests/php/Controllers/SilverStripeNavigatorTest/VersionedRecord.php new file mode 100644 index 00000000..18dbc6ba --- /dev/null +++ b/tests/php/Controllers/SilverStripeNavigatorTest/VersionedRecord.php @@ -0,0 +1,42 @@ + 'Text', + ]; + + private static $extensions = [ + Versioned::class, + ]; + + public function PreviewLink($action = null) + { + return $this->PreviewLinkTestProperty; + } + + public function getMimeType() + { + return 'text/html'; + } + + public function CMSEditLink() + { + return null; + } +} diff --git a/tests/php/Controllers/SilverStripeNavigatorTest_ProtectedTestItem.php b/tests/php/Controllers/SilverStripeNavigatorTest_ProtectedTestItem.php deleted file mode 100644 index 0343a101..00000000 --- a/tests/php/Controllers/SilverStripeNavigatorTest_ProtectedTestItem.php +++ /dev/null @@ -1,30 +0,0 @@ - Date: Fri, 6 May 2022 14:30:01 +1200 Subject: [PATCH 3/3] FIX Ensure unstaged versioned objects can be previewed. --- .../SilverStripeNavigatorItem_Unversioned.php | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/code/Controllers/SilverStripeNavigatorItem_Unversioned.php b/code/Controllers/SilverStripeNavigatorItem_Unversioned.php index bc532efc..2ee5f4e3 100644 --- a/code/Controllers/SilverStripeNavigatorItem_Unversioned.php +++ b/code/Controllers/SilverStripeNavigatorItem_Unversioned.php @@ -40,12 +40,24 @@ class SilverStripeNavigatorItem_Unversioned extends SilverStripeNavigatorItem public function canView($member = null) { return ( - !$this->getRecord()->hasExtension(Versioned::class) + $this->recordIsUnversioned() && $this->showUnversionedLink() && $this->getLink() ); } + private function recordIsUnversioned(): bool + { + $record = $this->getRecord(); + // If the record has the Versioned extension, it can be considered unversioned + // for the purposes of this class if it has no stages and is not archived. + if ($record->hasExtension(Versioned::class)) { + return (!$record->hasStages()) && !$this->isArchived(); + } + // Completely unversioned. + return true; + } + /** * True if the record is configured to display this item. *