django-ninja icon indicating copy to clipboard operation
django-ninja copied to clipboard

[BUG] ForeignKey field in modelSchema do not use the related alias_generator

Open stvdrsch opened this issue 1 year ago • 3 comments

Describe the bug When using an alias_generator in the config of a modelSchema the id's returned for ForeignKey Fields do not use that generator

Versions (please complete the following information):

  • Python version: [ 3.11.4]
  • Django version: [4.1.5]
  • Django-Ninja version: [0.20.0]
  • Pydantic version: [1.10.4]

I have this Dealer model

class Dealer(AbstractDisableModel, AddressBase, ContactInfoBase):
    ...
    distributor = models.ForeignKey(
        Distributor, on_delete=models.DO_NOTHING, related_name="dealers"
    )
    ...

Which adheres to this modelschema

class DealerSchema(ModelSchema):
    ...

    class Config(CamelModelSchema.Config):
        ...

which uses this schema that converts properties to camelcase

class CamelModelSchema(Schema):
    class Config:
        alias_generator = to_camel
        allow_population_by_field_name = True

All of this works for the fields directly attached to the instances, but foreignkey fields (that end in _id) don't seem to be converted. Would it be possible to have the generator adjust ALL fields?

E.g. this is the schema of the response of a dealer instance

image

distributor_id is still not camelcased although the rest of the keys are. The distributor_id field is not sent through the alias_generator. Applying the same alias generator to the modelschema of the distributor also doesn't fix the issue.

I would guess this is a bug with the framework? Or potentially it is expected to behave like this...

stvdrsch avatar Aug 18 '23 09:08 stvdrsch

Hi @stvdrsch

See #811 - you need to add by_alias=True :

@api.get('/dealers', response=list[DealerSchema], by_alias=True)
def dealers(request):
    return Dealer.objects.all()

to make this behaviour by default you can extend the Router and add by_alias as default:

from ninja import Router


class MyRouter(Router):
    def add_api_operation(self, *a, **kw):
           kw['by_alias'] = True
           return super().add_api_operation(*a, **kw)

...

router = MyRouter()

@router.get('/dealers', response=list[DealerSchema])
def dealers(request):
    return Dealer.objects.all()

vitalik avatar Aug 18 '23 10:08 vitalik

Yes, that was added and the by_alias works for every other field. But the ForeignKey fields, and any other fields that use related models I would guess, still just outputs ..._id

In the response in the initial question fields like countryCode and postalCode are defined like country_code and postal_code in the original Django model and get correctly aliased. It's just the foreignkey id field that doesn't seem to pass through the alias_generator.

stvdrsch avatar Aug 18 '23 10:08 stvdrsch

I think I have a solution for this. Let's say we have a django model and ninja schema like so

class Book(models.Model):
    id = models.AutoField(primary_key=True)
    full_name = models.CharField(max_length=100)
    author = models.ForeignKey(Author, on_delete=models.CASCADE)

class BookSchema(ModelSchema):
    model_config = dict(alias_generator=to_camel, populate_by_name=True)
    class Meta:
        model = Book
        fields = ["id", "full_name", "author"]

The ModelSchema metaclass factory currently creates fields for ForeignKeys with aliases that correspond with their Django field attribute name (author here gets an author_id alias). This is the reason why Pydantic fields mapping foreign keys need the alias at the moment and why there is a conflict with the alias_generator.

Instead, we could set the property names on the Pydantic model to match these field attribute names, so BookSchema.author would become BookSchema.author_id, and stop setting the alias on that field. Then, it will be aliased for the API if you do .model_dump(by_alias=True) and properly named for Django if you do .model_dump(by_alias=False).

This causes a new problem where users can no longer access the property by the field name as they would expect since it has been renamed (BookSchema.author is now at BookSchema.author_id instead). To solve this, we can additionally generate property functions to access through the expected name.

The final generated class would be equivalent to:

 class BookSchema(Schema):
    model_config = dict(alias_generator=to_camel, populate_by_name=True)
   
    id: int = Field(...)
    full_name: str = Field(...)  # generated alias "fullName"
    author_id: int = Field(...)  # generated alias "authorId"
    
    @property
    def author(self):
        return self.author_id

    @author.setter
    def author(self, value):
        self.author_id = value

How does this sound @vitalik ? If you're OK with this solution I can write a PR for it.

pmdevita avatar Mar 21 '24 19:03 pmdevita