Revision entity queries shouldn't return deleted entities

Created on 7 February 2024, 11 months ago
Updated 8 April 2024, 9 months ago

Problem/Motivation

Might be not an issue as this is by-design, but I can see

      // Loading a revision is explicit. So when we try to load one we should do
      // so without a condition on the deleted flag.

A somehow common pattern (that in this case produced a bug on my site when Trash is used with lightning_scheduler module):

    $IDs = $storage->getQuery()
      ->latestRevision()
      ->accessCheck(FALSE)
      ->condition('scheduled_transition_date.value', $now, '<=')
      ->execute();

    foreach (array_keys($IDs) as $revision_id) {
      yield $storage->loadRevision($revision_id);
    }

On a trashed content, query returns valid revisions, but the storage won't be able to load them.

I can fix that with

  if (Drupal::moduleHandler()->moduleExists('trash') && \Drupal::hasService('trash.manager')) {
    \Drupal::service('trash.manager')->setTrashContext('ignore');
  }

But that implies that any contrib module needs to explicitly integrate with trash.

Steps to reproduce

  1. With lightning_scheduler, schedule a transition in the past.
  2. Trash that content.
  3. Run cron.

Proposed resolution

Confirm this is by design.
Maybe we need to document this somewhere?
Maybe by default we should not return revisions for trashed content on entity queries?

Remaining tasks

User interface changes

API changes

Data model changes

🐛 Bug report
Status

Fixed

Version

3.0

Component

Code

Created by

🇪🇸Spain penyaskito Seville 💃, Spain 🇪🇸, UTC+2 🇪🇺

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Production build 0.71.5 2024