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

Support usage of 'source' in extra_kwargs. #4688

Merged
merged 1 commit into from Mar 13, 2017

Conversation

theosotr
Copy link
Contributor

@theosotr theosotr commented Nov 17, 2016

This commit fixes the issue when you set the keyword argument source
and your have not set the serializer fields explicitly. Then the
construction of field failed because there is not actually any model
field with that name.

However, you are still able to imply the name of model field by
taking advantage of the source keyword argument.

This commit fixes the issue when you set the keyword argument `source`
and your have not set the serializer fields explicitly. Then the
construction of field failed because there is not actually any model
field with that name.

However, you are still able to imply the name of model field by
providing the `source` keyword argument.
@xordoquy
Copy link
Collaborator

xordoquy commented Nov 17, 2016

Hi, I think a test case here would help understand what this change brings to the table.

@theosotr
Copy link
Contributor Author

theosotr commented Nov 17, 2016

Imagine the following scenario:

class MyModel(models.Model):
    model_field_name = models.CharField(max_length=255)
class MySerializer(serializers.ModelSerializer):
    class Meta(object):
        fields = ['api_field_name']
        extra_kwargs = {
            'api_field_name': {
                'source': 'model_field_name'
            }
        }
        model = models.MyModel

This code will break down with the following exception.

Traceback (most recent call last):
  File "/home/thodoris/projects/apimas/local/lib/python2.7/site-packages/django/core/handlers/base.py", line 149, in get_response
    response = self.process_exception_by_middleware(e, request)
  File "/home/thodoris/projects/apimas/local/lib/python2.7/site-packages/django/core/handlers/base.py", line 147, in get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/home/thodoris/projects/apimas/local/lib/python2.7/site-packages/django/views/decorators/csrf.py", line 58, in wrapped_view
    return view_func(*args, **kwargs)
  File "/home/thodoris/projects/apimas/local/lib/python2.7/site-packages/rest_framework/viewsets.py", line 87, in view
    return self.dispatch(request, *args, **kwargs)
  File "/home/thodoris/projects/apimas/local/lib/python2.7/site-packages/rest_framework/views.py", line 474, in dispatch
    response = self.handle_exception(exc)
  File "/home/thodoris/projects/apimas/local/lib/python2.7/site-packages/rest_framework/views.py", line 434, in handle_exception
    self.raise_uncaught_exception(exc)
  File "/home/thodoris/projects/apimas/local/lib/python2.7/site-packages/rest_framework/views.py", line 471, in dispatch
    response = handler(request, *args, **kwargs)
  File "/home/thodoris/projects/apimas/apimas/modeling/adapters/drf/mixins.py", line 116, in list
    serializer = self.get_serializer(queryset, many=True)
  File "/home/thodoris/projects/apimas/local/lib/python2.7/site-packages/rest_framework/generics.py", line 111, in get_serializer
    return serializer_class(*args, **kwargs)
  File "/home/thodoris/projects/apimas/local/lib/python2.7/site-packages/rest_framework/serializers.py", line 102, in __new__
    return cls.many_init(*args, **kwargs)
  File "/home/thodoris/projects/apimas/local/lib/python2.7/site-packages/rest_framework/serializers.py", line 123, in many_init
    child_serializer = cls(*args, **kwargs)
  File "/home/thodoris/projects/apimas/apimas/modeling/adapters/drf/serializers.py", line 24, in __init__
    serializer_fields = self.fields
  File "/home/thodoris/projects/apimas/local/lib/python2.7/site-packages/rest_framework/serializers.py", line 340, in fields
    for key, value in self.get_fields().items():
  File "/home/thodoris/projects/apimas/apimas/modeling/adapters/drf/serializers.py", line 100, in get_fields
    field_name, info, model, depth
  File "/home/thodoris/projects/apimas/local/lib/python2.7/site-packages/rest_framework/serializers.py", line 1104, in build_field
    return self.build_unknown_field(field_name, model_class)
  File "/home/thodoris/projects/apimas/local/lib/python2.7/site-packages/rest_framework/serializers.py", line 1211, in build_unknown_field
    (field_name, model_class.__name__)
ImproperlyConfigured: Field name `api_field_name` is not valid for model `MyModel`.

My commit fixes this issue so that I will no longer need to specify the api_field_name explicitly as follows:

class MySerializer(serializers.ModelSerializer):
    api_field_name = serializers.CharField(source='model_field_name')
    class Meta(object):
        fields = ['api_field_name']
        model = models.MyModel

gkorf pushed a commit to grnet/apimas that referenced this pull request Jan 16, 2017
This commit fixes the issue when you set the keyword argument `source`
and your have not set the serializer fields explicitly. Then the
construction of field failed because there is not actually any model
field with that name.

However, you are still able to imply the name of model field by
providing the `source` keyword argument at `Meta.extra_kwargs`.

For more information, see:
encode/django-rest-framework#4688
@theosotr
Copy link
Contributor Author

theosotr commented Mar 12, 2017

What about this?

@tomchristie tomchristie added this to the 3.6.3 Release milestone Mar 13, 2017
@tomchristie
Copy link
Member

tomchristie commented Mar 13, 2017

Yup, seems perfectly reasonable.

@tomchristie tomchristie merged commit 2df80c3 into encode:master Mar 13, 2017
3 checks passed
@tomchristie tomchristie changed the title Do not ignore source when building fields dynamically Support usage of 'source' in extra_kwargs. Mar 13, 2017
@sccovey
Copy link

sccovey commented Apr 22, 2019

This doesn't seem to work for following related fields - I cannot do

extra_kwargs = {
            'name': {'source': 'related_field.name'},
            --or--
            'name': {'source': 'related_field__name'},
}

Does this seem correct? In my testing, an explicit field attribute was required

@xordoquy
Copy link
Collaborator

xordoquy commented Apr 23, 2019

The ModelSerializer needs to generate the field from its model directly. It likely can not do so from related models.
Might be possible but I'm not sure the feature is worth the added complexity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants