Skip to content

Move remaining coach plugin API code into kolibri/plugins/coach/viewsets/ #14819

@rtibbles

Description

@rtibbles

This issue is not open for contribution. Visit Contributing guidelines to learn about the contributing process and how to find suitable issues.

Overview

Move the coach plugin's remaining API code — api.py, class_summary_api.py, unit_report_api.py, unit_lesson_progress_api.py, and serializers.py — into a kolibri/plugins/coach/viewsets/ package, one module per domain, deleting the old modules. This is the cleanup issue for the coach plugin under the File Organization convention in #14036; it covers everything except the classroom notifications domain, which moves with its migration in #14311.

Complexity: Medium
Target branch: develop

Context

The coach plugin already deviates from the single-module pattern — its API code spans four modules plus serializers.py:

  • api.py: LessonReportViewset (serializer in serializers.py), the difficult-questions family (BaseExerciseDifficultQuestionsViewset, ExerciseDifficultQuestionsViewset, QuizDifficultQuestionsViewset, PracticeQuizDifficultQuestionsViewset), and the classroom notifications domain (moving in Migrate ClassroomNotificationsViewset to serializer-derived values pattern #14311)
  • class_summary_api.py: ClassSummaryViewSet plus its serialization helper functions
  • unit_report_api.py: UnitReportViewSet plus test-score computation helpers
  • unit_lesson_progress_api.py: UnitLessonProgressViewSet

Apart from ClassroomNotificationsViewset, none of these use the ValuesViewset serialization path, so this is a pure move with no serialization changes. Permission classes, filters, and module-level helpers move with their viewsets. A natural split is lesson_report.py, difficult_questions.py, class_summary.py, unit_report.py, and unit_lesson_progress.py; the final split is decided in the PR.

The Change

Move the remaining coach API code into kolibri/plugins/coach/viewsets/, one module per domain, with LessonReportSerializer inline above LessonReportViewset. Update imports in api_urls.py and anywhere else that references the moved classes, then delete api.py, class_summary_api.py, unit_report_api.py, unit_lesson_progress_api.py, and serializers.py. Land each module's relocation as a pure-move commit, separate from import updates.

Moving code breaks GitHub blame (local git blame -C -C still works), so digest the history the move obscures:

  1. Run blame over the code being moved; flag hunks whose purpose isn't evident from the code or tests (odd guards, magic orderings, special cases).
  2. Chase each flagged hunk to its originating PR/issue; where the rationale is non-obvious and load-bearing, carry it forward as a code comment citing the reference.
  3. Where the rationale turns out to be obsolete, flag the code for removal — but as a pure move, this PR should not remove it; raise it for follow-up instead.

How to Get There

All affected endpoints are routed through the router in kolibri/plugins/coach/api_urls.py (lessonreport, classsummary, the difficult-questions routes, unitreport, etc.) — the single routing module whose imports this move updates.

Out of Scope

  • Any change to endpoint behavior or response shape — this is a pure move
  • The classroom notifications domain (ClassroomNotificationsViewset, its filter and permissions) — moves with its migration in Migrate ClassroomNotificationsViewset to serializer-derived values pattern #14311
  • Migrating any coach viewset to the serializer-derived pattern — none of the moved viewsets use the ValuesViewset serialization path
  • Removing obsolete code found during blame digestion — flag for follow-up instead

Acceptance Criteria

  • kolibri/plugins/coach/viewsets/ contains all remaining coach API viewsets, permissions, filters, and helper functions, one module per domain, with LessonReportSerializer inline above LessonReportViewset
  • api.py, class_summary_api.py, unit_report_api.py, unit_lesson_progress_api.py, and serializers.py are deleted
  • All references updated (api_urls.py, tests, any cross-module imports)
  • Each relocation lands as a pure-move commit, separate from import updates
  • Existing API tests pass unchanged
  • Comments citing the originating PR/issue added where blame digestion found non-obvious, load-bearing rationale

Testing

  • pytest kolibri/plugins/coach/test/ must pass unchanged — test_class_summary.py, test_difficult_questions.py, test_lesson_report.py, test_unit_report.py, and test_unit_lesson_progress.py cover all moved endpoints.

AI usage

Drafted with AI assistance (Claude Code) from an interactive design discussion, section by section, with review and final edits by @rtibbles. Module contents, routing, and test coverage were verified against the codebase during drafting.

Metadata

Metadata

Assignees

Labels

APP: CoachRe: Coach App (lessons, quizzes, groups, reports, etc.)DEV: backendPython, databases, networking, filesystem...TAG: tech update / debtChange not visible to user

Type

No fields configured for Task.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions