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

Filtering data by relationship fields #54

Closed
lcsdms opened this issue Mar 21, 2021 · 13 comments
Closed

Filtering data by relationship fields #54

lcsdms opened this issue Mar 21, 2021 · 13 comments

Comments

@lcsdms
Copy link

lcsdms commented Mar 21, 2021

Hi! First of all, thank you for the great package!

I'm learning and testing it, and I wanted to know, if this package does support filtering data by model and its relationship (like a inner join) ?

Example:

Resources (Models) :

  • posts (id, title, author_id )
  • author (id,name,email) - Belongs to posts

And so I want to get:

  • posts with title 'test'
  • including the author
  • additional filter by author, where id = 1
GET https://myaddress.com/api/v1/posts?include=author&filter['title']=test&filter['author.id']=1

(The url sintax is just an example, there are use cases where people would use [author][id]=1 for example.

I've checked the issue #49 but could not find a conclusive answer.

@lcsdms lcsdms changed the title Filtering data by relationship Filtering data by relationship fields Mar 21, 2021
@lindyhopchris
Copy link
Contributor

Hi! Thanks, glad you like the package!

So my advice with this is always: write the query as an Eloquent builder query before working out what the filter should be

So in your example, title and author id query would be:

Post::query()->where('title', $title)->where('author_id', $authorId)

So that's very simple to implement using the Where filter class. It might be worth using WhereIn for the author_id which would allow the client to send one-to-many author ids to filter by - that's typically what I do in the APIs that I write.

This works for author because the author_id is stored on the posts table, so is super easy to implement. If you wanted to filter by a relationship that did not have an id stored on the table being queried, then in Eloquent you use the has and whereHas methods on the query builder:
https://laravel.com/docs/8.x/eloquent-relationships#querying-relationship-existence

So if we wanted to find posts that a specific user has commented on:

Post::query()->whereHas('comments', function ($query) use ($userId) {
    $query->where('user_id', $userId);
});

So for that you'd need to write a JSON:API filter that calls the whereHas method.

@lcsdms
Copy link
Author

lcsdms commented Mar 22, 2021

Hey @lindyhopchris , thanks for the response!

Yeah, the author id was a bad example 😅 , better using author name.

The whereHas seems to be the way to go! I'll try it!

I'll start simple for now, but later on I was thinking of making a filter that receives the relationship schema and builds the filter reusing the available filters set on the schema, so I don't need to rewrite it (unless I want it to override it or something).

It would be nice if I could reuse the validation structure that validates accepted filters.

What do you think , is that viable and a good idea ?

@lindyhopchris
Copy link
Contributor

Yes, I definitely think a filter that allows you to use whereHas with filters from the related schema (e.g. CommentSchema in the example I gave above with the comments relationship) would be totally possible. I think that should be added to the laravel-json-api/eloquent package as an official filter.

I think probably three filters need to be added:

  • Has which would expect a boolean value from the client, which then calls $query->has('comments') if true, and $query->doesntHave('comments') if false.
  • WhereHas which expects an array of filters it can apply based on the related schema (as you suggested) using the $query->whereHas() method.
  • WhereDoesntHave which expects an array of filters it can apply based on the related schema using the $query->whereDoesntHave() method.

I'm really pushed at the moment - spending an incredible amount of time on this package and need to crack on with real work! Any chance you'd have time to take a look at implementing Has and WhereHas?

@lcsdms
Copy link
Author

lcsdms commented Mar 22, 2021

Yep, I'll take a look at the structure in the next few days and try implementing it and get back to you with a proof of concept so we can align the best implementation ! Thanks !

@lcsdms
Copy link
Author

lcsdms commented Mar 23, 2021

@lindyhopchris , I've implemented a working poc of the whereHas method, but I noticed that I can't access the filter from the related schema, unless the ones that are defined using the "withFilters" method from the relationship configuration on schema.

On the documentation It says you can provide additional filters (https://laraveljsonapi.io/docs/1.0/schemas/filters.html#relationship-filters) , using the withFilters method, but getting them from the schema->relationship($this->relationName)->filters(); does not return the original filters described on the relation schema, only the ones declared on the withFilters method.

If I don't use the withFilters method, no filters are returned.

Is this intentional ? Because it would generate duplicate code if I were to allow all Schema fields to be filtered. Also I couldn't find a way to access the relationship schema to get those filters as well.

Here's an example and filter:
UserSchema.php

//...class definition
public function fields(): array
    {
        return [
            // other fields...
            HasMany::make('roles')->readOnly(
                static fn ($request) => !$request->user()->can('edit user')
            )->withoutDefaultPagination()->withFilters(
                Where::make('name'),
                WhereIn::make('id')
            )
        ]
    }

public function filters(): array
    {
        return [
            WhereHas::make('roles', $this)
        ];
    }

WhereHas.php

<?php

namespace App\JsonApi\Filters;

use LaravelJsonApi\Eloquent\Contracts\Filter;
use LaravelJsonApi\Eloquent\Filters\Concerns\DeserializesValue;
use LaravelJsonApi\Eloquent\Filters\Concerns\IsSingular;
use LaravelJsonApi\Eloquent\Schema;

class WhereHas implements Filter
{
    use DeserializesValue;
    use IsSingular;

    /**
     * @var string
     */
    private string $relationName;

    private Schema $schema;

    /**
     * @var string
     */
    private string $operator;

    /**
     * SearchFilter constructor.
     *
     * @param string $relationName
     * @param array $columns
     */
    public function __construct(string $relationName, $schema)
    {
        $this->relationName = $relationName;
        $this->schema = $schema;

        if (!$this->schema->isRelationship($relationName)) {
            throw new \LogicException("Relationship with name $relationName not defined in ". get_class($schema)." schema.");
        }
    }

    /**
     * Create a new filter.
     *
     * @param string $relationName
     * @param Schema $schema
     * @return WhereHas
     */
    public static function make(string $relationName, Schema $schema): self
    {
        return new static($relationName, $schema);
    }

    /**
     * @inheritDoc
     */
    public function key(): string
    {
        return $this->relationName;
    }

    /**
     * Apply the filter to the query.
     *
     * @param \Illuminate\Database\Eloquent\Builder $query
     * @param mixed $value
     * @return \Illuminate\Database\Eloquent\Builder
     */
    public function apply($query, $value)
    {
        $deserializedValues = $this->deserialize($value);

        $availableFilters = $this->schema->relationship($this->relationName)->filters();

        $keyedFilters = collect($availableFilters)->keyBy(function ($filter) {
            return $filter->key();
        })->all();

        return $query->whereHas($this->relationName, function ($query) use ($deserializedValues, $keyedFilters) {
            foreach ($deserializedValues as $key => $value) {
                if (isset($keyedFilters[$key])) {
                    $keyedFilters[$key]->apply($query, $value);
                }
            }
        });
    }
}

@lindyhopchris
Copy link
Contributor

So you need to get the filter from the inverse schema, as well as the relationship filters. The inverse schema is the schema for the resources on the other side of the relationship - i.e. the CommentSchema for the comments relationship on a posts resource.

To access the inverse schema, you use the relationship's schema() method.

So for your WhereHas class you'd need to do:

$relation = $this->schema->relationship($this->relationName);
$availableFilters = collect($relation->schema()->filters())->merge($relation->filters());

@lcsdms
Copy link
Author

lcsdms commented Mar 24, 2021

Thanks @lindyhopchris ! Working like a charm now.

I'm going to create a pull request on the filters repository, Is the WhereHas Filter OK as is now, or you think some new adjustments should be made ?

Following your specs, they should work along thoses lines as I understand:

Has::make($relationName, $schema)
usage: api/v1/posts?[filter][comments][true]

WhereHas::make($relationName, $schema)
usage: api/v1/posts?[filter][comments][body]=Hello

WhereDoesntHave::make($relationName, $schema)
usage: api/v1/posts?[filter][comments][body]=Hello

@lcsdms
Copy link
Author

lcsdms commented Mar 24, 2021

Also, I was taking a look at some filtering relationship ideas (since it's not strictly specified on the json api) and I encountered also this idea, that filters the result of the relationship instead of filtering the main resource.

https://discuss.jsonapi.org/t/filtering-querying-deep-relationships-a-proposal-for-a-syntax/1746

But for this to work, it would need some adjustments on the package to identity the dot syntax on the filter name, right ?

Examples:

WhereHas($relationName,$this)
Eg: api/v1/posts[filter][comments][body]=Hello?include=comments
Result: Filters posts that have comments with Hello

WhereRelationHas($relationName,$this)
Eg: api/v1/posts[filter][comments.body]=Hello?include=comments
Returns all posts and filters related comments that has Hello in it.

@lindyhopchris
Copy link
Contributor

@lcsdms if you submit a PR to the Eloquent package, I can comment on it when I review it. The class works but needs some tidying up - it'd be easier for me to do that via review comments.

As for filtering included relationships. Sounds nice, but would be a considerable amount of work. The problem is it wouldn't be implemented via filters - it actually needs to modify the eager loading implemention, which is already significantly complex. That's because in Eloquent terms it would need to do this:
https://laravel.com/docs/8.x/eloquent-relationships#constraining-eager-loads

I've never investigated how constraining eager loads works with nested relationships. The eager loading implementation supports nesting as the JSON:API include path uses dot notation.

Can we keep this issue on topic and not venture into modifying the eager loading implementation?! Just to keep my sanity?! There's already too much for me to do at the moment so I can't really have mission creep into nice-to-have-crazily-complex features!!

@lcsdms
Copy link
Author

lcsdms commented Mar 27, 2021

No problem! Let's keep this into topic, I'll create the pull request and we'll go from there. Thanks !

@devinfd
Copy link

devinfd commented Apr 8, 2021

As I'm creating my API it seems that I could also use a "WhereHas" relationship filter. Or maybe there is an alternative for my use case? I have a Rescuer schema with the following indexQuery:
note: A rescuer is defined as person who has patients

public function indexQuery(?Request $request, Builder $query): Builder
{
    return $query->where('account_id', $request->user()->account_id)->whereHas('patients');
}

What I would like to have is a filter on the ->whereHas('patients'); part, ie:

public function indexQuery(?Request $request, Builder $query): Builder
{
    return $query->where('account_id', $request->user()->parent_account_id)->whereHas('patients', function ($query) use ($request) {
        $query->where('admitted_at', '>=', $request->filters['dateFrom']);
    });
}

However this requires defining the filters in the Schema which would affect the query in other ways.

@lindyhopchris
Copy link
Contributor

@devinfd what you need to do is write a filter that does that whereHas with the sub-query.

Then in the schema's indexQuery() method, only call whereHas('patients') if the request does not have the filter. I.e. you only need to call whereHas in the indexQuery when the filter is not provided.

@lcsdms
Copy link
Author

lcsdms commented Apr 24, 2021

Closing and moving additional discussion on the correct repository:

laravel-json-api/eloquent#8

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

No branches or pull requests

3 participants