Skip to content

Conversation

@dsanders11
Copy link
Contributor

This PR fixes the case where a nested field is a OneToOneField and the related object doesn't exist. Attempting to access the attribute throws a RelatedObjectDoesNotExist exception which causes django-elasticsearch-dsl to not be able to index the model.

Fix simply checks if the exception thrown is an ObjectDoesNotExist exception (which is a subclass of AttributeError) and if so returns None for the instance, allowing the field to be skipped when the related object doesn't exist.

Example reproduce code:

# models.py

class Car(models.Model):
    name = models.CharField()
    color = models.CharField()
    manufacturer = models.ForeignKey('Manufacturer')

class Manufacturer(models.Model):
    name = models.CharField()
    country_code = models.CharField(max_length=2)
    created = models.DateField()
    headquarters = models.OneToOneField('Headquarters')

class Headquarters(models.Model):
    city = models.CharField()
# documents.py

from django_elasticsearch_dsl import DocType, Index
from .models import Car

car = Index('cars')
car.settings(
    number_of_shards=1,
    number_of_replicas=0
)


@car.doc_type
class CarDocument(DocType):
    manufacturer = fields.ObjectField(properties={
        'name': fields.StringField(),
        'country_code': fields.StringField(),
        'headquarters': fields.ObjectField(properties={
            'city': fields.StringField()
        })
    })

    class Meta:
        model = Car
        fields = [
            'name',
            'color',
        ]

except (TypeError, AttributeError) as e:
if isinstance(e, ObjectDoesNotExist):
return None

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could catch the ObjectDoesNotExist before the except block that catches AttributeError, instead of doing an isinstance check

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stevelacey, you're right, that's much cleaner, not sure what I was thinking. I'll update the PR to use that style.

@sabricot sabricot merged commit 61a3071 into django-es:master Mar 17, 2018
@sabricot
Copy link
Member

Thanks @dsanders11 !

@dsanders11 dsanders11 deleted the nested-one-to-one branch March 19, 2018 20:47
@dsanders11 dsanders11 restored the nested-one-to-one branch April 2, 2018 13:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants