[Feature/Inconsistency fix] Allow N-N relationships to save the same pivot more than once.

[michprev]

Bug report

Hello, I have a problem using the relationship field with custom id primary key in the pivot table.

What I did

I have two models, Order and CartItem, with belongsToMany/belongsToMany (N-N) relationship. The pivot table looks as follows:

OrderCartItem
------------------
id [pk]
order_id [fk]
cart_item_id [fk]
quantity
discount_percent

This is the definition of the relationship field in OrderCrudController:

CRUD::addField([
    'name' => 'cart_items',
    'label' => 'Cart items',
    'type' => 'relationship',
    'subfields' => [
        [
            'name' => 'quantity',
            'type' => 'number',
            'label' => 'Quantity',
            'default' => 1,
            'attributes' => ['min' => 1],
            'wrapper' => ['class' => 'form-group col-md-4'],
        ],
        [
            'name' => 'discount_percent',
            'type' => 'number',
            'label' => 'Discount',
            'default' => 0,
            'suffix' => '%',
            'attributes' => ['min' => 0, 'max' => 100],
            'wrapper' => ['class' => 'form-group col-md-4'],
        ]
    ],
    'pivotSelect' => [
        'attribute' => 'label',
        'label' => 'Cart item',
    ],
    'new_item_label' => 'Add cart item',
    'reorder' => false,
]);

The problem is that I cannot insert multiple pivot table entries with the same order_id and cart_item_id using this field.

What I expected to happen

When adding multiple cart items with the same id using the relationship field, I expect all of them stored in the pivot table.

What happened

Only the last cart item entry with the given id is stored in the pivot table.

What I've already tried to fix it

I have tried different configurations of the relationship field, have not found any working configuration. Also tried manually attaching Laravel models - this works as expected:

$o = Order::first();
$i = CartItem::first();
$o->cart_items()->attach($i, ['quantity' => 1, 'discount_percent' => 0]);
$o->cart_items()->attach($i, ['quantity' => 1, 'discount_percent' => 0]);
$o->refresh();

Two cart item entries (with the same CartItem.id) are attached to the order.

Is it a bug in the latest version of Backpack?

Yes

Backpack, Laravel, PHP, DB version

When I run php artisan backpack:version the output is:

### PHP VERSION:
PHP 8.1.13 (cli) (built: Dec 12 2022 23:45:33) (NTS)
Copyright (c) The PHP Group
Zend Engine v4.1.13, Copyright (c) Zend Technologies

### LARAVEL VERSION:
v9.45.1@faeb20d3fc61b69790068161ab42bcf2d5faccbc

### BACKPACK VERSION:
5.4.11@ffbadcdb6478822646600b2cd763490caa927155

Backpack PRO version: 1.1.1

[welcome[bot]]

Hello there! Thanks for opening your first issue on this repo!

Just a heads-up: Here at Backpack we use Github Issues only for tracking bugs. Talk about new features is also acceptable. This helps a lot in keeping our focus on improving Backpack. If you issue is not a bug/feature, please help us out by closing the issue yourself and posting in the appropriate medium (see below). If you're not sure where it fits, it's ok, a community member will probably reply to help you with that.

Backpack communication channels:

