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 1/3] 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;
+    }
+}

From 83c4ab8d180954b3d80d16ed5f5764e3c647ca6d Mon Sep 17 00:00:00 2001
From: Damian Mooyman <damian@silverstripe.com>
Date: Wed, 21 Feb 2018 10:32:00 +1300
Subject: [PATCH 2/3] BUG Fix test regressions in CMS page filters

---
 tests/php/Controllers/CMSSiteTreeFilterTest.php | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/tests/php/Controllers/CMSSiteTreeFilterTest.php b/tests/php/Controllers/CMSSiteTreeFilterTest.php
index e21f5dd8..2093de55 100644
--- a/tests/php/Controllers/CMSSiteTreeFilterTest.php
+++ b/tests/php/Controllers/CMSSiteTreeFilterTest.php
@@ -2,6 +2,7 @@
 
 namespace SilverStripe\CMS\Tests\Controllers;
 
+use Page;
 use SilverStripe\CMS\Model\SiteTree;
 use SilverStripe\Versioned\Versioned;
 use SilverStripe\CMS\Controllers\CMSSiteTreeFilter_Search;
@@ -65,14 +66,17 @@ class CMSSiteTreeFilterTest extends SapphireTest
 
     public function testChangedPagesFilter()
     {
+        /** @var Page $unchangedPage */
         $unchangedPage = $this->objFromFixture('Page', 'page1');
         $unchangedPage->publishRecursive();
 
+        /** @var Page $changedPage */
         $changedPage = $this->objFromFixture('Page', 'page2');
         $changedPage->Title = 'Original';
         $changedPage->copyVersionToStage(Versioned::DRAFT, Versioned::LIVE);
         $changedPage->Title = 'Changed';
         $changedPage->write();
+        $changedPageVersion = $changedPage->Version;
 
         // Check that only changed pages are returned
         $f = new CMSSiteTreeFilter_ChangedPages(array('Term' => 'Changed'));
@@ -93,8 +97,9 @@ class CMSSiteTreeFilterTest extends SapphireTest
 
         // If we roll back to an earlier version than what's on the published site, we should still show the changed
         $changedPage->Title = 'Changed 2';
+        $changedPage->write();
         $changedPage->copyVersionToStage(Versioned::DRAFT, Versioned::LIVE);
-        $changedPage->doRollbackTo(1);
+        $changedPage->doRollbackTo($changedPageVersion);
 
         $f = new CMSSiteTreeFilter_ChangedPages(array('Term' => 'Changed'));
         $results = $f->pagesIncluded();

From 289d6a87a2215ec3a5b7cdceeb50281babea055f Mon Sep 17 00:00:00 2001
From: Damian Mooyman <damian@silverstripe.com>
Date: Wed, 21 Feb 2018 10:32:13 +1300
Subject: [PATCH 3/3] MINOR Simplify i18n keys

---
 code/Controllers/CMSSiteTreeFilter_ChangedPages.php            | 2 +-
 code/Controllers/CMSSiteTreeFilter_DeletedPages.php            | 2 +-
 code/Controllers/CMSSiteTreeFilter_PublishedPages.php          | 2 +-
 code/Controllers/CMSSiteTreeFilter_Search.php                  | 3 ++-
 code/Controllers/CMSSiteTreeFilter_StatusDeletedPages.php      | 2 +-
 code/Controllers/CMSSiteTreeFilter_StatusDraftPages.php        | 2 +-
 .../CMSSiteTreeFilter_StatusRemovedFromDraftPages.php          | 2 +-
 7 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/code/Controllers/CMSSiteTreeFilter_ChangedPages.php b/code/Controllers/CMSSiteTreeFilter_ChangedPages.php
index c37d506d..7fe09ac7 100644
--- a/code/Controllers/CMSSiteTreeFilter_ChangedPages.php
+++ b/code/Controllers/CMSSiteTreeFilter_ChangedPages.php
@@ -13,7 +13,7 @@ class CMSSiteTreeFilter_ChangedPages extends CMSSiteTreeFilter
 
     public static function title()
     {
-        return _t('SilverStripe\\CMS\\Controllers\\CMSSiteTreeFilter_ChangedPages.Title', "Modified pages");
+        return _t(__CLASS__ . '.Title', "Modified pages");
     }
 
     public function getFilteredPages()
diff --git a/code/Controllers/CMSSiteTreeFilter_DeletedPages.php b/code/Controllers/CMSSiteTreeFilter_DeletedPages.php
index 6fb94e94..926f2b74 100644
--- a/code/Controllers/CMSSiteTreeFilter_DeletedPages.php
+++ b/code/Controllers/CMSSiteTreeFilter_DeletedPages.php
@@ -25,7 +25,7 @@ class CMSSiteTreeFilter_DeletedPages extends CMSSiteTreeFilter
 
     public static function title()
     {
-        return _t('SilverStripe\\CMS\\Controllers\\CMSSiteTreeFilter_DeletedPages.Title', "All pages, including archived");
+        return _t(__CLASS__ . '.Title', "All pages, including archived");
     }
 
     public function getFilteredPages()
diff --git a/code/Controllers/CMSSiteTreeFilter_PublishedPages.php b/code/Controllers/CMSSiteTreeFilter_PublishedPages.php
index 2efea77b..2e23f8ce 100644
--- a/code/Controllers/CMSSiteTreeFilter_PublishedPages.php
+++ b/code/Controllers/CMSSiteTreeFilter_PublishedPages.php
@@ -20,7 +20,7 @@ class CMSSiteTreeFilter_PublishedPages extends CMSSiteTreeFilter
      */
     public static function title()
     {
-        return _t('SilverStripe\\CMS\\Controllers\\CMSSIteTreeFilter_PublishedPages.Title', "Published pages");
+        return _t(__CLASS__ . '.Title', "Published pages");
     }
 
     /**
diff --git a/code/Controllers/CMSSiteTreeFilter_Search.php b/code/Controllers/CMSSiteTreeFilter_Search.php
index f36ee187..d7c9d666 100644
--- a/code/Controllers/CMSSiteTreeFilter_Search.php
+++ b/code/Controllers/CMSSiteTreeFilter_Search.php
@@ -1,4 +1,5 @@
 <?php
+
 namespace SilverStripe\CMS\Controllers;
 
 use SilverStripe\CMS\Model\SiteTree;
@@ -10,7 +11,7 @@ class CMSSiteTreeFilter_Search extends CMSSiteTreeFilter
 
     public static function title()
     {
-        return _t('SilverStripe\\CMS\\Controllers\\CMSSiteTreeFilter_Search.Title', "All pages");
+        return _t(__CLASS__ . '.Title', "All pages");
     }
 
     /**
diff --git a/code/Controllers/CMSSiteTreeFilter_StatusDeletedPages.php b/code/Controllers/CMSSiteTreeFilter_StatusDeletedPages.php
index f83732df..039648df 100644
--- a/code/Controllers/CMSSiteTreeFilter_StatusDeletedPages.php
+++ b/code/Controllers/CMSSiteTreeFilter_StatusDeletedPages.php
@@ -24,7 +24,7 @@ class CMSSiteTreeFilter_StatusDeletedPages extends CMSSiteTreeFilter
 
     public static function title()
     {
-        return _t('SilverStripe\\CMS\\Controllers\\CMSSiteTreeFilter_StatusDeletedPages.Title', 'Archived pages');
+        return _t(__CLASS__ . '.Title', 'Archived pages');
     }
 
     /**
diff --git a/code/Controllers/CMSSiteTreeFilter_StatusDraftPages.php b/code/Controllers/CMSSiteTreeFilter_StatusDraftPages.php
index 0279f380..e15a0bca 100644
--- a/code/Controllers/CMSSiteTreeFilter_StatusDraftPages.php
+++ b/code/Controllers/CMSSiteTreeFilter_StatusDraftPages.php
@@ -14,7 +14,7 @@ class CMSSiteTreeFilter_StatusDraftPages extends CMSSiteTreeFilter
 
     public static function title()
     {
-        return _t('SilverStripe\\CMS\\Controllers\\CMSSiteTreeFilter_StatusDraftPages.Title', 'Draft pages');
+        return _t(__CLASS__ . '.Title', 'Draft pages');
     }
 
     /**
diff --git a/code/Controllers/CMSSiteTreeFilter_StatusRemovedFromDraftPages.php b/code/Controllers/CMSSiteTreeFilter_StatusRemovedFromDraftPages.php
index 41afb042..d098f7ef 100644
--- a/code/Controllers/CMSSiteTreeFilter_StatusRemovedFromDraftPages.php
+++ b/code/Controllers/CMSSiteTreeFilter_StatusRemovedFromDraftPages.php
@@ -14,7 +14,7 @@ class CMSSiteTreeFilter_StatusRemovedFromDraftPages extends CMSSiteTreeFilter
 
     public static function title()
     {
-        return _t('SilverStripe\\CMS\\Controllers\\CMSSiteTreeFilter_StatusRemovedFromDraftPages.Title', 'Live but removed from draft');
+        return _t(__CLASS__ . '.Title', 'Live but removed from draft');
     }
 
     /**