From 7ee9efed3eabd24472d79d393cb1a69b0690f567 Mon Sep 17 00:00:00 2001 From: nkryptic Date: Mon, 11 Mar 2013 23:13:55 -0400 Subject: [PATCH 1/3] add conditional handling for broken filter lookup on reverse relationships; fixes #80 --- django_filters/filterset.py | 27 ++++- test_django_filters/filtering_tests.py | 147 +++++++++++++++++-------- test_django_filters/filterset_tests.py | 89 ++++++++++++++- test_django_filters/models.py | 30 +++++ 4 files changed, 237 insertions(+), 56 deletions(-) diff --git a/django_filters/filterset.py b/django_filters/filterset.py index ac5ea3b67..87cd6fd5e 100644 --- a/django_filters/filterset.py +++ b/django_filters/filterset.py @@ -65,12 +65,11 @@ def get_model_field(model, f): rel, model, direct, m2m = opts.get_field_by_name(parts[-1]) except FieldDoesNotExist: return None - if not direct: - return rel.field.rel.to_field return rel -def filters_for_model(model, fields=None, exclude=None, filter_for_field=None): +def filters_for_model(model, fields=None, exclude=None, filter_for_field=None, + filter_for_reverse_field=None): field_dict = SortedDict() opts = model._meta if fields is None: @@ -82,7 +81,10 @@ def filters_for_model(model, fields=None, exclude=None, filter_for_field=None): if field is None: field_dict[f] = None continue - filter_ = filter_for_field(field, f) + if isinstance(field, RelatedObject): + filter_ = filter_for_reverse_field(field, f) + else: + filter_ = filter_for_field(field, f) if filter_: field_dict[f] = filter_ return field_dict @@ -117,7 +119,8 @@ def __new__(cls, name, bases, attrs): getattr(new_class, 'Meta', None)) if opts.model: filters = filters_for_model(opts.model, opts.fields, opts.exclude, - new_class.filter_for_field) + new_class.filter_for_field, + new_class.filter_for_reverse_field) filters.update(declared_filters) else: filters = declared_filters @@ -340,6 +343,20 @@ def filter_for_field(cls, f, name): if filter_class is not None: return filter_class(**default) + @classmethod + def filter_for_reverse_field(cls, f, name): + rel = f.field.rel + queryset = f.model._default_manager.all() + default = { + 'name': name, + 'label': capfirst(rel.related_name), + 'queryset': queryset, + } + if rel.multiple: + return ModelMultipleChoiceFilter(**default) + else: + return ModelChoiceFilter(**default) + class FilterSet(six.with_metaclass(FilterSetMetaclass, BaseFilterSet)): pass diff --git a/test_django_filters/filtering_tests.py b/test_django_filters/filtering_tests.py index baa1c1afc..0eaa8177b 100644 --- a/test_django_filters/filtering_tests.py +++ b/test_django_filters/filtering_tests.py @@ -32,6 +32,8 @@ from .models import Location from .models import Account from .models import Profile +from .models import Node +from .models import DirectedNode from .models import STATUS_CHOICES @@ -224,7 +226,9 @@ class Meta: # this is how it would come through a browser f = F({'published': check_dt}, queryset=qs) - self.assertEqual(len(f.qs), 1, + self.assertEqual( + len(f.qs), + 1, "%s isn't matching %s when cleaned" % (check_dt, ten_min_ago)) self.assertQuerysetEqual(f.qs, [2], lambda o: o.pk) @@ -494,11 +498,17 @@ class Meta: self.assertQuerysetEqual(f.qs, [1], lambda o: o.pk) def test_reverse_o2o_relation(self): - with self.assertRaises(AttributeError): - class F(FilterSet): - class Meta: - model = Account - fields = ('profile',) + class F(FilterSet): + class Meta: + model = Account + fields = ('profile',) + + f = F() + self.assertEqual(f.qs.count(), 4) + + f = F({'profile': 1}) + self.assertEqual(f.qs.count(), 1) + self.assertQuerysetEqual(f.qs, [1], lambda o: o.pk) def test_o2o_relation_attribute(self): class F(FilterSet): @@ -589,26 +599,24 @@ def test_reverse_fk_relation(self): Comment.objects.create(text='comment 3', author=jacob, time=time, date=date) - with self.assertRaises(AttributeError): - class F(FilterSet): - class Meta: - model = User - fields = ['comments'] + class F(FilterSet): + class Meta: + model = User + fields = ['comments'] - # qs = User.objects.all() - # f = F({'comment': 2}, queryset=qs) - # self.assertQuerysetEqual(f.qs, ['alex'], lambda o: o.username) + qs = User.objects.all() + f = F({'comments': [2]}, queryset=qs) + self.assertQuerysetEqual(f.qs, ['alex'], lambda o: o.username) - with self.assertRaises(AttributeError): - class F(FilterSet): - comment = AllValuesFilter() + class F(FilterSet): + comments = AllValuesFilter() - class Meta: - model = User - fields = ['comments'] + class Meta: + model = User + fields = ['comments'] - # f = F({'comments': 2}, queryset=qs) - # self.assertQuerysetEqual(f.qs, ['alex'], lambda o: o.username) + f = F({'comments': 2}, queryset=qs) + self.assertQuerysetEqual(f.qs, ['alex'], lambda o: o.username) def test_fk_relation_attribute(self): now_dt = now() @@ -728,28 +736,26 @@ class Meta: self.assertQuerysetEqual(f.qs, [], lambda o: o.username) def test_reverse_m2m_relation(self): - with self.assertRaises(AttributeError): - class F(FilterSet): - class Meta: - model = Book - fields = ['lovers'] - - # qs = User.objects.all() - # f = F({'lovers': [1]}, queryset=qs) - # self.assertQuerysetEqual( - # f.qs, ["Ender's Game", "Rainbow Six"], lambda o: o.title) + class F(FilterSet): + class Meta: + model = Book + fields = ['lovers'] + + qs = Book.objects.all() + f = F({'lovers': [1]}, queryset=qs) + self.assertQuerysetEqual( + f.qs, ["Ender's Game", "Rainbow Six"], lambda o: o.title) - with self.assertRaises(AttributeError): - class F(FilterSet): - lovers = AllValuesFilter() + class F(FilterSet): + lovers = AllValuesFilter() - class Meta: - model = Book - fields = ['lovers'] + class Meta: + model = Book + fields = ['lovers'] - # f = F({'lovers': 1}, queryset=qs) - # self.assertQuerysetEqual( - # f.qs, ["Ender's Game", "Rainbow Six"], lambda o: o.title) + f = F({'lovers': 1}, queryset=qs) + self.assertQuerysetEqual( + f.qs, ["Ender's Game", "Rainbow Six"], lambda o: o.title) def test_m2m_relation_attribute(self): class F(FilterSet): @@ -872,15 +878,60 @@ def test_fk_relation_attribute_on_m2m_relation(self): pass -class SelfReferentialRelationshipTests(TestCase): +class SymmetricalSelfReferentialRelationshipTests(TestCase): - @unittest.skip('todo') - def test_self_referential_m2m_relation(self): - pass + def setUp(self): + n1 = Node.objects.create(name='one') + n2 = Node.objects.create(name='two') + n3 = Node.objects.create(name='three') + n4 = Node.objects.create(name='four') + n1.adjacents.add(n2) + n2.adjacents.add(n3) + n2.adjacents.add(n4) + n4.adjacents.add(n1) - @unittest.skip('todo') - def test_reverse_self_referential_m2m_relation(self): - pass + def test_relation(self): + class F(FilterSet): + class Meta: + model = Node + fields = ['adjacents'] + + qs = Node.objects.all().order_by('pk') + f = F({'adjacents': ['1']}, queryset=qs) + self.assertQuerysetEqual(f.qs, [2, 4], lambda o: o.pk) + + +class NonSymmetricalSelfReferentialRelationshipTests(TestCase): + + def setUp(self): + n1 = DirectedNode.objects.create(name='one') + n2 = DirectedNode.objects.create(name='two') + n3 = DirectedNode.objects.create(name='three') + n4 = DirectedNode.objects.create(name='four') + n1.outbound_nodes.add(n2) + n2.outbound_nodes.add(n3) + n2.outbound_nodes.add(n4) + n4.outbound_nodes.add(n1) + + def test_forward_relation(self): + class F(FilterSet): + class Meta: + model = DirectedNode + fields = ['outbound_nodes'] + + qs = DirectedNode.objects.all().order_by('pk') + f = F({'outbound_nodes': ['1']}, queryset=qs) + self.assertQuerysetEqual(f.qs, [4], lambda o: o.pk) + + def test_reverse_relation(self): + class F(FilterSet): + class Meta: + model = DirectedNode + fields = ['inbound_nodes'] + + qs = DirectedNode.objects.all().order_by('pk') + f = F({'inbound_nodes': ['1']}, queryset=qs) + self.assertQuerysetEqual(f.qs, [2], lambda o: o.pk) class MiscFilterSetTests(TestCase): diff --git a/test_django_filters/filterset_tests.py b/test_django_filters/filterset_tests.py index f725d820e..db6913272 100644 --- a/test_django_filters/filterset_tests.py +++ b/test_django_filters/filterset_tests.py @@ -12,16 +12,23 @@ from django_filters.filterset import get_model_field from django_filters.filters import CharFilter from django_filters.filters import ChoiceFilter +from django_filters.filters import ModelChoiceFilter from django_filters.filters import ModelMultipleChoiceFilter from .models import User from .models import AdminUser from .models import Book +from .models import Profile +from .models import Comment from .models import Restaurant from .models import NetworkSetting from .models import SubnetMaskField from .models import Account from .models import BankAccount +from .models import Node +from .models import DirectedNode +from .models import Worker +from .models import Business class HelperMethodsTests(TestCase): @@ -103,15 +110,16 @@ def test_filter_not_found_for_field(self): result = FilterSet.filter_for_field(f, 'id') self.assertIsNone(result) - def test_filter_for_field_with_extras(self): + def test_field_with_extras(self): f = User._meta.get_field('favorite_books') result = FilterSet.filter_for_field(f, 'favorite_books') self.assertIsInstance(result, ModelMultipleChoiceFilter) self.assertEqual(result.name, 'favorite_books') self.assertTrue('queryset' in result.extra) self.assertIsNotNone(result.extra['queryset']) + self.assertEqual(result.extra['queryset'].model, Book) - def test_filter_for_field_with_choices(self): + def test_field_with_choices(self): f = User._meta.get_field('status') result = FilterSet.filter_for_field(f, 'status') self.assertIsInstance(result, ChoiceFilter) @@ -119,16 +127,91 @@ def test_filter_for_field_with_choices(self): self.assertTrue('choices' in result.extra) self.assertIsNotNone(result.extra['choices']) - def test_filter_for_field_that_is_subclassed(self): + def test_field_that_is_subclassed(self): f = User._meta.get_field('first_name') result = FilterSet.filter_for_field(f, 'first_name') self.assertIsInstance(result, CharFilter) + def test_symmetrical_selfref_m2m_field(self): + f = Node._meta.get_field('adjacents') + result = FilterSet.filter_for_field(f, 'adjacents') + self.assertIsInstance(result, ModelMultipleChoiceFilter) + self.assertEqual(result.name, 'adjacents') + self.assertTrue('queryset' in result.extra) + self.assertIsNotNone(result.extra['queryset']) + self.assertEqual(result.extra['queryset'].model, Node) + + def test_non_symmetrical_selfref_m2m_field(self): + f = DirectedNode._meta.get_field('outbound_nodes') + result = FilterSet.filter_for_field(f, 'outbound_nodes') + self.assertIsInstance(result, ModelMultipleChoiceFilter) + self.assertEqual(result.name, 'outbound_nodes') + self.assertTrue('queryset' in result.extra) + self.assertIsNotNone(result.extra['queryset']) + self.assertEqual(result.extra['queryset'].model, DirectedNode) + + def test_m2m_field_with_through_model(self): + f = Business._meta.get_field('employees') + result = FilterSet.filter_for_field(f, 'employees') + self.assertIsInstance(result, ModelMultipleChoiceFilter) + self.assertEqual(result.name, 'employees') + self.assertTrue('queryset' in result.extra) + self.assertIsNotNone(result.extra['queryset']) + self.assertEqual(result.extra['queryset'].model, Worker) + @unittest.skip('todo') def test_filter_overrides(self): pass +class FilterSetFilterForReverseFieldTests(TestCase): + + def test_reverse_o2o_relationship(self): + f = Account._meta.get_field_by_name('profile')[0] + result = FilterSet.filter_for_reverse_field(f, 'profile') + self.assertIsInstance(result, ModelChoiceFilter) + self.assertEqual(result.name, 'profile') + self.assertTrue('queryset' in result.extra) + self.assertIsNotNone(result.extra['queryset']) + self.assertEqual(result.extra['queryset'].model, Profile) + + def test_reverse_fk_relationship(self): + f = User._meta.get_field_by_name('comments')[0] + result = FilterSet.filter_for_reverse_field(f, 'comments') + self.assertIsInstance(result, ModelMultipleChoiceFilter) + self.assertEqual(result.name, 'comments') + self.assertTrue('queryset' in result.extra) + self.assertIsNotNone(result.extra['queryset']) + self.assertEqual(result.extra['queryset'].model, Comment) + + def test_reverse_m2m_relationship(self): + f = Book._meta.get_field_by_name('lovers')[0] + result = FilterSet.filter_for_reverse_field(f, 'lovers') + self.assertIsInstance(result, ModelMultipleChoiceFilter) + self.assertEqual(result.name, 'lovers') + self.assertTrue('queryset' in result.extra) + self.assertIsNotNone(result.extra['queryset']) + self.assertEqual(result.extra['queryset'].model, User) + + def test_reverse_non_symmetrical_selfref_m2m_field(self): + f = DirectedNode._meta.get_field_by_name('inbound_nodes')[0] + result = FilterSet.filter_for_reverse_field(f, 'inbound_nodes') + self.assertIsInstance(result, ModelMultipleChoiceFilter) + self.assertEqual(result.name, 'inbound_nodes') + self.assertTrue('queryset' in result.extra) + self.assertIsNotNone(result.extra['queryset']) + self.assertEqual(result.extra['queryset'].model, DirectedNode) + + def test_reverse_m2m_field_with_through_model(self): + f = Worker._meta.get_field_by_name('employers')[0] + result = FilterSet.filter_for_reverse_field(f, 'employers') + self.assertIsInstance(result, ModelMultipleChoiceFilter) + self.assertEqual(result.name, 'employers') + self.assertTrue('queryset' in result.extra) + self.assertIsNotNone(result.extra['queryset']) + self.assertEqual(result.extra['queryset'].model, Business) + + class FilterSetClassCreationTests(TestCase): def test_no_filters(self): diff --git a/test_django_filters/models.py b/test_django_filters/models.py index 574dbc4d2..31f7c3061 100644 --- a/test_django_filters/models.py +++ b/test_django_filters/models.py @@ -137,3 +137,33 @@ class Profile(models.Model): class BankAccount(Account): amount_saved = models.IntegerField(default=0) + +class Node(models.Model): + name = models.CharField(max_length=20) + adjacents = models.ManyToManyField('self') + + +class DirectedNode(models.Model): + name = models.CharField(max_length=20) + outbound_nodes = models.ManyToManyField('self', + symmetrical=False, + related_name='inbound_nodes') + + +class Worker(models.Model): + name = models.CharField(max_length=100) + + +class HiredWorker(models.Model): + salary = models.IntegerField() + hired_on = models.DateField() + worker = models.ForeignKey(Worker) + business = models.ForeignKey('Business') + + +class Business(models.Model): + name = models.CharField(max_length=100) + employees = models.ManyToManyField(Worker, + through=HiredWorker, + related_name='employers') + From 5bf5bb6222b1307674eef1230eac1193d83aa439 Mon Sep 17 00:00:00 2001 From: nkryptic Date: Mon, 11 Mar 2013 23:15:19 -0400 Subject: [PATCH 2/3] update project for pending release --- .gitignore | 1 + CHANGES.rst | 26 ++++++++++++++++++++++++++ MANIFEST.in | 4 ++++ setup.py | 2 +- 4 files changed, 32 insertions(+), 1 deletion(-) create mode 100644 CHANGES.rst diff --git a/.gitignore b/.gitignore index 6d582b312..45aba068c 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,3 @@ *.pyc +*.egg-info docs/_build diff --git a/CHANGES.rst b/CHANGES.rst new file mode 100644 index 000000000..dc3d48d06 --- /dev/null +++ b/CHANGES.rst @@ -0,0 +1,26 @@ +Version 0.6 (Unreleased) +------------------------ + +* raised minimum Django version to 1.4.x + +* added Python 3.2 and Python 3.3 support + +* added Django 1.5 support and initial 1.6 compatability + +* FEATURE: recognition of custom model field subclasses + +* FEATURE: allow optional display names for order_by values + +* FEATURE: addition of class-based FilterView + +* FEATURE: addition of count() method on FilterSet to prevent pagination + from loading entire queryset + +* FIXED: attempts to filter on reverse side of m2m, o2o or fk would + raise an error + + +Version 0.5.4 (2012-11-16) +-------------------------- + +* project brought back to life diff --git a/MANIFEST.in b/MANIFEST.in index e5e17c2a5..e70c1a58c 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -1,5 +1,9 @@ include AUTHORS.txt +include CHANGES.rst include LICENSE include README.rst +include runshell.py +include runtests.py recursive-include docs * +recursive-include requirements * recursive-include test_django_filters/templates/test_django_filters * diff --git a/setup.py b/setup.py index 35647f213..6eed32d24 100644 --- a/setup.py +++ b/setup.py @@ -6,7 +6,7 @@ setup( name='django-filter', - version='0.5.5a1', + version='0.6a1', description=('Django-filter is a reusable Django application for allowing' ' users to filter querysets dynamically.'), long_description=readme, From 6f3fa15dface8ab548364e12bbe6f36e9164b8fe Mon Sep 17 00:00:00 2001 From: nkryptic Date: Mon, 11 Mar 2013 23:42:59 -0400 Subject: [PATCH 3/3] fix django 1.6 assertQuerySetEqual usage and replace deprecated assertions for py3k --- test_django_filters/filter_tests.py | 2 +- test_django_filters/filtering_tests.py | 2 +- test_django_filters/filterset_tests.py | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/test_django_filters/filter_tests.py b/test_django_filters/filter_tests.py index 5e60fcc24..a5493d8df 100644 --- a/test_django_filters/filter_tests.py +++ b/test_django_filters/filter_tests.py @@ -35,7 +35,7 @@ def test_creation(self): def test_creation_order(self): f = Filter() f2 = Filter() - self.assert_(f2.creation_counter > f.creation_counter) + self.assertTrue(f2.creation_counter > f.creation_counter) def test_default_field(self): f = Filter() diff --git a/test_django_filters/filtering_tests.py b/test_django_filters/filtering_tests.py index 0eaa8177b..d3e37f2a1 100644 --- a/test_django_filters/filtering_tests.py +++ b/test_django_filters/filtering_tests.py @@ -741,7 +741,7 @@ class Meta: model = Book fields = ['lovers'] - qs = Book.objects.all() + qs = Book.objects.all().order_by('title') f = F({'lovers': [1]}, queryset=qs) self.assertQuerysetEqual( f.qs, ["Ender's Game", "Rainbow Six"], lambda o: o.title) diff --git a/test_django_filters/filterset_tests.py b/test_django_filters/filterset_tests.py index db6913272..4faf72887 100644 --- a/test_django_filters/filterset_tests.py +++ b/test_django_filters/filterset_tests.py @@ -324,14 +324,14 @@ class F(FilterSet): class Meta: model = Restaurant - self.assertEquals(set(F.base_filters), set(['name', 'serves_pizza'])) + self.assertEqual(set(F.base_filters), set(['name', 'serves_pizza'])) class F(FilterSet): class Meta: model = Restaurant fields = ['name', 'serves_pizza'] - self.assertEquals(set(F.base_filters), set(['name', 'serves_pizza'])) + self.assertEqual(set(F.base_filters), set(['name', 'serves_pizza'])) def test_custom_field_ignored(self): class F(FilterSet):