Drupal 10 compatibility | Drupal\Core\Entity\Query\QueryException: Entity queries must explicitly set whether the query should be access checked or not.

Here is patch to fix issue.

Most of these should be accessCheck(FALSE) - anything that's not a listing query for showing information to the user. In practice there won't be query access on most of these entity types, but better to be explicit.

When I was developing this patch I was trying to not break current functionality and keep it the same as it was before.
So If before we had:
BEFORE:

// This gets all answers the current user can view.
$query = $answer_storage->getQuery();
$aids = $query->condition('user_id', $this->getOwnerId())
  ->condition('user_module_status', $this->id())
  ->execute();

then, to make result the same we need to change like this:
AFTER:

Unchanged: This gets all answers the current user can view.
$query = $answer_storage->getQuery();
$aids = $query->condition('user_id', $this->getOwnerId())
  ->condition('user_module_status', $this->id())
  ->accessCheck()
  ->execute();

Please, check this example BEFORE and AFTER https://www.drupal.org/node/3201242 → . It shows the similar query examples but with articles content type and how to achieve the same query results after explicitly calling accessCheck().

I would also argue for keeping functionality as is, so adding accessCheck() to all queries that didn't call said method before seems fair. Having said that, when I updated Group and its ecosystem to D10 I did break this rule for config entity types.

Config entity types never ran query access to begin with, so adding accessCheck() to them does nothing. I chose to add all of these access checks with accessCheck(FALSE) except for the list builder. If core then ever decides to actually add access checking to config entities, then all queries but the list will keep behaving like they did before (not checking access).

Some nitpicks:

1. +++ b/opigno_module.module
   @@ -1009,6 +1010,7 @@ function opigno_module_views_query_alter(ViewExecutable $view, QueryPluginBase $
            $query_a = $activities_storage->getQuery();
   +        $query_a->accessCheck();
   

   Can keep this as a oneliner as accessCheck() returns the query object.

2. +++ b/src/Controller/OpignoModuleManagerController.php
   @@ -501,6 +501,7 @@ class OpignoModuleManagerController extends ControllerBase {
        $query = $activities_storage->getQuery();
   +    $query->accessCheck();
   

   Same as above

Long story short: Keep the behavior unchanged by adding accessCheck() rather than accessCheck(FALSE). Then perhaps in a follow-up we could argue whether these actually make sense by anaylizing all accessCheck() calls and re-evaluating them (also keeping my note about config entities in mind).

But this issue is merely about adding D10 support and for that purpose, the current patch is sufficient.

Looks good to me and as far as I am concerned this is RTBC but waiting a bit to see if @catch has anything else to add.

Yeah adding them as-is then re-evaluating in a follow-up seems OK. The worst situation is when sites use accessCheck(TRUE) in updates, which can mean skipping entities in the update.

I'd say fix the nitpicks and create a follow-up "Task" issue "Evaluate the accessCheck() calls on entity queries" and then this looks good to me.

Hi @oleksandr.s,

I have fixed this issue "Drupal 10 compatibility | Drupal\Core\Entity\Query\QueryException: Entity queries must explicitly set whether the query should be access checked or not." and also attached patch ,
please review and verify.

Hi all!

Here is a patch for the 3.1.2 module version

Please review and verify, regards!

Hi all!

This is a patch for module version 3.1.2,

Please check and verify, regards!

Hi all!

This is a patch for module version 3.1.2, please verify, regards!