We've had this patch running in production on D10 for a number of months now successfully. Looks like it doesn't need any updates for D11. Changes are pretty straight forward. Marking as RBTC.
There is something wrong with this issue fork. The patch in MR!15 may apply to 8.x-1.x in this issue fork, but it does not apply against the 8.x-1.0-beta6 release, nor does it apply against the latest 8.x-1.x of the actual project.
First of all, this fork's 8.x-1.x branch should be rebased against the project 8.x-1.x, as it is currently at the beta6 tag. I tried to do so, but don't have push access.
Second, there is something odd with this "project-update-bot-only" branch which prevents the MR!15 patch to be applied outside this fork. Attempting to apply the MR!15 patch to either the beta6 tag or the most recent 8.x-1.x of the module results in the following rejection to be generated. a.k.a workflow_buttons.module.rej:
@@ -105,7 +105,7 @@
'workflow_buttons' => TRUE,
];
$form_builder = \Drupal::service('entity.form_builder');
- assert($form_builder instanceof FormBuilder);
+ assert($form_builder instanceof EntityFormBuilderInterface);
try {
$form = $form_builder->getForm($entity, 'default', $form_state_additions);
}
I've tried to figure out how this discrepancy crept in, and so far have not figured things out. Basically, assert($form_builder instanceof FormBuilder); doesn't exist in either (beta6 or the module's latest 8.x-1.x), so the patch fails to find the line to remove.
Hopefully someone more familiar with this module's history can figure it out. As it is, someone trying to get their implementation of this module ready for Drupal 11 is going to have a hard time.
I'm setting this back to "Needs work" since the patch from MR15 doesn't apply to the latest -dev release.
In the process of testing the -dev upgrade on Drupal 10, I encountered the assertion problem mentioned by @ultrabob in this issue, so this is not specifically a Drupal 11 problem. I've proposed breaking that out to a separate issue to help simplify this one focused on Drupal 11 compatibility only. See 🐛 Assertion for entity.form_builder service is incorrect Active .
As far as I can tell, from the latest -dev release, the only changes needed to make this module (and submodule) D11-ready is to add ^11 to each of the .info.yaml files, as in the original automated updates in #4 📌 Automated Drupal 11 compatibility fixes for workflow_buttons Active . Upgrade Status module scans report no other issues if you make these changes.
Please also credit @ultrabob for also proposing this change in 📌 Automated Drupal 11 compatibility fixes for workflow_buttons Active , although the problem exists in Drupal 10 with the latest dev release of this module.
daletrexel → created an issue.
Looking over the merge request and update bot history, I think there may be some confusion over this project's branches, especially with respect to the 2.0.x releases. MR1 also updated composer.json to include ^11, but this doesn't show in the repo code history.
The Drupal.org UI seemed to also have problems setting up the initial issue fork/branch.
I would think that this project needs a separate 2.0.x branch off which 2.0.2 and 2.0.3 are split.
daletrexel → created an issue.
Updating to RBTC since we now have the latest 2.x version of GCM (2.0.5) in production with this patch/MR and all is working fine for us.
I'm also adding illustrations of Problem #2. I'm not sure how likely this scenario is, but I came across it while writing tests. The solution for Problem #1 also solves this problem.
If you have a menu where depth is RELATIVE (to the current page), LEVEL ("Initial menu level") then corresponds to how down from the current page the menu should start, and DEPTH ("Maximum number of menu levels to display") gives the number of levels from wherever the menu starts that should be displayed. Therefore, if Relative depth is chosen, Level 2 means the menu should start one level below the current page (i.e., the child pages of the current page), and Depth of 1 means no further levels should be shown (grand-child pages or below).
In the functional tests, and in these screenshots, GCM as of 3.0.5 shows NO links under this scenario, instead of showing the child pages: 3224816-3.x-Problem2-error.jpg vs. 3224816-3.x-Problem2-fixed.jpg. I've only shown screenshots for the 2nd level links in the Group, but the error manifests at all levels within the menu: no links shown. The calculation of link depth relative to the current page without the proposed fix is incorrect. My MR includes a functional test for this specific scenario.
I've tweaked the issue summary to update the "Steps to Reproduce" for Problem #2 because the outcome, while still incorrect, is now not what I initially described.
I'm also adding images to illustrate the two Problems, since it may be difficult for others to visualize the interaction of Level, Depth and Relative Position parameters.
For the 2.0.x branch, screenshots are from a site I manage, which has a special Group for our Law Library's content within the overall Law School website. Their pages have a main menu across the top, and then a vertical menu on the left showing page structure within one of the four main sections in the Main Menu. 3224816-2.x-Problem1-fixed.jpg → shows how the menu is desired to look, and how it looks with this patch in place. 3224816-2.x-Problem1-error.jpg → shows the menu without the patch: many links from other branches of the Main Menu are mixed in with the current branch.
The 3.0.x branch demonstration uses a bare bones Drupal build with a multi-branch menu of similar nature: a main menu on the upper left, and a secondary menu for the current branch on the right. 3224816-3.x-Problem1-fixed.jpg → vs. 3224816-3.x-Problem1-error.jpg →
Ready for review, so updating issue status.
Menu Block Plugin has been updated for minor merge conflicts, but previous logic remains unchanged.
Functional tests were redone and cleaned up to accommodate the significant (and helpful!) changes to the test code. As before, with the original version of the Menu Block plugin, tests fail in several places to demonstrate the bugs covered in this issue, but after updating the plugin, the tests all pass.
Both 2.0.x and 3.0.x branches are covered.
Happy to get feedback BEFORE the next version release!
Cleanup and update in progress. The previous 2.0.x branch had gotten very messy due to errors in merges, so this is a clean break starting from the 2.0.5 release. I've also renamed the branches to better track the updated issue title.
The latest branches and MRs have pulled over the bug fix to src/Plugin/Block/GroupMenuBlock.php, resolving merge conflicts with 2.0.5.
I still need to cherry-pick the appropriate tests. The biggest merge conflicts with 2.0.5 are in tests/src/Functional/GroupContentMenuTest.php
daletrexel → changed the visibility of the branch 3224816-expand-all-3.x to hidden.
daletrexel → changed the visibility of the branch 3224816-expand-all-2.x to hidden.
daletrexel → changed the visibility of the branch 3224816-menu-below-2-2.x to active.
daletrexel → changed the visibility of the branch 3224816-menu-below-2-2.x to hidden.
Strong support
Digging into the source of the problem on my site, I believe that the problem may be due to inconsistency in the Link Attributes Widget module data schema (perhaps a change that wasn't fully migrated?).
We use the Link Attributes Widget module → , which allows you to add attributes such as classes to menu links. For the page that was throwing this menu block error, re-saving the parent menu item resolved the error and allowed the page to show.
In the database, that parent page's 'link_options' field in the 'menu_link_content_data' table looked like this prior to re-saving:
a:1:{s:10:"attributes";a:1:{s:5:"class";s:0:"";}}
And like this after saving:
a:1:{s:10:"attributes";a:1:{s:5:"class";a:1:{i:0;s:0:"";}}}
The class is changed from a string to an array, allowing $attributes['class'][] = 'menu-item--active-trail'; to work without error.
The current patch/MR approach of setting $attributes['class'] to an empty array, regardless of what was there before seems to work for us, but I wonder if it's potentially removing classes that other users may want to keep?
I'm not 100% certain Link Attributes Widget is to blame for the ultimate problem here, but it seems likely given what is involved.
Can confirm experiencing the same error and that patch/MR work to resolve the problem.
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.
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!
daletrexel → changed the visibility of the branch 3224816-3170013-3x-combo to hidden.
daletrexel → changed the visibility of the branch 3224816-3170013-2x-combo to hidden.
JayDarnell: Is this something that you had working before on previous versions of this module, or is it new?
I have been unable to replicate what you describe: make a Group Content Menu appear on a Group page after checking to restrict the menu block to one or more of the content types in its configuration. I've tried various combinations of Group 3.2.* with and without patches from 📌 More contexts needed Needs work , and Group Content Menu 3.0.* with and without patches from this issue. I'm also currently running Drupal 10.2.8-dev on my test site. In all my tests, as soon as you restrict by content type (even if you check all available types) the menu appears on the Group page.
If you were able to get GCM to appear on a Group page when using the block content type restriction setting enabled, please share the versions of Group and GCM, plus patches applied! I'd definitely like to see what's going on.
But if this is something new that you've discovered, then it should be reported as a separate issue.
(In my own testing previously, I was never able to figure out how one might use the Content Type restrictions settings OR the two confusing "Group Types" and "Group Type" settings tabs on the menu block, so I wouldn't be surprised if you've found something no one else has tried and/or reported yet.)
Adding 🐛 Starting level deeper than 1 ignores active trail, when "Expand all items" is enabled Needs work as related, where MRs merged with MRs from this issue are available as a single branch, for review and patching purposes.
Adding related issue 📌 Group Menu not visible on Node pages Needs review
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.
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!
DaleTrexel → changed the visibility of the branch 3224816-starting-level-deeper to hidden.
DaleTrexel → changed the visibility of the branch 3.0.x to hidden.
(As noted over in [#])
I was curious about this topic in general after seeing the pattern appear in some code I was reviewing. A search turned up this article, which has a very different opinion on the matter:
https://medium.com/@steven_3682/the-definitive-case-against-a-backslash-before-a-php-native-function-9f1f51c4415
Has anyone done benchmark tests other than the ones in the original article to confirm that a change this big in Drupal is worth the predicted performance gains? It looks like the ones run in #3458726: Coding Standards Meeting Tuesday 2024-07-16 2100 UTC → were only the ones from the original article, and even then found to be pretty insignificant.
I was curious about this topic in general after seeing the pattern appear in some code I was reviewing. A search turned up this article, which has a very different opinion on the matter:
https://medium.com/@steven_3682/the-definitive-case-against-a-backslash-before-a-php-native-function-9f1f51c4415
Has anyone done benchmark tests to confirm that a change this big in Drupal is worth the predicted performance gains?
Setting this to Needs Review to get some more eyes on it. Would love to get some feedback!
Have been working on a minimal Drupal setup. Will be checking on a copy of the main site that I manage next (2.x).
DaleTrexel → changed the visibility of the branch 3170013-2x-group-menu-context to hidden.
Updating version to prioritize issue fork off of 3.x branch.
Also updating issue description to acknowledge that ✨ Integrate some of Menu Block module's options Fixed has added template theme suggestions.
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.
Have you tried the latest patch from 🐛 Starting level deeper than 1 ignores active trail, when "Expand all items" is enabled Needs work ? That also deals with showing menu branches 2+ levels down, but focuses on the "expand all items" setting and not individual nodes' settings. I'm curious if the logic added by that patch solves your problem too.
Also note that the recently released version 2.0.3/3.0.3 resolved 🐛 Active trail is cached incorrectly Fixed which deals with active trail, so check to see if that helped your problem too, even without the patch.
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.
Is there a way we can determine when the wrapper is actually needed in the Select2 class, vs. simply setting the dropdownParent to the form ID (without adding any wrapper), as was the approach at the start?
After further review of the impacts throughout our site, I've found that the wrapper is not actually needed for any of our Select2 fields, but when present it DOES cause significant styling issues in some cases.
I think it would be well worth figuring out how to be more selective about the inclusion of the wrapper, to minimize impact to users of this module. I'd take a stab at it myself, but I don't have any use cases that actually need it, so it's hard for me to know where to begin and how to test.
I should add that I have just found some cases where this patch does break styling of some Select2 elements on the site I applied it to, so a change record/notice is definitely important if you stick with the current solution. If you can place the ID used by dropdownParent in some other pre-existing wrapper, that would have less impact.
I was not able to get the patch in #2 to work for my site. Array key 'widget' does not exist.
However, 🐛 Issue with z-index when opening in modal dialog RTBC in the Select2 issue queue DOES work for me in my use case (testing patch #17). So, as @justin2pin suggests, this is probably better fixed in Select2 for all modals, rather than here for Layout Paragraphs. And no new issue/patch needs to be created!
Just dropping a note here that the fix applied in this issue (patch #5) no longer works. The problem with Select2 in modals has resurfaced, and a new solution is being worked on in 🐛 Issue with z-index when opening in modal dialog RTBC . So, if your search for help on this topic lead you here, hop over to this other issue and check out the patch available there.
One more success with #17, Drupal 10.1.8, PHP 8.2. Thanks for the work on this!
@herved the patch does not work without the #prefix/#suffix wrapper because the ID is key for the dropdownParent property to have a unique ID to work with/reference. If $wrapper_id isn't added as an ID here, then you'd need to add the ID to some other element wrapper around the field, but within the modal.
Thanks, @generalredneck! I wasn't 100% certain whether or not your removal of those entries would have impacts to users' configurations too, whether exported before or after the update that broke things.
Also, it gave me the chance to confirm that the updated patch had been reviewed and tested good enough for my purposes!
Patch #14 doesn't change the config output in my case (no cruft for us) but it continues to work for me. Changing back to RBTC.
After reviewing the comment timeline along with the recent beta releases, I'm confident that ✨ There is no display available. Please select another view or change the field settings. Needs review is directly related to this issue, especially in the most recent comment (prior to mine).
Based on the error message, this is due to a regression (re)introduced in 8.x-2.0-beta7, where unserialize() is being called without first checking for a null value (which is no longer allowed as of PHP 8.1).
This had been fixed in beta5 (released August 2023), but changes in beta7 brought it back. This tracks with comments here: the issue existed prior to August 2023, jeff.hartman found that upgrading from beta4 to beta6 (skipping over beta5 where it was first fixed) solved the problem, then mikesimmons observed it was back with beta7.
I've proposed a solution in 🐛 unserialize() of NULL deprecation returns to ViewsReferenceFieldFormatter Needs review . This specifically addresses the null value being passed to unserialize() in line 97 of ViewsReferenceItem.php. Please give the patch in that issue a try and report back if it solves your problem here!
I've opened a new issue 🐛 unserialize() of NULL deprecation returns to ViewsReferenceFieldFormatter Needs review since there has been a re-emergence of a problem that this issue had resolved when it was closed.
🐛
Issue 2919092 breaks BC and causes issues for existing sites
Needs review
, which was included in the beta7 release, allows null data to be passed to the unserialize() function again, without checking whether data might be null or not: unserialize($item->getValue()['data'], ['allowed_classes' => FALSE]) As I and others have found, null 'data' is possible in views reference fields (likely those saved a while ago when expectations for this field were looser), so we really need to bring back a sanity check here before calling unseralize().
My new ticket proposes a solution to re-fix this particular instance (line 98 of ViewsReferenceFieldFormatter.php)
(The 'data' field stores the "extra settings" configuration, which is also involved in 🐛 Configuration schema for "Enable extra settings" (title,pagination,...) incorrect Fixed . I wouldn't be surprised if this is related, but so far I've been unable to reconstruct what previous scenario allowed an empty 'data' field to begin with. It was likely in Drupal 9, which I no longer have an easy way to test.)
While investigating 🐛 Configuration schema for "Enable extra settings" (title,pagination,...) incorrect Fixed I've realized that the data field is what contains the extra settings for views reference fields, and as far as I can tell, only these fields. I've still not been able to figure out when there was a point that we would create paragraphs with views reference fields having null data fields, but it seems that the schema has been under flux. I went back as far as I could on Drupal 10 (beta 4) without being able to replicate the null data field, and the paragraphs throwing the deprecation error have very likely not been touched since we were on Drupal 9.
Regardless of whether the data key is always populated now, it still seems important to ensure we check for a non-null value before sending the data field to unserialize().
Add one more RBTC for me: patch #4 is simple but does the trick.
You do have to go back and re-set views reference fields that were changed by this issue (created or edited after upgrade to beta 7), and that's likely going to be a case-by-case process, so it doesn't seem suitable for an update hook.
Uploading a patch for those who prefer it.
Also updating the status to Needs Review, since I believe this solves the problem and all that remains is confirmation that the approach meets approval.
Merge request submitted.
Noting related issues:
🐛
Passing null to parameter #1 ($data) of type string to unserialize() is deprecated
Fixed
comment
#29
🐛
Passing null to parameter #1 ($data) of type string to unserialize() is deprecated
Fixed
by
@drupalninja99 →
who should also get credit for this issue. Their
patch →
is similar, but based on the example I followed, we probably don't need the !empty($values['data']) check.
✨ There is no display available. Please select another view or change the field settings. Needs review Also references the unserialize on null deprecation error. Possibly related?
🐛 Contextuele filters not respected when used in paragraph Active Possibly related? Comments reference the first issue.
DaleTrexel → created an issue.
Ah. My apologies. I had missed that the build I was working on was ALSO being patched for 🐛 HTML5 Validation Prevents Submission in Tabs Postponed: needs info , which introduced the jquery.once error. The patch in my build was from comment #37 🐛 HTML5 Validation Prevents Submission in Tabs Postponed: needs info . The patch in #2 here fixed that use of jquery.once, but the problem has been fixed in that issue since #87 🐛 HTML5 Validation Prevents Submission in Tabs Postponed: needs info . I'll have to get the folks that manage our Drupal build to update the other patch to a more recent version.
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 branch
Not sure if it's safe to set this RBTC without the requested tests.
Patch #2 looks to be the correct syntax for replacing jquery.once with core/once in the one place that seems to be a problem on the 8.x-3.x branch, /formatters/fieldset/tabs.js
The three JS files mentioned in the issue description do not look to be a problem on 8.x-3.x: they're properly using the $(once()) syntax to use core/once within jQuery.
Patch #4 looks irrelevant because the file doesn't exist in 8.x-3.x. Is there another issue (un-committed) that adds this file?
I've applied the patch in #2 to field_group 8.x-3.4 on a D10 site that was giving the error reported in this issue (multiple times on node edit forms), and the errors went away afterwards.
Not sure if it's safe to mark this RBTC because of the confusion over the competing patches, but #2 looks good to me.
Trying out the web IDE for this simple change. Hopefully it all worked properly! I think I missed the chance to give the commit a proper message.
DaleTrexel → created an issue.
This file doesn't exist in either facets/js or any of the submodules. Facets_summary doesn't even have a js directory. Are you using a patched/forked version of this module, by chance?
https://git.drupalcode.org/project/facets/-/tree/2.0.x?ref_type=heads
I'm not finding the string being patched in the file referenced here, in 2.0.6 (or 2.0.x).
https://git.drupalcode.org/project/facets/-/blob/2.0.6/js/facets-views-a...
There is only one implementation of once() here, and it appears to be the proper application of core/once within a jQuery object:
$(once(facetId, '[data-drupal-facets-summary-id=' + facetSettings.facets_summary_id + '] ul li')).click(function (e) {...}
DaleTrexel → created an issue.
DaleTrexel → created an issue.
Adding 📌 Automated Drupal 10 compatibility fixes Fixed as related, since "new release compatible with latest Drupal" should include D10-compatibility (and the related issue, while closed, has not been incorporated into an official release).
This has been marked as "Closed (fixed)" but there has been no official D10-ready release. Can we expect a new release of this module, or is everyone expected to use the -dev branch?
@markdc @SalvadorP (or anyone else coming to this issue with the same problem):
One thing that can trip people up (including myself!) is that restoring a database from backup may leave bits of the previous database hanging around, depending on the tools you use.
Reading a database backup from a mysql dump will normally drop individual tables before restoring the from the backup, but tables not in the backup won't be touched. So, if you're iterating through the Group upgrade process and got past the point where the update creates a new table in the database, that table will remain if you don't clear out the entire database (or at least that new table) before re-importing from a previous backup for your next round of testing.
For example, if you're used to using drush sql:dump to generate backups, and something like drush sql:cli < backupFile.sql to re-import a previous backup, this could leave newly-generated tables in the database to confuse you.
Drush has a handy drush sql:drop command to drop and re-create the database, clearing out ALL tables, before you import a backup. Some local development tools like DDEV also have database import tools that clear out old tables before importing a backup, without having to do a two-step process (like you have to do with drush).
If you're still seeing table-already-exists errors after adding this step to your restore process (and you're sure the table hasn't snuck into your backup from a previous upgrade attempt!), then feel free to update here. I've been working on Group 1.5 to 2.x upgrades pretty heavily and the only times I've seen this error it has been my own fault. But that doesn't mean there isn't a real problem for certain scenarios.
I had this same error when upgrading a site patched 1.5 for ✨ Add views contextual filter to get group ID from either URL or group content Fixed , and had not included the patch in the 2.x build.
The errors went away when I re-rolled and applied the patch for 2.x: ✨ Add views contextual filter to get group ID from URL or group content 2.x Active . (New issue # due to previous issues status wonkiness.)
If that's not your case, then reviewing your group-related views plugins would be a good place to start.
See ✨ Add views contextual filter to get group ID from URL or group content 2.x Active for an updated patch for the Group 2.x branch.
I submitted a new issue in hopes to draw attention to this issue that was closed prematurely, and seems to have gotten overlooked.
Adding patch.
The only change needed (as far as I can tell), is to replace
\Drupal\group\Entity\GroupContent::loadByEntity($entity)
with
\Drupal\group\Entity\GroupRelationship::loadByEntity($entity)
DaleTrexel → created an issue.
Uploading patch.
DaleTrexel → created an issue.
I should also point out that the Upgrade Status module is currently listing all sites on Group 8.x-1.5 as D10 ready. See screenshot:
People are busily preparing for D9 EoL right now. Is "postponed" the correct status for this issue? Postponed until when? Fixing this ASAP will help prevent a lot of nasty surprises.
For the sake of expediency, does this need to be split into 2 separate issues: one to remove the incorrect D10 compatibility statement and another to deal with D8/9 compatibility?
Thank you for starting this issue! This is definitely going to catch other people off guard, like it did me.
Someone who is auditing their site for D10 compatibility with a long list of contrib modules to review is going to see "Works with Drupal: ^9.3 || ^10" and stop reading there, because they are not expecting the release notes to OUTRIGHT LIE to them, even if there is clarification below. They're going to tick this module off as covered for D10 and move on to the others in their list that need attention.
Thanks for the clarification, @heddn!
I don't know if the note in the Group 8.x-1.5 release has been recently added or if it was always there and I overlooked it because I saw "Works with Drupal: ^9.3 || ^10" at the top, which answered my immediately question about D10 compatibility, and it didn't occur to me to read on in case the release notes were allowed to outright lie. But I do see that message now.
For Group Content Menu module, it would definitely be helpful to add clarification to the project page, because the wording in the releases for 2.x and 3.x are terse enough to lead to ambiguity: does "support for" a version mean "only" that version, or does that particular release "add" support for that version (with the implication that it includes backwards compatibility with previous versions)?
I'm confused about two things:
1. Group 8.x-1.5 has been D10 compatible since August 2022 → . That seems in conflict with comment #10.
2. What versions of Group Content Menu work with which versions of Group? I don't see any information on the project page or in the project's readme file about compatibility and how to choose versions. Can you use Group Content Menu 2.x or 3.x with Group 1.x?
For context:
The University of Minnesota manages Drupal in a giant multisite format, and that base setup is on Group 8.x-1.5. Their plan is to stay on the 1.x branch until AFTER upgrading to D10, since Group 1.x is D10 compatible. I have no control over Group. However, I do have control over Group Content Menu, which we manage in our site repository. We're currently on the 1.x branch of Group Content Menu. I need to find a way to survive the pending D10 upgrade this summer. That means either patching Group Content Menu 1.x to be D10 compatible or upgrading to Group Content Menu 2.x or 3.x. Is the latter option even possible with Group 1.x?
Thanks to everyone who helped move this along!
DaleTrexel → created an issue.
What are the plans to formally release the 3.x branch from dev? Hopefully well before the D9 EoL in November!
Thank you! This is just what I was looking for (and surprised not to find in any official documentation).
Thanks to everyone who has been working on this issue! I've also implemented patch in #19 and it's working for me.
One caveat: I'm not using the unpublish functionality, so I can't speak to that working or not.
DaleTrexel → created an issue.
Recommendation under "Modifying attributes without printing them" fails under certain circumstances. Not sure if this is a bug in Twig parsing or "works as intended", so added work-around to documentation for others who may discover this behavior.