SiteTree::canCreate Doesn't Hide Pages since 3.1.11

171 views
Skip to first unread message

James Cocker

unread,
Apr 14, 2015, 9:22:38 AM4/14/15
to silverst...@googlegroups.com
(I did post this in reply to the 3.1.11 release post, but I think it got missed)

The SiteTree::canCreate method was given a bit of an overhaul for 3.1.11 to fix a security issue.

A side effect of this overhaul appears to be that any page types with canCreate set to false now appear in the Add page list, but are disabled. Whereas before they wouldn't display in this list at all.

Lots of people including myself used this method to tidy up the list of page types on the Add page screen, by setting canCreate to false for any page types that we didn't want CMS users to see.

If I update my sites to 3.1.11 suddenly lots of disabled page types are going to clutter up the Add new page type list.


Before (3.1.10):



After (3.1.11):



Is this intended?

If it is, how should we now go about completely hiding page types from the Add New list?

If it isn't intended, can we look at resolving this so that it functions as it did before in this regard?

Many Thanks
James 

Patrick Nelson

unread,
Apr 14, 2015, 11:09:30 AM4/14/15
to silverst...@googlegroups.com
Thanks for pointing this out James. I might actually happen to find this useful in my current situation at the moment. Thinking about this a little bit further, I know it's already possible to prevent items from being selected in this list via other means (e.g. ::$allowed_children and, coming soon, ::$allowed_parents).  So with that being said, does it make sense instead to possibly add a PHP preference somewhere which allows the developer to not only gray out these options but possibly prevent them from being visible at all? That of course isn't on a page-by-page basis, but might be a good workaround in case you don't happen to want to mix this functionality (i.e. either gray out disallowed page types or just don't show them).

Also, if it helps (and in case this gets shot down, which it may make sense to do so) - you can probably already do this pretty easily via CSS on your own:

#Form_AddForm_PageType li.disabled { display: none; }

And then in the PHP code, throw in the following in your _config.php (in case you aren't already including your own additional custom CSS file):

LeftAndMain::require_css('/mysite/css/cms.css');

I just tried it and works pretty well for me so I might do this myself :)

- Patrick


--
You received this message because you are subscribed to the Google Groups "SilverStripe Core Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to silverstripe-d...@googlegroups.com.
To post to this group, send email to silverst...@googlegroups.com.
Visit this group at http://groups.google.com/group/silverstripe-dev.
For more options, visit https://groups.google.com/d/optout.

James Cocker

unread,
Apr 14, 2015, 12:19:35 PM4/14/15
to silverst...@googlegroups.com
Of course, I've used that CSS technique before, I should have thought of that! Thanks Patrick, I'll do that in the meantime.

It would be good to have confirmation of if this was an intended consequence of the canCreate overhaul though.

James

Daniel Hensby

unread,
Apr 14, 2015, 2:19:14 PM4/14/15
to silverst...@googlegroups.com

I've Bern running a project against 3.x-dev for about 4 months and I don't recall it hiding pages that couldn't be created, in face I've used the HiddenPage interface to hide specific pages from the list...

I suspect this has been the behaviour for a while and I, personally, don't think it's a bug and is much less confusing to a user.

There will be a PR we can track down to get to the bottom of this, I'm sure. (But I'm out now so can't do it right now)

Dan

Patrick Nelson

unread,
Apr 14, 2015, 2:47:15 PM4/14/15
to silverst...@googlegroups.com
Good point Daniel! I just tested implementing HiddenClass and it works fine for hiding pages, except: In my case I need to hide extraneous pages setup by modules I've installed (in this case, gridfieldpages which I know you've contributed to actually). I don't want to modify composer managed code so I'd be stuck if I were in a situation where I would want to do both things, that is, hide some classes but just disable/toggle others (while still visible, just not able to be selected). Some sort of method (be it ->canAdd() or otherwise) would probably end up being ideal for this, albeit yet another way of doing the same thing. I guess ideally you'd have only a few ways to do something and those ways would be flexible enough to handle a majority of use cases.

