metadata: include ids in current-group list field (fix "None" publisher chip)#725
Merged
metadata: include ids in current-group list field (fix "None" publisher chip)#725
Conversation
When viewing metadata for a top-level group (publisher, imprint, series,
volume, folder, story arc), `_highlight_current_group` populated the
`*_list` field with `.values("name")` only. The frontend's
`toVuetifyItem` requires `ids` (or `pk`) to build chip values; without
them every entry collapsed into a single "None" chip and single-id
chips silently lost their click target.
Mirror the shape used by `_query_groups` for parent groups:
group_by(name) + JsonGroupArray("id") so each entry carries its ids.
Also fixes two latent bugs in the same code path:
- Volume's `number_to` was never selected, so `formattedVolumeName`
always rendered without the range suffix.
- `StoryArc.__name__.lower()` produced `storyarc_list`, but the
serializer reads `story_arc_list` — the data was being attached to
a field name nothing read. Map it via _GROUP_LIST_FIELD_OVERRIDES.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4 tasks
ajslater
added a commit
that referenced
this pull request
May 5, 2026
#726) The metadata view emitted ``*_list`` fields (publisher_list, imprint_list, series_list, volume_list, story_arc_list, folder_list) populated by two separate code paths that had to produce the same projection independently: ``_query_groups`` for parent groups and ``_highlight_current_group`` for the current group. PR #725 fixed the latter after it had drifted — formalize the contract in one place so it can't drift again. * New ``codex/views/browser/metadata/group_list.py`` exports ``annotate_group_list(qs)`` and ``group_list_field_name(model)``. * Both call sites now defer to the helpers; the duplicated ``only`` / ``group_by`` / ``JsonGroupArray`` chain and the ``_GROUP_LIST_FIELD_OVERRIDES`` map move with them. * Adds ``tests/test_metadata_group_list.py`` — HTTP regression tests pinning ``ids`` and ``number_to`` on the metadata response, plus unit tests on the field-name override map (covers the ``StoryArc`` → ``story_arc_list`` mapping that the HTTP path currently can't reach without a richer fixture). Pure refactor — behavior is identical to PR #725. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When viewing the metadata dialog for a top-level group (publisher, imprint, series, volume, folder, story arc), the current group's
*_listfield was emitted withoutids. The frontend'stoVuetifyItemrequiresids(orpk) to build chip values; without them every entry collapsed into a single "None" chip.Reproduced via
/api/v3/p/25,102/metadatareturning:…which the metadata dialog rendered as one "None" chip.
Root cause
_highlight_current_groupused a simplistic.values("name")query. The sibling helper_query_groups(which handles parent group lists) already had the right shape —group_by(name) + JsonGroupArray("id"). Only the current-group path was missing it.Fix
Mirror the
_query_groupsshape:group_by(*only)+JsonGroupArray("id", distinct=True, order_by="id"), projected via.values("ids", *only). Each entry now carries its aggregated ids and the frontend renders proper clickable chips.Latent bugs in the same code path, also fixed
While auditing for "similar components with similar data," three related defects shared the root cause and are resolved by the same edit:
MetadataTextreadsvalue.ids || [value.pk]; withoutids, the link evaluated to[undefined]and clicking did nothing.number_towas never selected, soformattedVolumeName(name, numberTo)always rendered without the range suffix.storyArcListwas always empty —StoryArc.__name__.lower()producedstoryarc_list, but the serializer readsstory_arc_list. The data was being attached to an attribute nothing read. Mapped via_GROUP_LIST_FIELD_OVERRIDES.Test plan
make fixcleanmake lintPython:0 errors, 0 warnings, 0 notes(the trailingremarkmarkdown error is pre-existing on develop, verified by stashing)uv run pytest tests/— 26 passedstoryArcListis populated🤖 Generated with Claude Code