From 5aac631c4aa21d6c5a18e0e14949bc2d219073a2 Mon Sep 17 00:00:00 2001 From: Jonas Braathen Date: Fri, 21 Aug 2026 01:22:03 +0200 Subject: [PATCH] dnscms: restore admin image search, keep it out of the public api --- dnscms/dnscms/wagtail_hooks.py | 53 ++++++++++++++++++++++--- dnscms/images/models.py | 2 - dnscms/tests/test_image_search.py | 64 +++++++++++++++++++++++++++++++ web/src/gql/graphql.ts | 10 ----- 4 files changed, 111 insertions(+), 18 deletions(-) create mode 100644 dnscms/tests/test_image_search.py diff --git a/dnscms/dnscms/wagtail_hooks.py b/dnscms/dnscms/wagtail_hooks.py index 4318074..21e8529 100644 --- a/dnscms/dnscms/wagtail_hooks.py +++ b/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") -def override_search_resolver(query_mixins): +# Both schema hooks below run at order=1, i.e. after Grapple's own +# 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: - 1. Restrict pages to live + public so drafts and access-restricted pages + Override Grapple's `search` field and resolver. Three fixes vs. upstream: + 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): diff --git a/dnscms/images/models.py b/dnscms/images/models.py index ad5b7f0..880a3a0 100644 --- a/dnscms/images/models.py +++ b/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") diff --git a/dnscms/tests/test_image_search.py b/dnscms/tests/test_image_search.py new file mode 100644 index 0000000..275bd45 --- /dev/null +++ b/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"]] == [] diff --git a/web/src/gql/graphql.ts b/web/src/gql/graphql.ts index 3adf94c..2f01697 100644 --- a/web/src/gql/graphql.ts +++ b/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 }