ViewsQueryAlter should use deleted = 0 in where clause

Patch attached in #2 should fix the issue in my case at least by creating an OR-condition group comparing both NULL and 0 values.

@jwwj, the deleted column you saw in SqlContentEntityStorageSchema::getDedicatedTableSchema() is the one provided for all dedicated field tables by core's SQL schema, it's not the deleted column added for the field provided by the Trash module. Trash adds its deleted field in trash_entity_base_field_info() as a single-value base field, which means it is stored in the entity base tables, for example node_field_data and node_field_revision for nodes.

Is there a specific problem that you bumped into with the deleted field?

I'm pretty sure that if there was an actual problem you bumped into, it was probably 🐛 Views queries are not properly altered Fixed . Closing for now but please re-open if that's not the case.

I'm going to have to re-open this issue, because I think there is still a problem, though perhaps not quite related to my original assumption. Based on the description in comment #3, this module should only look at deleted columns in base field definitions? So in the case of a node content type, it should only be looking at node_field_data.deleted, which is the column that has been added by trash_entity_base_field_info()? It should not be looking at deleted columns in any fields connected to the content entity, e.g. node__field_image. Am I correct?

If my above understanding is correct, then if the view query already contains a relationship to a node__field*, in my case for instance because of a custom filter that adds a relationship to a field__xx table, then ViewsQueryAlter will also process that table as if it's a base field definition. So to me it looks like ViewsQueryAlter is processing field__* -tables when it shouldn't. But I don't have a good enough understanding of how view queries work to be sure, or have an idea of how to fix it...

Basically my problem is that I have a custom view filter which adds a relationship to another table to the view query. This table is a field definition table for nodes, called node__field_loger. node__field_loger contains a deleted column as defined by Drupal core's SQL schema (which is how I understand it based on comment #3). When I enable the Trash module and run the query, it adds a check for null values against node__field_loger.deleted:

... AND ("node__field_loger"."deleted" IS NULL) ...

But since this field is not actually the field that trash adds to the base definition and it contains the integer 0, not a NULL value, the check will always fail and thus no results are returned. So based on this, I think the way in which the Trash module determines which tables it should take into account when adding the NULL check is wrong. I guess it should somehow resolve the base field definition to which it has added the "deleted" column, and not just scan for all tables in the query relationships that have a "deleted" column (which is what it seems to be doing in alterQueryForEntityType). But again, my understanding of view queries and also the code of this module is not quite good enough to be sure...

@jwwj, is it possible to post the code of that custom filter in a way that doesn't expose any site-specific information? I think I understand the problem now, but I need to be able to replicate it locally in order to work on a fix :)

Here is code for a sample filter based on our source. I've tried to simplify it as much as possible, but it probably does not work as a drop-in somewhere. But it shows how we create the relationship to the node tables. Hope this helps.

namespace Drupal\example_mod\Plugin\views\filter;

use Drupal\Core\Entity\EntityStorageInterface;
use Drupal\Core\Entity\EntityTypeManagerInterface;
use Drupal\Core\Plugin\ContainerFactoryPluginInterface;
use Drupal\Core\Session\AccountProxyInterface;
use Drupal\views\Views;
use Symfony\Component\DependencyInjection\ContainerInterface;

/**
 * @ingroup views_filter_handlers
 *
 * @ViewsFilter("example_mod_write_access_filter")
 */
class ExampleFilter extends LogeAccessFilterBase implements ContainerFactoryPluginInterface {

  protected EntityStorageInterface $logeStorage;

  /**
   *
   */
  public function __construct(array $configuration,
  $plugin_id,
  $plugin_definition,
  AccountProxyInterface $current_user,
  EntityTypeManagerInterface $entity_type_manager) {
    parent::__construct($configuration, $plugin_id, $plugin_definition, $current_user);

    $this->logeStorage = $entity_type_manager->getStorage('loge');
  }

