"Delete published entry" button bypasses editorial workflow

[pmpinto]

Describe the bug Can't really tell if this is a bug or a feature, but it really does fell like a bug to me. We have editorial workflow enabled, so that editors can create branches when suggesting changes and those changes can then be reviewed by someone else — this works great for us!

The issue seems to be when deleting something that's already published. Last Friday we needed to get rid of some old files and used the CMS for that. We were expecting — as we're using the editorial workflow — to have seen a PR and a branch. Just like when we add a new file.

But in fact, the changes were pushed directly to our backend.branch. This is problematic because:

• Editors can mis-click the "delete published entry" button and not read the alert popup message
• Other editors will never be aware of this change as no entry is created in the workflow tab and they would have to go through GitHub's commit history to see it

Also, to make things worse, as we have some requirements for merging PRs, including having 1 approval from someone with write access:

• When a user with higher level permissions attempts to delete an entry from the CMS, this requirement will not be asked from GitHub
• When a regular user with write access attempts to delete an entry from the CMS, GitHub will complain it needs approval — although there's no PR whatsoever, instead, this error is shown:

Failed to delete entry: API_ERROR: Required status check “cypress-run” is expected. At least 1 approving review is required by reviewers with write access.

To make it clear: regular users, who are able to add files and suggest changes to existing file, are unable to delete entries

To Reproduce

1. Visit a file collection in the CMS
2. Click on one of the entries
3. Click "Delete published entry" from the top bar
4. See the error (if regular user) / See the entry get deleted without a PR (if higher level permissions user)

Expected behavior As mentioned above: since we have editorial workflow enabled, it would make sense for a PR to be created and a change to be listed under the workflow tab in the CMS. Which should then be approved before the author could merge it.

Applicable Versions:

• Netlify CMS version: 2.15.30
• Git provider: GitHub
• OS: macOS 11.2.3
• Browser version: Brave v1.25.73
• Node.JS version: 12.17.0

[erezrokah]

Thanks @pmpinto, I'll classify this as a feature request as existing CMS users might have other expectations, but I can understand why it can be seen as a bug.

To make it clear: regular users, who are able to add files and suggest changes to existing file, are unable to delete entries

Are those users able to publish entries?

Regardless, this would make a nice contribution to the CMS. Not sure how we should display deleted entries though in the editor/workflow tab, so it would make sense to discuss it first here.

Also related to https://github.com/netlify/netlify-cms/issues/2

[pmpinto]

Are those users able to publish entries?

Yes. So, users with write permissions on a repo can use the CMS to create new entries, suggest changes to existing entries but not delete entries. Admin users can do whatever they want, basically.

I'll classify this as a feature request as existing CMS users might have other expectations

That's a good point, although I have no idea how this point doesn't surface more often, or even how we never found it until now.

For a protected branch where the CMS is writing to, users with write permissions can't delete entries through the CMS. But they can, manually through git.

I see how one could look at already existing users and want to avoid causing more issues while trying to fix one. But labelling this as a feature request seems odd to me — just based on the fact that the current behaviour isn't what's expected.

Wouldn't it be worth to keep this as a bug and expect a next major release, whenever that happens, to include a fix for this? A new major version shouldn't be expected to be backwards compatible anyway… So existing users can gain awareness for a change in behaviour.

[erezrokah]

Yes. So, users with write permissions on a repo can use the CMS to create new entries, suggest changes to existing entries but not delete entries.

Before those non-admin users publish the PR they would need someone to approve it on GitHub correct?

For a protected branch where the CMS is writing to, users with write permissions can't delete entries through the CMS. But they can, manually through git.

Do you mean deleting through git by going through a PR workflow? They shouldn't be able to push directly to the default branch

I see how one could look at already existing users and want to avoid causing more issues while trying to fix one. But labelling this as a feature request seems odd to me — just based on the fact that the current behaviour isn't what's expected.

Wouldn't it be worth to keep this as a bug and expect a next major release, whenever that happens, to include a fix for this? A new major version shouldn't be expected to be backwards compatible anyway… So existing users can gain awareness for a change in behaviour.

The classification as a feature comes from it requiring a new UI flow to facilitate deletion, I agree the boundary is not very clear. We can see if others comment on the issue to get more feedback on this.

Regardless I'll open a new issue with potential breaking changes and link to here.

[bytrangle]

Can I get more clarification for the reproduction steps?

1. Visit a file collection in the CMS. This means selecting the Content tab at the top of the admin UI, isn't it?
2. When clicking the *Delete published entry" button in the top bar, an alert box is displayed, isn't it?

It is not that easy to mis-click the Delete button IMHO, although I agree that it's not good when other editors will never be aware of this deletion unless they go through the commit history.

And how should this deletion be displayed in the UI? I'm thinking of having a new section under the Collections section in the left sidebar.

[erezrokah]

@bytrangle thanks for looking into it!

And how should this deletion be displayed in the UI? I'm thinking of having a new section under the Collections section in the left sidebar.

Since this is only relevant for editorial workflow, maybe we have an indication in the workflow tab.

@pmpinto can you elaborate on this:

Expected behavior As mentioned above: since we have editorial workflow enabled, it would make sense for a PR to be created and a change to be listed under the workflow tab in the CMS. Which should then be approved before the author could merge it.

from a CMS user perspective. How would deleted entries should look like in the workflow tab, and should the CMS allow to edit them?

[bytrangle]

If deleting a published entry is supposed to trigger a PR, then it would need someone with permissions to approve the PR. Then this means the “deleted entry” is actually not deleted. It is actually pending deletion and may never be deleted if the PR is not approved, correct?

This can get complicated fast. If there's real concern that editors can accidentally delete a published entry, then I think we can add an Undo toast that will be displayed for a few seconds to let users reverse their mistakes.

When I have time, I'll look into other CMSs to see if there is any good implementation of content deletion.