Add compatibility with Group v2 and v3

With this MR you'll be able to recover the site, alas that module will work as it adds the changes explained in https://www.drupal.org/node/3232814

In case useful I have used the ONE LINER on this page

https://drupal.stackexchange.com/questions/72927/manually-disable-a-module

as per....

drush eval "\$module_data = \Drupal::config('core.extension')->get('module'); unset(\$module_data['MODULE_NAME']); \Drupal::configFactory()->getEditable('core.extension')->set('module', \$module_data)->save();"

to deal with issues where the module didn'ty allow the usual uninstall techniques

Hope that may help some one!

Sam

Adding patch for composer

This patch did not apply for me on 1.0.0-alpha2.

Hi all, thanks for pushing work on this ahead. The patch in #7 worked okay for me with 1.0.0-alpha2 but the trouble I now have is that because the project's composer.json file specifies group 1.x as a dependency, you can't install it using composer until a new release of this module is published that states compatibility with group 2.x.

Would it be possible to put one out, even if it's a pre-release of some sort?

Thanks

As for comment #8 , it works for me with 1.0.x-dev and I see that there's no conflict with subsequent changes commited from 1.0.0.alpha2 as basically is adding js tests(for files that this patch is not touching), so it should be applied: https://git.drupalcode.org/project/entitygroupfield/-/compare/1.0.0-alph...

Anyway changing version here to reflect that it happens already in last dev release.

I guess that problems in #9 are solved with last commit, but it's a change that I guess is useless when using composer-patches as patches are applied after composer has been executed. We must wait that the project manager accepts the changes, and he needs to upgrade the release number in composer.json and in create a new release in drupal.org. I hope someone helps here, because despite the issue is in critical state we are not receiving feedback...

Group 3.x uses group_relationship instead of group_content. Also the corresponding classes is changed to GroupRelationship.
I added a addtional Mr. Propably a entitygroupfield is need for each group version

To use the patch you need to replace the repository in your composer.

 "repositories": {

        "entitygroupfield": {
            "type": "package",
            "package": {
                "name": "drupal/entitygroupfield",
                "version": "1.0.x-dev",
                "type": "drupal-module",
                "source": {
                    "url": "https://git.drupalcode.org/issue/entitygroupfield-3306512.git",
                    "type": "git",
                    "reference": "3306512-breaks-website-on-group-3"
                }
            }
        },

I'm on Group v3, I've installed this module, I ran into the error, coudl not apply the patches and now I can't uninstall and remove this module.

Nevermind, it worked

Thanks for working on this! I just marked a bunch of other issues as duplicates with this, since this is both the oldest issues, and the furthest along.

Propably a entitygroupfield is need for each group version

Bummer, I was hoping to avoid that. 😅 Perhaps I should leave the 1.0.x branch for compatibility with group 1*, skip 1.1.x 😬, start a 1.2.x branch for group 2.* compatibility, and a 1.3.x branch for group 3.*? It doesn't really make sense from a semver perspective for this module to jump major versions just because one of its dependencies jumps major versions...

Are we sure that MR 4 only works with group 2.x and MR 8 only works with group 3.x and neither still works with group 1.x? I don't have time tonight to start testing all the combinations, but I hope to do so soon.

Thanks again,
-Derek

Also, this isn't exactly a bug. We never claimed to support Group v2 or v3, yet. 😅 Changing this to a task and updating the title...

Not sure if this belongs here, but just FYI that the tests below are using the since-removed permission "bypass group access"

/tests/src/FunctionalJavascript/EntityGroupFieldWidgetTest.php:85:      'bypass group access',
/tests/src/Functional/EntityGroupFieldFormatterTest.php:49:      'bypass group access',
/tests/src/Kernel/GroupAutocompleteFormElementTest.php:131:        'bypass group access',

Noticed this when trying to uninstall the Group module and start over with G3 on a new build.

@scotwith1t: Thanks, that's helpful info, and yes, here is perfect. That said, I thought I had an issue open somewhere bemoaning the need to test with that permission 😉 But alas, I don't see it in the queue. Maybe I'm thinking of commit d5a7750. It's probably a good thing to stop having our tests rely on that perm. So maybe we should open a follow-up about ripping that out, regardless of V2/V3 porting.

Tested MR 8 with Group 3 - works well.

Thanks for testing MR8 with Group V3. Per the MR comment I just opened, we also need to confirm if that same code from MR8 works with Group V2 or not.

Also, will clearly need a new version for this. Looking more closely at the changes, some of them are pretty disruptive / breaking API changes if anyone was trying to extend entity_group_field. I should probably open a 2.0.x branch and only plan to commit this for a 2.0.0-alpha1 release, not the 1.0.x branch.

Verifying this does not work with Group v2. I can install it and the field is available and can be enabled on group content types, but the manage display tab for content types with the field enabled pops an error:

Drupal\Component\Plugin\Exception\PluginNotFoundException: The "group_relationship" entity type does not exist. in Drupal\Core\Entity\EntityTypeManager->getDefinition() (line 139 of /Users/fm56/Documents/Jobs/aws/hg/stage/web/core/lib/Drupal/Core/Entity/EntityTypeManager.php).

On creating new content with the field the Add to Group button churns for a long while but doesn't do anything but logs a similar error:

Drupal\Component\Plugin\Exception\PluginNotFoundException: The "group_relationship_type" entity type does not exist. in Drupal\Core\Entity\EntityTypeManager->getDefinition() (line 139 of /Users/fm56/Documents/Jobs/aws/hg/stage/web/core/lib/Drupal/Core/Entity/EntityTypeManager.php).

I'm thinking it ought to be possible for this module to check what version of Group is running and substitute group_content and group_content_type accordingly. Easier than trying to maintain parallel versions. I'm going to take a stab at writing a patch because I need this module rather intensely.

@phl3tch: Thanks for testing!

I can confirm now that if a few conditionals let a single branch work for both Group V2 and V3, I'd be thrilled and would probably commit/release that. In general, I think that would be better than having to maintain separate branches, and would make it easier for sites to upgrade from Group V2 to V3 when they're ready to do so.

The only thing I can foresee as a problem is that I'm not sure how to configure the d.o testbot so that we could run the test suite against both Group V2 and V3. If they're in separate branches, each branch can have its own specific dependency and then it's all simple and clear. If a single branch of ours can support multiple branches of Group, I don't know if testing a "dependency matrix" is possible or not. I'll do some research on this point...

FYI: I opened a Slack thread about this in the #testing channel https://drupal.slack.com/archives/C223PR743/p1681848554422859 -- hopefully someone has bright ideas. 😅

Yay, @moshe pointed me to an example of some "dependency matrix" testing using GitLab CI.

You dont need to maintain different branches. This is easier to do with bleeding edge of Gitlab CI. This is how keycdn does matrix testing https://git.drupalcode.org/project/keycdn/-/blob/8.x-1.x/.gitlab-ci.yml
Instead of composer-d9 and composer-d10 you want composer-group2 and composer-group3. You temporarily need to add a CI env variable to your project for this to work. I'm happy to help as you dig in.

I'll have to dig in more deeply once this is getting ready, but in principle, my concerns about testability are basically resolved, and I'd be very happy to only have to maintain a 2.0.x branch to support both Group V2 and V3, instead of having 3 separate branches here.

Thanks again!
-Derek

As you're probably aware, the patch for v2 compatibility is drop dead simple. Including it here. I didn't do the group version check because it sounds like the GitLab thing has it covered.

I assume the patch I submitted failed because it has to be applied atop MR8. No idea how to specify that when I upload the patch.

Thanks for working on this!

Yeah, you can't apply patches to MRs (automatically). You can either:

1. Push more commits directly to https://git.drupalcode.org/issue/entitygroupfield-3306512/-/tree/3306512... (for MR8)
2. Create a new branch from `3306512-breaks-website-on-group-3` (called something like `3306512-breaks-website-on-group-2-and-3`), apply your patch to that, commit it, push that to this issue fork repo, and open a new MR.
3. Upload a combined patch instead of using the MRs at all.

I'm happy with #1 and that's easier, but in some cases, #2 might be more appropriate. Some folks do #3, but the community is trying to migrate to entirely using GitLab for issues. Direct patch uploads aren't going to be around forever, so it's good not to rely on those. But if that's easier than dealing with GitLab, I totally understand and would still review such a patch. 😅

Thanks again!
-Derek

After a moderate amount of testing I'm confident saying that the current state of MR8 plus the patch in #30 functions with Group 2.x such that I'm moving forward with that on my site.

@dww Having a single branch compatible with Group 2 and 3 is great news! I don't know the intended implementation for that, so I'll leave it to you.

I'm looking over the existing tests. Due to the removal of the 'bypass group access' permission and other changes in role management, the tests need some big help. I hope to get something working there. I see you have GitLab CI access as of a few hours ago so running tests through this MR would be great.

I got EntityGroupFieldWidgetTest passing on MR8, phew. I didn't touch any of the assertions, just the group setup and the widget config for required fields. Since Group 2.x and 3.x support Drupal 10 this needs to as well, so some changes are aimed that too.

I learned that Group 2.x and 3.x, while they don't have any explicit PHP version constraints in composer.json, have syntax that can fail on PHP 7.3 or lower. I assume there's no special mention of this since PHP 7.4 and lower are all out of support. Anyway the PHP 7.3 tests will always fail due to this.

Edit: Use MR4 instead of this patch plus MR8.

----

This is a follow-on from the patch in #30 with more things set back to the Group 2.x machine names. Running tests locally gives the same results now with (1) the MR8 branch with Group 3.x and (2) the MR8 branch plus this patch with Group 2.x. Some of those test assertions are still errors.

That just updated the branch `3306512-breaks-website-on` as Group 2 compatible to be the same as `3306512-breaks-website-on-group-3`, with the appropriate type machine name differences.

Thanks @ekes, I had glossed over that branch, but your solution is good. I can confirm that MR4 is Group 2.x compatible, and identical to the MR8 plus #35 patch I had done. I've hidden away the patches in favor of the two PRs for now. @dww mentioned the ability to do Group 2.x and 3.x compatibility in one branch so eventually these will become one PR but I'm still not sure how that will be accomplished.

I am on this again.

I wouldn't recommend what #14 comment says, as this imply security issues on the site applied.
MR participation is open, so anyone could add arbitrary code directly to your site.

I am adding D10 to be able to test it on Drupal 10, also removing drupal/core requirement from composer.json as https://www.drupal.org/node/2514612#s-drupal-9-compatibility says.

Finally green!

After properly reviewing, a release for group v2 will be great. Then, when the needed conditionals are there, it could be added support for group v3 as well.
So I am marking it as needs review.
I understand that may be better to have just one new branch, but maybe we need to supersede this issue to a new one to work on v3 support. This long issue is scaring everyone as there are too many variables playing in this thread : group version, php version and drupal version.

Resolved the final notices from upgrade_status for Drupal 10. `core/once` was introduced in Drupal 9.2 so I bumped the info file's core_version_requirement to that. Group 2.x requires ^9.5 so this module de facto only supports ^9.5 anyway.

I also believe that the libraries besides `core/drupal.dropbutton` don't need to be depended on here but I don't have the time to prove that so I won't touch them.

The claro theme doesn't render the custom dropbutton implementation correctly but this occurs even before this libraries update.

I introduced two functions to handle group_[content/relationship] and group_[content/relationship]_type which provides both group 2.x and group 3.x support and tests are passing. @dww, you're good to do #29 now.

Thanks to everyone who has been moving this forward! I'm thrilled that MR 4 is now hopefully compatible with both Group V2 and V3! 🎉🙏 To hopefully minimize confusion, I closed MR 8.

💯 agree with #39 that #14 is dangerous practice that we should complete avoid. Sorry I didn't say so earlier. 😉

Re: #42

The claro theme doesn't render the custom dropbutton implementation correctly but this occurs even before this libraries update.

See #3183134: Broken dropbutton in Groups field when used with Claro for that. Don't need to mess with that here.

Re: #43 and #29: #3367521: Configure GitLab CI is where I added .gitlab-ci.yml for the 1.0.x branch. I'm going to work on configuring it for Group V2 + V3 locally now, and merge that into this issue branch. Stay tuned.

But first, MR 4 needs a rebase. Just pushed that much. Let's see what the Gitlab CI pipeline does with it: https://git.drupalcode.org/project/entitygroupfield/-/pipelines/12580

Weird. I pushed commit a339244 to a new 2.0.x branch here, but it's not getting automatically linked in this issue. 🤔

Sadly, getting the matrix testing for GitLab CI in here isn't trivial. 😢 I tried for a while locally, but as this is my first time messing with GitLab CI config, I wasn't successful. But I didn't want to block this any further on that problem, so I'm splitting that off to a follow-up: #3367547: Configure GitLab CI matrix testing.

Therefore, I manually did the matrix testing for the code in MR4 by running the full test suite on both D9 and D10, with both Group V2 and V3. All's well! 🎉

YAY! This is now done and merged. Thanks everyone!! 🙏

Great! Thank's for your work derek! 🎉