Skip to content
This repository has been archived by the owner on Jun 6, 2024. It is now read-only.

Fix included_data and data["included"] type consistency. #268

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from

Conversation

AdrianVandierAst
Copy link

The included resource objects in the json api are in a list but the schema implementation
needs it to be a dict indexed by type and id.
The new test checks that loading and then dumping with the same schema object on a
dict with an included resource works. It is needed for Flask-rest-jsonapi that re-uses schema between load and dump.

Adrian added 3 commits November 7, 2019 21:01
included resource objects in the json api is a list but the schema implementation
needs it to be a dict indexed by type and id.
The new test checks that loading and then dumping with the same schema object on a
dict with an included resource works. It is needed for Flask-rest-jsonapi that re-uses schema between load and dump.
@AdrianVandierAst AdrianVandierAst changed the title Fix included_data and data["included"] tpe consistency. … Fix included_data and data["included"] type consistency. Nov 7, 2019
{
"type": "people",
"id": "1",
"first_name": fake.first_name(),

Choose a reason for hiding this comment

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

These attributes should be inside the "attributes": {} key. I'm a tad surprised that this works.

Copy link
Author

Choose a reason for hiding this comment

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

Corrected in the last commit

@AdrianVandierAst
Copy link
Author

I will response here to #133 (comment)

Schema.included_data is consistently a dict for loading and dumping. And the "included" part of the json api object is consistently a list.
And yes, the type inconsistency is an issue only if you reuse the schema.

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

Successfully merging this pull request may close these issues.

2 participants