Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
mokhosh opened this issue Apr 19, 2024 · 4 comments
Milestone

Comments

@mokhosh
Copy link
Contributor

mokhosh commented Apr 19, 2024

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

@mokhosh mokhosh added bug Something isn't working low priority unconfirmed labels Apr 19, 2024
@danharrin
Copy link
Member

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

@danharrin danharrin closed this as not planned Won't fix, can't repro, duplicate, stale Apr 19, 2024
@mokhosh
Copy link
Contributor Author

mokhosh commented Apr 20, 2024

@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
Copy link
Member

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
Copy link
Contributor Author

mokhosh commented Apr 22, 2024

@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?

@danharrin danharrin reopened this Apr 22, 2024
@zepfietje zepfietje added this to the v3 milestone May 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Todo
Development

No branches or pull requests

3 participants