I've a feeling the developer docs for Rules state that you should do:

module_invoke_all('foo');
rules_invoke_event('foo');

whereas we invoke rules from inside our implementation of hook_flag().

CommentFileSizeAuthor
#3 1720196.flag_.move-rules-invoke.patch2.64 KBjoachim

Comments

joachim’s picture

As seen on http://drupal.org/node/884554:

> Usually one also provides a hook for modules - in that case first invoke the hook, then the rules event.

joachim’s picture

And in EntityAPI, which is by the same author as Rules:

    // Invoke the hook.
    module_invoke_all($this->entityType . '_' . $hook, $entity);
    // Invoke the respective entity level hook.
    if ($hook == 'presave' || $hook == 'insert' || $hook == 'update' || $hook == 'delete') {
      module_invoke_all('entity_' . $hook, $entity, $this->entityType);
    }
    // Invoke rules.
    if (module_exists('rules')) {
      rules_invoke_event($this->entityType . '_' . $hook, $entity);
    }

That clinches it :)

joachim’s picture

Status: Active » Needs review
StatusFileSize
new2.64 KB

Note that this preserves the bug in #1719942: RulesEvaluationException with node unflagged event in the code that's moved.

joachim’s picture

I'm also tempted to replace this test:

          if (entity_get_info($this->entity_type)) {

with something based on the class of the current flag object being a child class of flag_entity, but that can wait.

joachim’s picture

Status: Needs review » Fixed

Testbot is happy, patch is trivial, go go go.

Issue #1720196 by joachim: Changed Rules event to be invoked after hook_flag() rather than within it.

Status: Fixed » Closed (fixed)

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