fix: do not validate proctoring_provider if proctoring is disabled (#29803)

Co-authored-by: Simon Chen <schen@edx-c02fw0guml85.lan>

cms/djangoapps/contentstore/api/tests/test_validation.py

@@ -16,7 +16,7 @@ from xmodule.modulestore.tests.django_utils import TEST_DATA_SPLIT_MODULESTORE,
 from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory  # lint-amnesty, pylint: disable=wrong-import-order
 
 
-@override_settings(PROCTORING_BACKENDS={'DEFAULT': 'null', 'proctortrack': {}})
+@override_settings(PROCTORING_BACKENDS={'DEFAULT': 'proctortrack', 'proctortrack': {}})
 class CourseValidationViewTest(SharedModuleStoreTestCase, APITestCase):
     """
     Test course validation view via a RESTful API

cms/djangoapps/contentstore/rest_api/v1/tests/test_views.py

@@ -4,6 +4,7 @@ Unit tests for Contentstore views.
 
 import ddt
 from mock import patch
+from django.conf import settings
 from django.test.utils import override_settings
 from django.urls import reverse
 from opaque_keys.edx.keys import CourseKey
@@ -150,7 +151,7 @@ class ProctoringExamSettingsPostTests(ProctoringExamSettingsTestMixin, ModuleSto
 
     @override_settings(
         PROCTORING_BACKENDS={
-            'DEFAULT': 'null',
+            'DEFAULT': 'proctortrack',
             'proctortrack': {}
         },
     )
@@ -184,7 +185,7 @@ class ProctoringExamSettingsPostTests(ProctoringExamSettingsTestMixin, ModuleSto
 
     @override_settings(
         PROCTORING_BACKENDS={
-            'DEFAULT': 'null',
+            'DEFAULT': 'test_proctoring_provider',
             'test_proctoring_provider': {}
         },
     )
@@ -250,11 +251,14 @@ class ProctoringExamSettingsPostTests(ProctoringExamSettingsTestMixin, ModuleSto
     )
     def test_update_exam_settings_invalid_value(self):
         self.client.login(username=self.global_staff.username, password=self.password)
-        data = self.get_request_data(
-            enable_proctored_exams=True,
-            proctoring_provider='notvalidprovider',
-        )
-        response = self.make_request(data=data)
+        PROCTORED_EXAMS_ENABLED_FEATURES = settings.FEATURES
+        PROCTORED_EXAMS_ENABLED_FEATURES['ENABLE_PROCTORED_EXAMS'] = True
+        with override_settings(FEATURES=PROCTORED_EXAMS_ENABLED_FEATURES):
+            data = self.get_request_data(
+                enable_proctored_exams=True,
+                proctoring_provider='notvalidprovider',
+            )
+            response = self.make_request(data=data)
 
         # response is correct
         assert response.status_code == status.HTTP_400_BAD_REQUEST
@@ -278,7 +282,7 @@ class ProctoringExamSettingsPostTests(ProctoringExamSettingsTestMixin, ModuleSto
 
     @override_settings(
         PROCTORING_BACKENDS={
-            'DEFAULT': 'null',
+            'DEFAULT': 'proctortrack',
             'proctortrack': {}
         },
     )
@@ -296,7 +300,7 @@ class ProctoringExamSettingsPostTests(ProctoringExamSettingsTestMixin, ModuleSto
 
     @override_settings(
         PROCTORING_BACKENDS={
-            'DEFAULT': 'null',
+            'DEFAULT': 'proctortrack',
             'proctortrack': {},
             'software_secure': {},
         },

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

@@ -18,9 +18,11 @@ FEATURES_WITH_CERTS_ENABLED['CERTIFICATES_HTML_VIEW'] = True
 
 FEATURES_WITH_EXAM_SETTINGS_ENABLED = FEATURES_WITH_CERTS_ENABLED.copy()
 FEATURES_WITH_EXAM_SETTINGS_ENABLED['ENABLE_EXAM_SETTINGS_HTML_VIEW'] = True
+FEATURES_WITH_EXAM_SETTINGS_ENABLED['ENABLE_PROCTORED_EXAMS'] = True
 
 FEATURES_WITH_EXAM_SETTINGS_DISABLED = FEATURES_WITH_CERTS_ENABLED.copy()
 FEATURES_WITH_EXAM_SETTINGS_DISABLED['ENABLE_EXAM_SETTINGS_HTML_VIEW'] = False
+FEATURES_WITH_EXAM_SETTINGS_DISABLED['ENABLE_PROCTORED_EXAMS'] = True
 
 
 @ddt.ddt

common/djangoapps/student/tests/test_enrollment.py

@@ -288,6 +288,7 @@ class EnrollmentTest(UrlResetMixin, SharedModuleStoreTestCase, OpenEdxEventsTest
     @patch.dict(
         'django.conf.settings.PROCTORING_BACKENDS', {'test_provider_honor_mode': {'allow_honor_mode': True}}
     )
+    @patch.dict(settings.FEATURES, {'ENABLE_PROCTORED_EXAMS': True})
     def test_enroll_in_proctored_course_honor_mode_allowed(self):
         """
         If the proctoring provider allows honor mode, send proctoring requirements email when learners

common/lib/xmodule/xmodule/course_module.py

@@ -213,9 +213,14 @@ class ProctoringProvider(String):
         and include any inherited values from the platform default.
         """
         value = super().from_json(value)
-        self._validate_proctoring_provider(value)
-        value = self._get_proctoring_value(value)
-        return value
+        if settings.FEATURES.get('ENABLE_PROCTORED_EXAMS'):
+            # Only validate the provider value if ProctoredExams are enabled on the environment
+            # Otherwise, the passed in provider does not matter. We should always return default
+            self._validate_proctoring_provider(value)
+            value = self._get_proctoring_value(value)
+            return value
+        else:
+            return self.default
 
     def _get_proctoring_value(self, value):
         """

common/lib/xmodule/xmodule/tests/test_course_module.py

@@ -456,6 +456,7 @@ class CourseBlockTestCase(unittest.TestCase):
         assert self.course.number == COURSE
 
 
+@ddt.ddt
 class ProctoringProviderTestCase(unittest.TestCase):
     """
     Tests for ProctoringProvider, including the default value, validation, and inheritance behavior.
@@ -486,7 +487,8 @@ class ProctoringProviderTestCase(unittest.TestCase):
             'mock_proctoring_without_rules': {}
         }
     )
-    def test_from_json_with_invalid_provider(self):
+    @ddt.data(True, False)
+    def test_from_json_with_invalid_provider(self, proctored_exams_setting_enabled):
         """
         Test that an invalid provider (i.e. not one configured at the platform level)
         throws a ValueError with the correct error message.
@@ -494,11 +496,19 @@ class ProctoringProviderTestCase(unittest.TestCase):
         provider = 'invalid-provider'
         allowed_proctoring_providers = ['mock', 'mock_proctoring_without_rules']
 
-        with pytest.raises(InvalidProctoringProvider) as context_manager:
-            self.proctoring_provider.from_json(provider)
-        expected_error = f'The selected proctoring provider, {provider}, is not a valid provider. ' \
-                         f'Please select from one of {allowed_proctoring_providers}.'
-        assert str(context_manager.value) == expected_error
+        FEATURES_WITH_PROCTORED_EXAMS = settings.FEATURES.copy()
+        FEATURES_WITH_PROCTORED_EXAMS['ENABLE_PROCTORED_EXAMS'] = proctored_exams_setting_enabled
+
+        with override_settings(FEATURES=FEATURES_WITH_PROCTORED_EXAMS):
+            if proctored_exams_setting_enabled:
+                with pytest.raises(InvalidProctoringProvider) as context_manager:
+                    self.proctoring_provider.from_json(provider)
+                expected_error = f'The selected proctoring provider, {provider}, is not a valid provider. ' \
+                    f'Please select from one of {allowed_proctoring_providers}.'
+                assert str(context_manager.value) == expected_error
+            else:
+                provider_value = self.proctoring_provider.from_json(provider)
+                assert provider_value == self.proctoring_provider.default
 
     def test_from_json_adds_platform_default_for_missing_provider(self):
         """

lms/djangoapps/courseware/tests/test_rules.py

@@ -14,7 +14,13 @@ from xmodule.modulestore.tests.factories import CourseFactory  # lint-amnesty, p
 
 
 @ddt.ddt
-@patch.dict('django.conf.settings.FEATURES', {'ENABLE_SPECIAL_EXAMS': True})
+@patch.dict(
+    'django.conf.settings.FEATURES',
+    {
+        'ENABLE_SPECIAL_EXAMS': True,
+        'ENABLE_PROCTORED_EXAMS': True,
+    }
+)
 class PermissionTests(ModuleStoreTestCase):
     """
     Tests for permissions defined in courseware.rules

lms/djangoapps/instructor/tests/test_proctoring.py

@@ -160,13 +160,15 @@ class TestProctoringDashboardViews(SharedModuleStoreTestCase):
         self.instructor.save()
         self._assert_escalation_email_available(False)
 
-    @patch.dict(settings.PROCTORING_BACKENDS,
-                {
-                    'DEFAULT': 'test_proctoring_provider',
-                    'test_proctoring_provider': {},
-                    'proctortrack': {}
-                },
-                )
+    @patch.dict(
+        settings.PROCTORING_BACKENDS,
+        {
+            'DEFAULT': 'test_proctoring_provider',
+            'test_proctoring_provider': {},
+            'proctortrack': {}
+        },
+    )
+    @patch.dict(settings.FEATURES, {'ENABLE_PROCTORED_EXAMS': True})
     def test_proctortrack_provider_with_email(self):
         """
         Escalation email will be visible if proctortrack is the proctoring provider, and there