liberation / django-elasticsearch

Simple wrapper around elasticsearch-py to index/search a django Model.
MIT License
211 stars 73 forks source link

ManyToOneRel can't be serialized #32

Open skabbit opened 8 years ago

skabbit commented 8 years ago

ManyToManyField with 'through' Model bring us to some problems

Error on line 118 in django_elasticsearch/serializers.py: AttributeError: 'ManyToOneRel' object has no attribute 'rel'

lauxley commented 8 years ago

Hi, I added a use case in the test model here but can't replicate, do you have more informations ? django version, models used, any customizations of the serializer ? Note that in the meantime you can create a custom serializer for this field.

skabbit commented 8 years ago

My apologies, I've got this problem on serializing ManyToOne relation like this:

class Agency(EsIndexable, models.Model):
    name = models.CharField(
        max_length=300,
        verbose_name='Agency name',
        blank=True, null=True, default=None)

    class Elasticsearch(EsIndexable.Elasticsearch):
        serializer_class = AgencyEsSerializer
        fields = [
            'id',
            'name',
            'oldnames',
        ]

class AgencyOldName(models.Model):
    agency = models.ForeignKey(
        Agency,
        related_name='oldnames',
        on_delete=models.SET_NULL,
        blank=True, null=True, default=None)
    name = models.CharField(
        max_length=300,
        verbose_name='Old agency name',
        blank=True, null=True, default=None)

And yes, I'm using custom serializer to solve this problem.

lauxley commented 8 years ago

Hi, so the problem is that in the serializer for some reason 'field' is not an instance of Field but ManyToOneRel which is not a subclass of Field. Can you show me the query(ies) and the serializer ?

skabbit commented 8 years ago

Here is the AgencyEsSerializer, with custom serializer for this field:

class AgencyEsSerializer(EsJsonSerializer):
    """
    Agency serializer
    """

    def serialize_oldnames(self, instance, field_name):
        return list(instance.oldnames.all().values('name'))

And here is the code, causing the problem:

            Agency.es.reindex_all()
lauxley commented 8 years ago

Hi, the problem is that reverse relationships are not implemented, it's a big blunder so thx for pointing it out. I started a PR it should solve your problem but it will requires more work before i can merge it.

skabbit commented 8 years ago

Tnx a lot for this patch and for whole module of cause! :+1: I hope soon I'll have a time to contribute.