• Bug Reports, Feature Requests - Github Issues (here);
• Quick help (How do I do X) - Gitter Chatroom;
• Long questions (I have done X and Y and it won't do Z wtf) - Stackoverflow, using the backpack-for-laravel tag;
• Showing off something you've made, asking for opinion on Backpack/Laravel matters - Reddit;

Please keep in mind Backpack offers no official / paid support. Whatever help you receive here, on Gitter, Slack or Stackoverflow is thanks to our awesome awesome community members, who give up some of their time to help their peers. If you want to join our community, just start pitching in. We take pride in being a welcoming bunch.

Thank you!

-- Justin Case The Backpack Robot

[pxpm]

Hello @michprev thanks for the report.

Can you share your relationship definition ?

Thanks

[michprev]

Hey @pxpm,

class Order extends Model
{
    use CrudTrait;

    protected $guarded = ['id'];

    public function cart_items() {
        return $this->belongsToMany(CartItem::class, 'vandrawee_work.cart_items_orders', 'order_id', 'cart_item_id')->withPivot(['id', 'quantity', 'discount_percent']);
    }

class CartItem extends Model
{
    use CrudTrait;

    protected $connection = 'vandrawee_work';

    public function orders() {
        return $this->belongsToMany(\App\Models\Order::class, 'vandrawee_work.cart_items_orders', 'cart_item_id', 'order_id')->withPivot(['id', 'quantity', 'discount_percent']);
    }
}

Also, I captured the request data in update function of the OrderCrudController controller:

  '_method' => 'PUT',
  'id' => '2',

  'cart_items' => 
  array (
    0 => 
    array (
      'cart_items' => '1',
      'quantity' => '1',
      'discount_percent' => '0.00',
    ),
    1 => 
    array (
      'cart_items' => '1',
      'quantity' => '1',
      'discount_percent' => '0',
    ),
  ),
'_save_action' => 'save_and_back',

It shows that the data are correctly sent from the relationship field (the cart item entries are not deduplicated). That is why I am assuming the issue probably is in the implementation of the update function of UpdateOperation.

If I insert the entries manually like this:

MariaDB [vandrawee_work]> select * from cart_items_orders;
+----+--------------+----------+----------+------------------+
| id | cart_item_id | order_id | quantity | discount_percent |
+----+--------------+----------+----------+------------------+
|  6 |            1 |        2 |        1 |            10.00 |
|  7 |            1 |        2 |        1 |             0.00 |
|  8 |            1 |        2 |        1 |             0.00 |
+----+--------------+----------+----------+------------------+
3 rows in set (0.000 sec)

then all 3 entries are shown in the relationship field. If I delete 2 of these entries using the relationship field, all 3 entries are kept in the database.

[pxpm]

Thanks for getting back to me.

It could be an issue if using guarded instead of fillable.

Can you try using fillable instead of guarded in the model ?

Cheers

[michprev]

No, unfortunatelly using fillable instead of guarded does not change anything.

I have dug deeper and found that the pivot table entries are pruned in the following lines of code: https://github.com/Laravel-Backpack/CRUD/blob/23586dd93f128e2195cef5454f113c9430521ee6/src/app/Library/CrudPanel/Traits/Create.php#L122-L137

$values holds both/all entries, which is correct:

[{"cart_items":"1","quantity":"1","discount_percent":"0.00"},{"cart_items":"1","quantity":"1","discount_percent":"0"}] 

but after the foreach loop (lines 132-136), $relationValues holds only a single entry:

[{"quantity":"1","discount_percent":"0"}]

[pxpm]

Thank you @michprev I will investigate and get back to you shortly.

Cheers

[pxpm]

Hey @michprev I've been investigating your issue.

I think your error arises because you are attempting to use composite keys (multiple columns to identify one row), and Eloquent does not play nicely with them.

I've been trying to figure out if we could make something on Backpack side that would help this scenario, but it seems that is something very specific to each project and we can't really work that out on our side.

From my research you have two options:

1. move that composite key to a separate table, and in your pivot table use composition_table_id
2. try to use this package: https://github.com/topclaudy/compoships

Google laravel belongstomany composite keys should give you some direction on how to proceed.

Let me know how it goes.

Cheers

[michprev]

Hey @pxpm, thank you for taking a deeper look at this.

I am not sure this is the case you are describing. The primary key of the pivot table is just the id column.

I have found the following StackOverflow thread https://stackoverflow.com/questions/42091778/how-to-sync-multiple-values-of-the-same-attribute-in-laravel and the first two answers seem to be the solution.

With the following line of $values

[{"cart_items":"1","quantity":"1","discount_percent":"0.00"},{"cart_items":"1","quantity":"1","discount_percent":"0"}] 

this line https://github.com/Laravel-Backpack/CRUD/blob/f3fc812d57578d081c2b6eaa1db561168d975c19/src/app/Library/CrudPanel/Traits/Create.php#L145

basically does something like

$item->cart_items()->sync([1 => ['quantity' => '1', 'discount_percent' => '0.00']]);

where keys of the outer array actually represent the values of the cart_item_id column. Naturally, this syntax does not allow inserting multiple rows with the same cart_item_id (cannot have the same key in an array twice).

Based on the StackOverflow thread, there is another option. Instead of using array keys as values for the cart_item_id column, it should be possible to use the sync function without specific indexes and specify the cart_item_id directly.

This involves:

• getting the related foreign key name from the relationship

$relatedPivotKey = $item->{$relationMethod}()->getRelatedPivotKeyName();

• renaming the key cart_items (or $relationMethod generally) to cart_item_id (or $relatedPivotKey generally)

if (is_array($values) && is_multidimensional_array($values)) {
    foreach ($values as $value) {
        if (isset($value[$relationMethod])) {
            $value[$relatedPivotKey] = $value[$relationMethod];
            $relationValues[] = Arr::except($value, $relationMethod);
        }
    }
}

• clearing the pivot table and then calling the sync function with the modified argument

$item->{$relationMethod}()->sync([]);
$item->{$relationMethod}()->sync($relationValues);

Now, the sync function should be called somewhat like this

$item->cart_items()->sync([
    ['cart_item_id' => 1, 'quantity' => '1', 'discount_percent' => '0.00'],
    ['cart_item_id' => 1, 'quantity' => '1', 'discount_percent' => '0']
]);

which works fine for me.

However, this specific usage of the sync function does not seem to be documented by the Laravel's documentation - https://laravel.com/docs/9.x/eloquent-relationships#syncing-associations. Also, I cannot guarantee this solution will work correctly for all possible cases.

[pxpm]

Hello @michprev

thanks for getting back here. I get your issue now.

I have it in my todo list (allow belongsToMany with the same keys), but I am not sure if the best way would be to use sync like you described, or changing to ->attach() instead of ->sync().

I am now finish working in two other features and I plan to tackle this one as soon as I have time for it as I also think it's a must have feature in Backpack.

Thanks again, I will get back to you with news 👍

[imasubmarine]

Hello @pxpm @michprev

If i am not mistaken, this is exactly the same problem i was referring to in this post : https://github.com/Laravel-Backpack/community-forum/discussions/352#discussioncomment-4667700

Along with the temp solution i came up with to solve this. I'm glad to hear that this will be fixed.

[imasubmarine]

For some reason the link above doesn't work. So i'll post the temp solution i'm using to address this problem below (products, product_picto, pictos)

protected function syncAllowDuplicates(string $relation)
    {
        $callerName = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 2)[1]['function'];
        if (!in_array($callerName, ['store', 'update'])) {
            throw new \Exception('syncAllowDuplicates() can only be called from store() or update()');
        }
        //@TODO: check if relation exist on the current Model and is a many to many
        //@TODO: check if any of the foreign keys is a primary key, part of a unique index
        //or part of a composite unique index and throw an exception.

        $currentEntry = $this->crud->getCurrentEntry();
        if ($callerName === 'update') {
            //detach all the current entries
            // Not sure if it is more efficient to sync([]).
            $detachedIds = $currentEntry->{$relation}->pluck('id')->toArray();
            $currentEntry->{$relation}()->detach($detachedIds);
        }
        if (!empty(request()->input($relation))) {
            //attach the new entries
            // Not really efficent :-/
            array_map(function ($item) use ($currentEntry, $relation) {
                $pivotData = $item;
                unset($pivotData[$relation]);
                $currentEntry->{$relation}()->attach($item[$relation], $pivotData);
            }, request()->input($relation));
            $this->crud->removeField($relation);
        }
    }

    public function store()
    {
        //Need to validate the pictos array before the syncAllowDuplicates
        $this->crud->validateRequest();
        $this->syncAllowDuplicates('pictos');
        $response = $this->traitStore();
        // do something after save
        return $response;
    }

    public function update()
    {

        //Need to validate the pictos field before the syncAllowDuplicates
        $this->crud->validateRequest();
        $this->syncAllowDuplicates('pictos');
        $response = $this->traitUpdate();
        // do something after save
        return $response;
    }

If anyone knows better please let me know.

[Metalchocobo]

Exactly the same problem here. I'm going to try with the @imasubmarine fix. Thanks

[miquelangeld]

Same problem here. The workaround from @imasubmarine seems to works when update but not work with create operation.

In our case (pretty typical case) I'm in a project where we need to add more than once the same item in a n-n relation but with different pivot data.

@tabacitu @pxpm any plan/chance to solve this issue soon? Thanks!

[Kasparsu]

yeah ran into this issue recently. Was worried that it was my implementation of pro field relationships that was at fault until I realized when keys are different then it stores just fine. Laravel sync method is not very suitable in this case. Creating can be still done with sync if we provide array of fields with the related pivot key name. Updating is bit more tricky tho. If pivot table contains id field we could use that. if it doesn't then its going to be hard to do this. we could just query all relations of type and just rely on order to fill them out and just drop others or create new ones as need arises. Or simpler way would be to just delete all and recreate them at every update.

[miquelangeld]

Well, seems that @pxpm is close to solve our problems :) @Kasparsu

[Kasparsu]

This was merged https://github.com/Laravel-Backpack/CRUD/pull/5538 so can this be closed? @pxpm

[pxpm]

Right @Kasparsu thanks for the heads up 🙏 , I should have linked the PR to auto-close it.

Cheers and thanks everyone 🙏