From e07a0cc163f475e3a8b2bf0ca5213e7f970f03dd Mon Sep 17 00:00:00 2001 From: Jansen Kantor Date: Fri, 22 Mar 2024 12:23:48 -0400 Subject: [PATCH] feat: add comment to course reset model and endpoints (#34411) * feat: add comment to course reset model * feat: add comment info to list endpoint * feat: add comment to post endpoint * fixup! feat: add comment to post endpoint --- lms/djangoapps/support/admin.py | 23 ++++++++- .../migrations/0004_add_comment_field.py | 18 +++++++ lms/djangoapps/support/models.py | 1 + lms/djangoapps/support/tests/factories.py | 1 + lms/djangoapps/support/tests/test_views.py | 47 ++++++++++++++----- lms/djangoapps/support/views/course_reset.py | 25 ++++++++-- 6 files changed, 98 insertions(+), 17 deletions(-) create mode 100644 lms/djangoapps/support/migrations/0004_add_comment_field.py diff --git a/lms/djangoapps/support/admin.py b/lms/djangoapps/support/admin.py index d7bfc5750884..2b7fbc7b48d6 100644 --- a/lms/djangoapps/support/admin.py +++ b/lms/djangoapps/support/admin.py @@ -22,14 +22,33 @@ class CourseResetAuditAdmin(admin.ModelAdmin): """ Django admin for CourseResetAudit model """ list_display = ['course', 'user', 'status', 'created', 'completed_at', 'reset_by'] - fields = ['created', 'modified', 'status', 'completed_at', 'course', 'user', 'course_enrollment', 'reset_by'] + fields = [ + 'created', + 'modified', + 'status', + 'completed_at', + 'course', + 'user', + 'course_enrollment', + 'reset_by', + 'comment' + ] def get_readonly_fields(self, request, obj=None): """ If we are editing an existing model, we should only be able to change the status, for potential debugging """ if obj: - return ['created', 'modified', 'completed_at', 'course', 'user', 'course_enrollment', 'reset_by'] + return [ + 'created', + 'modified', + 'completed_at', + 'course', + 'user', + 'course_enrollment', + 'reset_by', + 'comment' + ] else: return ['created', 'modified', 'user'] diff --git a/lms/djangoapps/support/migrations/0004_add_comment_field.py b/lms/djangoapps/support/migrations/0004_add_comment_field.py new file mode 100644 index 000000000000..2f6daa2a26d5 --- /dev/null +++ b/lms/djangoapps/support/migrations/0004_add_comment_field.py @@ -0,0 +1,18 @@ +# Generated by Django 4.2.10 on 2024-03-22 13:37 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('support', '0003_course_reset'), + ] + + operations = [ + migrations.AddField( + model_name='courseresetaudit', + name='comment', + field=models.CharField(blank=True, default='', max_length=255), + ), + ] diff --git a/lms/djangoapps/support/models.py b/lms/djangoapps/support/models.py index 9fa254fe6ecc..a899c9ac5687 100644 --- a/lms/djangoapps/support/models.py +++ b/lms/djangoapps/support/models.py @@ -65,6 +65,7 @@ class CourseResetStatus(TextChoices): default=CourseResetStatus.ENQUEUED, ) completed_at = DateTimeField(default=None, null=True, blank=True) + comment = CharField(max_length=255, default="", blank=True) def status_message(self): """ Return a string message about the status of this audit """ diff --git a/lms/djangoapps/support/tests/factories.py b/lms/djangoapps/support/tests/factories.py index 93f68b11b3c7..619e669dec25 100644 --- a/lms/djangoapps/support/tests/factories.py +++ b/lms/djangoapps/support/tests/factories.py @@ -26,4 +26,5 @@ class Meta: course_enrollment = factory.SubFactory(CourseEnrollmentFactory) reset_by = factory.SubFactory(UserFactory) status = CourseResetAudit.CourseResetStatus.ENQUEUED + comment = factory.Sequence(lambda i: f'comment {i}') completed_at = None diff --git a/lms/djangoapps/support/tests/test_views.py b/lms/djangoapps/support/tests/test_views.py index 853a82fb3bc1..7fff6117b4ab 100644 --- a/lms/djangoapps/support/tests/test_views.py +++ b/lms/djangoapps/support/tests/test_views.py @@ -2165,6 +2165,7 @@ def test_course_not_started(self): 'course_id': self.course_id, 'display_name': self.course_overview.display_name, 'can_reset': False, + 'comment': '', 'status': 'Course Not Started' }]) @@ -2178,6 +2179,7 @@ def test_course_ended(self): 'course_id': self.course_id, 'display_name': self.course_overview.display_name, 'can_reset': False, + 'comment': '', 'status': 'Course Ended' } ]) @@ -2189,6 +2191,7 @@ def test_user_has_passing_grade(self, _): 'course_id': self.course_id, 'display_name': self.course_overview.display_name, 'can_reset': False, + 'comment': '', 'status': 'Learner Has Passing Grade' }]) @@ -2204,6 +2207,7 @@ def test_ended_with_passing_grade(self, _): 'course_id': self.course_id, 'display_name': self.course_overview.display_name, 'can_reset': False, + 'comment': '', 'status': 'Learner Has Passing Grade' }]) @@ -2213,6 +2217,7 @@ def test_available_course(self): 'course_id': self.course_id, 'display_name': self.course.display_name, 'can_reset': True, + 'comment': '', 'status': 'Available' }]) @@ -2237,29 +2242,39 @@ def test_audit(self, audit_status, expected_can_reset): 'course_id': self.course_id, 'display_name': self.course.display_name, 'can_reset': expected_can_reset, + 'comment': audit.comment, 'status': audit.status_message() }]) + def _set_up_course(self, opt_in=True): + """ + Make a course, enroll self.learner, and optionally opt into course reset + """ + course = CourseFactory.create(start=self.course.start, end=self.course.end) + CourseEnrollmentFactory.create(course_id=course.id, user=self.learner) + if opt_in: + CourseResetCourseOptInFactory.create(course_id=course.id) + return course + def test_multiple_courses(self): """ Test for the behavior of multiple courses """ - courses = [CourseFactory.create(start=self.course.start, end=self.course.end) for _ in range(4)] - for course in courses: - CourseEnrollmentFactory.create(course_id=course.id, user=self.learner) - CourseResetCourseOptInFactory.create(course_id=course.id) - other_courses = [CourseFactory.create(start=self.course.start, end=self.course.end) for _ in range(4)] - for course in other_courses: - CourseEnrollmentFactory.create(course_id=course.id, user=self.learner) + # Create four opted in courses and four non-opted-in courses + opted_in_courses = [self._set_up_course(opt_in=True) for _ in range(4)] + for _ in range(4): + self._set_up_course(opt_in=False) expected_response = [{ 'course_id': self.course_id, 'display_name': self.course.display_name, 'can_reset': True, + 'comment': '', 'status': 'Available' }] - for course in courses: + for course in opted_in_courses: expected_response.append({ 'course_id': str(course.id), 'display_name': course.display_name, + 'comment': '', 'can_reset': True, 'status': 'Available' }) @@ -2292,10 +2307,11 @@ def test_multiple_audits(self): status=CourseResetAudit.CourseResetStatus.IN_PROGRESS, ) - response = self.assertResponse([{ + self.assertResponse([{ 'course_id': self.course_id, 'display_name': self.course.display_name, 'can_reset': False, + 'comment': most_recent_audit.comment, 'status': most_recent_audit.status_message() }]) @@ -2325,10 +2341,11 @@ def test_multiple_failed_audits(self): status=CourseResetAudit.CourseResetStatus.FAILED, ) - response = self.assertResponse([{ + self.assertResponse([{ 'course_id': self.course_id, 'display_name': self.course.display_name, 'can_reset': True, + 'comment': most_recent_audit.comment, 'status': most_recent_audit.status_message() }]) @@ -2379,12 +2396,20 @@ def test_wrong_username(self): @patch('lms.djangoapps.support.views.course_reset.reset_student_course') def test_learner_course_reset(self, mock_reset_student_course): - response = self.client.post(self._url(username=self.user.username), data={'course_id': self.course_id}) + comment = str(uuid4()) + response = self.client.post( + self._url(username=self.user.username), + data={ + 'course_id': self.course_id, + 'comment': comment, + } + ) self.assertEqual(response.status_code, 201) self.assertEqual(response.data, { 'course_id': self.course_id, 'status': response.data['status'], 'can_reset': False, + 'comment': comment, 'display_name': self.course.display_name }) self.assertEqual( diff --git a/lms/djangoapps/support/views/course_reset.py b/lms/djangoapps/support/views/course_reset.py index 97c7b63d93dd..af7314d7a60c 100644 --- a/lms/djangoapps/support/views/course_reset.py +++ b/lms/djangoapps/support/views/course_reset.py @@ -19,6 +19,13 @@ User = get_user_model() +def get_latest_audit(course_enrollment): + try: + return course_enrollment.courseresetaudit_set.latest('modified') + except CourseResetAudit.DoesNotExist: + return None + + def can_enrollment_be_reset(course_enrollment): """ Args: enrollment (CourseEnrollment) @@ -35,9 +42,8 @@ def can_enrollment_be_reset(course_enrollment): if user_has_passing_grade_in_course(course_enrollment): return False, "Learner Has Passing Grade" - try: - audit = course_enrollment.courseresetaudit_set.latest('modified') - except CourseResetAudit.DoesNotExist: + audit = get_latest_audit(course_enrollment) + if audit is None: return True, None audit_status_message = audit.status_message() @@ -69,6 +75,7 @@ def get(self, request, username_or_email): 'course_id': 'display_name': 'status': + 'comment': 'can_reset': (boolean) } ] @@ -88,10 +95,12 @@ def get(self, request, username_or_email): for course_enrollment in course_enrollments: course_overview = course_enrollment.course_overview can_reset, status_message = can_enrollment_be_reset(course_enrollment) + course_reset_audit = get_latest_audit(course_enrollment) result.append({ 'course_id': str(course_overview.id), 'display_name': course_overview.display_name, 'can_reset': can_reset, + 'comment': course_reset_audit.comment if course_reset_audit else '', 'status': status_message if status_message else "Available" }) return Response(result) @@ -99,7 +108,11 @@ def get(self, request, username_or_email): @method_decorator(require_support_permission) def post(self, request, username_or_email): """ - Resets a course for the given learner + Resets a course for the given learner. + + POST params: + course_id (CourseKey): the course to reset + comment [optional] (str): 255 characters or fewer comment on why the course is being reset returns a dicts with the format { 'course_id': @@ -108,6 +121,7 @@ def post(self, request, username_or_email): 'can_reset': (boolean) } """ + comment = request.data.get('comment', '') course_id = request.data['course_id'] try: user = get_user_by_username_or_email(username_or_email) @@ -131,6 +145,7 @@ def post(self, request, username_or_email): and not user_passed ): course_reset_audit.status = CourseResetAudit.CourseResetStatus.ENQUEUED + course_reset_audit.comment = comment course_reset_audit.save() reset_student_course.delay(course_id, user.email, request.user.email) @@ -153,11 +168,13 @@ def post(self, request, username_or_email): course=opt_in_course, course_enrollment=enrollment, reset_by=request.user, + comment=comment, ) resp = { 'course_id': course_id, 'status': course_reset_audit.status_message(), 'can_reset': False, + 'comment': comment, 'display_name': course_overview.display_name }