[10.3] Convert RulesAction plugins and discovery to attributes

Ok, we're back to working.

I'm going to revert the changes to the ContextDefinition classes and put those changes into a separate issue.

ContextDefinition is too basic a function, which affects all Rules events, conditions, and actions. Significant changes to these classes shouldn't be buried in an issue about RulesActions, even though those changes are needed for this issue.

I will commit the ContextDefinition changes in a separate issue first, then come back to this ...

Current status:

RulesActions plugins were all re-written to use Attributes in addition to the (working) Annotations. The RulesActionManager was partially re-written to deal with:

1) Annotation-only discovery. This is overridden for Rules, and continues to work.
2) Attribute discovery. This almost works, not sure where the 4 new errors are coming from. See https://git.drupalcode.org/project/rules/-/jobs/2164620 for the failing tests.
3) Discovery of both Annotations and Attributes at the same time. This does not work yet. After 2) is fixed we can make more progress here, but as it stands now this is picking up the wrong ContextDefinition class - it uses the core ContextDefinition for some reason when it should be using the Rules version, and the core version does not have the Rules-added toArray() method. See https://git.drupalcode.org/project/rules/-/jobs/2164476 for the failing tests.

bsuttis → changed the visibility of the branch 3461559-10.3-convert-rulesaction to hidden.

bsuttis → changed the visibility of the branch 3461559-10.3-convert-rulesaction to active.

paramconverter_manager looks like it needs a mock for the Unit tests, but I don't know why this is happening all of a sudden - the tests all pass without this patch, and this patch does nothing that involves the paramconverter_manager. Likewise, I didn't find any core change notices that would indicate something about this has changed recently. But let me add the mock to see if it fixes the problem.

The error:

Drupal\Tests\rules\Functional\ActionsFormTest::testActionsFormWidgets
    with data set "32. User role add" ('rules_user_role_add',
    array('@user.current_user_context:cu...t_user', 'test-editor'), array(),
    array(), array('user'))
    Behat\Mink\Exception\ElementNotFoundException: Button with
    id|name|title|alt|value "edit-context-definitions-user-switch-button" not
    found.

actually did reveal a cut-and-paste problem that I made when re-writing all the Annotations as Attributes. Added a fix to the MR. Only a few problems left to track down for the Attributes-only discovery step in #4.

#8 is now fixed. But the paramconverter_manager mock still needs a little work. Only one other error left.

Still fumbling around on the paramconverter_manager, but I found the source of the only other error which was

    Drupal\Tests\rules\Kernel\ContextIntegrationTest::testAssignmentRestriction
    Failed asserting that null matches expected 'input'.

Added a fix for this to the MR.

OK, all tests now run with Attribute-based discovery.
The only thing left is to get discovery working for where there's a mix of Annotations AND Attributes. That requires work on the Rules-specific version of AttributeDiscoveryWithAnnotations.

As an aside, the whole paramconverter_manager issue was a red-herring. There was a wrongly typed Attribute parameter in one of the actions - it was set to entity:user instead of string. When Rules was passed a string in the tests, it saw it needed and entity:user and tried to automatically upcast the string. Upcasting to an entity *does* require the paramconverter_manager, which is why the Unit tests were trying to use it.

The type error triggered the paramconverter_manager error, which masked the original problem.

As we have functional tests for the upcasting, there is no need to try to do all the mocking in the Unit test base because we don't have any upcasting unit tests. Thus, I am going to remove those changes from the MR to reduce the complexity.

Current status:

RulesActions plugins were all re-written to use Attributes in addition to the (working) Annotations. The RulesActionManager was partially re-written to deal with:

1. Annotation-only discovery. This is overridden for Rules, and continues to work. See https://git.drupalcode.org/project/rules/-/pipelines/228372
2. Attribute-only discovery. This now works. See https://git.drupalcode.org/project/rules/-/pipelines/229391
3. Discovery of both Annotations and Attributes at the same time. This does not work yet.

I am changing the MR to turn on discover of plugins by BOTH Attributes and Annotations, so we can see the remaining issues and so Ican work on this last piece.

As I said in #4, errors like this:

    1)
    Drupal\Tests\rules\Unit\Integration\RulesAction\VariableAddTest::testExecute
    Error: Call to undefined method
    Drupal\Core\Plugin\Context\ContextDefinition::toArray()
    
    /builds/project/rules/src/Core/RulesActionBase.php:141
    /builds/project/rules/tests/src/Unit/Integration/RulesAction/VariableAddTest.php:28
    /builds/project/rules/vendor/phpunit/phpunit/src/Framework/TestResult.php:729
    /builds/project/rules/vendor/phpunit/phpunit/src/Framework/TestSuite.php:685
    /builds/project/rules/vendor/phpunit/phpunit/src/TextUI/TestRunner.php:651
    /builds/project/rules/vendor/phpunit/phpunit/src/TextUI/Command.php:146
    /builds/project/rules/vendor/phpunit/phpunit/src/TextUI/Command.php:99

make it clear that the core Attribute w/ Annotation discovery picks up the wrong ContextDefinition class - it is picking up the core ContextDefinition class (that does not have a toArray() method) rather than the Rules-specific one that does have the toArray() method.

Need to override how the core class AttributeDiscoveryWithAnnotations looks for annotations, but stupid core redefined that method as private so instead of simply overriding one method of AttributeDiscoveryWithAnnotations we have to copy the whole thing just to modify the private method.

That all worked. Great news. Now one more test - temporarily remove annotations from the Ban actions to make sure that this all works with a combination of Attribute discovery and Annotation discovery.

That worked too.
As far as I'm concerned, this is done.

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