Skip to content

Suggest: do not embed the child listing of folderish suggestions (#105) - #106

Open
reekitconcept wants to merge 1 commit into
mainfrom
kid/105/suggest-include-items
Open

Suggest: do not embed the child listing of folderish suggestions (#105)#106
reekitconcept wants to merge 1 commit into
mainfrom
kid/105/suggest-include-items

Conversation

@reekitconcept

Copy link
Copy Markdown
Member

Fixes #105.

What

SolrSuggest.serialize_brain() serializes a small set of portal types in full
via ISerializeToJson rather than from the catalog brain. For any Dexterity
container, plone.restapi resolves that to SerializeFolderToJson, which
defaults to include_items=True. So for a folderish suggest type, every single
suggestion:

  1. ran an extra catalog query over that object's children, and
  2. carried the complete child listing (items, items_total, batching) in
    the response.

Nothing in the suggest UI consumes those keys — it renders a title, a type and
a link.

Why it matters

Suggest is a type-ahead endpoint: it fires on nearly every keystroke, and
the cost multiplies by keystrokes x suggestions. It degrades exactly the
interaction that is supposed to feel instant, and it does so silently — no
error, no log entry, just a fat response and extra catalog work per request.

The Member branch is not dead code; the frontend has a matching render branch
in SolrSearchWidget.jsx. And folderish suggest types are not exotic:
collective.person's Person is a Container (it accepts File and
Image children), so a person-directory intranet that surfaces people in the
suggest dropdown lands here directly, with the payload growing with each
attachment on each person.

How

Pass include_items=False explicitly. Plain SerializeToJson accepts and
ignores the keyword, so the call stays correct for both folderish and
non-folderish types — no type check needed, and non-folderish types are
completely unaffected.

The inline ["Member"] literal is lifted into a module-level
FULL_SERIALIZATION_TYPES constant. That names the behaviour, and it lets the
test cover the folderish path without adding a Member content type to the
fixtures.

Tests

New backend/tests/services/suggest/test_suggest_serialize_brain.py, three
cases. They exercise serialize_brain() directly and only request the
integration fixture, so they need no running Solr despite living under
tests/services/suggest/.

Document stands in for the folderish suggest type: with plone.volto applied
it is a Container, so it genuinely routes through SerializeFolderToJson
the first assertion (is_folderish is True) pins that down, so the
"no items" assertions cannot pass vacuously.

Verified the test actually catches the bug: reverting just the
include_items=False argument makes test_full_serialization_omits_the_child_listing
fail on assert "items" not in data. With the fix, all three pass.

Note / known limitation

SerializeFolderToJson lets the request override the keyword:

include_items = self.request.form.get("include_items", include_items)

So /@solr-suggest?query=x&include_items=true still returns the items. That is
a caller opting in explicitly rather than an accidental default, so this PR
does not defend against it — recording it here so it is not a surprise later.

Scope

Small and self-contained; no API change for any existing consumer. Intended for
the batch of small fixes going into the upcoming release candidate — please
review independently, not to be merged as part of anything else.

`serialize_brain()` serializes a small set of portal types in full via
`ISerializeToJson` instead of from the catalog brain. plone.restapi resolves
that to `SerializeFolderToJson` for any Dexterity container, which defaults to
`include_items=True` - so for a folderish type every suggestion ran an extra
catalog query and carried its complete child listing (`items`, `items_total`,
`batching`) in the response.

Nothing in the suggest UI consumes those keys, and suggest is a type-ahead
endpoint, so the waste multiplies by keystrokes x suggestions on the one
interaction that is meant to feel instant.

Pass `include_items=False`. Plain `SerializeToJson` accepts and ignores the
keyword, so no type check is needed and non-folderish types are unaffected.

Also lift the inline `["Member"]` literal into `FULL_SERIALIZATION_TYPES`, so
the behaviour is named and the tests can cover it without adding a `Member`
content type to the fixtures.
@reekitconcept
reekitconcept force-pushed the kid/105/suggest-include-items branch from 82ec254 to 3ec581b Compare August 6, 2026 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Suggest: full serialization of folderish types embeds the whole child listing in every suggestion

1 participant