Group menu compatibility with group V2 and V3

I wouldn't say the issue fork is MR-ready, but so far the edits there are working with feature parity to the 1.x version as far as I could see. The module itself doesn't contain tests from what I saw, but I didn't add any, which is usually a blocker on contrib MRs.

The main things I'm iffy about before submitting an MR, in case anybody has insight:

• In groupmenu.services.yml I had to remove the tag and its priority from the config overrides, in order for the new DI of the entityTypeManager to not throw a circular dependency error
• It looks like the 2.x API for group handles permissions a bit differently. I only poked at it enough to get it working, so there's likely permissions and security concerns to approach in the issue branch

The issue branch, by the way, is set up fro 2.x rather than being explicitly 3.x compatible. From my digging, it seems the main difference is that the database tables are names like group_relationship rather than group_content, so making a 3.x version from what's in the issue branch should be about as easy as swapping some query parameter strings, but definitely ymmv with that.

In the last 9 days I've added some commits and tested the functionality more thoroughly. At this point I think it's ready for review and a new 2.x version branch is probably appropriate to differentiate it per the group parent module's majors.

The aspect I feel iffiest about is non-admin permissions at the group level. If you're an admin everything works as you'd expect, but I haven't tested nor dug into the code surrounding more granular permissions of menus associated with groups.

To use groupmenu with Group 1.x we had to patch Group to use strings as entity IDs ( https://www.drupal.org/project/group/issues/2797793#comment-14198690 ✨ Entities identified by strings as group content Closed: won't fix ). That patch no longer applies to Group 2.x, so will a version of groupmenu which works with Group 2.x require that patch?

Our team is very interested in this module for use with one of our current clients. Their site is on D10 and I can't install this fork or its patch due to the version constraints on the 8.x-1.x-dev version.

@ao5357
Would it be possible to create a new 2.x version branch? Is there anything we can do on our side to help that process along? We'd definitely be testing it a lot in the near future and can provide feedback as needed. Thanks!

@ji_gravityworks May be off-topic but group_content_menu has similar functionality, although a different architecture, and G2 and G3 support.

@msielski -- Thanks for the tip! We gave that a try already, but Group Menu is much closer to the functionality we need. Let me know if there's anything I can do to affirm this code so we can start a new version.

@JI_Gravityworks there shouldn't be any technical reason preventing another branch/release strategy to support both group 2.x and 3.x, but before I'd go down that route I'd want to get feedback from one or more of this module's maintainers. I posted this 2.x MR 6 months ago and it hasn't got much traction, so before putting in more effort I'd want to confirm that's a direction desired for this module.

Not sure which requirements would be preferable for groupmenu over group_content_menu, but overall I'd go with that module if at all possible. I only did the v2 port for an upgrade path of an existing project using this module, but would likely use group_content_menu for any new project.

Thanks for the suggestion, but Group Content Menu will not serve our purposes.

I downloaded the module, made some updates, and am currently using it with Groups v3 and D10 as though it were a custom module. It seems to be working fine. Since the work is done, I'm not certain if others would want a new branch if I uploaded the code.

@JI_Gravityworks We're trying to evaluate this module vs Group Content Menu. I would love to see your work if you have a repo somewhere.

@ericras I'm getting a stable version ready. I'm using the work of many other contributors and want to make sure I've got everything working, documented, and all attributions listed so that everyone will get proper credit. It may be a while longer but I'm definitely planning on at least submitting it for RTBC.

I did some fairly thorough testing, and now have a release candidate for D10/G3.0, with Subgroup module support.

@ao5357 thank you for such a strong starting point.
@msielski this fixes the string/integer menu id issue and requires no further patching.
@ericras feel free to review.

@chuyenlv, @Adarshsri786, @dasginganinja thanks for the patch code at https://www.drupal.org/project/groupmenu/issues/3308654 📌 Drupal 10 compatibility issues Needs review

At a first glance I think the subgroup support being in the same MR as the support for groups v2/v3 makes it a bit harder to review.
Not sure now what is absolutely necessary for the upgrade, and changes made to support the new features.

Most of the times it is probably best to keep MRs as small as possible and create separate issues for new features. As for the next steps, I would recommend we try to get these in first:
📌 Fix the issues reported by phpcs Needs work
📌 Drupal 10 compatibility issues Needs review

After that this PR should already be a bit smaller. If we can then create separate issues for the news features (like subgroup support), and split the MR up, I think we should be able to get everything merged more easily.

@SeanB -- I have rolled both these changes into this release:
#3305893: Fix the issues reported by phpcs
#3308654: Drupal 10 compatibility issues

I suppose you could approve them individually and then I can pull them in one at a time and fix any conflicts. Let me know if you would like to go in that direction. Otherwise, tomorrow at Drupal MidCamp I'm going to spend some time in the contribution room and separate the Subgroup functionality into its own MR. Thanks for taking the time to review my code!

@seanb -- Here's the new branch with the subgroup support removed:
https://git.drupalcode.org/issue/groupmenu-3351701/-/blob/groupmenu-3351...

And the new MR:
https://git.drupalcode.org/project/groupmenu/-/merge_requests/10

Thanks for your help!

I reviewed @JI_Gravityworks split MRs and everything looked good to me. Could use an independent review to get to RTBC.

Nice work! This was a lot easier to review than the previous PR. So thank you for that.
Added my review to the PR. Hopefully we can find more people to give this a try and thoroughly test it.

After this is fixed I will add this to a new major release. I suggest we create a 3.x branch to follow the versioning of the groups module.

Addressed most of the feedback in the MR, hopefully correctly—the only one I think I left outstanding was the question about catching exceptions. Leaving marked as Needs work, and I'll see if JI_Gravityworks and I can touch base on that last one tomorrow (we work together).

Added a new 3.x release with the changes from the PR https://git.drupalcode.org/project/groupmenu/-/merge_requests/10
Thanks everyone!

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