Skip to content

Commit

Permalink
Merge pull request #62 from Princeton-CDH/chore/signal-dependencies
Browse files Browse the repository at this point in the history
Handle more cases when identifying index dependencies for django models
  • Loading branch information
rlskoeser committed May 10, 2021
2 parents 93f7145 + 088e5f1 commit 6f7656b
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 7 deletions.
15 changes: 13 additions & 2 deletions parasolr/django/indexing.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ def index_data(self):
from parasolr.indexing import Indexable

try:
from django.db.models import Model
from django.db.models import Model, Manager
from django.apps import apps
from django.db.models.fields import related_descriptors
except ImportError:
Expand Down Expand Up @@ -96,6 +96,16 @@ def get_related_model(model, name):
related_descriptors.ReverseManyToOneDescriptor):
related_model = attr.rel.related_model

elif isinstance(attr,
related_descriptors.ForwardManyToOneDescriptor):
# many to one, i.e. foreign key
related_model = attr.field.related_model

elif isinstance(attr, Manager):
# specific to django-taggit TaggableManager
if hasattr(attr.through, 'tag_model'):
related_model = attr.through.tag_model()

if related_model:
return related_model

Expand Down Expand Up @@ -129,7 +139,8 @@ def identify_index_dependencies(cls):
if hasattr(model, dep):
attr = getattr(model, dep)
if isinstance(
attr, related_descriptors.ManyToManyDescriptor):
attr, (Manager,
related_descriptors.ManyToManyDescriptor)):
# add through model to many to many list
m2m.append(attr.through)

Expand Down
2 changes: 1 addition & 1 deletion parasolr/django/signals.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ def ready(self):
:class:`~parasolr.django.indexing.ModelIndexable` subclass with the
dependencies and signals that should trigger reindexing. Example::
class MyModel(Model, ModelIndexable):
class MyModel(ModelIndexable):
index_depends_on = {
'collections': {
Expand Down
18 changes: 14 additions & 4 deletions parasolr/django/tests/test_django.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@

try:
import django
from django.db.models import Manager
from django.core.exceptions import ImproperlyConfigured
from django.db import models
from django.test import override_settings

from parasolr.django import SolrClient, SolrQuerySet, \
Expand Down Expand Up @@ -183,12 +183,22 @@ def test_get_related_model(caplog):
assert ModelIndexable.get_related_model(
IndexItem, 'owner_set__collections') == Collection

# foreign key is not currently supported; should warn
# foreign key is now supported!
assert ModelIndexable.get_related_model(
IndexItem, 'primary') == Collection

# use mock to test taggable manager behavior
mockitem = Mock()
mockitem.tags = Mock(spec=Manager, through=Mock())
mockitem.tags.through.tag_model.return_value = 'TagBase'
assert ModelIndexable.get_related_model(mockitem, 'tags') == \
'TagBase'

# if relation cannot be determined, should warn
with caplog.at_level(logging.WARNING):
assert not ModelIndexable.get_related_model(IndexItem, 'primary')
assert not ModelIndexable.get_related_model(mockitem, 'foo')
assert 'Unhandled related model' in caplog.text


# these classes cannot be defined without django dependencies
if django:

Expand Down

0 comments on commit 6f7656b

Please sign in to comment.