PivotModel's beforeDelete not working

[meysammahfouzi]

Expected behavior

beforeDelete should be called for PivotModel when a pivot record is deleted from database.

Actual behavior

beforeDelete is not called for PivotModel when a pivot record is deleted from database.

Reproduce steps

The problem has been described here in detail: http://stackoverflow.com/q/42740344/69537
And also discussed here: https://octobercms.slack.com/archives/general/p1489352096625225

October build

393

[meysammahfouzi]

@LukeTowers I put a break point on this line:

https://github.com/octobercms/library/blob/master/src/Database/Pivot.php#L88

And realized that when I remove a friend, the delete method of October\Rain\Database\Pivot class is not called. Is this how things should work? Then, which function in what class is called to delete the friend pivot data?

[LukeTowers]

Check the belongsToMany relationship trait, specifically the remove() method

[meysammahfouzi]

@LukeTowers @daftspunk I traced the code and this is what I've figured out so far:

When I add a new friend relationship, the following functions are called in order:

1. onRelationManagePivotCreate() in RelationController.php
2. public function save() in Database\Model.php
3. return $this->saveInternal in Database\Model.php
4. public function save() in Eloquent\Model.php
5. protected function finishSave() in Eloquent\Model.php
6. $this->fireModelEvent('saved', false); in Eloquent\Model.php

Therefor, the afterSave method of my FriendsPivot model is called at the end as expected.

But for removing a friend relationship, the following sequence happens:

1. onRelationManageRemove() in RelationController.php
2. public function remove() in BelongsToMany.php
3. public function detach() in BelongsToMany.php
4. $results = $query->delete(); in BelongsToMany.php

And that's all! No event is fired and therefor the beforeDelete method of my FriendsPivot model does not get called.

[LukeTowers]

Thanks @meysam, that’s very helpful! If you would like to take a stab at a PR that would fix the issue, then I would welcome it very much. Otherwise we’ll take a look at fixing this when we’ve got some time.

The support demand for October is increasing, while the available time the founders have to give remains the same. These issues will be handled in time, however there are ways you can help:

• Submit a pull request that introduces this feature and wait for it to be reviewed
• Post a bug bounty for this specific issue so that more people can become involved with creating a fix
• Make a financial contribution to the project so we can allocate more time towards making improvements like these that benefit everyone

[daftspunk]

Yes, unfortunately the events exist only at the model level, not at the 1-tier lower query level. So when delete() is called on the query, it can produce a statement like DELETE FROM table WHERE ... that acts on multiple records, blind to the model and its events.

There could be some ways around this

• by changing the delete() call to target the model instead of a query (only works if there is one record)
• spoofing the event somehow
• creating an entirely new event

Either way a test case should be created for this so the fix can be implemented and proven to be resolved. Please submit a Pull Request to the test plugin for investigation that demonstrates the issue. Don't forget to include step by step instructions how how to replicate it using the test plugin.

[daftspunk]

One other thing, try to avoid referencing slack in issues: not everyone has access, and only 10,000 messages are kept on slack. Eventually the OP reference will be destroyed.

[LukeTowers]

Closing as it has been over a month since any activity on this occurred.

[meysammahfouzi]

@daftspunk @LukeTowers
Hi. I submitted a PR to demonstrate the steps to reproduce this problem. I hope it helps to get it fixed 🙂

octobercms/test-plugin#45

[meysammahfouzi]

@LukeTowers @daftspunk Any idea how the pivot model can be accessed in onRelationManageRemove function? Then we could manually call fireEvent on it.

[LukeTowers]

@meysammahfouzi you should just able to call ->pivot on each related record to get the pivot model for that relation.

[meysammahfouzi]

@LukeTowers calling ->pivot on related records apparently returns empty.

[LukeTowers]

@meysammahfouzi what do you mean by "apparently"?

[meysammahfouzi]

@LukeTowers I mean if I am not doing it wrong! Because when I log $relatedModel->pivot, it's logged as an empty array [].

[LukeTowers]

@meysammahfouzi try $this->model->{$this->relationName}->pivot

[bennothommo]

Closing as it has been over a month since any activity on this occurred and we are trying to figure out what issues are still relevant. If this is still something that you would like to see through to fruition please respond and we can get the ball rolling.