\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.

Support from Acquia helps fund testing for Drupal Acquia logo

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
FileSize
9.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
FileSize
10.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.