backdrop / backdrop-issues

Issue tracker for Backdrop core.
144 stars 40 forks source link

[D8][WP][UX] Forward Revisions (API capabilities only) #4354

Closed stpaultim closed 4 years ago

stpaultim commented 4 years ago

Because issue #1475 has drifted from it's original intention and gone through so many changes/discussion, we decided to close that issue and open two new issues - including this one.

Description of the need

Currently, Backdrop CMS provides the ability to create multiple revisions for a specific node, however in it's current state only the most recent revision can be the revision that is visible when viewing the node.

It would be helpful for a content editor to be able to make a Draft revision of node that is not yet Approved.

For example: a site editor should be able to make changes to the "About Us" page on a site and save their changes, without changing the public revision of the page until another editor has approved their changes and approves it.

Glossary of terms:

Approved - The revision of a node that EVERYONE sees when they view the node. This is also the revision they will copy when they click on the "Edit" button for that node.

Not-approved - Any revision of a node that is not the approved revision. This state only applies to revision of a node, and should not be used when referring to the node itself. This state is defined by the status column on the node_revision table.

NOTE: The UI does not necessarily distinguish between draft and past revision revisions. These are defined below primarily for purposes of this discussion. If we decide not to distinguish the difference, we propose that all Not approved revisions be named Drafts.

(It is not yet clear if there is any technical difference between Draft and Past revisions other than the publish date)

Revisions / Versions - Perhaps we should switch to Versions in the UI - separate issue: https://github.com/backdrop/backdrop-issues/issues/4337

Published vs Unpublished - This refers to the state of a node. For the purposes of this document, only the node itself can be published or unpublished. A revision of a node is never published or unpublished.

Proposed solution

  1. Once revisions have been enabled on a node, no revision can ever be edited. Any saved changes to any revision will create a new revision.

  2. Once revisions are enabled, we should use “Clone,” “Revert,” or “Copy” - but not “Edit.” Anytime changes are saved to a revision, we are ALWAYS creating a new (most recent) revision*. Saving changes to a revision does not necessarily make it the approved revision - but it does put that revision on the top of the list of revision (most recent).

  3. When editing any a node (regardless of which revision) a user has the option to either Save and approve OR Save draft.

Alternatives that have been considered

See issue [#1475] for more history of this issue.

Additional information

I don't know that we have exact consensus on the exact wording of buttons and options. But, I think this is very close to what we discussed. Please, help clarify. I hope that this is close enough to get us to a next draft PR (if not, let us know).

Better_workflow_States_-_Google_Docs

We discussed several options for the "approve" option here. Unfortunately, I don't recall exactly what we settled on. I'll go with the simplest for now and await feedback from @jenlampton or @klonos (or anyone else).

Forward_Revisions_0r_Better_Workflow_States_-_Google_Docs

HISTORIAL DOCUMENTS :-)

Jen's first draft summary https://docs.google.com/document/d/1svJioZPoF9Ex7e5icMa4KGkOsQ23cLSNNHxGW6iEiBk/edit#

Tim's second draft summary https://docs.google.com/document/d/1bZ2i5FboKLYjKNeR1aiTtvyqarNmfLD9IkBIDwbsh1g/edit#

Screencast of Django interface for similar feature: https://youtu.be/Vxrrd-027OY


Temporary PR by @docwilmot: backdrop/backdrop#3051 PR by @docwilmot: https://github.com/backdrop/backdrop/pull/3124

stpaultim commented 4 years ago

This was part of our most recent document, but I don't recall coming to consensus on this. I'm afraid that adding an option to save without creating a new revision will just get too complicated. But, I don't object to trying it if people think it's helpful.

@klonos - counter-proposal for consideration:

