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 conditionable allowed filter #987

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

luilliarcec
Copy link
Contributor

@luilliarcec luilliarcec commented Dec 24, 2024

This adds the possibility of providing a closure to the AllowedFilter class, to determine whether or not it should be applied.

@AlexVanderbist
Copy link
Member

Hey @luilliarcec, thanks for the PR!

Currently, it's already possible to dynamically "disable" filters in the allowedFilters method by simply omitting them.

For example:

$queryBuilder
    ->allowedFilters(array_filter([
        $someCondition ? AllowedFilter::exact('user_id') : null
    ]))
    ->get();

I agree that the ->when syntax is more fluent tho, but I'm curious if you have a practical applications for this?

@luilliarcec
Copy link
Contributor Author

luilliarcec commented Jan 2, 2025

Hey @luilliarcec, thanks for the PR!

Currently, it's already possible to dynamically "disable" filters in the allowedFilters method by simply omitting them.

For example:

$queryBuilder
    ->allowedFilters(array_filter([
        $someCondition ? AllowedFilter::exact('user_id') : null
    ]))
    ->get();

I agree that the ->when syntax is more fluent tho, but I'm curious if you have a practical applications for this?

Hi @AlexVanderbist

I don't want to pass up the opportunity to wish you and the spatie team a happy new year, thanks for your great contributions to the Laravel community.

Well, I was born the need, when in an application, certain users could make use of a filter in a matter of a role or permission.

It could even be extended further, to sorts, fields and includes, through a trait.

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.

2 participants