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.51k stars 2.98k forks source link

Can't have medialibrary fields in two fieldsets for the same relationship in the same form #12401

Open mokhosh opened 7 months ago

mokhosh commented 7 months ago

Package

filament/spatie-laravel-media-library-plugin

Package Version

v3.2.70

Laravel Version

v11.4.0

Livewire Version

No response

PHP Version

8.3.6

Problem description

I have a wizard that saves some data on User, and some data on Profile which belongs to a user.

The fields that are related to Profile are put on two different steps of a Wizard inside a Fieldset with relationship method pointing to the HasOne profile relationship on User model.

It works fine if I only have one fieldset linked to this relationship, but breaks when I have two. This only happens with SpatieMediaLibraryFileUpload component. Normal fields work fine. They are on the same relationship, but I've used different names in the make method.

Cloning the relationship works, but this is a nasty hacky fix. Any way we can avoid that?

Expected behavior

SpatieMediaLibraryFileUpload should also just work like other form fields on related models.

Steps to reproduce

  1. Have two related models
  2. Create two fieldsets on the related model inside a form
  3. Put SpatieMediaLibraryFileUpload components in one of the fieldsets

Reproduction repository

https://github.com/mokhosh/filament-medialibrary-multiple-fieldset-bug

Relevant log output

No response

Donate đŸ’° to fund this issue

Fund with Polar

danharrin commented 7 months ago

Unfortunately this is not possible, you may only have one component for a particular relationship per form, otherwise they will overwrite each others' data.

mokhosh commented 7 months ago

@danharrin thanks for your response, but this does work with normal components just fine. Only medialibrary components have this issue. Doesn't this mean maybe we can work around whatever the issue is for medialibrary components as well?

Just for reference, a kind user on discord suggested I clone the relationship with another name, to which I said this is a nasty hacky fix, and he asked why I think so, and this is my answer:

I think it's nasty because I will have an unnecessary duplication in my model code, which is only there because of a presentation layer decision to separate the form into multiple wizard steps. Otherwise I would never create two identical relationships with different names. One sign of this smell is the fact that I have to write a comment above one of the relationship methods to avoid any confusion why we have two methods for the same relationship. And you can't easily test this, so if someone confident comes in the future and deletes the method, they won't know that they have broken the app. They will just think I was stupid to have such duplication in my code. Does this make sense?

danharrin commented 7 months ago

I know it's a nasty fix, but I don't know how technically possible this is. Does this also happen on the Spatie tags input?

mokhosh commented 7 months ago

@danharrin Yes I can confirm this happens with Spatie tags as well. I've updated the reproduction repo to include tags.

Can we maybe have this open at least as a low priority issue, maybe we can find a way?

javierpomachagua commented 2 months ago

Was it resolved?

abbasmashaddy72 commented 5 days ago

The issue occurs because, in Spatie Media Library, the collection name defaults to a generic value when it is not explicitly set. This means that multiple SpatieMediaLibraryFileUpload fields, without explicitly defined collection names, will share the same default collection. This causes one field's data to override the other, leading to conflicts.

Solution

To resolve this, you need to explicitly specify unique collection names for each SpatieMediaLibraryFileUpload component. Here's how you can do it:

SpatieMediaLibraryFileUpload::make('previous_policy')
    ->collection('previous_policy') // Assign a unique collection name
    ->acceptedFileTypes([
        'application/zip', 'application/x-zip-compressed', 'application/octet-stream',
        'application/x-rar-compressed', 'application/vnd.rar', 'application/x-rar',
        'application/x-tar', 'application/pdf',
    ])
    ->helperText('.zip, .rar, .tar, .pdf'),

SpatieMediaLibraryFileUpload::make('current_policy')
    ->collection('current_policy') // Assign a unique collection name
    ->acceptedFileTypes([
        'application/zip', 'application/x-zip-compressed', 'application/octet-stream',
        'application/x-rar-compressed', 'application/vnd.rar', 'application/x-rar',
        'application/x-tar', 'application/pdf',
    ])
    ->helperText('.zip, .rar, .tar, .pdf'),

Why Does This Happen?

By default, Spatie Media Library assigns a generic collection name if none is specified. When multiple SpatieMediaLibraryFileUpload fields in the same form or wizard share the default collection, they conflict because:

This approach is essential when working with forms or wizards that involve multiple SpatieMediaLibraryFileUpload fields pointing to the same relationship or model. By explicitly defining unique collection names, you ensure that each field operates as intended without unintended side effects.

@mokhosh

mokhosh commented 5 days ago

@abbasmashaddy72 I don't think this is the issue.

Both in the reproduction repo, and in my actual project I am using explicit collection names.

abbasmashaddy72 commented 5 days ago

In reproduction url i don't see you are using SpatieMediaLibraryFileUpload multiple times if you are facing the issue still try adding ->statePath("data") I faced the same issue of upload in relationshipManager may that can fix the issue