Automated Drupal 11 compatibility fixes for toc_api

Thanks all. Need to fix

• phpstan
• upgrade status

Ok, it's as far forward as I can take it, this is probably "good enough" to merge. If someone can improve this then great. I say merge it, and cut an alpha or beta or rc release so that we can avoid doing overrides in our composer for Drupal 11 compatibility.

There's a multitude of fixes here. I fixed:

• cspell
• stylelint
• many phpstan errors were fixed
• deprecation fixes in the automated tests
• deprecation fixes in the module code/classes.

Prior to these changes there was nothing but failures everywhere.

Thanks for your contribution @joseph.olstad, but you cannot commit code and review it yourself.
Someone else need to review it and test it.

I recommend merging this and cutting a 1.6.0-beta1 release so that we can scrap our composer overrides (please!).

All you have to do is merge and tag the 8.x-1.x as 1.6.0-beta1

This way 1.5 stays as the recommended release and we can work on improving 1.6.0 in Drupal 11 without resorting to painful composer overrides that complicate upgrades for those using our distribution of Drupal (WxT).

the drupal ci/cd does the rest.

@joseph.olstad tests are almost green. There are just 2 phpunit tests that I don't know how to fix. There is 1 about a missing core service, that's confusing. And another 1 is about regex and I don't think we've changed anything there and this must have failed already before.

Could you probably have a look into that? Everything else is resolved.

Yes the missing core service is confusing. And for the regex, these tests were extremely broken prior to this merge request and they have deleted the historical jenkins results so I don't know when this was last working (if ever). This merge request is a vast improvement. With that said, we'll continue to use this merge request and review the results manually. Merging this would be good if we could merge and tag an alpha or beta or rc release for D11 that'd be a step in the right direction.

I'm testing this on core 11.1.1, using toc_api from this MR, along with toc_filter 2.3.0. I'm running into an error when saving a node that is using the [toc] token, assume the error is coming from toc_api:

TypeError: Drupal\Core\Render\MetadataBubblingUrlGenerator::generateFromRoute(): Argument #2 ($parameters) must be of type array, null given, called in /var/www/html/core/lib/Drupal/Core/Url.php on line 769 in Drupal\Core\Render\MetadataBubblingUrlGenerator->generateFromRoute() (line 104 of core/lib/Drupal/Core/Render/MetadataBubblingUrlGenerator.php).

Drupal\Core\Url->toString(1) (Line: 175)
Drupal\Core\Utility\LinkGenerator->generate(Object, Object) (Line: 102)
Drupal\Core\Render\Element\Link::preRenderLink(Array)
call_user_func_array(Array, Array) (Line: 107)
Drupal\Core\Render\Renderer->doTrustedCallback(Array, Array, 'Render #pre_render callbacks must be methods of a class that implements \Drupal\Core\Security\TrustedCallbackInterface or be an anonymous function. The callback was %s. See https://www.drupal.org/node/2966725', 'exception', 'Drupal\Core\Render\Element\RenderCallbackInterface') (Line: 825)
Drupal\Core\Render\Renderer->doCallback('#pre_render', Array, Array) (Line: 387)
Drupal\Core\Render\Renderer->doRender(Array, ) (Line: 203)
Drupal\Core\Render\Renderer->render(Array) (Line: 484)
Drupal\Core\Template\TwigExtension->escapeFilter(Object, Array, 'html', NULL, 1) (Line: 120)
__TwigTemplate_e9204f7060e2bc684929174b8b8efc1d->{closure}() (Line: 2058)
Twig\Extension\CoreExtension::captureOutput(Object) (Line: 88)
__TwigTemplate_e9204f7060e2bc684929174b8b8efc1d->macro_tree_links(Array) (Line: 67)
__TwigTemplate_e9204f7060e2bc684929174b8b8efc1d->doDisplay(Array, Array) (Line: 387)
Twig\Template->yield(Array, Array) (Line: 343)
Twig\Template->display(Array) (Line: 358)
Twig\Template->render(Array) (Line: 35)
Twig\TemplateWrapper->render(Array) (Line: 33)
twig_render_template('modules/contrib/toc_api/templates/toc-tree.html.twig', Array) (Line: 354)
Drupal\Core\Theme\ThemeManager->render('toc_tree', Array) (Line: 446)
Drupal\Core\Render\Renderer->doRender(Array, ) (Line: 203)
Drupal\Core\Render\Renderer->render(Array) (Line: 484)
Drupal\Core\Template\TwigExtension->escapeFilter(Object, Array, 'html', NULL, 1) (Line: 51)
__TwigTemplate_2dc37c9b3f055a0c562225983c06be41->doDisplay(Array, Array) (Line: 387)
Twig\Template->yield(Array, Array) (Line: 343)
Twig\Template->display(Array) (Line: 358)
Twig\Template->render(Array) (Line: 35)
Twig\TemplateWrapper->render(Array) (Line: 33)
twig_render_template('modules/contrib/toc_api/templates/toc-default.html.twig', Array) (Line: 354)
Drupal\Core\Theme\ThemeManager->render('toc_default', Array) (Line: 446)
Drupal\Core\Render\Renderer->doRender(Array, ) (Line: 203)
Drupal\Core\Render\Renderer->render(Array) (Line: 157)
Drupal\toc_api\TocBuilder->renderToc(Object) (Line: 184)
Drupal\toc_filter\Plugin\Filter\TocFilter->process

Hmm,
Seems there's smoke in the twig files?

templates/toc-tree.html.twig

and

templates/toc-default.html.twig

and this

#pre_render callbacks must be methods of a class that implements \Drupal\Core\Security\TrustedCallbackInterface or be an anonymous function. The callback was %s. See https://www.drupal.org/node/2966725',

jurgenhaas, want to take a look?

I'm not deep into twig, so this is certainly a task for someone else.

ok @jurgenhaas, I'll have to go through the test scenarios manually in my environment. Hopefully some time this week or next week!

Thanks for your work on this!

Seems like an issue was fixed in #3487659: TypeError: ..\MetadataBubblingUrlGenerator::generateFromRoute(): Argument #2 ($parameters) must be of type array.
Can you have a look?

Re "these tests were extremely broken prior to this merge request": The current merge request adds the .gitlab-ci.yml file. I suggest committing #3384686: Add testing configuration. This adds .gitlab-ci.yml and gets the tests passing. Use that issue to get tests passing and then ensure that they keep passing in the current issue.

Need work based on CI.

I am looking at the PHPSTAN issues, is the disabledTestRender() test method really needed?

tests/src/Kernel/TocManagerTest.php

  /**
   * Tests TOC rendering. Temporarily disabled.
   *
   * @return bool|string
   *   This return a string or a boolean value.
   */
  public function disabledTestRender() {
    /** @var \Drupal\Core\Render\RendererInterface $renderer */
    $renderer = \Drupal::service('renderer');
    /** @var \Drupal\toc_api\TocManager $toc_manager */
    $toc_manager = \Drupal::service('toc_api.manager');
    /** @var \Drupal\toc_api\TocBuilder $toc_builder */
    $toc_builder = \Drupal::service('toc_api.builder');

I fixed all of the PHPSTAN / PHPCS issues that were popping up in the tests/ directory, I ran the tests and they all passed now, I'm not familiar with ths module so I'll need to check if I broke other components of the module by fixing this:

daniel@drupal:/var/www/html$ ./vendor/bin/phpunit web/modules/contrib/toc_api/tests/ --verbose -c web/core/phpunit.xml
PHPUnit 10.5.45 by Sebastian Bergmann and contributors.

Unknown option "--verbose"
daniel@drupal:/var/www/html$ ./vendor/bin/phpunit web/modules/contrib/toc_api/tests/ -c web/core/phpunit.xml
HTML output directory sites/simpletest/browser_output is not a writable directory.

PHPUnit 10.5.45 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.3.14
Configuration: /var/www/html/web/core/phpunit.xml

.................................                                 33 / 33 (100%)

Time: 00:02.034, Memory: 6.00 MB

OK (33 tests, 178 assertions)
daniel@drupal:/var/www/html$ 

I am aware that they are other issues other than the CI messages and the test directory, for now I am moving this to "Needs Review"

@danrod, great work, fixing the phpstan issues appears to have also fixed the automated tests. 33 tests pass.

Hi everyone and thank you for your effort.

I'm trying to use patch from the MR: https://git.drupalcode.org/project/toc_api/-/merge_requests/16 but for some reason not having luck, getting conflicts (can not be applied). EDIT: Patch I tried to apply: https://git.drupalcode.org/project/toc_api/-/merge_requests/16.patch

I tried the following branches as base:
- 8.x-1.7 (meaning 8.x-1.x branch with tag 8.x-1.7)
- 2.0.x-dev#89c0b377 using the latest commit hash (meaning 2.0.x branch with commit hash 89c0b377). This seems to be having the same tag 8.x-1.7 as the 8.x-1.x branch has (which is confusing).

I git-cloned the MR branch and made the patch manually from branch 3455312-drupal11-fixes against branch 8.x-1.x as base. The 2.0.x branch does not exists in the MR request issue repository (not being up to date with the original).
- The patch did not work and I got conflicts to resolve.
- I can see that the MR started on version 1.5 or so and then the conflicts were resolved (merged back) for: 1.6 and 1.7. But maybe something went wrong with the merge conflicts resolutions, I do know exactly.

I appreciate any advice on getting this MR into patch that I can apply to toc_api contrib.

@dabbor , sometimes you'll get better results using the diff instead of the patch, especially if you're doing a --dry-run test of it.

https://git.drupalcode.org/project/toc_api/-/merge_requests/16.diff

I just did a dry run of the diff against 2.0.x, and it's 100% clean.

2.0.x ▶
 ╰❯ $ patch -p1 --dry-run < 16.diff
checking file .cspell-project-words.txt
checking file css/toc.tree.css
checking file js/toc.menu.js
checking file js/toc_type.js
checking file modules/toc_api_example/toc_api_example.info.yml
checking file modules/toc_api_example/toc_api_example.module
checking file phpstan.neon
checking file src/Entity/TocType.php
checking file src/Plugin/Block/TocBlockBase.php
checking file src/Toc.php
checking file src/TocBuilder.php
checking file src/TocBuilderInterface.php
checking file src/TocFormatter.php
checking file src/TocFormatterInterface.php
checking file src/TocInterface.php
checking file src/TocManager.php
checking file src/TocManagerInterface.php
checking file src/TocTypeForm.php
checking file src/TocTypeListBuilder.php
checking file templates/toc-menu.html.twig
checking file templates/toc-tree.html.twig
checking file tests/src/Kernel/TocBuilderTest.php
checking file tests/src/Kernel/TocManagerTest.php
checking file tests/src/Unit/TocFormatterTest.php
checking file tests/src/Unit/TocTest.php
checking file toc_api.info.yml
checking file toc_api.install
checking file toc_api.module

A full apply of the diff works just as expected

2.0.x ▶
 ╰❯ $ patch -p1 < 16.diff
patching file .cspell-project-words.txt
patching file css/toc.tree.css
patching file js/toc.menu.js
patching file js/toc_type.js
patching file modules/toc_api_example/toc_api_example.info.yml
patching file modules/toc_api_example/toc_api_example.module
patching file phpstan.neon
patching file src/Entity/TocType.php
patching file src/Plugin/Block/TocBlockBase.php
patching file src/Toc.php
patching file src/TocBuilder.php
patching file src/TocBuilderInterface.php
patching file src/TocFormatter.php
patching file src/TocFormatterInterface.php
patching file src/TocInterface.php
patching file src/TocManager.php
patching file src/TocManagerInterface.php
patching file src/TocTypeForm.php
patching file src/TocTypeListBuilder.php
patching file templates/toc-menu.html.twig
patching file templates/toc-tree.html.twig
patching file tests/src/Kernel/TocBuilderTest.php
patching file tests/src/Kernel/TocManagerTest.php
patching file tests/src/Unit/TocFormatterTest.php
patching file tests/src/Unit/TocTest.php
patching file toc_api.info.yml
patching file toc_api.install
patching file toc_api.module

Remember, you have to patch against the HEAD of 2.0.x, meaning, you need the latest 2.0.x, so get the latest before you try applying the patch.

Hi @joseph.olstad,

Thank you very much for your advice, I'm happy to learn something new :)

I believe I understood the difference. The patch includes history of the commits and its application tries to do something like git merge while the diff provides just the differences which can be applied without any knowledge of commits or processes that led to the point.

The https://git.drupalcode.org/project/toc_api/-/merge_requests/16.diff works for me, so I'm adding a static patch file from it for referencing in composer (to have a static input that is not changing with the MR updates).

It seems that while saving the https://git.drupalcode.org/project/toc_api/-/merge_requests/16.diff as a static patch I made a mistake copy pasting it into the file instead of savign the https://git.drupalcode.org/project/toc_api/-/merge_requests/16.diff from browser and as such I misses some character encoding or so making the previous patch not working so I'm replacing it now.

Ok all tests are now passing, functionally toc_api works as expected from my testing. This is ready for review and a new tagged release!

Yes !
Is there any chance to reach out to the maintainers for a tagged release?

We need this merged and a release tagged asap, this is a release blocker for our distribution and we're looking to publish our Drupal 11 release in the next week or two at the latest!

Merging

Creditting

https://www.drupal.org/project/toc_api/releases/2.0.0

Checking crediting system.

patch for toc_filter

For comment #45
#3518549: Make TocFilterBlock::blockAccess() return type be compatible with parent
and
#3520464: Layout builder fatal error