@david.muffley

Looks good. Handles the deprecation warning and bumps the version requirement to one that includes the new Bom enum.

Uploading a patch file for the current diff of MR!40.

david.muffley → created an issue.

david.muffley → made their first commit to this issue’s fork.

david.muffley → created an issue.

I encountered this same issue on my Drupal 10.2 site and came to the same conclusion. MR 7294 fixes the issue.

This occurs on my site as well. You can replicate it by installing version 2.0.3, saving a view with a value in the attach_library field, and then upgrading the module to 2.0.4. That version changed the array lookup where the string is stored (it removed a second `attach_library`) but didn't update any existing information. I suggest adding an update hook to fix the data stored in every view.

Thanks for putting in the legwork Eric!

I had this same issue but now realize it was due to a mistake I had made, and it's the same mistake @jabberwooki made in #12. The computed field class for the link should extend `\Drupal\Core\Field\FieldItemList`, not `\Drupal\Core\Field\EntityReferenceFieldItemList`. It was due to this mistake that Drupal was calling ValidReferenceConstraintValidator on this link field which caused the crash in the issue summary.

After extending from the correct class I removed the patch in #2 from my site and I'm still able to save the node without the crash.

3415707-add-compatibility-with (https://git.drupalcode.org/project/group_storage/-/merge_requests/6) is based off of 3.x and preserves its code changes while still using group 2.x IDs (group_content instead of group_relationship).

3415707-add-compatibility-2.x (https://git.drupalcode.org/project/group_storage/-/merge_requests/7) is based off of 2.x and just fixes the view configuration and doesn't change any code. All other code seemed to use the right IDs.

I see. I am still a little confused because group_storage 1.x has composer support for group 2.x, while group_storage 3.x (and now 2.x) has quite a bit of different code. Even still 2.x and 3.x have notable differences that I don't have insight into. From my perspective the 2.x and 3.x versions of group_storage should be identical only with different ID values (group_content vs group_relationship) everywhere.

However, in the absence of my full understanding I'm going to just make two forks (one based on 3.x and one on 2.x), both targeting the 2.x branch with whatever changes I find are needed. You can choose the path forward you want.

david.muffley → created an issue.

Attaching a patch that fixes my issue and doesn't disrupt Drupal 10.1 users.

Traceback:

#0 /my/web/root/web/core/modules/file/file.module(1240): Drupal\Core\StringTranslation\ByteSizeMarkup::create()
#1 [internal function]: template_preprocess_file_upload_help()
#2 /my/web/root/web/core/lib/Drupal/Core/Theme/ThemeManager.php(261): call_user_func_array()
#3 /my/web/root/web/core/lib/Drupal/Core/Render/Renderer.php(480): Drupal\Core\Theme\ThemeManager->render()
#4 /my/web/root/web/core/lib/Drupal/Core/Render/Renderer.php(240): Drupal\Core\Render\Renderer->doRender()
#5 /my/web/root/web/core/lib/Drupal/Core/Render/Renderer.php(165): Drupal\Core\Render\Renderer->render()
#6 /my/web/root/web/core/lib/Drupal/Core/Render/Renderer.php(627): Drupal\Core\Render\Renderer->Drupal\Core\Render\{closure}()
#7 /my/web/root/web/core/lib/Drupal/Core/Render/Renderer.php(166): Drupal\Core\Render\Renderer->executeInRenderContext()
#8 /my/web/root/web/core/modules/image/src/Plugin/Field/FieldWidget/ImageWidget.php(130): Drupal\Core\Render\Renderer->renderPlain()
#9 /my/web/root/web/core/lib/Drupal/Core/Field/WidgetBase.php(117): Drupal\image\Plugin\Field\FieldWidget\ImageWidget->formMultipleElements()
#10 /my/web/root/web/core/lib/Drupal/Core/Entity/Entity/EntityFormDisplay.php(186): Drupal\Core\Field\WidgetBase->form()
#11 /my/web/root/web/modules/contrib/inline_entity_form/src/Form/EntityInlineForm.php(174): Drupal\Core\Entity\Entity\EntityFormDisplay->buildForm()
#12 /my/web/root/web/modules/contrib/inline_entity_form/src/Element/InlineEntityForm.php(149): Drupal\inline_entity_form\Form\EntityInlineForm->entityForm()
#13 [internal function]: Drupal\inline_entity_form\Element\InlineEntityForm::processEntityForm()
#14 /my/web/root/web/core/lib/Drupal/Core/Form/FormBuilder.php(1013): call_user_func_array()
#15 /my/web/root/web/core/lib/Drupal/Core/Form/FormBuilder.php(1076): Drupal\Core\Form\FormBuilder->doBuildForm()
... truncated 

david.muffley → created an issue.

Rerolled #15 for 10.2.

Rerolled #177 for 10.2.

I've been using the issue fork for MR!2 without issue.

Bumping to critical as there is no release of the module for any currently supported version of Drupal. The composer.json file added is a minimally-viable version and you may want to add more information to it.

This is happening on my site with a node view when using a relationship on a taxonomy term field as well, so just fixing the included view probably won't be enough.

Also bumping to critical as there is no release of the module for any currently supported version of Drupal.

I've been using the issue fork on Drupal 10.1 with no issues.

Attached a patch that creates the link.

david.muffley → created an issue.

This patch has worked for me with s3fs.

Also it's been merged into 6.0.0.

Interdiff for #37.

One more test base class.

Updated the expected permissions in the test failing in #34. I'll leave it to someone else to review whether that change should be made or not.

#27 doesn't work with Drupal ^10.1 when a view's ajax is a GET. Here's an update to handle older core versions with POST and newer ones with GET. Manually tested with both Drupal 9.5 and Drupal 10.1, both with and without a base path, and with and without valid access. This is just begging for automated tests.

I'm still having this issue on a form submit button whose state depends on some facets elements. #177 resolves the issue for me. I'll try to produce a trimmed down reproducible scenario like the one I'm experiencing soon™.

#11 fixed the issue I'm having with 2.0.6. The commit at #12 is different from the #11 patch. This should be reopened for that fix.

Confirming the #8 - 3.0.x patch resolves the deprecated warning during a 403, and on success serves up CSVs with the correct Content-Type header. The new implementation to register the csv format looks odd at first glance but it's how core does this too (see FileServiceProvider and JsonapiServiceProvider).

I didn't review the other patch.

Removed the nitpicks from #31 so moving to RTBC.

Rerolled #15 for 10.1.x.

Rerolling #17 for 10.1.x.

david.muffley → made their first commit to this issue’s fork.

david.muffley → made their first commit to this issue’s fork.

david.muffley → created an issue.

I've been using this patch on my production site for a year at this point. The test changes validate that the response doesn't leak the metadata. I'm calling this reviewed.

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.

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.

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.

Patch to resolve the issue.

david.muffley → created an issue.

Changed to 2.x-dev since 1.x is security fix only. This still applies as-is to 2.x, and it's still necessary to fix the issue.

Patches for 2.x and 3.x. I've applied the 2.x patch on my dev site, but am only hopeful that the 3.x patch is functional.

There's a reroll for the changed `use` statements. Renamed 'group_content_enabler' to 'group_relation'. For 3.x updated the route to 'entity.group_relationship.create_form'.

Interdiff from #21 failed so it's instead a diff of the actual patch files.

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.

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.

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.

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.

+1. I had the same issue, this patch fixes the automatic upgrade process.

Thanks mt-i-1. I'll go with a container element wrapped around which has the same effect. And I'll stop spamming this closed issue.

Like #115 I'm seeing an issue with the same change. We have a button element inside a form and it has the states settings and the js-form-submit class. Before its disabled status was being controlled due to that first `.prop('disabled', e.value)`, and not it's not working.

Probably needs to have a new issue for that, right?

I'm unsure what tests changed, but I re-ran tests for MR#28 and it also failed the same tests as #39. Something must've changed in this module through another commit to break these tests.

I see several tests were updated in the merge request built off of #28. Here's a patch that includes my changes in #37 and those test updates.

I had a similar issue and I resolved it by resetting the cache of each indexed entity in PostRequestIndexing.php. My solution is just a bandaid to get past the issue I was having and has several holes in its implementation. Specifically it only resets the cache of the top-level indexed entities, any child entities that were loaded during the indexing action are not cleared. I'll attach it anyway as a starting point for further development.

While making #21 I noticed Geocode::getParsedInput() had its logic changed for when it throws the InvalidArgumentException. I'm changing it back to how it is in the dev branch because it also matches how every other LocationInputPluginBase implements the logic: throw the exception if $input['value'] (or lat and lng) are empty. See Map.php and Raw.php.

I reverted two tests to continue testing for that logic. I added one more test to match the ones #15 had added. I'm basing these tests off of the ones in RawTest.php.

Rerolling #15 to 8.x-1.0-alpha3. Updated service usages given the new geocoderProviderStorage instance variable. Removed the unused instance variable geocoderConfig and its associated service argument.

I started rerolling this for 1.0-alpha3 and found that the try block is overzealous. It only needs to capture an exception around $location = $plugin->getParsedInput($this->value);. Additionally since the catch block was doing the same thing as when $location is falsey, I've combined those two into one section. If we ever want different error messages then add the logic into the catch block in another patch.

I share the thought that these messages should be customizable but I think that's out of scope of this issue at this point.