Allow entity query to query the latest revision

In which use case do you want to query only the latest draft revision? What if you have two draft revisions after the default one - do you still want to query only the last one?

In which use case do you want to query only the latest draft revision?

For example when you are building a list of moderated entities (without Views) and you need to show the latest revisions instead of the default ones.

What if you have two draft revisions after the default one - do you still want to query only the last one?

Of course :)

You can see why this is needed in the interdiff from #2902187-35: Provide a way for users to moderate content (hint: ModeratedNodeListBuilder::load() from that interdiff).

Just like in #2865579: Rewrite the 'Latest revision' views filter and remove the revision_tracker table, using the query suggested by @joachim in #2784201-9: Track the latest_revision and latest_revision_translation_affected ID in the entity data table for revisionable entity types works nicely for entity queries too :)

This is how a simple query without any conditions looks like:

SELECT base_table.vid AS vid, base_table.nid AS nid
FROM 
{node_revision} base_table
LEFT OUTER JOIN {node_revision} base_table_2 ON base_table.nid = base_table_2.nid AND base_table.vid < base_table_2.vid
WHERE base_table_2.nid IS NULL

Conditions work just fine as well, as can be seen in the tests added by the patch.

What doesn't work, however, is querying for the latest revision of a *relationship*, i.e. the value of a field from the latest revision of a referenced entity. Consider this scenario:

- The node entity type has an entity reference field to media entities
- There is a media item with revision 1 (name: 'published', default revision) and revision 2 (name: 'draft', pending revision)
- A node is referencing that media item

The following query will not return any results:

\Drupal::entityTypeManager()->getStorage('node')->getQuery()
  ->latestRevision()
  ->condition('field_media.entity.name', 'draft', 'CONTAINS')
  ->execute();

But this one will return the correct result:

\Drupal::entityTypeManager()->getStorage('node')->getQuery()
  ->latestRevision()
  ->condition('field_media.entity.name', 'published', 'CONTAINS')
  ->execute();

This is the query generated in this case:

SELECT base_table.vid AS vid, base_table.nid AS nid
FROM 
{node_revision} base_table
LEFT OUTER JOIN {node_revision} base_table_2 ON base_table.nid = base_table_2.nid AND base_table.vid < base_table_2.vid
INNER JOIN {node_revision__field_media} node_revision__field_media ON node_revision__field_media.revision_id = base_table.vid
LEFT OUTER JOIN {media} media ON media.mid = node_revision__field_media.field_media_target_id
INNER JOIN {media_field_revision} media_field_revision ON media_field_revision.vid = media.vid
WHERE (base_table_2.nid IS NULL) AND (media_field_revision.name LIKE :db_condition_placeholder_0 ESCAPE '\\')

And this is the problematic part:

LEFT OUTER JOIN {media} media ON media.mid = node_revision__field_media.field_media_target_id
INNER JOIN {media_field_revision} media_field_revision ON media_field_revision.vid = media.vid

We are joining the base table for media instead of the revision table, so the condition for the second join (ON media_field_revision.vid = media.vid) will use the revision ID of the default revision.

But QueryInterface::allRevisions() behaves in the same way so I don't think we should be concerned by this case.

Oops, forgot a debug statement in there :)

Can't see any reason not to RTBC.

That was a random fail, back to RTBC.

Sorry, I have two questions:

1. +++ b/core/lib/Drupal/Core/Entity/Query/QueryInterface.php
   @@ -255,6 +255,13 @@ public function orConditionGroup();
   +   * Queries the latest revision.
   

   I think some documentation what "latest revision" means compared to "current revision" would be helpful?

2. +++ b/core/lib/Drupal/Core/Entity/Query/QueryInterface.php
   @@ -255,6 +255,13 @@ public function orConditionGroup();
   +  public function latestRevision();
   

   Isn't this a BC break?

Thanks for the review :)

