Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 78 additions & 6 deletions kolibri/core/notifications/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@
from django.db.models import Sum
from django.db.models import When
from le_utils.constants import content_kinds
from le_utils.constants import exercises

from kolibri.core.content.models import ContentNode
from kolibri.core.courses.models import CourseSession
from kolibri.core.courses.models import CourseSessionAssignment
from kolibri.core.courses.models import UnitTestAssignment
from kolibri.core.exams.models import Exam
from kolibri.core.exams.models import ExamAssignment
from kolibri.core.lessons.models import Lesson
Expand All @@ -18,6 +20,7 @@
from kolibri.core.logger.models import ExamAttemptLog
from kolibri.core.logger.models import ExamLog
from kolibri.core.logger.models import MasteryLog
from kolibri.core.logger.utils.pre_post_test import get_synthetic_content_id
from kolibri.core.logger.utils.quiz import annotate_response_summary
from kolibri.core.query import annotate_array_aggregate

Expand Down Expand Up @@ -908,28 +911,97 @@ def parse_attemptslog(attemptlog, contentnode_id=None, course_session_id=None):
save_notifications(notifications)


_IS_COACH_MONITORED_QUIZ = Q(

@rtibblesbot rtibblesbot Sep 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved — addressed in the current code.

praise: Paired constants make the exclude/filter complementarity structural.

masterylog__mastery_criterion__contains="coach_assigned"
) | Q(masterylog__mastery_criterion__contains=exercises.PRE_POST_TEST)
_IS_COACH_MONITORED_QUIZ_MASTERYLOG = Q(
mastery_criterion__contains="coach_assigned"
) | Q(mastery_criterion__contains=exercises.PRE_POST_TEST)


def _resolve_prepost_test_course_session_id(user, content_id, cache=None):
"""
A pre/post test's content_id is a one-way hash of
(course_session_id, unit_id, test_type) - see get_synthetic_content_id -
so it can't be decoded back into a course_session_id directly. Instead,
reverse-match it against the small set of UnitTestAssignment rows
reachable via the user's classroom memberships.

Returns the matching course_session_id, or None if none matches (e.g.
the assignment has since been deleted).
"""
cache_key = (user.id, content_id)
Comment thread
marcellamaki marked this conversation as resolved.
if cache is not None and cache_key in cache:
return cache[cache_key]

candidate_collection_ids = user.memberships.all().values_list(
"collection_id", flat=True
)
candidates = (
UnitTestAssignment.objects.filter(collection_id__in=candidate_collection_ids)
.values("course_session_id", "unit_contentnode_id", "test_type")
.distinct()
)
result = None
for candidate in candidates:
if (
get_synthetic_content_id(
candidate["course_session_id"],
candidate["unit_contentnode_id"],
candidate["test_type"],
)
== content_id
):
result = candidate["course_session_id"]
break

if cache is not None:
cache[cache_key] = result
return result


def batch_process_attemptlogs(attemptlog_ids):
for attemptlog in AttemptLog.objects.filter(id__in=attemptlog_ids).exclude(
masterylog__mastery_criterion__contains="coach_assigned"
_IS_COACH_MONITORED_QUIZ

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: Fixes #15189 closes an issue whose resource half stays broken and untracked.

Step 4 of #15189's repro is "complete a pre test (or post test or mark a resource as completed)". The PR body defers the resource case, which I'm taking as a deliberate scoping call — the problem is the closing keyword. batch_process_summarylogs and this parse_attemptslog path both go through get_assignments, a Lesson-only lookup whose dicts never carry a course_session_id, so every notification they produce has course_session_id = None. Both modes of useCourseNotificationPolling require it truthy, so those can never trigger a course refresh.

Either file the follow-up issue and link it here, or change to Refs #15189 so QA closes it once both halves land.

):
parse_attemptslog(attemptlog)


