Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[5.x]: Entry field queries returning _all entries_ if the id() param is used to exclude entry IDs #15570

Closed
mmikkel opened this issue Aug 20, 2024 · 6 comments
Assignees
Labels

Comments

@mmikkel
Copy link
Contributor

mmikkel commented Aug 20, 2024

What happened?

Description

Since upgrading from 5.2.10 to 5.3.4, I'm seeing some odd behaviour with entry queries derived from Entries fields, where adding an .id(['not', id) param to the entry field query results in the query returning every single (non-nested) entry in the site (except the one(s) with the excluded ID(s)), i.e. completely disregarding the actual relations in the field.

Prior to 5.3.4, this query would return entries related to entry in the field entriesField, except an entry with the ID 23432234:

{% set entries = entry.entriesField.id(['not', 23432234]).all() %}

On 5.3.4, the above query will return all entries in the site (except any with the ID 23432234).

Crucially, this only appears to happen if the source entry has been re-saved after upgrading to 5.3.4. I.e. immediately after upgrading to 5.3 things work as before, but once the relation source entry is re-saved (either manually or via CLI), the described behaviour manifests.

I dumped the raw SQL from entry.entriesField.id(['not', 23432234]) before and after upgrading the site to 5.3.4 (and re-saving the entry) – here's a diff to highlight what changed (likely relevant, the INNER JOIN relations part is somehow gone): https://www.diffchecker.com/l25MWZtQ/ (the SQL from 5.2.10 is on the left; 5.3.4 on the right).

I'm able to reproduce this on a brand new 5.2.10 install after upgrading to 5.3.4; here's some steps for a reduced test case:

Steps to reproduce

  1. Install Craft 5.2.10
  2. Create a section and an entry type for that section
  3. Create an entries field with the handle entriesField, and add it to the entry type
  4. Create 3 entries in the section; "Entry 1", "Entry 2", "Entry 3"
  5. Add "Entry 2" to the entries field for "Entry 1"
  6. Add the following Twig to the entry template for the section, and visit "Entry 1"'s URL:
{% set entries = entry.entriesField.id(['not', 23432234]).all() %}

{% for entry in entries %}
    {{ entry.title }}<br/>
{% endfor %}

(The element ID in the .id(['not', ..) param is completely fictional, and it doesn't matter; the important thing is that the .id() param with the 'not' directive, is used).

As expected, the output should be

Entry 2<br/>
  1. Upgrade the site to Craft 5.3.4
  2. Reload "Entry 1"'s URL front end template and confirm that the Twig above still outputs the same result as before.
  3. Resave "Entry 1", either manually from the control panel or via the craft resave/entries CLI command
  4. Reload "Entry 1"'s URL front end template again, and confirm that the query now outputs the following:
Entry 1<br/>
Entry 2<br/>
Entry 3<br/>

Expected behavior

Adding .id(['not', id]) to an entry field query should return related entries excepting the one with the specified ID.

Actual behavior

After resaving the source entry in Craft 5.3.4, adding .id(['not', id]) to an entry query results in all entries except the excluded one being returned, regardless of the entry field's actual relations.

Similarly, adding .id(id) to the entry query results in the entry with the specified id being returned, even if it is not in fact related to the source entry in the entries field.

Craft CMS version

5.3.4

PHP version

8.2.20

Operating system and version

macOS, DDEV v1.23.3

Database type and version

Ver 8.0.36 for Linux on aarch64 (Source distribution)

Image driver and version

No response

Installed plugins and versions

None

@mmikkel mmikkel added the bug label Aug 20, 2024
@i-just i-just self-assigned this Aug 21, 2024
@i-just
Copy link
Contributor

i-just commented Aug 21, 2024

Hi @mmikkel, thanks for reporting!

The fact that after resaving there’s no more join with the relations table is expected. With making the relation fields multi-instance, their content is now saved in the elements_sites table in the content json column (like for most other fields). There’s still a provision that makes the “old” relation storage work. This is why the queries differ between 5.2.x and 5.3.x and why the issue you reported only starts to be visible after re-saving.

I do see what’s causing the id() issue, and I’m working on a solution.

@brandonkelly
Copy link
Member

While we discuss the solution for this, worth noting that you can resolve by setting the ID condition using andWhere() instead, which feels generally safer anyway.

entry.myRelationField.andWhere(['not', ['id' => 123]])

@mmikkel
Copy link
Contributor Author

mmikkel commented Aug 22, 2024

@brandonkelly Good to know, thanks. Excepting the current behavior, in what way can using .id() on relational field queries be considered less than safe? I've been doing that since forever; having to pivot to using .andWhere() to achieve the same thing would be a potentially painful breaking change in getting sites upgraded to Craft 5 😅

@brandonkelly
Copy link
Member

Fixed for the next release.

Excepting the current behavior, in what way can using .id() on relational field queries be considered less than safe?

Relational field queries have always used the id param to some degree – when populated from POST data, or when the “Maintain Hierarchy” setting is enabled. So in those cases you could have gotten into trouble if you were tampering with id.

@mmikkel
Copy link
Contributor Author

mmikkel commented Aug 28, 2024

Fantastic, thanks both!

So in those cases you could have gotten into trouble if you were tampering with id.

TIL! Might be more careful with that going forward – but nevertheless, it's a relief that it was possible to restore the pre-5.3 behaviour :)

@brandonkelly
Copy link
Member

Craft 5.4.0 is out now with that fix. Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants