Drupal 11 compatibility fixes for rules

@tim - could you please tag the actual ticket for D11.

Hello Tim,

It appears that there is no duplicate ticket, could you please assist us in finding it as we are currently working on fixing Drupal 11 compatibility issues.

Because there's nothing here, and the things that need to be addressed for D11 already have issues open. And if there's something specific not covered by an open issue, open a new specific issue.

I am re-opening this issue to keep track of the progress of D11 readiness work. I will also tag the related issue here so that we can work on the remaining deprecations.

I ran tests locally and can see the following error:

There were 124 errors:

1) Drupal\Tests\rules\Functional\ActionsFormTest::testActionsFormWidgets with data set "3. List item add" ('rules_list_item_add', array('node.uid.entity.roles', '1'), array(true, 'start'))
Behat\Mink\Exception\DriverException: Textual and file form fields don't support array or boolean values.

/Users/chandan.singh/Sites/acquia_cms/vendor/behat/mink-browserkit-driver/src/BrowserKitDriver.php:427
/Users/chandan.singh/Sites/acquia_cms/vendor/behat/mink/src/Element/NodeElement.php:118
/Users/chandan.singh/Sites/acquia_cms/vendor/behat/mink/src/Element/TraversableElement.php:166
/Users/chandan.singh/Sites/acquia_cms/docroot/modules/contrib/rules/tests/src/Functional/RulesBrowserTestBase.php:97
/Users/chandan.singh/Sites/acquia_cms/docroot/modules/contrib/rules/tests/src/Functional/ActionsFormTest.php:134
/Users/chandan.singh/Sites/acquia_cms/vendor/phpunit/phpunit/src/Framework/TestResult.php:729

2) Drupal\Tests\rules\Functional\ActionsFormTest::testActionsFormWidgets with data set "12. Entity save" ('rules_entity_save', array('node'), array(true))
Behat\Mink\Exception\DriverException: Textual and file form fields don't support array or boolean values.

/Users/chandan.singh/Sites/acquia_cms/vendor/behat/mink-browserkit-driver/src/BrowserKitDriver.php:427
/Users/chandan.singh/Sites/acquia_cms/vendor/behat/mink/src/Element/NodeElement.php:118
/Users/chandan.singh/Sites/acquia_cms/vendor/behat/mink/src/Element/TraversableElement.php:166
/Users/chandan.singh/Sites/acquia_cms/docroot/modules/contrib/rules/tests/src/Functional/RulesBrowserTestBase.php:97
/Users/chandan.singh/Sites/acquia_cms/docroot/modules/contrib/rules/tests/src/Functional/ActionsFormTest.php:134
/Users/chandan.singh/Sites/acquia_cms/vendor/phpunit/phpunit/src/Framework/TestResult.php:729

3) Drupal\Tests\rules\FunctionalJavascript\EventBundleTest::testEventBundleSelection
RuntimeException: Unable to complete AJAX request.

/Users/chandan.singh/Sites/acquia_cms/docroot/core/tests/Drupal/FunctionalJavascriptTests/JSWebAssert.php:117
/Users/chandan.singh/Sites/acquia_cms/docroot/core/tests/Drupal/FunctionalJavascriptTests/JSWebAssert.php:38
/Users/chandan.singh/Sites/acquia_cms/docroot/modules/contrib/rules/tests/src/FunctionalJavascript/EventBundleTest.php:62
/Users/chandan.singh/Sites/acquia_cms/vendor/phpunit/phpunit/src/Framework/TestResult.php:729

4) Drupal\Tests\rules\Unit\Integration\Condition\EntityHasFieldTest::testConditionEvaluation
Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException: You have requested a non-existent service "plugin.manager.field.field_type_category".

For issue: You have requested a non-existent service "plugin.manager.field.field_type_category". tagging the related issue.

Ran test locally on Drupal 10.3 (included commit) and all tests are passing, created separate issue #3455432: Fix failing tests for Drupal 11, hence requesting review.

Thanks @vishalkhode I have updated the MR with requested changes, please take a look.

Reviewed changes and looks good to me. Hence, moving it ahead.

No, this patch does not make Rules compatible with D11, and does break Rules for lower versions of Drupal. Some of the changes are unexplained and IMO wrong, and none of the issues in the summary are addressed.

For example:

-        ['immediate' => TRUE],
+        ['immediate' => '1'],

Because 'immediate' is a boolean variable, the existing test against TRUE is the correct way to do it. Why was this changed to '1'? And what does this change have to do with the original issue or D11 compatibility?

