Automated Drupal 11 compatibility fixes for layout_builder_st

This is an automated patch generated using Upgrade Status and Drupal Rector. Please see the issue summary for more details. A merge request is also openend and updated.

It is important that any automated tests available are run and that you manually test the changes.

Drupal 11 Compatibility

According to the Upgrade Status module → , even with these changes, this module is not yet compatible with Drupal 11.

Currently Drupal Rector, version 0.20.1, cannot fix all Drupal 11 compatibility problems.

Therefore these changes did not update the info.yml file for Drupal 11 compatibility.

Leaving this issue open, even after committing the current patch, will allow the Project Update Bot → to post additional Drupal 11 compatibility fixes as they become available in Drupal Rector.

Debug info

This patch was created using these packages:

1. drupal/upgrade_status: 4.1.0
2. mglaman/phpstan-drupal: 1.2.7
3. palantirnet/drupal-rector: 0.20.1

This comment was forced and has ignored the check if a change was already posted. This is only done when we want to update the issue without waiting for changes to happen.

This is an automated patch generated using Upgrade Status and Drupal Rector. Please see the issue summary for more details. A merge request (MR) is also openend and updated.

It is important that any automated tests available are run and that you manually test the changes.

Drupal 11 Compatibility

According to the Upgrade Status module → , even with these changes, this module is not yet compatible with Drupal 11.

Currently Drupal Rector, version 0.20.1, cannot fix all Drupal 11 compatibility problems.

Therefore, these changes did not update the info.yml file for Drupal 11 compatibility.

The compatibility issues that Upgrade Status found after the Drupal Rector fixes were applied are attached to help you resolve them manually.

Leaving this issue open, even after committing the current patch or merging the MR, will allow the Project Update Bot → to post additional Drupal 11 compatibility fixes as they become available in Drupal Rector.

Debug information

These packages were used to generate the fixes:

1. drupal/upgrade_status: 4.1.0
2. mglaman/phpstan-drupal: 1.2.10
3. palantirnet/drupal-rector: 0.20.1

This is an automated patch generated using Upgrade Status and Drupal Rector. Please see the issue summary for more details. A merge request (MR) is also openend and updated.

It is important that any automated tests available are run and that you manually test the changes.

Drupal 11 Compatibility

According to the Upgrade Status module → these changes make this module compatible with Drupal 11! 🎉
Therefore these changes update the info.yml file for Drupal 11 compatibility.

Leaving this issue open, even after committing the current patch or merging the MR, will allow the Project Update Bot → to post additional Drupal 11 compatibility fixes as they become available in Drupal Rector.

Debug information

These packages were used to generate the fixes:

1. drupal/upgrade_status: 4.3.2
2. mglaman/phpstan-drupal: 1.2.11
3. palantirnet/drupal-rector: 0.20.2

Now that Drupal 11 is out, any maintainer plans here?

There are quite a few issues to solve first, before this is D11 ready:

• 🐛 Error: Call to a member function getConfig() OverridesSectionStorage.php Active
• 🐛 Calling ClassResolver::__construct without the $container argument is deprecated in drupal:10.3.0 Active

And other issues, that are not listed in the issues tab. I'll take a look, if we can simply fix the combined.

grevil → changed the visibility of the branch 3431600-automated-drupal-11 to hidden.

grevil → changed the visibility of the branch 3431600-automated-drupal-11 to active.

grevil → changed the visibility of the branch project-update-bot-only to hidden.

Ok, this seems to do the trick! If merged, we can also close the child issues!

Make sure to credit @justcaldwell and @almador, which helped me to fix / point out the issues!

Unsure about existing installations, since the dependencies changed. Some people (not using drush and or composer) might have problems with the update.

Re #13 maybe create a new 2.x branch then?

We're changing the services.yml - don't we need to clear the container cache @grevil?

We're changing the services.yml - don't we need to clear the container cache @grevil?

Last time I checked this, clearing the container cache did not make a difference, but yea, I can add that.

Done.

Thanks!

This seems reasonable enough but I'm not entirely sold on the "override container services" approach, rather than decorating them, assuming they have interfaces (which they do). Also, there seems to be an out-of-scope change from another issue and I'd rather not commit that here.

