Ticket #11707: limit_ForeignKey.1.3.X.patch

django/forms/models.py

@@ -873 +873 @@
             yield (u"", self.field.empty_label)
         if self.field.cache_choices:
             if self.field.choice_cache is None:
-                self.field.choice_cache = [
-                    self.choice(obj) for obj in self.queryset.all()
-                ]
+                self.field.choice_cache = list(self.distinct_choices())
             for choice in self.field.choice_cache:
                 yield choice
         else:
-            for obj in self.queryset.all():
+            for choice in self.distinct_choices():
+                yield choice
+
+    def distinct_choices(self):
+        """Yields a choice for each distinct object in the queryset.
+
+        If you use limit_choices_to you could have more than one results for a
+        single Instance. DISTINCT in the db won't work as expected so we do
+        the check here."""
+        seen_choices = set()
+        for obj in self.queryset.all():
+            if obj.pk not in seen_choices:
+                seen_choices.add(obj.pk)
                 yield self.choice(obj)
 
     def __len__(self):
@@ -969 +979 @@
             return None
         try:
             key = self.to_field_name or 'pk'
-            value = self.queryset.get(**{key: value})
+            values = self.queryset.filter(**{key: value})
+            # If you use limit_choices_to you could have more than one results
+            # for a single Instance. DISTINCT in the db won't work as expected
+            # so we do the check here.
+            if not values:
+                raise self.queryset.model.DoesNotExist
+            if len(values) == 1:
+                value = values[0]
+            else:
+                pks = set(value.pk for value in values)
+                if len(pks) == 1:
+                    value = values[0]
+                else:
+                    raise self.queryset.model.MultipleObjectsReturned
         except (ValueError, self.queryset.model.DoesNotExist):
             raise ValidationError(self.error_messages['invalid_choice'])
         return value

tests/regressiontests/model_fields/models.py

@@ -29 +29 @@
     b = models.CharField(max_length=10)
     a = models.ForeignKey(Foo, default=get_foo)
 
+class Baz(models.Model):
+    a = models.CharField(max_length=5)
+    #Only Foos related to Bars starting with 'a'
+    foo = models.ForeignKey(Foo, limit_choices_to=models.Q(bar__b__startswith='a'))
+
 class Whiz(models.Model):
     CHOICES = (
         ('Group 1', (

tests/regressiontests/model_fields/tests.py

@@ -1 +1 @@
 import datetime
 from decimal import Decimal
+import re
 
 from django import test
 from django import forms
@@ -8 +9 @@
 from django.db.models.fields.files import FieldFile
 from django.utils import unittest
 
-from models import Foo, Bar, Whiz, BigD, BigS, Image, BigInt, Post, NullBooleanModel, BooleanModel, Document
+from models import Foo, Bar, Baz, Whiz, BigD, BigS, Image, BigInt, Post, NullBooleanModel, BooleanModel, Document
 
 # If PIL available, do these tests.
 if Image:
@@ -95 +96 @@
         # This should not crash. That counts as a win for our purposes.
         Foo.objects.filter(d__gte=100000000000)
 
+
+class BazForm(forms.ModelForm):
+    class Meta:
+        model = Baz
+
+
 class ForeignKeyTests(test.TestCase):
     def test_callable_default(self):
         """Test the use of a lazy callable for ForeignKey.default"""
@@ -102 +109 @@
         b = Bar.objects.create(b="bcd")
         self.assertEqual(b.a, a)
 
+    def test_distinct_choice_limit(self):
+        """Doesn't make sense to offer the same ForeignKey multiple times in a form"""
+        a = Foo.objects.create(a='a', d=Decimal("-1"))
+        b = Foo.objects.create(a='b', d=Decimal("1"))
+        Bar.objects.create(b='ah', a=a)
+        Bar.objects.create(b='aha', a=a)
+        Bar.objects.create(b='bla', a=b)
+        form = BazForm()
+        fk_field = str(form['foo'])
+        self.assertEqual(len(re.findall(r'value="%s"' % b.pk, fk_field)), 0)
+        self.assertEqual(len(re.findall(r'value="%s"' % a.pk, fk_field)), 1)
+
+    def test_distinct_choice_limit_save(self):
+        """Doesn't make sense to offer the same ForeignKey multiple times in a form"""
+        a = Foo.objects.create(a='a', d=Decimal("-1"))
+        b = Foo.objects.create(a='b', d=Decimal("1"))
+        Bar.objects.create(b='ah', a=a)
+        Bar.objects.create(b='aha', a=a)
+        Bar.objects.create(b='bla', a=b)
+        form = BazForm({'foo': a.pk, 'a': 'a'})
+        self.assertTrue(form.is_valid())
+        obj = form.save()
+        self.assertEqual(obj.foo.pk, a.pk)
+
 class DateTimeFieldTests(unittest.TestCase):
     def test_datetimefield_to_python_usecs(self):
         """DateTimeField.to_python should support usecs"""