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

[5.8] Fix BelongsToMany read wrong parent key. #28317

Merged
merged 1 commit into from
Apr 25, 2019
Merged

Conversation

tinpont
Copy link
Contributor

@tinpont tinpont commented Apr 24, 2019

Sometimes we don't have primary id in pivot table. For example:
I have three tables.

// users
[{
    id: 1,
    uuid: "c3b5c66a-aebe-4c76-9a51-9c305ff6dad1",
    name: "John"
}]

// shops
[{
    id: 1,
    uuid: "25adf508-8dff-4a7f-ad26-3320f4d23f15",
    name: "Nice Pizza"
}, {
    id: 1,
    uuid: "84a11000-0a09-4054-bc9c-c4e49e2f48c5",
    name: "Nice Flower"
}]

// user_shop(pivot table)
[{
    user_uuid: "c3b5c66a-aebe-4c76-9a51-9c305ff6dad1",
    shop_uuid: "25adf508-8dff-4a7f-ad26-3320f4d23f15"
}]

User extends Eloquent\Model

    /**
     * Has many shops
     *
     * @return \Illuminate\Database\Eloquent\Relations\BelongsToMany
     */
    public function shops()
    {
        return $this->belongsToMany(Shop::class, 'user_shop', 'user_uuid',
            'shop_uuid', 'uuid', 'uuid')
            ->using(UserShop::class);
    }

UserShop extends Eloquent\Relations\Pivot

// Empty
<?php
    $user = \App\Models\User::first();
    $user->shops()->sync(['84a11000-0a09-4054-bc9c-c4e49e2f48c5']);
    dd($user->shops->toArray());

expect

delete from `user_shop` where (`user_uuid` = 'c3b5c66a-aebe-4c76-9a51-9c305ff6dad1' and `shop_uuid` = '25adf508-8dff-4a7f-ad26-3320f4d23f15')

actual

delete from `user_shop` where (`user_uuid` = 1 and `shop_uuid` = '25adf508-8dff-4a7f-ad26-3320f4d23f15')

Because user_uuid read $this->parent->getKey().

@tinpont tinpont changed the title [5.8]Fix BelongsToMany read wrong parent key. [5.8] Fix BelongsToMany read wrong parent key. Apr 24, 2019
@driesvints
Copy link
Member

Shouldn't it be the following instead? (actual shop uuid)

delete from `user_shop` where (`user_uuid` = 'c3b5c66a-aebe-4c76-9a51-9c305ff6dad1' and `shop_uuid` = '25adf508-8dff-4a7f-ad26-3320f4d23f15')

Your examples are a little confusing since you use twice the same primary key and uuid for your shop records.

Can you paste your full user and shop model?

@driesvints
Copy link
Member

@staudenmeir can you maybe confirm this pr?

@tinpont
Copy link
Contributor Author

tinpont commented Apr 24, 2019

@driesvints Sorry for my typo. Actually the first shop uuid is '25adf508-8dff-4a7f-ad26-3320f4d23f15';

Here is my full Models.

User

<?php

namespace App\Models;

use Illuminate\Database\Eloquent\Model;

class User extends Model
{
    public $timestamps = false;
    
    public function shops()
    {
        return $this->belongsToMany(Shop::class, 'user_shop', 'user_uuid',
            'shop_uuid', 'uuid', 'uuid')
            ->using(UserShop::class);
    }
}

Shop

<?php

namespace App\Models;

use Illuminate\Database\Eloquent\Model;

class Shop extends Model
{
    public $timestamps = false;
}

UserShop

<?php

namespace App\Models;

use Illuminate\Database\Eloquent\Relations\Pivot;

class UserShop extends Pivot
{
}

You can also run the test case to reproduce this bug.

@themsaid themsaid self-requested a review April 25, 2019 11:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants