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

feat: add support to override media model on spatie media file upload #12829

Merged

Conversation

WayneHarris
Copy link
Contributor

@WayneHarris WayneHarris commented May 16, 2024

Description

Adding support to override the media model on SpatieMediaLibraryFileUpload in line with the ability to do so in the Spatie Media Libday since version 11.4.0

Visual changes

Functional changes

  • Code style has been fixed by running the composer cs command.
  • Changes have been tested to not break existing functionality.
  • Documentation is up-to-date.

@WayneHarris
Copy link
Contributor Author

@danharrin, What are your thoughts on this? I can test and review further if you think it is worthwhile.

Copy link
Member

@danharrin danharrin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the part that reads getMediaModel() from the record is fine, but I don't understand the need for the new property and methods to set a component-specific media class? Especially when it's only used within the method that reorders files, not anywhere else? I think this could be greatly simplified to support the new method on the model?

@danharrin danharrin added the enhancement New feature or request label May 17, 2024
@danharrin danharrin added this to the v3 milestone May 17, 2024
@danharrin danharrin self-assigned this May 17, 2024
@WayneHarris
Copy link
Contributor Author

I think the part that reads getMediaModel() from the record is fine, but I don't understand the need for the new property and methods to set a component-specific media class? Especially when it's only used within the method that reorders files, not anywhere else? I think this could be greatly simplified to support the new method on the model?

Agree, I did wonder on this too, I'll confirm it's redundant and remove it.

@WayneHarris WayneHarris marked this pull request as ready for review May 17, 2024 11:58
@danharrin
Copy link
Member

Thank you

@danharrin danharrin merged commit 66f7a2b into filamentphp:3.x May 17, 2024
10 checks passed
@WayneHarris WayneHarris deleted the feat/3.x-support-overide-media-model branch May 17, 2024 12:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

2 participants