Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Some fixes for Drupal 11.0.x compatibility #4354

Open
wants to merge 44 commits into
base: 2.x
Choose a base branch
from
Open

Some fixes for Drupal 11.0.x compatibility #4354

wants to merge 44 commits into from

Conversation

dafeder
Copy link
Member

@dafeder dafeder commented Dec 9, 2024 •
edited
Loading

Fixes #4356

Fixes to get DKAN build-able and passing with Drupal 11.0. I still haven't gotten the CI to work, because mysql 8.0, at least in ddev, doesn't allow the db user to turn off strict mode. MariaDB works fine. Need to decide whether to fix what's going on in mysql or switch to mariadb somehow in CI. But I'd like to get these changes in before they get stale.

The circleCI changes ended up not being needed since I couldn't get the Drupal 11 job to run anyway, but I left them in because they're probably useful in the future.

To test Drupal 11:

  1. ddev config --auto, ddev get getdkan/ddev-dkan
  2. Edit config.dkan.yml, change database to mariadb 10.6, php version to 8.3.
  3. ddev dkan-init --project-version=11.0.x
  4. ddev dkan-site-install
  5. ddev dkan-phpunit

Changes of note:

  • Full removal of Drupal's ContainerAwareEventDispatcher
  • Full removal of DKAN's LoggerTrait
  • Many PHPUnit data provider functions changed to be abstract
  • Replaced many calls to getMockForAbstractClass to simply getMock() due to deprecation. This also meant mocking, for instance, SQLite connection class instead of just the abstract connection class.
  • No more use of StatementWrapper in Drupal queries

@dafeder
Copy link
Member Author

dafeder commented Dec 10, 2024

Update: as of last push, fixed many errors, but still getting some from:

  • Drupal\Tests\dkan_js_frontend\Unit\SimpleSitemapArbitraryLinksAlterTest
  • Drupal\Tests\metastore_search\Unit\SearchTest
  • Drupal\Tests\metastore\Kernel\DataDictionarySettingsFormTest
  • Drupal\Tests\metastore\Unit\MetastoreServiceTest

Also will need

  • to re-run with --display-phpunit-deprecations to make sure there are no more of those
  • search whole repo for getMockForAbstractClass() which should really be replaced now
  • check for other deprecations in linter

@dafeder
Copy link
Member Author

dafeder commented Dec 12, 2024

Only test failing at this point seems to be AdminDatasetFileUploadTest::testCreateDatasetWithFileUpload(). I think this is related to something wrong with select2; I also see issues with the form when loading in my browser. May put this aside until there is a proper select2 release for D11.

@dafeder dafeder changed the title Trying some D11 compatibility Add Drupal 11.0.x compatibility Dec 12, 2024
@dafeder dafeder marked this pull request as ready for review December 16, 2024 19:28
@dafeder dafeder marked this pull request as draft December 16, 2024 19:30
@dafeder
Copy link
Member Author

dafeder commented Dec 19, 2024 •
edited
Loading

Another option, probably better - once all tests pass, revert changes to info.yml files and merge this, so that the other fixes don't get stale.

@dafeder dafeder changed the title Add Drupal 11.0.x compatibility Some fixes for Drupal 11.0.x compatibility Dec 19, 2024
@dafeder dafeder force-pushed the drupal-11-dan branch 2 times, most recently from ecbf9e9 to 4658533 Compare December 23, 2024 21:38
@dafeder dafeder force-pushed the drupal-11-dan branch 2 times, most recently from 4d36404 to eeabfce Compare January 16, 2025 18:09
dafeder added 24 commits January 28, 2025 12:26
@dafeder
More EventDispatcher Replacements
38dd2b0
@dafeder
Version constraint for datastore_mysql_import.info.yml
15f8a17
@dafeder
Fixed docs in DataDictionarySettingsForm
46081c2
@dafeder
Fix HarvestHashesEntityDatabaseTableTest
2865968
@dafeder
Fix MySQLQueryTest
dbb3c72
@dafeder
Add phpunit cache to gitignore
63a5e0e
@dafeder
Fix file validation in json_form_widget
515075e
@dafeder
Temporarily allow dev branch of select2
5497335
@dafeder
Fix one test
7a12226
@dafeder
Restore connection class
b68384c
@dafeder
Revert phpunit changes
cb83ea9
@dafeder
Use newer select2
38cbb97
@dafeder
Fix includes on DatabaseTableFactoryTest
6f6ff3c
@dafeder
Unit tests running again
b0380bf
@dafeder
Add D11 to matrix
d30f0bf
@dafeder
Tweaking circleCI
3a599aa
@dafeder
More circleCI
ae0b928
@dafeder
More circleCI
adc70e8
@dafeder
Typo
d027777
@dafeder
Just test for ya on runner
abd6e72
@dafeder
Use yq for config
ea420ff
@dafeder
debug
d1eb2e4
@dafeder
Fix mysql paramaters
da6d4ab
@dafeder
Remove D11 from matrix
eef0ef2
@dafeder dafeder marked this pull request as ready for review January 28, 2025 22:26
paul-m
paul-m requested changes Jan 30, 2025
View reviewed changes
Copy link
Contributor

@paul-m paul-m left a comment

Choose a reason for hiding this comment

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

We should update the tests on https://github.com/GetDKAN/recommended-project/tree/11.0.x since we don't need PHP 8.1 or PHP 8.2 any more.

composer.json Outdated Show resolved Hide resolved
paul-m
paul-m requested changes Jan 30, 2025
View reviewed changes
modules/datastore/tests/src/Kernel/Service/PostImportTest.php
@@ -8,10 +8,13 @@
use Drupal\datastore\PostImportResult;
use Drupal\datastore\Service\ResourceProcessor\DictionaryEnforcer;
use Drupal\datastore\Service\ResourceProcessor\ResourceDoesNotHaveDictionary;
use Drupal\Core\Database\Query\SelectInterface;
Copy link
Contributor

Choose a reason for hiding this comment

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

My IDE tells me these changes are unneeded.

modules/datastore/tests/src/Unit/Controller/QueryControllerTest.php
@@ -24,6 +24,7 @@
use Psr\Log\LoggerInterface;
use Symfony\Component\DependencyInjection\Container;
use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\EventDispatcher\EventDispatcher;
Copy link
Contributor

Choose a reason for hiding this comment

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

Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher is now an unneeded use.

modules/datastore/tests/src/Unit/Controller/QueryDownloadControllerTest.php
@@ -24,6 +24,7 @@
use Psr\Log\LoggerInterface;
use Symfony\Component\DependencyInjection\Container;
use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\EventDispatcher\EventDispatcher;
Copy link
Contributor

Choose a reason for hiding this comment

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

Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher is now an unneeded use.

@paul-m
Copy link
Contributor

paul-m commented Jan 30, 2025 •
edited
Loading

I guess we still need to add D11 to the testing matrix.
Never mind... The scope here is to have D10 compatible changes which get us ready for more changes in follow-ups, including CI changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@paul-m paul-m paul-m requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Drupal 11 Compatibility
2 participants
@dafeder @paul-m