Yea no idea. I haven't decorated a symfony service before, so I am unsure what I am doing wrong here.... https://symfony.com/doc/current/service_container/service_decoration.html is not very helpful to be honest.

The current implementation should be correct, as we autowire the class_resolver as the constructor argument, but I still get the following error:

[error] ArgumentCountError: Too few arguments to function Drupal\layout_builder_st\DependencyInjection\DecoratedClassResolver::__construct(), 0 passed in /var/www/html/web/core/lib/Drupal/Component/DependencyInjection/Container.php on line 259 and exactly 1

Even if I do it the non autowire way, like described in the docs, I get the exact same error.

If I add: class_resolver: ~ I get an error, that the decorated class couldn't get found (which makes sense, as we delete it, but its in the docs for some reason).

@phenaproxima do you have experience with decorating symfony services? Could you help me finish this?

want to set up a meeting to look at this together @Grevil?

Step #1:

Resolve this issue:
🐛 Error: Call to a member function getConfig() OverridesSectionStorage.php Active

Step #2, continue working on the rest.

Applied the patch from:
https://git.drupalcode.org/project/layout_builder_st/-/merge_requests/16...

And received the error:
PHP Fatal error: Uncaught ArgumentCountError: Too few arguments to function Drupal\layout_builder_st\DependencyInjection\DecoratedClassResolver::__construct(), 0 passed in /app/public_html/core/lib/Drupal/Component/DependencyInjection/Container.php on line 261 and exactly 1 expected in /app/public_html/modules/contrib/layout_builder_st/src/DependencyInjection/DecoratedClassResolver.php:17

