$result is used earlier in actions_do. It is later overriden with an ->execute() call. This should fix it.

... Not sure it's critical, but it's causing errors in testing on head.

Beginning of function....

  $actions = array();
  $available_actions = action_list();
  $result = array();
  if (is_array($action_ids)) {
    $conditions = array();
    foreach ($action_ids as $action_id) {

End of function....

    }
  }
  $stack--;
  return $result;

$result being overriden....

      $query = db_select('actions');
      $query->addField('actions', 'aid');
      $query->addField('actions', 'type');
      $query->addField('actions', 'callback');
      $query->addField('actions', 'parameters');
      $query->condition('aid', $conditions, 'IN');
      $result = $query->execute();
      foreach ($result as $action) {
        $actions[$action->aid] = $action->parameters ? unserialize($action->parameters) : array();
        $actions[$action->aid]['callback'] = $action->callback;
        $actions[$action->aid]['type'] = $action->type;
      }
CommentFileSizeAuthor
result.patch788 bytesRobLoach
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

RobLoach’s picture

Issue summary: View changes

Updated issue summary.

RobLoach’s picture

Issue summary: View changes

Updated issue summary.

effulgentsia’s picture

Status: Needs review » Reviewed & tested by the community

I see the variable collision in the code, but I don't see any HEAD tests that are currently broken, nor do I see a failure locally that this fixes. Regardless, removing the collision makes sense.

effulgentsia’s picture

Priority: Critical » Normal
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests

Actually, we should find out why there isn't a test in HEAD that catches this, and add it.

RobLoach’s picture

Status: Needs work » Needs review
Issue tags: -Needs tests

Sorry, but I'm not making a test for this. It's simple variable name conflict degregation from #1008166: Actions should be a module. Agreed that it's not really critical though.

sun’s picture

Component: other » action.module
Status: Needs review » Reviewed & tested by the community

Unfortunately, we pretty much removed each and every test coverage for actions through the removal of Trigger module.

So, two things:

1) Let's fix this blatant mistake.

2) Let's create a follow-up issue to add some serious test coverage for Actions module.

That is, because the entire functionality is pretty much untested, so adding a single, uncoordinated test assertion for this here doesn't really improve the situation.

catch’s picture

Status: Reviewed & tested by the community » Fixed

Created the follow-up: #1797658: Add test coverage for Actions module.

Committed/pushed the patch.

Status: Fixed » Closed (fixed)

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

Anonymous’s picture

Issue summary: View changes

Updated issue summary.