Skip to content

Conversation

@stephen-cox
Copy link
Member

What does this change?

Add support for Drupal 11

How to test

Install in a Drupal 11 site.

@stephen-cox
Copy link
Member Author

I'm not too familiar with this module so not sure the best way to get this tested.

There are a couple of test failures for Drupal 11 on PHP 8.3

1) Drupal\Tests\localgov_forms_date\Kernel\DateWebformElementTest::testFormSubmission
TypeError: Drupal\Component\Utility\Html::decodeEntities(): Argument #1 ($text) must be of type string, null given, called in /app/web/core/lib/Drupal/Core/Form/FormBuilder.php on line 1380

/app/web/core/lib/Drupal/Component/Utility/Html.php:393
/app/web/core/lib/Drupal/Core/Form/FormBuilder.php:1380
/app/web/core/lib/Drupal/Core/Form/FormBuilder.php:1315
/app/web/core/lib/Drupal/Core/Form/FormBuilder.php:1003
/app/web/core/lib/Drupal/Core/Form/FormBuilder.php:1073
/app/web/core/lib/Drupal/Core/Form/FormBuilder.php:1073
/app/web/core/lib/Drupal/Core/Form/FormBuilder.php:571
/app/web/core/lib/Drupal/Core/Form/FormBuilder.php:495
/app/web/modules/contrib/localgov_forms/modules/localgov_forms_date/tests/src/Kernel/DateWebformElementTest.php:38
/app/web/modules/contrib/localgov_forms/modules/localgov_forms_date/tests/src/Kernel/DateWebformElementTest.php:22

2) Drupal\Tests\localgov_forms_lts\Kernel\LtsStorageForWebformSubmissionTest::testLtsDbUsage
LogicException: The database driver has no TransactionManager implementation

/app/web/core/lib/Drupal/Core/Database/Connection.php:1114
/app/web/core/lib/Drupal/Core/Database/Connection.php:1095
/app/web/core/lib/Drupal/Core/Database/Connection.php:227
/app/web/core/lib/Drupal/Core/Database/Database.php:455
/app/web/core/lib/Drupal/Core/Database/Database.php:383
/app/web/core/tests/Drupal/KernelTests/KernelTestBase.php:714

ERRORS!
Tests: 13, Assertions: 81, Errors: 2, Deprecations: 1, PHPUnit Deprecations: 1.

@nccchris
Copy link

I've used this to get through a test upgrade to D11. Afterwards it seems fine with some basic testing : I can edit an existing webform and create a new webform.

composer.json Outdated
"minimum-stability": "dev",
"require": {
"drupal/webform": "^6.0",
"drupal/webform": "^6.3@beta",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would we be able to stick to ^6.2 here? Just that this involves installing a beta that has quite a bit of rewrites. To install localgov_forms I think you need a minimum stability of dev anyway so it should allow webform 6.3 beta to install.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume you mean ^6.2 || ^6.3@beta?

The main difference between the two, at least at the beginning seems to have been 11 support. I've not checked how far they've diverged since.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree that the ^6.2 requirement should install 6.2 and 6.3 beta, but no harm in explicitly stating this, so have updated the requirement with Ekes' suggestion.

@ekes
Copy link
Member

ekes commented Apr 29, 2025

@Adnan-cds you got any idea about this
Just looked at the code and I think it's the mocked DbConnection::class that's used to moke the LtsDbConnection.

2) Drupal\Tests\localgov_forms_lts\Kernel\LtsStorageForWebformSubmissionTest::testLtsDbUsage
LogicException: The database driver has no TransactionManager implementation

/app/web/core/lib/Drupal/Core/Database/Connection.php:1114
/app/web/core/lib/Drupal/Core/Database/Connection.php:1095
/app/web/core/lib/Drupal/Core/Database/Connection.php:227
/app/web/core/lib/Drupal/Core/Database/Database.php:455
/app/web/core/lib/Drupal/Core/Database/Database.php:383
/app/web/core/tests/Drupal/KernelTests/KernelTestBase.php:714

@stephen-cox
Copy link
Member Author

Fixed the test failing on the missing TransactionManager by using the mysql one. Maybe we need to stub our own as I can't find a stub class for this.

@Adnan-cds
Copy link
Contributor

@Adnan-cds you got any idea about this Just looked at the code and I think it's the mocked DbConnection::class that's used to moke the LtsDbConnection.

I will have a better look and get back within a day.

Copy link
Member

@ekes ekes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Manual testing by @nccchris in addition.
Tests now pass thanks to @Adnan-cds 's addition since last week.

@finnlewis finnlewis merged commit 828efec into 1.x May 6, 2025
15 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants