diff --git a/cms/djangoapps/coursegraph/apps.py b/cms/djangoapps/coursegraph/apps.py index 95d7873fce..71ae91ad49 100644 --- a/cms/djangoapps/coursegraph/apps.py +++ b/cms/djangoapps/coursegraph/apps.py @@ -3,7 +3,7 @@ Coursegraph Application Configuration Signal handlers are connected here. """ - +import warnings from django.apps import AppConfig @@ -15,3 +15,11 @@ class CoursegraphConfig(AppConfig): name = 'cms.djangoapps.coursegraph' from cms.djangoapps.coursegraph import tasks + + def ready(self) -> None: + warnings.warn( + "Neo4j support is going to be dropped after Sumac release," + "to read more here is a github issue https://github.com/openedx/edx-platform/issues/34342", + DeprecationWarning, + stacklevel=2 + ) diff --git a/lms/djangoapps/grades/tests/integration/test_events.py b/lms/djangoapps/grades/tests/integration/test_events.py index 881ad049b3..f058cc8179 100644 --- a/lms/djangoapps/grades/tests/integration/test_events.py +++ b/lms/djangoapps/grades/tests/integration/test_events.py @@ -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'}) diff --git a/lms/djangoapps/instructor/enrollment.py b/lms/djangoapps/instructor/enrollment.py index f8bf1dd866..1dd451a41c 100644 --- a/lms/djangoapps/instructor/enrollment.py +++ b/lms/djangoapps/instructor/enrollment.py @@ -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( diff --git a/lms/djangoapps/support/admin.py b/lms/djangoapps/support/admin.py index 2b7fbc7b48..2fde5d3465 100644 --- a/lms/djangoapps/support/admin.py +++ b/lms/djangoapps/support/admin.py @@ -1,21 +1,32 @@ """ Django admins for support models """ +from django import forms from django.contrib import admin from lms.djangoapps.support.models import CourseResetCourseOptIn, CourseResetAudit +from openedx.core.lib.courses import clean_course_id + + +class CourseResetCourseOptInAdminForm(forms.ModelForm): + """ Form for the CourseResetCourseOptIn Django admin page """ + class Meta: + model = CourseResetCourseOptIn + fields = ['course_id', 'active'] + + def clean_course_id(self): + return clean_course_id(self) class CourseResetCourseOptInAdmin(admin.ModelAdmin): """ Django admin for CourseResetCourseOptIn model """ - list_display = ['course_id', 'active'] - fields = ['course_id', 'active', 'created', 'modified'] + form = CourseResetCourseOptInAdminForm + list_display = ['course_id', 'active', 'created', 'modified'] def get_readonly_fields(self, request, obj=None): """ Ensure that 'course_id' cannot be edited after creation. """ if obj: - return ['course_id', 'created', 'modified'] - else: - return ['created', 'modified'] + return ['course_id'] + return [] class CourseResetAuditAdmin(admin.ModelAdmin): @@ -33,6 +44,7 @@ class CourseResetAuditAdmin(admin.ModelAdmin): 'reset_by', 'comment' ] + actions = ['mark_failed'] def get_readonly_fields(self, request, obj=None): """ @@ -56,6 +68,10 @@ class CourseResetAuditAdmin(admin.ModelAdmin): def user(self, obj): return obj.course_enrollment.user + @admin.action(description="Fail selected reset attempts") + def mark_failed(self, request, queryset): + queryset.update(status=CourseResetAudit.CourseResetStatus.FAILED) + admin.site.register(CourseResetCourseOptIn, CourseResetCourseOptInAdmin) admin.site.register(CourseResetAudit, CourseResetAuditAdmin) diff --git a/lms/djangoapps/support/migrations/0005_unique_course_id.py b/lms/djangoapps/support/migrations/0005_unique_course_id.py new file mode 100644 index 0000000000..baf89bedc3 --- /dev/null +++ b/lms/djangoapps/support/migrations/0005_unique_course_id.py @@ -0,0 +1,19 @@ +# Generated by Django 4.2.10 on 2024-03-29 18:56 + +from django.db import migrations +import opaque_keys.edx.django.models + + +class Migration(migrations.Migration): + + dependencies = [ + ('support', '0004_add_comment_field'), + ] + + operations = [ + migrations.AlterField( + model_name='courseresetcourseoptin', + name='course_id', + field=opaque_keys.edx.django.models.CourseKeyField(max_length=255, unique=True), + ), + ] diff --git a/lms/djangoapps/support/models.py b/lms/djangoapps/support/models.py index a899c9ac56..32df989eed 100644 --- a/lms/djangoapps/support/models.py +++ b/lms/djangoapps/support/models.py @@ -22,7 +22,7 @@ class CourseResetCourseOptIn(TimeStampedModel): """ Model that represents a course which has opted in to the course reset feature. """ - course_id = CourseKeyField(max_length=255) + course_id = CourseKeyField(max_length=255, unique=True) active = BooleanField() def __str__(self): diff --git a/lms/djangoapps/support/tasks.py b/lms/djangoapps/support/tasks.py index 62b78c3751..cf00823d98 100644 --- a/lms/djangoapps/support/tasks.py +++ b/lms/djangoapps/support/tasks.py @@ -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 diff --git a/lms/djangoapps/support/tests/test_tasks.py b/lms/djangoapps/support/tests/test_tasks.py index 67a268cdf3..181f9022f4 100644 --- a/lms/djangoapps/support/tests/test_tasks.py +++ b/lms/djangoapps/support/tests/test_tasks.py @@ -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, ) ]) diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 738ab2e885..36cc3f27c3 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -415,7 +415,7 @@ drf-yasg==1.21.5 # edx-api-doc-tools edx-ace==1.7.0 # via -r requirements/edx/kernel.in -edx-api-doc-tools==1.7.0 +edx-api-doc-tools==1.8.0 # via # -r requirements/edx/kernel.in # edx-name-affirmation @@ -513,7 +513,7 @@ edx-opaque-keys[django]==2.5.1 # lti-consumer-xblock # openedx-events # ora2 -edx-organizations==6.12.1 +edx-organizations==6.13.0 # via -r requirements/edx/kernel.in edx-proctoring==4.16.1 # via @@ -1203,7 +1203,7 @@ watchdog==4.0.0 # via -r requirements/edx/paver.txt wcwidth==0.2.13 # via prompt-toolkit -web-fragments==2.1.0 +web-fragments==2.2.0 # via # -r requirements/edx/kernel.in # crowdsourcehinter-xblock diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 12928a3b74..28d0b07a5c 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -676,7 +676,7 @@ edx-ace==1.7.0 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt -edx-api-doc-tools==1.7.0 +edx-api-doc-tools==1.8.0 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt @@ -801,7 +801,7 @@ edx-opaque-keys[django]==2.5.1 # lti-consumer-xblock # openedx-events # ora2 -edx-organizations==6.12.1 +edx-organizations==6.13.0 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt @@ -2155,7 +2155,7 @@ wcwidth==0.2.13 # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt # prompt-toolkit -web-fragments==2.1.0 +web-fragments==2.2.0 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index ba02015bda..516e428e7b 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -493,7 +493,7 @@ drf-yasg==1.21.5 # edx-api-doc-tools edx-ace==1.7.0 # via -r requirements/edx/base.txt -edx-api-doc-tools==1.7.0 +edx-api-doc-tools==1.8.0 # via # -r requirements/edx/base.txt # edx-name-affirmation @@ -590,7 +590,7 @@ edx-opaque-keys[django]==2.5.1 # lti-consumer-xblock # openedx-events # ora2 -edx-organizations==6.12.1 +edx-organizations==6.13.0 # via -r requirements/edx/base.txt edx-proctoring==4.16.1 # via @@ -1468,7 +1468,7 @@ wcwidth==0.2.13 # via # -r requirements/edx/base.txt # prompt-toolkit -web-fragments==2.1.0 +web-fragments==2.2.0 # via # -r requirements/edx/base.txt # crowdsourcehinter-xblock diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index e0de1f1f55..88f7b2585c 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -517,7 +517,7 @@ drf-yasg==1.21.5 # edx-api-doc-tools edx-ace==1.7.0 # via -r requirements/edx/base.txt -edx-api-doc-tools==1.7.0 +edx-api-doc-tools==1.8.0 # via # -r requirements/edx/base.txt # edx-name-affirmation @@ -616,7 +616,7 @@ edx-opaque-keys[django]==2.5.1 # lti-consumer-xblock # openedx-events # ora2 -edx-organizations==6.12.1 +edx-organizations==6.13.0 # via -r requirements/edx/base.txt edx-proctoring==4.16.1 # via @@ -1582,7 +1582,7 @@ wcwidth==0.2.13 # via # -r requirements/edx/base.txt # prompt-toolkit -web-fragments==2.1.0 +web-fragments==2.2.0 # via # -r requirements/edx/base.txt # crowdsourcehinter-xblock