HasMany relationship does not work with custom primary key

[iwasherefirst2]

Version information

• Laravel: v8.40.0
• Voyager: 1.x-dev
• PHP: 8.0.5
• Database: MySQL 8.0

Description

Using hasMany relationship in bread does not show relations, if relation has a custom primary key.

Steps To Reproduce

I have a table items (id, name, type, created_at, updated_at) which has many item_skus (sku, item_id, created_at, updated_at)

I have set the relationship in the BREAD builder:

Although each item has many skus, no results are shown:

Expected behavior

There should be a multiselect on the bread page.

Additional context

Only special thing here may be that there is no primary AI in the ItemSku model. Its primary key is given by the sku field.

<?php

namespace App\Models\Products;

use Illuminate\Database\Eloquent\Factories\HasFactory;
use Illuminate\Database\Eloquent\Model;

class ItemSku extends Model
{
    use HasFactory;

    public $incrementing = false;

    protected $guarded = [];

    protected $primaryKey = 'sku';
}

[emptynick]

I don't think the primary-key is the problem. HasMany relationships are not "editable" in Voyager. While I agree that this is counter-intuitive, that's how Laravel works (Or worked). Before, there was no way to attach a model to a parent-model in HasOneOrMany (nowadays there is save)

[iwasherefirst2]

@emptynick thanks for the quick reply.

Okay, does it mean there is no multi-select for relations? Because its stated in the docs:

You can also specify which columns you would like to see in the dropdown or the multi-select.

Also, even if HasMany is not yet editable, it should be at least displayed? Or are HasMany relationships currently not supported at all (neither shown, not editable)?

If thats the case, would you accept a pull-request to fix this, or do you not want to support this?

[emptynick]

They should be displayed - but not selectable. That's only the case for HasOne and HasMany. Of course we are open for pull requests.

[iwasherefirst2]

The issue is caused in the relationship.blade.php for has ManyRelations (see row 73-75)

 $selected_values = $model::where($options->column, '=', $relationshipData->{$options->key})->get()->map(function ($item, $key) use ($options) {
                        return $item->{$options->label};
 })->all();

$relationshipData is the Item model and the id column that is referenced by the relationsship is needed. $options->key is supposed to be that value, however, $options->key is the value that one selects in the GUI

It should be id actually, However, I can't select the field id, because its not in the select box (the box only contains columns of the item_skus table). If I change in table data_rows in column details inside the json object the key to id, then everything works.

One simple solution would be, to just always use the primary key of the $relationshipData model for a hasMany relation.

Another option I could imagine, is that one allows to change the key in the "Relationship Details".

{
   "key": "id"
}

However, currently default fields cannot be overwritten: https://github.com/the-control-group/voyager/blob/f24e18b335629f28350457a8c8e0583ea26ea550/src/Models/DataType.php#L202

From array_merge documentation:

If the input arrays have the same string keys, then the later value for that key will overwrite the previous one.

So if we switch the order, we could overwrite default fields:

  $merge = array_merge($relationshipDetails, $details);

@emptynick what do you think about both alternatives? The current solution that the parent needs to have an id that one can choose from the child does not look correct to me.

[emptynick]

The relationship builder should actually show Store the items...id (and display all fields from Items) right?

Looks like this is a "simple" display bug which should be easy to fix.

Edit: I think something sneaked in here: https://github.com/the-control-group/voyager/pull/5194

[iwasherefirst2]

If its a simple display fix, even better!

Yes it should display all fields from items in case of a hasMany relation. For the other Eloquent relations I am not entirely sure.

Maybe it makes sense to improve the wording as well. Instead of "Store the item_skus" one could say "Which column of items is referenced by the item_skus table?

[MrCrayon]

5194 should have fixed this problem.

I see this: @iwasherefirst2 exactly which version do you have installed with commit? you can do something like

composer show | grep "tcg/voyager"

I think you either need to update or clear cache.

[emptynick]

Reading through your PR again, it says it should display the same table - while it should display the foreign table. Am I wrong?

[MrCrayon]

If you are setting a HasMany relationship on items BREAD what needs to be stored on item_skus is a value of items like items.id, so it needs to be displayed the same table as the BREAD you are in.

A bit convoluted to explain but what is shown in my screenshot is the right behaviour.

[emptynick]

Yeah, confusing but you are right

[iwasherefirst2]

@MrCrayon Sorry for the late reply, forgot that I left this question open

@iwasherefirst2 exactly which version do you have installed with commit? you can do something like

composer show | grep "tcg/voyager"

This is the output:

tcg/voyager 1.x-dev e9160b7 A Laravel Admin Package ...

[MrCrayon]

@iwasherefirst2 so it looks like you need to update. If you don't want to update all and only want that fixed you can create a fork and only apply this https://github.com/the-control-group/voyager/pull/5194