Closed (fixed)
Project:
Group
Version:
3.0.x-dev
Component:
Code
Priority:
Major
Category:
Bug report
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
4 May 2022 at 15:26 UTC
Updated:
5 Feb 2026 at 09:23 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #3
ricovandevin commentedComment #4
valthebaldSame issue exists in 3.0.x and 8.x-1.x. I believe 3.0.x should be patched first (the patch is the same), and then the change should be backported to 2.0.x and 8.x-1.x?
I can confirm that this specific changes fixes deprecation warning in all branches, but there are other places where EntityQuery is executed without explicit access check, probably it'd be worth creating several issues (per branch) to fix all access checks.
Here are the calls of EntityQueryInterface::execute() without explicit access checks for the 3.0.x branch:
Comment #5
valthebaldHere are the calls of EntityQueryInterface::execute() without explicit access checks for the 2.0.x branch (same list as for 3.0.x)
Comment #6
valthebaldHere are the calls of EntityQueryInterface::execute() without explicit access checks for the 8.x-1.x branch:
Comment #10
heddnLet's see how the 3.x branch fares. I just now pushed all the changes needed on localdev to get tests passing green on Drupal 10.
Comment #11
heddnRelabeling since this is all about Drupal 10 compatibility in general, not just access checks.
Comment #12
heddnLinking some contrib module issues that depend on this landing.
Comment #13
heddnThis also is blocking #3262067: [META] Make Webform compatible with Drupal 10 from landing because it needs this module to support Drupal 10.
Comment #14
mxh commentedSince this is giving an error on group listing pages which makes it unusable, and D10 is now stable, this is critical.
Comment #15
kristiaanvandeneyndeCan't commit this as is, there's more than just the query access going on in the patch. I see public properties becoming protected, views yml files changing, php type-hinting being added, ...
As it stands this is impossible for me to commit because the diff is too large and diverse and I'd risk missing something crucial that introduces a new bug. Please focus on just the info.yml changes in one issue, just the accessCheck() calls in another, etc.
This can all be done in child issues.
Also, not critical as per the Priority guidelines (there is a workaround: don't upgrade to D10 yet)
Comment #16
heddnI've split the boilerplate changes for tests into #3328308: Test changes for D10 compatibility and the explicit access checks for entity queries into #3328351: Explicit access checks on content entity queries. Plus, views have been cleaned-up in #3328366: Rebuild views to remove 'core' key.
The patch here is _much_ smaller and addresses the below remaining change records. #3000490 says it is for Drupal 9.4+, but our use of it is in tests only. There is no mention of it outside of a test. However, the symfony route object change came in with 9.1. For that reason I've updated .info.yml files with a minimum version of 9.1. We could be less cautious and mark that 9.4 seeing as 9.3 no longer has support. But I'll leave that up to committers when they land this thing.
https://www.drupal.org/node/3151009
https://www.drupal.org/node/3159012
https://www.drupal.org/node/3000490
Maybe obvious, but we should land the children issues before this one.
Comment #19
kristiaanvandeneyndeOne event seemed to have flown under the radar but I caught it :) Also not sure why we no longer need
$this->moduleHandler->getImplementations('entity_type_build')->willReturn([]);in the relation type manager test, but tests seem to still go green without it.Comment #20
heddnAdding an empty patch here to start a test run on 10.1, just to confirm everything is OK on the testbot on 10.x.
Comment #21
heddnGreat, they came back green. Moving to RTBC in hopes that we can get test runners setup on commit for 10.x branch so no regressions.
Comment #22
kristiaanvandeneyndeMade default tests run on D10, with commit-based tests also for D9
Comment #23
berdirthe latest 8.x-1.x release is also tagged as D10 compatible, it might not be too much work to backport these fixes there if you're OK with committing that. The vast majority of sites are still on that version and since it can be installed without actually being compatible it's probably going to create a bunch of support requests and might be more work in the end to deal with those ;)
Comment #24
kristiaanvandeneyndeAs far as I'm concerned and for my own sanity, Group 1 will be EOL as soon as 2/3 are stable early Q1 2023. Having said that, if someone was willing to take all patches here and backport them to 8.x-1.x, I would consider committing them and tagging a final release for the 1.x branch.
For all other intents and purposes, the 1.x branch is "feature complete" as in it won't be getting any new ones. The API is just too old and annoying to cater to new features with compared to v2/3.
Comment #25
berdir> I would consider committing them and tagging a final release for the 1.x branch.
That's all I'm asking for, definitely not expecting any new features :) We have a ton of customizations, extra modules and patches in our project, and updating that is likely not going to be trivial.
I might work on that some time early next year, but no promises. Depends on what we expect to be easier.
Comment #27
ajitsUploading a patch for 8.x-1.x that fixes the fatal errors.