dnscms: restore admin image search, keep it out of the public api

dnscms/dnscms/wagtail_hooks.py

@@ -1,6 +1,10 @@
+import graphene
+
 from django.templatetags.static import static
 from django.utils.html import format_html
 from grapple.registry import registry as grapple_registry
+from grapple.types.images import get_image_type
+from grapple.types.structures import QuerySetList
 from wagtail import hooks
 from wagtail.models import Page
 from wagtail.search.backends import get_search_backend
@@ -18,13 +22,21 @@ def register_button_link(features):
     register_button_link_feature(features)
 
 
-@hooks.register("register_schema_query")
+# Both schema hooks below run at order=1, i.e. after Grapple's own
-def override_search_resolver(query_mixins):
+# register_schema_query hook, because Grapple populates parts of its type
+# registry from inside that hook (the base `Page` type is registered by
+# `PagesQuery()`). They still insert their mixin at the front of the list so it
+# wins the MRO against Grapple's.
+@hooks.register("register_schema_query", order=1)
+def override_search_field(query_mixins):
     """
-    Override Grapple's `search` resolver. Two fixes vs. the upstream version:
+    Override Grapple's `search` field and resolver. Three fixes vs. upstream:
-    1. Restrict pages to live + public so drafts and access-restricted pages
+    1. Narrow the result union to page types. Upstream builds it from the whole
+       Grapple registry, so images, renditions and snippets are advertised as
+       possible search results even though nothing ever returns one.
+    2. Restrict pages to live + public so drafts and access-restricted pages
        don't leak via the public API.
-    2. Run a single search across all `Page` subclasses (instead of iterating
+    3. Run a single search across all `Page` subclasses (instead of iterating
        per-model) so results are ranked by relevance across types rather than
        grouped by content type. Specific instances are fetched in a second
        bulk query and reordered to match the search ranking.
@@ -33,10 +45,16 @@ def override_search_resolver(query_mixins):
     includes them, but the frontend search page only renders Page types and
     discards everything else, so iterating those indexes is wasted work.
     """
-    if not grapple_registry.class_models:
+    if not grapple_registry.pages:
         return
 
+    class Search(graphene.Union):
+        class Meta:
+            types = tuple(grapple_registry.pages.values())
+
     class SearchOverrideMixin:
+        search = graphene.List(graphene.NonNull(Search), query=graphene.String(), required=True)
+
         def resolve_search(self, info, **kwargs):
             query = kwargs.get("query")
             if not query:
@@ -52,6 +70,29 @@ def override_search_resolver(query_mixins):
     query_mixins.insert(0, SearchOverrideMixin)
 
 
+@hooks.register("register_schema_query", order=1)
+def disable_public_image_search(query_mixins):
+    """
+    Re-declare Grapple's root `images` field with search turned off.
+
+    `CustomImage` is indexed so that the image listing and chooser in the admin
+    have a working search box. Grapple hands any indexed model a `searchQuery`
+    argument on its list field, which would publish that index through the
+    public API, so the field is declared here without it. Grapple's own
+    `resolve_images` still serves it — only the argument list changes.
+    """
+
+    class ImagesNoSearchMixin:
+        images = QuerySetList(
+            graphene.NonNull(get_image_type()),
+            enable_search=False,
+            required=True,
+            collection=graphene.Argument(graphene.ID, description="Filter by collection id"),
+        )
+
+    query_mixins.insert(0, ImagesNoSearchMixin)
+
+
 @hooks.register("construct_page_action_menu")
 def make_publish_default_action(menu_items, request, context):
     for index, item in enumerate(menu_items):

dnscms/images/models.py

@@ -48,8 +48,6 @@ class CustomImage(AbstractImage):
         GraphQLString("attribution"),
     ]
 
-    search_fields = []
-
 
 class Rendition(AbstractRendition):
     image = models.ForeignKey(CustomImage, on_delete=models.CASCADE, related_name="renditions")

dnscms/tests/test_image_search.py

