Browse files

BUG Fix orphaned pages reporting they can be viewed

  • Loading branch information...
1 parent 7adbf81 commit 3204ab5af35796ef5aa28331b821bea1549a4c2d @tractorcow tractorcow committed Apr 7, 2014
Showing with 77 additions and 0 deletions.
  1. +19 −0 code/model/SiteTree.php
  2. +58 −0 tests/model/SiteTreeTest.php
View
19 code/model/SiteTree.php
@@ -587,6 +587,22 @@ public function isSection() {
}
/**
+ * Check if the parent of this page has been removed (or made otherwise unavailable), and
+ * is still referenced by this child. Any such orphaned page may still require access via
+ * the cms, but should not be shown as accessible to external users.
+ *
+ * @return bool
+ */
+ public function isOrphaned() {
+ // Always false for root pages
+ if(empty($this->ParentID)) return false;
+
+ // Parent must exist and not be an orphan itself
+ $parent = $this->Parent();
+ return !$parent || !$parent->exists() || $parent->isOrphaned();
+ }
+
+ /**
* Return "link" or "current" depending on if this is the {@link SiteTree::isCurrent()} current page.
*
* @return string
@@ -885,6 +901,9 @@ public function canView($member = null) {
// check to make sure this version is the live version and so can be viewed
if (Versioned::get_versionnumber_by_stage($this->class, 'Live', $this->ID) != $this->Version) return false;
}
+
+ // Orphaned pages (in the current stage) are unavailable, except for admins via the CMS
+ if($this->isOrphaned()) return false;
// Standard mechanism for accepting permission changes from extensions
$extended = $this->extendedCan('canView', $member);
View
58 tests/model/SiteTreeTest.php
@@ -944,6 +944,64 @@ public function testMetaTagGeneratorDisabling() {
Config::inst()->update('SiteTree', 'meta_generator', $generator);
}
+ /**
+ * Test that orphaned pages are handled correctly
+ */
+ public function testOrphanedPages() {
+ $origStage = Versioned::get_reading_mode();
+
+ // Setup user who can view draft content, but lacks cms permission.
+ // To users such as this, orphaned pages should be inaccessible. canView for these pages is only
+ // necessary for admin / cms users, who require this permission to edit / rearrange these pages.
+ $permission = new Permission();
+ $permission->Code = 'VIEW_DRAFT_CONTENT';
+ $group = new Group(array('Title' => 'Staging Users'));
+ $group->write();
+ $group->Permissions()->add($permission);
+ $member = new Member();
+ $member->Email = 'someguy@example.com';
+ $member->write();
+ $member->Groups()->add($group);
+
+ // both pages are viewable in stage
+ Versioned::reading_stage('Stage');
+ $about = $this->objFromFixture('Page', 'about');
+ $staff = $this->objFromFixture('Page', 'staff');
+ $this->assertFalse($about->isOrphaned());
+ $this->assertFalse($staff->isOrphaned());
+ $this->assertTrue($about->canView($member));
+ $this->assertTrue($staff->canView($member));
+
+ // Publishing only the child page to live should orphan the live record, but not the staging one
+ $staff->publish('Stage', 'Live');
+ $this->assertFalse($staff->isOrphaned());
+ $this->assertTrue($staff->canView($member));
+ Versioned::reading_stage('Live');
+ $staff = $this->objFromFixture('Page', 'staff'); // Live copy of page
+ $this->assertTrue($staff->isOrphaned()); // because parent isn't published
+ $this->assertFalse($staff->canView($member));
+
+ // Publishing the parent page should restore visibility
+ Versioned::reading_stage('Stage');
+ $about = $this->objFromFixture('Page', 'about');
+ $about->publish('Stage', 'Live');
+ Versioned::reading_stage('Live');
+ $staff = $this->objFromFixture('Page', 'staff');
+ $this->assertFalse($staff->isOrphaned());
+ $this->assertTrue($staff->canView($member));
+
+ // Removing staging page should not prevent live page being visible
+ $about->deleteFromStage('Stage');
+ $staff->deleteFromStage('Stage');
+ $staff = $this->objFromFixture('Page', 'staff');
+ $this->assertFalse($staff->isOrphaned());
+ $this->assertTrue($staff->canView($member));
+
+ // Cleanup
+ Versioned::set_reading_mode($origStage);
+
+ }
+
}
/**#@+

5 comments on commit 3204ab5

@jedateach

@tractorcow for my shop module, this is not a bug. I want to be able to have many products that don't show up in the SiteTree.

I basically perform my own route/url handling and render a product page with ModelAsController using the passed url segment.

I'm guessing the above 'fix' is to prevent content from being viewed which isn't technically editable with the stock CMS.

Perhaps we can introduce a config that disables this check?

Any thoughts?

@tractorcow

Hi @jedateach, could you not override the isOrphaned method for your product page type?

@jedateach

Yes, this is essentially what I've done in the mean time. However, I think it would be cleaner to say that orphan pages can be viewed, rather than pretending orphaned pages are not.

@tractorcow

The problem is that orphaned pages can't be viewed without a routing mechanism. They are only viewable in your case due to your custom routing code. they aren't root pages, so /page-url won't resolve, and /orphaned-parent/page-url won't resolve because the /orphaned-page parent produces a 404.

@Matthew-Bonner

I think adding the isOrphaned method in is a good idea, we use it to check for pages that are no longer accessible from the menu. I'm aware that this isn't a bug, so would rather see the method implemented in core so we don't have to keep applying this patch each time we update the website.

Please sign in to comment.