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

Problem when creating/updating ManyToOne nested field if a serializer is using fields = '__all__' #254

Closed
farasu opened this issue Aug 3, 2021 · 12 comments
Labels
bug Something isn't working

Comments

@farasu
Copy link

farasu commented Aug 3, 2021

when try to create nested record; show this error Field name _is not valid for modelModelName.

@yezyilomo
Copy link
Owner

Let’s see a serializer for the model which gives error.

@farasu
Copy link
Author

farasu commented Aug 4, 2021

class AddressSerializer(ModelSerializer):
     class Meta:
        model = Address
        fields = '__all__'
class EducationSerializer(ModelSerializer):
    class Meta:
        model = Education
        fields = '__all__'
class ExprienceSerializer(ModelSerializer):
    class Meta:
        model = Exprience
        fields = '__all__'
class EmployeeSerializer(NestedModelSerializer):
    address = NestedField(AddressSerializer, many=True)
    education = NestedField(EducationSerializer, many=True)
    exprience =  NestedField(ExprienceSerializer, many=True)

    class Meta:
            model = Employee
            fields = '__all__'
Shows the error : Field name `_` is not valid for model `Address`.

@yezyilomo
Copy link
Owner

All serializers seems okay, now can I see json data which you're sending when creating Employee.

@farasu
Copy link
Author

farasu commented Aug 4, 2021

{
        "address": {
            "create":[
            {
                "current": true,
                "province": "Kabul",
                "district": "5 District",
                "area": "Gullae"
            },
            {
                "current": true,
                "province": "Kabul",
                "district": "5 District",
                "area": "Gullae"
            }
        ]
        },
        "education":{
            "create":  [
            {
                "education_level": "master",
                "education_field": "Computer Scince",
                "university": "Rana",
                "start_date": "2021-08-03",
                "end_date": "2021-08-03"
            },
            {
                "education_level": "master",
                "education_field": "Computer Scince",
                "university": "Rana",
                "start_date": "2021-08-03",
                "end_date": "2021-08-03"
            }
        ]
        },
        "exprience": {
            "create": [
            {
                "position": "IT Manager",
                "sallary": 2000,
                "id_card": "IT-97867",
                "start_date": "2021-08-03",
                "end_date": "2021-08-03"
            }
        ]
        },
        "name": "Farasu",
        "lname": "farasu",
        "fname": "farasu",
        "age": "2021-08-03",
        "gender": "male",
        "marital_state": "single",
        "phone": "0770000000",
        "email": "[email protected]",
        "identity_no": "1000-230-4567",
        "employee_type": "doctor",
        "status": true
    }

`

@sommelon
Copy link

sommelon commented Aug 4, 2021

I looked into it and I found that in ModelSerializer on line 1041 the value of field_names is ['_', '_', 'a', 'l', 'l', '_', '_']. So the fields='__all__' string gets converted to a list somewhere in the code.

@yezyilomo
Copy link
Owner

yezyilomo commented Aug 4, 2021

{
        "address": {
            "create":[
            {
                "current": true,
                "province": "Kabul",
                "district": "5 District",
                "area": "Gullae"
            },
            {
                "current": true,
                "province": "Kabul",
                "district": "5 District",
                "area": "Gullae"
            }
        ]
        },
        "education":{
            "create":  [
            {
                "education_level": "master",
                "education_field": "Computer Scince",
                "university": "Rana",
                "start_date": "2021-08-03",
                "end_date": "2021-08-03"
            },
            {
                "education_level": "master",
                "education_field": "Computer Scince",
                "university": "Rana",
                "start_date": "2021-08-03",
                "end_date": "2021-08-03"
            }
        ]
        },
        "exprience": {
            "create": [
            {
                "position": "IT Manager",
                "sallary": 2000,
                "id_card": "IT-97867",
                "start_date": "2021-08-03",
                "end_date": "2021-08-03"
            }
        ]
        },
        "name": "Farasu",
        "lname": "farasu",
        "fname": "farasu",
        "age": "2021-08-03",
        "gender": "male",
        "marital_state": "single",
        "phone": "0770000000",
        "email": "[email protected]",
        "identity_no": "1000-230-4567",
        "employee_type": "doctor",
        "status": true
    }

`

This looks okay too, but am still not able to reproduce it, maybe it’s a version specific bug, I’ve tried to use __all__ in fields but still it’s running just fine.

@yezyilomo
Copy link
Owner

I looked into it and I found that in ModelSerializer on line 1041 the value of field_names is ['_', '_', 'a', 'l', 'l', '_', '_']. So the fields='__all__' string gets converted to a list somewhere in the code.

This could probably be the cause, have you managed to reproduce it?.

@sommelon
Copy link

sommelon commented Aug 5, 2021