I did some cursory research and I'm seeing some instances where ->canCreate() and "HiddenClass" are being called. It would be significant, but "throwing it out there" just in case it sticks, maybe a method like "->isHidden()" could be dropped into Object and that can then check to see if it is itself an instance of "HiddenClass" and inheriting classes could then override that to define extra functionality? A refactoring could be done to reference that functionality centrally instead of repeatedly doing "instanceof" checks. Thankfully since this module extends "Page" I can override that method, however I understand that some external (core/module/etc) classes may not extend something the developer has control over, mitigating this usefulness there, but may still be more flexible than the current interface technique.

The check on the object could be delegated to a framework function, e.g.
function objectIsHidden($object) {
// Needs to be an object.
if (!is_object($object)) return false;
if ($object instanceof HiddenClass) return true;
if ($object instanceof Object) return $object->isHidden();
return false;
}
Then again that creates another bottleneck of extensibility (this is part of the why I can appreciate Laravel's ability to setup service providers and register facades to call those providers, which can be overridden as needed by the dev).

Ingo Schommer

unread,
Apr 26, 2015, 9:15:45 PM4/26/15
to silverst...@googlegroups.com, dam...@silverstripe.com
I can confirm that the class visibility on the "add page" interface has changed with the 3.1.11 release. Specifically, this line here: https://github.com/silverstripe/silverstripe-cms/commit/3df41e1176385215f15fffb04fcba033a5151fb4#diff-be3d9d3389b70ee37e226dd91acf851dL494

My assumption is that canCreate() was removed here because we need a full set of list elements rendered on the client which are further filtered through updateSelectionFilter() in CMSMain.AddPage.js.
The problem is that we can no longer distinguish between "can't create at all" and "can't create in this parent tree context". If you can never create a page as an author, there's no point showing it. If you can create it in other contexts, showing it as disabled gives some indication of the underlying permission model.

Overall, I'm leaning towards hiding list items rather than just disabling them. Creating a distinction between the two "can't create" cases above in the PHP API will make it even more complex than it already is ($allowed_children, $can_be_root, $can_create, $hide_ancestor, $default_parent, $default_child, canEdit, canCreate, SiteConfig->CanEdit, SiteConfig->CanCreateTopLevel). 

As mentioned, HiddenPage isn't a viable workaround because it relies on code modifications. And we shouldn't rely on CSS customisations to influence this fairly significant UI aspect.

How do people feel about the usability impact of this? Its closer to the pre-3.1.11 user experience, and will allow a much less cluttered UI for the common "singleton holder page" pattern without resorting to hacks. @Damian: Can you please provide some context?

Damian Mooyman

unread,
Apr 27, 2015, 7:28:44 PM4/27/15
to Ingo Schommer, silverst...@googlegroups.com
Hi @ingo, as you've probably guessed, the reason for this specific change is that we can't effectively rule out that a page can definitely never be created in any context. There is no "can't create at all" anymore (other than being given HiddenClass directly or $hidesAncestor via config). There is only "can be created in this context" or "cannot be created in this context".

I would agree with hiding elements rather than disabling them, if you felt that was a better UX experience. I think it doesn't make sense to show options that a user can't select (even if only in one particular context). This would make it more consistent with the right-click menu on the site tree.

Kind regards,


Damian Mooyman | Senior Platform Developer
SilverStripe

Skype: tractorcow

Dominik

unread,
Apr 28, 2015, 2:08:50 PM4/28/15
to silverst...@googlegroups.com, ingo.s...@gmail.com
I would like a functionality to hide classes from config without implementing a interface like HiddenClass.

We have many Modules, which are required by other modules, that provides Page types which are not needed by most customers.
Currently we have no chance to hide these classes from Add-Page-Dialog.

The canCreate haven't worked (without overriding in Page-class) also because ADMIN-user can always create any page type, because the ADMIN-check is done before calling the canCreate method on Extension classes.

I looking forward for a solution :).
Reply all
Reply to author
Forward
0 new messages