Skip to content

Conversation

@pmdevita
Copy link
Owner

@pmdevita pmdevita commented Jan 5, 2025

Currently for ModelSchema, Ninja uses Pydantic aliases to alias between a ForeignKey field's property name as defined by the user, and the attribute name which holds the ID referenced by the foreign key (author vs author_id). However, this prevents users from using Pydantic's alias_generator with ForeignKey field names.

This PR removes the alias and then changes the property name on the Pydantic model to match the attribute name on the Django model (author_id), which should allow for data dumped out with my_schema.model_dump() to still correctly match up to Django fields as it does now. It then adds property fields to alias between the attribute name and the property name, so any accesses on the model, like my_schema.author, will get aliased to the real property name, my_schema.author_id.

@pmdevita pmdevita self-assigned this Jan 5, 2025
@pmdevita pmdevita force-pushed the bug/foreign-key-alias branch from bf1b749 to 83ec9d4 Compare January 5, 2025 19:51
@pmdevita
Copy link
Owner Author

pmdevita commented Feb 9, 2025

Some extra context I wrote up since it's been a while and I don't remember this all to well anymore.

Sort of an example just with Pydantic, the old way would look like this

class MySchema(ModelSchema):
    author: int = Field(..., alias="author_id")  # generated by ModelSchema
    
    class Meta:
        model = Book
        fields = ["author"]

which would of course then cause a problem with use of automatic alias generation. My implementation did this

class MySchema(ModelSchema):
    author_id: int = Field(...)  # generated by ModelSchema
    
    @property                    # generated by ModelSchema
    def author(self):
        return author

    @author.setter               # generated by ModelSchema
    def author(self, value):
        self.author = value
    
    class Meta:
        model = Book
        fields = ["author"]

which would then free up alias to be changed.

In the time since I've written this, I did run into a similar issue to what vitalik states here vitalik/django-ninja#1111 (comment). The trouble is, *_id has to be the field's real name in order for that to be used in alias generation. But additionally as I stated in my follow up comment, and as I found out, you were already going to be in trouble if you were trying to do that, even without this patch being merged.

There is a very similar problem that I realized with this patch right now though, which is what happens if you do this.

class MySchema(ModelSchema):
    author: AuthorSchema   # my manually added field

    author_id: int = Field(...)  # generated by ModelSchema
    
    # @property                    # generated by ModelSchema?
    # def author(self):
    #     return author

    # @author.setter               # generated by ModelSchema?
    # def author(self, value):
    #     self.author = value
    
    class Meta:
        model = Book
        fields = ["author"]

We can fix this by making sure the property are not added so they don't override Author. I'll update this PR to fix that and add a test.

@pmdevita pmdevita merged commit 3973f7e into master Feb 9, 2025
74 checks passed
@pmdevita pmdevita deleted the bug/foreign-key-alias branch February 9, 2025 19:02
@pmdevita pmdevita restored the bug/foreign-key-alias branch February 9, 2025 19:02
@pmdevita pmdevita deleted the bug/foreign-key-alias branch February 9, 2025 19:08
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.

2 participants