Yes, but I'm not exactly sure why it's happening. I've tried reproducing this issue and I've got this error. Here is the traceback:

    File "e:\Documents\Work\blablabla\backend\.venv\lib\site-packages\rest_framework\generics.py", line 242, in post
    return self.create(request, *args, **kwargs)
  File "e:\Documents\Work\blablabla\backend\.venv\lib\site-packages\rest_framework\mixins.py", line 18, in create
    serializer.is_valid(raise_exception=True)
  File "e:\Documents\Work\blablabla\backend\.venv\lib\site-packages\rest_framework\serializers.py", line 220, in is_valid
    self._validated_data = self.run_validation(self.initial_data)
  File "e:\Documents\Work\blablabla\backend\.venv\lib\site-packages\rest_framework\serializers.py", line 419, in run_validation
    value = self.to_internal_value(data)
  File "e:\Documents\Work\blablabla\backend\.venv\lib\site-packages\django_restql\mixins.py", line 546, in to_internal_value
    validated_data = super().to_internal_value(data)
  File "e:\Documents\Work\blablabla\backend\.venv\lib\site-packages\rest_framework\serializers.py", line 476, in to_internal_value
    validated_value = field.run_validation(primitive_value)
  File "e:\Documents\Work\blablabla\backend\.venv\lib\site-packages\rest_framework\serializers.py", line 607, in run_validation
    value = self.to_internal_value(data)
  File "e:\Documents\Work\blablabla\backend\.venv\lib\site-packages\django_restql\fields.py", line 215, in to_internal_value
    return self.data_for_create(data)
  File "e:\Documents\Work\blablabla\backend\.venv\lib\site-packages\django_restql\fields.py", line 171, in data_for_create
    validation_methods[operation](values)
  File "e:\Documents\Work\blablabla\backend\.venv\lib\site-packages\django_restql\fields.py", line 138, in validate_create_list
    return self.validate_data_list(data)
  File "e:\Documents\Work\blablabla\backend\.venv\lib\site-packages\django_restql\fields.py", line 120, in validate_data_list
    parent_serializer.is_valid(raise_exception=True)
  File "e:\Documents\Work\blablabla\backend\.venv\lib\site-packages\rest_framework\serializers.py", line 725, in is_valid
    self._validated_data = self.run_validation(self.initial_data)
  File "e:\Documents\Work\blablabla\backend\.venv\lib\site-packages\rest_framework\serializers.py", line 607, in run_validation
    value = self.to_internal_value(data)
  File "e:\Documents\Work\blablabla\backend\.venv\lib\site-packages\rest_framework\serializers.py", line 643, in to_internal_value
    validated = self.child.run_validation(item)
  File "e:\Documents\Work\blablabla\backend\.venv\lib\site-packages\rest_framework\serializers.py", line 419, in run_validation
    value = self.to_internal_value(data)
  File "e:\Documents\Work\blablabla\backend\.venv\lib\site-packages\rest_framework\serializers.py", line 472, in to_internal_value
    for field in fields:
  File "e:\Documents\Work\blablabla\backend\.venv\lib\site-packages\rest_framework\serializers.py", line 355, in _writable_fields
    for field in self.fields.values():
  File "e:\Documents\Work\blablabla\backend\.venv\lib\site-packages\django\utils\functional.py", line 80, in __get__
    res = instance.__dict__[self.name] = self.func(instance)
  File "e:\Documents\Work\blablabla\backend\.venv\lib\site-packages\rest_framework\serializers.py", line 349, in fields
    for key, value in self.get_fields().items():
  File "e:\Documents\Work\blablabla\backend\.venv\lib\site-packages\rest_framework\serializers.py", line 1054, in get_fields
    source, info, model, depth
  File "e:\Documents\Work\blablabla\backend\.venv\lib\site-packages\rest_framework\serializers.py", line 1199, in build_field
    return self.build_unknown_field(field_name, model_class)
  File "e:\Documents\Work\blablabla\backend\.venv\lib\site-packages\rest_framework\serializers.py", line 1319, in build_unknown_field
    (field_name, model_class.__name__)
django.core.exceptions.ImproperlyConfigured: Field name `_` is not valid for model `Milestone`.

Serializers

class MilestoneASerializer(serializers.ModelSerializer):
    class Meta:
        model = Milestone
        fields = '__all__'

class ProjectSerializer(DynamicFieldsMixin, NestedModelSerializer):
    milestone_set = NestedField(MilestoneASerializer, many=True, required=False, allow_null=True)

    class Meta:
        model = Project
        fields = '__all__'

Request

POST 'api/projects/
{
  "name": "string",
  "identifier": "striasdngaa",
  "milestone_set": {"create":[{"name":"cdsacc", "project":1}]}
}

Versions

Django==2.2.24
django-restql==0.11.2
djangorestframework==3.12.2

@yezyilomo
Copy link
Owner

Thank you @sommelon for posting the whole error trace back, I think the part causing it is

fields = filter(contain_field, serializer_class.Meta.fields)
original_fields = copy.copy(serializer_class.Meta.fields)
serializer_class.Meta.fields = list(fields)

We forgot to handle a case where the value of serializer_class.Meta.fields is __all__, so it’s definitely a bug.

@yezyilomo yezyilomo added the bug Something isn't working label Aug 5, 2021
@sommelon
Copy link

sommelon commented Aug 5, 2021

No problem.
I think some Issue templates with guidelines would be great so that people know what information to provide.

@yezyilomo
Copy link
Owner

No problem.
I think some Issue templates with guidelines would be great so that people know what information to provide.

Yeah, I’ll work on it, if you have an idea of how issue template should look like for this project feel free to open an issue about it, I’ll appreciate the help.

@sommelon
Copy link

sommelon commented Aug 5, 2021

@yezyilomo I've created the issue #256

@yezyilomo yezyilomo changed the title Nested Create Problem Problem when creating/updating ManyToOne nested field if a serializer is using fields = '__all__' Aug 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants