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

Unable to use "<field>_id" for FK fields with ModelSchema [BUG]

Open ihelmer07 opened this issue 2 years ago • 6 comments

Bug 1 In an ideal case - one should be able to set the FK of a field directly without a separate query as the Django docs highlights in the following link: https://docs.djangoproject.com/en/4.0/topics/db/optimization/#use-foreign-key-values-directly

However using ModelSchema the field gets validated to the FK model instance and the 'foo_id' shortcut can't be used.

Bug 2 Additionally if one does use "foo_id" in payload (POST or PUT or PATCH) it gets resolved to "foo".

For example:

class City(models.Model):
    """Model with simple info."""

    city = models.CharField(_("City"), max_length=3)

    def __str__(self):
        """Str Method override."""
        return self.city


class Order(models.Model):
    """Model with multiple many to many and fk relations."""

    city = models.ForeignKey(City, verbose_name=_("Name"), on_delete=models.CASCADE)

    def __str__(self):
        """Str Method override."""
        return f"Order {self.pk} for {self.city.name}"

class OrderPostSchema(ModelSchema):
    """Order Post Schema"""

    class Config:
        """Config."""

        model = Order
        model_exclude = ["id"]


@api.post("/order")
def create_order(request, payload: OrderPostSchema):
    """Create Order."""
    data = payload.dict()
    obj = Order.objects.create(**data) 
    return {"id": obj.id}

# POSTing to "/order" with payload = {"city_id": 1}
# Throws error: ValueError: Cannot assign "1": "Order.city" must be a "City" instance.

The only workaround (while using ModelSchema):

@api.post("/order")
def create_order(request, payload: OrderPostSchema):
    """Create Order."""
    data = payload.dict()
    city_id = data.pop('city') #<--- notice not city_id as expected from payload
    data["city"] = City.objects.get(pk=city) <--- causes another DB Query
    obj = Order.objects.create(**data) 
    return {"id": obj.id}

I am trying to convert a huge codebase (30+ apps) from DRF and Django Ninja is the best option by far, however being unable to use the ModelSchema with "all" or standard "POST" logic makes it a hard sell given one can use DRF to post the same payload without issue.

Expected Behavior Be able to use "field_id" in ModelSchema.

Versions (please complete the following information):

  • Python version: 3.9
  • Django version: 4.0.4
  • Django-Ninja version: 0.19.1

ihelmer07 avatar Jul 29 '22 01:07 ihelmer07

i think this may helps you

#add manager to modle file
class OrderManager(models.Manager):
    def add_city_to_order(self, city_id):
        return self.create(city=City.objects.get(pk=city_id))

# and change api endpoint to below
@api.post("/order")
def create_order(request, payload: OrderPostSchema):
    """Create Order."""
    data = payload.dict()
    obj = Order.objects.add_city_to_order(data["city"])
    return {"id": obj.id}


alirezapla avatar Aug 14 '22 09:08 alirezapla

It's pretty weird that this is not done automatically by django-ninja... is there no other way without hacking it?

martinlombana avatar Aug 19 '22 19:08 martinlombana

if more than one foreignkey? i feel this solution is not very well.

sunboy123 avatar Sep 22 '22 23:09 sunboy123

错误 1​​在理想情况下 - 应该能够直接设置字段的 FK 而无需单独的查询,因为 Django 文档在以下链接中突出显示:https ://docs.djangoproject.com/en/4.0/topics/db /优化/#use-foreign-key-values-directly

但是,使用 ModelSchema,该字段将被验证到 FK 模型实例,并且不能使用“foo_id”快捷方式。

错误 2 此外,如果确实在有效负载(POST 或 PUT 或 PATCH)中使用了“foo_id”,它会被解析为“foo”。

例如:

class City(models.Model):
    """Model with simple info."""

    city = models.CharField(_("City"), max_length=3)

    def __str__(self):
        """Str Method override."""
        return self.city


class Order(models.Model):
    """Model with multiple many to many and fk relations."""

    city = models.ForeignKey(City, verbose_name=_("Name"), on_delete=models.CASCADE)

    def __str__(self):
        """Str Method override."""
        return f"Order {self.pk} for {self.city.name}"

class OrderPostSchema(ModelSchema):
    """Order Post Schema"""

    class Config:
        """Config."""

        model = Order
        model_exclude = ["id"]


@api.post("/order")
def create_order(request, payload: OrderPostSchema):
    """Create Order."""
    data = payload.dict()
    obj = Order.objects.create(**data) 
    return {"id": obj.id}

# POSTing to "/order" with payload = {"city_id": 1}
# Throws error: ValueError: Cannot assign "1": "Order.city" must be a "City" instance.

唯一的解决方法(使用 ModelSchema 时):

@api.post("/order")
def create_order(request, payload: OrderPostSchema):
    """Create Order."""
    data = payload.dict()
    city_id = data.pop('city') #<--- notice not city_id as expected from payload
    data["city"] = City.objects.get(pk=city) <--- causes another DB Query
    obj = Order.objects.create(**data) 
    return {"id": obj.id}

我正在尝试从 DRF 转换一个庞大的代码库(30 多个应用程序),而 Django Ninja 是迄今为止最好的选择,但是由于无法将 ModelSchema 与“ all ”或标准“POST”逻辑一起使用,因此很难卖使用 DRF 毫无问题地发布相同的有效负载。

预期行为 能够在 ModelSchema 中使用“field_id”。

版本(请填写以下信息):

  • Python版本:3.9
  • Django 版本:4.0.4
  • Django-Ninja 版本:0.19.1

i have the same problem! is there other more elegant solution?

sunboy123 avatar Sep 22 '22 23:09 sunboy123

@sunboy123 @martinlombana @alirezapla

if more than one foreignkey? i feel this solution is not very well.

I guess the easiest solution for now is to manually write ids in schema:

class OrderPostSchema(ModelSchema):
    city_id: int  # ! 
    class Config:
        model = Order
        model_exclude = ["id", "city"]  # ! 

but I'm open to any other suggestions to how to improve this... maybe some connfig flag

    class Config:
        model = Order
        model_fk_use_pks = True  # (or model_fk_use_pks = ['city', 'otherfk']

vitalik avatar Sep 23 '22 09:09 vitalik

IMO such weird mapping of dropping _id should not happen at all - super confusing. Working with dynamic schemas is supper difficult, because adding such overwrites defeats the whole purpose of dynamic schema. So provided workarounds are not much help here :( Are there any plans to solve this? It's been almost 2 years since this got reported.

rkulinski avatar Mar 26 '24 16:03 rkulinski