Data integrity and uninstall issues with moderation state field

There is a related UX problem of disambiguation and logical inconsistency.

A moderation state which is "published" but not "live" on the site is possible and that seems just plain wrong.

I would suggest that the word "published" be reserved for status only, never workflow state.

Tagging as 'WI critical' - this is an issue we need to resolve properly before content_moderation is stable.

In a typical workflow, the Published state is the only state that will be set to mark an entity as "published". Also, bear in mind that WBM/CM doesn't apply just to nodes; it applies to any bundleable/revisionable content entity, and not all of those have their own published concept (block_content in core, for instance, does not).

"Approved" is another possible label, but that could also be confused with "Approved to get published later using the revision_scheduler module".

In a typical workflow, the Published state is the only state that will be set to mark an entity as "published".

This is the problem though, it's the atypical workflow that's going to produce ambiguity and potential confusion. We have a real semantic problem around the word published and the logic that underpins it right now. The commonly understood meaning of "Published" is "Visible to the public" if our logic dictates that there could be "published" nodes that are not visible to the public, either we need to use a different term or we need to restrict the use of that term only to situations where it matches the users mental model.

Basically there should never be a case where something is "published" but not "public"*
My suggestion would be:

1. Do not allow workflow states to be named "published"
2. Display the status with the state for published nodes eg. Draft, Live (Published), Archived
3. Do not allow states with a status of Published to be hidden (essentially unpublished).

*I understand that there is some ambiguity where the audience is entirely authenticated, like in an intranet where a node may be "published" but the meaning of "public" is restricted to members. This would preclude a programmatic solution but not a best-practices guideline where "public" is understood as "visible to the ultimate intended audience."

Special-casing certain State names, whether whitelist or blacklist, is an ugly slipper slope I'd rather avoid entirely. If someone wants to create a state named Published then I see no compelling reason to stop them. That doesn't have to be in the default configuration, of course, but blacklisting names just introduces more complexity, and a magic name that must always imply something else is even worse. And then, remember multi-lingual concerns. I don't think magic meaning for state names is viable or advisable.

