Merge pull request #7878 from open-sausages/pulls/4.0/fix-unstaged-versioned

BUG Fix behaviour towards versioned but unstagable records
2024-10-22 14:05:37 +02:00 · 2018-02-20 11:44:05 +00:00 · 2018-02-20 11:44:05 +00:00 · 2074f8fc69
commit 2074f8fc69
parent 4e5a664183 0e26c06644
9 changed files with 250 additions and 34 deletions
--- a/src/Forms/GridField/GridFieldVersionedState.php
+++ b/src/Forms/GridField/GridFieldVersionedState.php
@ -5,6 +5,7 @@ namespace SilverStripe\Forms\GridField;
 use SilverStripe\ORM\DataObject;
 use SilverStripe\Versioned\Versioned;
 use SilverStripe\Core\Convert;
+use SilverStripe\View\HTML;

 /**
 * @todo Move to siverstripe/versioned module
@ -46,8 +47,9 @@ class GridFieldVersionedState implements GridField_ColumnProvider
            return;
        }

-        $model = $gridField->getModelClass();
-        $isModelVersioned = $model::has_extension(Versioned::class);
+        /** @var Versioned|DataObject $model */
+        $model = DataObject::singleton($gridField->getModelClass());
+        $isModelVersioned = $model->hasExtension(Versioned::class) && $model->hasStages();
        if (!$isModelVersioned) {
            return;
        }
@ -92,10 +94,12 @@ class GridFieldVersionedState implements GridField_ColumnProvider
            if (is_string($data)) {
                $data = array('text' => $data);
            }
-            $flagContent .= sprintf(
-                " <span class=\"ss-gridfield-badge badge %s\"%s>%s</span>",
-                'status-' . Convert::raw2xml($class),
-                (isset($data['title'])) ? sprintf(' title="%s"', Convert::raw2xml($data['title'])) : '',
+            $flagContent .= HTML::createTag(
+                'span',
+                array_merge(
+                    [ 'class' => 'ss-gridfield-badge badge status-' . $class ],
+                    array_intersect_key($data, ['title' => true])
+                ),
                Convert::raw2xml($data['text'])
            );
        }
