[5.8] Sync method return wrong updated record and event with Pivot class

[link08]

• Laravel Version: 5.8.10
• PHP Version: 7.2.16
• Database Driver & Version: mysql 5.7

Description:

After the update from Laravel 5.7 to Laravel 5.8 the sync() method with pivot class doesn't return the correct values.

Problem 1

If I try to read the response from the ->sync() method, inside the 'updated' value there are all the values, even if there wasn't any change.

Problem 2

I updated to Laravel 5.8 to use the Observer with the pivot class but the updated event is triggered every time and I can't get the original value of a pivot attribute with the method ->getOriginal()

Steps To Reproduce:

Models:

class Person extends Model
{
    //
}

class Business extends Model
{
    public function people()
    {
        return $this->belongsToMany(Person::class, 'business_person_role')
            ->using(BusinessPersonRole::class)
            ->withPivot([
                'role_id',
            ])
            ->withTimestamps();
    }
}

class Role extends Model
{
    //
}

class BusinessPersonRole extends Pivot
{
    protected $table = 'business_person_role';
}

Sample code:

    $person1 = factory(Person::class)->create(); // Id: 1
    $person2 = factory(Person::class)->create(); // Id: 2
    $person3 = factory(Person::class)->create(); // Id: 3
    $person4 = factory(Person::class)->create(); // Id: 4

    $business1 = factory(Business::class)->create(); // Id:1

    $role1 = factory(Role::class)->create(); // Id: 1
    $role2 = factory(Role::class)->create(); // Id: 2

    $business1->people()->attach([
        $person1->id => ['role_id' => $role1->id],
        $person2->id => ['role_id' => $role1->id],
        $person3->id => ['role_id' => $role1->id],
    ]);

    var_dump($business1->people()->get()->pluck('pivot.role_id', 'id')->toArray());
    /*
     * array (size=3)
     * 1 => int 1
     * 2 => int 1
     * 3 => int 1
     */

    $changes = $business1->people()->sync([
        $person1->id => ['role_id' => $role2->id],
        $person3->id => ['role_id' => $role1->id],
        $person4->id => ['role_id' => $role1->id],
    ]);

    var_dump($business1->people()->get()->pluck('pivot.role_id', 'id')->toArray());
    /*
     * array (size=3)
     * 1 => int 2
     * 3 => int 1
     * 4 => int 1
     */

    var_dump($changes);
    /*
     * array (size=3)
     *   'attached' => 
     *     array (size=1)
     *       0 => int 4
     *   'detached' => 
     *     array (size=1)
     *       1 => int 2
     *   'updated' => 
     *     array (size=2)
     *       0 => int 1
     *       1 => int 3    <======= This value is wrong
     */

with Laravel 5.7 the last var_dump return:

    var_dump($changes);
    /*
     * array (size=3)
     *   'attached' => 
     *     array (size=1)
     *       0 => int 4
     *   'detached' => 
     *     array (size=1)
     *       1 => int 2
     *   'updated' => 
     *     array (size=1)
     *       0 => int 1
     */

Observer:

// Observer registered inside the Service Provider
BusinessPersonRole::observe(BusinessPersonRoleObserver::class);

// BusinessPersonRoleObserver.php
class BusinessPersonRoleObserver
{
    public function updated(BusinessPersonRole $businessPersonRole)
    {
        var_dump('------');
        var_dump($businessPersonRole->person_id);
        var_dump($businessPersonRole->business_id);
        var_dump($businessPersonRole->role_id);
        var_dump($businessPersonRole->getOriginal('role_id'));

        /*
         *  === > Correct record <===
         * string '------' (length=6)
         * int 1
         * int 1
         * int 2
         * null
         *
         * === > Wrong record <===
         * string '------' (length=6)
         * int 3
         * int 1
         * int 1
         * null
         */
    }
}

[staudenmeir]

Probably caused by #27571. /cc @ralphschindler

[ralphschindler]

@link08 regarding "Problem 1", when you run sync() with a Pivot class that has timestamps, aren't you at least updating the timestamps? Hence it listed in the update?

Regarding "problem 2", what if you hooked into the updating() event and look for the attribute there? I will dig more soon.

[link08]

@link08 regarding "Problem 1", when you run sync() with a Pivot class that has timestamps, aren't you at least updating the timestamps? Hence it listed in the update?

Regarding "problem 2", what if you hooked into the updating() event and look for the attribute there? I will dig more soon.

Question 1

No, the updated_at column is not updated.

Checking if in Laravel 5.7 it works correctly I found another strange thing. Adding sleep(3) between the attach() and the sync() methods, Laravel 5.7 return the same error found in Laravel 5.8 adding the wrong value inside updated array. If it can comfort us the updated_at is updating correctly in Laravel 5.7.

    $business1->people()->attach([
        $person1->id => ['role_id' => $role1->id],
        $person2->id => ['role_id' => $role1->id],
        $person3->id => ['role_id' => $role1->id],
    ]);

    dump($business1->people()->get()->pluck('pivot.role_id', 'id')->toArray());

    sleep(3);

    $changes = $business1->people()->sync([
        $person1->id => ['role_id' => $role2->id],
        $person3->id => ['role_id' => $role1->id],
        $person4->id => ['role_id' => $role1->id],
    ]);

Question 2

Yes, the method updating return the same results.

    public function updating(BusinessPersonRole $businessPersonRole)
    {
        dump('-------');
        dump($businessPersonRole->person_id);
        dump($businessPersonRole->business_id);
        dump($businessPersonRole->role_id);
        dump($businessPersonRole->getOriginal('role_id'));
    }

Test code

I've created a public repository with my test code:
https://github.com/link08/laravel-sync-issue-28150

The sample code is inside: routes/web.php

[hulkur]

Problem appears to be in that sync does not check if the update is needed. No check on additional fields, only relational fields.

It will check what needs to be deleted and added. Everything else is updated.

delete from `business_person_role` where (`business_id` = 4 and `person_id` = 14)
2.16ms
/routes/web.php:43
update `business_person_role` set `role_id` = 8 where `business_id` = 4 and `person_id` = 13
590μs
/routes/web.php:43
update `business_person_role` set `role_id` = 7 where `business_id` = 4 and `person_id` = 15
360μs
/routes/web.php:43
insert into `business_person_role` (`person_id`, `business_id`, `created_at`, `updated_at`, `role_id`) values (16, 4, '2019-04-10 13:51:47', '2019-04-10 13:51:47', 7)
480μs
/routes/web.php:43

[hulkur]

InteractsWithPivotTable::sync():91

        $current = $this->newPivotQuery()->pluck(
            $this->relatedPivotKey
        )->all();

Takes only related id-s, not full records.

[driesvints]

Prs for this were merged.