Intercompatibility with Group module new versions

thanks @thatguy, i just tried your patch on my Drupal 9.5.2 and Group 3.1.0 and it fixed the problem (:

Thanks @thatguy, Patch #5 also solved my issue.

+++ b/quick_node_clone.module
@@ -84,15 +86,14 @@ function _quick_node_clone_has_clone_permission(ContentEntityInterface $node) {
+      $group_relationships = GroupRelationship::loadByEntity($node);
+      foreach ($group_relationships as $group_relationship) {
+        // Check that user has a permission to create relationship.
+        $plugin_manager = \Drupal::service('group_relation_type.manager');
+        assert($plugin_manager instanceof GroupRelationTypeManagerInterface);
+        $access_handler = $plugin_manager->getAccessControlHandler($group_relationship->getPluginId());
+        $access_check = $access_handler->entityCreateAccess($group_relationship->getGroup(), $current_user);
+        $access = AccessResult::allowedIf($access_check);

This only checks the access control by Group itself. Ideally, you check for access control on the group relationship entity type while providing the right bundle. Then, if another module implements the create access hook, that is also taken into account.

So, I am running quick node clone 8.x-1.16. If I want to install Group 2.x (coming from 1.x), so if I apply patch #5 I'm good.

But, what about this? https://www.drupal.org/project/quick_node_clone/issues/3306677#comment-1... ✨ Intercompatibility with Group module new versions Needs work

Does this need to be applied also, and if so, how?

Thanks!

Patch #5 does not apply against Group 2.x. Patch #2 does not apply either and seems to be missing most of the code it should add. The approach in 🐛 Group module - Error: Class Drupal\group\Entity\GroupContent not found Needs review seems quite hacky. It seems that at this point there is no proper way to use Quick Node Clone with Group 2.x.

The MR aims to work for Group 2.x with backwards compatibility for Group 1.x (did not test it on 1.x). Not sure if it works for Group 3.x too. I think Group 3.x deserves it's own solution without BC for 1.x, maybe in a new major release for Quick Node Clone.

The changes in the MR look pretty good to be for providing support for 2.x along with backwards compatibility with 1.x, but I am a bit conflicted about how to continue to support the Group module effectively given these compatibility breaks and the ones in 3.x → .

We could just cut a new major version release that drops Group 1.x/2.x support and adds support for 3 but it feel strange given that we don't have any hard dependencies on the group module. If we did, we could use semantic versioning to help us here, but then we would be forcing people to install the Group module when they may have no use for it.

I wonder if the right solution is to break this functionality into a sub-module (`quick_node_clone_group`?) Are there any arguments against that path? If not, we could ship this MR and create a new issue to tackle the sub-module.

drops Group 1.x/2.x support

I'm not sure about this, but aren't the majority of people using the Group module using either 1.x or 2.x?

I'm not sure about this, but aren't the majority of people using the Group module using either 1.x or 2.x?

According to the usage stats more people are using 3.x than 2.x → , but both of those constituencies are growing with 1.x shrinking. The problem here is that without any kind of explicit requirement via semantic versioning the implication is that this module supports them all.

Perhaps you could get away with an entity type definition check or a group version check?

This:
$group_contents = $this->entityTypeManager->getStorage('group_content')->loadByEntity($original_entity);
would become something like:

$entity_type_id = $group_version_2 ? 'group_content' : 'group_relationship';
$group_contents = $this->entityTypeManager->getStorage($group_version_2)->loadByEntity($original_entity);

Or:

$entity_type_id = $group_content_entity_type_exists ? 'group_content' : 'group_relationship';
$group_contents = $this->entityTypeManager->getStorage($group_version_2)->loadByEntity($original_entity);

I've not recommended something like the above before because it's usually cumbersome for Group-supporting modules to have to use that all over the place, but here it seems to be limited to one spot so it might just work.

Reroll for module version 1.17

Please submit any changes via the current or a new merge request so that GitLabCI tests will run against it. Thank you!

I've moved patch #18 to the merge request.

I just want to weigh in on the question of Group 2.0 and 3.0 support which didn't have a particularly clear answer above. Both 2.0 and 3.0 are currently supported Group versions and will be for some time -- or such is my understanding. I just updated from 1.0 to 2.0 for instance. The module page says:

"Note: functionality between version 2 and 3 are kept in sync. Version 3 does not have an upgrade path from version 2 or version 1 as it requires a migration."

This is a complicated problem to solve elegantly.

Both 2.0 and 3.0 are currently supported Group versions and will be for some time -- or such is my understanding

Given this, I am inclined to prioritize reviewing and merging support for 2.x (MR17) and then circle back to 3.x maybe with a new issue. All that to say, please review and provide feedback on MR 17.

Having looked at the group code, merge request #20 should work fine with groups 2 and 3.

The only difference between versions 2 and 3 is the machine name of the relationship entities: group_content -> group_relationship.

The group integration from #20 uses the GroupRelationship (2, 3) class, the group_relation_type.manager (2, 3) service, and the group entity method addRelationship (2, 3). All of them are in versions 2 and 3. That is, the machine name is not used directly to load relationship entities.

I added support for group version 1 in merge request #20. That is, it now supports all versions of groups.

Does anyone have any ideas on how to cover this with tests? That is, run tests with different versions of groups.

I added support for group version 1 in merge request #20. That is, it now supports all versions of groups.

Does anyone have any ideas on how to cover this with tests? That is, run tests with different versions of groups.

Thank you @kksandr! In that case, I agree, let's turn our attention to MR 20.

@kksandr Thank you for your work on this, especially with the new tests! Do we need to add anything test-wise for Group v3?

No need. Testing version 3 occurs in the phpunit test by default, since the latest version itself is installed from the development dependencies.
Two (^1, ^2) additional runs of phpunit lower the version to 1 and 2, respectively.
I left a note about this in .gitlab-ci.yml

@kksandr Thanks for clarifying that. Fantastic!

Rebased and resolved conflicts.

Automatically closed - issue fixed for 2 weeks with no activity.

Hi @markdorison, can you please release this. MR.

Thanks for the bump. 8.x-1.18 has been released with this included.