🇩🇪Germany @tgauges

Thank you a lot for your efforts. I think the remaining discussion point is whether Drupal 9 to 10.2 compatibility (+ Drush 12 during development) needs to be maintained. In my opinion it should not be maintained because there isn't security support anyway and we can update the minor version in line with #357742 🌱 Guidelines for semantic versioning and Drupal core support Active .

Please take a look at the review and discuss.

This issue is superseded by #3482939 📌 Automated Drupal 11 compatibility fixes for entity_reference_delete_check Needs review .

You can take a look at https://git.drupalcode.org/issue/field_group-2978747/-/tree/2978747-hide-group-with-mutation-observer for my solution for #42.

There don't seem to be any tests implemented for this issue?

Would MutationObserver be an acceptable solution for #42? One would need to observe subtree, childList, and attributes.

I will create a new branch in the issue fork based on 2978747-hide-group-if.

tgauges → changed the visibility of the branch 2960456-2.0.1-deal-with-multiple to hidden.

I merged the current 1.0.x-Branch into the issue branch and updated a comment, please review :)

Thanks for the suggestions. The MR can be reviewed now.

Thanks for the hint regarding the numeric plugin. I fixed the issues and the MR can be reviewed.

@scott_euser

yes, this is for when both types of filters exist.

\Drupal\views\ViewExecutable::generateHandlerId does exactly that: It only changes the ID if it is not unique.

tgauges → created an issue.

tgauges → created an issue.

Please review :)

tgauges → created an issue.

tgauges → changed the visibility of the branch 3494471-10.4.x-psql-rename-str-starts-with to hidden.

tgauges → changed the visibility of the branch 3494471-10.4.x-psql-rename-str-starts-with to hidden.

I closed the MR https://git.drupalcode.org/project/drupal/-/merge_requests/10591 because it is irrelevant. Please review https://git.drupalcode.org/project/drupal/-/merge_requests/10587 which also contains a test which fails without the fix.

Are more tests needed?

The Needs Review Queue Bot should not test MR 10591 since that one targets 10.3.x. It does not apply to 11.x by design.

It also is a draft MR and hidden.

tgauges → changed the visibility of the branch 3494471-10.3.x-psql-rename-str-starts-with to hidden.

I implemented my proposed resolution, please review.

tgauges → created an issue.

tgauges → created an issue.

My vote is for option 2 as well.

I updated the issue summary, should the status be "Needs review" again?

Changing status to "Needs Review" to signal that input/discussion is needed.

I forgot to put this issue back into "needs review", please take a look at my response in the MR.

tgauges → changed the visibility of the branch 3485655-3046447-ajax-focus-duplicate-forms to hidden.

Please take a look :)

The implemented test case is based on the existing test case which is disabled via 📌 [random test failure] Re-enable AjaxTest::testAjaxFocus() Needs work . So I'm not sure whether that new test case will be flaky as well...

tgauges → created an issue.

I implemented a fix and a test case. The test case is not run because of 📌 [random test failure] Re-enable AjaxTest::testAjaxFocus() Needs work . I'm not sure how to continue because the flaky tests in #3396536 📌 [random test failure] Re-enable AjaxTest::testAjaxFocus() Needs work seem to only be happening in the pipeline and I don't have enough expertise for that.

The test is fixed.

I implemented a failing test. But that test is inside a test function which is currently being skipped because of 📌 [random test failure] Re-enable AjaxTest::testAjaxFocus() Needs work .

Should this issue be dependent on the other one?

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

I implemented an alternative solution which makes use of the new refocus-blur AJAX setting → to refocus the last active element.

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

I fixed the relevant test but now another unrelated (?) test is failing. Could you take a look at it?

I created this issue to

• have a discussion about the problem
• have a (temporary?) solution available
• have a discussion about the solution

Since my proposed solution can be seen as a breaking change, I left the merge request as a draft. Could there be other solutions?

tgauges → created an issue.

Fixed in version 1.0.4 →

@chiragp185, Do you have the option to test the code in https://git.drupalcode.org/project/entity_reference_delete_check/-/merge_requests/2?

I tested the changes against version 2.37 of the module and it works as expected. One could argue that this is not really a bug and merging this change would amount to a breaking change. In this case this issue lends itself to version 3 of this module, but that is for the maintainer to decide.

