Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fire beforeDelete on pivot models #3620

Closed
wants to merge 1 commit into from

Conversation

meysammahfouzi
Copy link
Contributor

This commit tries to fix #2747

I am not sure though if this is the best possible solution!

@LukeTowers
Copy link
Contributor

This pull request has been made to the wrong branch. Please review the contributing guidelines as all PRs need to be made to the develop branch.

I'll fix it for you this time, but please ensure you make any future PRs to the develop branch, not the master branch.

@LukeTowers LukeTowers changed the base branch from master to develop June 29, 2018 16:55
@meysammahfouzi
Copy link
Contributor Author

@LukeTowers Thanks

@LukeTowers
Copy link
Contributor

@meysammahfouzi I've sent this to @daftspunk for review as he's more familiar with this piece than I am.

@LukeTowers
Copy link
Contributor

@meysammahfouzi is there a way that we can fire the deleting model event on the pivot model instead as that would then allow the beforeDelete method, the deleting laravel model event and the model.beforeDelete event to be fired?

@meysammahfouzi
Copy link
Contributor Author

@LukeTowers Well, I tried to find a way to do that, but unfortunately could not. So just tried to simulate it. Could @daftspunk take a look at it?

@LukeTowers
Copy link
Contributor

I can assign it to his project but whether or not he's actually going to look at it is a completely different issue.

@daftspunk
Copy link
Member

This is one of those cases where it would be too magical to include in the core, with too many different possibilities. So instead you may want to try hooking up the event yourself. Here is an example:

    public $belongsToMany = [
        'roles_pivot_model' => [
            'October\Test\Models\Role',
            'table' => 'october_test_users_roles',
            'pivot' => ['clearance_level', 'is_executive'],
            'timestamps' => true,
            'pivotModel' => 'October\Test\Models\UserRolePivot',
        ],
    ];

    public function afterFetch()
    {
        $this->bindEvent('model.relation.beforeDetach', function($relation, $ids) {
            if ($relation == 'roles_pivot_model') {
                $this->roles_pivot_model()->whereIn('id', $ids)->get()->each(function($model) {
                    $model->pivot->beforeDelete();
                });
            }
        });
    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants