Automated Drupal 10 compatibility fixes

Carlos Romero → made their first commit to this issue’s fork.

The patch doesn't apply in my local.

eugene.brit → made their first commit to this issue’s fork.

This is an automated patch generated by Drupal Rector. Please see the issue summary for more details.

It is important that any automated tests available are run with this patch and that you manually test this patch.

Drupal 10 Compatibility

According to the Upgrade Status module → , even with this patch, this module is not yet compatible with Drupal 10.

Currently Drupal Rector, version 0.15.1, cannot fix all Drupal 10 compatibility problems.

Therefore this patch does not update the info.yml file for Drupal 10 compatibility.

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

Debug info

This patch was created using these packages:

1. mglaman/phpstan-drupal: 1.1.35
2. palantirnet/drupal-rector: 0.15.1

The last submitted patch, 11: entity_reference_uuid.2.0.x-dev.rector.patch, failed testing. View results →
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

I ran upgrade_status to check what are the missing manual fixes to apply. This patch seems to cover them all.

The last submitted patch, 13: 3287296-13.patch, failed testing. View results →
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

[#3197514]

Patch #15 got applied cleanly for Entity Reference UUID version 2.0.x-dev@dev on Drupal 9.5.10.
Checked on upgrade status 4.0.0, the Entity Reference UUID module is now compatible with Drupal 10 and there were no additional deprecation errors.

Reviewed & Tested by Community.

I am reviewing this issue.

It is a little confusing to have a merge request (MR) and also several patches. According to the comments, the MR was created based on one of the patches from the update bot. (But the first commit in the MR was an optimistic change to the .info.yml file.) I plan to update the MR, referring to the latest patch, and hide the patches.

One of the changes in the tests is to replace $this->verbose() with dump() (not $this->dump()). See the change record (CR) Usage of ::verbose() in tests is deprecated → . The dump() function (from Symfony's VarDumper component) was added in Drupal 9.2.0, so it makes some sense to declare compatibility with ^9.2 || ^10. But since that only affects tests, I suggest that we do not base compatibility on that change.

The early patches from the update bot added ->accessCheck(TRUE) to several entity queries (probably all of them). See the CR Access checking must be explicitly specified on content entity queries → . Adding an access check is generally a good idea: it prevents information disclosure. But it is a change in behavior. At this time, I am not reviewing for security, so I will accept the change in the most recent patch, which adds ->accessCheck(FALSE).

According to the CR drupal_set_message() and drupal_get_messages() replaced by Messenger service → , the Messenger service was introduced in 8.5.0, so we should not declare compatibility with earlier versions.

I installed the module with Drupal 10.2.x and ran the PHPUnit tests locally:

PHPUnit 9.6.15 by Sebastian Bergmann and contributors.

Testing /var/www/html/modules/contrib/entity_reference_uuid/tests/src
.                                                                   1 / 1 (100%)

Time: 00:00.985, Memory: 4.00 MB

OK (1 test, 15 assertions)

Remaining self deprecation notices (1)

  1x: Using a translatable string as a category for field type is deprecated in drupal:10.2.0 and is removed from drupal:11.0.0. See https://www.drupal.org/node/3375748
    1x in EntityReferenceUuidItemTest::testEntityReferenceFieldValidation from Drupal\Tests\entity_reference_uuid\Kernel

That CR is "New API for defining field type categories". It has instructions for making the field type annotations compatible with Drupal 10.2+ and with earlier versions. I implemented those instructions.

I tested with Drupal 10.1.x and 10.2.x. With both versions, I installed the Umami demo profile and added this module. I added a "Content by UUID" field to the Article content type. I edited an Article node, adding references to a couple of recipes. I updated the layout of Article nodes to show the new field. I viewed my edited article, and the recipes were listed. In the second test, I checked the database:

MariaDB [db]> select * from node__field_related_recipes_by_uuid;
+---------+---------+-----------+-------------+----------+-------+-------------------------------------------+
| bundle  | deleted | entity_id | revision_id | langcode | delta | field_related_recipes_by_uuid_target_uuid |
+---------+---------+-----------+-------------+----------+-------+-------------------------------------------+
| article |       0 |        12 |          39 | en       |     0 | 49ba3e96-8e2c-4e99-a3ce-8edd2b6efa09      |
| article |       0 |        12 |          39 | en       |     1 | d2dfd66c-bcd0-4d5b-874f-ec961276035b      |
+---------+---------+-----------+-------------+----------+-------+-------------------------------------------+
2 rows in set (0.000 sec)

As far as the previous changes (the patch in #15) are concerned, I consider this issue RTBC. Since I added further changes, I am setting the status back to NW.

I re-read the CR, and I had it backwards. Using accessCheck(TRUE) is backwards compatible (and also more secure).

There are two places where this comes up. (I trust the tools to find all places where entity queries are used.) In both cases, the parent class uses accessCheck(TRUE), so that is consistent (with the parent class) as well as more secure and BC.

I updated the MR.

I also noticed that one of the places where it comes up, Drupal\entity_reference_uuid\EntityReferenceUuidFieldItemList::processDefaultValue(), is now identical to the parent method. I considered removing the function entirely, but decided that is out of scope for this issue.

In the second place where it comes up, I also moved the line to be more consistent with the parent method.

I did not re-test.

It seems that the testbot does not catch Calling KernelTestBase::installSchema() for the tables key_value and key_value_expire is deprecated → .

This change was introduced in Drupal 9.1.0. It only affects tests, so again I suggest that we do not require compatibility for it.

I noticed that there is a file PATCHES.txt in the module's top-level directory, Someone should clean it up, but that is not in scope for this issue.

Automatically closed - issue fixed for 2 weeks with no activity.