def batch_process_masterylogs_for_quizzes(masterylog_ids, attemptlog_ids):
course_session_cache = {}
for attemptlog in (
AttemptLog.objects.filter(id__in=attemptlog_ids)
.filter(masterylog__mastery_criterion__contains="coach_assigned")
.filter(_IS_COACH_MONITORED_QUIZ)
.select_related("masterylog", "user")
.annotate(quiz_id=F("masterylog__summarylog__content_id"))
.order_by("start_timestamp")
):
quiz_answered_notification(attemptlog, attemptlog.quiz_id)
course_session_id = None
if (
attemptlog.masterylog.mastery_criterion.get("type")
Comment thread
marcellamaki marked this conversation as resolved.
== exercises.PRE_POST_TEST
):
course_session_id = _resolve_prepost_test_course_session_id(
attemptlog.user, attemptlog.quiz_id, cache=course_session_cache
Comment thread
marcellamaki marked this conversation as resolved.
)
if course_session_id is None:
continue
quiz_answered_notification(attemptlog, attemptlog.quiz_id, course_session_id)
for masterylog in (
MasteryLog.objects.filter(id__in=masterylog_ids)
.filter(mastery_criterion__contains="coach_assigned")
.filter(_IS_COACH_MONITORED_QUIZ_MASTERYLOG)
.select_related("user")
.annotate(quiz_id=F("summarylog__content_id"))
):
quiz_started_notification(masterylog, masterylog.quiz_id)
quiz_completed_notification(masterylog, masterylog.quiz_id)
course_session_id = None
Comment thread
marcellamaki marked this conversation as resolved.
Comment thread
marcellamaki marked this conversation as resolved.
if masterylog.mastery_criterion.get("type") == exercises.PRE_POST_TEST:
course_session_id = _resolve_prepost_test_course_session_id(
masterylog.user, masterylog.quiz_id, cache=course_session_cache
)
if course_session_id is None:
continue
quiz_started_notification(masterylog, masterylog.quiz_id, course_session_id)
Comment thread
marcellamaki marked this conversation as resolved.

@rtibblesbot rtibblesbot Sep 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved — addressed in the current code.

suggestion: re-raising — the synthetic quiz_id still breaks the classSummary and Coach Home paths. classSummary/actions.js:30 reloads whenever !examMap[quiz_id], and a synthetic pre/post-test content_id can never be an Exam id, so every poll carrying one triggers a full /coach/api/classsummary/ fetch that can never satisfy the check; coachNotifications/getters.js:42 drops the same notifications, so Coach Home reads "No activity in your class". Gating both on notification.course_session_id being set is the smaller change, but the card routing (notificationLink()EXAM_SUMMARY/QUIZ_LEARNER_PAGE_ROOT) has to move to course_session_id at the same time or the cards render blank.

quiz_completed_notification(masterylog, masterylog.quiz_id, course_session_id)


def batch_process_examlogs(examlog_ids, examattemptlog_ids):
Expand Down
126 changes: 126 additions & 0 deletions kolibri/core/notifications/test/test_api.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import uuid
from datetime import timedelta

from django.db import connection
from django.test.utils import CaptureQueriesContext
from le_utils.constants import content_kinds
from le_utils.constants import exercises
from mock import patch
from rest_framework.test import APITestCase

Expand All @@ -13,6 +16,8 @@
from kolibri.core.content.models import ContentNode
from kolibri.core.courses.models import CourseSession
from kolibri.core.courses.models import CourseSessionAssignment
from kolibri.core.courses.models import TestType
from kolibri.core.courses.models import UnitTestAssignment
from kolibri.core.exams.models import Exam
from kolibri.core.exams.models import ExamAssignment
from kolibri.core.lessons.models import Lesson
Expand Down Expand Up @@ -1986,6 +1991,13 @@ def setUpTestData(cls):
cls.synthetic_content_id = get_synthetic_content_id(
cls.course_session.id, cls.unit_node.id, "pre"
)
cls.unit_test_assignment = UnitTestAssignment.objects.create(
course_session=cls.course_session,
unit_contentnode_id=cls.unit_node.id,
collection=cls.classroom,
test_type=TestType.Pre,
activated_by=cls.superuser,
)

def _create_prepost_masterylog(self, complete=False):
summarylog = ContentSummaryLogFactory.create(
Expand All @@ -2002,6 +2014,11 @@ def _create_prepost_masterylog(self, complete=False):
mastery_level=-1,
complete=complete,
completion_timestamp=now if complete else None,
mastery_criterion={
"type": exercises.PRE_POST_TEST,
"version": "A",
"test_type": "pre",
},
)
return masterylog

