filamentphp / filament

A collection of beautiful full-stack components for Laravel. The perfect starting point for your next app. Using Livewire, Alpine.js and Tailwind CSS.
https://filamentphp.com
MIT License
19.3k stars 2.96k forks source link

Repeater with `BelongsToMany` relationship doesn't work as expected #8616

Closed sprtk-ches closed 1 year ago

sprtk-ches commented 1 year ago

Package

filament/forms

Package Version

v3.0.53

Laravel Version

v10.24.0

Livewire Version

v3.0.5

PHP Version

8.2

Problem description

Following the documentation for the Repeater type doesn't work as expected.

  1. Not all related fields are shown. In the linked repo example, we have a user with 2 roles but only one is shown
  2. Any edits to the roles usually lead to some sort of error, depending on the action (eg trying to remove an existing role might give a different error than trying to add a new one)

I have tried changing the name of the repeater and other suggested solutions from here (discord link) but nothing seemed to work.

Expected behavior

Using the Repeater I should be able to manage BelongsToMany relationships

Steps to reproduce

Using the linked repo

  1. Build the app
  2. Run the migrations and serve
  3. Navigate to the homepage
  4. You should have at least one user with 2 roles but only one will appear
  5. Try editing the roles

Note that creating new relationships seems to work for the 1st time. After that, we get back to the original issue where not all records appear in the form.

Reproduction repository

https://github.com/sprtk-ches/laravel-filament

Relevant log output

No response

MarkKhromov commented 1 year ago

@sprtk-ches, hi, friend! I had the same problem. While researching the Filament code, I discovered that it uses a model key to display the elements in the repeater. Try adding something like this to your pivot model:

protected $primaryKey = 'custom_id';
public function getCustomIdAttribute(): string {
    return $this->model1_id. '_' . $this->model2_id;
}

This code will fix the problem with displaying one entry, but it will not be possible to change it. I'm dealing with the change right now.

MarkKhromov commented 1 year ago

@sprtk-ches after studying the Filament code in more detail, I came to the conclusion that the easiest way would be to add an identifier to your Pivot, for example, some kind of UUID. This will solve almost all problems with changing, adding and deleting records. I'll do some more digging to see if I can fix this on the Filament side.

sprtk-ches commented 1 year ago

@MarkKhromov Thanks for the workaround 👍 ! I'll leave the issue open and let the filament team decide how they want to handle this.

danharrin commented 1 year ago

Hi, this is explained in the docs. https://filamentphp.com/docs/3.x/forms/fields/repeater#integrating-with-a-belongstomany-eloquent-relationship

You must use the HasMany pivot model as the relationship for the repeater, as each item represents a pivot record and not a related record

MarkKhromov commented 1 year ago

Hi, this is explained in the docs. https://filamentphp.com/docs/3.x/forms/fields/repeater#integrating-with-a-belongstomany-eloquent-relationship

You must use the HasMany pivot model as the relationship for the repeater, as each item represents a pivot record and not a related record

@danharrin Yes, that's exactly what we do. But this doesn't work at all as expected. This will only work correctly with one entry. I won't be able to add a new entry if the identifier is not specified. Then it is necessary to describe this in the documentation. My code uses the Pivot table, as in the example. It has two columns work_id, employee_id and nothing else. And it doesn't work for me. I studied the repeater code and noticed that it simply cannot work correctly if there is no identifier for the record, no matter whether it is a pivot or a regular model. I suggest considering the possibility of working with a primary key formed from several columns. For example, if the model is set to $primaryKey = ['column1', 'column2']; Just if you provide for this in the repeater, then the scenario with pivots will work correctly, and not as it does now. How do you look at this? I can help, try to implement this mechanism, but I need your help.

danharrin commented 1 year ago

We need a primary key, otherwise we cant easily keep track of which pivot entries have been created or deleted.

MarkKhromov commented 1 year ago

We need a primary key, otherwise we cant easily keep track of which pivot entries have been created or deleted.

That's what I'm talking about too, but for a pivot table, the primary key can be made up of two columns.

danharrin commented 1 year ago

You can just use an easy auto increment, it doesnt need to be composite

MarkKhromov commented 1 year ago

You can just use an easy auto increment, it doesnt need to be composite

Yes, I understand you. In order not to use auto-increment, I simply made my identifier in the pivot as uuid and everything works fine for me. But still, it wasn't entirely obvious that it should be used that way. It seems to me that the documentation should be supplemented to indicate that the table must have an identifier with a primary key.

georgknabl commented 1 year ago

You can just use an easy auto increment, it doesnt need to be composite

@danharrin Another perspective from my side: I typically only create data structures that are absolutely necessary. Hence, most pivot tables do not have an "id" column as the primary key is combined of both referenced tables. This is fine in vanilla Laravel. However, when using Filament, this becomes an issue as belongsToMany is built using hasMany structures. Not providing a single primary key column in the pivot table causes strange bugs in Filament forms that, in my case, took significant time to debug and pinpoint the issue to the "missing" id column. If Filament requires such a column, it would be great if this was stated in the docs. Even better: throwing an error, if such an incompatible configuration was found.

danharrin commented 1 year ago

Great, I agree it should be in the docs. There is an "Edit" button at the bottom of each docs page when you are ready to make the contribution 😄

sathishreddynp commented 8 months ago

Please add this in docs

dharen008 commented 6 months ago

i just use formatStateUsing

TextEntry::make( 'members' ) ->formatStateUsing( function ( $state ) { return $state->name; } ) ->placeholder( 'No Members' ) ->badge(),