While improving our test, I noticed that hook_entity_insert() (and presumably the other entity hooks) don't get passed an actual flagging entity.

  stdClass::__set_state(array(
     'flagging_id' => '1',
     'flag_name' => 'flag_hook_test_flag',
     'entity_id' => '1',
     'uid' => '2',
     'sid' => 0,
  )),

This is missing a lot of the properties a Flagging has if you load it from the database.

Comments

joachim’s picture

joachim CreditAttribution: joachim commented

Calling flagging_load() gets me an object with these properties:

flagging_id
fid
entity_type
entity_id
uid
sid
timestamp
flag_name
rdf_mapping

The rdf_mapping is added by core's RDF module I presume.

joachim’s picture

joachim CreditAttribution: joachim commented
Parent issue: » #2202969: clean up flag() and its low-level helpers
joachim’s picture

joachim CreditAttribution: joachim commented
Status: Active » Fixed

I took a look at this and found that the hooks get a proper entity. This seemed odd, as I've done nothing to fix it yet.

So I figured that the refactoring done in the parent issue had fixed it as a byproduct: WOOHOO!

Except that comparing the before and after, I don't see how.

Then I noticed this:

    $this->assertEqual($hook_data_variable['hook_entity_presave']['parameters'][0]->flag_name, $flag->name, "The Flagging entity passed to hook_entity_presave() has the expected flag name property.");
    //$this->assertEqual($hook_data_variable['hook_entity_insert']['parameters'][0]->fid, $flag->fid);
    //$this->assertEqual($hook_data_variable['hook_entity_insert']['parameters'][0]->entity_type, 'node');

Spot the mistake: the hook name (the first level key in the $hook_data_variable array) changes!

So it's quite likely this was never a bug, just a mistake in the tests I was writing due to copy-pasting assertions, and I jumped to the wrong conclusion because the code that was being tested was convoluted and hard to read...

Committing a change logged to this issue that restores all the commented-out assertions.

  • Commit a2411a9 on 7.x-3.x by joachim: 
    Issue #2196055 by joachim: Added testing of Flagging properties,...

Status: Fixed » Closed (fixed)

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