Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Make HyperlinkedRelatedField includable #733

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
Anton-Shutik wants to merge 1 commit into django-json-api:master from Anton-Shutik:master

Conversation

Copy link
Contributor

@Anton-Shutik Anton-Shutik commented Nov 12, 2019

For now it is impossible to skip "data" rendering for any relation and be able to include it via GET parameter. This PR fixes that

Description of the Change

class ProductSerializer():
 variants = json_api_relations.HyperlinkedRelatedField(
 many=True
 )

It fails to GET api/product/1/?include=variants

But with this PR it will work

Checklist

  • PR only contains one change (considered splitting up PR)
  • unit-test added
  • documentation updated
  • CHANGELOG.md updated (only for user relevant changes)
  • author name in AUTHORS

Copy link

codecov bot commented Nov 13, 2019 •
edited
Loading

Codecov Report

Merging #733 into master will decrease coverage by 0.03%.
The diff coverage is 88.88%.

Impacted file tree graph 

@@ Coverage Diff @@
## master #733 +/- ##
==========================================
- Coverage 97.08% 97.04% -0.04% 
==========================================
 Files 54 54 
 Lines 2742 2744 +2 
==========================================
+ Hits 2662 2663 +1 
- Misses 80 81 +1
Impacted Files Coverage Δ
rest_framework_json_api/renderers.py 86.55% <88.88%> (-0.25%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4290b1b...9c0819d. Read the comment docs.

Copy link
Contributor Author

@sliverc would you mind taking a look at this PR ?

Copy link
Member

sliverc commented Nov 14, 2019

Not so sure this is a good idea. A relationship doesn't need to have a relationship pointing back to its parent and therefore we will have an included resource object which is not linked in the compound document.

Also see specification in terms of full linkage. What in the specification do you think would allow combining includes and links in a way this PR allows it?

Copy link
Member

sliverc commented Dec 13, 2019

Any feedback to #733 (comment) or shall we close this PR?

Copy link
Contributor Author

@sliverc Yep, the comment makes a sense and looks like we cannot include related objects if there are no links from resource object due to the spec. I think it can be closed

@Anton-Shutik Anton-Shutik changed the title (削除) Make HyperLinkedOnly field includable (削除ここまで) (追記) Make HyperlinkedRelatedField includable (追記ここまで) Feb 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /