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

Setting array delimiter for a single filter #587

Closed
ppisecky opened this issue Jan 19, 2021 · 3 comments
Closed

Setting array delimiter for a single filter #587

ppisecky opened this issue Jan 19, 2021 · 3 comments

Comments

@ppisecky
Copy link

ppisecky commented Jan 19, 2021

As mentioned in issue #450 setting a delimiter on one filter overrides every other filter because the value is stored statically on the class.

It used to be that this was overriding includes and sorts but pull request 451 solved that problem: #451

But it didn't solve the bigger problem of these values being stored globally as a static property of the class and not being limited to the single filter instance being created.

I have found out because I noticed that my date time range filters weren't working because the value that the custom filter received was suddenly a string "2021-01-02 23:00:00,2021-02-23 22:59:59" instead of an array ["2021-01-02 23:00:00","2021-02-23 22:59:59"]. So while I solved my problem of being able to search for strings containing the , character it introduced a new problem where every other filter that relies on comma delimited arrays breaks.

// The first call will set the delimiter statically and thus apply to every other filter
AllowedFilter::scope('search', null, '|')->ignore(null, ""),
AllowedFilter::custom('deadline', new FilterDateTimeRange),

I am of the opinion that if you're going to provide a delimiter symbol parameter in a function call for creating a single filter, then that function call should not have any effect on all the other filters.

@AlexVanderbist
Copy link
Member

That makes sense! Feel free to send a PR that limits the scope of the delimiter to the current instance and I'll be happy to merge that in :)

@kskckbm
Copy link

kskckbm commented Mar 27, 2023

Hi,
any updates on this?
The issue is still existing on version 5.2.0

@Tryfciu
Copy link

Tryfciu commented Aug 22, 2023

Hello, recently we also came across this issue. Any chance it will be adressed in the near future?
@AlexVanderbist

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

No branches or pull requests

4 participants