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

Pick doesn't serialise ids correctly for nested model properties #325

Open
kyczawon opened this issue Feb 10, 2024 · 3 comments
Open

Pick doesn't serialise ids correctly for nested model properties #325

kyczawon opened this issue Feb 10, 2024 · 3 comments

Comments

@kyczawon
Copy link
Contributor

kyczawon commented Feb 10, 2024

I found an issue with serialisation where parent id overrides children ids during serialisation.

Example

Assume I have the following 2 models and a property in the parent which extract data from the child:

class ChildInfo(TypedDict):
    id: str
    description: str

class ParentModel(models.Model):
   @property
    def children_info(self) -> List[ChildInfo]:
        children = ChildModel.objects.filter(parent__pk=self.id)
        return [ChildInfo(id=child.id, description=child.description) for child in children]

class ChildModel(models.Model):
    parent = models.ForeignKey(
        to=Parent,
        verbose_name="linked [parent",
    )

    description = models.TextField()

Template:

@template
class View(NamedTuple):
    parent: Pick[models. ParentModel, "id", "children_info"]

Now assume I have ParentModel (id:1) linked to two ChildModels (id:20 & id: 25)

When I print children_info on django side I get the ids as expected:

[{'id': 20, 'description': 'test'}, {'id': 25, 'description': 'test2'}]

However, on client side the parent id overrides the children ids:

{
'id': 1,
'children_info': [{'id': 1, 'description': 'test'}, {'id': 1, 'description': 'test2'}]
}

Temporary Fix

Changing the definition of ChildInfo fixed the issue:

class ChildInfo(TypedDict):
    version_id: str
    description: str

And I correctly get the same output on both django and client side:

[{'version_id': 20, 'description': 'test'}, {'version_id': 25, 'description': 'test2'}]

Question

Is there an issue with serialisation when a name clash between ids happens?

@silviogutierrez
Copy link
Owner

That's nuts, I can't even imagine why it would happen. You're not going through the Pick layer for the ChildInfo. You're manually instantiating it. So I have no explanation.

Should be pretty easy to reproduce with a test in tests/serialization.py. See if you can take a stab then just run pytest.

@kyczawon
Copy link
Contributor Author

Realised that my design was probably flawed here. Instead I opted to use pick on the child and do the query on the view & this bug doesn't happen there:

@template
class View(NamedTuple):
    parent: Pick[models. ParentModel, "id", "children_info"]
    children: List[Pick[models. childModel, "id", "description"]]

And then doing the fetch within the view:

parent = get_object_or_404(
        models.ParentModel, org=request.user.current_org, id=prompt_id
    )
children = get_list_or_404(
        models.ChildModel, parent=parent
    )
return templates.View(
        parent=parent,
        children=children,
    ).render(request)

@silviogutierrez
Copy link
Owner

You can also just do the reverse relationship, far easier:

Add related_name="children" to the parent = models.ForeignKey declaration. See https://docs.djangoproject.com/en/5.0/ref/models/fields/#django.db.models.ForeignKey.related_name

Then just pick right at the parent level:

Pick[models. ParentModel, "id", "children.id", "children.description"]

And you only need to load the parent and pass it to the template. Magic!

PS. Technically you don't need a related name, just childmodel_set already exists on the parent model. But I don't like that name as much.

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

No branches or pull requests

2 participants