\Drupal\rules\Engine\ExpressionManager::createAction() requires the $configuration to be passed as array, however else we usually expect the ContextConfig object - e.g. when using addAction(). This is inconsistent and should be fixed by expecting ContextConfig here also.

CommentFileSizeAuthor
#5 context-config.patch10.27 KBtr
#3 context-config.patch9.7 KBtr

Comments

fago created an issue. See original summary.

tr’s picture

Assigned: Unassigned » fago

So, if I understand correctly, right now in our tests we do things like:

    $action = $this->rulesExpressionManager->createAction('rules_entity_save', ContextConfig::create()
      ->map('entity', 'unknown_variable')
      ->toArray()
    );

The ->toArray() is needed because the signature for createAction() is ExpressionManager::createAction($id, array $configuration)

What you're proposing is to change the signature to ExpressionManager::createAction($id, ContextConfig $config) (with similar changes for createRule(), createActionSet(), and createCondition()).

This simply requires us to change the function signatures and move the ->toArray() call into the above ExpressionManager methods rather than calling ->toArray() and passing the result to the ExpressionManager methods.

Right?

That seems to be an almost trivial task if I correctly understand what you're proposing.

tr’s picture

Assigned: fago » tr
Status: Active » Needs review
StatusFileSize
new9.7 KB

Status: Needs review » Needs work

The last submitted patch, 3: context-config.patch, failed testing. View results

tr’s picture

Status: Needs work » Needs review
StatusFileSize
new10.27 KB

  • TR committed 3a0d23a on 8.x-3.x
    Issue #2721211 by TR: Expression manager helpers do not require...
tr’s picture

Status: Needs review » Fixed

Committed. Feel free to re-open if I missed something.

Status: Fixed » Closed (fixed)

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