Merge branch 'master' into chris/FAL-3788-create-collection

cms/djangoapps/contentstore/rest_api/v1/views/tests/test_proctoring.py

@@ -277,18 +277,14 @@ def test_update_exam_settings_invalid_value(self):
 
         # response is correct
         assert response.status_code == status.HTTP_400_BAD_REQUEST
-        self.assertDictEqual(
-            response.data,
+        self.assertIn(
             {
-                "detail": [
-                    {
-                        "proctoring_provider": (
-                            "The selected proctoring provider, notvalidprovider, is not a valid provider. "
-                            "Please select from one of ['test_proctoring_provider']."
-                        )
-                    }
-                ]
+                "proctoring_provider": (
+                    "The selected proctoring provider, notvalidprovider, is not a valid provider. "
+                    "Please select from one of ['test_proctoring_provider']."
+                )
             },
+            response.data['detail'],
         )
 
         # course settings have been updated
@@ -408,18 +404,14 @@ def test_400_for_disabled_lti(self):
 
         # response is correct
         assert response.status_code == status.HTTP_400_BAD_REQUEST
-        self.assertDictEqual(
-            response.data,
+        self.assertIn(
             {
-                "detail": [
-                    {
-                        "proctoring_provider": (
-                            "The selected proctoring provider, lti_external, is not a valid provider. "
-                            "Please select from one of ['null']."
-                        )
-                    }
-                ]
+                "proctoring_provider": (
+                    "The selected proctoring provider, lti_external, is not a valid provider. "
+                    "Please select from one of ['null']."
+                )
             },
+            response.data['detail'],
         )
 
         # course settings have been updated

cms/djangoapps/contentstore/views/tests/test_exam_settings_view.py

@@ -162,6 +162,39 @@ def test_exam_settings_alert_with_exam_settings_disabled(self, page_handler):
         else:
             assert 'To update these settings go to the Advanced Settings page.' in alert_text
 
+    @override_settings(
+        PROCTORING_BACKENDS={
+            'DEFAULT': 'test_proctoring_provider',
+            'proctortrack': {},
+            'test_proctoring_provider': {},
+        },
+        FEATURES=FEATURES_WITH_EXAM_SETTINGS_ENABLED,
+    )
+    @ddt.data(
+        "advanced_settings_handler",
+        "course_handler",
+    )
+    def test_invalid_provider_alert(self, page_handler):
+        """
+        An alert should appear if the course has a proctoring provider that is not valid.
+        """
+        # create an error by setting an invalid proctoring provider
+        self.course.proctoring_provider = 'invalid_provider'
+        self.course.enable_proctored_exams = True
+        self.save_course()
+
+        url = reverse_course_url(page_handler, self.course.id)
+        resp = self.client.get(url, HTTP_ACCEPT='text/html')
+        alert_text = self._get_exam_settings_alert_text(resp.content)
+        assert (
+            'This course has proctored exam settings that are incomplete or invalid.'
+            in alert_text
+        )
+        assert (
+            'The proctoring provider configured for this course, \'invalid_provider\', is not valid.'
+            in alert_text
+        )
+
     @ddt.data(
         "advanced_settings_handler",
         "course_handler",

cms/djangoapps/models/settings/course_metadata.py

@@ -490,18 +490,31 @@ def validate_proctoring_settings(cls, block, settings_dict, user):
             enable_proctoring = block.enable_proctored_exams
 
         if enable_proctoring:
+
+            if proctoring_provider_model:
+                proctoring_provider = proctoring_provider_model.get('value')
+            else:
+                proctoring_provider = block.proctoring_provider
+
+            # If the proctoring provider stored in the course block no longer
+            # matches the available providers for this instance, show an error
+            if proctoring_provider not in available_providers:
+                message = (
+                    f'The proctoring provider configured for this course, \'{proctoring_provider}\', is not valid.'
+                )
+                errors.append({
+                    'key': 'proctoring_provider',
+                    'message': message,
+                    'model': proctoring_provider_model
+                })
+
             # Require a valid escalation email if Proctortrack is chosen as the proctoring provider
             escalation_email_model = settings_dict.get('proctoring_escalation_email')
             if escalation_email_model:
                 escalation_email = escalation_email_model.get('value')
             else:
                 escalation_email = block.proctoring_escalation_email
 
-            if proctoring_provider_model:
-                proctoring_provider = proctoring_provider_model.get('value')
-            else:
-                proctoring_provider = block.proctoring_provider
-
             missing_escalation_email_msg = 'Provider \'{provider}\' requires an exam escalation contact.'
             if proctoring_provider_model and proctoring_provider == 'proctortrack':
                 if not escalation_email:

common/djangoapps/util/tests/test_db.py

@@ -1,5 +1,6 @@
 """Tests for util.db module."""
 
+import unittest
 from io import StringIO
 
 import ddt
@@ -120,6 +121,9 @@ class MigrationTests(TestCase):
     Tests for migrations.
     """
 
+    @unittest.skip(
+        "Temporary skip for ENT-8972 while the unencrypted client id and secret are removed from sap config."
+    )
     @override_settings(MIGRATION_MODULES={})
     def test_migrations_are_in_sync(self):
         """

lms/djangoapps/instructor/views/api.py

@@ -108,7 +108,7 @@
 from lms.djangoapps.instructor_task.models import ReportStore
 from lms.djangoapps.instructor.views.serializer import (
     AccessSerializer, RoleNameSerializer, ShowStudentExtensionSerializer,
-    UserSerializer, SendEmailSerializer
+    UserSerializer, SendEmailSerializer, StudentAttemptsSerializer
 )
 from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
 from openedx.core.djangoapps.course_groups.cohorts import add_user_to_cohort, is_course_cohorted
@@ -1817,23 +1817,24 @@ def post(self, request, course_id):
         return Response(serializer.data)
 
 
-@transaction.non_atomic_requests
-@require_POST
-@ensure_csrf_cookie
-@cache_control(no_cache=True, no_store=True, must_revalidate=True)
-@require_course_permission(permissions.GIVE_STUDENT_EXTENSION)
-@require_post_params(
-    problem_to_reset="problem urlname to reset"
-)
-@common_exceptions_400
-def reset_student_attempts(request, course_id):
+@method_decorator(cache_control(no_cache=True, no_store=True, must_revalidate=True), name='dispatch')
+@method_decorator(transaction.non_atomic_requests, name='dispatch')
+class ResetStudentAttempts(DeveloperErrorViewMixin, APIView):
     """
-
     Resets a students attempts counter or starts a task to reset all students
     attempts counters. Optionally deletes student state for a problem. Limited
     to staff access. Some sub-methods limited to instructor access.
+    """
+    http_method_names = ['post']
+    permission_classes = (IsAuthenticated, permissions.InstructorPermission)
+    permission_name = permissions.GIVE_STUDENT_EXTENSION
+    serializer_class = StudentAttemptsSerializer
 
-    Takes some of the following query parameters
+    @method_decorator(ensure_csrf_cookie)
+    @transaction.non_atomic_requests
+    def post(self, request, course_id):
+        """
+        Takes some of the following query parameters
         - problem_to_reset is a urlname of a problem
         - unique_student_identifier is an email or username
         - all_students is a boolean
@@ -1843,65 +1844,74 @@ def reset_student_attempts(request, course_id):
         - delete_module is a boolean
             requires instructor access
             mutually exclusive with all_students
-    """
-    course_id = CourseKey.from_string(course_id)
-    course = get_course_with_access(
-        request.user, 'staff', course_id, depth=None
-    )
-    all_students = _get_boolean_param(request, 'all_students')
-
-    if all_students and not has_access(request.user, 'instructor', course):
-        return HttpResponseForbidden("Requires instructor access.")
+        """
+        course_id = CourseKey.from_string(course_id)
+        serializer_data = self.serializer_class(data=request.data)
 
-    problem_to_reset = strip_if_string(request.POST.get('problem_to_reset'))
-    student_identifier = request.POST.get('unique_student_identifier', None)
-    student = None
-    if student_identifier is not None:
-        student = get_student_from_identifier(student_identifier)
-    delete_module = _get_boolean_param(request, 'delete_module')
+        if not serializer_data.is_valid():
+            return HttpResponseBadRequest(reason=serializer_data.errors)
 
-    # parameter combinations
-    if all_students and student:
-        return HttpResponseBadRequest(
-            "all_students and unique_student_identifier are mutually exclusive."
-        )
-    if all_students and delete_module:
-        return HttpResponseBadRequest(
-            "all_students and delete_module are mutually exclusive."
+        course = get_course_with_access(
+            request.user, 'staff', course_id, depth=None
         )
 
-    try:
-        module_state_key = UsageKey.from_string(problem_to_reset).map_into_course(course_id)
-    except InvalidKeyError:
-        return HttpResponseBadRequest()
+        all_students = serializer_data.validated_data.get('all_students')
 
-    response_payload = {}
-    response_payload['problem_to_reset'] = problem_to_reset
+        if all_students and not has_access(request.user, 'instructor', course):
+            return HttpResponseForbidden("Requires instructor access.")
 
-    if student:
-        try:
-            enrollment.reset_student_attempts(
-                course_id,
-                student,
-                module_state_key,
-                requesting_user=request.user,
-                delete_module=delete_module
+        problem_to_reset = strip_if_string(serializer_data.validated_data.get('problem_to_reset'))
+        student_identifier = request.POST.get('unique_student_identifier', None)
+        student = serializer_data.validated_data.get('unique_student_identifier')
+        delete_module = serializer_data.validated_data.get('delete_module')
+
+        # parameter combinations
+        if all_students and student:
+            return HttpResponseBadRequest(
+                "all_students and unique_student_identifier are mutually exclusive."
+            )
+        if all_students and delete_module:
+            return HttpResponseBadRequest(
+                "all_students and delete_module are mutually exclusive."
             )
-        except StudentModule.DoesNotExist:
-            return HttpResponseBadRequest(_("Module does not exist."))
-        except sub_api.SubmissionError:
-            # Trust the submissions API to log the error
-            error_msg = _("An error occurred while deleting the score.")
-            return HttpResponse(error_msg, status=500)
-        response_payload['student'] = student_identifier
-    elif all_students:
-        task_api.submit_reset_problem_attempts_for_all_students(request, module_state_key)
-        response_payload['task'] = TASK_SUBMISSION_OK
-        response_payload['student'] = 'All Students'
-    else:
-        return HttpResponseBadRequest()
 
-    return JsonResponse(response_payload)
+        try:
+            module_state_key = UsageKey.from_string(problem_to_reset).map_into_course(course_id)
+        except InvalidKeyError:
+            return HttpResponseBadRequest()
+
+        response_payload = {}
+        response_payload['problem_to_reset'] = problem_to_reset
+
+        if student:
+            try:
+                enrollment.reset_student_attempts(
+                    course_id,
+                    student,
+                    module_state_key,
+                    requesting_user=request.user,
+                    delete_module=delete_module
+                )
+            except StudentModule.DoesNotExist:
+                return HttpResponseBadRequest(_("Module does not exist."))
+            except sub_api.SubmissionError:
+                # Trust the submissions API to log the error
+                error_msg = _("An error occurred while deleting the score.")
+                return HttpResponse(error_msg, status=500)
+            response_payload['student'] = student_identifier
+
+        elif all_students:
+            try:
+                task_api.submit_reset_problem_attempts_for_all_students(request, module_state_key)
+                response_payload['task'] = TASK_SUBMISSION_OK
+                response_payload['student'] = 'All Students'
+            except Exception:  # pylint: disable=broad-except
+                error_msg = _("An error occurred while attempting to reset for all students.")
+                return HttpResponse(error_msg, status=500)
+        else:
+            return HttpResponseBadRequest()
+
+        return JsonResponse(response_payload)
 
 
 @transaction.non_atomic_requests
@@ -1938,8 +1948,10 @@ def reset_student_attempts_for_entrance_exam(request, course_id):
 
     student_identifier = request.POST.get('unique_student_identifier', None)
     student = None
+
     if student_identifier is not None:
         student = get_student_from_identifier(student_identifier)
+
     all_students = _get_boolean_param(request, 'all_students')
     delete_module = _get_boolean_param(request, 'delete_module')
 

lms/djangoapps/instructor/views/api_urls.py

@@ -34,7 +34,7 @@
     path('get_anon_ids', api.GetAnonIds.as_view(), name='get_anon_ids'),
     path('get_student_enrollment_status', api.get_student_enrollment_status, name="get_student_enrollment_status"),
     path('get_student_progress_url', api.StudentProgressUrl.as_view(), name='get_student_progress_url'),
-    path('reset_student_attempts', api.reset_student_attempts, name='reset_student_attempts'),
+    path('reset_student_attempts', api.ResetStudentAttempts.as_view(), name='reset_student_attempts'),
     path('rescore_problem', api.rescore_problem, name='rescore_problem'),
     path('override_problem_score', api.override_problem_score, name='override_problem_score'),
     path('reset_student_attempts_for_entrance_exam', api.reset_student_attempts_for_entrance_exam,

lms/djangoapps/instructor/views/serializer.py

@@ -79,6 +79,57 @@ def validate_student(self, value):
         return user
 
 
+class StudentAttemptsSerializer(serializers.Serializer):
+    """
+    Serializer for resetting a students attempts counter or starts a task to reset all students
+    attempts counters.
+    """
+    problem_to_reset = serializers.CharField(
+        help_text="The identifier or description of the problem that needs to be reset."
+    )
+
+    # following are optional params.
+    unique_student_identifier = serializers.CharField(
+        help_text="Email or username of student.", required=False
+    )
+    all_students = serializers.CharField(required=False)
+    delete_module = serializers.CharField(required=False)
+
+    def validate_all_students(self, value):
+        """
+        converts the all_student params value to bool.
+        """
+        return self.verify_bool(value)
+
+    def validate_delete_module(self, value):
+        """
+        converts the all_student params value.
+        """
+        return self.verify_bool(value)
+
+    def validate_unique_student_identifier(self, value):
+        """
+        Validate that the student corresponds to an existing user.
+        """
+        try:
+            user = get_student_from_identifier(value)
+        except User.DoesNotExist:
+            return None
+
+        return user
+
+    def verify_bool(self, value):
+        """
+        Returns the value of the boolean parameter with the given
+        name in the POST request. Handles translation from string
+        values to boolean values.
+        """
+        if value is not None:
+            return value in ['true', 'True', True]
+
+        return False
+
+
 class SendEmailSerializer(serializers.Serializer):
     """
     Serializer for sending an email with optional scheduling.