When editing a draft version, we could consider having the option to “Update current draft” or “Save as new draft” - I envision these being 2 separate submit buttons (or a single, submit dropbutton). This would prevent the accumulation of too many unnecessary versions (i.e. cover the use case of: don’t create a new draft if all I’m doing is fixing a typo). The same can be done while editing the currently published version - two options: “Update published version” (saves on top of the published + keeps the node published - does not create a new draft) or “Save as new draft” (leaves the published version alone + creates a new draft). In other words, we always have 2 submit buttons/options when editing content: “Save as new draft” will always be the one option, while depending on whether the version being edited is the published one or not, the other button would either be “Update published version”, or “Update current draft”.

@klonos's other awesome idea :)

If we are able to clone a node (or a revision of it) into a new draft, then that may be a window to allow cloning into an entirely new node (which is practically node_clone - so I understand if that’s for contrib-land)

Graham-72 commented 4 years ago

I will find this very useful for co-working in Backdrop. I feel the current title 'Forward Revisions' doesn't convey the significance or importance to users of this proposal.

Is there a PR for this?

Does this need some sort of priority rating to get in the May release?

stpaultim commented 4 years ago

This issue has been on the active development agenda (during weekly meetings) for the last couple of months. It was held up for several weeks (or more) as we discussed the user interface for this feature, which has the potential to be very confusing if we are not careful. We recently came up with some ideas to move the issue forward again.

Temporary PR by @docwilmot: backdrop/backdrop#3051 (This PR is close to what we are shooting for but still a draft. If you are curious, it might be worth looking at this PR, noting that several things are changing to match the specs in this issue.)

To better understand the history of this issue, read/skim #1475

I agree, that 'forward revisions' is a very technical description of what we are doing and not very user friendly. We should think more about how to present this feature to the public once it is complete. We have also discussed this as an advanced workflow or maybe an "enhanced workflow."

stpaultim commented 4 years ago

@docwilmot just asked in Zulp about our level of confidence in the proposed changes to the node form.

But I'm struck though by the dramatic shift in our approach to node form changes! We started off wanting it seems to limit any significant changes to the node form. See this issue comment by @quicksketch, although maybe I was misinterpreting his comment again. But now we've added extra buttons, renamed links, etc.

It occurs to me, that @quicksketch did have some clear reservations in the past, but to my knowledge has not signed off on this proposal. Maybe, we need the attention of at least one core committer before investing too much more work on changes that might in the end be problematic?

stpaultim commented 4 years ago

Here are some concerns that were raised here by @quicksketch which I think have been addressed (or I am proposing a fix):

The "Published" radio button being selected when editing a previous revision. But it's not the "active" revision, so calling it "Published" is confusing, it's not accessible to the public.

I think the last PR fixed this concern by not showing the "Publishing Option" tab when creating a new non-current draft.

The tab at the top of the page is "Edit Draft", but this may have been a previous revision. Draft indicates something that hasn't been published yet.

Here are my proposed resolution to this item: LAST PR (when viewing a NON-approved revision): Revision_of_Turtles_from_Mon__02_10_2020_-_2_42am___PR_3051_backdrop_backdrop_testing_site

PROPOSED (this is just my idea, but seems consistent with proposal): Revision_of_Turtles_from_Mon__02_10_2020_-_2_42am___PR_3051_backdrop_backdrop_testing_site

Editing a non-active revision doesn't repopulate the "Revision Log" field.

I think our proposal will address this.

And what happens when I check "Create new revision" off an old revision? It seems like only the active revision should have the ability to create a new revision.

I think we have addressed this in the proposal, the current proposal allows you to "Clone" and then edit any revision and improves the language around this.

Yes, I think so. If you switch it from a checkbox to a #type = value then we keep the ability to make new, non-active revisions but it is not directly exposed as an option on the node form. Contrib could then set this value via a UI of it's choice.

Nate raised this concern earlier than the above concerns. This is the concern that most directly addresses the question by @docwilmot in that @quicksketch seems to be suggesting that we want to keep much of this out of the UI.