@ -156,7 +160,7 @@ class GridFieldVersionedState implements GridField_ColumnProvider
     */
    protected function getStatusFlags($record)
    {
-        if (!$record->hasExtension(Versioned::class)) {
+        if (!$record->hasExtension(Versioned::class) || !$record->hasStages()) {
            return [];
        }

--- a/src/ORM/DatabaseAdmin.php
+++ b/src/ORM/DatabaseAdmin.php
@ -337,8 +337,9 @@ class DatabaseAdmin extends Controller
                    $updateQueries = [sprintf($updateQuery, '')];

                    // Remap versioned table ClassName values as well
+                    /** @var Versioned|DataObject $class */
                    $class = DataObject::singleton($newClassName);
-                    if ($class->has_extension(Versioned::class)) {
+                    if ($class->hasExtension(Versioned::class)) {
                        if ($class->hasStages()) {
                            $updateQueries[] = sprintf($updateQuery, '_Live');
                        }
--- a/src/ORM/Hierarchy/Hierarchy.php
+++ b/src/ORM/Hierarchy/Hierarchy.php
@ -208,12 +208,14 @@ class Hierarchy extends DataExtension
     */
    public function AllChildrenIncludingDeleted()
    {
-        $stageChildren = $this->owner->stageChildren(true);
+        /** @var DataObject|Hierarchy|Versioned $owner */
+        $owner = $this->owner;
+        $stageChildren = $owner->stageChildren(true);

        // Add live site content that doesn't exist on the stage site, if required.
-        if ($this->owner->hasExtension(Versioned::class)) {
+        if ($owner->hasExtension(Versioned::class) && $owner->hasStages()) {
            // Next, go through the live children.  Only some of these will be listed
-            $liveChildren = $this->owner->liveChildren(true, true);
+            $liveChildren = $owner->liveChildren(true, true);
            if ($liveChildren) {
                $merged = new ArrayList();
                $merged->merge($stageChildren);
@ -221,7 +223,7 @@ class Hierarchy extends DataExtension
                $stageChildren = $merged;
            }
        }
-        $this->owner->extend("augmentAllChildrenIncludingDeleted", $stageChildren);
+        $owner->extend("augmentAllChildrenIncludingDeleted", $stageChildren);
        return $stageChildren;
    }

@ -233,15 +235,19 @@ class Hierarchy extends DataExtension
     */
    public function AllHistoricalChildren()
    {
-        if (!$this->owner->hasExtension(Versioned::class)) {
-            throw new Exception('Hierarchy->AllHistoricalChildren() only works with Versioned extension applied');
+        /** @var DataObject|Versioned|Hierarchy $owner */
+        $owner = $this->owner;
+        if (!$owner->hasExtension(Versioned::class) || !$owner->hasStages()) {
+            throw new Exception(
+                'Hierarchy->AllHistoricalChildren() only works with Versioned extension applied with staging'
+            );
        }

-        $baseTable = $this->owner->baseTable();
-        $parentIDColumn = $this->owner->getSchema()->sqlColumnForField($this->owner, 'ParentID');
+        $baseTable = $owner->baseTable();
+        $parentIDColumn = $owner->getSchema()->sqlColumnForField($owner, 'ParentID');
        return Versioned::get_including_deleted(
-            $this->owner->baseClass(),
-            [ $parentIDColumn => $this->owner->ID ],
+            $owner->baseClass(),
+            [ $parentIDColumn => $owner->ID ],
            "\"{$baseTable}\".\"ID\" ASC"
        );
    }
@ -337,15 +343,17 @@ class Hierarchy extends DataExtension
     */
    public function liveChildren($showAll = false, $onlyDeletedFromStage = false)
    {
-        if (!$this->owner->hasExtension(Versioned::class)) {
-            throw new Exception('Hierarchy->liveChildren() only works with Versioned extension applied');
+        /** @var Versioned|DataObject|Hierarchy $owner */
+        $owner = $this->owner;
+        if (!$owner->hasExtension(Versioned::class) || !$owner->hasStages()) {
+            throw new Exception('Hierarchy->liveChildren() only works with Versioned extension applied with staging');
        }

-        $hideFromHierarchy = $this->owner->config()->hide_from_hierarchy;
-        $hideFromCMSTree = $this->owner->config()->hide_from_cms_tree;
-        $children = DataObject::get($this->owner->baseClass())
-            ->filter('ParentID', (int)$this->owner->ID)
-            ->exclude('ID', (int)$this->owner->ID)
+        $hideFromHierarchy = $owner->config()->hide_from_hierarchy;
+        $hideFromCMSTree = $owner->config()->hide_from_cms_tree;
+        $children = DataObject::get($owner->baseClass())
+            ->filter('ParentID', (int)$owner->ID)
+            ->exclude('ID', (int)$owner->ID)
            ->setDataQueryParam(array(
                'Versioned.mode' => $onlyDeletedFromStage ? 'stage_unique' : 'stage',
                'Versioned.stage' => 'Live'
@ -356,7 +364,7 @@ class Hierarchy extends DataExtension
        if ($hideFromCMSTree && $this->showingCMSTree()) {
            $children = $children->exclude('ClassName', $hideFromCMSTree);
        }
-        if (!$showAll && DataObject::getSchema()->fieldSpec($this->owner, 'ShowInMenus')) {
+        if (!$showAll && DataObject::getSchema()->fieldSpec($owner, 'ShowInMenus')) {
            $children = $children->filter('ShowInMenus', 1);
        }

--- a/src/Security/InheritedPermissions.php
+++ b/src/Security/InheritedPermissions.php
@ -577,8 +577,9 @@ class InheritedPermissions implements PermissionChecker
        if (!class_exists(Versioned::class)) {
            return false;
        }
+        /** @var Versioned|DataObject $singleton */
        $singleton = DataObject::singleton($this->getBaseClass());
-        return $singleton->hasExtension(Versioned::class);
+        return $singleton->hasExtension(Versioned::class) && $singleton->hasStages();
    }

    public function clearCache()
--- a/tests/php/Forms/FormFactoryTest/ControllerExtension.php
+++ b/tests/php/Forms/FormFactoryTest/ControllerExtension.php
@ -8,6 +8,7 @@ use SilverStripe\Core\Extension;
 use SilverStripe\Forms\FieldList;
 use SilverStripe\Forms\FormAction;
 use SilverStripe\Forms\LiteralField;
+use SilverStripe\ORM\DataObject;
 use SilverStripe\Versioned\Versioned;

 /**
@ -40,8 +41,9 @@ class ControllerExtension extends Extension
        if (empty($context['Record'])) {
            return;
        }
+        /** @var Versioned|DataObject $record */
        $record = $context['Record'];
-        if ($record->hasExtension(Versioned::class)) {
+        if ($record->hasExtension(Versioned::class) && $record->hasStages()) {
            $actions->push(new FormAction('publish', 'Publish'));
        }
    }
@ -60,8 +62,9 @@ class ControllerExtension extends Extension
        if (empty($context['Record'])) {
            return;
        }
+        /** @var Versioned|DataObject $record */
        $record = $context['Record'];
-        if ($record->hasExtension(Versioned::class)) {
+        if ($record->hasExtension(Versioned::class) && $record->hasStages()) {
            $link = $controller->Link('preview');
            $fields->unshift(
                new LiteralField(
--- a/tests/php/Security/InheritedPermissionsTest.php
+++ b/tests/php/Security/InheritedPermissionsTest.php
@ -10,6 +10,7 @@ use SilverStripe\Security\Member;
 use SilverStripe\Security\PermissionChecker;
 use SilverStripe\Security\Test\InheritedPermissionsTest\TestPermissionNode;
 use SilverStripe\Security\Test\InheritedPermissionsTest\TestDefaultPermissionChecker;
+use SilverStripe\Security\Test\InheritedPermissionsTest\UnstagedNode;
 use SilverStripe\Versioned\Versioned;

 class InheritedPermissionsTest extends SapphireTest
@ -18,6 +19,7 @@ class InheritedPermissionsTest extends SapphireTest

    protected static $extra_dataobjects = [
        TestPermissionNode::class,
+        UnstagedNode::class,
    ];

    /**
@ -27,18 +29,38 @@ class InheritedPermissionsTest extends SapphireTest

    protected function setUp()
    {
-        parent::setUp();
+        $this->rootPermissions = new TestDefaultPermissionChecker();

        // Register root permissions
-        $permission = InheritedPermissions::create(TestPermissionNode::class)
+        $permission1 = InheritedPermissions::create(TestPermissionNode::class)
            ->setGlobalEditPermissions(['TEST_NODE_ACCESS'])
-            ->setDefaultPermissions($this->rootPermissions = new TestDefaultPermissionChecker());
+            ->setDefaultPermissions($this->rootPermissions);
        Injector::inst()->registerService(
-            $permission,
+            $permission1,
            PermissionChecker::class . '.testpermissions'
        );

        // Reset root permission
+        $permission2 = InheritedPermissions::create(UnstagedNode::class)
+            ->setGlobalEditPermissions(['TEST_NODE_ACCESS'])
+            ->setDefaultPermissions($this->rootPermissions);
+        Injector::inst()->registerService(
+            $permission2,
+            PermissionChecker::class . '.unstagedpermissions'
+        );
+
+        parent::setUp();
+
+        $permission1->clearCache();
+        $permission2->clearCache();
+    }
+
+    protected function tearDown()
+    {
+        Injector::inst()->unregisterNamedObject(PermissionChecker::class . '.testpermissions');
+        Injector::inst()->unregisterNamedObject(PermissionChecker::class . '.unstagedpermissions');
+        $this->rootPermissions = null;
+        parent::tearDown();
    }

    public function testEditPermissions()
@ -160,6 +182,49 @@ class InheritedPermissionsTest extends SapphireTest
        $this->assertFalse($history->canView($editor));
    }

+    public function testUnstagedViewPermissions()
+    {
+        $history = $this->objFromFixture(UnstagedNode::class, 'history');
+        $contact = $this->objFromFixture(UnstagedNode::class, 'contact');
+        $contactForm = $this->objFromFixture(UnstagedNode::class, 'contact-form');
+        $secret = $this->objFromFixture(UnstagedNode::class, 'secret');
+        $secretNested = $this->objFromFixture(UnstagedNode::class, 'secret-nested');
+        $protected = $this->objFromFixture(UnstagedNode::class, 'protected');
+        $protectedChild = $this->objFromFixture(UnstagedNode::class, 'protected-child');
+        $editor = $this->objFromFixture(Member::class, 'editor');
+
+        // Not logged in user can only access Inherit or Anyone pages
+        Member::actAs(
+            null,
+            function () use ($protectedChild, $secretNested, $protected, $secret, $history, $contact, $contactForm) {
+                $this->assertTrue($history->canView());
+                $this->assertTrue($contact->canView());
+                $this->assertTrue($contactForm->canView());
+                // Protected
+                $this->assertFalse($secret->canView());
+                $this->assertFalse($secretNested->canView());
+                $this->assertFalse($protected->canView());
+                $this->assertFalse($protectedChild->canView());
+            }
+        );
+
+        // Editor can view pages restricted to logged in users
+        $this->assertTrue($secret->canView($editor));
+        $this->assertTrue($secretNested->canView($editor));
+
+        // Cannot read admin-only pages
+        $this->assertFalse($protected->canView($editor));
+        $this->assertFalse($protectedChild->canView($editor));
+
+        // Check root permissions
+        $this->assertTrue($history->canView($editor));
+
+        UnstagedNode::getInheritedPermissions()->clearCache();
+        $this->rootPermissions->setCanView(false);
+
+        $this->assertFalse($history->canView($editor));
+    }
+
    /**
     * Test that draft permissions deny unrestricted live permissions
     */
--- a/tests/php/Security/InheritedPermissionsTest.yml
+++ b/tests/php/Security/InheritedPermissionsTest.yml
@ -100,3 +100,70 @@ SilverStripe\Security\Test\InheritedPermissionsTest\TestPermissionNode:
    Title: Child
    CanViewType: Inherit
    Parent: =>SilverStripe\Security\Test\InheritedPermissionsTest\TestPermissionNode.protected
+
+SilverStripe\Security\Test\InheritedPermissionsTest\UnstagedNode:
+  about:
+    Title: About Us
+    CanEditType: OnlyTheseUsers
+    EditorGroups: =>SilverStripe\Security\Group.admins
+  about-staff:
+    Title: Staff
+    CanEditType: Inherit
+    Parent: =>SilverStripe\Security\Test\InheritedPermissionsTest\UnstagedNode.about
+  about-staff-ceo:
+    Title: CEO
+    Parent: =>SilverStripe\Security\Test\InheritedPermissionsTest\UnstagedNode.about-staff
+  about-staffduplicate:
+    Title: Staff
+    Parent: =>SilverStripe\Security\Test\InheritedPermissionsTest\UnstagedNode.about
+  products:
+    Title: Products
+    CanEditType: OnlyTheseUsers
+    EditorGroups: =>SilverStripe\Security\Group.editors
+  products-product1:
+    Title: 1.1 Test Product
+    Parent: =>SilverStripe\Security\Test\InheritedPermissionsTest\UnstagedNode.products
+    CanEditType: Inherit
+  products-product2:
+    Title: Another Product
+    Parent: =>SilverStripe\Security\Test\InheritedPermissionsTest\UnstagedNode.products
+    CanEditType: Inherit
+  products-product3:
+    Title: Another Product
+    Parent: =>SilverStripe\Security\Test\InheritedPermissionsTest\UnstagedNode.products
+    CanEditType: Inherit
+  products-product4:
+    Title: Another Product
+    Parent: =>SilverStripe\Security\Test\InheritedPermissionsTest\UnstagedNode.products
+    CanEditType: OnlyTheseUsers
+    EditorGroups: =>SilverStripe\Security\Group.admins
+  history:
+    Title: History
+    CanViewType: Inherit
+    CanEditType: Inherit
+  history-gallery:
+    Title: Gallery
+    CanViewType: Inherit
+    Parent: =>SilverStripe\Security\Test\InheritedPermissionsTest\UnstagedNode.history
+  contact:
+    Title: Contact Us
+    CanViewType: Anyone
+  contact-form:
+    Title: Send us a message
+    CanViewType: Inherit
+    Parent: =>SilverStripe\Security\Test\InheritedPermissionsTest\UnstagedNode.contact
+  secret:
+    Title: Secret
+    CanViewType: LoggedInUsers
+  secret-nested:
+    Title: Nested
+    CanViewType: Inherit
+    Parent: =>SilverStripe\Security\Test\InheritedPermissionsTest\UnstagedNode.secret
+  protected:
+    Title: Protected
+    CanViewType: OnlyTheseUsers
+    ViewerGroups: =>SilverStripe\Security\Group.admins
+  protected-child:
+    Title: Child
+    CanViewType: Inherit
+    Parent: =>SilverStripe\Security\Test\InheritedPermissionsTest\UnstagedNode.protected
--- a/tests/php/Security/InheritedPermissionsTest/TestPermissionNode.php
+++ b/tests/php/Security/InheritedPermissionsTest/TestPermissionNode.php
@ -7,7 +7,6 @@ use SilverStripe\Dev\TestOnly;
 use SilverStripe\ORM\DataObject;
 use SilverStripe\Security\InheritedPermissions;
 use SilverStripe\Security\InheritedPermissionsExtension;
-use SilverStripe\Security\Member;
 use SilverStripe\Security\PermissionChecker;
 use SilverStripe\Security\Security;
 use SilverStripe\Versioned\Versioned;
--- a/tests/php/Security/InheritedPermissionsTest/UnstagedNode.php
+++ b/tests/php/Security/InheritedPermissionsTest/UnstagedNode.php
@ -0,0 +1,68 @@
+<?php
+
+namespace SilverStripe\Security\Test\InheritedPermissionsTest;
+
+use SilverStripe\Core\Injector\Injector;
+use SilverStripe\Dev\TestOnly;
+use SilverStripe\ORM\DataObject;
+use SilverStripe\Security\InheritedPermissions;
+use SilverStripe\Security\InheritedPermissionsExtension;
+use SilverStripe\Security\PermissionChecker;
+use SilverStripe\Security\Security;
+use SilverStripe\Versioned\Versioned;
+
+/**
+ * @method UnstagedNode Parent()
+ * @mixin Versioned
+ * @mixin InheritedPermissionsExtension
+ */
+class UnstagedNode extends DataObject implements TestOnly
+{
+    private static $db = [
+        "Title" => "Varchar(255)",
+    ];
+
+    private static $has_one = [
+        "Parent" => self::class,
+    ];
+
+    private static $table_name = 'InheritedPermissionsTest_UnstagedNode';
+
+    private static $extensions = [
+        Versioned::class . '.versioned',
+        InheritedPermissionsExtension::class,
+    ];
+
+    /**
+     * @return InheritedPermissions
+     */
+    public static function getInheritedPermissions()
+    {
+        /** @var InheritedPermissions $permissions */
+        return Injector::inst()->get(PermissionChecker::class . '.unstagedpermissions');
+    }
+
+    public function canEdit($member = null)
+    {
+        if (!$member) {
+            $member = Security::getCurrentUser();
+        }
+        return static::getInheritedPermissions()->canEdit($this->ID, $member);
+    }
+
+    public function canView($member = null)
+    {
+        if (!$member) {
+            $member = Security::getCurrentUser();
+        }
+        return static::getInheritedPermissions()->canView($this->ID, $member);
+    }
+
+    public function canDelete($member = null)
+    {
+        if (!$member) {
+            $member = Security::getCurrentUser();
+        }
+        return static::getInheritedPermissions()->canDelete($this->ID, $member);
+    }
+}