Feature/graphql sources 20260906 - #897
Conversation
* Update ConceptDocument to use `display_name` field instead of `name` for GraphQL projections * Ensure GraphQL resolvers and tests correctly utilize the `display_name` field in the Elasticsearch index * This change allows clients to retrieve the human-readable display name instead of the internal name when fetching concepts via GraphQL
|
Still draft, but updated, please consider check @snyaggarwal |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 091bfb0377
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| 'display': ('display_name',), | ||
| 'description': ('preferred_description',), |
There was a problem hiding this comment.
Reindex locale-derived fields after source locale changes
When a source's default_locale or supported_locales changes, these projections continue serving the previously indexed display_name and preferred_description, even though both values are derived from the parent source's locale configuration. Source persistence only reindexes concepts for release or match-algorithm changes, and the new signal propagation handles only activity and visibility, so a lean query requesting display or description remains stale indefinitely while the ORM path returns the new locale selection. Reindex the source's concepts when the relevant locale settings change, or avoid directly projecting these derived fields.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Don't think we have use case for it. Seems harmless, but I will create a ticket for it
There was a problem hiding this comment.
* Add `display_name` field to `ConceptDocument` projections * Refactor source projection URI to be rebuilt instead of stored to ensure data integrity and accurate URL generation
Solves: OpenConceptLab/ocl_issues#2757
Why? CIEL Lab needs improvements when loading worklists to load fast using ES-only retrieve for minimal payloads
Also: OpenConceptLab/ocl_issues#2577
GraphQL: permission-aware source and concept projections
Context
OCL's REST search latency is a bottleneck for several existing flows and blocks new ones from
being viable. Many searches only need a small, fixed payload (a display name, a description, a
few metadata fields) — hydrating a full ORM object and its relations for that is unnecessary
cost. This PR introduces a GraphQL path that serves these minimal payloads straight from
Elasticsearch, without touching the database, while keeping full-object retrieval available
for callers that need it.
Design principles
indexed filters alone, without a database round-trip. Whether to hydrate, and how much, is a
per-request decision driven by which fields the caller (source or concept payload) actually
selects — not a fixed retrieval mode for the whole query.
building blocks instead of duplicating them for GraphQL.
in one place and apply identically to both REST and GraphQL, wherever that's possible.
payload requested, GraphQL's retrieval path (index-only vs. hydrated) can legitimately differ
from REST's, even while reusing REST's search and authorization building blocks.
Documentation is free and detailed
Four fields are new on the search documents:
ConceptDocument.is_active,.is_head,.display_name, andSourceDocument.is_active. They're additive to the mapping, but existingdocuments don't have them populated, and Elasticsearch can't backfill them on its own.
If you deploy without reindexing first,
conceptssearches silently return zero results.The index projection filters on
is_active/is_head; old documents don't match, and azero-hit Elasticsearch response is not an error — it's
{"totalCount": 0, "results": []},indistinguishable from a legitimate empty search. (
sourcedegrades safely instead: missingis_activethere falls back to the database automatically.)Reindex before routing traffic:
Treat this as a pre-deploy gate, not cleanup — there's no error to notice afterward.
Known limitation
display_nameis derived from the source'sdefault_locale/supported_locales. Changing asource's locale configuration does not trigger a concept reindex (
persist_changesonlyreindexes concepts on
releasedor match-algorithm changes), so previously indexeddisplay_namevalues go stale until a manual reindex. Not fixed in this PR; scope kept to thepermission/projection work.
New GraphQL API surface
conceptIdsdoes exact, case-sensitive mnemonic matching, dedupes, preserves order, andtakes precedence over
query. Omit bothorgandsourcefor global search.page/limitgo together; result window is 10,000 (totalCountstill reflects the true total).versionresolves toHEAD, falling back to the latest release only ifHEADisabsent; an explicit missing version does not fall back.
Permissions
Reuses the existing REST visibility rule directly. Concept visibility relies on the indexed
public_can_viewflag, already kept in sync from the parent repository bycore/sources/signals.pywhenever the repository's access changes.Not indexed, by design
description— locale-resolved per concept; selecting it routes the whole requestthrough the ORM.
uri— rebuilt at query time via Django'sreverse()from indexedowner,owner_type,mnemonic,version, reproducing the ORM's percent-encoding exactly (verifiedagainst all sources in the database, including versions with reserved characters).
Compatibility
No database migrations, no new environment variables. Existing REST search relevance and
behavior are unchanged.
Testing
68 GraphQL tests + 249 concepts/sources regression tests passing. Integration suite runs
against real Elasticsearch with temporary indexes, covering zero-SQL projections, owner
isolation, HEAD/release selection, inactive/retired filtering, private-repository visibility,
the rebuilt source URI, and the database fallback for
description. Pylint clean (10.00/10).