Initiative for Group 1.x compatibility under Drupal 10

Yup, I want to get this done. I've just been too slammed with other things for the last few months, and most of my paying clients don't need Group module. 😢 I'm about to be AFK for a couple weeks, but hopefully in mid July when I'm back online I can move this forward.

Thanks/sorry,
-Derek

+++ b/src/Access/GroupContentCreateAnyAccessCheck.php
@@ -57,7 +57,7 @@ class GroupContentCreateAnyAccessCheck implements AccessInterface {
+    $group_content_type_ids = $entity_query->accessCheck(FALSE)->execute();

+++ b/src/Access/SynchronizedGroupPermissionCalculator.php
@@ -53,7 +53,7 @@ class SynchronizedGroupPermissionCalculator extends GroupPermissionCalculatorBas
+    foreach ($group_type_storage->getQuery()->accessCheck(FALSE)->execute() as $group_type_id) {

+++ b/src/Entity/Controller/GroupContentListBuilder.php
@@ -85,7 +85,7 @@ class GroupContentListBuilder extends EntityListBuilder {
+    return $query->accessCheck(FALSE)->execute();

+++ b/src/Entity/Controller/GroupListBuilder.php
@@ -157,7 +157,7 @@ class GroupListBuilder extends EntityListBuilder {
+    return $query->accessCheck(FALSE)->execute();

+++ b/src/Entity/Controller/GroupRoleListBuilder.php
@@ -69,7 +69,7 @@ class GroupRoleListBuilder extends DraggableListBuilder {
+    return array_values($query->accessCheck(FALSE)->execute());

+++ b/src/Entity/Form/GroupRevisionDeleteForm.php
@@ -138,6 +138,7 @@ class GroupRevisionDeleteForm extends ConfirmFormBase {
+      ->accessCheck(FALSE)

+++ b/src/Entity/GroupType.php
@@ -160,7 +160,7 @@ class GroupType extends ConfigEntityBundleBase implements GroupTypeInterface {
+    return $query->accessCheck(FALSE)->execute();

+++ b/src/Entity/Storage/GroupRoleStorage.php
@@ -211,7 +211,7 @@ class GroupRoleStorage extends ConfigEntityStorage implements GroupRoleStorageIn
+      $group_type_ids = $this->entityTypeManager->getStorage('group_type')->getQuery()->accessCheck(FALSE)->execute();

+++ b/src/GroupRoleSynchronizer.php
@@ -86,7 +86,7 @@ class GroupRoleSynchronizer implements GroupRoleSynchronizerInterface {
+    $group_type_ids = $this->entityTypeManager->getStorage('group_type')->getQuery()->accessCheck(FALSE)->execute();

+++ b/tests/src/Kernel/EntityQueryAlterCacheabilityTest.php
@@ -81,7 +81,7 @@ class EntityQueryAlterCacheabilityTest extends GroupKernelTestBase {
+      $storage->getQuery()->accessCheck(FALSE)->execute();

+++ b/tests/src/Kernel/EntityQueryAlterComplexTest.php
@@ -519,7 +519,7 @@ class EntityQueryAlterComplexTest extends GroupKernelTestBase {
+    $ids = $this->storage->getQuery()->accessCheck(FALSE)->execute();

+++ b/tests/src/Kernel/EntityQueryAlterTest.php
@@ -303,7 +303,7 @@ class EntityQueryAlterTest extends GroupKernelTestBase {
+    $ids = $this->storage->getQuery()->accessCheck(FALSE)->execute();

The default for an access check is TRUE. This would technically be a regression.

Primarily, the recommendation is to simply upgrade the module from Group 1.x to 2.x, there is no upgrade path for this, but the 2.x version is compatible with Drupal 10.

The bold part is simply not true. I've gone ahead and updated the IS.

Having said that, I see changes like these:

+++ b/group.install
@@ -23,7 +23,7 @@ function group_update_8001(&$sandbox) {
-    $sandbox['ids'] = $storage_handler->getQuery()->accessCheck(FALSE)->execute();
+    $sandbox['ids'] = $storage_handler->getQuery()->accessCheck(TRUE)->execute();

You can't go around and change access checks like that. The D10 requirement is that all entity queries specifically call accessCheck, but ones that were already deliberately set to either TRUE or FALSE cannot be changed. You can check the original issue here: #3278740: D10 compatibility

That should give you some guidance on what changes we expect to see (e.g. the ProphecyTrait is a false one, don't do that one) and how to split these changes into smaller patches that are easier to review. Ideally, you have one child issue per type of change to make the review process easier.

I just did a quick re-roll add some of the missing voids for some of the test functions so my phpunit would pass.

This doesn't address any of @kristiaanvandeneynde feedback.

Unfortunately in my case, upgrading from version 1.5 to 2.1, removes all previously added groups.

Rerolling patch for failed tests.

The patch is working for us in all automated tests and scenarios in our internal Drupal distro.

The patch is also passing all the module's automated tests for D9 and d10.

It would be great to have another 1.x release with it !

The patch looks good. RTBC.

+++ b/tests/src/Unit/GroupContentEnablerManagerTest.php
@@ -69,13 +69,12 @@ class GroupContentEnablerManagerTest extends UnitTestCase {
-    $this->moduleHandler->getImplementations('entity_type_build')->willReturn([]);

Looking at the patch, why was this line removed?

Line doesn't seem to be needed anymore and tests were green without it.

There were similar changes and comments I could find related to similar fixes:
https://www.drupal.org/project/rules/issues/3346842
https://www.drupal.org/project/group/issues/3278740#comment-14838701

Did a first round of review, found a bunch of config entity queries that had no accessCheck added. This is possible because config entity queries don't run any SQL query. I've made all config entities' access check FALSE except for listing pages so that if core ever starts supporting access checks on config entity queries, the listing pages immediately work as expected.

Further review incoming.

Cross referenced with the v2 and v3 work and looks good. In fact, we might want to check the config entity queries there too. Also replaced accessCheck(TRUE) with accessCheck().

Will tag a new release tomorrow.