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

Automatically filter by the parent resource #177

Merged
merged 1 commit into from
Sep 29, 2020

Conversation

multimeric
Copy link
Contributor

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
Copy link

Coverage Status

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

@multimeric multimeric changed the title Database independent design Automatically filter by the parent resource Nov 2, 2019
@akira-dev akira-dev merged commit f34a12d into miLibris:master Sep 29, 2020
@kurianbenoy-aot
Copy link

kurianbenoy-aot commented Dec 31, 2020

@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
Copy link
Contributor Author

multimeric commented Dec 31, 2020

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

@kurianbenoy-aot
Copy link

kurianbenoy-aot commented Dec 31, 2020

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
Copy link
Contributor Author

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
Copy link

kurianbenoy-aot commented Jan 1, 2021

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.

  • Isn't this PR not applicable for ResourceList to do similar mapping of relationships as you mentioned in the PR description and issue?

Thanks!

@multimeric
Copy link
Contributor Author

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

@kurianbenoy-aot
Copy link

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,
                              }
  • Shouldn't I remove query and before_create_object?

The full code can be found here

@multimeric
Copy link
Contributor Author

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
Copy link

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

@kurianbenoy-aot
Copy link

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

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

Successfully merging this pull request may close these issues.

Automatically filter related resources by their owner
4 participants