Note that there are 2 possible "Extra meanings" on States: Mark published (if the entity has such a concept, and which we may also use for other logic), and Set as Default (ie, make the thing that $storage->load() returns if you don't ask for a specific revision). Do we need to show both of those in the parenthetical? Does that get too visually complicated? I'd say yes, but it's fair to ask. I'm not against (Published) in the display label if that ends up being the easiest solution.

Note that "Live" is also a word that sometimes has special meaning. I think I used it in WBM somewhere internally, but forget off the top of my head. (I think there's an isLive() method somewhere.) There was a discussion in the core queue a few years ago about what to call the "default" revision, and I don't think it ever came to resolution. I don't have the link handy, unfortunately.

Part of the issue here, as well as the uninstall issue mentioned in #2725533-67: Add experimental content_moderation module is due to using a field for this. I'm wondering if an entity type might be easier in the long run.

When a revision is the default one (and also published if the entity supports that concept) , then storing a 'moderation state' for it isn't really necessary - it's not in moderation at all then.

A separate entity would allow moderation state to be changed without actually updating the entity itself (while still maintaining moderation history since that entity could itself have revisions). Uninstall just deletes the entity type altogether.

Re-titling and bumping to major per #2725533-71: Add experimental content_moderation module.

Well, switching from a field to an internal content entity would make it internally more like WBM D7. However, that's a rather heavy refactoring, bordering on rewriting large swaths of the module. (I tried to abstract as much as I could, but I doubt I was THAT good.) That's a very not-small task.

Special-casing certain State names, whether whitelist or blacklist, is an ugly slipper [sic] slope I'd rather avoid entirely.

Ok, but what we have now is the diametric opposite, actually using published as both a default state and status name. Could we at least remove that and advise users against using existing status names as state names in the descriptive text?

Note that there are 2 possible "Extra meanings" ... Do we need to show both of those in the parenthetical? Does that get too visually complicated?

Yes it is too visually complicated and what's more, it's mentally complicated. This points to the issue that we have not sufficiently clarified to the author the fact that there are multiple revisions and that the one that opens when you click "edit" may not be the one that is published. This is a really big problem, which relates to your comment:

There was a discussion in the core queue a few years ago about what to call the "default" revision, and I don't think it ever came to resolution.

I think that the nature of the split between what is "live" and what is "Most recently worked on" needs to be explicitly addressed in the drop-buttons at /content. My suggestion would be:

[edit latest revision] [edit published version] [view other revisions] [delete].

This way we are not arbitrarily choosing which revision to open based on some logic that is opaque to the user.

As to what to call it, that becomes less important if we remove programmatic selection of the default revision because then "default" has no effective logic. There's no need for a "default" if the it's the user—not the system—that chooses which revision to open. FWIW the label I use above, "Latest revision" is the one I think makes most sense. "Default" is out, and anything with "current" invites confusion with "Published."

Note that I opened an issue #2753673: Add status and workflow state to page title block. to address the lack of clear messaging about what the status an state of the revision being edited are, but that still only applies to the revision that has already been opened and does not address the problem above.

Kevin: Editing from the published revision MUST be forbidden. That results in data loss. Editing from the latest revision is the one and only place that editing can happen, since revisions *are* linear. (The Workspaces module lets you create branched environments, but that's not what we're talking about here and isn't getting into core for a while.) The latest revision may or may not also be the default revision, but if you ever expose non-latest revisions for editing you will have data loss. Do not do that.

"Default" is the name we're stuck with, though. It's not my favorite, but it's all over the code and API and internal documentation. Calling it default in all APIs but something else in the UI would just confuse developers, as we've seen in the past, and the terms would still leak out and confuse users, too.

Better integrating the Default/Latest split into the /admin/content page makes sense. We could probably do that just by changing the label of "Edit" to "Edit latest revision", since in either case all it does is link to /node/X/edit, which loads the latest revision, which is what we want to happen.

However, that's not what I was getting at before. The "Statename (published)" label you suggested would be fairly easy to implement; just check if the state has the "make published" flag set, and if so append "(published)" to its name when displaying. I was referring more to the fact that there's a second flag on states, "make default", which is semi-independent of "make published." (Published implies default, but default does not imply published. If that weren't the case, Archived would be impossible.) But "Statename (published, default)" is a mouthful, I agree. So do we surface both flags in the display of states, and if so, how?

Or, the approach that WBM took in contrib, was to not do so and allow that to be defined by the site admin. They can pick states/transitions that are self-explanatory for their institution, and whose names imply published/default as contextually appropriate. In that case, having the only state whose "make published" flag is set to true also be named Published *makes a ton of sense*, because you're saying "yes, make this the published revision" (aka, live, aka what anon users see, etc.)

Calling it default in all APIs but something else in the UI would just confuse developers, as we've seen in the past, and the terms would still leak out and confuse users, too.

Actually, no. The clear mandate from the massive Drupal survey was that we should be optimizing for content authors first, site builders second, and developers a far distant third. If clarifying the label of something in the UI dramatically helps the former two audiences, we should do it, despite developer confusion. We have other ways to address developer confusion, e.g. expanded docs.

@Crell

Kevin: Editing from the published revision MUST be forbidden.

Right, but we allow the published version to be made the current revision, correct? So what if we were to have "View published revision" and from there allow something like "Save as latest revision."

I was referring more to the fact that there's a second flag on states, "make default", which is semi-independent of "make published."

It's this "semi-independent" part that's going to be very confusing to users who are going to "semi-know" which revision they are actually working on, which is why I suggested "Edit latest revision".

...having the only state whose "make published" flag is set to true also be named Published *makes a ton of sense*, because you're saying "yes, make this the published revision" (aka, live, aka what anon users see, etc.)

Yes, but I'd suggest that we go a bit beyond just making that the default, because we need to allow for the case of a user who re-configures that state to be labeled something like "Available" that has more ambiguity around whether it is published or not. Perhaps for the default Published state, it would have both the state name "Published" and status of published, then in the state configuration UI provide a checkbox of "Use publishing status as state name." If unchecked both the State name and status would be shown, eg. Live (published).

case, having the only state whose "make published" flag is set to true also be named Published *makes a ton of sense*, because you're saying "yes, make this the published revision" (aka, live, aka what anon users see, etc.)

"Default" is the name we're stuck with, though. It's not my favorite, but it's all over the code and API and internal documentation.

Just to add to what @timmillwood and @webchick wrote, our effort to make Ui text more comprehensible we should strive to fully abstract UI text from code and APIs so that any changes we need to make can be done so easily.

Renaming the "Edit" tab/link to "Edit latest version" sounds fine to me. We agree on that. It's the "edit published version" link/button that needs to never exist.

I'm a bit wary of checkbox fatigue, but if adding a "show published state in state name" checkbox for each state is what we end up with, I won't raise a fuss.

@crell

...I won't raise a fuss.

Great! I mentioned this in UX meeting and it got no negative feedback.

Can this get some screenshots? I am not sure what all that implies.

@Bojhan, sadly, most of the comments here are actually unrelated to the task at hand. The work that was done in order to fix this issue did not touch the UI at all.

Also we haven't yet made the module uninstallable once you've moderated something because it content moderation content entities exist. @xjm has said that this is a release blocker. Maybe we should just add a form linked to for the module description that drops all the entities tables and just say this is a necessary pre-uninstall step. And then we can address content entity removal on uninstall in a generic way later - see #2688945: Allow removing a module's content entities prior to module uninstallation

I'm also wondering what happens with forward revisions on uninstall? Do you go back to be only able to edit the default revision...

I'm also wondering what happens with forward revisions on uninstall? Do you go back to be only able to edit the default revision...

I just tested this scenario in combination with the patch from #2688945: Allow removing a module's content entities prior to module uninstallation and that's exactly what happens, all the data of the forward revisions is kept in the database but you can only edit the default revision.

If that's not an acceptable state in which to leave a site after content_moderation is uninstalled, maybe we can set the last revision as the default revision. Or.. are there any other options?

Leaving forward revisions as they are after uninstall seems great (and don't see another way we could handle it either).

Yes with the new entity this is fine afaik. Closing as duplicate.