Here is the list of my patches:
"drupal/layout_builder_st": {
"Issue #3420063: Error: Call to a member function getConfig() OverridesSectionStorage.php":
" https://www.drupal.org/files/issues/2024-03-29/layout_builder_st-call_to... →
3420063-26.patch",
Issue #3069964 Argument 1 passed to Drupal.. must implement interface Drupal null given":
" https://www.drupal.org/files/issues/2019-07-24/isTranslation-null-given.... → ",
"Issue #3431600: Automated Drupal 11 compatibility fixes for layout_builder_st":
"patches/layout_builder_st_3431600_16_2.patch"

Manual Dependency Injection with Decorator Pattern

• Manual Injection avoids relying on the #[AutowireDecorated] attribute, which is not natively supported in Drupal's DI system.
• Explicit Definition aligns with Drupal's preferred service configuration, ensuring compatibility with core patterns.
• Backward and Forward Compatible: This approach works with both current and future versions of Drupal.

Hi, Joseph!

Can't apply patch from:
https://git.drupalcode.org/project/layout_builder_st/-/merge_requests/16...

@almador , sometimes the patch file doesn't work depending on how the patch is being applied, try the diff file instead.

https://git.drupalcode.org/project/layout_builder_st/-/merge_requests/16...

With that said, I'm not done fixing.

Restored the original override approach as the decorator approach does NOT work. The class_revolver.inner parameter is not available and the service is not available.

Hi, Joseph and thank you for your efforts!

AFAIU, it's not the final version yet?

Tried both diff and patch in my local environment, they are not applying cleanly:
https://git.drupalcode.org/project/layout_builder_st/-/merge_requests/16...
https://git.drupalcode.org/project/layout_builder_st/-/merge_requests/16...

If I'm applying only 2 first patches (without the patch from this topic):

"drupal/layout_builder_st": {
"Issue #3420063: Error: Call to a member function getConfig() OverridesSectionStorage.php": "https://www.drupal.org/files/issues/2024-03-29/layout_builder_st-call_to_a_member_function_getconfig-3420063-26.patch",
"Issue #3069964: Argument 1 passed to Drupal.. must implement interface Drupal null given": "https://www.drupal.org/files/issues/2019-07-24/isTranslation-null-given.patch"
},

I'm receiving this error:
Symfony\Component\DependencyInjection\Exception\ServiceCircularReferenceException: Circular reference detected for service "class_resolver", path: "string_translation -> string_translator.locale.lookup -> config.factory -> config.typed -> class_resolver -> entity_type.manager". in Drupal\Component\DependencyInjection\Container->get() (line 149 of core/lib/Drupal/Component/DependencyInjection/Container.php).

@almador, delete your version, back off your other patches, grab the 8.x-1.x dev release clean clone or the latest tagged release and apply the merge request diff, see how that works.
The only other patch you might need other than the MR diff is this one: https://www.drupal.org/files/issues/2025-01-02/3069964-null-fix.patch →

I'll get back to reviewing this the next few days, with that said, follow my instructions, this diff is applying cleanly against the head of 8.x-1.x provided there's no conflicting patches being applied and I had no choice but to add two fixes from two other issues into this MR. The only fix I didn't include that I'm using elsewhere is this one: https://www.drupal.org/files/issues/2025-01-02/3069964-null-fix.patch →

Thank you so much,@ Joseph!

I followed your instructions and successfully applied 2 patches, at least I can now clear caches from Drupal admin interface without errors!

Here are my steps (if someone want to repeat them):
1. composer require 'drupal/layout_builder_st:1.x-dev@dev'

2. I applied the patch in composer.json

"drupal/layout_builder_st": {
"Issue #3069964: Argument 1 passed to Drupal.. must implement interface Drupal null given": "https://www.drupal.org/files/issues/2025-01-02/3069964-null-fix.patch"}

3. I downloaded https://git.drupalcode.org/project/layout_builder_st/-/merge_requests/16... to my patches folder and renamed it to 16.patch.

4. I added the new patch in composer.json (now we have 2 patches):

"drupal/layout_builder_st": {
"Issue #3069964: Argument 1 passed to Drupal.. must implement interface Drupal null given": "https://www.drupal.org/files/issues/2025-01-02/3069964-null-fix.patch",
"Issue #3431600: Automated Drupal 11 compatibility fixes for layout_builder_st":"16.patch"
}

5. composer install

I am going to mark this as RTBC as an iterative improvement, this is a significant improvement over what we currently have.

BEFORE:

The dev branch release and tagged releases of layout_builder_st are crashing on supported releases of Drupal core (D10.3 and will not install at all on D11).
The last successful test run was in July 2024 with Drupal core 9.5. Drupal 9.5 is no longer a supported release of Drupal core.

AFTER:

An installable Drupal 11 build
An installable Drupal 10.3 build
The module is able to install without crashing using this merge request code

The path forward:

I've updated the tests to make it a bit more obvious what is going on.

1 Jul 2024 at 00:50 EDT PHP 7.3 & MySQL 5.7, D9.5 40 pass →

Conclusion

While this module in it's current state IS broken on Drupal 10.3 / Drupal 11.1, this MR should be merged so that we can move forward with the various fixes.

Open followup issues to deal with the rest in a future release.

Marking this as critical due to the current state of compatibility. (if we do nothing, fatally broken project that basically crashes sites unless patching with this merge request).

Merging this MR is the best action forward so that we can focus on the remaining translation issue as seen in the test failures.

Can you run the tests locally and replicate the 403 statuses? Do you see 403 outside of tests?

 @liam morland,

these tests last succeeded under drupal 9.5

We hadn't EVER ran them on 10 until now, and take a look, I used the variables to go back to run 10.4, same thing as 11.0 and 11.1, same results.

If the results are the same on every version tested, then that does decouple fixing the tests from Drupal 11 compatibility.

Sure, with that said, this module crashes if installed with D10.4.0 unless patching. I've included those fixes in the MR.

Holy smoke, this is making way too many changes for us to merge it. If there's a bug here (and it seems like there might be), we should solve it in another issue and then only make compatibility fixes here.

It's impossible to review enormous patches (that make many sweeping changes) with confidence. Let's do this incrementally.

First things first: let's open a new issue to add .gitlab-ci.yml. It's okay if there are failures, I'll merge it anyway because it gives us a roadmap for what we need to fix next.

If it helps, in that issue we can drop support for Drupal <10.2.

It is preferable to get tests passing on the currently-supported version and then do the compatibility changes.

This project is 100% broken on current supported drupals (WSOD) unless patching.

Gotcha - let's fix that, starting with a new issue that enables GitLab CI.

it's as good as I can get it for now.

Ok so good news, the automated tests are stuck on a 403 error, this means that the test user lacks a permission.

Should be easy to figure out, likely related to the other fails. I'd say that this is a tests only failure. We've been using this merge request branch in our builds now and no issues have been reported yet.

Great @joseph.olstad so would you suggest setting this RTBC or should we first further investigate the test failure?

Perhaps at least it might make sense to get this into dev?

This merge request is the best we've got so far. To resolve the remaining fails, the best way would be to take this merge request branch, run manual tests against it to try to mimic the automated test failures and I'm sure that will make things obvious and lead to fixes. I just haven't had a chance to do this yet. Please do try this if you have any spare cycles to spend on this.

It'd be great to resolve this, probably easy.

We've been testing this merge request in a Drupal 11 upgrade for over a month now. Haven't had any issues with it yet.

With that said we haven't focused our testing 100% on layout builder but it is functional.

Could we please merge this and get an alpha tagged release?

This would allow us to replace one of our two remaining overrides in our distribution release build.

Would be great if we could get at least a dev release with this! Thank you! :)

