Drupal 10 compatibility

Hi, I would like to ask, are you planning on updating the Fixed Block Content module to work in Drupal 10?

The changes themselves look good. I don't know what the maintainers plans for supporting the HAL migration to contrib but that seems to be the only issue connected to a Drupal 10 release. I might be able to help if we know what to do.hal seems to sort itself out and require the contrib module so that's a non-issue.

During testing I faced the error on "Set contents as default" action: Fatal error: Declaration of Drupal\fixed_block_content\Normalizer\BlockContentNormalizer::normalize($object, $format = null, array $context = []) must be compatible with Drupal\hal\Normalizer\ContentEntityNormalizer::normalize($entity, $format = null, array $context = []): ArrayObject|array|string|int|float|bool|null in /var/www/html/web/modules/contrib/fixed_block_content/src/Normalizer/BlockContentNormalizer.php on line 22

So I had to provide a new patch to fix it.

Needs to be reviewed according to the new patch

Yeah thanks, I was only reviewing this in D9. This correctly supports the 2.x HAL branch in Drupal 10 and I confirmed its backwards compatible with Drupal 9 and the 1.x branch.

Almost done, thanks contributors on this!. Unfortunately, tests didn't run well on D10. Changes:

• test compatibility review
• compatibility with D8 dropped
• updated CHANGELOG

Created fork + merge request with changes from #10. Only change I made was removing 'core' key from the fixed_block_content.info.yml file, as upon apply the patch in #10, I was getting the following error message:

Drupal\Core\Extension\InfoParserException: The 'core_version_requirement' constraint (^9 || ^10) requires the 'core' key not be set in modules/contrib/fixed_block_content/fixed_block_content.info.yml in Drupal\Core\Extension\InfoParserDynamic->parse()

#10 looks reasonable code-wise so i think this can be committed.

I saw the test configuration is outdated (i can help with that), so manually triggered a test. Looks like the module needs 'hal' as dev dependency, because it moved to contrib in D10. But that can be fixed in a fix-tests followup to unblock D10 adoption.

EDIT: Some links (1,2) indicate that removing the composer.json of the module and relying on d.o autogenerated shim is necessary in a situation like this.)

Hi everyone,

Thank you very much for your great help porting the module to D10 and for all the code contributions, it's greatly appreciated.

I would like to gladly confirm successfully testing the current fork from #11:

Tested upgrade with:

• PHP:8.1.16
• Drupal core: 10.0.7
• fixed_block_content from issue fork 3297241-drupal-10-compatibility at 7fe2ce82

Important note: Re: #9
In drupal 10 core module HAL:

has been deprecated in Drupal 9.4 and will be removed from Drupal 10.0. The project has moved to the contributed Hypermedia Application Language (HAL) module. The contributed module works with Drupal 9.4+ and Drupal 10.

https://www.drupal.org/node/3223395#s-recommendations-for-deprecated-mod...

Therefore, at this point the module should probably address adding an explicit dependency to contrib module hal, which is why I added the commit at #15.
Without an explicit dependency from the module to contrib hal, it needs to be added at the project's level (composer require drupal/hal).

We would greatly appreciate if you could please try testing and reviewing the latest changes, in particular, whether the changes to module's dependencies are acceptable and would work in this patch.

Could you please give us your feedback and let us know if the changes work as expected?

Feel free to let us know if you have any questions or concerns on any of the recent code changes, or this ticket in general, we would surely be glad to help.
Thanks in advance.

Yes i wondered if the contrib module works with D9 as well as D10, and you added docs that it does.
Great news.

Did you test that manually for D9 too?

Also it would be nice to trigger the testbot on that all, but i only see the button on the patches, but not MR !2 - any idea why?

Hi Axel,

Thanks a lot for your prompt and positive reply on this, it's greatly appreciated.

Glad we're on the same page for the module's dependencies.

Did you test that manually for D9 too?

ah no, not yet, unfortunately, I've mostly tested the fork/patch with the upgrade straight on D10.
So not tested the fork on a D9 yet, but if I get the time, I will make sure I report back here.

Also it would be nice to trigger the testbot on that all, but i only see the button on the patches, but not MR !2 - any idea why?

I've asked on slack at:
https://drupal.slack.com/archives/C1BB308HH/p1681914427058869
https://drupal.slack.com/archives/C51GNJG91/p1681915645086199
and I'm not entirely sure, but it might be because automatic testing isn't enabled for the module in issues... perhaps?
I've checked here: https://www.drupal.org/node/2919542/qa
and all I could see is a disabled test: disabled, testing configuration no longer available.

So maybe if there is at least one valid test configured to be automatically triggered with issues and commits, it should probably fire with changes to merge requests.

Therefore, short of getting the maintainer to configure an automated test for the module, we're down to uploading the exported MR as a patch file and triggering a test manually.
 

Thanks again very much for your great help moving this ticket forward.
Cheers!

Upload static patch of the latest MR.

Please, if there's a (more modern and maineainer-friendly) MR, don't duplicate that work in patches.
(If you use them for deployment, note so and hide.)
Hiding.

Latest MR seems OK for me, but unfortunately, starting from Drupal 10.1, block management pages have new paths and menu items. This change directly impacts on the Fixed Block Content UI, which relies on existing block management routes and paths.

To avoid further delays in a D10-compatible release, my proposal here is to restrict compatibility of the D10 initial version to 10.0.x core branch and continue the work for 10.1.x compatibility in a new issue.

Sounds like a steady plan. I think a lot of people are waiting on a 10.x stable release for this module.

MR merged, thank you everybody!

? what does ~10.0.0 mean ?

Most other projects are specifying only the major version like ^10

I'm confused, and upgrade_status says to collaborate with the maintainers of this project as if there's no upgrade yet available despite the fact we're using the latest release from just recently

Actually, any constrain that is accepted by \Composer\Semver\Semver::satisfies() can be used for core_version_requirement.

Hi Joseph,

As mentioned above at #22 and if you check the tests at #20, it seems the current code of the module would need to be changed slightly to be compatible with D10.1, thus the tests breaking for 10.1, if you look at #20.
As mentioned at #22 some routes and maybe other parts of module's code would need to be updated, which should be addressed in related ticket #3357439: UI updating to block management changes in Drupal core 10.1.

A new branch might have to be created to support versions ^10.1, otherwise, at this point, module's dev version should support ~10.0.0:
https://git.drupalcode.org/project/fixed_block_content/-/blob/8.x-1.x/fi...

and therefore be compatible with 10.0.x versions.

We hope we were able to answer your questions, but feel free to let us know if you encounter more issues or have more concerns, we would surely be glad to help.
Thanks!