Bug or Oversight: allowed filter with value of an empty string bypasses the filter

[jpgilchrist]

I have a controller method, that I want to allow a user to switch a scope filter on and off for controlling the data they are authorized to see that will be returned.

For instance and admin account may want to just see their own personal data or they may want to see all the data for every user they have access manage.

So I created a scope query, but when playing with the query parameter values for the filter method, I noticed that if a user specifies an empty string as the parameter, then it's completely ignored despite having a default value set. I need to turn this behavior off.

/api/widgets?filter[all_users]= triggers the undesired behavior.

        $query = QueryBuilder::for(Widget::class)
            ->allowedIncludes('type', 'type.settings', 'type.settings.options', 'values')
            ->allowedFilters([
                AllowedFilter::scope('all_users')->default(false)
            ]);

    public function scopeAllUsers (Builder $query, bool $value): Builder {
        dd($value);
        if ($value) {
            return $query->whereIn('UserID',  Auth::user()->accessibleUserIds());
        }

        return $query->where('UserID', '=', Auth::user()->Id);
    }

I've also tried to set the ignored values to an empty array or nothing with something like

AllowedFilter::scope('all_users')->default(false)->ignore()

OR

AllowedFilter::scope('all_users')->default(false)->ignore([])

neither work, as the scope function is bypassed completely and the dd($value) is never hit.

[dominikb]

Hi @jpgilchrist,

I've tried reproducing the behavior you are describing but failed at that. Can you specify which version you are using or provide a sample project to reproduce this error?

Heres the test setup I used.

class TestModel {
    public function scopeOnlyFirst(Builder $query, bool $value) : Builder{
        if ($value)
            return $query->where('id', 1);
        return $query;
    }
}

it('interprets an empty string as boolean false', function() {
    $models = createQueryFromFilterRequest(['only_first' => ''])
        ->allowedFilters(AllowedFilter::scope('only_first')->default(false))
        ->get();

    expect($models)->toHaveCount(5);
    expect(TestModel::all()->count())->toBe(5);
});

If you want to dig deeper yourself a good starting point would be FiltersQuery#addFiltersToQuery.

[jpgilchrist]

@dominikb I can try to setup an example if need be. I'm on 5.0.1 currently, just upgraded the other day.

I did do some digging into the FiltersQuery#addFiltersToQuery yesterday and what I found was the following:

    protected function addFiltersToQuery()
    {
        $this->allowedFilters->each(function (AllowedFilter $filter) {
            if ($this->isFilterRequested($filter)) { // this is true
                $value = $this->request->filters()->get($filter->getName()); // $value === null here
                $filter->filter($this, $value); // stepped into this function (see below)

                return; // this returns so a default is never provided since the filter is requested but the value is null
            }

            if ($filter->hasDefault()) {
                $filter->filter($this, $filter->getDefault());

                return;
            }
        });
    }

Stepping into $filter->filter($this, $value);

    public function filter(QueryBuilder $query, $value)
    {
        $valueToFilter = $this->resolveValueForFiltering($value); // step into this function (but returns null)

        if (is_null($valueToFilter)) { // value is null
            return; // returns here
        }

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

Stepping into $this-resolveValueForFiltering($value);

    protected function resolveValueForFiltering($value)
    {
        if (is_array($value)) { // not true
            $remainingProperties = array_diff_assoc($value, $this->ignored->toArray());

            return ! empty($remainingProperties) ? $remainingProperties : null;
        }

        return ! $this->ignored->contains($value) ? $value : null; // $value is null so null is returned (return to above block for continued analysis)
    }