- 🇺🇸United States daletrexel Minnesota, USA
Patch in #5 works for latest 2.x-dev as well.
Have successfully applied and used patch in #5 on a Drupal 10 site running Group and Group Content Menu 2.x branches. We'd previously been using the original patch on the 1.x branch, in production, with success. Patch is still needed for 2.x to do what we need to do. (We upgraded to 2.x along with our Drupal 10 upgrade.)
Reviewed the patch and the file it patches, src/Plugin/Block/GroupMenuBlock.php:
- The only difference in this file between 2.x and 3.x branches is line 270, far past the lines the patch touches
- The patch additions do not include group_relationship/group_content, so it can work with either branchNot sure if it's safe to set this RBTC without the requested tests.
- heddn Nicaragua
@DaleTrexel, it would be nice to have a test case. I've tried to manually reproduce this situation myself several times and wasn't able to. Perhaps the key is "expand all items"? Anyway, it shouldn't be hard to extend an existing test to confirm this fixes the issue.
- First commit to issue fork.
- 🇺🇸United States daletrexel Minnesota, USA
Thanks @electrokate for the updated patch! I've tested in on Group Content Menu 2.0.3 (Group 2.2.2, Drupal 10.2.7) and can confirm that the patch applies. I can also confirm that the update to 2.0.3 does NOT resolve this issue, and that this patch is still required to fix it. (Testing so far only on a local copy of the site I manage, but not yet in production.) +1 RBTC
A question for those of you following this issue and/or using this patch (and @heddn): is anyone using Group Content Menu with this patch only and not also in conjunction with 📌 Group Menu not visible on Node pages Needs review and/or 📌 More contexts needed Needs work ? I am finally getting a chance to work on adding tests for this issue, but so far I am unable to do so without also patching the module -- which is incompatible with writing tests! Is there any way to set up multi-level menus with Group Content Menu unpatched that I'm overlooking? It feels like this module is extremely limited without #3170013 to provide context for menu blocks placed on node pages.
- 🇺🇸United States daletrexel Minnesota, USA
Updating the version to 3.0.x so that the issue fork is off the correct branch.
We're actually using 2.0.x, but this should apply to both, and we may be able to solve more than one issue with this, so keeping relevance by focusing on the highest version.
Also simplifying/shortening the title.
- 🇺🇸United States daletrexel Minnesota, USA
DaleTrexel → changed the visibility of the branch 3.0.x to hidden.
- 🇺🇸United States daletrexel Minnesota, USA
DaleTrexel → changed the visibility of the branch 3224816-starting-level-deeper to hidden.
- Status changed to Needs review
5 months ago 8:29pm 4 August 2024 - 🇺🇸United States daletrexel Minnesota, USA
Updating issue description to include new issue brought in by ✨ Integrate some of Menu Block module's options Fixed , also surfacing when levels of 2+ are chosen.
Have updated code, and added tests. #3366930 was also quite useful in showing me how this issue might be tested without having to wait for 📌 Group Menu not visible on Node pages Needs review to be resolved! Thanks for that!
If possible, I'd like to make sure that Wilbur Ince, @wylbur from Electric Citizen gets credit on this issue. He was instrumental in getting me up and running with phpunit and writing tests. Thanks!
- 🇺🇸United States daletrexel Minnesota, USA
The 3224816-3170013-3x-combo and 3224816-3170013-2x-combo branches contain merged code from this issue and my respective merge requests for 📌 Group Menu not visible on Node pages Needs review . These versions may be useful to some, having already worked out minor merge conflicts, but I haven't issued merge requests yet, in case doing so complicates matters for the reviewers.
These branches have both passed my local phpunit, phpcs and phpstan testing.
- 🇺🇸United States daletrexel Minnesota, USA
Adding related issue 📌 Group Menu not visible on Node pages Needs review
- 🇺🇸United States daletrexel Minnesota, USA
daletrexel → changed the visibility of the branch 3224816-3170013-2x-combo to hidden.
- 🇺🇸United States daletrexel Minnesota, USA
daletrexel → changed the visibility of the branch 3224816-3170013-3x-combo to hidden.
- 🇺🇸United States daletrexel Minnesota, USA
Removed (hid) the combo branches now that 📌 Group Menu not visible on Node pages Needs review has been closed as will-not-fix. Fortunately, we can still provide tests for this issue that don't rely on running a patched Group module (the reason I started working on #3170013 in the first place).
Still would love to have some eyes on this other than my own!
- 🇺🇸United States daletrexel Minnesota, USA
Uploading a patch for the 2.x branch, currently up to date with 2.0.3.
Something is definitely wrong with the 2.x branch MR !29 at the moment. I can't tell if this is a recent development or not, but I'm surprised because it originally seemed fine and passed tests. But now it seems to be confused with the 3.x branch. Also, I can't create any NEW branches on this issue fork with the "Create new branch" link on this page (it leads to a 404 error), so that suggests it's a Gitlab issue.
This patch should also work for the 3.x branch of Group Content Menu, because it doesn't touch any code that references the machine names that differ between the 2.x and 3.x branches.
- 🇫🇮Finland Alexander Tallqvist
alexander tallqvist → changed the visibility of the branch 3224816-3170013-3x-combo to active.
- 🇫🇮Finland Alexander Tallqvist
alexander tallqvist → changed the visibility of the branch 3224816-3170013-3x-combo to hidden.
- Status changed to Needs work
28 days ago 3:23pm 20 November 2024