Problem/Motivation
Views relationships (joins) are missing the trash deleted check. This allows deleted content to show in views listings.
The current views query alter logic only adds the deleted check on the base table, but it should also do it for relationships.
Steps to reproduce
- Content type "Article" with an entity reference field "Related Articles" which allows referencing other article nodes.
- Trash enabled for Article.
- Create 3 articles: Article A (no references), Article B (references Article A), and Article C (references Article C)
- Create a view that will show a list of articles that reference the article being viewed. This requires a contextual filter for Content ID (the article being viewed), and a relationship "Content using field_related_articles". Then for fields to display, we display "Content: Title" and use the relationship.
- Delete (move to trash) Article B
- Using views preview, put in the ID of Article A for the contextual filter
Expectation: Only Article C is listed.
Reality: Article B and Article C are listed, even though Article B is in the trash.
This is the query produced:
SELECT node_field_data.created AS node_field_data_created, node_field_data.nid AS nid, field_related_articles_node_field_data.nid AS field_related_articles_node_field_data_nid
FROM node_field_data node_field_data
INNER JOIN node__field_related_articles node__field_related_articles ON node_field_data.nid = node__field_related_articles.field_related_articles_target_id AND node__field_related_articles.deleted = '0'
INNER JOIN node_field_data field_related_articles_node_field_data ON node__field_related_articles.entity_id = field_related_articles_node_field_data.nid
WHERE (((node_field_data.nid = '4')) AND (node_field_data.deleted IS NULL)) AND (node_field_data.status = '1')
ORDER BY node_field_data_created DESC
LIMIT 5 OFFSET 0
This is what it should be to exclude the deleted articles in the join:
SELECT node_field_data.created AS node_field_data_created, node_field_data.nid AS nid, field_related_articles_node_field_data.nid AS field_related_articles_node_field_data_nid
FROM node_field_data node_field_data
INNER JOIN node__field_related_articles node__field_related_articles ON node_field_data.nid = node__field_related_articles.field_related_articles_target_id AND node__field_related_articles.deleted = '0'
INNER JOIN node_field_data field_related_articles_node_field_data ON node__field_related_articles.entity_id = field_related_articles_node_field_data.nid AND field_related_articles_node_field_data.deleted IS NULL
WHERE (((node_field_data.nid = '4')) AND (node_field_data.deleted IS NULL)) AND (node_field_data.status = '1')
ORDER BY node_field_data_created DESC
LIMIT 5 OFFSET 0
Proposed resolution
I'm not very familiar with views query alters, but I think we need to somehow alter the join to add the additional condition for deleted is NULL check.
Remaining tasks
User interface changes
API changes
Data model changes
Issue fork trash-3549548
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
bkosborneI created an MR with what should be a failing test. It includes a view that demonstrates the problem.
I don't really know how to fix this problem yet though.
Note also that in my test I had to manually re-enable trash after executing a view. The Trash module is supposed to do this automatically in the a post render hook for views, but the tests never render a view, so the hook isn't called. This may lead to some hard to reproduce and diagnose bugs on sites that manually execute views without rendering their output. I created #3549607: Views alter may disable trash and never re-enable it, or re-enable it too late to address that.
Comment #4
bkosborneOkay I narrowed down the problem area. The views query alter already handles adding deleted conditions for relationships, but there's a logic issue if the relationship is to the same base table (same entity type). The query alter never adds the .deleted IS NULL condition because it thinks it was already added.
The problem is in the
hasDeleteConditioncode. It incorrectly thinks a delete condition already exists on the relationship because it's seeing that the delete condition was added for the main base table of the view. This method needs to be updated to check for the specific alias of the relationship I think.Comment #5
bkosborneOkay, I fixed this, but I'm not really sure if this will break something else. I couldn't figure out why some of the original code was written as it was. But this update is very explicit about checking if the deleted condition exists for the specific table alias now. The existing views test passes and the new one I added passes, but there wasn't much test coverage here for edge cases. I didn't test this with views based on revisions for example, though I think it should be fine?
Comment #6
amateescu commentedLooking at the issue where I wrote that code (#3393095: Trash breaks revision-based views and relation queries).. I can't figure out either :) But the change in this MR makes a lot of sense, we surely don't need to loop over all possible field table names of the deleted field if we're already looping through all the tables involved in the views query.
I also tested a revision view with the same setup described in the issue summary and it works fine.
Merged into 3.x, thanks!
Comment #9
bkosborneThank you!
Comment #10
twodI just found a View we where this new version crashes because it can't add in the field data table (
addTable()returns false and causes a type error on the next line).It's showing a custom entity type with a relation to a node.
I tried figuring out how this code is supposed to work - and realized I had actually part in creating it - but I just can't remember why it was this way. Maybe time to start from scratch? XD
Anway, I'll be investigating this a bit more, and see what comes out of it.
(Edit: The View has 7 joins prior to alteration, so it may take a while... ;_;)
Update: So far I've narrowed it down to 3 joins and it likely trying to join from the node revisions table back to the node field data table, which doesn't normally happen - and is why adding the table fails.
I've looked at how Views normally handles entities and their data tables (or falling back to the base table in case it doesn't exist) with and without revisions. It seems those tables will always be available, and since that is where the 'deleted' column lives, we may not have to join at all. The WIP code I have passes the existing query alter tests, doesn't crash, and avoids an extra join we don't really need. Unfortunately I had to put this on temporary hold today but will try to get back to it ASAP.
Comment #11
amateescu commented@twod, can you please check if the change from #3549900: Update from 3.0.18 to 3.0.20 breaks views with non-entity type relationships fixed the problem for you as well?
Comment #12
twod@amateescu, unfortunately, it still does not work. In our case the problem is that it's still trying to merge in the field data table from the revision table, but that is not possible. (The field data table is already there anyway so we should not be needing to add it again). I'll see if I can merge in the approach I had with the recent changes.
For reference: my issue was fixed in #3552957: Tables added through Views query alteration should use a relationship when @amateescu and I first met at DrupalCon Vienna 2025 🥳.