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

withAnyTagsOfAnyType not filtering on multiple values #243

Closed francoism90 closed 5 years ago

francoism90 commented 5 years ago

Take the following example:

// https:// .. /api/media?include=image,tags&filter[tagged]=Drama,Horror,Docu

public function index()
    {
        $query = QueryBuilder::for(Media::class)
            ->allowedIncludes(['tags', 'image'])
            ->allowedFilters([
                Filter::scope('tagged', 'withAnyTagsOfAnyType'),
            ])
            ->allowedSorts('title', 'created_at', Sort::field('duration', 'duration_secs'))
            ->jsonPaginate();

        return MediaResource::collection($query);
    }

It should return two models: 1 (tagged Drama & Horror) and 3 (tagged Docu), however it only returns the one as given in as first value (1). Using Docu would return 3.

Is this normal? Should this be converted into an array or something else I should do?

Thanks!

PS. I'm using the latest laravel-tags

francoism90 commented 5 years ago

I've solved this by using a custom filter:

<?php

namespace App\Filters;

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

class FilterIsTagged implements Filter
{
    public function __invoke(Builder $query, $value, string $property) : Builder
    {
        return $query->withAllTags($value); // or withAllTagsOfAnyType
    }
}
$query = QueryBuilder::for(Media::class)
            ->allowedIncludes(['tags', 'image'])
            ->allowedFilters([
                Filter::custom('tagged', FilterIsTagged::class),
            ])
            ->allowedSorts('title', 'created_at', Sort::field('duration', 'duration_secs'))
            ->jsonPaginate();

Is this a bug in the scope? Or something I should have done in the first place? :)

Have a nice weekend.

leeovery commented 5 years ago

I noticed this too. It's actually an issue with the tag package and not this package.

That scope results in pulling the first tag from the DB to search against. It needs a little tweak to pull them all.

https://github.com/spatie/laravel-tags/blob/a55e1ae6212e1c014aa809e67b15d26ff4e3e826/src/Tag.php#L79

Rather than using ->first() it should probably pull an array. Not dug into it. I ended up using a custom filter like you :)

francoism90 commented 5 years ago

@leeovery I'll keep using the filter/sorters when needed. :)

Thanks for looking into the issue, hopefully it will be fixed any time soon.