MY INTERPRETATION: I think he was overall addressing the fact that some of the UI changes were pretty confusing at the time and unless we could make it much easier to use, it might make sense to leave those changes to contrib. Hopefully, we've improved the changes to the UI enough that he would be on board.

I don't understand the implications of If you switch it from a checkbox to a #type = value enough to comment on whether or not it's been addressed or is still of concern.

docwilmot commented 4 years ago

I think you are right. At the very least I will continue as if you are. 😄

stpaultim commented 4 years ago

Just chatting with @docwilmot about the term clone and alternatives. I don't think that any of us liked clone, but it was the best we have so far. Could we replace it with "Copy and Edit"?

Instead of Clone we use Copy and Edit? Instead of Clone and Edit we use Copy and Edit?

My concern is that's it's too wordy. But, it might just work fine and I think it does more accurately describe what is happening.

How about replacing "Clone" and "Clone and Edit" with "Copy and Edit"?

olafgrabienski commented 4 years ago

Coming late to the party, I have to admit that I have difficulties to fully understand this feature request. Guess I get the idea, though.

I had a look at the temporary PR (https://github.com/backdrop/backdrop/pull/3051) to get a more real impression, being aware that the PR is a (re-)start. Here my first thoughts:

screen-backdrop-pr-3051-node-form
docwilmot commented 4 years ago

When I hit the button, I get a HTTP error 500. Actually, I was assuming that Save draft makes a new revision.

Right on both counts. Fixed.

What's the relation between the Draft publishing option and the Save draft button?

This is my latest challenge, not sure how to resolve it. Any thoughts?

Graham-72 commented 4 years ago

I think it might help the UI if there were to be a fourth radio button in addition to the current Published / Draft / Save for later. The meaning of the fourth button being 'Published, with amendments pending'. This would give a clear visual indication of the state of the node.

In a co-working situation 'save draft' would mean updating the current pending amendments (revision) or save a first such revision, and 'Save' would mean publish the amended node. After a 'Save' the radio buttons should indicate a status of 'Published'.

Maybe 'Save for later' is not quite clear in this scenario because it relates just to the initial publication of the node, not to the proposed amendments.

olafgrabienski commented 4 years ago

What's the relation between the Draft publishing option and the Save draft button?

This is my latest challenge, not sure how to resolve it. Any thoughts?

It depends! Is there an actual relation between Draft and Save draft (challenge: making it more clear and coherent), or are they just using the same term (challenge: naming things differently)?

stpaultim commented 4 years ago

@docwilmot - We discussed this issue during dev meeting today. Even had a short screen share. It was just @jenlampton and myself. She likes "Copy and Edit" better than "Clone." Thanks for updating the PR with that. It's nice to actually see it in place.

olafgrabienski commented 4 years ago

What's the relation between the Draft publishing option and the Save draft button?

This is my latest challenge, not sure how to resolve it. Any thoughts?

I've had another look at the (updated) PR and confirmed that the Draft publishing option on the one hand and the Save draft button on the other hand use the same term but do different things:

That's more than confusing, and one of the terms should be changed, but which one, and how? Let's discuss!

olafgrabienski commented 4 years ago

one of the terms should be changed, but which one, and how?

In my opinion, there are arguments for both.

At first sight and in the current situation it seems reasonable to keep the name of the publishing option Draft. It's handy and it sounds more elegant than e.g. "Unpublished". However, it can also be misleading: An editor at one of my sites one day used the Draft publishing option to save a page version with changes she wanted to apply later, and she was quite surprised that she actually unpublished the page.

So, at second sight, I'd suggest to change the name of the Publishing option to something more clear like "Unpublished". (If we find s.th. more elegant, even better.) Important benefit: We could use Save Draft not only for revisions in the current PR but also for more advanced editorial workflows (Draft > Accepted > Peer-reviewed > Whatever > Published).

However, at third sight, a Save Draft button isn't only used in this PR but also in the quite popular Save Draft module. The button saves a node as not published, and the module page claims this to be a usability improvement. Considering the Save Draft module and the current PR, we'd have two identical buttons with different meaning! How would we deal with that?

Finally, if we consider to rename the Save draft button of the current PR, what would be good alternatives? If we don't find any, we could stick with "revision" for the moment, i.e. Save revision. Thoughts?

docwilmot commented 4 years ago

I'm wondering if we could rename the Publish/Draft/Schedule radios to Available/Offline/Schedule? Or something similar that suggests that these options are not making drafts but actually determine if this node is available at all.

jenlampton commented 4 years ago

I'm wondering if we could rename the Publish/Draft/Schedule radios to Available/Offline/Schedule?

We need to keep the word Publish for nodes for backwards-compatible terminology reasons. That word has a very specific meaning for anyone in Drupal/Backdrop and removing it will cause too much confusion. I do really like Offline as an alternative to Draft though!

docwilmot commented 4 years ago

I do really like Offline as an alternative to Draft though!

Great, will do that. I think this was the last (mental) blocker. I will go ahead and finish this off if possible.

Also considering adding an option to the schedule radio to allow to choose a version to publish. Should be a nice touch.

olafgrabienski commented 4 years ago

I do really like Offline as an alternative to Draft though!

Great, will do that. I think this was the last (mental) blocker.

In the meantime I'm also more decided to change the name of the Draft publishing option (and not the Save draft button). Offline is fine as a start but we should still look for something better. Deserves an accurate discussion, in my opinion.

olafgrabienski commented 4 years ago

Considering the Save Draft module and the current PR, we'd have two identical buttons with different meaning! How would we deal with that?, https://github.com/backdrop/backdrop-issues/issues/4354#issuecomment-612021576

@laryn What do you think, as Save Draft maintainer?

Graham-72 commented 4 years ago

We could have 'propose' instead of 'save draft' since it is the creation of a proposed new version isn't it?

docwilmot commented 4 years ago

We had a brief sprint and went over things so far. Here are some of the answers to remaining questions about this:

with the proposed new Save draft buttons, the checkbox to "create a new revision" becomes unnecessary, does this mean the node type setting to "enable revisions by default" becomes unnecessary? All that node type setting does is make the "create a new revision" checkbox checked by default.

Proposed was to deal with this in followup, but for now just hide the checkbox by making it a form value instead.

do we need any new permissions for the changes in this PR?

No

Other workflow type modules have navigation links for the revisions (like a pager, with previous/next), should we do this?

Later? Also considered popping up a list of revisions from a link, like GitHub does for issue edits, or in a Backdrop dialog from the node revision view page, but not required now. It would be good to get the revision tab to show up on the revision view page as well, will look into doing that.

should we add the Save Draft and Save and approve buttons to node preview bar?

Yes, preview bar should mirror the node form

add a confirm form before any revision is made the approved revision? Including from the node form when clicking the Save and approve button. We dont currently have any situation where the node form does that.

yes

add a view of pending revisions?

Not needed in core

the node revisions on view now use "Draft of node title from date ...". Should we use the revision title instead? How then to clearly show that this is a revision?

we considered options; unpublished nodes now have a red bar above the node content and below the node title saying the node is unpublished. We could do that to be standardized, maybe should, but we dont like it, and propose using a standard blue info message for details (that says"Draft of node title from date ...).

Also considered

laryn commented 4 years ago

Considering the Save Draft module and the current PR, we'd have two identical buttons with different meaning! How would we deal with that?, #4354 (comment)

@laryn What do you think, as Save Draft maintainer?

I'm tracking this issue only lightly at the moment (but am excited about the functionality that is being developed). From the perspective of the "Save Draft" module maintainer, two identical buttons with different meanings is not good. I'm open to adjusting the "Save Draft" module in some way (in fact, I'm guessing that will be required, although I have no idea what that would/should look like and would love input from anyone on that), or does something like "Save a Copy" make sense here?

docwilmot commented 4 years ago

Updated with recommendations. Please test, before I start writing tests. 😄

Graham-72 commented 4 years ago

This is excellent and I am sure will be very useful, particularly for co-working on a site. Thank you.

My one worry is that I will not remember the full meaning and significance of the new terms 'copy and edit', 'save and approve', 'set as approved' and 'save draft' unless I am using them frequently. Oh for pop-up help tips!

I have tried the PR briefly and everything seems to work well.

olafgrabienski commented 4 years ago

Below some observations from trying the PR's sandbox site. (Generally, I like the direction! At the same time, the user interface remains a challenge.)

olafgrabienski commented 4 years ago

Regarding terminology, I've had a quick look at WordPress which uses "Draft" as status for not published pages (or posts), like we did before the current state of the PR (now "Offline").

In WordPress, there is also a "Save Draft" button, and it saves a page with status "Draft". Just like Backdrop does when using the popular "Save draft" module. In contrast, with this PR, we use the button to save a new revision. Still not sure if we really should do so, sorry for the back and forth! Can we get more feedback on this question? Here some ideas from above, btw:

if we consider to rename the Save draft button of the current PR, what would be good alternatives? ... we could stick with "revision" for the moment, i.e. Save revision

We could have 'propose' instead of 'save draft' since it is the creation of a proposed new version (by @Graham-72)

does something like "Save a Copy" make sense here? (by @laryn)

jenlampton commented 4 years ago

I'm adding labels to this issue based on where I think it is, please update if I got them wrong :)

olafgrabienski commented 4 years ago

We had an interesting discussion about this feature request at the last weekly meeting (April 16) and decided to drop the UI changes but to leave the API additions. Here's the recording of the discussion: https://youtu.be/OaGgfUbJ9gY?t=1872

Graham-72 commented 4 years ago

I would very much like to see some basic provision for co-working to be added to Backdrop core, though it must of course be provided in a way that is easy for users to understand and use. Viewing this video of the recent discussion I have the strong feeling that the problem with the UI is caused by the use of the radio buttons as both an indication of the status of the node and as a means of changing from one state to another.

What might be better, though perhaps revolutionary, is to have some other way of indicating to the user the current status of the node and then provide ordinary buttons as appropriate for that status as was suggested at one point in the discussion. The indication of the current state might be a simple heading, e.g. 'This node is not yet published' for state 1 (see below.)

The four states of a node that I envisage are:

  1. Not yet published (i.e a draft}
  2. Scheduled for publishing (i.e ready for publication at a specified time/date)
  3. Published (i.e. without any proposals for further revision)
  4. Published with pending revisions (i.e. one or more re-drafts exist)

In each of these states there would be appropriate 'action' buttons determining what is done with any changes made in the fields of the node, or revealing further information. Once the radio buttons are abandoned I don't think it would be difficult to resolve what the action buttons should be in each state and then choose appropriate wording.

From state 1 the user could (a) remain in that state with a revised draft, or (b) publish, i.e. move to state 3, or (c) schedule for future publishing - state 2.

From state 2 the user might be able to cancel or change the scheduled publication time.

From state 3 the user might be able to simply publish the revised version of the node, or if revisions are enabled publish a proposed revision and thereby move to state 4.

From state 4 there might be a button for 'save as revision' and another for 'view revisions', the latter leading to a list of any revisions drafted so far (re-drafts); only from this latter list would one be able to publish a revised version of the node.

docwilmot commented 4 years ago

New PR significantly simplified, no node UI changes, and includes tests of forward revisions capability.

quicksketch commented 4 years ago

Thanks @docwilmot! I reviewed https://github.com/backdrop/backdrop/pull/3124#pullrequestreview-400424685 and it was a LOT easier for me to focus on with just the API parts.

Conceptually looks good. I have a concern about the new return constant SAVED_REVISION. I think we can probably remove that without any serious impact on useful return values.

Now that we're only discussing the API, I'd like to visit the terminology discussion again (sorry!) with just the API in mind. Let's take new property and method as our examples:

The property: $node->is_default_revision The method: $node->isDefaultRevision()

Same as in other discussions, we need to decide what word to use here. I'm not sure when (or even if) "Default" was chosen, perhaps as part of the UI discussions at some point? The current PHPdoc refers to having a "Current" revision. Other suggestions I've heard include "Approved", "Visible" and "Active".

Here's my take on the options:

Word Reasoning Con
Active Implies "In Use" or "Doing the work" ??
Approved After something has revisions, this is the one that an editor has selected Implies a multi-person workflow, and that multiple revisions would be created before publishing.
Current Existing documentation and UI terminology uses "Current" Unfortunately associated with time. A newer revision at the current time might not actually be the current revision.
Default When visiting the URL directly without a revision ID in it, this is the one they're shown We're using this word in other places for different purposes, such as a field default or for configs that "revert to default", where they use the module-provided version.
Visible When visiting the node URL, this is the "visible" revision Other revisions are also "visible" if you have the access revisions permission. So not entirely accurate.

I can't find a fault with "Active" from an API stand-point. Thoughts? (and sorry for opening the can of worms again).

EDIT: Added "Approved" word as well.

jenlampton commented 4 years ago

I personally like Active the best, and can't I remember the con we came up with when we talked about it. Was it that if you were actively editing a revision that wasn't the Active one you might get confused? If so, I think this con is a bit of a stretch, and that most people would understand if they were editing an active revision vs an inactive revision.

However, if we can't make a decision on this soon I think we should go with Current because that's equivalent to "not making a decision" since that's what's in the UI and documentation already.

I also like both Approved and Visible in spite of the cons listed here -- the only one I feel strongly against is Default.

docwilmot commented 4 years ago

I had forgotten about the naming of the things.

I think previous decisions were made from the point of view of a workflow, hence "approved". It probably sounds a bit less acceptable now without the workflow component associated.

I think I was the only person who commented against "Active" but really I have no major issues with this. "Default" came over from the D7 code I based this on.

So "Active" then? Shall I let this stew for a few more hours till @klonos and @stpaultim wake up? Or change it and we merge it quick before they wake up? (There are advantages to both options) 😄

jenlampton commented 4 years ago

Or change it and we merge it quick before they wake up?

How about we change it, merge it, and then when they wake up we can file a follow-up PR if they feel strongly :)

docwilmot commented 4 years ago

Changes up in PR.

quicksketch commented 4 years ago

Thanks! Looking better. More comments: https://github.com/backdrop/backdrop/pull/3124#pullrequestreview-400448740

stpaultim commented 4 years ago

Sounds good to me. I have thoughts on this topic, but no really strong opinions, especially from the point of view of the API.

quicksketch commented 4 years ago

Looks even better now. I'd like to see the code comments consistently refer to the active revision rather than default or current: https://github.com/backdrop/backdrop/pull/3124#pullrequestreview-400555533

docwilmot commented 4 years ago

Changes made, including a few more that were missed. Only one unrelated date timezone fail on sandbox.

quicksketch commented 4 years ago

Looks ready to me. :100:

Going to let this sit for either another core committer or give it a day or two before re-reviewing.

quicksketch commented 4 years ago

I re-reviewed again and it all looks great still. I've merged https://github.com/backdrop/backdrop/pull/3124 into 1.x for 1.16.0! Thanks @stpaultim for advocating and wrangle us. Thanks @docwilmot for your patience and tireless iterations on this work.

I'm stoke about the possibilities this opens up for contrib and future core work. Review of revisions before publishing? Scheduled revisions? Both now much more accessible with these improvements.