If this is to be a tracking issue for D11 compatibility, then things should NOT be fixed piecemeal here. Don't just throw a bunch of unrelated changes into a patch. A new issue should be open for each change, and the patches in those issues should not contain out-of-scope changes, and those issues should be linked here. There should be no patches here in this tracking issue.

This is part of the reason I closed the issue - I don't want to have to maintain a tracking issue and have to keep asking people to open new issues for each problem. And I don't want to have to keep trying to untangle dozens of patches for dozens of different things all posted here under "D11 Compatibility".

Actual D11 compatibility requires some structural changes, like turning on matrix testing with GitLab CI, porting the dependencies (typed_data), dropping D9 support, moving to semantic versioning for D10+, determining the lowest version of D10 that can be supported with these changes (see the original post where some of these changes only work for >D10.3), and yes addressing some of the deprecations.

No, this patch does not make Rules compatible with D11, and does break Rules for lower versions of Drupal. Some of the changes are unexplained and IMO wrong, and none of the issues in the summary are addressed.

1. I don't see any test failure on lower version of Drupal with this changes considering this CI → is passing for lower version, let me know what's breaking on lower version.
2. Regarding un unexplained changes: There are following two test which are failing on D10.3 and hence those changes are made
   

   1) Drupal\Tests\rules\Functional\ActionsFormTest::testActionsFormWidgets with data set "3. List item add" ('rules_list_item_add', array('node.uid.entity.roles', '1'), array(true, 'start'))
   Behat\Mink\Exception\DriverException: Textual and file form fields don't support array or boolean values.
   2) Drupal\Tests\rules\Functional\ActionsFormTest::testActionsFormWidgets with data set "12. Entity save" ('rules_entity_save', array('node'), array(true))
   Behat\Mink\Exception\DriverException: Textual and file form fields don't support array or boolean values.
   

3. Regarding issue summary: I have only committed those changes which are required to make this module compatible with D11 and rest are tagged as separate issue as suggested in the this issue (Don't commit unrelated changes here)

If this is to be a tracking issue for D11 compatibility, then things should NOT be fixed piecemeal here. Don't just throw a bunch of unrelated changes into a patch. A new issue should be open for each change, and the patches in those issues should not contain out-of-scope changes, and those issues should be linked here. There should be no patches here in this tracking issue.

I do agree with you on this point. The only changes I have done here to fix other D10.3 test failure which I can revert if you want.

This is part of the reason I closed the issue - I don't want to have to maintain a tracking issue and have to keep asking people to open new issues for each problem. And I don't want to have to keep trying to untangle dozens of patches for dozens of different things all posted here under "D11 Compatibility".

Yes! I have followed the same approach by creating separate issue unrelated to this ticket.

Actual D11 compatibility requires some structural changes, like turning on matrix testing with GitLab CI, porting the dependencies (typed_data), dropping D9 support, moving to semantic versioning for D10+, determining the lowest version of D10 that can be supported with these changes (see the original post where some of these changes only work for >D10.3), and yes addressing some of the deprecations.

I think we already have issues for things like turning on matrix testing with GitLab CI, dropping D9 support, moving to semantic versioning for D10+ etc and its been tagged here. Additionally I have also run the tests locally on Drupal 11 by including these issues fixes to see the remaining task and accordingly created separate issue specially for test failure(#3455432: Fix failing tests for Drupal 11).

With respect to Drupal 11 compatibility, I don't see any other pending issues that are not already reported and tagged here.

"I don't see any test failure on lower version of Drupal"
Yes currently that was true. But if you make all the changes proposed in this issue, it breaks Rules for core < 10.3.

"Regarding un unexplained changes: There are following two test which are failing on D10.3 and hence those changes are made"
Please open a separate [10.3] issue for those test failures!

I have opened other issues for the D10 deprecations noted above - please contribute patches in those issues.

I don't see anything in here that is D11 specific, other than the direct dependency changes, which are done wrong anyway because this module can't be D9 and D11 compatible at the same time, and things like the typed_data dependency need their versions changed as well.

I will fix the dependencies when all the D10 test issues have been resolved - that way we can test against D11 and see what real issues we have rather than just declaring this module D11 compatible even though it fails.

Again, this is why I didn't want an issue like this open. It is FAR MORE work to sort through all these unrelated changes and open new issues myself. So again, since there doesn't seem to be anything D11 specific here, and it's not serving as a tracking issue, and the specific issues raised are not being moved to their own issues like I requested, I am closing this issue again.

I just finished getting typed_data working in D11, which is a prerequisite for Rules, and there are other very specific issues that need to be solved before Rules can be ready for D11. In particular 📌 [10.3] Update ConditionManager to support PHP Attributes Active . Please help with those.