Regression: _entity_form routes should not get load_latest_revision_flag by default: changes default behavior (= regression) while only Content Moderation needs it

Please credit @hchonov and @plach.

1. +++ b/core/modules/content_moderation/src/Routing/ContentModerationRouteSubscriber.php
   @@ -0,0 +1,104 @@
   + * Subscriber for entity translation routes.
   

   s/translation/form/

   Or better yet:

   revisionable entity forms

2. +++ b/core/modules/content_moderation/src/Routing/ContentModerationRouteSubscriber.php
   @@ -0,0 +1,104 @@
   +    $events[RoutingEvents::ALTER] = ['onAlterRoutes', 10];
   

   Why 10? Can't this be 0? If not, let's document why.

1. +++ b/core/modules/content_moderation/content_moderation.services.yml
   @@ -20,3 +20,8 @@ services:
   +  content_moderation.subscriber:
   +    class: Drupal\content_moderation\Routing\ContentModerationRouteSubscriber
   +    arguments: ['@entity_type.manager']
   
   +++ b/core/modules/content_moderation/src/Routing/ContentModerationRouteSubscriber.php
   @@ -0,0 +1,113 @@
   + * @internal
   

   The class is marked @internal, great. Can we also set public: false then?

2. +++ b/core/modules/content_moderation/src/EntityOperations.php
   @@ -133,6 +146,9 @@ public function entityUpdate(EntityInterface $entity) {
   +    elseif ($entity instanceof Workflow && $entity->getTypePlugin()->getPluginId() == 'content_moderation') {
   +      $this->routerBuilder->rebuild();
   +    }
   

   So … whenever a Workflow entity is updated, we must rebuild routes.

   First: this should use ::setRebuildNeeded() AFAICT.

   Second: why do we even need to do this? Why is the route subscriber alone not enough? I think it's because the route subscriber only sets that load_latest_revision flag on entity types for which a Workflow entity exists that dictates a content_moderation-powered workflow, right? It is not trivial to connect those dots. It'd be good to have a @see \F\Q\C\N::getEntityTypes(), that'd help connect those dots.

3. +++ b/core/modules/content_moderation/src/Routing/ContentModerationRouteSubscriber.php
   @@ -0,0 +1,113 @@
   + * Subscriber for revisionable entity forms.
   

   Still not accurate, apparently it's only for moderated revisionable entity forms?

4. +++ b/core/modules/content_moderation/src/Routing/ContentModerationRouteSubscriber.php
   @@ -0,0 +1,113 @@
   +   * Returns the moderated entity types.
   ...
   +  protected function getEntityTypes() {
   

   getModeratedEntityTypes()

5. +++ b/core/modules/content_moderation/tests/src/Unit/ContentModerationRouteSubscriberTest.php
   @@ -0,0 +1,219 @@
   +   * {@inheritdoc}
   +   *
   +   * @covers ::__construct
   

   This is highly unusual. A ::setUp() with a @covers. Is this really correct?

6. +++ b/core/modules/content_moderation/tests/src/Unit/ContentModerationRouteSubscriberTest.php
   @@ -0,0 +1,219 @@
   +      'Multiple entity parameters on an entity form' => [
   

   Nice edge case, I didn't even realize this was possible!

7. +++ b/core/modules/content_moderation/tests/src/Unit/ContentModerationRouteSubscriberTest.php
   @@ -0,0 +1,219 @@
   +      'Non-revisionable entity type will not change' => [
   

   Glad to see this edge case tested.

   But I'm not seeing test coverage for an entity form for a non-moderated entity type?

+++ b/core/modules/content_moderation/tests/src/Unit/ContentModerationRouteSubscriberTest.php
@@ -110,6 +108,21 @@ public function setLatestRevisionFlagTestCases() {
+        [
+          'entity_test_mulrev' => [
+            'type' => 'entity:entity_test_mulrev',
+          ],
+        ],
+        [
+          'entity_test_mulrev' => [
+            'type' => 'entity:entity_test_mulrev',
+          ],
+        ],

We're testing that they're not affected. In other test cases where we're testing that a case is unaffected, we don't pass that final array, because it is the same. We should do the same here.

Other than that, this looks great.

👍

But why is this prioritized as Critical?

Because without this landing before 8.5.0, we'll have shipped a behavior change and hence a BC break.