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.

Sign up for GitHub

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

Jump to bottom

Adding pivot values causes the pivot model observer not work properly #55026

Open
amir9480 opened this issue Mar 14, 2025 · 4 comments · May be fixed by #55280
Open

Adding pivot values causes the pivot model observer not work properly #55026

amir9480 opened this issue Mar 14, 2025 · 4 comments · May be fixed by #55280
Labels
help wanted

Comments

@amir9480
Copy link
Contributor

Laravel Version

12.2.0

PHP Version

8.3.6

Database Driver & Version

No response

Description

Suppose we have a many-to-many relationship with a custom pivot model class. In that case, we can use an observer to handle events for the pivot model (and the relationship).

But here is the issue. If I add a withPivotValue to my relationship, the update and delete events will not work anymore.

Steps To Reproduce

In my project, I have a many to many relation like this:
app/Models/Project.php

class Project // ...
{
    public function allMembers(): BelongsToMany
    {
        return $this->belongsToMany(User::class, 'project_members')
            ->withPivot(...)
            ->using(ProjectMember::class);
    }

    public function members(): BelongsToMany
    {
        return $this->allMembers()->withPivotValue('role', 'member');
    }

    public function managers(): BelongsToMany
    {
        return $this->allMembers()->withPivotValue('role', 'manager');
    }
}

I have an observer for the project member model like this:

class ProjectMemberObserver
{
    public function created(ProjectMember $projectMember): void
    {
        dd('created called');
    }

    public function updated(ProjectMember $projectMember): void
    {
        dd('updated called');
    }

    public function deleted(ProjectMember $projectMember): void
    {
        dd('deletedcalled');
    }
}

Now, if I use the relation without the pivot value, my observer works as expected:

$project = Project::find(...);

$project->allMembers()->attach(...); // created called
$project->allMembers()->updateExistingPivot(..., [...]); // updated called
$project->allMembers()->detach(...); // deleted called

But if I use the relation with pivot value, only the create handler will work, and the updated or deleted will not work as expected.

$project = Project::find(...);

$project->managers()->attach(...); // created called
$project->managers()->updateExistingPivot(..., [...]); // Observer not working
$project->managers()->detach(...); // Observer not working
@github-actions GitHub Actions
Copy link

Thank you for reporting this issue!

As Laravel is an open source project, we rely on the community to help us diagnose and fix issues as it is not possible to research and fix every issue reported to us via GitHub.

If possible, please make a pull request fixing the issue you have described, along with corresponding tests. All pull requests are promptly reviewed by the Laravel team.

Thank you!

@macropay-solutions
Copy link

macropay-solutions commented Mar 14, 2025 •
edited
Loading

The delete is a mass delete that does not trigger events.

            // Once we have all of the conditions set on the statement, we are ready
            // to run the delete on the pivot table. Then, if the touch parameter
            // is true, we will go ahead and touch all related models to sync.
            $results = $query->delete();

or delete on model:

        if ($this->using &&
            ! empty($ids) &&
            empty($this->pivotWheres) &&
            empty($this->pivotWhereIns) &&
            empty($this->pivotWhereNulls)) {
            $results = $this->detachUsingCustomClass($ids); // here
        } else {
        foreach ($this->parseIds($ids) as $id) {
            $results += $this->newPivot([
                $this->foreignPivotKey => $this->parent->{$this->parentKey},
                $this->relatedPivotKey => $id,
            ], true)->delete();
        }

But

    public function wherePivot($column, $operator = null, $value = null, $boolean = 'and')
    {
        $this->pivotWheres[] = func_get_args(); // adds elements to pivotWheres

The update works in 2 ways also:

Mass update

        $updated = $this->newPivotStatementForId($this->parseId($id))->update(
            $this->castAttributes($attributes)
        );

Or with save() that will trigger events when using custom class

    public function updateExistingPivot($id, array $attributes, $touch = true)
    {
        if ($this->using &&
            empty($this->pivotWheres) &&
            empty($this->pivotWhereIns) &&
            empty($this->pivotWhereNulls)) {
            return $this->updateExistingPivotUsingCustomClass($id, $attributes, $touch); // here
        }
    protected function updateExistingPivotUsingCustomClass($id, array $attributes, $touch)
    {
        $pivot = $this->getCurrentlyAttachedPivots()
                    ->where($this->foreignPivotKey, $this->parent->{$this->parentKey})
                    ->where($this->relatedPivotKey, $this->parseId($id))
                    ->first();

        $updated = $pivot ? $pivot->fill($attributes)->isDirty() : false;

        if ($updated) {
            $pivot->save();
        }

But

    public function wherePivot($column, $operator = null, $value = null, $boolean = 'and')
    {
        $this->pivotWheres[] = func_get_args(); // adds elements to pivotWheres

@macropay-solutions
Copy link

macropay-solutions commented Mar 14, 2025 •
edited
Loading

We see it as expected behavior because when you put extra conditions on the pivot like role must be member or manager, the delete or update statement will contain 3 conditions that might lead to model not being found.

updateExistingPivot($id, // will produce where role='manager' and foreign_id = {$id} and foreign_id2 = {}

Analog for detach if $ids is not empty.

So to fix this, first you have to find a logic way of handling not found rows from the pivot.

@Amirhf1
Copy link
Contributor

Amirhf1 commented Apr 1, 2025

I want to work on this issue. I'll start by reproducing the problem and investigating the underlying cause in the Laravel codebase.

@Amirhf1 Amirhf1 mentioned this issue Apr 1, 2025
Open
This was referenced Apr 5, 2025
Closed
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
help wanted
Projects
None yet
Milestone
No milestone
4 participants
@crynobone @amir9480 @Amirhf1 @macropay-solutions