Automated Drupal 11 compatibility fixes for commerce_reports

This is an automated patch generated using Upgrade Status and Drupal Rector. Please see the issue summary for more details. A merge request is also openend and updated.

It is important that any automated tests available are run and that you manually test the changes.

Drupal 11 Compatibility

According to the Upgrade Status module → , even with these changes, this module is not yet compatible with Drupal 11.

Currently Drupal Rector, version 0.20.1, cannot fix all Drupal 11 compatibility problems.

Therefore these changes did not update the info.yml file for Drupal 11 compatibility.

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

Debug info

This patch was created using these packages:

1. drupal/upgrade_status: 4.1.0
2. mglaman/phpstan-drupal: 1.2.7
3. palantirnet/drupal-rector: 0.20.1

The last submitted patch, 2: commerce_reports.8.x-1.x-dev.rector.patch, failed testing. View results →

This comment was forced and has ignored the check if a change was already posted. This is only done when we want to update the issue without waiting for changes to happen.

This is an automated patch generated using Upgrade Status and Drupal Rector. Please see the issue summary for more details. A merge request (MR) is also openend and updated.

It is important that any automated tests available are run and that you manually test the changes.

Drupal 11 Compatibility

According to the Upgrade Status module → , even with these changes, this module is not yet compatible with Drupal 11.

Currently Drupal Rector, version 0.20.1, cannot fix all Drupal 11 compatibility problems.

Therefore, these changes did not update the info.yml file for Drupal 11 compatibility.

The compatibility issues that Upgrade Status found after the Drupal Rector fixes were applied are attached to help you resolve them manually.

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

Debug information

These packages were used to generate the fixes:

1. drupal/upgrade_status: 4.1.0
2. mglaman/phpstan-drupal: 1.2.10
3. palantirnet/drupal-rector: 0.20.1

The last submitted patch, 4: commerce_reports.1.x-dev.rector.patch, failed testing. View results →

The test failure seems not related to the current issue ; updating the status to needs review for branch 3428485-manual-drupal-11.

Drupal 11 is out so this is important, also for Commerce 3.x compatibility. I think we should also add a 3.x branch here for clarification.

grevil → changed the visibility of the branch project-update-bot-only to hidden.

grevil → changed the visibility of the branch 3428485-manual-drupal-11 to hidden.

There were a couple of issues with the original branch, I created a new one, with the D11 changes.

Nice @maintainers could you maybe create a 3.x branch to comply with commerce 3.x? Thanks!

Need maintainer feedback on https://git.drupalcode.org/project/commerce_reports/-/merge_requests/20#....

All other issues are blocked by this one as tests are currently failing against D11. I left a comment on the MR (requested feedback).

Adjusted accordingly @zaporylie, let's see if the tests succeed.

grevil → changed the visibility of the branch 3428485-TESTS-WILL-FAIL to hidden.

I am fairly sure the test failures are unrelated, but I can't prove it. So back to NW because of the test failures.

nicolasgraph → closed merge request !18

@grevil, not sure which of your MR is the one you're woking on but I think you should keep the change I made to ReportsListBuilder.php in the MR#18. See #3342975 📌 Deprecate Url::toRenderArray() and Url::renderAccess() Fixed + #3406432 📌 Remove calls of deprecated toRenderArray() method Fixed .

I'm looking at the test failures, first concentrating on kernel test. The failures in OrderReportGeneratorTest are mildly concerning. Given the following stack trace:

Drupal\Tests\commerce_reports\Kernel\OrderReportGeneratorTest::testGenerateReports
Error: Call to a member function getPattern() on null

