Too many queries n-n relation

[imasubmarine]

Hello there,

I'm fairly new to Backpack so please bear with me.
I have 3 tables products, pictos and picto_product.
The pivot have an extra field called dimension.

I have setup everything like stated in the doc :
On the Product model

public function pictos()
    {
        return $this->belongsToMany(Picto::class)->withPivot(['dimension']);
    }

On the ProductCrudController setupCreateOperation

CRUD::addField([
            'name' => 'pictos',
            'label' => 'Déclinaisons',
            'type'      => 'relationship',
            'tab' => 'Déclinaisons',
            'pivot' => true,
            'wrapper' => [
                        'class' => 'col-md-6',
                    ],
            'subfields' => [
                [
                    'name' => 'dimension',
                    'label' => 'Dimensions',
                    'type' => 'text',
                    'wrapper' => [
                        'class' => 'form-group col-md-6',
                    ]
                ],
            ],
            'pivotSelect'=> [
                'placeholder' => 'Choisissez une déclinaison',
                'wrapper' => [
                    'class' => 'form-group col-md-6',
                ],
            ],
        ]);

And it works fine but :
1°) When i edit a product, for each entry in the pivot table, it generates 2 additional queries
ex: for 2 entries

SELECT * FROM pictos
SELECT * FROM pictos WHERE id=15
SELECT * FROM pictos
SELECT * FROM pictos WHERE id=23

I tried eager loading in different places

protected function setupUpdateOperation()
    {
        $this->crud->with(['pictos']);
        $this->setupCreateOperation();
    }

But it doesn't seems to make a difference...

I'm probably missing something. Any ideas on how to optimize this ?

2°) Additionally how do you validate the extra pivot field with Backpack ?

[pxpm]

Hello @imasubmarine

Thanks for the suggestion about the queries. Actually I started working on a branch to solve that issue.

I haven't created the PR yet, but I hope to be able to create it by the end of this week, that will drastically reduce the query number on relations and more.

About the validation, you can validate in the fields by defining 'validationRules' => 'required|smt_else', or in the Request if you are already using a Request class in that form with: 'pictos.*.name_of_the_field' => rules

Cheers

[imasubmarine]

Hello @pxpm

I have another question regarding the same context. (Although it would be probably better to open a new thread)

When i add a new item with the Field relationship (n-n) :
I might need to add multiple times the same item(picto) but with different dimensions (extra column in pivot table).
Then the last item override the previous item when I hit save.
(even though there is no composite key in the pivot table)

Schema::create('picto_product', function (Blueprint $table) {
            $table->increments('id');
            $table->foreignId('picto_id')->constrained()->cascadeOnDelete();
            $table->foreignId('product_id')->constrained()->cascadeOnDelete();
            $table->string('dimension', 45);
        });

So either it is not permitted by your component and Backpack should raise an error.
Or it is and it should not override the previous row for the same item in the pivot table and silently discard data.

Any ideas on how i can solve this ?
Or is there another way to do that ?

Thanks

[pxpm]

Hello @imasubmarine I don't think you will be able to achieve that with that table structure.

Users and roles are a similar relation, why would you store multiple times that user1_id has admin_role_id multiple times ? Like:

user_id	role_id
1	1
1	1
1	1
1	1
1	2
1	2

I think you need to rethink your relations, and/or your implementation.

Laravel provides a pivot model, similar to a model, but represent a pivot table (your picto_product table) https://laravel.com/api/9.x/Illuminate/Database/Eloquent/Relations/Pivot.html

Technically what you are trying to achieve is that your picto_product => HasMany => dimensions

You can create that PictoProductPivotModel, and an additional table like picto_product_dimensions

Alternatively you can use a table field https://backpackforlaravel.com/docs/5.x/crud-fields#table-pro and save the dimensions as json in a text column in your pivot table.

Hope it helps,
Cheers

[imasubmarine]

Thanks for answer @pxpm !

I totally agree with your User, Role example. But it doesn't match my case.

My primary goal was to use the right field in the Product CRUD Controller.
The Relationship PRO seemed a good candidate.

I agree i could rethink my relations in a more semantically meaningfull way like so:
Product (hasMany) --> <-- (belongsTo) former pivot model with extra dimension (belongsTo) --> <-- (hasMany) Picto
But then which Backpack field should i use on the Product CRUD?

Why would i want duplicates in the pivot table ?

1°) a Product is a type of Sofa Design

• It can exist in many forms represented by an icon and a name (Picto) : 3seats, 2seats, 21/2seats, armchair, long armchair... (there are lots)
• Each form (Picto) for a given product have different dimensions (Extra pivot column)
• Each Product can have the same form (picto) more than once with different dimensions
  example: 3seats( width: 200), 3seat(width: 240)
• i might add a price later based on (product, picto, dimension) combination.

2°) I didn't read anything that says you cannot have duplicate in a pivot table
Actually a lot of people complain that they have unwanted duplicates because you can do:

$product->pictos()->attach(23 => ['dimension' => '120 x 120 x 80'])
$product->pictos()->attach(23 => ['dimension' => '200 x 210'])

And it would work
But you cannot sync all at once because keys would be merged.
(although you could sync([]) then foreach attach which would be inefficent.

Actually Nova addresses that:

By default, Laravel Nova ensures that "belongs to many" relationships are unique. However, if necessary, you may instruct Nova to allow duplicate relationship entries.
BelongsToMany::allowDuplicateRelations()

(but i'll stick to Backpack ;-))

So i'm confused and i agree it might be a Laravel ambiguity.
So if your goal with Relationship PRO is to not allow duplicates maybe you should enforce it.
But if (Feature request) you could add a parameter that explictly states if you allow/disallow duplicates and sync accordingly, that could remove the amiguity.

With all those new elements what would be your advice to solve my case with current Backpack version ?

Thanks

[pxpm]

Thanks for the reply @imasubmarine

I agree with you, we are developers and eventually we will make it work anyway we want if possible.
At the present moment you can do it by overriding the store method, remove that relation from the request and let Backpack save the rest of the inputs, then manually sync/attach/manage that relation.
https://backpackforlaravel.com/docs/5.x/crud-operation-create#override-the-store-method

I recall that I worked on something related to don't allow the selection of the same pivot multiple times, I need to see if I messed up something and didn't merge that, I will have a look at that issue later today.

Thanks for the suggestion, I think there is something more that we Backpack can do here, I will have a better look at this issue in the next weeks when I have free time from developing v6 planned features, and hopefully we can get into v6 with some improvements in this regard.

[imasubmarine]

Thanks @pxpm !

Following your advice here is what i came up with for the ProductCrudController:

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;
    }

While this seems to work fine for me with the Relationship PRO field, there might be loopholes.
Plus i don't think it is really efficient.
If anyone has a better idea or any improvement ideas i'll take them :-)

Cheers