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

Use recursive method to correctly handle one_to_many fields #45

Conversation

pedro-prose
Copy link

@pedro-prose pedro-prose commented Jan 31, 2022

Given:

class LabelSchema(ModelSchema):
    class Config:
        model = OrderItemLabel
        include = ['value']

class OrderItemSchema(ModelSchema):
    label: LabelSchema | None
    quantity: int

    class Config:
        model = OrderItem
        include = ['label', 'quantity']

class CustomerSchema(ModelSchema):
    class Config:
        model = Customer
        include = ['username']

class OrderSchema(ModelSchema):
    items: list[OrderItemSchema]
    customer: CustomerSchema
    
    class Config:
        model = Order
        include = ['items', 'customer']

When calling:

OrderSchema.from_django(order).dict()

Will return:

 {
    'items': [
        {'quantity': 1, 'label': None},
        {'quantity': 1, 'label': None},
        {'quantity': 1, 'label': 1}
    ],
    'customer': {'username': 'exemple@gmail.com'}
}

Note that label = 1, the ID and not the value (str) expected from LabelSchema.

With this PR change, the new output is:

{
    'items': [
        {'quantity': 1, 'label': None},
        {'quantity': 1, 'label': None},
        {'quantity': 1, 'label': {'value': 'Product 10'}}
    ],
     'customer': {'username': 'c@i.com'}}

@pedro-prose pedro-prose marked this pull request as draft January 31, 2022 18:23
@pedro-prose pedro-prose marked this pull request as ready for review January 31, 2022 18:53
@jordaneremieff
Copy link
Owner

Thanks @pedro-prose! I'll give this a closer look later in the week. In the meantime, would you be able to write a test for this?

@pedro-prose pedro-prose closed this Feb 6, 2022
@jordaneremieff
Copy link
Owner

Hi @pedro-prose, what was the reason for closing this? I had not had a chance to look into it yet, but was planning to today or tomorrow.

@phbernardes
Copy link
Contributor

Hi @jordaneremieff, there were some performance problems with this proposal. I'll do some checks and propose something better.

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

3 participants