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
61 changes: 61 additions & 0 deletions lms/djangoapps/grades/events.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@
STATE_DELETED_EVENT_TYPE = 'edx.grades.problem.state_deleted'
SUBSECTION_OVERRIDE_EVENT_TYPE = 'edx.grades.subsection.score_overridden'
SUBSECTION_GRADE_CALCULATED = 'edx.grades.subsection.grade_calculated'
COURSE_GRADE_PASSED_FIRST_TIME_EVENT_TYPE = 'edx.course.grade.passed.first_time'
COURSE_GRADE_NOW_PASSED_EVENT_TYPE = 'edx.course.grade.now_passed'
COURSE_GRADE_NOW_FAILED_EVENT_TYPE = 'edx.course.grade.now_failed'


def grade_updated(**kwargs):
Expand Down Expand Up @@ -135,3 +138,61 @@ def course_grade_calculated(course_grade):
'grading_policy_hash': str(course_grade.grading_policy_hash),
}
)


def course_grade_passed_first_time(user_id, course_id):
"""
Emits an event edx.course.grade.passed.first_time
with data from the passed course_grade.
"""
event_name = COURSE_GRADE_PASSED_FIRST_TIME_EVENT_TYPE
context = contexts.course_context_from_course_id(course_id)
# TODO (AN-6134): remove this context manager
with tracker.get_tracker().context(event_name, context):
tracker.emit(
event_name,
{
'user_id': str(user_id),
'course_id': str(course_id),
'event_transaction_id': str(get_event_transaction_id()),
Copy link
Contributor

Choose a reason for hiding this comment

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

[double checking] Are event_transaction_id and event_transaction_type meaningful in these new events?

Copy link
Author

@rehanedly rehanedly Aug 25, 2021

Choose a reason for hiding this comment

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

@nasthagiri Added those for consistency with other events of grades. These fields might not be meaning full for xAPI/Caliper but could be meaningful for other backends like logging or segmentation and these fields are present in many of other events emits from edx-platform.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good.

'event_transaction_type': str(get_event_transaction_type())
}
)


def course_grade_now_passed(user, course_id):
"""
Emits an edx.course.grade.now_passed event
with data from the course and user passed now .
"""
event_name = COURSE_GRADE_NOW_PASSED_EVENT_TYPE
context = contexts.course_context_from_course_id(course_id)
with tracker.get_tracker().context(event_name, context):
tracker.emit(
event_name,
{
'user_id': str(user.id),
'course_id': str(course_id),
'event_transaction_id': str(get_event_transaction_id()),
'event_transaction_type': str(get_event_transaction_type())
}
)


def course_grade_now_failed(user, course_id):
"""
Emits an edx.course.grade.now_failed event
with data from the course and user failed now .
"""
event_name = COURSE_GRADE_NOW_FAILED_EVENT_TYPE
context = contexts.course_context_from_course_id(course_id)
with tracker.get_tracker().context(event_name, context):
tracker.emit(
event_name,
{
'user_id': str(user.id),
'course_id': str(course_id),
'event_transaction_id': str(get_event_transaction_id()),
'event_transaction_type': str(get_event_transaction_type())
}
)
7 changes: 6 additions & 1 deletion lms/djangoapps/grades/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
persisted, course grades are also immune to changes in course content.
"""


import json
import logging
from base64 import b64encode
Expand All @@ -28,6 +27,7 @@
from lms.djangoapps.courseware.fields import UnsignedBigIntAutoField
from lms.djangoapps.grades import events # lint-amnesty, pylint: disable=unused-import
from openedx.core.lib.cache_utils import get_cache
from lms.djangoapps.grades.signals.signals import COURSE_GRADE_PASSED_FIRST_TIME

log = logging.getLogger(__name__)

Expand Down Expand Up @@ -645,6 +645,11 @@ def update_or_create(cls, user_id, course_id, **kwargs):
defaults=kwargs
)
if passed and not grade.passed_timestamp:
COURSE_GRADE_PASSED_FIRST_TIME.send(
sender=None,
course_id=course_id,
user_id=user_id
)
grade.passed_timestamp = now()
grade.save()

Expand Down
37 changes: 36 additions & 1 deletion lms/djangoapps/grades/signals/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,12 @@
PROBLEM_WEIGHTED_SCORE_CHANGED,
SCORE_PUBLISHED,
SUBSECTION_OVERRIDE_CHANGED,
SUBSECTION_SCORE_CHANGED
SUBSECTION_SCORE_CHANGED,
COURSE_GRADE_PASSED_FIRST_TIME
)
from openedx.core.djangoapps.signals.signals import (
COURSE_GRADE_NOW_FAILED,
COURSE_GRADE_NOW_PASSED
)

log = getLogger(__name__)
Expand Down Expand Up @@ -264,3 +269,33 @@ def recalculate_course_and_subsection_grades(sender, user, course_key, countdown
course_key=str(course_key)
)
)


@receiver(COURSE_GRADE_NOW_PASSED)
def listen_for_passing_grade(sender, user, course_id, **kwargs): # pylint: disable=unused-argument
"""
Listen for a signal indicating that the user has passed a course run.

