Make node aware of the latest_version route.

The Needs Review Queue Bot → tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Re-roll patch need to 11.x.

Rerolled for 11.x.

This will need test coverage. Why is the one test being removed?

The test that was removed was specifically testing the CM preprocessor which was removed so the test was no longer needed. Instead I've repurposed it to test the node_is_page function.

I was in 2 minds about doing this or 📌 Deprecate node_is_page and move into a service Active first but this might be easier to get in first.

Alternatively if we make it a service I guess CM could decorate it to add the condition to its own route 🤔

Can we update the issue summary to reflect the new scope and findings please.

Hiding patches

@smustgrave what exactly is missing from the IS? It explains exactly what we're doing?

left some comments, still marking as the changes to the test didn't break.

I don't think we should hardcode this. How about we add something to route so we have an API here that other modules could use.

Something like
if ($route_match->getRouteName() == 'entity.node.canonical' || $route_match->getRouteObject()->getOption('node.is_full_page_view')) {

And then in \Drupal\content_moderation\Entity\Routing\EntityModerationRouteProvider::getLatestVersionRoute() do ->setOption($entity_type_id . '.is_full_page_view', TRUE) on the route it returns. This way any entity that has similar needs to node can do things in the same way.

We should also update the docs in template_preprocess_node() as the following is not correct...

  // The 'page' variable is set to TRUE in two occasions:
  // - The view mode is 'full' and we are on the 'node.view' route.
  // - The node is in preview and view mode is either 'full' or 'default'.

We're actually trying to deprecate this variable entirely so I guess this should be postponed 📌 Deprecate $variables['page'] for node.html.twig Active

I do like the idea in #35 though!

Confirming we can get rid of all of this code once 📌 Deprecate $variables['page'] for node.html.twig Active is done, however CM doesn't seem to have any functional test coverage of what this was doing (i.e hiding the duplicate h2 heading on the latest version page).

I think in this issue we should add that test coverage. I've opened an MR to remove the associated code and will add the test coverage which will fail until 3458589 is merged and this is rebased.

acbramley → changed the visibility of the branch 2877924-make-node-aware to hidden.

This looks good, but as I commented as in the parent issue, we can only do this in D13 when the page variable actually gets removed, otherwise templates relying on the BC behavior would be broken with content moderation?

Should we split this up? Get the test coverage in now and then the removal later? We could do one postponed page variable cleanup issue and write down all the parts that can go then, because there's a lot and we might miss some bits otherwise (node itself + views/content moderation special cases + tests)