Fixed: "Create new revision by default" checkbox behavior is confusing.

[kswan]

Description of the bug

The Revision settings for a content type are not clear. The options under Revision settings are:

• Show option to create new revisions
  Revisions allow content editors to view changes over time and revert changes if needed.
• Create new revision by default

The current functionality is that the "Show option to create new revisions" actually enables or disables revisions for the content type. If "Show option to create new revisions" is off, the "Create new revision by default" is ignored.

Steps To Reproduce

To reproduce the behavior:

1. Open the configuration page for a content type.
2. Set the Revision settings to uncheck "Show option to create new revisions" and check "Create new revision by default"
3. Edit a node of the content type that was edited.
4. View the revisions of the edited node.

Actual behavior

No revisions is saved for the edited node.

Expected behavior

Since the Revision settings are showing "Show option to create new revisions" unchecked and "Create new revision by default" checked, the revision setting shouldn't be shown on the node edit page, but the revision should be saved.

Alternatively, the Revision settings can be reworded to reflect the actual behavior. Maybe:

• Enable the option to create new revisions
  Revisions allow content editors to view changes over time and revert changes if needed.
• Create new revision by default
  And "Create new revision by default" should be hidden if the first option is disabled.

Additional information

In my use case, I want revisions saved for every node edit without the option to not save a revision. I am able to accomplish this by enabling both check boxes under Revision settings and using the Simplify module to remove the revision setting from the node edit form.

Add any other information that could help, such as:

• Backdrop CMS version: 1.24.2

[klonos]

I confirm this, and find it to be a bug 👍🏼

Since the Revision settings are showing "Show option to create new revisions" unchecked and "Create new revision by default" checked, the revision setting shouldn't be shown on the node edit page, but the revision should be saved.

Yup, that's what I would expect too 👍🏼 ...if the intention was to only save new revisions by default if the "Show option to create new revisions" is ticked, then I would expect some rewording in the checkboxes and help text + have the "Create new revision by default" checkbox be dependent on the "Show option..." checkbox (via states). However, I believe that the intention of having "Show option..." unticked and "Create new revision by default" ticked is to configure the content type to be saving revisions automatically each time the content is saved/updated.

This looks interesting, so I'd like to attempt a crack at it...

[klonos]

...FTR, here's the respective setting in D10 (on by default OOTB for the "Basic page" content type):

Noting the help text:

Users with sufficient access rights will be able to override these options.

...once that checkbox is ticked, there seem to be no permissions specific to allowing people to skip creating a revision. The only relevant permissions in D10 are the following:

• %content_type%: Revert revisions
• %content_type%: View revisions
• Delete all revisions
• Revert all revisions
• View all revisions

So once that setting is enabled for a content type, this is shown to the sidebar when adding new content of that type, at all times for everyone:

Once a piece of content has been created, when editing it, you have the option to create a revision or not:

If you untick the "Create new revision" option for that content type, then editing an existing piece of content of that type has the option for creating a revision off by default, but you can choose to create one if you want:

[klonos]

...D7 works pretty much the same, with the only difference being that by default the "Basic page" content type has the option for revisions off OOTB.

[klonos]

...and here's what we currently have in Backdrop:

The help text refers to what revisions are and why they are useful, yet it is attached to one of the settings in that vertical tab (while it is a generic help text that applies to revisions in general).

I think that the UI should be changed to this, to make things clear (and functionality adapted to match it):

[kswan]

@klonos, I think your proposal is very clear and resolves the misleading nature of the current UI.

I will note that the second option (Always create revisions) is currently not possible with core. So, technically, this proposal combines a bug fix and a feature. For me, that is the option I typically would select so I support this proposal.

[klonos]

So, technically, this proposal combines a bug fix and a feature.

Yeah, I realized that, but at the same time I also don't like half-baked solutions (we'd be trying to fix something in a UI that doesn't work well to begin with) 😉 ...I'd be happy to split my UI proposal into a new issue, and leave this issue here to focus only on fixing the bug that has been reported.

[kswan]

I also don't like half-baked solutions (we'd be trying to fix something in a UI that doesn't work well to begin with)

I agree.

[yorkshire-pudding]

@klonos - what is the status of this? I don't see a linked ticket

[klonos]

...this was brought up in (and is somewhat blocking) #3733.

[klonos]

Noting that if we are to introduce the feature of allowing the option to always create a new revision, then we also need to specify a safe maximum number of revisions to retain (not sure if that should also be allowed to be set via the admin UI or not - leaning towards not) + a way to purge/cycle revisions (delete the oldest if a new one is created when the limit has been reached). In my previous job, we had a site where a couple specific pages were crashing, and that was caused because they were updating these specific nodes programatically via cron runs from external sources. In that case, the customer needed to use https://www.drupal.org/project/node_revision_delete.

