Skip to content

Ensured that URL and id field are kept when using sparse fields #1231

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

Merged
merged 1 commit into from
Jun 4, 2024

Conversation

sliverc
Copy link
Member

@sliverc sliverc commented Jun 4, 2024

Fixes #1228

Description of the Change

URL field is considered a field in DRF but it is not in JSON:API spec therefore we may not exclude it during sparse fields. ID on the other hand is a required field and may not be filtered out either as we allow in DJA to overwrite primary key of model.

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

…fields

URL field is considered a field in DRF but is not in JSON:API spec
therefore we may not exclude it.

ID on the other hand is a required field and may not be filtered.
@sliverc sliverc requested a review from n2ygk June 4, 2024 21:07
@sliverc
Copy link
Member Author

sliverc commented Jun 4, 2024

@n2ygk I have finally figured it out. The URL field needs to be part of the fields in the serializer meta class for the error to happen. In the code where you found the error all fields are included and as it is a HyperlinkedModelSerializer URL is also part of it.

Additionally, as mentioned before custom id was also affected by the same error which should be fixed with this PR as well.

Could you try it out with the tutorial repo whether this PR works as expected?

Thanks.

Copy link
Contributor

@n2ygk n2ygk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix worked. Looks good! Thanks for the quick turnaround.

@n2ygk n2ygk merged commit dbe939a into django-json-api:main Jun 4, 2024
11 checks passed
@sliverc sliverc deleted the hyperlinked_sparse branch June 5, 2024 03:25
@sliverc
Copy link
Member Author

sliverc commented Jun 6, 2024

Glad that it is working. Great that you brought this issue up and found it. That was an enlightening dive into our renderer once again. As this is a regression, let me create a new version.

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.

Release 7.0.0 breaks sparse fields request
2 participants