Code

Ticket #15103: ticket_15103_trunk.diff

File ticket_15103_trunk.diff, 9.0 KB (added by lukeplant, 3 years ago)

Initial patch that fixes the issue

Line 
1diff -r 09ceaf49c46f django/contrib/admin/options.py
2--- a/django/contrib/admin/options.py   Mon Jan 17 23:58:51 2011 +0000
3+++ b/django/contrib/admin/options.py   Fri Jan 21 16:24:16 2011 +0000
4@@ -205,7 +205,16 @@
5             qs = qs.order_by(*ordering)
6         return qs
7 
8-    def lookup_allowed(self, lookup):
9+    def lookup_allowed(self, lookup, value):
10+        model = self.model
11+        # Check FKey lookups that are allowed, so that popups produced by
12+        # ForeignKeyRawIdWidget, on the basis of ForeignKey.limit_choices_to,
13+        # are allowed to work.
14+        for l in model._meta.related_fkey_lookups:
15+            for k, v in widgets.url_params_from_lookup_dict(l).items():
16+                if k == lookup and v == value:
17+                    return True
18+
19         parts = lookup.split(LOOKUP_SEP)
20 
21         # Last term in lookup is a query term (__exact, __startswith etc)
22@@ -217,7 +226,6 @@
23         # if foo has been specificially included in the lookup list; so
24         # drop __id if it is the last part. However, first we need to find
25         # the pk attribute name.
26-        model = self.model
27         pk_attr_name = None
28         for part in parts[:-1]:
29             field, _, _, _ = model._meta.get_field_by_name(part)
30diff -r 09ceaf49c46f django/contrib/admin/views/main.py
31--- a/django/contrib/admin/views/main.py        Mon Jan 17 23:58:51 2011 +0000
32+++ b/django/contrib/admin/views/main.py        Fri Jan 21 16:24:16 2011 +0000
33@@ -183,16 +183,18 @@
34 
35             # if key ends with __in, split parameter into separate values
36             if key.endswith('__in'):
37-                lookup_params[key] = value.split(',')
38+                value = value.split(',')
39+                lookup_params[key] = value
40 
41             # if key ends with __isnull, special case '' and false
42             if key.endswith('__isnull'):
43                 if value.lower() in ('', 'false'):
44-                    lookup_params[key] = False
45+                    value = False
46                 else:
47-                    lookup_params[key] = True
48+                    value = True
49+                lookup_params[key] = value
50 
51-            if not self.model_admin.lookup_allowed(key):
52+            if not self.model_admin.lookup_allowed(key, value):
53                 raise SuspiciousOperation(
54                     "Filtering by %s not allowed" % key
55                 )
56diff -r 09ceaf49c46f django/contrib/admin/widgets.py
57--- a/django/contrib/admin/widgets.py   Mon Jan 17 23:58:51 2011 +0000
58+++ b/django/contrib/admin/widgets.py   Fri Jan 21 16:24:16 2011 +0000
59@@ -91,6 +91,22 @@
60     template_with_clear = (u'<span class="clearable-file-input">%s</span>'
61                            % forms.ClearableFileInput.template_with_clear)
62 
63+def url_params_from_lookup_dict(lookups):
64+    """
65+    Converts the type of lookups specified in a ForeignKey limit_choices_to
66+    attribute to a dictionary of query parameters
67+    """
68+    params = {}
69+    if lookups and hasattr(lookups, 'items'):
70+        items = []
71+        for k, v in lookups.items():
72+            if isinstance(v, list):
73+                v = u','.join([str(x) for x in v])
74+            else:
75+                v = unicode(v)
76+            items.append((k, v))
77+        params.update(dict(items))
78+    return params
79 
80 class ForeignKeyRawIdWidget(forms.TextInput):
81     """
82@@ -108,33 +124,23 @@
83         related_url = '../../../%s/%s/' % (self.rel.to._meta.app_label, self.rel.to._meta.object_name.lower())
84         params = self.url_parameters()
85         if params:
86-            url = '?' + '&amp;'.join(['%s=%s' % (k, v) for k, v in params.items()])
87+            url = u'?' + u'&amp;'.join([u'%s=%s' % (k, v) for k, v in params.items()])
88         else:
89-            url = ''
90+            url = u''
91         if "class" not in attrs:
92             attrs['class'] = 'vForeignKeyRawIdAdminField' # The JavaScript looks for this hook.
93         output = [super(ForeignKeyRawIdWidget, self).render(name, value, attrs)]
94         # TODO: "id_" is hard-coded here. This should instead use the correct
95         # API to determine the ID dynamically.
96-        output.append('<a href="%s%s" class="related-lookup" id="lookup_id_%s" onclick="return showRelatedObjectLookupPopup(this);"> ' % \
97+        output.append(u'<a href="%s%s" class="related-lookup" id="lookup_id_%s" onclick="return showRelatedObjectLookupPopup(this);"> ' % \
98             (related_url, url, name))
99-        output.append('<img src="%simg/admin/selector-search.gif" width="16" height="16" alt="%s" /></a>' % (settings.ADMIN_MEDIA_PREFIX, _('Lookup')))
100+        output.append(u'<img src="%simg/admin/selector-search.gif" width="16" height="16" alt="%s" /></a>' % (settings.ADMIN_MEDIA_PREFIX, _('Lookup')))
101         if value:
102             output.append(self.label_for_value(value))
103         return mark_safe(u''.join(output))
104 
105     def base_url_parameters(self):
106-        params = {}
107-        if self.rel.limit_choices_to and hasattr(self.rel.limit_choices_to, 'items'):
108-            items = []
109-            for k, v in self.rel.limit_choices_to.items():
110-                if isinstance(v, list):
111-                    v = ','.join([str(x) for x in v])
112-                else:
113-                    v = str(v)
114-                items.append((k, v))
115-            params.update(dict(items))
116-        return params
117+        return url_params_from_lookup_dict(self.rel.limit_choices_to)
118 
119     def url_parameters(self):
120         from django.contrib.admin.views.main import TO_FIELD_VAR
121diff -r 09ceaf49c46f django/db/models/fields/related.py
122--- a/django/db/models/fields/related.py        Mon Jan 17 23:58:51 2011 +0000
123+++ b/django/db/models/fields/related.py        Fri Jan 21 16:24:16 2011 +0000
124@@ -890,6 +890,8 @@
125         # don't get a related descriptor.
126         if not self.rel.is_hidden():
127             setattr(cls, related.get_accessor_name(), ForeignRelatedObjectsDescriptor(related))
128+            if self.rel.limit_choices_to:
129+                cls._meta.related_fkey_lookups.append(self.rel.limit_choices_to)
130         if self.rel.field_name is None:
131             self.rel.field_name = cls._meta.pk.name
132 
133diff -r 09ceaf49c46f django/db/models/options.py
134--- a/django/db/models/options.py       Mon Jan 17 23:58:51 2011 +0000
135+++ b/django/db/models/options.py       Fri Jan 21 16:24:16 2011 +0000
136@@ -55,6 +55,10 @@
137         self.abstract_managers = []
138         self.concrete_managers = []
139 
140+        # List of all lookups defined in ForeignKey 'limit_choices_to' options
141+        # from *other* models. Needed for some admin checks.
142+        self.related_fkey_lookups = []
143+
144     def contribute_to_class(self, cls, name):
145         from django.db import connection
146         from django.db.backends.util import truncate_name
147diff -r 09ceaf49c46f tests/regressiontests/admin_views/models.py
148--- a/tests/regressiontests/admin_views/models.py       Mon Jan 17 23:58:51 2011 +0000
149+++ b/tests/regressiontests/admin_views/models.py       Fri Jan 21 16:24:16 2011 +0000
150@@ -178,6 +178,26 @@
151 class ThingAdmin(admin.ModelAdmin):
152     list_filter = ('color__warm', 'color__value')
153 
154+class Actor(models.Model):
155+    name = models.CharField(max_length=50)
156+    age = models.IntegerField()
157+    def __unicode__(self):
158+        return self.name
159+
160+class Inquisition(models.Model):
161+    expected = models.BooleanField()
162+    leader = models.ForeignKey(Actor)
163+    def __unicode__(self):
164+        return self.expected
165+
166+class Sketch(models.Model):
167+    title = models.CharField(max_length=100)
168+    inquisition = models.ForeignKey(Inquisition, limit_choices_to={'leader__name': 'Palin',
169+                                                                   'leader__age': 27,
170+                                                                   })
171+    def __unicode__(self):
172+        return self.title
173+
174 class Fabric(models.Model):
175     NG_CHOICES = (
176         ('Textured', (
177@@ -632,6 +652,9 @@
178 admin.site.register(ModelWithStringPrimaryKey)
179 admin.site.register(Color)
180 admin.site.register(Thing, ThingAdmin)
181+admin.site.register(Actor)
182+admin.site.register(Inquisition)
183+admin.site.register(Sketch)
184 admin.site.register(Person, PersonAdmin)
185 admin.site.register(Persona, PersonaAdmin)
186 admin.site.register(Subscriber, SubscriberAdmin)
187diff -r 09ceaf49c46f tests/regressiontests/admin_views/tests.py
188--- a/tests/regressiontests/admin_views/tests.py        Mon Jan 17 23:58:51 2011 +0000
189+++ b/tests/regressiontests/admin_views/tests.py        Fri Jan 21 16:24:16 2011 +0000
190@@ -392,6 +392,17 @@
191         response = self.client.get("/test_admin/admin/admin_views/workhour/?employee__person_ptr__exact=%d" % e1.pk)
192         self.assertEqual(response.status_code, 200)
193 
194+    def test_allowed_filtering_15103(self):
195+        """
196+        Regressions test for ticket 15103 - filtering on fields defined in a
197+        ForeignKey 'limit_choices_to' should be allowed, otherwise raw_id_fields
198+        can break.
199+        """
200+        try:
201+            self.client.get("/test_admin/admin/admin_views/inquisition/?leader__name=Palin&leader__age=27")
202+        except SuspiciousOperation:
203+            self.fail("Filters should be allowed if they are defined on a ForeignKey pointing to this model")
204+
205 class SaveAsTests(TestCase):
206     fixtures = ['admin-views-users.xml','admin-views-person.xml']
207