#11.1: I don't think the entity query interface is the best place to document the difference between the default (a.k.a. current) revision and the latest revision, a documentation page on d.o would be much more helpful. But I tried my best and write a short paragraph for it.

#11.2: Nope, we are allowed to add methods to interfaces in minor releases, see https://www.drupal.org/core/d8-bc-policy#interfaces

I forgot this was allowed.

Looks like a reasonable addition.

+++ b/core/lib/Drupal/Core/Entity/Query/QueryInterface.php
@@ -248,13 +248,27 @@ public function andConditionGroup();
-   * Queries the current revision.
+   * Queries the default revision.
    *
    * @return $this
    */
   public function currentRevision();

This looks borderline out-of-scope-ish, though I see it was added in #11 to clarify the difference between this and latest revision; should we also deprecate currentRevision() and add defaultRevision() as a more explicit name? Could be a followup and we could stick a @todo on the method here.

+++ b/core/lib/Drupal/Core/Entity/Query/QueryInterface.php
@@ -248,13 +248,27 @@ public function andConditionGroup();
+   *
+   * The difference between the default revision (which is also referred to as
+   * the "current" revision) and the latest revision is that the default
+   * revision is the latest version of an entity in a published state, while the
+   * latest revision can be either in a published or unpublished state. This
+   * means that if the latest version of an entity is in a published state, the
+   * default (current) revision and the latest revision are the same.
+   *

I'm not sure this explains clearly enough what's going on, generally when trying to explain this, I avoid the word 'published' as much as possible, since the default revision can be unpublished (status = 0) too.

Something like this instead maybe:

"The latest revision is the most recent revision of an entity. This will be either the default revision, or a draft revision if one exists newer than the default."

Implemented as per #15. Here is an updated patch and interdiff

Thanks @Dinesh18 :) The comment needed to be wrapped at 80 cols and since we're not mentioning the current revision anymore we can use undo the out-of-scope change mentioned by @xjm in #14.

The feedback has been addressed so back to RTBC.

+1 to RTBC

+++ b/core/lib/Drupal/Core/Entity/Query/QueryInterface.php
@@ -257,9 +257,9 @@ public function currentRevision();
+   * The latest revision is the most recent revision of an entity. This will be
+   * either the default revision, or a draft revision if one exists and it is
+   * newer than the default.

Didn't we settle on the "pending revision" terminology instead of draft?

Yep, we did :P

Ahh yes. That reads so much better with pending revision too.

Committed/pushed to 8.5.x and cherry-picked to 8.4.x. Thanks!

Awesome, can we add a change notice please?

@plach, sure thing, here it is: https://www.drupal.org/node/2918184

Thanks!

This adds a method to an interface, so I don't think it should have been backported as-is. We could make a backportable version by adding the method to the base class but not to the interface yet.

I'll wait to check with catch, but I think it should be reverted from 8.4.x for now.

Instead of reverting, this can be committed to 8.4.x if needed.

Thanks @amateescu; I committed and pushed the followup to 8.4.x.

I had a problem using `latestRevision()` today.
I am trying to get nodes that reference a specific nid.
my query is this:

$referencedNodes = \Drupal::entityTypeManager()->getStorage('node')->getQuery()
      ->latestRevision()
      ->condition('field_release.target_id', $thisId)
      ->execute();

The field_release (entity reference) value has changed between the current revision and the latest revision.
It seems that the condition is looking on the current revision rather than the latest revision.

Is this expected behavior?

In case other people find this via Google... This change caused an issue on a client site. On this client they had migrated from D7 to D8. They ran node migrations first, then later ran node revision migrations. This had the side effect where the published node revision id is smaller than the previous revision ids (because they were migrated later).

After upgrading to 8.5.x, when the client Edits a node, the form is loaded with the previous revision data (the highest revision id) rather than the current published data.

This matches the intent of the original issue, so makes sense. So Drupal core is probably doing the right thing here. Just be aware that this change can have consequences on existing sites where migrations were not done correctly.