-
Notifications
You must be signed in to change notification settings - Fork 299
JSON:API 1.1: return empty 'included' array when ?include= is present #1301
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
JSON:API 1.1: return empty 'included' array when ?include= is present #1301
Conversation
Fixes django-json-api#1109. Ensures top-level 'included' is returned as [] when the query param ?include= is provided but no related resources exist. Updates integration tests to reflect JSON:API v1.1 spec compliance.
Fixed tox lint config issue. Also, verified locally with tox -e lint and all checks passing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this PR.
I would recommend being careful when you use AI. Always verify what it does, before accepting it.
For instance the pull request template and its checklist is actually made for human beings and not AI.
In the checklist it states to add changelog entry and add yourself to authors. Great if you could follow up on those.
Let me know if you have any questions.
Thanks for the feedback @sliverc. I’ll make sure to review my changes instead of relying on AI.
Regarding the failing test i.e. test_search_keywords, i noticed it fails because included: [] is now always present. should i update the test to match this new behavior, or should i adjust the implementation to keep the previous behavior?
Thanks for following up. You need to adjust the test_search_keywords test because this test relies on a view which has default included resources, so there should be an empty-included array.
@sliverc you can let me know if you expect any changes to be made further.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There were some not necessary new lines added which I reverted, it looks good now and ready for merging. Thanks for your work on this.
@sliverc thanks for helping me make my first open source contribution! Looking forward to connect with you linkedin.
Uh oh!
There was an error while loading. Please reload this page.
Fixes #1109
This PR fixes handling of the included array so that it correctly respects both JSONAPIMeta.included_resources defaults and explicit include query parameters.
Description of the Change
Checklist
CHANGELOG.mdupdated (only for user relevant changes)AUTHORS