Framework: Pivot events do not fire

Created on 16 Apr 2015  路  9Comments  路  Source: laravel/framework

When creating (or performing any other action on a Pivot table), no model events are fired.

There is a proof of concept repository here: https://github.com/tpavlek/pivot-events-test

WelcomeController@index creates a new user and a new role, and attaches the role to the user. in AppServiceProvider we register this in boot()

    Pivot::creating(function($event) {
        \Log::info("Creating pivot...");
    });

Even after running the function in WelcomeController no data is logged, meaning no Pivot creating event was fired.

Most helpful comment

@GrahamCampbell Is it possible to re-open/consider this?

There are a number of other issues/PRs/feature-requests all relating to the same thing;

10435 #10545 #10770 #4022 #2303 #1703

Some have workaround in app-level code but it seems widely expected that the framework should fire events on pivot table changes.

I think the limitation is that BelongsToMany doesn't use the Pivot model to interface with the DB but uses Query Builder directly; so doesn't benefit from the events Model provides.

Should BelongsToMany be updated to use the Pivot model - or - should BelongsToMany just fire it's own events? Or some other mechanism? It'd be good to get some feedback as to what a good approach here would be before coming up with a PR.

All 9 comments

has your user_role table an eloquent model?

@tpavlek There are no pivot events at all. Your model is not used by the BelongsToMany relation when attaching/syncing related models.

It would require changing the way this relation works, or adding such events to be fired. The latter was rejected by Taylor long time ago, can't remember why.

Closing due to inactivity.

@GrahamCampbell I don't really understand the justification for closing? Is this the same as labeling this a wontfix?

@GrahamCampbell Is it possible to re-open/consider this?

There are a number of other issues/PRs/feature-requests all relating to the same thing;

10435 #10545 #10770 #4022 #2303 #1703

Some have workaround in app-level code but it seems widely expected that the framework should fire events on pivot table changes.

I think the limitation is that BelongsToMany doesn't use the Pivot model to interface with the DB but uses Query Builder directly; so doesn't benefit from the events Model provides.

Should BelongsToMany be updated to use the Pivot model - or - should BelongsToMany just fire it's own events? Or some other mechanism? It'd be good to get some feedback as to what a good approach here would be before coming up with a PR.

I just want to point out that without Pivot model events or a way to override BelongsToMany::attach(), it forces us to break the don't repeat yourself (DRY) principle throughout our apps. Currently there is no solution to this issue:

http://stackoverflow.com/questions/37569142/overriding-attach-method-from-belongstomany-relationship

This is a real problem when trying to implement concepts from graph databases, since instead of being able to say $node->nodesWithSomeEdgeType('my_edge_type')->save($myNode), we have to create various convenience functions like $node->saveNodeWithSomeEdgeType('my_edge_type', $myNode).

I am considering extending the BelongsToMany class and overriding attach() or createAttachRecords() so that I can set additional columns on the pivot table row. Unfortunately, Model::belongsToMany() is too complex of a function to override in my child model class in order to return an ExtendedBelongsToMany, and there is no clear clone() or copy constructor for BelongsToMany that the extended class could use.

I feel this level of obfuscation is hurting developer productivity for more advanced use cases. We already have BelongsToMany::withPivot() and BelongsToMany::withTimestamps(), I don't see any reason not to have:

BelongsToMany::withPivot(['column' => function () {
    // return column value to be inserted into attach($model, $attributes) array
}])

or something to that effect for setting these columns upon attach(). I also don't see any reason not to provide a setPivotAttributes() accessor function of some kind on BelongsToMany that could be used within a model relationship method to prevent breaking the DRY principal. I hope these ideas are considered for the next version of Laravel. Thanks for all your hard work until then!

take a look at my package : https://github.com/fico7489/LaravelPivot

This should be fired by framework itself.
@GrahamCampbell can we open this issue?

Why can't you use Model observers to handle real model events, also including relationship events in your flow as well?

Was this page helpful?
0 / 5 - 0 ratings

Related issues

felixsanz picture felixsanz  路  3Comments

CupOfTea696 picture CupOfTea696  路  3Comments

gabriellimo picture gabriellimo  路  3Comments

SachinAgarwal1337 picture SachinAgarwal1337  路  3Comments

klimentLambevski picture klimentLambevski  路  3Comments