Fix Course notifications

[marcellamaki]

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

Target branch: develop

Observed behavior

Pre/post-test notifications carry a synthetic quiz_id hash (see get_synthetic_content_id), not a real Exam id. Three coach-frontend paths assume otherwise:

• classSummary/actions.js: updateWithNotifications reloads the full class summary on !examMap[quiz_id]. A synthetic id never matches, so every pre/post-test notification triggers a full reload, every poll, forever.
• coachNotifications/getters.js: reshapeNotification drops the notification (return null) when classSummary.exams[quiz_id] misses. Pre/post-test activity never appears in the coach's activity feed.
• gettersUtils.js: notificationLink() routes Quiz notifications to Exam-only pages. Not reachable today (the getter above already drops these first) — but fixing the drop without also fixing this routes clicks into a 404.

Expected behavior

All three paths should branch on notification.course_session_id (present on all notifications — confirmed serialized in ClassroomNotificationsSerializer) before falling back to exam-specific logic:

• classSummary/actions.js: when notification.course_session_id is truthy, skip the !examMap[notification.quiz_id] check entirely (don't factor it into reloadSummary). All other checks in updateWithNotifications (learnerMap, lessonMap, contentNodeMap) are unaffected.
• coachNotifications/getters.js: when classSummary.exams[notification.quiz_id] misses but notification.course_session_id is truthy, render the notification with a generic label (e.g. "Course activity") instead of returning null. No new title data is fetched.
• gettersUtils.js: when notification.course_session_id is truthy, notificationLink() returns { name: PageNames.COURSE_SUMMARY, params: { classId: notification.collection.id, courseSessionId: notification.course_session_id } } instead of matching against pageNameToNotificationPropsMap.

User-facing consequences

Coaches can't see pre/post-test activity on the Coach Home activity feed, and classSummary silently reloads on every poll, which is a waste. Future fixes to feed visibility without updating the routing as well would cause errors.

Steps to reproduce

1. Assign a course to a classroom and activate a pre/post-test for a unit (as coach).
2. As a learner, start and complete the pre/post-test.
3. As coach, open Coach Home and watch the activity feed / network tab.
4. Observe: the classSummary API reloads on every notification poll for this activity (visible in network tab), and the pre/post-test completion never appears in the activity feed, even though the notification exists (confirmed via DB/store inspection).

Claim 3 (broken routing) isn't independently reproducible today — it only manifests once someone fixes claim 2 without also fixing routing, so no repro steps for it here.

Context

Application version: kolibri/0.20.0a2 (develop branch), same codebase as PR #15260.
Related: #15189, #15228, PR #15260 (this issue is item #5 from that PR's review, deferred as out of scope — fixing it means making shared coach-notification infrastructure course-aware, not something scoped to that PR's files).

Acceptance Criteria

• classSummary/actions.js's updateWithNotifications: reloadSummary is never set true via the exam-map check for a notification with course_session_id set; other checks in that function are unaffected.
• coachNotifications/getters.js's reshapeNotification: a notification with course_session_id set and no classSummary.exams match renders with a generic label instead of returning null.
• gettersUtils.js's notificationLink(): a notification with course_session_id set routes to PageNames.COURSE_SUMMARY with { classId: notification.collection.id, courseSessionId: notification.course_session_id }.
• Existing exam/lesson/resource notification behavior (reload triggers, feed rendering, routing) is unchanged for notifications without course_session_id.
• Tests cover all three branches for course_session_id-present notifications, plus regression coverage for the existing exam/lesson/resource paths.

AI usage

Drafted by Claude (Claude Code) from a code-review finding surfaced by an automated PR review bot (@rtibblesbot) on PR #15260. Claude independently verified all three claims by reading the cited source directly before drafting, including checking data availability (classSummary state has no course/unit title data — confirmed via direct code inspection) and the exact routing target (PageNames.COURSE_SUMMARY, notification.collection.id, serializer fields) before finalizing Expected Behavior and Acceptance Criteria. I edited/confirmed each section.

[rtibblesbot]

Verified all three claims against source before drafting these. Confirmed: synthetic quiz_id (progress_tracking.py:381, dispatched :558; asserted notifications/test/test_api.py:2019), course_session_id serialized (classroom_notifications.py:103), and COURSE_SUMMARY → /:classId/courses/:courseSessionId (routes/utils.js:43). Three things block implementation.

1. Resource/Lesson course notifications are broken identically — in or out of scope?

get_course_lesson_dict (core/notifications/api.py:138) sets lesson_id to the parent ContentNode id, not a Lesson id — asserted in test_api.py:1795,1808. That id is never in lessonMap/classSummary.lessons (those come from serialize_lessons), so course Resource and Lesson notifications hit the same two failures as the quiz path: permanent reload (actions.js:33) and feed drop (getters.js:44). contentNodeMap (actions.js:36) too.

Course browsing generates far more Resource/Lesson notifications than pre/post tests. But AC bullet 4 ("existing lesson/resource behavior is unchanged") reads as forbidding this. Fix all objects with course_session_id, or quizzes only?

2. The AC as written ships a TypeError

Skipping only the exam-map check still falls through to actions.js:55-65, which pushes exam_id: <synthetic>. APPLY_NOTIFICATION_UPDATES then dereferences state.examMap[update.exam_id].question_count (classSummary/index.js:435) → crash. contentNodeMap[...].content_id (actions.js:49) crashes the same way if the resource path is in scope.

Proposal: skip a course_session_id notification's status-map contribution entirely, not just the check — course pages get progress from useCourseSession, so examLearnerStatusMap shouldn't carry synthetic ids. Confirm?

3. What is the generic label, exactly?

reshapeNotification builds assignment from classSummary.exams[quiz_id].title (getters.js:75), which also throws on the course path — so assignment needs its own branch. What name/type/id?

And "Course activity" lands in cardTextForNotification as {itemName} inside quotes (notificationStrings.js:155): "Alice completed 'Course activity'". Options:

• a. Reword as new string variants — "{learnerName} completed an activity in a course".
• b. Keep {itemName} and accept the phrasing.
• c. Add unit_id to LearnerProgressNotification + serializer and render the real unit title (backend change, beyond the issue's stated scope). Confirmed the title is otherwise unrecoverable client-side: classSummary has no course data and the synthetic quiz_id is a hash.

Defaults if unanswered: all three objects, no status-map contribution, option (a).

[rtibbles]

1 In scope.
2 Confirmed.
3 I think it would be ideal to at least be able to distinguish a resource from a lesson, from a pre/post test here. We presumably have this information somewhere, so it would be important to include it, and then use this to generate the label, even without the title. However, if we have to make tweaks to return the type, we may as well return the title as well.

[rtibblesbot]

Answers resolved. Design below; the type/title source turned out to already exist, so no model change or migration.

Where the type and title come from

UnitTestAssignment(course_session, unit_contentnode_id, test_type) (core/courses/models.py:505) is the missing mapping — recomputing get_synthetic_content_id over a session's assignments recovers the unit node and pre/post behind a synthetic quiz_id. Resource and Lesson rows already carry real ContentNode ids (contentnode_id, and lesson_id = the parent node). So every course notification is resolvable today.

Notifications live in a separate SQLite DB (NotificationsRouter), so no join — resolution is a second query in consolidate().

Backend — classroom_notifications.py

Three read-only serializer fields, all in deferred_fields so consolidate() owns them: title, kind (card icon), test_type. Populated only for rows with course_session_id, in a bounded number of queries per page:

1. UnitTestAssignment.objects.filter(course_session_id__in=...) → recompute synthetic ids → match Quiz rows.
2. One ContentNode.objects.filter(id__in=...).values("id", "title", "kind") covering Resource → contentnode_id, Lesson → lesson_id, Quiz → the unit node from step 1.

Classic notifications stay null and keep taking their title from classSummary, so existing deployments see no added per-poll cost. Copy the str(...) argument form from unit_report.py:161 — get_synthetic_content_id string-formats its inputs, and that caller is already proven to match.

Frontend

1. classSummary/actions.js — skip a course_session_id row entirely (checks and status-map push), per answer 2.
2. coachNotifications/getters.js — branch the whole assignment block, not just the null-return: classSummary.exams[quiz_id].title (getters.js:75) throws on the course path too. Build it (and getResource's fallback) from title/kind/test_type.
3. gettersUtils.js — route to COURSE_SUMMARY.
4. notificationStrings.js — label variants per resource / lesson / pre-test / post-test, with a fallback for a null title (content deleted).

One correction to AC bullet 3

classId must come from notification.classroom_id, not notification.collection.id. collection.id is the LearnerGroup id whenever the learner matches an assignment group (getters.js:64) — which a course assigned to a group via CourseSessionAssignment hits directly. A group id in :classId routes into a 404. classroom_id is already serialized (classroom_notifications.py:93) and survives reshapeNotification's spread.

Proceeding on this unless you object.