Stop ModelAdmin from failing when filtering over a foreign key relation

Supersedes #4998
As per https://github.com/wagtail/wagtail/pull/4998#issuecomment-471005219, the implementation of `lookup_allowed` is flawed and breaks on some valid lookups while allowing invalid ones. We are therefore better off removing that validation entirely.

CHANGELOG.txt

@@ -7,6 +7,7 @@ Changelog
  * Removed support for Python 3.4
  * Added support for `short_description` for field labels in modeladmin's `InspectView` (Wesley van Lee)
  * Rearranged SCSS folder structure to the client folder and split them approximately according to ITCSS. (Naomi Morduch Toubman, Jonny Scholes, Janneke Janssen, Hugo van den Berg)
+ * Fix: ModelAdmin no longer fails when filtering over a foreign key relation (Jason Dilworth, Matt Westcott)
 
 
 2.5 (xx.xx.xxxx) - IN DEVELOPMENT

CONTRIBUTORS.rst

@@ -363,6 +363,7 @@ Contributors
 * Katie Locke
 * Cassidy Brooke
 * dthompson86
+* Jason Dilworth
 
 Translators
 ===========

docs/releases/2.6.rst

@@ -21,7 +21,7 @@ Other features
 Bug fixes
 ~~~~~~~~~
 
- * ...
+ * ModelAdmin no longer fails when filtering over a foreign key relation (Jason Dilworth, Matt Westcott)
 
 
 Upgrade considerations

wagtail/contrib/modeladmin/tests/test_page_modeladmin.py

@@ -364,3 +364,16 @@ class TestHeaderBreadcrumbs(TestCase, WagtailTestUtils):
         position_of_header = content_str.index('<header')  # intentionally not closing tag
         position_of_breadcrumbs = content_str.index('<ul class="breadcrumb">')
         self.assertLess(position_of_header, position_of_breadcrumbs)
+
+
+class TestSearch(TestCase, WagtailTestUtils):
+    fixtures = ['test_specific.json']
+
+    def setUp(self):
+        self.login()
+
+    def test_lookup_allowed_on_parentalkey(self):
+        try:
+            self.client.get('/admin/tests/eventpage/?related_links__link_page__id__exact=1')
+        except AttributeError:
+            self.fail("Lookup on parentalkey raised AttributeError unexpectedly")

wagtail/contrib/modeladmin/views.py

@@ -3,8 +3,7 @@ from collections import OrderedDict
 from functools import reduce
 
 from django import forms
-from django.contrib.admin import FieldListFilter, widgets
-from django.contrib.admin.exceptions import DisallowedModelAdminLookup
+from django.contrib.admin import FieldListFilter
 from django.contrib.admin.options import IncorrectLookupParameters
 from django.contrib.admin.utils import (
     get_fields_from_path, label_for_field, lookup_needs_distinct, prepare_lookup_value, quote, unquote)
@@ -12,9 +11,8 @@ from django.contrib.auth.decorators import login_required
 from django.core.exceptions import ImproperlyConfigured, PermissionDenied, SuspiciousOperation
 from django.core.paginator import InvalidPage, Paginator
 from django.db import models
-from django.db.models.constants import LOOKUP_SEP
 from django.db.models.fields import FieldDoesNotExist
-from django.db.models.fields.related import ForeignObjectRel, ManyToManyField
+from django.db.models.fields.related import ManyToManyField
 from django.shortcuts import get_object_or_404, redirect
 from django.template.defaultfilters import filesizeformat
 from django.utils.decorators import method_decorator
@@ -290,54 +288,6 @@ class IndexView(WMABaseView):
 
         return queryset, use_distinct
 
-    def lookup_allowed(self, lookup, value):
-        # 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 self.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)
-        # This term can be ignored.
-        if len(parts) > 1 and parts[-1] in QUERY_TERMS:
-            parts.pop()
-
-        # Special case -- foo__id__exact and foo__id queries are implied
-        # if foo has been specifically included in the lookup list; so
-        # drop __id if it is the last part. However, first we need to find
-        # the pk attribute name.
-        rel_name = None
-        for part in parts[:-1]:
-            try:
-                field = self.model._meta.get_field(part)
-            except FieldDoesNotExist:
-                # Lookups on non-existent fields are ok, since they're ignored
-                # later.
-                return True
-            if hasattr(field, 'remote_field'):
-                if field.remote_field is None:
-                    # This property or relation doesn't exist, but it's allowed
-                    # since it's ignored in ChangeList.get_filters().
-                    return True
-                model = field.remote_field.model
-                rel_name = field.remote_field.get_related_field().name
-            elif isinstance(field, ForeignObjectRel):
-                model = field.model
-                rel_name = model._meta.pk.name
-            else:
-                rel_name = None
-        if rel_name and len(parts) > 1 and parts[-1] == rel_name:
-            parts.pop()
-
-        if len(parts) == 1:
-            return True
-        clean_lookup = LOOKUP_SEP.join(parts)
-        return clean_lookup in self.list_filter
-
     def get_filters_params(self, params=None):
         """
         Returns all params except IGNORED_PARAMS
@@ -356,11 +306,6 @@ class IndexView(WMABaseView):
         lookup_params = self.get_filters_params()
         use_distinct = False
 
-        for key, value in lookup_params.items():
-            if not self.lookup_allowed(key, value):
-                raise DisallowedModelAdminLookup(
-                    "Filtering by %s not allowed" % key)
-
         filter_specs = []
         if self.list_filter:
             for list_filter in self.list_filter: