Skip to content

Include only a link in the relationships object #296

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

Closed
arielpontes opened this issue Nov 15, 2016 · 9 comments
Closed

Include only a link in the relationships object #296

arielpontes opened this issue Nov 15, 2016 · 9 comments

Comments

@arielpontes
Copy link
Contributor

According to the JSONAPI specification, if I have an Article model with a to-many relationship to a Comment model and I want to express this when I retrieve an Article resource, there are multiple valid ways I can do this:

  1. Include the comments by default in the response, having a list of resource identifier objects in the relationships object and a list with the actual resource objects in the included object.
  2. Return a list of resource identifier objects in the relationships object only, without including the actual attributes of the comments.
  3. Include only a link (e.g. /articles/1/comments) in the relationships object.

I have a model that links to a lot of related objects so the 3rd option would be the best for me, but I can't configure the ResourceRelatedField to act like that. It always returns a data object with the full list of related resource identifiers. Is there a way I can avoid this? Or do I have to extend the class myself? Doesn't seem very trivial to do that in a clean way.

Ideally, since the JSONAPI specs accept these 3 options, I think this library should allow these 3 configurations out of the box.

@benjaminlong
Copy link

benjaminlong commented Feb 7, 2018

Hello @arielpontes, have you managed to remove the data attribute when trying to use ResourceRelatedField ? I would like to have the same behavior. I know your question is an old one... but it could really help me.

@benjaminlong
Copy link

benjaminlong commented Feb 18, 2018

Hello ! if the community thinks it's a good improvement, I will have some time to work on this in the next month. Let me know if there are any first thoughts/ideas.

  • Should we add a link_only option ?
  • Should we...
    • return the link, when related_link_view_name and related_link_lookup_field options of the ResourceRelatedFiled are presents.
    • return the data if those optiosn aren't presents.
  • A third option ?

All the best.

@sliverc
Copy link
Member

sliverc commented May 3, 2018

There is a relations.HyperlinkedRelatedField do you think this does what you expect?

@Anton-Shutik
Copy link
Contributor

Anton-Shutik commented Jul 12, 2018

@sliverc I didn't find a way I can use relations.HyperlinkedRelatedField and get links only in the relationship payload. May be I doing smth wrong ? Anyway, I think that impossible to do via DRF serializers or fields, because I've walked through extract_relationships and there is no way to get data key omitted and have links only

@arielpontes I've made a PR that skips the "data", I think that is what you need to get your point 3. Hope that helps you

@sliverc
Copy link
Member

sliverc commented Jul 12, 2018

@Anton-Shutik I guess a patch would need to be created then where data is not included when using HyperlinkedRelatedField (which a user expects I guess when reading DRF documentation) instead of patching ResourceRelatedField. This feels more natural too me and better follows DRF.

Marking this as a bug then.

@sliverc sliverc changed the title Add basic options for ResourceRelatedField Include only a link in the relationships object Jul 12, 2018
@Anton-Shutik
Copy link
Contributor

@sliverc first of all, thank you so much for fast responses :)

Agree, up to a point. But DRF's HyperlinkedRelatedField does not handle self and related links, it just returns a single link. All that stuff is handled in ResourceRelatedField. I'm not sure if it possible to use the DRF's HyperlinkedRelatedField to get a valid json api links object.
What if I will add HyperlinkedResourceRelatedField class, inherited from ResourceRelatedField that will omit data in the output by default ? Actually I don't like the idea too much, because ResourceRelatedField handles both links and data, but not just data. We may want to rename it to HyperlinkedResourceRelatedField, and add a parameter render_data or links_only or smth.
What do you think about the ideas ? Or may be you have your own that seems better ?

@sliverc
Copy link
Member

sliverc commented Jul 12, 2018

@Anton-Shutik

But DRF's HyperlinkedRelatedField does not handle self and related links, it just returns a single link.

Valid point. When looking at your PR I just have a feeling using ResourceRelatedField leads to many workarounds to avoid running of SQL.

Potentially implementing a json api specific HyperlinkedRelatedField deriving from DRF version might make this easier.

I have only looked briefly into it so I am not sure but I think it is properly good to investigate as it might be the more elegant solution without potential side effects for users of ResourceRelatedField.
What do you think?

@Anton-Shutik
Copy link
Contributor

@sliverc I agree that having two fields is better than having a big one :)

But it is not the case, I think. Since ResourceRelatedField renders "data", "links", "meta", it should handle all these things. We cant have 3 Fields like LinksField, DataField, MetaField, and use them separately. We have one ResourceField that handles all these things.

I've tried to create HyperlinkedRelatedField based on DRF's but came up rewriting same code from ResourceRelatedField about link handling.

My changes does not affect ResourceRelatedField until you pass render_data=False explicitly.

I'm opened to other approaches to implement "data" skipping, but for now I use my fork and if you're ok with merging my PR I can add some tests and docs.

@sliverc
Copy link
Member

sliverc commented Jul 16, 2018

Let's move our discussion into the PR as our discussion somehow moved into implementation details, but I think the issue here is clear how the end result should look like.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants