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

[4.x] Make contains modifier handle array needles #8357

Conversation

ryanmitchell
Copy link
Contributor

Sometimes you need to compare arrays to arrays, and it would be nice if the contains modifier was smart enough to handle it.

This PR updates that modifier to check if the intersect count between the 2 arrays is greater than zero, and if so it's true. This allows you to do things like:

{{ collection:pages array_field:contains="this|or|that" }}

or

{{ my_value | contains("this|or|that") }}

Which people seem to assume should work (judging by discord).

I thought it better to do it this way rather than a new modifier.

You could argue it should be a strict comparison, where the intersect count needs to match the needle count, but this approach matches whereJsonContains, and I've updated the queriesConditions contains method too.

@edalzell edalzell changed the title Make contains modifier handle array needles [4.x] Make contains modifier handle array needles Jun 26, 2023
@jasonvarga
Copy link
Member

This allows you to do things like:
{{ collection:pages array_field:contains="this|or|that" }}

Are you sure? I don't believe that uses the modifier.

@ryanmitchell
Copy link
Contributor Author

This allows you to do things like:
{{ collection:pages array_field:contains="this|or|that" }}

Are you sure? I don't believe that uses the modifier.

Forgot to push up the queriesConditions change - I blame @edalzell, he was rushing me

Copy link
Member

@jasonvarga jasonvarga left a comment

Choose a reason for hiding this comment

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

A test for the condition side of this would be very welcomed too. There's a QueriesConditionsTest class.

@ryanmitchell
Copy link
Contributor Author

No problem - added.

Genuine question - is contains the right name/place for this? Going by the modifier it is, looking at queriesConditions I'm not so sure as its very string orientated.

@jasonvarga
Copy link
Member

The functionality is probably fine, but you're right to question the naming. We should give this a little more thought.

@jackmcdade
Copy link
Member

jackmcdade commented Sep 18, 2023

What if we named it has, like Laravel's collection helper that is for essentially the same purpose? https://laravel.com/docs/10.x/helpers#method-array-has

@ryanmitchell
Copy link
Contributor Author

I think has is for keys, where this is checking values?

@jackmcdade
Copy link
Member

jackmcdade commented Sep 19, 2023

I think has is for keys, where this is checking values?

Oh yeah you're right, my apologies.

@jasonvarga
Copy link
Member

Closing in favor of #9491

@jasonvarga jasonvarga closed this Mar 6, 2024
@ryanmitchell ryanmitchell deleted the feature/contains-modifier-handle-array-needles branch March 7, 2024 08:29
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.

None yet

3 participants