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 not recognizing relationship #640

Closed mitchierichie closed 3 years ago

mitchierichie commented 3 years ago

https://spatie.be/docs/laravel-query-builder/v3/features/filtering#exact-or-partial-filters-for-related-properties

Following the instructions in the link above, I tried to create an AllowedFilter for one of my relationships. My query fails because instead of filtering by my related model, it tries to find a column with that name on the main model's table.

SQLSTATE[42S22]: Column not found: 1054 Unknown column 'product_model.slug' in 'where clause' 

I did some digging - seems Spatie\QueryBuilder\Filters\FiltersExact, L51 is responsible. I've included the source for that method here:

    protected function isRelationProperty(Builder $query, string $property): bool
    {
        if (!Str::contains($property, '.')) {
            return false;
        }

        if (in_array($property, $this->relationConstraints)) {
            return false;
        }

// line 51
        $firstRelationship = Str::camel(explode('.', $property)[0]);

        if (!method_exists($query->getModel(), $firstRelationship)) {
            return false;
        }

        return is_a($query->getModel()->{$firstRelationship}(), Relation::class);
    }

Why would it change the requested relationship name to camel case? Wouldn't you just use the exact relationship name provided by the developer, not modify it and then fail to find relationships?

AlexVanderbist commented 3 years ago

Why would it change the requested relationship name to camel case? Wouldn't you just use the exact relationship name provided by the developer, not modify it and then fail to find relationships?

This tried to follow 2 best practices: case insensitive urls & camelcase for Laravel relationships. Ultimately not the best choice for this package. Let's continue this issue in the #641

mitchierichie commented 3 years ago

@AlexVanderbist I see, well I appreciate the quick turnaround