feat: add flag to silence signals and events when clearing state (#34434)
* feat: add flag to silence signals and events when clearing state * feat: don't emit signals and events when clearing entire course
This commit is contained in:
@@ -1,6 +1,7 @@
|
||||
"""
|
||||
Test grading events across apps.
|
||||
"""
|
||||
import ddt
|
||||
from unittest.mock import call as mock_call
|
||||
from unittest.mock import patch
|
||||
|
||||
@@ -20,6 +21,7 @@ from xmodule.modulestore.tests.factories import CourseFactory, BlockFactory # l
|
||||
from ... import events
|
||||
|
||||
|
||||
@ddt.ddt
|
||||
class GradesEventIntegrationTest(ProblemSubmissionTestMixin, SharedModuleStoreTestCase):
|
||||
"""
|
||||
Tests integration between the eventing in various layers
|
||||
@@ -113,56 +115,67 @@ class GradesEventIntegrationTest(ProblemSubmissionTestMixin, SharedModuleStoreTe
|
||||
any_order=True,
|
||||
)
|
||||
|
||||
def test_delete_student_state(self):
|
||||
@ddt.data(True, False)
|
||||
def test_delete_student_state(self, emit_signals):
|
||||
self.submit_question_answer('p1', {'2_1': 'choice_choice_2'})
|
||||
|
||||
with patch('lms.djangoapps.instructor.enrollment.tracker') as enrollment_tracker:
|
||||
with patch('lms.djangoapps.grades.events.tracker') as events_tracker:
|
||||
reset_student_attempts(
|
||||
self.course.id, self.student, self.problem.location, self.instructor, delete_module=True,
|
||||
self.course.id,
|
||||
self.student,
|
||||
self.problem.location,
|
||||
self.instructor,
|
||||
delete_module=True,
|
||||
emit_signals_and_events=emit_signals
|
||||
)
|
||||
course = self.store.get_course(self.course.id, depth=0)
|
||||
|
||||
event_transaction_id = enrollment_tracker.method_calls[0][1][1]['event_transaction_id']
|
||||
enrollment_tracker.emit.assert_called_with(
|
||||
events.STATE_DELETED_EVENT_TYPE,
|
||||
{
|
||||
'user_id': str(self.student.id),
|
||||
'course_id': str(self.course.id),
|
||||
'problem_id': str(self.problem.location),
|
||||
'instructor_id': str(self.instructor.id),
|
||||
'event_transaction_id': event_transaction_id,
|
||||
'event_transaction_type': events.STATE_DELETED_EVENT_TYPE,
|
||||
}
|
||||
)
|
||||
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,
|
||||
)
|
||||
if not emit_signals:
|
||||
enrollment_tracker.assert_not_called()
|
||||
enrollment_tracker.emit.assert_not_called()
|
||||
events_tracker.emit.assert_not_called()
|
||||
else:
|
||||
event_transaction_id = enrollment_tracker.method_calls[0][1][1]['event_transaction_id']
|
||||
enrollment_tracker.emit.assert_called_with(
|
||||
events.STATE_DELETED_EVENT_TYPE,
|
||||
{
|
||||
'user_id': str(self.student.id),
|
||||
'course_id': str(self.course.id),
|
||||
'problem_id': str(self.problem.location),
|
||||
'instructor_id': str(self.instructor.id),
|
||||
'event_transaction_id': event_transaction_id,
|
||||
'event_transaction_type': events.STATE_DELETED_EVENT_TYPE,
|
||||
}
|
||||
)
|
||||
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):
|
||||
self.submit_question_answer('p1', {'2_1': 'choice_choice_3'})
|
||||
|
||||
@@ -7,6 +7,7 @@ Does not include any access control, be sure to check access before calling.
|
||||
|
||||
import json
|
||||
import logging
|
||||
from contextlib import ExitStack, contextmanager
|
||||
from datetime import datetime
|
||||
|
||||
import pytz
|
||||
@@ -19,7 +20,7 @@ from edx_ace import ace
|
||||
from edx_ace.recipient import Recipient
|
||||
from eventtracking import tracker
|
||||
from submissions import api as sub_api # installed from the edx-submissions repository
|
||||
from submissions.models import score_set
|
||||
from submissions.models import score_set, score_reset
|
||||
|
||||
from common.djangoapps.course_modes.models import CourseMode
|
||||
from common.djangoapps.student.models import ( # lint-amnesty, pylint: disable=line-too-long
|
||||
@@ -238,7 +239,28 @@ def send_beta_role_email(action, user, email_params):
|
||||
send_mail_to_student(user.email, email_params, language=get_user_email_language(user))
|
||||
|
||||
|
||||
def reset_student_attempts(course_id, student, module_state_key, requesting_user, delete_module=False):
|
||||
@contextmanager
|
||||
def _conditionally_disconnect_submissions_signal_recievers(emit_score_reset):
|
||||
"""
|
||||
Context manager helper.
|
||||
- Disconnects the score_set signal on enter, reconnects on exit
|
||||
- If emit_score_reset, disconnects score_reset signal on enter, reconnects on exit
|
||||
"""
|
||||
with ExitStack() as context_stack:
|
||||
context_stack.enter_context(disconnect_submissions_signal_receiver(score_set))
|
||||
if not emit_score_reset:
|
||||
context_stack.enter_context(disconnect_submissions_signal_receiver(score_reset))
|
||||
yield
|
||||
|
||||
|
||||
def reset_student_attempts(
|
||||
course_id,
|
||||
student,
|
||||
module_state_key,
|
||||
requesting_user,
|
||||
delete_module=False,
|
||||
emit_signals_and_events=True,
|
||||
):
|
||||
"""
|
||||
Reset student attempts for a problem. Optionally deletes all student state for the specified problem.
|
||||
|
||||
@@ -248,6 +270,10 @@ def reset_student_attempts(course_id, student, module_state_key, requesting_user
|
||||
`student` is a User
|
||||
`problem_to_reset` is the name of a problem e.g. 'L2Node1'.
|
||||
To build the module_state_key 'problem/' and course information will be appended to `problem_to_reset`.
|
||||
`delete_module`: Instead of resetting attempts, delete the learner's StudentModule
|
||||
`emit_signals_and_events`: If this is False, don't fire django signals or emit events. This is intended for
|
||||
the case where we are calling this function many times, and want to handle the signalling
|
||||
and eventing at a bulk level rather than firing every individual call to this function
|
||||
|
||||
Raises:
|
||||
ValueError: `problem_state` is invalid JSON.
|
||||
@@ -275,7 +301,7 @@ def reset_student_attempts(course_id, student, module_state_key, requesting_user
|
||||
# Inform these blocks of the reset and allow them to handle their data.
|
||||
clear_student_state = getattr(block, "clear_student_state", None)
|
||||
if callable(clear_student_state):
|
||||
with disconnect_submissions_signal_receiver(score_set):
|
||||
with _conditionally_disconnect_submissions_signal_recievers(emit_signals_and_events):
|
||||
clear_student_state(
|
||||
user_id=user_id,
|
||||
course_id=str(course_id),
|
||||
@@ -300,11 +326,15 @@ def reset_student_attempts(course_id, student, module_state_key, requesting_user
|
||||
user_id,
|
||||
str(course_id),
|
||||
str(module_state_key),
|
||||
emit_signal=emit_signals_and_events,
|
||||
)
|
||||
|
||||
def _reset_or_delete_module(studentmodule):
|
||||
if delete_module:
|
||||
studentmodule.delete()
|
||||
if not emit_signals_and_events:
|
||||
return
|
||||
|
||||
create_new_event_transaction_id()
|
||||
set_event_transaction_type(grades_events.STATE_DELETED_EVENT_TYPE)
|
||||
tracker.emit(
|
||||
|
||||
@@ -62,7 +62,14 @@ def reset_student_course(course_id, learner_email, reset_by_user_email):
|
||||
# Clear student state and score
|
||||
for data in blocks:
|
||||
try:
|
||||
reset_student_attempts(course.id, user, data.scope_ids.usage_id, reset_by_user, True)
|
||||
reset_student_attempts(
|
||||
course.id,
|
||||
user,
|
||||
data.scope_ids.usage_id,
|
||||
reset_by_user,
|
||||
delete_module=True,
|
||||
emit_signals_and_events=False
|
||||
)
|
||||
except StudentModule.DoesNotExist:
|
||||
pass
|
||||
|
||||
|
||||
@@ -123,28 +123,32 @@ class ResetStudentCourse(TestSubmittingProblems):
|
||||
self.student_user,
|
||||
self.p1.location,
|
||||
self.user,
|
||||
True
|
||||
delete_module=True,
|
||||
emit_signals_and_events=False,
|
||||
),
|
||||
call(
|
||||
self.course.id,
|
||||
self.student_user,
|
||||
self.p2.location,
|
||||
self.user,
|
||||
True
|
||||
delete_module=True,
|
||||
emit_signals_and_events=False,
|
||||
),
|
||||
call(
|
||||
self.course.id,
|
||||
self.student_user,
|
||||
self.p3.location,
|
||||
self.user,
|
||||
True
|
||||
delete_module=True,
|
||||
emit_signals_and_events=False,
|
||||
),
|
||||
call(
|
||||
self.course.id,
|
||||
self.student_user,
|
||||
self.video.location,
|
||||
self.user,
|
||||
True
|
||||
delete_module=True,
|
||||
emit_signals_and_events=False,
|
||||
)
|
||||
])
|
||||
|
||||
@@ -168,21 +172,24 @@ class ResetStudentCourse(TestSubmittingProblems):
|
||||
self.student_user,
|
||||
self.p1.location,
|
||||
self.user,
|
||||
True
|
||||
delete_module=True,
|
||||
emit_signals_and_events=False,
|
||||
),
|
||||
call(
|
||||
self.course.id,
|
||||
self.student_user,
|
||||
self.p2.location,
|
||||
self.user,
|
||||
True
|
||||
delete_module=True,
|
||||
emit_signals_and_events=False,
|
||||
),
|
||||
call(
|
||||
self.course.id,
|
||||
self.student_user,
|
||||
self.p3.location,
|
||||
self.user,
|
||||
True
|
||||
delete_module=True,
|
||||
emit_signals_and_events=False,
|
||||
)
|
||||
])
|
||||
|
||||
|
||||
Reference in New Issue
Block a user