PS: the project page of node_revision_delete linked above has a huge list of similar modules by the way (which to me indicates that it should be in core). Also, there seems to have been a coordinated effort in the community to consolidate this functionality in a single module instead of having a dozen to choose from for the same functionality.

[yorkshire-pudding]

Noting that if we are to introduce the feature of allowing the option to always create a new revision, then we also need to specify a safe maximum number of revisions to retain (not sure if that should also be allowed to be set via the admin UI or not - leaning towards not) + a way to purge/cycle revisions (delete the oldest if a new one is created when the limit has been reached). In my previous job, we had a site where a couple specific pages were crashing, and that was caused because they were updating these specific nodes programatically via cron runs from external sources. In that case, the customer needed to use https://www.drupal.org/project/node_revision_delete.

PS: the project page of node_revision_delete linked above has a huge list of similar modules by the way (which to me indicates that it should be in core). Also, there seems to have been a coordinated effort in the community to consolidate this functionality in a single module instead of having a dozen to choose from for the same functionality.

Should we also consider given the example above that not all revisions have equal value? I would be more interested in keeping revisions saved by a person than those by an automated update as more likely to be a meaningful change, whereas those updated by a script should be more predictable and therefore less interesting other than very recent history.

[klonos]

...whereas those updated by a script should be more predictable and therefore less interesting other than very recent history.

You would think that, but the code that does it is created by humans 😅, so the risk of a destructive change that needs to be reverted is still there. But yes, I see what you mean 👍🏼 ...that though (being able to distinguish between human-initiated vs automatic/programatic changes) sounds like a different feature 🤔

[yorkshire-pudding]

though (being able to distinguish between human-initiated vs automatic/programatic changes) sounds like a different feature

I think automatic purging of revisions is also outside the scope of this ticket, but perhaps the fine grained control of revision purging would be a logical part of such a feature?

As an aside, if anyone needs to enforce revisions before this bug fix is implemented, then the Enforce Revision Logs module gives that ability.

[klonos]

OK, who wants to give it a spin?: backdrop/backdrop#4546 🙂

[klonos]

During the UX meeting this week we started playing with the PR sandbox, and it turns out that something's amiss (most likely config values not saved properly) ...I'll have to take another look at the code. Please feel free to give it a quick spin and provide feedback on the general UX though.

[kswan]

@klonos I tried to find the cause of the problem you found in the UX meeting, but couldn't find it. The config files are still showing the "revision_enabled" setting that should be changed to "revisions".

I found a line in function node_node_type_load(&$types) that is still showing "revision_enabled" (I added a comment to the PR). I am not sure if that is causing the config issue.

[klonos]

Thanks @kswan 🙏🏼 ...I'll fix that. The odd bit though is that things on my local are working as expected (same branch as in the PR) 🤔

[klonos]

...it also works as expected in the PR, but only for existing content types - not for new ones. Taking a deeper look.

[izmeez]

I appreciate everyone's efforts on this issue.

Looking at the new PR 4649 and again at the previous PR 4546.

Both of them, like the current state, could be improved with the addition to the wording of "... for this content type". This may help to reinforce it applies only to each specific content type.

I think the new PR 4649 provides additional information about the "Administer content" permission which is helpful. However, I find it is less clear that it is enabling a choice for content editors which can then be used to create a revision or not. A simpler wording change as suggested in #6103 (comment) may help to address this.

Overall, I think PR 4546 provides more clarity and the new feature to "enforce" revisions may be useful to sites and would not cause problems for existing sites as that option is not selected by default.

The bottom line is that the current system does work and once a newcomer overcomes the hurdle of the confusion the existing UI may cause, they can continue to use it. It is different than D7 where the option to create a new revision is in the "Publishing options" tab such that those migrating will have a little shift to make.

[herbdool]

I closed backdrop/backdrop#4649 and recommended it be on a new issue where there might be backwards-incompatible changes. Maybe it'll still gain traction, not sure. As for my PR, I've put it ready for testing and review. With a small rewording change.

[izmeez]

Tested the PR and it works for me. Also reviewed the code and it looks good. It would be nice to hear what others think. Thank you.

[kswan]

I agree backdrop/backdrop#4649 works for me and the code looks good.

[quicksketch]

Thanks folks! I did not read over this entire issue, but the fix seems good and @kswan, @izmeez, and @herbdool all seem to be in agreement that it is an improvement. I've merged backdrop/backdrop#4649 into 1.x and 1.27.x. Thanks!