Automated Drupal 11 compatibility fixes for conditional_fields

Tests are failing on errors around EntityReferenceTestTrait. We also might want to just set the minimum Drupal core version to 10.1 and remove some of the BC logic in the latest MR. That said, I'll let another maintainer comment about jumping the minimum supported version.

It looks like the tests are passing now! With the addition of a composer.json file, the module now installs with composer and can be enabled in Drupal 11. Just need someone else to confirm, please.

That said, I'll let another maintainer comment about jumping the minimum supported version.

@heddn Thanks for keeping track on this issue. My vote for (agree) on 10.1 minimum since Drupal 9 EOL was in 2023

@BenStallings: thanks for working on it.

Just need someone else to confirm, please.

These are just warnings but maybe we should care them since they are Drupal 11 related and this way we would have a clean compatibility fix.

Blocker - Ignored error pattern #^Call to deprecated function watchdog_exception\(\)\: in drupal\:10\.1\.0 and is removed from drupal\:11\.0\.0\. Use Use \\Drupal\\Core\\Utility\\Error\:\:logException\(\) instead\.$# in path /builds/issue/conditional_fields-3428532/src/Form/ConditionalFieldEditForm.php was not matched in reported errors.
in src/Form/ConditionalFieldEditForm.php:0

Major - \Drupal calls should be avoided in classes, use dependency injection instead
in src/Form/ConditionalFieldEditForm.php:578

EDIT: After rethinking the whole issue - which is actually only about the automated Drupal 11 compatibility fixes with some custom tweaks - we maybe should move on and add any further discussion to other issues related to Drupal 11 optimizations for CF.

Adding a patch with the current D11 compatibility changes.

We've got the BC layer working for 10.1, so let's just go with it.

We've got the BC layer working for 10.1, so let's just go with it.

@heddn: Yeah, agreed, see my edited previous comment. I just forgot to press enter yesterday on night shift. *facepalm*

Nope, needs work. Tests are failing. And since many of the test fixes are related to the 10.1 deprecations, feel free to reduce compatibility to 10.1+.

"Nope" to what? ... Sorry it's not my week. RTBC came from you. Maybe language barrier ... ?

Sorry, that was a bit terse. My "No" was in reference to my setting this as RTBC. It isn't ready for that as the tests fail on D11 and that needs to be resolved before this can go back to that state.

Link to the failing test: https://git.drupalcode.org/project/conditional_fields/-/jobs/2555379

Thanks for coming back on this and helping me understanding. No need to sorry :) It would be surely obvious to me usually if I wouldn't be so tired at the moment. In that condition non native languages become more and more cloudy over the night, hah.

composer.json got added upstream I found it easier to create a new branch.

Looks like we're missing a dependency on jquery somewhere.

This seems to be a duplicate of #3478862: Drupal 11 compatibility fixes for conditional_fields

The opposite is the case. We usually mark the newer as duplicate in favour of the former, and if the newer has more progress on the exact same issue it should be merged into the former to not loose track of overlaps and do not loose credits on both efforts.

Thanks @mably for pushing your efforts from the duplicate into the original issue. +1 Reviews to move on quickly would be great.

Adding the IS update tag. There are 3 open MRs and several patches. Can we get some notes added here on which should be reviewed (and please document why a previous MR/patch can't be used any longer).

@heddn Agreed in terms of clutter. That was not intended this way. But please lets cherish and value the contributions at first. Read the last issue comments here and on the duplicate issue for a birds eye view. It just takes some minutes. It took me 1 hour to sort the motivations of each issue and I try to prevent additional resources wasted and explained clearly that I tried to merge forces here. And it seemed that it hasn't been cared about that 2 issues where co-existing and running MRs parallel without somebody chiming in to clarify. It should be handled respectful on each of the issues, that contributors put efforts in them. And it would not be OK to close this one here in favour of the newer one, which was already RTBC or the other way around. In both issues work has been done which could possibly be interacted in each other. Which needs inlaying changes line by line, actually. And that's what I asked for. I only can ask the contributors to cooperate. I cannot force them to do it the way which is most useful to you or me or others passing by. Possible reason for different MRs could be that others can not add into the previous MRs? Not checked yet. I need to take time to look deeper into each of them (like anyone could do) but I am on the road since some days because my brother died tragically some days ago and my heart is broken and we are about to bring the family together and bury him in the next week. So, my apologies for only conducting without going into the lines at this moment.

It would be interesting to know if last MR in here passes tests.

#24: I asked for support and contribution on this issue and I am thankful for each effort but I am not sure if another branch was helpful at this moment.

#28: I actually asked for merging your efforts into this issue here and not just creating another MR without comments. But I am thankful for all contributions.

I created a new one because I don't like pushing to the project bot only branch and we had to rebase cause the paragraphs issue also created the composer.json.

I'm happy to close mine.

Looking briefly at the new one, it has a lot of changes unrelated to drupal 11 support: 43 files + 624 − 661.

Whether that is acceptable is left up to the maintainers, but that is a lot more to review than the changes in my MR, though my MR still needs the jquery dependency issue resolved.

That's a very helpful quick review @nicxvan. Thank you.

EDIT: In fact it is not recommended to push into the bot MR and from a glimpse I think the last MR worked on before your additions was not the bot one. Not sure yet. But as you have explained that has been done for different reasons. Anyways your try to help is very much appreciated. As always .

• Bot one is 43.
• Mine is 59.
• Newest is 62.

Sorry I edited my comment. I added:

EDIT: In fact it is not recommended to push into the bot MR and from a glimpse I think the last MR worked on before your additions was not the bot one. Not sure yet. But as you have explained that has been done for different reasons. Anyways your try to help is very much appreciated. As always .

Mine previously of yours was 54 I think.

Never mind, let's merge forces and let's try to pick best of all to narrow down this issue to a reviewable state. I will try my best this night to take a look into each of the MRs deeply...

Looking briefly at the new one, it has a lot of changes unrelated to drupal 11 support: 43 files + 624 − 661.

Hm, that's not good. @mably would you mind to address these reported flaws in your MR?

Just to clarify, whether those code style changes are acceptable here is up to the maintainer. It's not an inherent issue.

If you're ok with addressing code style issues here too then we can review it as is.

I am fully with you and heddn on this. And please let us do not waste your or other users resources. I asked the contributor from the duplicate issue to add his "ideas" into the original issue while I had to close his ones and he was kind enough to do so. But of course I did not meant by asking to only copy paste it without looking at the issue scope, the issue parts already fixed and without checking which of his additions are obsolete, or useful, still missing, and which not. We should start from the MR with the most progress and less things left to do.

This would be a bit easier to review if this wasn't such a mix of code quality and Drupal 11 fixes. For future note, let's keep those distinct. I've posted some feedback on the MR. Hopefully not too onerous.

Let's call this good enough. I'll leave it a few days, maybe a week before I cut a new release in case something crops up between now and then. But the dev branch is now D11 compatible. Thanks everyone for you work here.

Thanks for the work for D11 compatibility, everyone! Since this only drops support for Drupal 8, which is unsupported, it would be wonderful to see even a 4.0.0-alpha6 release of this module that includes this change. Thanks!

https://www.drupal.org/project/conditional_fields/releases/4.0.0-alpha6 tagged today

Thanks for your efforts:)