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

Add support for callables in hasMedia() filters #3696

Conversation

crossiatlas
Copy link
Contributor

Description of Changes
I encountered a scenario where I expected $model->hasMedia('my-collection', $filters) to accept a closure for $filters, but it's typehinted to only support an array.
2024-08-20_11-26

This PR updates the typehint to match getMedia()

In the meantime I worked around this via:

$model->getMedia('my-collection', fn (Media $media) => isset($media->custom_properties['something'])))->isNotEmpty()

Changes Made

  • Update hasMedia() to support both array and callable filters, as per getMedia()
  • Update test coverage

Resolves #3526

(NOTE: This is my first contribution to this repository, I believe I've met the requirements but feel free to let me know if there's anything I'm missing and I'll update my PR. Thanks!)

* Update hasMedia to support both array and callable filters, as per getMedia()
* Update test coverage
@freekmurze
Copy link
Member

Could you also update the docs with an example?

@crossiatlas
Copy link
Contributor Author

@freekmurze Absolutely!

It looks like the hasMedia() method isn't documented at all currently (I found it through autocomplete in my IDE), so I assume I'd just need to add this to the Retrieving Media docs?

While I'm at it, public function getFirstMedia(string $collectionName = 'default', $filters = []): ?Media isn't typehinted at all, but it calls getMedia() anyway. So for consistency I will update it to:

public function getFirstMedia(string $collectionName = 'default', array|callable $filters = []): ?Media

I don't think that change will require additional test coverage, getFirstMedia() already has tests for filters (array and callable). The only difference a user may experience is the TypeError they get by passing an invalid $filters type would come from getFirstMedia() instead of getMedia(). Same result, just one level higher.

As far as I can see those are the only methods dependent on getMedia() so we shouldn't need to change any others.

@timvandijck
Copy link
Member

Looks good to me. Thanks for the contribution!

@timvandijck timvandijck merged commit 6a39eca into spatie:main Oct 18, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

hasMedia accept only array
3 participants