Emits an edx.course.grade.now_passed event
"""
events.course_grade_now_passed(user, course_id)


@receiver(COURSE_GRADE_NOW_FAILED)
def listen_for_failing_grade(sender, user, course_id, **kwargs): # pylint: disable=unused-argument
"""
Listen for a signal indicating that the user has failed a course run.

Emits an edx.course.grade.now_failed event
"""
events.course_grade_now_failed(user, course_id)


@receiver(COURSE_GRADE_PASSED_FIRST_TIME)
def listen_for_course_grade_passed_first_time(sender, user_id, course_id, **kwargs): # pylint: disable=unused-argument
"""
Listen for a signal indicating that the user has passed course grade first time.

Emits an event edx.course.grade.passed.first_time
"""
events.course_grade_passed_first_time(user_id, course_id)
11 changes: 11 additions & 0 deletions lms/djangoapps/grades/signals/signals.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,3 +106,14 @@
# score that was created.
]
)


# This Signal indicates that the user has received a passing grade in the course for the first time.
# Any subsequent grade changes that may vary the passing/failing status will not re-trigger this event.
# Emits course grade passed first time event
COURSE_GRADE_PASSED_FIRST_TIME = Signal(
Copy link
Contributor

Choose a reason for hiding this comment

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

[request to follow-through on renaming] Can you follow through on updating the name of the course_completed events elsewhere in this PR?

Essentially, within the core grades layer, let's be precise of what the event actually is. Then, in a higher layer (for now, it could be at the xapi-caliper transformation layer), we can decide that a course_grade_passed_first_time event translates to a course_completion event.

Copy link
Author

Choose a reason for hiding this comment

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

@nasthagiri I have renamed this event. we will. handle translation in the event routing backend

providing_args=[
'course_id', # Course object id
'user_id', # User object id
]
)
41 changes: 27 additions & 14 deletions lms/djangoapps/grades/tests/integration/test_events.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,20 +135,33 @@ def test_delete_student_state(self):
'event_transaction_type': events.STATE_DELETED_EVENT_TYPE,
}
)

events_tracker.emit.assert_called_with(
events.COURSE_GRADE_CALCULATED,
{
'percent_grade': 0.0,
'grading_policy_hash': 'ChVp0lHGQGCevD0t4njna/C44zQ=',
'user_id': str(self.student.id),
'letter_grade': '',
'event_transaction_id': event_transaction_id,
'event_transaction_type': events.STATE_DELETED_EVENT_TYPE,
'course_id': str(self.course.id),
'course_edited_timestamp': str(course.subtree_edited_on),
'course_version': str(course.course_version),
}
events_tracker.emit.assert_has_calls(
[
mock_call(
events.COURSE_GRADE_CALCULATED,
{
'percent_grade': 0.0,
'grading_policy_hash': 'ChVp0lHGQGCevD0t4njna/C44zQ=',
'user_id': str(self.student.id),
'letter_grade': '',
'event_transaction_id': event_transaction_id,
'event_transaction_type': events.STATE_DELETED_EVENT_TYPE,
'course_id': str(self.course.id),
'course_edited_timestamp': str(course.subtree_edited_on),
'course_version': str(course.course_version),
}
),
mock_call(
events.COURSE_GRADE_NOW_FAILED_EVENT_TYPE,
{
'user_id': str(self.student.id),
'event_transaction_id': event_transaction_id,
'event_transaction_type': events.STATE_DELETED_EVENT_TYPE,
'course_id': str(self.course.id),
}
),
],
any_order=True,
)

def test_rescoring_events(self):
Expand Down
4 changes: 3 additions & 1 deletion lms/djangoapps/grades/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -425,10 +425,12 @@ def test_passed_timestamp(self):
assert grade.letter_grade == ''
assert grade.passed_timestamp == passed_timestamp

def test_passed_timestamp_is_now(self):
@patch('lms.djangoapps.grades.signals.signals.COURSE_GRADE_PASSED_FIRST_TIME.send')
def test_passed_timestamp_is_now(self, mock):
with freeze_time(now()):
grade = PersistentCourseGrade.update_or_create(**self.params)
assert now() == grade.passed_timestamp
self.assertEqual(mock.call_count, 1)

def test_create_and_read_grade(self):
created_grade = PersistentCourseGrade.update_or_create(**self.params)
Expand Down
128 changes: 127 additions & 1 deletion lms/djangoapps/grades/tests/test_signals.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,20 @@
import pytz
from django.test import TestCase
from submissions.models import score_reset, score_set
from opaque_keys.edx.locator import CourseLocator

from common.djangoapps.track.event_transaction_utils import get_event_transaction_id, get_event_transaction_type
from common.djangoapps.util.date_utils import to_timestamp

from ..constants import ScoreDatabaseTableEnum
from ..signals.handlers import (
disconnect_submissions_signal_receiver,
problem_raw_score_changed_handler,
submissions_score_reset_handler,
submissions_score_set_handler
submissions_score_set_handler,
listen_for_course_grade_passed_first_time,
listen_for_passing_grade,
listen_for_failing_grade
)
from ..signals.signals import PROBLEM_RAW_SCORE_CHANGED

Expand Down Expand Up @@ -259,3 +264,124 @@ def test_disconnect_manager_bad_arg(self):
with pytest.raises(ValueError):
with disconnect_submissions_signal_receiver(PROBLEM_RAW_SCORE_CHANGED):
pass


class CourseEventsSignalsTest(TestCase):
"""
Tests to ensure that the courseware module correctly catches
course grades passed/failed signal and emit course related event
"""
SIGNALS = {
'score_set': score_set,
'score_reset': score_reset,
}

def setUp(self):
"""
Configure mocks for all the dependencies of the render method
"""
super().setUp()
self.signal_mock = self.setup_patch(
'lms.djangoapps.grades.signals.signals.COURSE_GRADE_PASSED_FIRST_TIME.send',
None,
)
self.user_mock = MagicMock()
self.user_mock.id = 42
self.get_user_mock = self.setup_patch(
'lms.djangoapps.grades.signals.handlers.user_by_anonymous_id',
self.user_mock
)
self.course_id = CourseLocator(
org='some_org',
course='some_course',
run='some_run'
)

def setup_patch(self, function_name, return_value):
"""
Patch a function with a given return value, and return the mock
"""
mock = MagicMock(return_value=return_value)
new_patch = patch(function_name, new=mock)
new_patch.start()
self.addCleanup(new_patch.stop)
return mock

def test_course_grade_passed_first_time_signal_handler(self):
"""
Ensure that on course grade passed first tim signal, course grade passed first time event is triggered
"""
handler = listen_for_course_grade_passed_first_time

with patch('lms.djangoapps.grades.events.tracker') as tracker_mock:
handler(None, self.user_mock.id, self.course_id)
self._assert_tracker_emitted_course_grade_passed_first_time_event(
tracker_mock,
self.user_mock.id,
self.course_id
)

def _assert_tracker_emitted_course_grade_passed_first_time_event(self, tracker_mock, user_id, course_id):
"""
Helper function to ensure that the mocked event tracker
was called with the expected info based on the course grade passed first time.
"""
tracker_mock.emit.assert_called_with(
'edx.course.grade.passed.first_time',
{
'user_id': str(user_id),
'course_id': str(course_id),
'event_transaction_id': str(get_event_transaction_id()),
'event_transaction_type': str(get_event_transaction_type()),
}
)

def test_now_passed_signal_handler(self):
"""
Ensure that on course now passed signal, course now passed event is triggered
"""
handler = listen_for_passing_grade

with patch('lms.djangoapps.grades.events.tracker') as tracker_mock:
handler(None, self.user_mock, self.course_id)
self._assert_tracker_emitted_course_now_passed_event(tracker_mock, self.user_mock, self.course_id)

def _assert_tracker_emitted_course_now_passed_event(self, tracker_mock, user, course_id):
"""
Helper function to ensure that the mocked event tracker
was called with the expected info based on passed course.
"""
tracker_mock.emit.assert_called_with(
'edx.course.grade.now_passed',
{
'user_id': str(user.id),
'course_id': str(course_id),
'event_transaction_id': str(get_event_transaction_id()),
'event_transaction_type': str(get_event_transaction_type()),
}
)

def test_now_failed_signal_handler(self):
"""
Ensure that on course now failed signal, course now failed event is triggered
"""
handler = listen_for_failing_grade

with patch('lms.djangoapps.grades.events.tracker') as tracker_mock:
handler(None, self.user_mock, self.course_id)
self._assert_tracker_emitted_course_now_failed_event(tracker_mock, self.user_mock, self.course_id)

def _assert_tracker_emitted_course_now_failed_event(self, tracker_mock, user, course_id):
"""
Helper function to ensure that the mocked event tracker
was called with the expected info based on failed course.
"""
tracker_mock.emit.assert_called_with(
'edx.course.grade.now_failed',
{
'user_id': str(user.id),
'course_id': str(course_id),
'event_transaction_id': str(get_event_transaction_id()),
'event_transaction_type': str(get_event_transaction_type()),
}
)