Skip to content

Migrate the channel metadata viewsets to serializer derivation - #15180

Open
rtibblesbot wants to merge 7 commits into
learningequality:developfrom
rtibblesbot:issue-14312-87cd34
Open

Migrate the channel metadata viewsets to serializer derivation#15180
rtibblesbot wants to merge 7 commits into
learningequality:developfrom
rtibblesbot:issue-14312-87cd34

Conversation

@rtibblesbot

@rtibblesbot rtibblesbot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Summary

The internal and public channel metadata endpoints built their responses from an explicit values tuple, a field_map of renames and lambdas, and a consolidate() pass, so neither had a serializer describing what it returns. Both now derive their fields from a serializer pair — the public one keeps the raw base64 thumbnail column, the internal one returns a thumbnail URL — and the domain moves into its own module under content/viewsets/.

References

Fixes #14312. Unblocks #14300, which needs SplitTextField and content/utils/cache.py.

Acceptance criteria not met

Criterion Reason
values, field_map and consolidate() removed from BaseChannelMetadataMixin consolidate() survives for included_languages alone, taking the ordering escape hatch the issue grants — annotate_array_aggregate reorders the list on SQLite.

Reviewer guidance

  • kolibri/core/content/viewsets/channel_metadata.py:142 — holding the sort_value order of included_languages costs a second query. Is that element order contractual enough to buy?
  • kolibri/core/content/viewsets/channel_metadata.py:83last_published renders as a raw datetime matching last_updated only because KolibriModelSerializer maps the model's DateTimeTzField. Is that coupling intended?

Evidence

Viewset Time Queries Data hash
ChannelMetadataViewSet 1.887 → 0.911 ms 2 → 2 match
PublicChannelMetadataViewSet 5.445 → 1.230 ms 2 → 2 match
Suite Result
content, public, core, device — SQLite 1740 passed, 9 skipped
content, public — PostgreSQL 17 193 passed, 9 skipped

AI usage

Used Claude Code to write the characterization tests, migrate both viewsets to serializer derivation, and move the domain, following a pre-approved plan. Verified with the full test suite on SQLite and PostgreSQL, the viewset serialization benchmark, and prek.


@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly

How was this generated?
  • Ran pre-flight CI checks (lint, format, tests) and verified all pass
  • Rebased onto the target branch and resolved any conflicts
  • Reorganized commit history into clean, logical commits
  • Audited the diff to ensure only issue-relevant files are changed
  • Built PR body from the repository's PR template with evidence blocks
@rtibblesbot

🟡 Waiting for feedback

Last updated: 2026-08-12 03:28 UTC

@github-actions github-actions Bot added DEV: backend Python, databases, networking, filesystem... APP: Device Re: Device App (content import/export, facility-syncing, user permissions, etc.) SIZE: large labels Aug 12, 2026
@github-actions

github-actions Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

@rtibblesbot
rtibblesbot force-pushed the issue-14312-87cd34 branch 2 times, most recently from 9c72527 to 8adce42 Compare August 12, 2026 02:28
@rtibblesbot
rtibblesbot marked this pull request as ready for review August 12, 2026 02:54
rtibblesbot and others added 7 commits August 11, 2026 20:11
The internal and public channel endpoints have no assertions on
included_languages, included_categories, included_grade_levels,
last_published, thumbnail or record order, and the public one has no API
tests at all. These pin current behaviour and must pass unchanged after
the migration.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Replaces the _split_text_field lambdas the channel viewsets pass through
field_map. learningequality#14300 needs the same field for ContentNode.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
BaseChannelMetadataMixin drops its values tuple and field_map for a
serializer pair: the public viewset keeps the raw thumbnail column, the
internal one overrides it with the URL.

included_languages stays in consolidate(). A SQL array aggregate loses
the sort_value order SortedManyToManyField gives the relation, and the
GROUP BY it needs would also drop Meta.ordering on Django 3.2.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Its only consumer was the device plugin's viewset, a plain
ReadOnlyModelViewSet that needs the model-instance-based fields.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both this issue and learningequality#14300 need them importable from a content-side
module once the public viewsets move out of kolibri.core.public.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copy only — the originals stay in place and nothing imports this yet, so
git blame -C -C can follow the code across the move.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@rtibbles rtibbles self-assigned this Aug 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

APP: Device Re: Device App (content import/export, facility-syncing, user permissions, etc.) DEV: backend Python, databases, networking, filesystem... SIZE: large

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Migrate ChannelMetadata viewsets to serializer-derived values pattern

2 participants