@joseph.olstad maybe ping a maintainer?

Currently is four options on the table:

1. Fix the failing test and then ping the maintainer.
2. Fork this project and publish a release of the fork.
3. Use composer lenient. (I don't like this option)
4. Use an override approach (We're doing this) and wait until 1) or 2) is resolved.

What has already been done:

• 🐛 Make spelling corrections and exceptions Active
• 📌 Make tests pass Active
• 📌 Enable GitLab CI automated testing Active

These three changes were all merged into the above merge request.

Remaining work is:

• Fix the remaining fail in the automated test.

@liammorland, thanks, you fixed 3 errors. I just pushed a couple small changes, we'll see if it helps.

Small change I made was added in an attempt to resolve the 403 errors on the jsonapi

https://issue.pages.drupalcode.org/-/layout_builder_st-3431600/-/jobs/44...

If the change doesn't help (specifying jsonapi as a dependency) which I suspect it won't, perhaps lacking a permission as a 403 is an access denied error.

I did add some extra layout permissions also in an attempt to resolve the 200 status code assertion

I reverted the changes I attempted, they didn't fix anything.

@lkmorland, you're on a roll, I'll let you continue with this.

setting to active for @lkmorland
only 4 to go.

The scope of this PR is unmanagable in my opinion. For example, the first file in the PR is js/contextual.js. That file has nothing to do with D11 compatibility.

That issue is addressed in 🐛 Contextual links for translation are removed by core RTBC .

The PR in this issue goes against standard issue encapsulation practices and puts roadblocks to the community to participate. It's not even clear to me what should be tested here, as this has turned into a meta issue, rather than a D11 compatibility issue.

Humble suggestion: close issues with merges that are considered blockers first?

Thanks for the confidence in #55 and #56. Unfortunately, I will not likely have time to work more on this.

It would be good to get tests passing first, even if just on Drupal 10. I have them passing in 📌 Make coding standards fixes Active , though that required removing some of the tests.

The tests (and the module it'self) has been broken for quite some time yet thanks to the work in this issue not only is the merge request make the module functional in Drupal 11.1.3 but we're down to only 4 automated test errors thanks to the recent changes by @lkmorland.

The deprecations in 10.3/10.4 made things complicated, don't forget the massive changes in PHPUnit it'self which was significantly overhauled upstream and the switch from jenkins to gitlab lost our history of test results as those were flushed out.

We're very close to getting all the tests to pass.

@kmonty, there's already a duplicate open issue just as you wished for 📌 Automated Drupal 11 compatibility fixes for layout_builder_st Needs review

It has nothing but the bot changes however be warned, there are currently 16 errors in the automated tests.

I highly suggest you re-consider your opinion and start rowing with us here because we're down to only 4 errors in this issues merge request.

Would be nice to get some extra eyes on this and finish it.