diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py index 91dc3e0953..71f08df87c 100644 --- a/django/contrib/admin/options.py +++ b/django/contrib/admin/options.py @@ -173,7 +173,16 @@ class BaseModelAdmin(object): return None declared_fieldsets = property(_declared_fieldsets) - def lookup_allowed(self, lookup): + def lookup_allowed(self, lookup, value): + model = self.model + # Check FKey lookups that are allowed, so that popups produced by + # ForeignKeyRawIdWidget, on the basis of ForeignKey.limit_choices_to, + # are allowed to work. + for l in model._meta.related_fkey_lookups: + for k, v in widgets.url_params_from_lookup_dict(l).items(): + if k == lookup and v == value: + return True + parts = lookup.split(LOOKUP_SEP) # Last term in lookup is a query term (__exact, __startswith etc) diff --git a/django/contrib/admin/views/main.py b/django/contrib/admin/views/main.py index 961b092cd0..911436adf9 100644 --- a/django/contrib/admin/views/main.py +++ b/django/contrib/admin/views/main.py @@ -184,16 +184,18 @@ class ChangeList(object): # if key ends with __in, split parameter into separate values if key.endswith('__in'): - lookup_params[key] = value.split(',') + value = value.split(',') + lookup_params[key] = value # if key ends with __isnull, special case '' and false if key.endswith('__isnull'): if value.lower() in ('', 'false'): - lookup_params[key] = False + value = False else: - lookup_params[key] = True + value = True + lookup_params[key] = value - if not self.model_admin.lookup_allowed(key): + if not self.model_admin.lookup_allowed(key, value): raise SuspiciousOperation( "Filtering by %s not allowed" % key ) diff --git a/django/contrib/admin/widgets.py b/django/contrib/admin/widgets.py index d3f30bfc13..17067346f6 100644 --- a/django/contrib/admin/widgets.py +++ b/django/contrib/admin/widgets.py @@ -97,6 +97,23 @@ class AdminFileWidget(forms.FileInput): output.append(super(AdminFileWidget, self).render(name, value, attrs)) return mark_safe(u''.join(output)) +def url_params_from_lookup_dict(lookups): + """ + Converts the type of lookups specified in a ForeignKey limit_choices_to + attribute to a dictionary of query parameters + """ + params = {} + if lookups and hasattr(lookups, 'items'): + items = [] + for k, v in lookups.items(): + if isinstance(v, list): + v = u','.join([str(x) for x in v]) + else: + v = unicode(v) + items.append((k, v)) + params.update(dict(items)) + return params + class ForeignKeyRawIdWidget(forms.TextInput): """ A Widget for displaying ForeignKeys in the "raw_id" interface rather than @@ -112,33 +129,23 @@ class ForeignKeyRawIdWidget(forms.TextInput): related_url = '../../../%s/%s/' % (self.rel.to._meta.app_label, self.rel.to._meta.object_name.lower()) params = self.url_parameters() if params: - url = '?' + '&'.join(['%s=%s' % (k, v) for k, v in params.items()]) + url = u'?' + u'&'.join([u'%s=%s' % (k, v) for k, v in params.items()]) else: - url = '' + url = u'' if not attrs.has_key('class'): attrs['class'] = 'vForeignKeyRawIdAdminField' # The JavaScript looks for this hook. output = [super(ForeignKeyRawIdWidget, self).render(name, value, attrs)] # TODO: "id_" is hard-coded here. This should instead use the correct # API to determine the ID dynamically. - output.append(' ' % \ + output.append(u' ' % \ (related_url, url, name)) - output.append('%s' % (settings.ADMIN_MEDIA_PREFIX, _('Lookup'))) + output.append(u'%s' % (settings.ADMIN_MEDIA_PREFIX, _('Lookup'))) if value: output.append(self.label_for_value(value)) return mark_safe(u''.join(output)) def base_url_parameters(self): - params = {} - if self.rel.limit_choices_to and hasattr(self.rel.limit_choices_to, 'items'): - items = [] - for k, v in self.rel.limit_choices_to.items(): - if isinstance(v, list): - v = ','.join([str(x) for x in v]) - else: - v = str(v) - items.append((k, v)) - params.update(dict(items)) - return params + return url_params_from_lookup_dict(self.rel.limit_choices_to) def url_parameters(self): from django.contrib.admin.views.main import TO_FIELD_VAR diff --git a/django/db/models/fields/related.py b/django/db/models/fields/related.py index 795292f279..115bd87826 100644 --- a/django/db/models/fields/related.py +++ b/django/db/models/fields/related.py @@ -745,6 +745,8 @@ class ForeignKey(RelatedField, Field): def contribute_to_related_class(self, cls, related): setattr(cls, related.get_accessor_name(), ForeignRelatedObjectsDescriptor(related)) + if self.rel.limit_choices_to: + cls._meta.related_fkey_lookups.append(self.rel.limit_choices_to) def formfield(self, **kwargs): defaults = { diff --git a/django/db/models/options.py b/django/db/models/options.py index 34dd2aac34..0fd752fb48 100644 --- a/django/db/models/options.py +++ b/django/db/models/options.py @@ -53,6 +53,10 @@ class Options(object): self.abstract_managers = [] self.concrete_managers = [] + # List of all lookups defined in ForeignKey 'limit_choices_to' options + # from *other* models. Needed for some admin checks. Internal use only. + self.related_fkey_lookups = [] + def contribute_to_class(self, cls, name): from django.db import connection from django.db.backends.util import truncate_name diff --git a/tests/regressiontests/admin_views/models.py b/tests/regressiontests/admin_views/models.py index ded5266d0b..bce39d8846 100644 --- a/tests/regressiontests/admin_views/models.py +++ b/tests/regressiontests/admin_views/models.py @@ -146,6 +146,26 @@ class Thing(models.Model): class ThingAdmin(admin.ModelAdmin): list_filter = ('color',) +class Actor(models.Model): + name = models.CharField(max_length=50) + age = models.IntegerField() + def __unicode__(self): + return self.name + +class Inquisition(models.Model): + expected = models.BooleanField() + leader = models.ForeignKey(Actor) + def __unicode__(self): + return self.expected + +class Sketch(models.Model): + title = models.CharField(max_length=100) + inquisition = models.ForeignKey(Inquisition, limit_choices_to={'leader__name': 'Palin', + 'leader__age': 27, + }) + def __unicode__(self): + return self.title + class Fabric(models.Model): NG_CHOICES = ( ('Textured', ( @@ -519,6 +539,9 @@ admin.site.register(Section, save_as=True, inlines=[ArticleInline]) admin.site.register(ModelWithStringPrimaryKey) admin.site.register(Color) admin.site.register(Thing, ThingAdmin) +admin.site.register(Actor) +admin.site.register(Inquisition) +admin.site.register(Sketch) admin.site.register(Person, PersonAdmin) admin.site.register(Persona, PersonaAdmin) admin.site.register(Subscriber, SubscriberAdmin) diff --git a/tests/regressiontests/admin_views/tests.py b/tests/regressiontests/admin_views/tests.py index 9c0bed2d91..f699acf85b 100644 --- a/tests/regressiontests/admin_views/tests.py +++ b/tests/regressiontests/admin_views/tests.py @@ -300,6 +300,17 @@ class AdminViewBasicTest(TestCase): except SuspiciousOperation: self.fail("Filters should be allowed if they involve a local field without the need to whitelist them in list_filter or date_hierarchy.") + def test_allowed_filtering_15103(self): + """ + Regressions test for ticket 15103 - filtering on fields defined in a + ForeignKey 'limit_choices_to' should be allowed, otherwise raw_id_fields + can break. + """ + try: + self.client.get("/test_admin/admin/admin_views/inquisition/?leader__name=Palin&leader__age=27") + except SuspiciousOperation: + self.fail("Filters should be allowed if they are defined on a ForeignKey pointing to this model") + class SaveAsTests(TestCase): fixtures = ['admin-views-users.xml','admin-views-person.xml']