diff --git a/lms/djangoapps/support/tests/test_views.py b/lms/djangoapps/support/tests/test_views.py index 7fff6117b4ab..b3a532760c1b 100644 --- a/lms/djangoapps/support/tests/test_views.py +++ b/lms/djangoapps/support/tests/test_views.py @@ -54,7 +54,7 @@ from common.djangoapps.third_party_auth.tests.factories import SAMLProviderConfigFactory from common.test.utils import disable_signal from lms.djangoapps.program_enrollments.tests.factories import ProgramCourseEnrollmentFactory, ProgramEnrollmentFactory -from lms.djangoapps.support.models import CourseResetAudit, CourseResetCourseOptIn +from lms.djangoapps.support.models import CourseResetAudit from lms.djangoapps.support.serializers import ProgramEnrollmentSerializer from lms.djangoapps.support.tests.factories import CourseResetCourseOptInFactory, CourseResetAuditFactory from lms.djangoapps.verify_student.models import VerificationDeadline @@ -2071,10 +2071,10 @@ def test_verified_in_another_course(self): self.assertEqual(response_data['current_status']['course_id'], self.course_id) -@ddt.ddt -class TestResetCourseViewGET(SupportViewTestCase): - """ Tests for the list endpoint for course reset """ - +class ResetCourseViewTestBase(SupportViewTestCase): + """ + Shared base class for course reset view tests + """ def _url(self, username): """ Helper to generate URL """ return reverse("support:course_reset", kwargs={'username_or_email': username}) @@ -2086,7 +2086,6 @@ def setUp(self): super().setUp() SupportStaffRole().add_users(self.user) self.now = datetime.now().replace(tzinfo=UTC) - self.course = CourseFactory.create( start=self.now - timedelta(days=90), end=self.now + timedelta(days=90), @@ -2097,6 +2096,11 @@ def setUp(self): self.enrollment = CourseEnrollmentFactory.create(user=self.learner, course_id=self.course.id) self.opt_in = CourseResetCourseOptInFactory.create(course_id=self.course.id) + +@ddt.ddt +class TestResetCourseListView(ResetCourseViewTestBase): + """ Tests for the list endpoint for course reset """ + def assert_course_ids(self, expected_course_ids, learner=None): """ Helper that asserts the course ids that will be returned from the listing endpoint """ learner = learner or self.learner @@ -2117,7 +2121,7 @@ def test_not_opted_in(self): it will not be returned by the endpoint """ non_opted_in_course = CourseFactory.create() - enrollment = CourseEnrollmentFactory.create(user=self.learner, course_id=non_opted_in_course.id) + CourseEnrollmentFactory.create(user=self.learner, course_id=non_opted_in_course.id) self.assert_course_ids([self.course_id]) def test_deactivated_opt_in(self): @@ -2125,7 +2129,6 @@ def test_deactivated_opt_in(self): If a learner is enrolled in a course that has opted in, but that opt-in is deactivated, it will not be returned from the endpoint """ - response = self.client.get(self._url(self.learner)) self.assert_course_ids([self.course_id]) self.opt_in.active = False @@ -2138,7 +2141,6 @@ def test_deactivated_enrollment(self): If a learner's enrollment in an opted in course is deactivated, the course will not be returned by the endpoint """ - response = self.client.get(self._url(self.learner)) self.assert_course_ids([self.course_id]) self.enrollment.is_active = False @@ -2146,10 +2148,9 @@ def test_deactivated_enrollment(self): self.assert_course_ids([]) - def assertResponse(self, expected_response, learner=None): + def assertResponse(self, expected_response): """ Helper to assert the contents of the response from the listing endpoint """ - learner = learner or self.learner - response = self.client.get(self._url(learner)) + response = self.client.get(self._url(self.learner)) self.assertEqual(response.status_code, 200) actual_response = response.json() @@ -2350,60 +2351,74 @@ def test_multiple_failed_audits(self): }]) -class TestResetCourseViewPost(SupportViewTestCase): +class TestResetCourseCreateView(ResetCourseViewTestBase): """ - Tests for creating course request + Tests for POST endpoint for performing course reset """ - def setUp(self): - super().setUp() - SupportStaffRole().add_users(self.user) + def request(self, username=None, course_id=None, comment=None): + """ Helper to perform request """ + username = username or self.learner.username + return self.client.post( + self._url(username), + data={ + "course_id": course_id if course_id else self.course_id, + "comment": comment if comment else "" + } + ) - self.course_id = 'course-v1:a+b+c' + def assert_error_response(self, response, expected_status_code, expected_error_message): + """ Helper to assert status code and error message """ + self.assertEqual(response.status_code, expected_status_code) + self.assertEqual(response.data['error'], expected_error_message) - self.other_user = User.objects.create(username='otheruser', password='test') + def test_wrong_username(self): + """ A request with a username which does not exits returns 404 """ + response = self.request(username='does_not_exist') + self.assert_error_response(response, 404, "User does not exist") - self.course = CourseFactory.create( - org='a', - course='b', - run='c', - enable_proctored_exams=True, - proctoring_provider=settings.PROCTORING_BACKENDS['DEFAULT'], - ) - self.enrollment = CourseEnrollmentFactory( - is_active=True, - mode='verified', - course_id=self.course.id, - user=self.user - ) - self.opt_in = CourseResetCourseOptInFactory.create(course_id=self.course.id) + def test_invalid_course_id(self): + """ A request for an invalid course id returns 400 """ + response = self.request(course_id='thisisnotacourseid') + self.assert_error_response(response, 400, "invalid course id") - self.other_course = CourseFactory.create( - org='x', - course='y', - run='z', - ) + def test_missing_course_id(self): + """ A request without a course id returns 400 """ + response = self.client.post(self._url(self.learner.username)) + self.assert_error_response(response, 400, "Must specify course id") - def _url(self, username): - return reverse("support:course_reset", kwargs={'username_or_email': username}) + def test_course_not_opt_in(self): + """ A request for a course which isn't opted into the feature returns 404 """ + self.opt_in.active = False + self.opt_in.save() - def test_wrong_username(self): - """ - Test that a request with a username which does not exits returns 404 - """ - response = self.client.post(self._url(username='does_not_exist'), data={'course_id': 'course-v1:aa+bb+c'}) - self.assertEqual(response.status_code, 404) + response = self.request() + self.assert_error_response(response, 404, "Course is not eligible") + + def test_unenrolled(self): + """ A request for a learner who isn't enrolled in the given course returns a 404 """ + self.enrollment.is_active = False + self.enrollment.save() + + response = self.request() + self.assert_error_response(response, 404, "Learner is not enrolled in course") + + @patch('lms.djangoapps.support.views.course_reset.can_enrollment_be_reset') + def test_cannot_reset(self, mock_can_reset): + """ A request for a course which isn't able to be reset returns a 404 """ + mock_status = str(uuid4()) + mock_can_reset.return_value = (False, mock_status) + + response = self.request() + self.assert_error_response(response, 400, f"Cannot reset course: {mock_status}") @patch('lms.djangoapps.support.views.course_reset.reset_student_course') def test_learner_course_reset(self, mock_reset_student_course): + """ Happy path test """ comment = str(uuid4()) - response = self.client.post( - self._url(username=self.user.username), - data={ - 'course_id': self.course_id, - 'comment': comment, - } - ) + + # A request for a given learner and course with a comment should return a 201 + response = self.request(comment=comment) self.assertEqual(response.status_code, 201) self.assertEqual(response.data, { 'course_id': self.course_id, @@ -2412,49 +2427,40 @@ def test_learner_course_reset(self, mock_reset_student_course): 'comment': comment, 'display_name': self.course.display_name }) + # The reset task should be queued + mock_reset_student_course.delay.assert_called_once_with(self.course_id, self.learner.id, self.user.id) + # And an audit should be created as ENQUEUED self.assertEqual( - mock_reset_student_course.delay.call_count, 1 + self.enrollment.courseresetaudit_set.first().status, + CourseResetAudit.CourseResetStatus.ENQUEUED ) - def test_course_not_opt_in(self): - response = self.client.post(self._url(username=self.user.username), data={'course_id': 'course-v1:aa+bb+c'}) - self.assertEqual(response.status_code, 404) - @patch('lms.djangoapps.support.views.course_reset.reset_student_course') def test_course_reset_failed(self, mock_reset_student_course): - course = CourseFactory.create( - org='xx', - course='yy', - run='zz', - ) - enrollment = CourseEnrollmentFactory( - is_active=True, - mode='verified', - course_id=course.id, - user=self.user - ) - - opt_in_course = CourseResetCourseOptIn.objects.create( - course_id=course.id, - active=True - ) - + """ An audit that has failed previously should be able to be run successfully """ CourseResetAudit.objects.create( - course=opt_in_course, - course_enrollment=enrollment, - reset_by=self.other_user, + course=self.opt_in, + course_enrollment=self.enrollment, + reset_by=self.user, status=CourseResetAudit.CourseResetStatus.FAILED ) - response = self.client.post(self._url(username=self.user.username), data={'course_id': course.id}) - self.assertEqual( - mock_reset_student_course.delay.call_count, 1 - ) - self.assertEqual(response.status_code, 200) + response = self.request() + self.assertEqual(response.status_code, 201) + self.assertEqual(response.data, { + 'course_id': self.course_id, + 'status': response.data['status'], + 'can_reset': False, + 'comment': '', + 'display_name': self.course.display_name + }) + mock_reset_student_course.delay.assert_called_once_with(self.course_id, self.learner.id, self.user.id) - def test_course_reset_dupe(self): - CourseResetAuditFactory.create( + def test_course_reset_already_reset(self): + """ A course that has an audit that hasn't failed should not be allowed to be run again """ + additional_audit = CourseResetAuditFactory.create( course=self.opt_in, course_enrollment=self.enrollment, + status=CourseResetAudit.CourseResetStatus.ENQUEUED ) - response2 = self.client.post(self._url(username=self.user.username), data={'course_id': self.course_id}) - self.assertEqual(response2.status_code, 204) + response = self.request() + self.assert_error_response(response, 400, f"Cannot reset course: {additional_audit.status_message()}") diff --git a/lms/djangoapps/support/views/course_reset.py b/lms/djangoapps/support/views/course_reset.py index af7314d7a60c..256d6df9d78c 100644 --- a/lms/djangoapps/support/views/course_reset.py +++ b/lms/djangoapps/support/views/course_reset.py @@ -3,10 +3,11 @@ from rest_framework.response import Response from django.contrib.auth import get_user_model from django.utils.decorators import method_decorator +from opaque_keys import InvalidKeyError +from opaque_keys.edx.keys import CourseKey from rest_framework.views import APIView from rest_framework.permissions import IsAuthenticated - from common.djangoapps.student.models import CourseEnrollment, get_user_by_username_or_email from common.djangoapps.student.helpers import user_has_passing_grade_in_course from lms.djangoapps.support.decorators import require_support_permission @@ -118,67 +119,46 @@ def post(self, request, username_or_email): 'course_id': 'display_name': 'status': + 'comment': 'can_reset': (boolean) } """ - comment = request.data.get('comment', '') - course_id = request.data['course_id'] try: + course_id = request.data['course_id'] + course_key = CourseKey.from_string(course_id) user = get_user_by_username_or_email(username_or_email) + opt_in_course = CourseResetCourseOptIn.objects.get(course_id=course_key, active=True) + except KeyError: + return Response({'error': 'Must specify course id'}, status=400) + except InvalidKeyError: + return Response({'error': 'invalid course id'}, status=400) except User.DoesNotExist: return Response({'error': 'User does not exist'}, status=404) - try: - opt_in_course = CourseResetCourseOptIn.objects.get(course_id=course_id) except CourseResetCourseOptIn.DoesNotExist: return Response({'error': 'Course is not eligible'}, status=404) - enrollment = CourseEnrollment.objects.get( - course=course_id, - user=user, - is_active=True + + if not CourseEnrollment.is_enrolled(user, course_key): + return Response({'error': 'Learner is not enrolled in course'}, status=404) + course_enrollment = CourseEnrollment.get_enrollment(user, course_key) + + can_reset, status_message = can_enrollment_be_reset(course_enrollment) + if not can_reset: + return Response({'error': f'Cannot reset course: {status_message}'}, status=400) + + course_reset_audit = CourseResetAudit.objects.create( + course=opt_in_course, + course_enrollment=course_enrollment, + reset_by=request.user, + comment=request.data.get('comment', ''), ) - user_passed = user_has_passing_grade_in_course(enrollment=enrollment) - course_overview = enrollment.course_overview - course_reset_audit = CourseResetAudit.objects.filter(course_enrollment=enrollment).first() - - if course_reset_audit and ( - course_reset_audit.status == CourseResetAudit.CourseResetStatus.FAILED - 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) - - resp = { - 'course_id': course_id, - 'status': course_reset_audit.status_message(), - 'can_reset': False, - 'display_name': course_overview.display_name - } - return Response(resp, status=200) - - elif course_reset_audit and course_reset_audit.status in ( - CourseResetAudit.CourseResetStatus.IN_PROGRESS, - CourseResetAudit.CourseResetStatus.ENQUEUED - ): - return Response(None, status=204) - - if enrollment and opt_in_course and not user_passed: - course_reset_audit = CourseResetAudit.objects.create( - 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 - } - reset_student_course.delay(course_id, user.email, request.user.email) - return Response(resp, status=201) - else: - return Response(None, status=400) + resp = { + 'course_id': course_id, + 'status': course_reset_audit.status_message(), + 'can_reset': False, + 'comment': course_reset_audit.comment, + 'display_name': course_enrollment.course_overview.display_name + } + + reset_student_course.delay(course_id, user.id, request.user.id) + return Response(resp, status=201)