spatie / laravel-query-builder

Easily build Eloquent queries from API requests
https://spatie.be/docs/laravel-query-builder
MIT License
4.06k stars 397 forks source link

Filter doesn't not filter by null/empty #624

Closed vincent-lu closed 3 years ago

vincent-lu commented 3 years ago

This issue seemed was fixed a long time ago here https://github.com/spatie/laravel-query-builder/issues/88 / https://github.com/spatie/laravel-query-builder/commit/2334c94e840de882dd20e8b54c51b357109e5bcd

But using current version of "spatie/laravel-query-builder": "^3.3" I don't get the expected result when:

Request: /users?filter[last_name]= Expected behaviour: User::where('last_name', '')->orWhere('last_name', null) Current behaviour: User::all() // no filtering on last_name at all

Thanks in advance.

freekmurze commented 3 years ago

I'm thinking that changing this behaviour now could be a breaking change. It's probably something for us to fix in the next major release.

vincent-lu commented 3 years ago

np thanks @freekmurze

rickgoemans commented 3 years ago

I'm facing the same issue. I solved it my adding a new scope null and added that scope to the allowed filters.

Scope:

use Illuminate\Database\Eloquent\Builder;

/**
 * Scope nullable due to Spatie's query builder skipping null values in filtering
 * Usage: ?null={column}
 * Example: ?null=description
 *
 * @see https://github.com/spatie/laravel-query-builder/issues/624
 *
 * @param Builder $query
 * @param string $column
 * @return Builder
 */
public function scopeNull(Builder $query, string $column): Builder
{
    return $query->whereNull($column);
}

Controller adding the scope:

QueryBuilder::for(Model::class)
  ->allowedFilters([
    AllowedFilter::exact('id'),
    //...
    AllowedFilter::scope('null'),
  ])

You can use it then by doing /users/?filter[null]=last_name

Hope this helps you (and future others) until nullable functionality is added.

EDIT: Typo and added Eloquent Builder import

rickgoemans commented 3 years ago

This of course only works for a single column with a minor adjustment by the usage of a delimiter (in the example I used "|") you can do something like this:

/**
 * Scope nullable due to Spatie's query builder skipping null values in filtering
 * Usage: ?null={column}|{column2}
 * Example: ?null=description
 * Example: ?null=description|reference
 *
 * @see https://github.com/spatie/laravel-query-builder/issues/624
 *
 * @param Builder $query
 * @param string $columnsString
 * @return Builder
 */
public function scopeNull(Builder $query, string $columnsString): Builder
{
    $columns = explode('|', $columnsString);

    foreach ($columns as $column) {
        $query = $query->whereNull($column);
    }

    return $query;
}
TheDoctor0 commented 2 years ago

There is a simpler solution to this problem - extending the AllowedFilter and overriding the filter function:

class AllowedNullableFilter extends AllowedFilter
{
    public function filter(QueryBuilder $query, $value): void
    {
        $valueToFilter = $this->resolveValueForFiltering($value);

        ($this->filterClass)($query, $valueToFilter, $this->internalName);
    }
}

Then it can be used as you would expect:

QueryBuilder::for(Model::class)
  ->allowedFilters([
    AllowedNullableFilter::exact('column'),
  ])
benswinburne commented 2 years ago

Another way you can do this is

<?php

namespace App\Filters;

use Illuminate\Database\Eloquent\Builder;
use Spatie\QueryBuilder\Filters\Filter;

class TypeNullFilter implements Filter
{
    public function __invoke(Builder $query, $value, string $property)
    {
        $query->when(
            $value === 'null',
            fn($query) => $query->whereNull($property),
            fn($query) => $query->where($property, $value)
        );
    }
}
$query->allowedFilters([
    AllowedFilter::custom('type', new TypeNullFilter()),
]);

Which allows you to pass filter[type]=null onto the query string.

This is obviously an issue if you expect a string value "null" but in my case it was filtering an enum column so no risk of that.

mitchierichie commented 2 years ago

@freekmurze any chance we could revisit this issue since it's been a couple major releases since it was labeled/closed?

I was thinking %00 would be a good/maybe the most safe way to represent NULL in filter strings. Thoughts?

W3 URL Encoding Reference

bianchi commented 1 year ago

I needed to support arrays allowing null values, so I made another condition in @benswinburne code:

<?php declare(strict_types=1);

namespace App\QueryBuilder\Custom;

use Illuminate\Database\Eloquent\Builder;
use Spatie\QueryBuilder\Filters\Filter;

class ExactFilterAllowNull implements Filter
{
    public function __invoke(Builder $query, $value, string $property): void
    {
        if (is_array($value)) {
            $hasNull = collect($value)->contains('null');
            $valuesWithoutNull = collect($value)->filter(fn ($value) => $value !== 'null')->toArray();

            $query->whereIn($property, $valuesWithoutNull);

            if ($hasNull) {
                $query->orWhereNull($property);
            }

            return;
        }

        $query->when(
            $value === 'null',
            fn ($query) => $query->whereNull($property),
            fn ($query) => $query->where($property, $value)
        );
    }
}
Chomiciak commented 8 months ago

Will this issue be addressed?

sneycampos commented 7 months ago

ping @freekmurze this was fixed or any solution in the current release?