Skip to content
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

Simplify from_django by relying in Pydantic. Add multiple level relationship support. #52

Merged
merged 2 commits into from Mar 26, 2022

Conversation

phbernardes
Copy link
Contributor

Hi,

I open this PR/discussion to:

  • Add support for multiple level relationships (user -> user.orders -> user.orders.order_items -> user.orders.order_items.order_items_details ...)
  • Add support for double underscore aliases (first_name: str = Field(alias="user__first_name")). We can replace it by dot aliases if you prefer (first_name: str = Field(alias="user.first_name"))
  • Simplify from_django logic by relying always that possible in Pydantic, this will increase Djantic performance (reducing serialization time).
  • Fix ManyToMany relations serialization: ManyToMany relations should access related field by related_name or by *_set and should return a list of instances (from a QuerySet) instead of a instance.
  • Harmonize ID/PK usage: instead of returning pk for some fields and id for other, return id for all.
  • Calling XSchema.from_django(xs, many=True) should return a list of XSchema instances instead of a list of dicts.
  • Include GenericForeignKey in schemas.

Merging this request will cause some breaking changes:

  • ManyToMany relations will return a list of instances instead of a instance. The attribute will be called *_set or the Django's field related_name.
  • ForeignObjectRel will return a list of dicts with pk instead of id: [{'id': 1}] instead of [{'pk': 1}].
  • XSchema.from_django(xs, many=True) will return a list of XSchema instances instead of a list of dicts.

These changes can be seem in test_relations.py and test_queries.py.

I din't separated this PR in several commits as these changes are linked, but I can separate it if it's hard to understand.

Thank you.

@jordaneremieff
Copy link
Owner

Thanks @phbernardes! I will give this a look on the weekend.

@jordaneremieff
Copy link
Owner

@phbernardes this looks great, and I think it's okay to merge. Could you add some documentation to demonstrate the usage of the multiple level relationships? Even just a README example would do.

@phbernardes
Copy link
Contributor Author

@phbernardes this looks great, and I think it's okay to merge. Could you add some documentation to demonstrate the usage of the multiple level relationships? Even just a README example would do.

Of course, please see the example I added in README in 943c1bd
(I'll squash this fixup before merging)

@jordaneremieff jordaneremieff merged commit 0234275 into jordaneremieff:main Mar 26, 2022
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.

None yet

2 participants