  /**
   * {@inheritdoc}
   */
  public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) {
    // @see \Drupal\Core\Field\FormatterPluginManager::createInstance().
    return new static(
      $configuration,
      $plugin_id,
      $plugin_definition,
      $container->get('current_user'),
      $container->get('entity_type.manager'),
    );
  }

  /**
   *
   */
  public function query() {

    $roles = $this->currentUser->getRoles();

    if ($this->hasDefaultWriteAccess($roles)) {
      return;
    }

    $readWrites = $this->normalizeReadWriteByRoom($this->extractReadWriteRolePrefix($roles));

    if (empty($readWrites)) {
      $readWrites = ['DenyAccessRole'];
    }

    // Join from event unit reference to daily availability entity.
    $configuration = [
      'table' => 'node__field_availability_daily',
      'field' => 'field_availability_daily_target_id',
      'left_table' => 'unit_bat_event__event_bat_unit_reference',
      'left_field' => 'id',
    ];

    $join = Views::pluginManager('join')->createInstance('standard', $configuration);
    $rel = $this->query->addRelationship('node__field_availability_daily', $join, 'unit');

    // Join from daily availability entity to bookable entity (the actual room).
    $configuration = [
      'table' => 'node__field_loger',
      'field' => 'entity_id',
      'left_table' => 'node__field_availability_daily',
      'left_field' => 'entity_id',
    ];

    $join = Views::pluginManager('join')->createInstance('standard', $configuration);
    $rel = $this->query->addRelationship('node__field_loger', $join, 'node');

    // Join from bookable entity to loge entity.
    $configuration = [
      'table' => 'loge',
      'field' => 'id',
      'left_table' => 'node__field_loger',
      'left_field' => 'field_loger_target_id',
    ];

    $join = Views::pluginManager('join')->createInstance('standard', $configuration);
    $rel = $this->query->addRelationship('loge', $join, 'loge');

    $this->ensureMyTable();

    // Join to Loge entity, check IN on ad_prefix and readWrites.
    $this->operator = 'in';
    $group = $this->options['group'];
    $this->query->addWhere($group, 'loge.ad_prefix', $readWrites, $this->operator);

  }

  /**
   *
   */
  protected function normalizeReadWriteByRoom(array $readWrites) :array {

    $loges = $this->logeStorage->loadByProperties(['ad_prefix' => $readWrites]);
    $rtn = [];
    $lastRoom = NULL;
    foreach ($loges as $loge) {

      if (empty($loge->get('field_standard_room')->entity)) {
        continue;
      }

      $roomId = $loge->get('field_standard_room')->entity->id();

      if ($roomId === $lastRoom) {
        continue;
      }
      $lastRoom = $roomId;
      if (!in_array($loge->getADPrefix(), $rtn)) {
        $rtn[] = $loge->getADPrefix();
      }

    }

    return $rtn;
  }

}

I've tried writing a custom filter similar to the one posted in #9 using "standard" entity reference fields, but I wasn't able to reproduce the situation where a field table was considered an entity base table.

This is the filter I tried:

/**
 * @ingroup views_filter_handlers
 *
 * @ViewsFilter("trash_test")
 */
class TrashTest extends FilterPluginBase {

  public function query() {
    // Join from daily availability entity to bookable entity (the actual room).
    $configuration = [
      'table' => 'node__field_tags',
      'field' => 'entity_id',
      'left_table' => 'node__field_image',
      'left_field' => 'entity_id',
    ];

    $join = Views::pluginManager('join')->createInstance('standard', $configuration);
    $rel = $this->query->addRelationship('node__field_tags', $join, 'node');

    // Join from bookable entity to loge entity.
    $configuration = [
      'table' => 'media',
      'field' => 'mid',
      'left_table' => 'node__field_media',
      'left_field' => 'field_media_target_id',
    ];

    $join = Views::pluginManager('join')->createInstance('standard', $configuration);
    $rel = $this->query->addRelationship('media', $join, 'media');

    $this->ensureMyTable();
  }

}

and defined like this in \Drupal\node\NodeViewsData:

    $data['node_field_data']['trash_test'] = [
      'title' => $this->t('Trash test'),
      'help' => $this->t('ABCD.'),
      'filter' => [
        'id' => 'trash_test',
        'label' => $this->t('XYZ'),
      ],
    ];