/builds/issue/commerce_reports-3428485/web/core/lib/Drupal/Core/Datetime/DateFormatter.php:133
vfs://root/sites/simpletest/94104550/files/php/twig/674dca7cb14c7_commerce-order-receipt.ht_3Oi4ZV8pyQw7FpmRkQSEdRWRT:90
/builds/issue/commerce_reports-3428485/vendor/twig/twig/src/Template.php:393
/builds/issue/commerce_reports-3428485/vendor/twig/twig/src/Template.php:349
/builds/issue/commerce_reports-3428485/vendor/twig/twig/src/Template.php:364
/builds/issue/commerce_reports-3428485/vendor/twig/twig/src/TemplateWrapper.php:35
/builds/issue/commerce_reports-3428485/web/core/themes/engines/twig/twig.engine:33
/builds/issue/commerce_reports-3428485/web/core/lib/Drupal/Core/Theme/ThemeManager.php:348
/builds/issue/commerce_reports-3428485/web/core/lib/Drupal/Core/Render/Renderer.php:446
/builds/issue/commerce_reports-3428485/web/core/lib/Drupal/Core/Render/Renderer.php:203
/builds/issue/commerce_reports-3428485/web/core/lib/Drupal/Core/Render/Renderer.php:120
/builds/issue/commerce_reports-3428485/web/core/lib/Drupal/Core/Render/Renderer.php:593
/builds/issue/commerce_reports-3428485/web/core/lib/Drupal/Core/Render/Renderer.php:119
/builds/issue/commerce_reports-3428485/web/modules/contrib/commerce/commerce.module:73
/builds/issue/commerce_reports-3428485/web/core/lib/Drupal/Core/Extension/ModuleHandler.php:407
/builds/issue/commerce_reports-3428485/web/core/lib/Drupal/Core/Mail/MailManager.php:273
/builds/issue/commerce_reports-3428485/web/core/lib/Drupal/Core/Mail/MailManager.php:181
/builds/issue/commerce_reports-3428485/web/core/lib/Drupal/Core/Render/Renderer.php:593
/builds/issue/commerce_reports-3428485/web/core/lib/Drupal/Core/Mail/MailManager.php:180
/builds/issue/commerce_reports-3428485/web/modules/contrib/commerce/src/MailHandler.php:105
/builds/issue/commerce_reports-3428485/web/modules/contrib/commerce/modules/order/src/Mail/OrderReceiptMail.php:76
/builds/issue/commerce_reports-3428485/web/modules/contrib/commerce/modules/order/src/EventSubscriber/OrderReceiptSubscriber.php:63
/builds/issue/commerce_reports-3428485/vendor/symfony/event-dispatcher/EventDispatcher.php:246
/builds/issue/commerce_reports-3428485/vendor/symfony/event-dispatcher/EventDispatcher.php:206
/builds/issue/commerce_reports-3428485/vendor/symfony/event-dispatcher/EventDispatcher.php:56
/builds/issue/commerce_reports-3428485/web/modules/contrib/state_machine/src/Plugin/Field/FieldType/StateItem.php:425
/builds/issue/commerce_reports-3428485/web/modules/contrib/state_machine/src/Plugin/Field/FieldType/StateItem.php:392
/builds/issue/commerce_reports-3428485/web/core/lib/Drupal/Core/Field/FieldItemList.php:233
/builds/issue/commerce_reports-3428485/web/core/lib/Drupal/Core/Field/FieldItemList.php:198
/builds/issue/commerce_reports-3428485/web/core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php:938
/builds/issue/commerce_reports-3428485/web/core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php:979
/builds/issue/commerce_reports-3428485/web/core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php:896
/builds/issue/commerce_reports-3428485/web/modules/contrib/commerce/src/CommerceContentEntityStorage.php:56
/builds/issue/commerce_reports-3428485/web/modules/contrib/commerce/modules/order/src/OrderStorage.php:89
/builds/issue/commerce_reports-3428485/web/core/lib/Drupal/Core/Entity/EntityStorageBase.php:564
/builds/issue/commerce_reports-3428485/web/core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php:781
/builds/issue/commerce_reports-3428485/web/core/lib/Drupal/Core/Entity/EntityStorageBase.php:489
/builds/issue/commerce_reports-3428485/web/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php:805
/builds/issue/commerce_reports-3428485/web/modules/contrib/commerce/modules/order/src/OrderStorage.php:169
/builds/issue/commerce_reports-3428485/web/core/lib/Drupal/Core/Entity/EntityBase.php:354
/builds/issue/commerce_reports-3428485/tests/src/Kernel/OrderReportGeneratorTest.php:140

The failure is due to a failure while rendering the commerce-order-receipt.html.twig template. The offending line is where the order date is rendered:

              <div style="margin-top: 5px;"><span style="font-weight: bold;">{{ 'Order date:'|t }}</span> {{ order_entity.getPlacedTime|format_date('short') }}</div>

In short, the test setup is missing the core.date_format.short config entity. I am able to fix the error locally by inserting $this->installConfig(['system']); into the setUp() method. On the other hand, it would probably be better to disable order receipts in the first place while testing. Going to try that next.

The remaining test fail is a bit tricky:

Drupal\Tests\commerce_reports\Kernel\ReportType\OrderReportTest::testOrderReport
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
     'given_name' => 'Frederick'
     'additional_name' => null
     'family_name' => 'Pabst'
+    'address_line3' => null
 )

/builds/issue/commerce_reports-3428485/tests/src/Kernel/ReportType/OrderReportTest.php:129

I guess this is due to address issue ✨ Add address line 3 Fixed . I think the actual and expected values should just run through array_filter() before comparing them. It looks like commerce hides address_line3 by default, but order reports doesn't. Which is fine IMHO.

Addressed #24 in a slightly different way. Let's just follow a core example (e.g. ContactFormListBuilder).

Wow, great work @znerol! Code LGTM! I'll test it locally and come back here.

Alright, I just tested it and I run into the following issue: 🐛 "InvalidArgumentException: Field amount is unknown." when accessing "/admin/commerce/reports/orders" Active . Although this is completely unrelated to this issue, as it also happens pre patch, and is related to the "amount" entity field, and NOT the changes done for "$row['title']['data']".

When commenting out the "amount" related code inside "ReportsListBuilder" (which were untouched in this MR), everything works as expected! Meaning, the changes by @znerol targeting "$row['title']['data']" inside the "ReportsListBuilder" are working as expected!

So RTBC!

I'm curious? What is the change that forces to ditch compatibility < D10.3?
I think ReportDateField can be updated to not override the contructors by simply overridding the create method and calling the parent create() method. This way we don't have to keep track of the parent constructor parameters.

Can we change this so I can merge the MR?

• Calling createUser() with $values as the first parameter and/or $permissions as the second parameter is deprecated → is 10.1 (though since invocations are now using named parameters, this could work in previous core versions as well).
• Url::toRenderArray() and Url::renderAccess() are deprecated → is 10.1 (thogh Link::fromTextAndUrl() did exist before, thus could work with previous core versions as well)

I think ReportDateField can be updated to not override the contructors by simply overridding the create method and calling the parent create() method. This way we don't have to keep track of the parent constructor parameters.

This would be a BC break for any contrib / custom code which inherits from ReportDateField. Is that acceptable?

Also dug up the CR → for the views EntityField change. This change was introduced in Drupal 10.3.

There is no stable release yet. So let's forget about BC.

Added an hook_update_N in order to actually update the entity type definition.

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