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
This commit is contained in:
Jansen Kantor
2024-03-22 12:23:48 -04:00
committed by GitHub
parent 85dd7f35e3
commit e07a0cc163
6 changed files with 98 additions and 17 deletions

View File

@@ -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']

View File

@@ -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),
),
]

View File

@@ -65,6 +65,7 @@ class CourseResetAudit(TimeStampedModel):
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 """

View File

@@ -26,4 +26,5 @@ class CourseResetAuditFactory(DjangoModelFactory): # lint-amnesty, pylint: disa
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

View File

@@ -2165,6 +2165,7 @@ class TestResetCourseViewGET(SupportViewTestCase):
'course_id': self.course_id,
'display_name': self.course_overview.display_name,
'can_reset': False,
'comment': '',
'status': 'Course Not Started'
}])
@@ -2178,6 +2179,7 @@ class TestResetCourseViewGET(SupportViewTestCase):
'course_id': self.course_id,
'display_name': self.course_overview.display_name,
'can_reset': False,
'comment': '',
'status': 'Course Ended'
}
])
@@ -2189,6 +2191,7 @@ class TestResetCourseViewGET(SupportViewTestCase):
'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 @@ class TestResetCourseViewGET(SupportViewTestCase):
'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 @@ class TestResetCourseViewGET(SupportViewTestCase):
'course_id': self.course_id,
'display_name': self.course.display_name,
'can_reset': True,
'comment': '',
'status': 'Available'
}])
@@ -2237,29 +2242,39 @@ class TestResetCourseViewGET(SupportViewTestCase):
'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 @@ class TestResetCourseViewGET(SupportViewTestCase):
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 @@ class TestResetCourseViewGET(SupportViewTestCase):
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 @@ class TestResetCourseViewPost(SupportViewTestCase):
@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(

View File

@@ -19,6 +19,13 @@ from ..tasks import reset_student_course
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 @@ class CourseResetAPIView(APIView):
'course_id': <course id>
'display_name': <course display name>
'status': <status of the enrollment wrt/reset, to be displayed to user>
'comment': <comment left by user performing reset. may be blank>
'can_reset': (boolean) <can the course be reset for this learner>
}
]
@@ -88,10 +95,12 @@ class CourseResetAPIView(APIView):
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 @@ class CourseResetAPIView(APIView):
@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': <course id>
@@ -108,6 +121,7 @@ class CourseResetAPIView(APIView):
'can_reset': (boolean) <can the course be reset for this learner>
}
"""
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 @@ class CourseResetAPIView(APIView):
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 @@ class CourseResetAPIView(APIView):
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
}