I created an issue fork with the existing patch applied. The tests seem fine. I'm currently in the process of testing the code in a project.

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

Here is a patch against 4.0.0 for reproducible builds.

tgauges → created an issue.

I created an alternative solution, adding only and exclude lists to the purge method to give even more control. Here is a static patch for reproducible builds.

I created a new branch and MR against 8.x-1.6: https://git.drupalcode.org/project/entity_embed/-/merge_requests/42

I created a new branch against 11.x which first sorts the translations by their source string and then the context. The context is needed for equal source string with different context.

Before review, the test should be extended to also test the case for equal source strings but different context.

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

This feature might be classified as a breaking change because the fixture loading behavior is changed. One could circumvent this by defining a new method/interface which can be voluntarily implemented in fixtures.

tgauges → created an issue.

I created a patch of my current changes, also committed to the issue fork. The patch is only for replicable builds.

tgauges → created an issue.

@joinso With 1.17 and Drupal 10.2 it should be possible to have references to paragraph entities. I'm not a maintainer of this module so maybe a new support request issue should be opened?

In my opinion this issue is solved by #3414495 📌 Change paragraphs_form_field_storage_config_edit_form_alter() to only discourage paragraph target references, not disallow Fixed . At least for Drupal versions 10.2 and higher.

I had the same problem on one of my sites and applying the patch fixed the problem.
Changing the status to "Needs Review" for one or more additional reviews.

I think I might need some help with the update hook. I have some experience with updating contrib fields (added via GUI), but not base fields. Do you know some guide of existing code I can take a look at?

What also should be a part of the update hook in my opinion:

• updating the view (if it exists)
• removing broken values in the database

By beginning my implementation I noticed the form_data field also uses the `serialize` method but not the `map` field type. Form data probably doesn't include class instances but following the recommendation of the PHP documentation seems like a good idea.

tgauges → created an issue.

I'm done for now with my contribution. There is some half-baked validation of the duration string because I noticed it will be better with https://www.drupal.org/node/3373502 → which will only be available with Drupal 10.2 and I didn't want to raise the core version requirement. I did my best to document this in the code.

There are no tests.

@svendecabooter Would you like to have tests for this feature?

I'd like to take a crack at this.

This might not have been a feature of the D7 version but it's still a nice feature. The patch applied to the current version in my project and works correctly.

I opened a merge request with my changes.

tgauges → created an issue.

I applied this issue's patch to version 2.0.x of the project. Only one location needed a small adjustment.

I implemented the necessary logic in an issue fork. Before writing tests, I'd like to know if this feature even has a chance to be merged.

tgauges → created an issue.

I changed the existing code in 2921987-nesting-filter-groups to allow for nested condition groups.
With this change, one would still need to implement the views query alter hook, but at least now the nested conditions defined there will work.

I'm not sure about the consequences of this new implementation, I just implemented what seemed to make sense. Surely there will need to be new tests and maybe also documentation?

The next step would be to implement the definition of nested condition groups via the Views UI.

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

I simply removed the hook removing the option to configure a simple entity reference to a paragraph: https://git.drupalcode.org/project/paragraphs/-/merge_requests/82.

I kept the MR as draft because it probably goes against the goals of this module? If the option exists, there should probably be a warning or something if it is selected. What do the maintainers think?

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

Seems like it, yes. I added a link to eLogger in the description.
Thanks for showing me the module.
Can this issue be closed?

My changes in the issue fork work for my use case. I'm not sure that I found every use of the link dependency, please review.

The error seems to be caused by linkediting.js#L394 because the 'link' command ist not available as well. The same issue will pop up a few lines above.

I found out that adding core/ckeditor5.link to the entity_embed/entity_embed library and link.LinkEditing and link.LinkUI to plugins entry of the CKEditor 5 plugin PHP class fixes the initial problem.
This causes another error very unhelpful error:

TypeError: e is undefined

tgauges → created an issue.

I implemented this feature in the issue fork. Please review :)

tgauges → created an issue.

That's very strange. I'm not getting those errors with the same versions. I'll be closing this issue as not replicable.

Could you run phpcs --version as well, please? For me it's 3.7.2.