Support non-database fields

[dgaus]

Hi, I have a model which has some nested relations, something like:

class Document(models.Model):
    node_positions = models.ManyToMany('NodePosition')

    def get_nodes(self):
        return Node.objects.filter(node_position__document=self)

class NodePosition(models.Model):
    node = models.ForeignKey('Node')

class Node(models.Model):
    pass

and I would like, when fetching a document, to sideload both NodePositions and Nodes. However, due to the way the SideloadSerializer is implemented, it depends on django-reported relations, which don't include Nodes. So if my DocumentBaseSerializer does

class DocumentBaseSerializer(serializers.ModelSerializer):
    nodes = serializers.ReadOnlyField(source='get_nodes')

I get the node IDs but I am not able to sideload the nodes. I am thinking of overriding _configure_sideloads to try and support this use case as well, but I'm wondering if you have tried this before or have any input on the matter. Thanks!

[g-cassie]

Could you use a ManyToMany field with through=NodePosition instead of get_nodes()? I might not be understanding your data structure though.

https://docs.djangoproject.com/en/1.8/topics/db/models/#intermediary-manytomany

Otherwise happy to accept a PR that implements this provided it does not hamper the DB performance for standard functions.

[dgaus]

Thanks for the input, I haven't tried using a ManyToMany with through, which does work.
But to complicate matters further, my Node model is polymorphic (using inheritance), and the SideloadSerializer seems to use the model on the Meta class as the key for the output. I am using django-model-utils' InheritanceManager, so if I do select_subclasses() I get a queryset containing all the descendants (instances of NodeA, NodeB, etc). I have been serializing these using object.class.name as the key. I know this is a bit of a fringe case, but I'd love to get this working out of the box. I'm thinking of replacing self.base_key for a callable that takes the model instance, and to_representation to call this for each instance on lists. That shouldn't trigger any extra DB queries, and the performance penalty should be minor. Would you agree with this functionality, or do you foresee any problems?
There could also be a config option on the serializer to activate this, I think it would be useful given that ember supports polymorphic relations and these map quite nicely to django. Cheers.

[g-cassie]

So if you have an endpoint like /fruits/ you would return a queryset like Fruit.objects.all() but your response looks something like this:

{
  'apples': [...],
  'bananas': [...],
  'oranges': [...]
}

Is this correct?

[g-cassie]

Ok I looked in more detail at the code and understand the problem in your original message. I agree the current code does not support this. The only issue is I don't see how you can do what you want to do without adding another field to the sideload configuration tuple - serializer_field. Perhaps it is possible to call the serializer's .get_attribute() and use the return value to deduce the type of model you are looking at.

Going back to your example serializer, have you tried doing something like this. It won't work with ember_drf at this point, but if you use a RelatedField it might make it easier to implement the functionality you want.

class DocumentBaseSerializer(serializers.ModelSerializer):
    nodes = serializers.PrimaryKeyRelatedField(many=True, read_only=True, source='get_nodes')

In any event, I am happy to accept a PR that implements this. It would be ideal if we can leave the sideload config tuple format the same, but at the very least it should make the serializer field argument optional.

[dgaus]

That's correct, and adding a PrimaryKeyRelatedField with source='get_nodes' also works. The thing is, when you include a polymorphic model, you need to specify not only the ID but also the type. So for example, instead of having "fruit": [ 1, 2 ] I return

fruit: [ 
  { id: 1, type: 'banana' },
  { id: 2, type: 'apple'}
]

So Ember can then load the appropiate models. There are other conventions but they are similar. I see there's already a third parameter you can pass to sideloads with the queryset, you if I get this right are proposing an optional fourth parameter with the field name, which can be used (if specified) instead of the auto-detected field?

[g-cassie]

Yes, the fourth field would be the serializer field name and override the autodetection. If you can do it without adding a parameter that would be cool. However, last time I looked at this it was not possible to infer the model for a RelatedField which is the why the code examines the model class to determine what fields represent relationships that can be sideloaded.

I think you could subclass RelatedField and modify to_representation() to get that format.