From 3be0478e1c40cd2b9f577818596c4222b365b6b6 Mon Sep 17 00:00:00 2001
From: Damian Mooyman <damian@silverstripe.com>
Date: Tue, 20 Feb 2018 11:03:23 +1300
Subject: [PATCH] BUG Fix behaviour towards versioned but unstagable records

---
 .../Controllers/SilverStripeNavigatorItem.php | 21 +++++-----
 .../SilverStripeNavigatorItem_ArchiveLink.php |  9 +++--
 .../SilverStripeNavigatorItem_LiveLink.php    |  7 +++-
 .../SilverStripeNavigatorItem_StageLink.php   |  8 +++-
 .../Controllers/SilverStripeNavigatorTest.php | 16 ++++++++
 .../UnstagedRecord.php                        | 40 +++++++++++++++++++
 6 files changed, 84 insertions(+), 17 deletions(-)
 create mode 100644 tests/php/Controllers/SilverStripeNavigatorTest/UnstagedRecord.php

diff --git a/code/Controllers/SilverStripeNavigatorItem.php b/code/Controllers/SilverStripeNavigatorItem.php
index 3077839f..04e619be 100644
--- a/code/Controllers/SilverStripeNavigatorItem.php
+++ b/code/Controllers/SilverStripeNavigatorItem.php
@@ -121,22 +121,23 @@ abstract class SilverStripeNavigatorItem extends ViewableData
      */
     public function isArchived()
     {
-        $recordClass = get_class($this->record);
-        if (!$recordClass::has_extension(Versioned::class)) {
+        /** @var Versioned|DataObject $record */
+        $record = $this->record;
+        if (!$record->hasExtension(Versioned::class) || !$record->hasStages()) {
             return false;
         }
 
-        if (!isset($this->record->_cached_isArchived)) {
-            $baseClass = $this->record->baseClass();
-            $currentDraft = Versioned::get_by_stage($baseClass, Versioned::DRAFT)->byID($this->record->ID);
-            $currentLive = Versioned::get_by_stage($baseClass, Versioned::LIVE)->byID($this->record->ID);
+        if (!isset($record->_cached_isArchived)) {
+            $baseClass = $record->baseClass();
+            $currentDraft = Versioned::get_by_stage($baseClass, Versioned::DRAFT)->byID($record->ID);
+            $currentLive = Versioned::get_by_stage($baseClass, Versioned::LIVE)->byID($record->ID);
 
-            $this->record->_cached_isArchived = (
-                (!$currentDraft || ($currentDraft && $this->record->Version != $currentDraft->Version))
-                && (!$currentLive || ($currentLive && $this->record->Version != $currentLive->Version))
+            $record->_cached_isArchived = (
+                (!$currentDraft || ($currentDraft && $record->Version != $currentDraft->Version))
+                && (!$currentLive || ($currentLive && $record->Version != $currentLive->Version))
             );
         }
 
-        return $this->record->_cached_isArchived;
+        return $record->_cached_isArchived;
     }
 }
diff --git a/code/Controllers/SilverStripeNavigatorItem_ArchiveLink.php b/code/Controllers/SilverStripeNavigatorItem_ArchiveLink.php
index 277dd7ca..b573b183 100644
--- a/code/Controllers/SilverStripeNavigatorItem_ArchiveLink.php
+++ b/code/Controllers/SilverStripeNavigatorItem_ArchiveLink.php
@@ -4,6 +4,7 @@ namespace SilverStripe\CMS\Controllers;
 use SilverStripe\CMS\Model\RedirectorPage;
 use SilverStripe\Control\Controller;
 use SilverStripe\Core\Convert;
+use SilverStripe\ORM\DataObject;
 use SilverStripe\ORM\FieldType\DBDatetime;
 use SilverStripe\ORM\FieldType\DBField;
 use SilverStripe\Versioned\Versioned;
@@ -53,12 +54,14 @@ class SilverStripeNavigatorItem_ArchiveLink extends SilverStripeNavigatorItem
 
     public function canView($member = null)
     {
-        $recordClass = get_class($this->record);
+        /** @var Versioned|DataObject $record */
+        $record = $this->record;
         return (
-            $recordClass::has_extension(Versioned::class)
+            $record->hasExtension(Versioned::class)
+            && $record->hasStages()
             && $this->isArchived()
             // Don't follow redirects in preview, they break the CMS editing form
-            && !($this->record instanceof RedirectorPage)
+            && !($record instanceof RedirectorPage)
         );
     }
 
diff --git a/code/Controllers/SilverStripeNavigatorItem_LiveLink.php b/code/Controllers/SilverStripeNavigatorItem_LiveLink.php
index 38f31283..86b46e98 100644
--- a/code/Controllers/SilverStripeNavigatorItem_LiveLink.php
+++ b/code/Controllers/SilverStripeNavigatorItem_LiveLink.php
@@ -4,6 +4,7 @@ namespace SilverStripe\CMS\Controllers;
 use SilverStripe\CMS\Model\RedirectorPage;
 use SilverStripe\Control\Controller;
 use SilverStripe\Core\Convert;
+use SilverStripe\ORM\DataObject;
 use SilverStripe\Versioned\Versioned;
 
 class SilverStripeNavigatorItem_LiveLink extends SilverStripeNavigatorItem
@@ -51,9 +52,11 @@ class SilverStripeNavigatorItem_LiveLink extends SilverStripeNavigatorItem
 
     public function canView($member = null)
     {
-        $recordClass = get_class($this->record);
+        /** @var Versioned|DataObject $record */
+        $record = $this->record;
         return (
-            $recordClass::has_extension(Versioned::class)
+            $record->hasExtension(Versioned::class)
+            && $record->hasStages()
             && $this->getLivePage()
             // Don't follow redirects in preview, they break the CMS editing form
             && !($this->record instanceof RedirectorPage)
diff --git a/code/Controllers/SilverStripeNavigatorItem_StageLink.php b/code/Controllers/SilverStripeNavigatorItem_StageLink.php
index bdb7181e..83aacefd 100644
--- a/code/Controllers/SilverStripeNavigatorItem_StageLink.php
+++ b/code/Controllers/SilverStripeNavigatorItem_StageLink.php
@@ -5,6 +5,7 @@ use SilverStripe\CMS\Model\RedirectorPage;
 use SilverStripe\Control\Controller;
 use SilverStripe\Core\ClassInfo;
 use SilverStripe\Core\Convert;
+use SilverStripe\ORM\DataObject;
 use SilverStripe\Versioned\Versioned;
 use SiteTreeFutureState;
 
@@ -57,11 +58,14 @@ class SilverStripeNavigatorItem_StageLink extends SilverStripeNavigatorItem
 
     public function canView($member = null)
     {
+        /** @var Versioned|DataObject $record */
+        $record = $this->record;
         return (
-            $this->record->hasExtension(Versioned::class)
+            $record->hasExtension(Versioned::class)
+            && $record->hasStages()
             && $this->getDraftPage()
             // Don't follow redirects in preview, they break the CMS editing form
-            && !($this->record instanceof RedirectorPage)
+            && !($record instanceof RedirectorPage)
         );
     }
 
diff --git a/tests/php/Controllers/SilverStripeNavigatorTest.php b/tests/php/Controllers/SilverStripeNavigatorTest.php
index 92400b97..bfb3f7ee 100644
--- a/tests/php/Controllers/SilverStripeNavigatorTest.php
+++ b/tests/php/Controllers/SilverStripeNavigatorTest.php
@@ -3,6 +3,8 @@
 namespace SilverStripe\CMS\Tests\Controllers;
 
 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\Dev\SapphireTest;
 use SilverStripe\Security\Member;
@@ -11,6 +13,10 @@ class SilverStripeNavigatorTest extends SapphireTest
 {
     protected static $fixture_file = 'CMSMainTest.yml';
 
+    protected static $extra_dataobjects = [
+        SilverStripeNavigatorTest\UnstagedRecord::class,
+    ];
+
     public function testGetItems()
     {
         $page = $this->objFromFixture('Page', 'page1');
@@ -29,6 +35,8 @@ class SilverStripeNavigatorTest extends SapphireTest
             $classes,
             'Autodiscovers new classes'
         );
+
+        // Non-versioned items don't have stage / live
     }
 
     public function testCanView()
@@ -47,5 +55,13 @@ class SilverStripeNavigatorTest extends SapphireTest
         $items = $navigator->getItems();
         $classes = array_map('get_class', $items->toArray());
         $this->assertContains(SilverStripeNavigatorTest_ProtectedTestItem::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());
+        $this->assertNotContains(SilverStripeNavigatorItem_LiveLink::class, $classes);
+        $this->assertNotContains(SilverStripeNavigatorItem_StageLink::class, $classes);
+        $this->assertNotContains(SilverStripeNavigatorItem_ArchiveLink::class, $classes);
     }
 }
diff --git a/tests/php/Controllers/SilverStripeNavigatorTest/UnstagedRecord.php b/tests/php/Controllers/SilverStripeNavigatorTest/UnstagedRecord.php
new file mode 100644
index 00000000..6af3e221
--- /dev/null
+++ b/tests/php/Controllers/SilverStripeNavigatorTest/UnstagedRecord.php
@@ -0,0 +1,40 @@
+<?php
+
+namespace SilverStripe\CMS\Tests\Controllers\SilverStripeNavigatorTest;
+
+use SilverStripe\Dev\TestOnly;
+use SilverStripe\ORM\CMSPreviewable;
+use SilverStripe\ORM\DataObject;
+use SilverStripe\Versioned\Versioned;
+
+/**
+ * Versioned but not staged
+ */
+class UnstagedRecord extends DataObject implements TestOnly, CMSPreviewable
+{
+    private static $table_name = 'SilverStripeNavigatorTest_UnversionedRecord';
+
+    private static $extensions = [
+        Versioned::class . '.versioned',
+    ];
+
+    public function PreviewLink($action = null)
+    {
+        return null;
+    }
+
+    /**
+     * To determine preview mechanism (e.g. embedded / iframe)
+     *
+     * @return string
+     */
+    public function getMimeType()
+    {
+        return 'text/html';
+    }
+
+    public function CMSEditLink()
+    {
+        return null;
+    }
+}