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 filter by operator #940

Merged
merged 4 commits into from
Sep 27, 2024
Merged

Conversation

AbdelrahmanBl
Copy link
Contributor

@AbdelrahmanBl AbdelrahmanBl commented May 15, 2024

Dear sir
I had been developed out of the box a method for filtering by operator.
for the example:

QueryBuilder::for(User::class)
->allowedFilters([
    AllowedFilter::operator('salary', FilterOperator::GREATER_THAN),
])
->get();

Note: I realized that there is some old test cases not working.

@siarheipashkevich
Copy link

@AbdelrahmanBl is it possible to pass the operator through query parameters?

@AbdelrahmanBl
Copy link
Contributor Author

AbdelrahmanBl commented May 15, 2024

It's really a great idea!
Now you can search by a dynamic filter operator.
for the example:

// users?filter[salary]=>=1000,<=3000
QueryBuilder::for(User::class)
->allowedFilters([
    AllowedFilter::operator('salary', FilterOperator::DYNAMIC),
])
->get();

@AbdelrahmanBl
Copy link
Contributor Author

@freekmurze

@freekmurze freekmurze requested a review from AlexVanderbist May 21, 2024 12:25
@AbdelrahmanBl
Copy link
Contributor Author

Hello sir! How are you? Can you check this feature?
@freekmurze

@benswinburne
Copy link

benswinburne commented Jun 1, 2024

Hello sir! How are you? Can you check this feature? @freekmurze

@AbdelrahmanBl Please stop deleting your previous comment and @ people. No doubt they're busy and they'll get to it when they get to it.

@AbdelrahmanBl
Copy link
Contributor Author

AbdelrahmanBl commented Jun 1, 2024

Thank you for your response 🙏 and i won't contribute with spatie anymore for not annoying them.

@benswinburne
Copy link

I'm not from Spatie, just someone whose inbox you were spamming. Repeatedly @ing anyone isn't productive be it Spatie or anyone else.

I'd subscribed to the thread because i'm interested in the feature as I'm sure others are too. Closing it because someone suggest you be patient seems overkill, but it is your PR to do what you will with.

@AbdelrahmanBl
Copy link
Contributor Author

I thought you were from spatie and no problem i will reopen it to benefit the others 😊

@AbdelrahmanBl AbdelrahmanBl reopened this Jun 2, 2024
@AbdelrahmanBl
Copy link
Contributor Author

@freekmurze kindly check this commit!

@cgarcia-lightit
Copy link

cgarcia-lightit commented Aug 30, 2024

Hi @freekmurze,

This improvement could be considered to be released. I mean, it's a good feature, I don't understand why not implement the feature. Are there some pending considerations? Or Do you want to implement this feature in another way? It will be great to know it.

Thanks

@freekmurze
Copy link
Member

Could you update the readme with clear examples on how to use this?

@cgarciagarcia
Copy link
Contributor

Hi @AbdelrahmanBl ,

Could you generate the requested documentation? We are close to get merge this pr 😄

@AbdelrahmanBl
Copy link
Contributor Author

@cgarciagarcia
Okay i will implement that as soon as possible

@cgarciagarcia
Copy link
Contributor

Hello @AbdelrahmanBl , I would like to know if there are posibilities to include the documententation that freekmurze requested to merge this PR. I like your feature and I think it's a good resolution approach.

@AlexVanderbist
Copy link
Member

Thanks everyone for your input and hard work! I'll add docs for this feature and tag it as a new feature release.

@AlexVanderbist AlexVanderbist merged commit 09d9162 into spatie:main Sep 27, 2024
2 checks passed
@siarheipashkevich
Copy link

@AbdelrahmanBl thank you

@AbdelrahmanBl
Copy link
Contributor Author

thanks guys, i'm very busy nowadays and that is the reason for not completing the doc

@cgarcia-lightit
Copy link

Thanks @AbdelrahmanBl for your work! and spatie team for your care.

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.

8 participants