Side loading support about ember-django-adapter HOT 13 OPEN

It feels more natural to me to support the EmbeddedRecordsMixin instead of adding support for side loading.

http://emberjs.com/api/data/classes/DS.EmbeddedRecordsMixin.html

DRF supports nested serializers (as you demonstrate in your example) so I think a user would just need to extend the RESTSerializer to add the EmbeddedRecordsMixin. We should actually consider adding the EmbeddedRecordsMixin to the RESTSerializer by default considering how easy it is to add nested or embedded records with DRF. I haven't looked into the details so I could be missing something. What do you think?

from ember-django-adapter.

EmbeddedRecordsMixin is already working well in EDA—but writable nested serializers are not supported by DRF. Side-loading would be a nice hybrid solution to have the writable pk field, but still get the embedded data in one request.

from ember-django-adapter.

Thanks for the insight. Ok yeah, I agree it's useful to have the side loading. Your proposal seems good to me.

from ember-django-adapter.

I am using embedded records and to support the writable pk field I use a custom field.

Edit: not sure if I have ever tried this with a hasMany relationship but it definitely works with a belongsTo.

from __future__ import unicode_literals
from django.core.exceptions import ObjectDoesNotExist
from rest_framework.relations import RelatedField


class EmbeddedPrimaryKeyRelatedField(RelatedField):
    default_error_messages = {
        'required': 'This field is required.',
        'does_not_exist': "Invalid pk '{pk_value}' - object does not exist.",
        'incorrect_type': 'Incorrect type. Expected pk value, received '
                          '{data_type}.',
    }

    def __init__(self, *args, **kwargs):
        self._serializer = kwargs.pop('serializer', None)()
        assert self._serializer is not None, \
            '`serializer` is a required argument.'
        super(EmbeddedPrimaryKeyRelatedField, self).__init__(*args, **kwargs)

    def to_internal_value(self, data):
        try:
            return self.get_queryset().get(pk=data)
        except ObjectDoesNotExist:
            self.fail('does_not_exist', pk_value=data)
        except (TypeError, ValueError):
            self.fail('incorrect_type', data_type=type(data).__name__)

    @property
    def fields(self):
        return self._serializer.fields

    def __iter__(self):
        for field in self._serializer:
            yield field

    def __getitem__(self, attr_name):
        return self._serializer[attr_name]

    def to_representation(self, data):
        return self._serializer.to_representation(data)

Then I can do this in my serializer:

class PersonSerializer(serializers.ModelSerializer):

    pets = EmbeddedPrimaryKeyRelatedField(many=True, queryset=Animal.objects.all(), 
                                                                        serializer=AnimalSerializer)

    class Meta:
        model = Person
        fields = ('name', 'pets')

In Ember you need to do this:

# /serializers/person.js
import ApplicationSerializer from './application';
import DS from 'ember-data';

export default ApplicationSerializer.extend(DS.EmbeddedRecordsMixin, {
  attrs: {
    pets: { serialize: 'id', deserialize: 'records' },
  }
});

from ember-django-adapter.

@g-cassie that's an interesting approach. So it's basically embedded records on read, pk on write?

from ember-django-adapter.

@dustinfarris that's correct. This approach is documented in the ED api docs as well so support should continue for it.

http://emberjs.com/api/data/classes/DS.EmbeddedRecordsMixin.html

from ember-django-adapter.

From the section on hasMany:

{ embedded: 'always' } is shorthand for: { serialize: 'records', deserialize: 'records' }

So you are just flipping the switch on serialize—I think that should work on hasMany as well if you were to do:

attrs: {
  pets: { serialize: 'ids', deserialize: 'records' }
}

Would it be easier to maintain documentation for two separate serializers for read/write instead of a custom field?

Like:

# serializers

class PersonSerializer(serializers.ModelSerializer):

    pets = AnimalSerializer(many=True, read_only=True)

    class Meta:
        model = Person


class WritablePersonSerializer(serializers.ModelSerializer):

    # PrimaryKeyRelatedField used by default for `pets`

    class Meta:
        model = Person


# viewsets

class PersonViewSet(viewsets.ModelViewSet):

    model = Person

    def get_serializer_class(self):
        if self.action in ['create', 'update']:
            return WritablePersonSerializer
        return PersonSerializer

Not saying this is any better, just wondering if there are any long-term pros/cons either way?

Does the embedded solution negate the need for side loading support altogether?

from ember-django-adapter.

Two serializers could work and your code example is simpler to document for sure. Personally I like that the custom field consolidates all the logic in serializers.py. If my custom field were available in the DRF core I think it would definitely be the way to go, but with no widely supported package available with my custom field your solution is probably easier to recommend.

I think with the implementation you are proposing you will not get the other benefits of side-loading - namely eliminating duplicate data in embedded payloads. ActiveModelSerializer and JSON API both do something like this:

data: [{<person 1 attrs>}, {<person 2 attrs}],
included: [{pet 1 attrs}, {pet 2 attrs}]

Without modifying DRF you will be stuck with something like this:

[
{<person 1 attrs>, included: [{pet 1 attrs}, {pet 2 attrs}]}, 
{<person 2 attrs>, included: [{pet 1 attrs}, {pet 2 attrs}]}, 
]

from ember-django-adapter.

@g-cassie: Good point, none of the solutions here address duplicate data in a ListView. To do that would require some hacking at the response/renderer level which is definitely out of scope for this project.

I think I am going to document the multiple serializer solution for now and forego formal side-loading support. Do you have a blog post on the custom field I can link to as an alternate solution?

from ember-django-adapter.

@dustinfarris No blog post and my bandwidth is limited right now. I threw it into a gist you can link to:
https://gist.github.com/g-cassie/8b4c62bf2da7d791bc36

I don't think I have ever tested this field with hasMany relationships but theoretically it should work.

from ember-django-adapter.

👍

from ember-django-adapter.

@g-cassie Thanks man! That works flawlessly for my needs at the moment 👍

from ember-django-adapter.

This duplicate issue - is it that I'm running into maybe?

I have an article with embedded events and send json like this
{title: 'Hello', events:[{start: '2014-01-01', end: '2014-02-02'}]}

and when I save this object my model has two events attached until i refresh the page. Any pointers as to how I should solve that? Calling .reload() on the model doesn't seem to help.

I can solve it by doing

        model.get('events').forEach(event => {
          if (!event.get('object_id')) {
            model.get('events').removeObject(event);
          }
        });

but that doesn't really seem optimal.

from ember-django-adapter.