Update actions & conditions to use doExecute / doEvaluate

Hi Josef, looks like low hanging fruit and the right task for me (and hopefully others) to get started. Don't have the big picture of rules and D8 in general yet so bear with me :D
Pull request: https://github.com/fago/rules/pull/232

I'll grab the DataIsEmpty condition.

[#2572945: doExecute / doEvaluate for DataIsEmpty], PR https://github.com/fago/rules/pull/235

Grabbing DataComparison ([#2573045: doEvaluate for rules_data_comparison]).

PR https://github.com/fago/rules/pull/236.

While waiting for the Barcelona to finish its breakfast...

Grabbing class DataListContains (condition_id = rules_list_contains)

PR https://github.com/fago/rules/pull/238 for [#2574513: doEvaluate for rules_list_contains]

Grabbing condition_id = rules_entity_field_access (class UserHasEntityFieldAccess).

I've listed all the execute()functions that are used in Actions.
But the function is also used in 2 core functions and 3 tests, not sure if those functions should be updated as well:
RulesActionBase
ExpressionBase
TestLogAction
TestNodeAction
TestStringAction

Boris,

I've listed all the execute()functions that are used in Actions.
But the function is also used in 2 core functions and 3 tests, not sure if those functions should be updated as well:
RulesActionBase
ExpressionBase
TestLogAction
TestNodeAction
TestStringAction

Generally speaking, in the tests you want to call evaluate() or execute(), because you want the system to dispatch the action or condition. Also: remember that doExecute() and doEvaluate() are deliberately declared "protected". You can't call them in tests. So you have to use these in test classes.

In RulesActionBase, execute() is the magic glue that calls doExecute() on the action plug-in by finding the context and setting up the call to the plugin's doExecute() method. This is only called if the plugin does not override execute(). So to do your change, we need to remove the plugin's execute call, and replace it with doExecute() with the arguments in the same order as the annotation's context items.

RulesConditionBase does the same job with conditions and doEvaluate. So it must implement evaluate() as a public method.

This sometimes does not work right now due to #2573823: Context in Rules plugins does not respect annotation order. Some tests will fail until this issue is resolved.

I haven't looked at any expressions yet, but I think the principle's the same. So ExpressionBase needs to know how to set up a call to doExecute(), I think.

After looking at this a bit, expressions work a bit differently. Typically, they get passed a RulesState object. So we probably should not include them under this issue. They're consumers of context.

I grabbed VariableAdd, and here's the PR: https://github.com/fago/rules/pull/259

Associated issue: #2583861: doExecute for VariableAdd

I grabbed DataListItemRemove and here's the PR: https://github.com/fago/rules/pull/260

Associated Issue: #2585019: doExecute for DataListItemRemove

I grabbed UserIsBlocked and here's the PR: https://github.com/fago/rules/pull/262

Associated Issue: #2585321: doEvaluate for UserIsBlocked

I grabbed PathHasAlias and here is the PR: https://github.com/fago/rules/pull/263

Associated Issue: #2585403: doEvaluate for PathHasAlias

Snagged the PathAliasExists condition. PR here: https://github.com/fago/rules/pull/264

Associated Issue: #2585419: doEvaluate for PathAliasExists

I grabbed NodeIsSticky and here's the PR: https://github.com/fago/rules/pull/265

Associated Issue: #2585429: doEvaluate for NodeIsSticky

I grabbed NodeIsPublished and here's the PR: https://github.com/fago/rules/pull/266

Associated Issue: #2585431: doEvaluate for NodeIsPublished

I grabbed NodeIsPromoted and here's the PR: https://github.com/fago/rules/pull/267

Associated Issue: #2585435: doEvaluate for NodeIsPromoted

Snagged the NodeIsOfType condition. PR here: https://github.com/fago/rules/pull/268

Associated Issue: #2585463: doEvaluate for NodeIsOfType

Snagged EntityIsOfType with a PR here: https://github.com/fago/rules/pull/269

Snagged EntityIsOfBundle condition. PR here: https://github.com/fago/rules/pull/270

Snagged EntityIsNew condition. PR here: https://github.com/fago/rules/pull/271

Snagged EntityHasField condition. PR here: https://github.com/fago/rules/pull/272

Snagged DataListCountIs condition. PR here: https://github.com/fago/rules/pull/272

Added last *not-refactored* conditions:
- DataIsEmpty
- UserHasRole

DataListItemAdd - created pull request - 299 (reeller_sk): needs work

can we mark items that needs work like this in the summary? It's hard to figure out what needs review and what not else.

Conditions are all done, yay!

Only one action remaining and updating the documentation.

Which item is remaining, Are you referring to PR #299.

Update DataListItemAdd action to use doExecute().
PR: https://github.com/fago/rules/pull/326

merged, thanks!

I think that was all for this issue, thanks everyone!