-
Notifications
You must be signed in to change notification settings - Fork 4.3k
feat: receiver for verified exam event #33390
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,7 @@ | |
|
|
||
| from django.dispatch import receiver | ||
| from opaque_keys.edx.keys import LearningContextKey | ||
| from openedx_events.learning.signals import EXAM_ATTEMPT_VERIFIED | ||
| from submissions.models import score_reset, score_set | ||
| from xblock.scorable import ScorableXBlockMixin, Score | ||
|
|
||
|
|
@@ -25,7 +26,7 @@ | |
| from openedx.core.lib.grade_utils import is_score_higher_or_equal | ||
|
|
||
| from .. import events | ||
| from ..constants import ScoreDatabaseTableEnum | ||
| from ..constants import GradeOverrideFeatureEnum, ScoreDatabaseTableEnum | ||
| from ..course_grade_factory import CourseGradeFactory | ||
| from ..scores import weighted_score | ||
| from .signals import ( | ||
|
|
@@ -122,6 +123,10 @@ def submissions_score_reset_handler(sender, **kwargs): # pylint: disable=unused | |
| def disconnect_submissions_signal_receiver(signal): | ||
| """ | ||
| Context manager to be used for temporarily disconnecting edx-submission's set or reset signal. | ||
|
|
||
| Clear Student State on ORA problems currently results in a set->reset signal pair getting fired | ||
| from submissions which leads to tasks being enqueued, one of which can never succeed. This context manager | ||
| fixes the issue by disconnecting the "set" handler during the clear_state operation. | ||
| """ | ||
| if signal == score_set: | ||
| handler = submissions_score_set_handler | ||
|
|
@@ -300,3 +305,20 @@ def listen_for_course_grade_passed_first_time(sender, user_id, course_id, **kwar | |
| """ | ||
| events.course_grade_passed_first_time(user_id, course_id) | ||
| events.fire_segment_event_on_course_grade_passed_first_time(user_id, course_id) | ||
|
|
||
|
|
||
| @receiver(EXAM_ATTEMPT_VERIFIED) | ||
| def exam_attempt_verified_event_handler(sender, signal, **kwargs): # pylint: disable=unused-argument | ||
| """ | ||
| Consume `EXAM_ATTEMPT_VERIFIED` events from the event bus. This will trigger | ||
| an undo section override, if one exists. | ||
| """ | ||
| from ..api import should_override_grade_on_rejected_exam, undo_override_subsection_grade | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And this is also related to the convo yesterday about needing to do the import inline?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes! This is what we chatted about in the huddle - because the python API for this service is meant to be used both externally and internally, we end up with some circular dependencies. To avoid that, we have to use an inline import here |
||
|
|
||
| event_data = kwargs.get('exam_attempt') | ||
| user_data = event_data.student_user | ||
| course_key = event_data.course_key | ||
| usage_key = event_data.usage_key | ||
|
|
||
| if should_override_grade_on_rejected_exam(course_key): | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice! |
||
| undo_override_subsection_grade(user_data.id, course_key, usage_key, GradeOverrideFeatureEnum.proctoring) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,103 @@ | ||
| """ | ||
| Tests for the grades handlers | ||
| """ | ||
| from datetime import datetime, timezone | ||
| from unittest import mock | ||
| from uuid import uuid4 | ||
|
|
||
| import ddt | ||
| from django.test import TestCase | ||
| from opaque_keys.edx.keys import CourseKey, UsageKey | ||
| from openedx_events.data import EventsMetadata | ||
| from openedx_events.learning.data import ExamAttemptData, UserData, UserPersonalData | ||
| from openedx_events.learning.signals import EXAM_ATTEMPT_VERIFIED | ||
|
|
||
| from common.djangoapps.student.tests.factories import UserFactory | ||
| from lms.djangoapps.grades.signals.handlers import exam_attempt_verified_event_handler | ||
| from ..constants import GradeOverrideFeatureEnum | ||
|
|
||
|
|
||
| @ddt.ddt | ||
| class ExamCompletionEventBusTests(TestCase): | ||
| """ | ||
| Tests for exam events from the event bus | ||
| """ | ||
| @classmethod | ||
| def setUpClass(cls): | ||
| super().setUpClass() | ||
| cls.course_key = CourseKey.from_string('course-v1:edX+TestX+Test_Course') | ||
| cls.subsection_id = 'block-v1:edX+TestX+Test_Course+type@sequential+block@subsection' | ||
| cls.usage_key = UsageKey.from_string(cls.subsection_id) | ||
| cls.student_user = UserFactory( | ||
| username='student_user', | ||
| ) | ||
|
|
||
| @staticmethod | ||
| def _get_exam_event_data(student_user, course_key, usage_key, exam_type, requesting_user=None): | ||
| """ create ExamAttemptData object for exam based event """ | ||
| if requesting_user: | ||
| requesting_user_data = UserData( | ||
| id=requesting_user.id, | ||
| is_active=True, | ||
| pii=None | ||
| ) | ||
| else: | ||
| requesting_user_data = None | ||
|
|
||
| return ExamAttemptData( | ||
| student_user=UserData( | ||
| id=student_user.id, | ||
| is_active=True, | ||
| pii=UserPersonalData( | ||
| username=student_user.username, | ||
| email=student_user.email, | ||
| ), | ||
| ), | ||
| course_key=course_key, | ||
| usage_key=usage_key, | ||
| requesting_user=requesting_user_data, | ||
| exam_type=exam_type, | ||
| ) | ||
|
|
||
| @staticmethod | ||
| def _get_exam_event_metadata(event_signal): | ||
| """ create metadata object for event """ | ||
| return EventsMetadata( | ||
| event_type=event_signal.event_type, | ||
| id=uuid4(), | ||
| minorversion=0, | ||
| source='openedx/lms/web', | ||
| sourcehost='lms.test', | ||
| time=datetime.now(timezone.utc) | ||
| ) | ||
|
|
||
| @ddt.data( | ||
| True, | ||
| False | ||
| ) | ||
| @mock.patch('lms.djangoapps.grades.api.should_override_grade_on_rejected_exam') | ||
| @mock.patch('lms.djangoapps.grades.api.undo_override_subsection_grade') | ||
| def test_exam_attempt_verified_event_handler(self, override_enabled, mock_undo_override, mock_should_override): | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice! Mocking is easy to read and understand. |
||
| mock_should_override.return_value = override_enabled | ||
|
|
||
| exam_event_data = self._get_exam_event_data(self.student_user, | ||
| self.course_key, | ||
| self.usage_key, | ||
| exam_type='proctored') | ||
| event_metadata = self._get_exam_event_metadata(EXAM_ATTEMPT_VERIFIED) | ||
|
|
||
| event_kwargs = { | ||
| 'exam_attempt': exam_event_data, | ||
| 'metadata': event_metadata | ||
| } | ||
| exam_attempt_verified_event_handler(None, EXAM_ATTEMPT_VERIFIED, ** event_kwargs) | ||
|
|
||
| if override_enabled: | ||
| mock_undo_override.assert_called_once_with( | ||
| self.student_user.id, | ||
| self.course_key, | ||
| self.usage_key, | ||
| GradeOverrideFeatureEnum.proctoring | ||
| ) | ||
| else: | ||
| mock_undo_override.assert_not_called() | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added comment to function that is causing circular dependency issue when trying to do a top level import of any functions from
api.py. This comment is taken from the PR that introduced this dependency.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this what was discussed in the huddle yesterday?