miLibris / flask-rest-jsonapi

Flask extension to build REST APIs around JSONAPI 1.0 specification.
http://flask-rest-jsonapi.readthedocs.io
MIT License
598 stars 153 forks source link

Automatically filter by the parent resource #177

Closed multimeric closed 3 years ago

multimeric commented 4 years ago

Closes #176.

Broadly this adds a new argument, filters, to the get_collection() data layer, and then uses this argument to filter the ResourceList so that it only returns results attached to a parent resource, e.g. GET /articles/1/comments should only return comments for article 1.

coveralls commented 4 years ago

Coverage Status

Coverage decreased (-0.06%) to 90.709% when pulling bd7da79315177d44e429ced9005ec382023b829f on TMiguelT:filter-related into b44bc08b11213d49fadae873650d3555889052ec on miLibris:master.

kurianbenoy-aot commented 3 years ago

@TMiguelT @akira-dev so is there is no reason of having this boilerplate as in the documentation now? So is ComputerList as the below snippet required?:

class ComputerList(ResourceList):
    schema = ComputerSchema
    data_layer = {'session': db.session,
                  'model': Computer}

It doesn't look like this feature doesn't work at my end.

multimeric commented 3 years ago

Are you referring to the before_get_object boilerplate in the PersonDetail? Yes, it should eliminate that.

kurianbenoy-aot commented 3 years ago

Are you referring to the before_get_object boilerplate in the PersonDetail? Yes, it should eliminate that.

Thanks @TMiguelT for your reply. I tried PersonDetail after removing before_get_object boilerplate. Yet when I call the URL associated with Relationship of PersonDetail. I get the following error:

KeyError: 'id'

I was expecting the issue to filter ResourceList type relationships, which just have one associated id to be resolved with this issue. Yet I am getting the same behaviour where calling /persons/2/computers returns the list of all computers instead of just computers associated with id 2.

I checked that my flask_rest_json_api version is 0.31.2 itself

I am sharing my complete version of getting started guide code here

multimeric commented 3 years ago

So in your case, I think the problem is that you need to have the route defined as: api.route(PersonDetail, 'person_detail', '/persons/<int:id>', '/computers/<int:id>/owner'). Note that I use id, not computer_id, because id is the actual name of the field on the Computer schema.

kurianbenoy-aot commented 3 years ago

Ok, now it works on changing the route: api.route(PersonDetail, 'person_detail', '/persons/<int:id>', '/computers/<int:id>/owner') with actual name of field on computer schema.

Thanks!

multimeric commented 3 years ago

Yes, I think this should also work for resource lists like /persons/<int:id>/computers

kurianbenoy-aot commented 3 years ago

Yes, I think this should also work for resource lists like /persons/<int:id>/computers

On running the /persons/<int:id>/computers, I get a list of all computers instead of just computers associated with that person.

class ComputerList(ResourceList):

    schema = ComputerSchema
    data_layer = {'session': db.session,
                  'model': Computer,
                              }

The full code can be found here

multimeric commented 3 years ago

Ah it's been a while since I wrote this PR but I believe that there are two things I never made clear:

  1. This PR only applies to ResourceList resources. The mechanism for filtering ResourceDetail endpoints is different: https://github.com/TMiguelT/flapison/blob/bd7da79315177d44e429ced9005ec382023b829f/flask_rest_jsonapi/data_layers/alchemy.py#L85-L88
  2. The filtering is done through the database layer, and bypasses the schemas. So you want the parent ID parameter in the URL to be the same as the parent foreign key. In the case of /persons/<int:id>/computers, the foreign key that connects a Computer to a Person is person_id, so it should be changed to /persons/<int:person_id>/computers. Note again that you only want to do this for the ResourceList resources. The other ones aren't affected by this PR.
kurianbenoy-aot commented 3 years ago

Thank you very much @TMiguelT your explanation helps a lot.

kurianbenoy-aot commented 3 years ago

I tried /persons/<int:person_id>/computers for getting the parent foreign key with flask_rest_jsonapi and flapison library, but the automatic filtering for ResourceList resources didn't work. It just returned all computers