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

Automated related ForeignKey Models in ModelSchema #444

Open
21adrian1996 opened this issue May 11, 2022 · 7 comments
Open

Automated related ForeignKey Models in ModelSchema #444

21adrian1996 opened this issue May 11, 2022 · 7 comments

Comments

@21adrian1996
Copy link

Can related models be handled automatically?

Model

class ExampleModel(models.Model):
    relation = models.ForeignKey(SomeOtherModel, on_delete=models.PROTECT, related_name='example')

ModelSchema

class ExampleSchema(ModelSchema):
   class Config:
       model = models.ExampleModel
       model_fields = ['id', 'relation']

API

@api.post('/example')
def create_example(request, payload: schemas.ExampleSchema):
    pl = payload.dict()
    relation = get_object_or_404(SomeOtherModel, id=payload.relation)
    pl['relation'] = relation
    example = models.ExampleModel.objects.create(**pl)
    return {'id': example.id}

The above example works with the payload {'relation_id': 4} if the object with ID 4 exists.

Question 1
Is there any specific reason why the field has to be named relation_id in the payload and not just relation?

Question 2
In the API, is it really necessary to do

    pl = payload.dict()
    relation = get_object_or_404(SomeOtherModel, id=payload.relation)
    pl['relation'] = relation

or can that be somehow done automatically?
It seems that writing the get_object_or_404() is currently needed, or can that be omitted with an other apporach?

@vitalik
Copy link
Owner

vitalik commented May 11, 2022

@21adrian1996

well the philosophy for incoming payloads are only to validate types/structure

but the database calls should happen inside the controller function

Maybe try doing this (relation_id instead of relation):

class ExampleSchema(ModelSchema):
   class Config:
       model = models.ExampleModel
       model_fields = ['id', 'relation_id'] # !!!

...
ExampleModel.objects.create(**pl) # I think this should validate relation_id automatically ?

@ghost
Copy link

ghost commented May 20, 2022

Hi @21adrian1996 and @vitalik

@21adrian1996

well the philosophy for incoming payloads are only to validate types/structure

but the database calls should happen inside the controller function

Maybe try doing this (relation_id instead of relation):

class ExampleSchema(ModelSchema):
   class Config:
       model = models.ExampleModel
       model_fields = ['id', 'relation_id'] # !!!

...
ExampleModel.objects.create(**pl) # I think this should validate relation_id automatically ?

This will yield a ninja.errors.ConfigError: Field(s) {'my_field'} are not in model.
I'm having the same "issue". I have to work a lot with foreign key relationships and as far as I see from the documentation I have to solve it for every field manually.

I found one workaround to check if the field is an FK-field via ìs_relation. It feels dirty because of how I hack my way to the FK-model (so if any one of you knows a better way please comment):

class MyModelSchema(ModelSchema):
  class Config:
    model = MyModel
    model_exclude = [ ... ] # lots of exclusions here for safety reasons


@router.post("add")
def add(request, payload: MyModelSchema):
  my_instance = MyModel()
    for k,v in payload.dict().items():
      if my_instance._meta.get_field(k).is_relation:
        target_model = my_instance._meta.get_field(k).target_field.model
        v = target_model.objects.get(pk=v)
      my_instance.__setattr__(k,v)
  my_instance.save()

Do you think this would be a good enhancement? I like the way Django handles database tables in the backend but appending/using _id in some places makes working with APIs cumbersome (which is of course not django-ninjas fault, I really like this project!).

@vitalik
Copy link
Owner

vitalik commented Jun 7, 2022

@fantasticle

....
This will yield a ninja.errors.ConfigError: Field(s) {'my_field'} are not in model.

I think it should not - can you give your example ?

Maybe workaround for you case would be to set FK ids as attributes:

class MyModelSchema(ModelSchema):
  some_relation_id: int
  other_relation_id: int
  class Config:
    model = MyModel
    fields = ['some_char_field']
...

@api.post(...)
def foo(request, payload: MyModelSchema)
      obj = MyModel(**payload.dict())
      obj.save()

@Bouni
Copy link

Bouni commented Jul 7, 2022

I had the same problem and your workaround using attributes works perfectly fine!

@chrisbodon
Copy link

chrisbodon commented Aug 19, 2022

I´m using that workaround but it gets this when I try to post:

ValueError: Cannot assign "18": "Activity.town" must be a "Town" instance.

My code:

URLS.PY

@api.post("/activities", response=ActivitySchema)
def create_activities(request, data: ActivitySchema):
	obj = Activity.objects.create(**data.dict())
	return obj

The data Im trying to post

{icon_id: '6', name: 'mn bnb', highlight: false, town_id: 18}

SCHEMAS.PY

class ActivitySchema(ModelSchema):
	town_id: int = None
	icon_id: int = None
	
	class Config:
		model = Activity
		model_fields = "__all__"

MODELS.PY

class Activity(models.Model):
	town = models.ForeignKey(Town, on_delete=models.CASCADE, null=False)
	icon = models.ForeignKey(MediaIcon, on_delete=models.CASCADE, null=False)
	name = models.CharField(max_length=255, null=False)
	highlight = models.BooleanField(default=False)
	
	def __str__(self):
		return self.name

@vlada1g
Copy link

vlada1g commented Dec 1, 2022

I have the same problem as you.
You should put instead of model_fields = "__all__" this line model_exclude = ['town', 'icon']

@cca32
Copy link

cca32 commented Dec 31, 2022

@fantasticle

....
This will yield a ninja.errors.ConfigError: Field(s) {'my_field'} are not in model.

I think it should not - can you give your example ?

Maybe workaround for you case would be to set FK ids as attributes:

class MyModelSchema(ModelSchema):
  some_relation_id: int
  other_relation_id: int
  class Config:
    model = MyModel
    fields = ['some_char_field']
...

@api.post(...)
def foo(request, payload: MyModelSchema)
      obj = MyModel(**payload.dict())
      obj.save()

I also tried the first approach to receive the same error Field(s) {'department_id'} are not in model <class 'my_ninja.models.Employee'>

the second option with attributes worked for me too!
wondering if it'd be worth it to have an option in class Config to auto-handle all Foreign Keys

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

6 participants