search

silverstripe / silverstripe-cms

Silverstripe CMS - this is a module for Silverstripe Framework rather than a standalone app. Use https://github.com/silverstripe/silverstripe-installer/ to set this up.
http://silverstripe.org/
BSD 3-Clause "New" or "Revised" License
515 stars 333 forks source link

Prevent deletion of homepage #2154

Closed chillu closed 5 years ago

chillu commented 6 years ago

Overview

It's far too easy to delete the homepage of a website, rendering it effectively useless to visitors. This is a common feedback from our operations team.

Acceptance Criteria

Notes

Review

robbieaverill commented 6 years ago

Related: https://github.com/silverstripe/cwp/issues/69

chillu commented 6 years ago

Discussed this with @lukereative @maxime-rainville @newleeland and we think that it's best to keep this minimal and deny the ability to delete/unpublish/move the homepage. So if you hit one of these actions as an author or CMS admin, you'll get a nicely styled error response and the actions aren't triggered. This ensures that it also applies to batch actions.

Ideally we'd have a confirmation warning dialog, but that's too much investment for this feature. @clarkepaul Are you OK with this approach?

clarkepaul commented 6 years ago

Yup all good @chillu . I would think that ensuring the Homepage remains on the top-level of the tree hierarchy would be a sufficient check rather than ensuring it isn't moved. Otherwise you might want to check if a page other than the Homepage is moved above the Homepage as well (if you are going to restrict moving the Homepage itself).

frankmullenger commented 5 years ago

One of our large projects had this same problem, it wasn't only the home page but several top level pages (of different types) that needed to remain published. We ended up implementing an "Un-unpublishable" checkbox in settings which users with certain permissions could check to remove UI elements that would normally allow a content author to archive/unpublish/change URL segment/move etc. As well as adding URL environment checks for those pages so that we could be alerted if they happen to disappear.

It's a little onerous to set this flag on every page instance, but it only ended up being set once and the number of pages this needed to be applied to didn't end up being many.

Recently a content author on a large project unpublished or archived a root level page and the action cascaded down to unpublish/archive all pages in that section of the site tree. They might be interested in a similar solution which would not be limited to any particular page type.

CC: @chillu

robbieaverill commented 5 years ago

What about some sort of "lock this page" mechanism, which could be enabled by a CMS user of some sort and would prevent certain actions e.g. save, publish, archive, perhaps modification?

clarkepaul commented 5 years ago

You'll still need to let people save and publish as per normal, unless you mean that only certain people can do that @robbieaverill ?

robbieaverill commented 5 years ago

Yeah, just an idea which wasn't very thoroughly thought through. In my mind a "locked page" should be put into a readonly state, including save and publish, but you could unlock it to edit it temporarily or even maybe still perform actions if you have admin privileges but be shown an indicator that the page is locked and your permission level is letting you modify it anyway.

brynwhyman commented 5 years ago

@silverstripeux we're keen to talk with you about whether this will be an advisory message, or prevents the user deleting the page entirely.

newleeland commented 5 years ago

After talking with @ScopeyNZ and @clarkepaul, we had several options.

Options

  1. Removal of Homepage Archive button. This is the simplest method. Clicking the archive button on the home would cause a toast message alert giving a reason why the home page cannot be deleted.

  2. Warning before removing homepage. This was the initial idea of preventing the homepage being removed. This might be used to create modal double checking the content author's true intentions and highlighting the effect it will have.

  3. Have requirement to have a least one page assigned as homepage. When the Archiving the homepage, a dialogue box is opened ask the content author to assign new homepage before removing the existing home page.

The only issue I have to using a permission method as @robbieaverill suggested was that the user roles and groups have issues with their UX and hence, many people don't use them correctly. i.e. everyone is an admin. More details about this research can be seen here: https://docs.google.com/document/d/1mDsvrAPlZNxnW1mmuiWKChik5W3K1qrNtprFShoDulQ/edit?usp=sharing

robbieaverill commented 5 years ago

The only issue I have to using a permission method as @robbieaverill suggested was that the user roles and groups have issues with their UX and hence, many people don't use them correctly

That sounds like a separate issue to me, we need to provide the foundations for people to use our software. If they choose not to that's up to them - unless it's too difficult in which case we can improve it for them

unclecheese commented 5 years ago

Whatever restriction model we come up with, can we bypass it in dev mode? There is a perfectly valid use case for deleting the home page when you're just mucking around trying to get your site tree built out.

michalkleiner commented 5 years ago

I'd vote for a warning while still allowing admins to do so. It's just another page with a specific slug. Is the slug configurable somewhere? What if I wanted it to be homepage as I wanted a page called home to act as a normal page in the URL structure?

newleeland commented 5 years ago

The only issue I have to using a permission method as @robbieaverill suggested was that the user roles and groups have issues with their UX and hence, many people don't use them correctly

That sounds like a separate issue to me, we need to provide the foundations for people to use our software. If they choose not to that's up to them - unless it's too difficult in which case we can improve it for them

I agree its is a separate issue. I just didn't want to make the experience debt larger for the experience for permissions, as there's a few issues with it and linking the experience with homepage archiving functionality will just add to the impact of those stories experience wise.

Even though we chose use a warning for now. In the long run it would be good to have a look change the warning dialog box to a dialogue that gives authors the ability to assign another page as homepage.

bergice commented 5 years ago

When running dev/build, the homepage will be recreated if it is missing. See: \SilverStripe\CMS\Model\SiteTree::requireDefaultRecords.

We may want to rethink how we phrase this warning or even change this logic as it will create duplicate home pages.

robbieaverill commented 5 years ago

@bergice maybe we need to put in a more structured way of determining what (if anything) is the home page. Related is that we talked about doing https://github.com/silverstripe/silverstripe-cms/issues/2051 as part of this issue, which would help. Then you could adjust the SiteTree default records logic so it can detect a home page that doesn't necessary exist at the top of the site tree or have "home" as its URL segment. Another option might be to add a boolean flag "IsHomePage" or something - I'm less into this idea because it would only be relevant for one DB entry and irrelevant for every other one, but at least then you could filter by it in data queries.

newleeland commented 5 years ago

Potential warning design

demo silverstripe org_admin_pages_edit_show_86 (4)

Recommended extra AC

include the dialogue to be show when unpublishing the home page as well

robbieaverill commented 5 years ago

@bergice I've published 0.0.5 of reactstrap-confirm for you