Expand Down Expand Up @@ -2200,6 +2217,115 @@ def test_prepost_test_completed_not_duplicated(self):
== 1
)

def test_batch_process_masterylogs_for_quizzes_creates_prepost_notifications(self):
"""
Regression test: the sync-time batch processor (used by the morango
post-transfer hook to regenerate notifications for an LOD sync) used
to filter on mastery_criterion__contains="coach_assigned", which
pre/post-test masterylogs never have, silently dropping them. It
should now recover the course_session_id via UnitTestAssignment
reverse-matching and create correctly-tagged notifications.
"""
masterylog = self._create_prepost_masterylog(complete=True)
batch_process_masterylogs_for_quizzes([masterylog.id], [])

started = LearnerProgressNotification.objects.filter(
quiz_id=self.synthetic_content_id,
notification_event=NotificationEventType.Started,
)
assert started.count() == 1
assert started[0].course_session_id == self.course_session.id
assert started[0].classroom_id == self.classroom.id

completed = LearnerProgressNotification.objects.filter(
quiz_id=self.synthetic_content_id,
notification_event=NotificationEventType.Completed,
)
assert completed.count() == 1
assert completed[0].course_session_id == self.course_session.id
assert completed[0].classroom_id == self.classroom.id

def test_batch_process_masterylogs_for_quizzes_no_notification_without_assignment(
Comment thread
marcellamaki marked this conversation as resolved.
self,
):
"""
If no UnitTestAssignment matches the synthetic content_id (e.g. the
assignment was deleted, or this isn't actually a course pre/post
test), no course_session_id can be recovered - no notification
should be created rather than one with a broken/empty classroom_id.
"""
self.unit_test_assignment.delete()
masterylog = self._create_prepost_masterylog(complete=True)
batch_process_masterylogs_for_quizzes([masterylog.id], [])

assert not LearnerProgressNotification.objects.filter(
quiz_id=self.synthetic_content_id,
).exists()

def test_batch_process_masterylogs_for_quizzes_creates_prepost_answered(self):
masterylog = self._create_prepost_masterylog()
sessionlog = ContentSessionLogFactory.create(
user=self.user1,
content_id=self.synthetic_content_id,
channel_id=None,
kind=content_kinds.QUIZ,
)
now = local_now()
attemptlog = AttemptLog.objects.create(
masterylog=masterylog,
sessionlog=sessionlog,
user=self.user1,
item="test_item",
start_timestamp=now,
end_timestamp=now + timedelta(seconds=5),
time_spent=5.0,
complete=True,
correct=1,
)
batch_process_masterylogs_for_quizzes([], [attemptlog.id])

answered = LearnerProgressNotification.objects.filter(
quiz_id=self.synthetic_content_id,
notification_event=NotificationEventType.Answered,
)
assert answered.count() == 1
assert answered[0].course_session_id == self.course_session.id
assert answered[0].classroom_id == self.classroom.id

def test_batch_process_masterylogs_for_quizzes_reuses_course_session_lookup(self):
Comment thread
marcellamaki marked this conversation as resolved.
masterylog = self._create_prepost_masterylog()
sessionlog = ContentSessionLogFactory.create(
user=self.user1,
content_id=self.synthetic_content_id,
channel_id=None,
kind=content_kinds.QUIZ,
)
now = local_now()
attemptlogs = [
AttemptLog.objects.create(
masterylog=masterylog,
sessionlog=sessionlog,
user=self.user1,
item="test_item_{}".format(i),
start_timestamp=now,
end_timestamp=now + timedelta(seconds=5),
time_spent=5.0,
complete=True,
correct=1,
)
for i in range(3)
]

with CaptureQueriesContext(connection) as ctx:
Comment thread
marcellamaki marked this conversation as resolved.
batch_process_masterylogs_for_quizzes([], [a.id for a in attemptlogs])

unit_test_assignment_queries = [
q
for q in ctx.captured_queries
if "courses_unittestassignment" in q["sql"].lower()
]
assert len(unit_test_assignment_queries) == 1


class CourseSessionRegressionTestCase(APITestCase):
"""
Expand Down
Loading
Loading