@@ -0,0 +1,64 @@
+from wagtail.search.index import class_is_indexed
+
+from images.models import CustomImage
+from tests.conftest import CustomImageFactory
+
+
+def test_image_model_is_indexed():
+    assert class_is_indexed(CustomImage)
+
+
+def test_admin_image_listing_renders_a_search_box(admin_client, db):
+    response = admin_client.get("/admin/images/")
+
+    assert response.status_code == 200
+    assert 'name="q"' in response.content.decode()
+
+
+def test_admin_image_listing_search_filters_results(admin_client, db):
+    CustomImageFactory(title="Konsertbilde fra Betong")
+    CustomImageFactory(title="Portrett av Jahn Teigen")
+
+    response = admin_client.get("/admin/images/", {"q": "Betong"})
+
+    assert [image.title for image in response.context["page_obj"]] == ["Konsertbilde fra Betong"]
+
+
+def test_admin_image_chooser_search_filters_results(admin_client, db):
+    CustomImageFactory(title="Konsertbilde fra Betong")
+    CustomImageFactory(title="Portrett av Jahn Teigen")
+
+    response = admin_client.get("/admin/images/chooser/results/", {"q": "Teigen"})
+
+    assert [image.title for image in response.context["results"]] == ["Portrett av Jahn Teigen"]
+
+
+def test_public_images_field_has_no_search_argument(graphql_post):
+    _, body = graphql_post('{ __type(name: "Query") { fields { name args { name } } } }')
+    args = {
+        field["name"]: [arg["name"] for arg in field["args"]]
+        for field in body["data"]["__type"]["fields"]
+    }
+
+    assert "searchQuery" not in args["images"]
+    assert "searchOperator" not in args["images"]
+
+
+def test_search_union_only_advertises_page_types(graphql_post):
+    _, body = graphql_post('{ __type(name: "Search") { possibleTypes { name } } }')
+    members = {t["name"] for t in body["data"]["__type"]["possibleTypes"]}
+
+    assert "CustomImage" not in members
+    assert "Rendition" not in members
+    assert "EventPage" in members
+
+
+def test_public_search_does_not_return_images(db, graphql_post):
+    CustomImageFactory(title="Betong")
+
+    _, body = graphql_post(
+        "query($q: String) { search(query: $q) { __typename } }", {"q": "Betong"}
+    )
+
+    assert "errors" not in body
+    assert [result["__typename"] for result in body["data"]["search"]] == []

web/src/gql/graphql.ts

@@ -137,24 +137,14 @@ export type SearchQueryVariables = Exact<{
 export type SearchQuery = { results: Array<
     | { __typename: 'AssociationIndex', id: string | null, title: string, url: string | null }
     | { __typename: 'AssociationPage', id: string | null, title: string, url: string | null, excerpt: string | null, associationType: string | null, logo: { ' $fragmentRefs'?: { 'ImageFragment': ImageFragment } } | null }
-    | { __typename: 'ContactEntity' }
     | { __typename: 'ContactIndex', id: string | null, title: string, url: string | null }
-    | { __typename: 'CustomImage' }
-    | { __typename: 'EventCategory' }
     | { __typename: 'EventIndex', id: string | null, title: string, url: string | null }
-    | { __typename: 'EventOccurrence' }
-    | { __typename: 'EventOrganizer' }
-    | { __typename: 'EventOrganizerLink' }
     | { __typename: 'EventPage', id: string | null, title: string, url: string | null, subtitle: string | null, featuredImage: { ' $fragmentRefs'?: { 'ImageFragment': ImageFragment } } | null, occurrences: Array<{ start: string }> }
     | { __typename: 'GenericPage', id: string | null, title: string, url: string | null, lead: string | null }
     | { __typename: 'HomePage', id: string | null, title: string, url: string | null }
-    | { __typename: 'HomePageFeaturedEvents' }
     | { __typename: 'NewsIndex', id: string | null, title: string, url: string | null }
     | { __typename: 'NewsPage', id: string | null, title: string, url: string | null, excerpt: string | null, firstPublishedAt: string | null, featuredImage: { ' $fragmentRefs'?: { 'ImageFragment': ImageFragment } } | null }
-    | { __typename: 'OpeningHoursItem' }
-    | { __typename: 'OpeningHoursSet' }
     | { __typename: 'Page', id: string | null, title: string, url: string | null }
-    | { __typename: 'Rendition' }
     | { __typename: 'SponsorsPage', id: string | null, title: string, url: string | null }
     | { __typename: 'StudioPage', id: string | null, title: string, url: string | null }
     | { __typename: 'VenueIndex', id: string | null, title: string, url: string | null }