Replace all uses of "forward revision" with "pending revision"

@timmillwood, Thanks for the patch, it replaces the word & there is no such place is remaining for a change. I checked your patch manually by applying in files. PFA.

@timmillwood, Thanks for the updated patch it works well with all changed instances, I checked your patch manually by applying it module files.
PFA

What happened to the second part of #8? :)

@timmillwood, Thanks!.. We'll take care. Also, I attached image just for the confirmation that we performed RTBC & patch works well for us.

Thanks again for future guidance.

Consider this signed-off, it makes a lot of sense to me.

+++ b/core/modules/content_moderation/src/ModerationInformation.php
@@ -121,7 +121,7 @@ public function isLatestRevision(ContentEntityInterface $entity) {
-  public function hasForwardRevision(ContentEntityInterface $entity) {
+  public function hasDraftRevision(ContentEntityInterface $entity) {
     return $this->isModeratedEntity($entity)

+++ b/core/modules/content_moderation/src/ModerationInformationInterface.php
@@ -102,15 +102,15 @@ public function getDefaultRevisionId($entity_type_id, $entity_id);
-  public function hasForwardRevision(ContentEntityInterface $entity);
+  public function hasDraftRevision(ContentEntityInterface $entity);

+++ b/core/modules/content_moderation/src/ParamConverter/EntityRevisionConverter.php
@@ -41,23 +41,23 @@ public function __construct(EntityManagerInterface $entity_manager, ModerationIn
-  protected function hasForwardRevisionFlag(array $definition) {
-    return (isset($definition['load_forward_revision']) && $definition['load_forward_revision']);
+  protected function hasDraftRevisionFlag(array $definition) {
+    return (isset($definition['load_draft_revision']) && $definition['load_draft_revision']);

+++ b/core/modules/content_moderation/src/Routing/EntityModerationRouteProvider.php
@@ -86,7 +86,7 @@ protected function getLatestVersionRoute(EntityTypeInterface $entity_type) {
-            'load_forward_revision' => 1,
+            'load_draft_revision' => 1,

I was gonna say "but these are BC breaks", not realizing that all of this is in the experimental content_moderation module. Which means this is fine. Yay — no dealing with messy BC layers!

RTBC++

If #2875154: Clarify and document wording around drafts, revisions, published & friends is still so early in discussion, how do we know if this is RTBC? I see Bojhan in #15 saying it is signed off. However, let's consider the implications for a bit. With the terminology on this issue, you can have a non-draft revision that is a draft ;) That is, you have an unpublished default revision (AKA draft) yet you don't have any draft revisions (AKA forward revisions).

I agree with #17, let's wait for a conclusion in #2875154: Clarify and document wording around drafts, revisions, published & friends.

@Bojhan asked for clarification on #17. His point was that the user does not necessarily need to get different terminology based on whether the draft has a prior published variant or not. That is a good point. So my point in #17 may be moot. However, we realized another thing in the meantime...

Draft is a workflow state included in core (alongside Archived and Published). Let's say I also add a state "Approved for translation (before publication)" which is not yet published but is more reviewed than Draft (goes between Draft and Published). Now I can have a forward revision that is "Approved for translation (before publication)" and is not a "Draft". Is this a draft revision?

@dhwani.addweb: as @timmillwood mentioned, posting screenshots of your codebase is not helpful, since the automated testing infrastructure tells us whether the patch applies correctly. So, I've removed your credit for this issue. In the future, you can get credit for issues by reading the issue to understand its purpose, and posting your review or testing of that purpose, not of whether the patch applies. Thanks!

I think it would be good to move the discussion of #17 and #19 on #2875154: Clarify and document wording around drafts, revisions, published & friends. I was also going to suggest including screenshots from this patch, but I've read it twice now and I don't actually see a single user-facing string change from this issue. So maybe not. :)

Ah. I think the reason this is happening first is that this rename is a BC break. As far as I can see this patch doesn't include any user-facing changes, at all. So the question is whether we want to be stuck providing BC for forward_revision in the API and machine names for things forever. I think that's why this issue was split off.

We seem to have unanimous support for replacing 'forward revision' with 'pending revision', so let's do that :)

Looks great!

I can still find quite some forward revision occurrences in the core directory:

./modules/book/src/Plugin/Validation/Constraint/BookOutlineConstraint.php: * Validation constraint for changing the book outline in forward revisions.
./modules/book/src/Plugin/Validation/Constraint/BookOutlineConstraintValidator.php: * Constraint validator for changing the book outline in forward revisions.
./modules/book/tests/src/Kernel/BookForwardRevisionTest.php: * Tests that the Book module handles forward revisions correctly.
./modules/book/tests/src/Kernel/BookForwardRevisionTest.php:   * Tests forward revision handling for books.
./modules/book/tests/src/Kernel/BookForwardRevisionTest.php:    // forward revisions.
./modules/book/tests/src/Kernel/BookForwardRevisionTest.php:    $this->assertFalse($book_manager->updateOutline($child), 'A forward revision can not change the book outline.');
./modules/book/tests/src/Kernel/BookForwardRevisionTest.php:    // forward revisions.
./modules/book/tests/src/Kernel/BookForwardRevisionTest.php:    $this->assertFalse($book_manager->updateOutline($child), 'A forward revision can not change the book outline.');
./modules/book/tests/src/Kernel/BookForwardRevisionTest.php:    // forward revisions.
./modules/book/tests/src/Kernel/BookForwardRevisionTest.php:    $this->assertFalse($book_manager->updateOutline($child), 'A forward revision can not change the book outline.');
./modules/content_moderation/src/EntityOperations.php:    // - The entity has forward revisions.
./modules/content_moderation/src/ModerationInformationInterface.php:   * Determines if forward revisions are allowed.
./modules/content_moderation/src/ModerationInformationInterface.php:   *   If forward revisions are allowed.
./modules/content_moderation/tests/src/Functional/ModerationFormTest.php:    // Add french forward revision (revision 3).
./modules/content_moderation/tests/src/Functional/ModerationFormTest.php:    // Publish the french forward revision (revision 4).
./modules/content_moderation/tests/src/Functional/ModerationFormTest.php:    // Add a english forward revision (revision 6).
./modules/content_moderation/tests/src/Functional/ModerationFormTest.php:    // We should be able to publish the english forward revision (revision 7)
./modules/content_moderation/tests/src/Functional/ModerationFormTest.php:    // Add a forward revision (revision 2).
./modules/content_moderation/tests/src/Functional/ModerationFormTest.php:    // It shouldn't be possible to translate as we have a forward revision.
./modules/content_moderation/tests/src/Kernel/EntityStateChangeValidationTest.php:    // Create a forward revision of the original node.
./modules/menu_ui/src/Plugin/Validation/Constraint/MenuSettingsConstraint.php: * Validation constraint for changing the menu settings in forward revisions.
./modules/menu_ui/src/Plugin/Validation/Constraint/MenuSettingsConstraintValidator.php: * Constraint validator for changing the menu settings in forward revisions.
./modules/menu_ui/src/Plugin/Validation/Constraint/MenuSettingsConstraintValidator.php:      // Handle the case when a menu link is added to a forward revision.
./modules/menu_ui/src/Plugin/Validation/Constraint/MenuSettingsConstraintValidator.php:      // Handle the case when the menu link is deleted in a forward revision.
./modules/menu_ui/src/Plugin/Validation/Constraint/MenuSettingsConstraintValidator.php:      // Handle all the other menu link changes in a forward revision.
./modules/menu_ui/tests/src/Functional/MenuUiContentModerationTest.php:    // Create a forward revision with no changes.
./modules/path/src/Plugin/Validation/Constraint/PathAliasConstraint.php: * Validation constraint for changing path aliases in forward revisions.
./modules/path/src/Plugin/Validation/Constraint/PathAliasConstraintValidator.php: * Constraint validator for changing path aliases in forward revisions.
./modules/path/tests/src/Functional/PathContentModerationTest.php:    // Add a forward revision with the same alias.
./modules/path/tests/src/Functional/PathContentModerationTest.php:      'title[0][value]' => 'forward revision',
./modules/path/tests/src/Functional/PathContentModerationTest.php:    // Add a forward revision with a new alias.
./modules/path/tests/src/Functional/PathContentModerationTest.php:      'title[0][value]' => 'forward revision',
./modules/path/tests/src/Functional/PathContentModerationTest.php:    // Add a forward revision with no path alias.
./modules/path/tests/src/Functional/PathContentModerationTest.php:      'title[0][value]' => 'forward revision',

I also found a few occurrences of "draft revision" that would be worth to replace or at least validate:

./modules/book/tests/src/Functional/BookContentModerationTest.php:    // Save a new draft revision for the node without any changes and check that
./modules/content_moderation/src/EntityTypeInfo.php:          $message = $this->t('<a href="@latest_revision_edit_url">Publish</a> or <a href="@latest_revision_delete_url">delete</a> the latest draft revision to allow all workflow transitions.', $args);
./modules/content_moderation/src/EntityTypeInfo.php:          $full_message = $this->t('Unable to save this @type_label. <a href="@latest_revision_edit_url">Publish</a> or <a href="@latest_revision_delete_url">delete</a> the latest draft revision to allow all workflow transitions.', $args);

Working on those missing items.

This should be complete. The occurrences mentioned in #29 were UI text and a test actually creating a node in "draft" state, so they are ok IMO.

RTBC + 1

So even though this doesn't make any user-facing changes, I wanted to verify that we were heading in a direction that would match labeling that might be exposed to users in the future. A usability maintainer hasn't commented on this issue, but @yoroy did post on #2875154: Clarify and document wording around drafts, revisions, published & friends:

Discussed this during todays UX meeting. Some potential guidelines could be:

1. Use "version" in user facing text. Do not use "revision" in user facing text. It could stay "revision" in code/comments
2. Use the word "Draft" only to indicate a moderation state. Do not use it to indicate how this entity relates to the default one. So "pending version" instead of "pending draft".

We liked "pending" better than "forward", it's clearer and more specific.

I *think* this mostly supports the suggestions in #2. I now realise we did not discuss "default".

Does this help? Do these 2 guidelines hold up or where could they break?

So that supports this change. Given that @amateescu also agreed in #23, we have the consensus of maintainers for Field, Entity, Content Moderation, and usability. Sounds like a quorum. :)

I didn't review the patch fully yet, but wanted to document my understanding of the signoffs first in case anyone else looks at it or has questions.

+++ b/core/modules/content_moderation/src/ModerationInformationInterface.php
@@ -109,9 +109,9 @@ public function getAffectedRevisionTranslation(ContentEntityInterface $entity);
-  public function isForwardRevisionAllowed(ContentEntityInterface $entity);
+  public function isPendingRevisionAllowed(ContentEntityInterface $entity);

@@ -126,15 +126,15 @@ public function isForwardRevisionAllowed(ContentEntityInterface $entity);
-  public function hasForwardRevision(ContentEntityInterface $entity);
+  public function hasPendingRevision(ContentEntityInterface $entity);

These are API changes; probably we should have a change record?

Everything else is tests and code comments.

I grepped for "forward" and I think I found some that were missed in:

core/modules/book/tests/src/Kernel/BookPendingRevisionTest.php
core/modules/content_moderation/tests/src/Functional/ModerationFormTest.php
core/modules/content_moderation/tests/src/Kernel/EntityOperationsTest.php
core/modules/content_moderation/tests/src/Kernel/EntityStateChangeValidationTest.php
core/modules/content_moderation/tests/src/Unit/LatestRevisionCheckTest.php
core/modules/taxonomy/tests/src/Kernel/PendingRevisionTest.php
core/tests/Drupal/KernelTests/Core/Entity/EntityRevisionTranslationTest.php

These are API changes; probably we should have a change record?

Content Moderation is still experimental, at least until 8.4.0-alpha1, and I don't think we write change records for experimental modules, do we?

Fixed all the other instances that you found :)

Interdiff looks good, tentatively moving back to RTBC.

+++ b/core/modules/taxonomy/tests/src/Kernel/PendingRevisionTest.php
@@ -92,8 +92,7 @@ public function testTaxonomyIndexWithPendingRevision() {
-    // Check that saving a forward (non-default) revision does not affect the
-    // taxonomy index.
+    // Check that saving a pending revision does not affect the taxonomy index.

yay :)

Adding xjm to issue credit for grepping for other instances.

Is there a resolution on #2875154: Clarify and document wording around drafts, revisions, published & friends yet? This was postponed on that but it still seems active.

If so, can we get an issue summary update over there and an RTBC (as its a policy-no-patch issue).

Updating the interface guidelines on drupal.org can then occur etc.

@larowlan, the parent issue is still open for debate on the 'revision' vs. 'version' and it probably won't make it to RTBC anytime soon, but we did get unanimous consensus for changing 'forward revision' to 'pending revision', see the second to last paragraph from #35.

Thanks @amateescu I'll update IS over there to include that.

Rerolled.

grep -ir --color --exclude-dir=*node_modules* forward ./core/|grep -i revision

...no results.

Committed as aee577b and pushed to 8.5.x.
Cherry-picked as 566d673 and pushed to 8.4.x.

We chose to call this out in the release notes since it could impact contrib authors who make reference to pending revisions.