From 8ad4d42e3b00786a11fc6ec936b191254fde41cc Mon Sep 17 00:00:00 2001 From: Muhammad Labeeb <72980976+mlabeeb03@users.noreply.github.com> Date: Tue, 25 Nov 2025 19:37:32 +0500 Subject: [PATCH] feat!: Remove proctortrack references; add requires_escalation_email and show_review_rules options (#37576) BREAKING CHANGE: All references to the hardcoded 'proctortrack' string have been removed from the codebase, as well as the `studio.show_review_rules` waffle flag. These were used to determine whether an escalation email is required and whether review rules should be shown. These decisions are now made based on the value of 'requires_escalation_email' (default False) and 'show_review_rules' (default True) config items in the PROCTORING_BACKENDS entry. Additionally: * The proctoring info api will now return the list of providers which require an escalation email so that frontend-app-learning does not need to use a hardcoded check agaist the provider name 'proctortrack'. * Removed translation commands, mock variables and user facing strings that contained 'proctortrack'. * Updated all test cases that were using proctortrack to use fake providers names. Part of: https://github.com/openedx/edx-platform/issues/36329 --- Makefile | 2 - .../contentstore/api/tests/test_validation.py | 9 ++- .../api/views/course_validation.py | 5 +- cms/djangoapps/contentstore/config/waffle.py | 4 -- .../rest_api/v1/serializers/proctoring.py | 5 +- .../rest_api/v1/views/proctoring.py | 18 +++++- .../v1/views/tests/test_proctoring.py | 28 +++++---- .../tests/test_course_settings.py | 62 ++++++++++--------- .../contentstore/views/tests/test_block.py | 45 +++++++++++++- .../views/tests/test_exam_settings_view.py | 13 ++-- .../xblock_storage_handlers/view_handlers.py | 11 +--- .../models/settings/course_metadata.py | 25 ++++---- cms/envs/mock.yml | 8 +-- common/djangoapps/util/proctoring.py | 26 ++++++++ .../0018-standarize-django-po-files.rst | 1 - .../instructor/tests/test_proctoring.py | 14 ++--- .../instructor/views/instructor_dashboard.py | 3 +- lms/envs/common.py | 8 --- lms/envs/mock.yml | 2 +- .../instructor_dashboard_2/special_exams.html | 2 +- openedx/envs/common.py | 28 ++++++++- requirements/edx/kernel.in | 1 - xmodule/course_block.py | 27 +++++++- 23 files changed, 235 insertions(+), 112 deletions(-) create mode 100644 common/djangoapps/util/proctoring.py diff --git a/Makefile b/Makefile index e9d644f39c..057d9839f2 100644 --- a/Makefile +++ b/Makefile @@ -35,8 +35,6 @@ swagger: ## generate the swagger.yaml file extract_translations: ## extract localizable strings from sources i18n_tool extract --no-segment -v cd conf/locale/en/LC_MESSAGES && msgcat djangojs.po underscore.po -o djangojs.po - cd conf/locale/en/LC_MESSAGES && msgcat django.po wiki.po edx_proctoring_proctortrack.po mako.po -o django.po - cd conf/locale/en/LC_MESSAGES && rm wiki.po edx_proctoring_proctortrack.po mako.po underscore.po pull_plugin_translations: ## Pull translations for edx_django_utils.plugins for both lms and cms python manage.py lms pull_plugin_translations --verbose $(ATLAS_OPTIONS) diff --git a/cms/djangoapps/contentstore/api/tests/test_validation.py b/cms/djangoapps/contentstore/api/tests/test_validation.py index 4928d31dc1..8da8004772 100644 --- a/cms/djangoapps/contentstore/api/tests/test_validation.py +++ b/cms/djangoapps/contentstore/api/tests/test_validation.py @@ -21,7 +21,12 @@ from common.djangoapps.student.tests.factories import UserFactory @ddt.ddt -@override_settings(PROCTORING_BACKENDS={'DEFAULT': 'proctortrack', 'proctortrack': {}}) +@override_settings( + PROCTORING_BACKENDS={ + "DEFAULT": "test_proctoring_provider", + "test_proctoring_provider": {"requires_escalation_email": True}, + } +) class CourseValidationViewTest(SharedModuleStoreTestCase, APITestCase): """ Test course validation view via a RESTful API @@ -33,7 +38,7 @@ class CourseValidationViewTest(SharedModuleStoreTestCase, APITestCase): cls.course = CourseFactory.create( display_name='test course', run="Testing_course", - proctoring_provider='proctortrack', + proctoring_provider='test_proctoring_provider', proctoring_escalation_email='test@example.com', ) cls.course_key = cls.course.id diff --git a/cms/djangoapps/contentstore/api/views/course_validation.py b/cms/djangoapps/contentstore/api/views/course_validation.py index be5208430a..c55f58d945 100644 --- a/cms/djangoapps/contentstore/api/views/course_validation.py +++ b/cms/djangoapps/contentstore/api/views/course_validation.py @@ -8,6 +8,7 @@ from rest_framework.response import Response from cms.djangoapps.contentstore.course_info_model import get_course_updates from cms.djangoapps.contentstore.views.certificates import CertificateManager +from common.djangoapps.util.proctoring import requires_escalation_email from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, view_auth_classes from xmodule.course_metadata_utils import DEFAULT_GRADING_POLICY # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order @@ -340,8 +341,8 @@ class CourseValidationView(DeveloperErrorViewMixin, GenericAPIView): return False def _proctoring_validation(self, course): - # A proctoring escalation email is currently only required for courses using Proctortrack + # A proctoring escalation email is required if 'required_escalation_email' is set on the proctoring backend return dict( - needs_proctoring_escalation_email=course.proctoring_provider == 'proctortrack', + needs_proctoring_escalation_email=requires_escalation_email(course.proctoring_provider), has_proctoring_escalation_email=bool(course.proctoring_escalation_email) ) diff --git a/cms/djangoapps/contentstore/config/waffle.py b/cms/djangoapps/contentstore/config/waffle.py index ae0e6ea467..40d1181766 100644 --- a/cms/djangoapps/contentstore/config/waffle.py +++ b/cms/djangoapps/contentstore/config/waffle.py @@ -22,10 +22,6 @@ ENABLE_CHECKLISTS_QUALITY = CourseWaffleFlag( # lint-amnesty, pylint: disable=t f'{WAFFLE_NAMESPACE}.enable_checklists_quality', __name__, LOG_PREFIX ) -SHOW_REVIEW_RULES_FLAG = CourseWaffleFlag( # lint-amnesty, pylint: disable=toggle-missing-annotation - f'{WAFFLE_NAMESPACE}.show_review_rules', __name__, LOG_PREFIX -) - # .. toggle_name: studio.custom_relative_dates # .. toggle_implementation: CourseWaffleFlag diff --git a/cms/djangoapps/contentstore/rest_api/v1/serializers/proctoring.py b/cms/djangoapps/contentstore/rest_api/v1/serializers/proctoring.py index 8398bd8ad6..f13ddd3aab 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/serializers/proctoring.py +++ b/cms/djangoapps/contentstore/rest_api/v1/serializers/proctoring.py @@ -5,7 +5,7 @@ API Serializers for proctoring from rest_framework import serializers from cms.djangoapps.contentstore.rest_api.serializers.common import ProctoringErrorListSerializer -from xmodule.course_block import get_available_providers +from xmodule.course_block import get_available_providers, get_requires_escalation_email_providers class ProctoredExamSettingsSerializer(serializers.Serializer): @@ -29,6 +29,9 @@ class ProctoredExamConfigurationSerializer(serializers.Serializer): """ Serializer for various metadata associated with proctored exam settings. """ proctored_exam_settings = ProctoredExamSettingsSerializer() available_proctoring_providers = serializers.ChoiceField(get_available_providers()) + requires_escalation_email_providers = serializers.ChoiceField( + get_requires_escalation_email_providers() + ) course_start_date = serializers.DateTimeField() diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/proctoring.py b/cms/djangoapps/contentstore/rest_api/v1/views/proctoring.py index 015ff31c29..f654ab874f 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/proctoring.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/proctoring.py @@ -14,7 +14,10 @@ from cms.djangoapps.contentstore.views.course import get_course_and_check_access from cms.djangoapps.contentstore.utils import get_proctored_exam_settings_url from cms.djangoapps.models.settings.course_metadata import CourseMetadata from common.djangoapps.student.auth import has_studio_advanced_settings_access -from xmodule.course_block import get_available_providers # lint-amnesty, pylint: disable=wrong-import-order +from xmodule.course_block import ( + get_available_providers, + get_requires_escalation_email_providers, +) # lint-amnesty, pylint: disable=wrong-import-order from openedx.core.djangoapps.course_apps.toggles import exams_ida_enabled from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, verify_course_exists, view_auth_classes from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order @@ -49,7 +52,8 @@ class ProctoredExamSettingsView(APIView): **Response** - In the case of a 200 response code, the response will proctored exam settings data + In the case of a 200 response code, the response will proctored exam settings data, + list of proctoring backends that have 'requires_escalation_email' set to 'True' as well as other metadata about the course or the requesting user that are necessary for rendering the settings page. @@ -65,7 +69,10 @@ class ProctoredExamSettingsView(APIView): }, "available_proctoring_providers": [ "mockprock", - "proctortrack" + "software_secure", + ], + "requires_escalation_email_providers": [ + "software_secure" ], "course_start_date": "2013-02-05T05:00:00Z", } @@ -108,10 +115,15 @@ class ProctoredExamSettingsView(APIView): data['course_start_date'] = course_metadata['start'].get('value') available_providers = get_available_providers() + requires_escalation_email_providers = ( + get_requires_escalation_email_providers() + ) if not exams_ida_enabled(CourseKey.from_string(course_id)): available_providers.remove('lti_external') + requires_escalation_email_providers.remove('lti_external') data['available_proctoring_providers'] = available_providers + data['requires_escalation_email_providers'] = requires_escalation_email_providers serializer = ProctoredExamConfigurationSerializer(data) diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_proctoring.py b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_proctoring.py index 8e220a334c..9eabf03f1b 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_proctoring.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_proctoring.py @@ -62,6 +62,7 @@ class ProctoringExamSettingsGetTests( }, "course_start_date": "2030-01-01T00:00:00Z", "available_proctoring_providers": ["null"], + "requires_escalation_email_providers": [], } def make_request(self, course_id=None, data=None): @@ -100,6 +101,7 @@ class ProctoringExamSettingsGetTests( }, "course_start_date": "2030-01-01T00:00:00Z", "available_proctoring_providers": ["null"], + "requires_escalation_email_providers": [], } assert response.data == expected_data @@ -122,6 +124,7 @@ class ProctoringExamSettingsGetTests( }, "course_start_date": "2030-01-01T00:00:00Z", "available_proctoring_providers": ["lti_external", "null"], + "requires_escalation_email_providers": ["lti_external"], } assert response.data == expected_data @@ -162,14 +165,17 @@ class ProctoringExamSettingsPostTests( return super().test_course_instructor(expect_status=expect_status) @override_settings( - PROCTORING_BACKENDS={"DEFAULT": "proctortrack", "proctortrack": {}}, + PROCTORING_BACKENDS={ + "DEFAULT": "test_proctoring_provider", + "test_proctoring_provider": {"requires_escalation_email": True}, + }, ) def test_update_exam_settings_200_escalation_email(self): - """update exam settings for provider that requires an escalation email (proctortrack)""" + """update exam settings for provider that requires an escalation email""" self.client.login(username=self.global_staff.username, password=self.password) data = self.get_request_data( enable_proctored_exams=True, - proctoring_provider="proctortrack", + proctoring_provider="test_proctoring_provider", proctoring_escalation_email="foo@bar.com", ) response = self.make_request(data=data) @@ -182,7 +188,7 @@ class ProctoringExamSettingsPostTests( "proctored_exam_settings": { "enable_proctored_exams": True, "allow_proctoring_opt_out": True, - "proctoring_provider": "proctortrack", + "proctoring_provider": "test_proctoring_provider", "proctoring_escalation_email": "foo@bar.com", "create_zendesk_tickets": True, } @@ -192,7 +198,7 @@ class ProctoringExamSettingsPostTests( # course settings have been updated updated = modulestore().get_item(self.course.location) assert updated.enable_proctored_exams is True - assert updated.proctoring_provider == "proctortrack" + assert updated.proctoring_provider == "test_proctoring_provider" assert updated.proctoring_escalation_email == "foo@bar.com" @override_settings( @@ -299,14 +305,17 @@ class ProctoringExamSettingsPostTests( assert response.status_code == status.HTTP_403_FORBIDDEN @override_settings( - PROCTORING_BACKENDS={"DEFAULT": "proctortrack", "proctortrack": {}}, + PROCTORING_BACKENDS={ + "DEFAULT": "test_proctoring_provider", + "test_proctoring_provider": {"requires_escalation_email": True}, + }, ) def test_200_for_instructor_request_compatibility(self): self.client.login(username=self.course_instructor, password=self.password) data = { "proctored_exam_settings": { "enable_proctored_exams": True, - "proctoring_provider": "proctortrack", + "proctoring_provider": "test_proctoring_provider", "proctoring_escalation_email": "foo@bar.com", } } @@ -315,16 +324,13 @@ class ProctoringExamSettingsPostTests( @override_settings( PROCTORING_BACKENDS={ - "DEFAULT": "proctortrack", - "proctortrack": {}, + "DEFAULT": "software_secure", "software_secure": {}, }, ) @patch("logging.Logger.info") @ddt.data( - ("proctortrack", False, False), ("software_secure", True, False), - ("proctortrack", True, True), ("software_secure", False, True), ) @ddt.unpack diff --git a/cms/djangoapps/contentstore/tests/test_course_settings.py b/cms/djangoapps/contentstore/tests/test_course_settings.py index ac8d9d3eba..e40d0c9256 100644 --- a/cms/djangoapps/contentstore/tests/test_course_settings.py +++ b/cms/djangoapps/contentstore/tests/test_course_settings.py @@ -1147,12 +1147,13 @@ class CourseMetadataEditingTest(CourseTestCase): @override_settings( PROCTORING_BACKENDS={ 'DEFAULT': 'test_proctoring_provider', - 'proctortrack': {} + 'test_proctoring_provider': {"requires_escalation_email": True}, }, ) def test_fetch_proctoring_escalation_email_present(self): """ - If 'proctortrack' is an available provider, show the escalation email setting + If proctoring provider has 'requires_escalation_email' set to 'True', + show the escalation email setting """ test_model = CourseMetadata.fetch(self.fullcourse) self.assertIn('proctoring_escalation_email', test_model) @@ -1160,12 +1161,13 @@ class CourseMetadataEditingTest(CourseTestCase): @override_settings( PROCTORING_BACKENDS={ 'DEFAULT': 'test_proctoring_provider', - 'alternate_provider': {} + 'test_proctoring_provider': {} }, ) def test_fetch_proctoring_escalation_email_not_present(self): """ - If 'proctortrack' is not an available provider, don't show the escalation email setting + If proctoring provider does not have 'requires_escalation_email' set to 'True', + don't show the escalation email setting """ test_model = CourseMetadata.fetch(self.fullcourse) self.assertNotIn('proctoring_escalation_email', test_model) @@ -1526,13 +1528,12 @@ class CourseMetadataEditingTest(CourseTestCase): @override_settings( PROCTORING_BACKENDS={ 'DEFAULT': 'test_proctoring_provider', - 'test_proctoring_provider': {}, - 'proctortrack': {} + 'test_proctoring_provider': {"requires_escalation_email": True}, }, ) - def test_validate_update_requires_escalation_email_for_proctortrack(self, include_blank_email): + def test_validate_update_requires_escalation_email_if_relevant_flag_is_set(self, include_blank_email): json_data = { - "proctoring_provider": {"value": 'proctortrack'}, + "proctoring_provider": {"value": 'test_proctoring_provider'}, } if include_blank_email: json_data["proctoring_escalation_email"] = {"value": ""} @@ -1549,14 +1550,13 @@ class CourseMetadataEditingTest(CourseTestCase): self.assertIsNone(test_model) self.assertEqual( errors[0].get('message'), - 'Provider \'proctortrack\' requires an exam escalation contact.' + 'Provider \'test_proctoring_provider\' requires an exam escalation contact.' ) @override_settings( PROCTORING_BACKENDS={ 'DEFAULT': 'test_proctoring_provider', 'test_proctoring_provider': {}, - 'proctortrack': {} } ) def test_validate_update_does_not_require_escalation_email_by_default(self): @@ -1573,14 +1573,14 @@ class CourseMetadataEditingTest(CourseTestCase): @override_settings( PROCTORING_BACKENDS={ - 'DEFAULT': 'proctortrack', - 'proctortrack': {} + 'DEFAULT': 'test_proctoring_provider', + 'test_proctoring_provider': {"requires_escalation_email": True}, }, ) - def test_validate_update_cannot_unset_escalation_email_when_proctortrack_is_provider(self): + def test_validate_update_cannot_unset_escalation_email_when_requires_escalation_email_set_on_provider(self): course = CourseFactory.create() CourseMetadata.update_from_dict( - {"proctoring_provider": 'proctortrack', "enable_proctored_exams": True}, + {"proctoring_provider": 'test_proctoring_provider', "enable_proctored_exams": True}, course, self.user ) @@ -1596,20 +1596,20 @@ class CourseMetadataEditingTest(CourseTestCase): self.assertIsNone(test_model) self.assertEqual( errors[0].get('message'), - 'Provider \'proctortrack\' requires an exam escalation contact.' + 'Provider \'test_proctoring_provider\' requires an exam escalation contact.' ) @override_settings( PROCTORING_BACKENDS={ - 'DEFAULT': 'proctortrack', - 'proctortrack': {} + 'DEFAULT': 'test_proctoring_provider', + 'test_proctoring_provider': {"requires_escalation_email": True}, } ) - def test_validate_update_set_proctortrack_provider_with_valid_escalation_email(self): + def test_validate_update_set_proctoring_provider_with_valid_escalation_email(self): did_validate, errors, test_model = CourseMetadata.validate_and_update_from_json( self.course, { - "proctoring_provider": {"value": "proctortrack"}, + "proctoring_provider": {"value": "test_proctoring_provider"}, "proctoring_escalation_email": {"value": "foo@bar.com"}, }, user=self.user @@ -1621,16 +1621,20 @@ class CourseMetadataEditingTest(CourseTestCase): @override_settings( PROCTORING_BACKENDS={ - 'DEFAULT': 'proctortrack', - 'proctortrack': {} + 'DEFAULT': 'test_proctoring_provider', + 'test_proctoring_provider': {"requires_escalation_email": True}, } ) def test_validate_update_disable_proctoring_with_no_escalation_email(self): course = CourseFactory.create() CourseMetadata.update_from_dict( - {"proctoring_provider": 'proctortrack', "proctoring_escalation_email": '', "enable_proctored_exams": True}, + { + "proctoring_provider": "test_proctoring_provider", + "proctoring_escalation_email": "", + "enable_proctored_exams": True, + }, course, - self.user + self.user, ) did_validate, errors, test_model = CourseMetadata.validate_and_update_from_json( course, @@ -1645,15 +1649,15 @@ class CourseMetadataEditingTest(CourseTestCase): @override_settings( PROCTORING_BACKENDS={ - 'DEFAULT': 'proctortrack', - 'proctortrack': {} + 'DEFAULT': 'test_proctoring_provider', + 'test_proctoring_provider': {"requires_escalation_email": True}, } ) def test_validate_update_disable_proctoring_and_change_escalation_email(self): did_validate, errors, test_model = CourseMetadata.validate_and_update_from_json( self.course, { - "proctoring_provider": {"value": "proctortrack"}, + "proctoring_provider": {"value": "test_proctoring_provider"}, "proctoring_escalation_email": {"value": ""}, "enable_proctored_exams": {"value": False}, }, @@ -1667,14 +1671,14 @@ class CourseMetadataEditingTest(CourseTestCase): @override_settings( PROCTORING_BACKENDS={ - 'DEFAULT': 'proctortrack', - 'proctortrack': {} + 'DEFAULT': 'test_proctoring_provider', + 'test_proctoring_provider': {"requires_escalation_email": True}, } ) def test_validate_update_disabled_proctoring_and_unset_escalation_email(self): course = CourseFactory.create() CourseMetadata.update_from_dict( - {"proctoring_provider": 'proctortrack', "enable_proctored_exams": False}, + {"proctoring_provider": 'test_proctoring_provider', "enable_proctored_exams": False}, course, self.user ) diff --git a/cms/djangoapps/contentstore/views/tests/test_block.py b/cms/djangoapps/contentstore/views/tests/test_block.py index bdab677981..46c1f83198 100644 --- a/cms/djangoapps/contentstore/views/tests/test_block.py +++ b/cms/djangoapps/contentstore/views/tests/test_block.py @@ -12,6 +12,7 @@ from django.http import Http404 from django.test import TestCase from django.test.client import RequestFactory from django.urls import reverse +from django.test.utils import override_settings from edx_toggles.toggles.testutils import override_waffle_flag from openedx.core.djangoapps.video_config.toggles import PUBLIC_VIDEO_SHARE from openedx_events.content_authoring.data import DuplicatedXBlockData @@ -3701,10 +3702,49 @@ class TestSpecialExamXBlockInfo(ItemTest): assert xblock_info["proctoring_exam_configuration_link"] == "test_url" assert xblock_info["supports_onboarding"] is True assert xblock_info["is_onboarding_exam"] is False + assert xblock_info["show_review_rules"] is True mock_get_exam_configuration_dashboard_url.assert_called_with( self.course.id, xblock_info["id"] ) + @patch_get_exam_configuration_dashboard_url + @patch_does_backend_support_onboarding + @patch_get_exam_by_content_id_success + @override_settings( + PROCTORING_BACKENDS={ + "DEFAULT": "null", + # By default "show_review_rules" is True unless you explicitly set it to False. + "test_proctoring_provider": {"show_review_rules": False}, + } + ) + def test_show_review_rules_xblock_info( + self, + mock_get_exam_by_content_id, + _mock_does_backend_support_onboarding, + mock_get_exam_configuration_dashboard_url, + ): + # Set course.proctoring_provider to test_proctoring_provider + self.course.proctoring_provider = 'test_proctoring_provider' + sequential = BlockFactory.create( + parent_location=self.chapter.location, + category="sequential", + display_name="Test Lesson 1", + user_id=self.user.id, + is_proctored_enabled=True, + is_time_limited=True, + default_time_limit_minutes=100, + is_onboarding_exam=False, + ) + sequential = modulestore().get_item(sequential.location) + xblock_info = create_xblock_info( + sequential, + include_child_info=True, + include_children_predicate=ALWAYS, + course=self.course, + ) + + assert xblock_info["show_review_rules"] is False + @patch_get_exam_configuration_dashboard_url @patch_does_backend_support_onboarding @patch_get_exam_by_content_id_success @@ -3757,7 +3797,7 @@ class TestSpecialExamXBlockInfo(ItemTest): (None, False), ) @ddt.unpack - def test_xblock_was_ever_proctortrack_proctored_exam( + def test_xblock_was_ever_linked_to_external_exam( self, external_id, expected_value, @@ -3787,7 +3827,7 @@ class TestSpecialExamXBlockInfo(ItemTest): @patch_get_exam_configuration_dashboard_url @patch_does_backend_support_onboarding @patch_get_exam_by_content_id_not_found - def test_xblock_was_never_proctortrack_proctored_exam( + def test_xblock_was_never_linked_to_external_exam( self, mock_get_exam_by_content_id, _mock_does_backend_support_onboarding_patch, @@ -3846,6 +3886,7 @@ class TestSpecialExamXBlockInfo(ItemTest): assert xblock_info["proctoring_exam_configuration_link"] is None assert xblock_info["supports_onboarding"] is True assert xblock_info["is_onboarding_exam"] is False + assert xblock_info["show_review_rules"] is True class TestLibraryXBlockInfo(ModuleStoreTestCase): diff --git a/cms/djangoapps/contentstore/views/tests/test_exam_settings_view.py b/cms/djangoapps/contentstore/views/tests/test_exam_settings_view.py index 88c4aa27eb..735c59acd2 100644 --- a/cms/djangoapps/contentstore/views/tests/test_exam_settings_view.py +++ b/cms/djangoapps/contentstore/views/tests/test_exam_settings_view.py @@ -87,7 +87,7 @@ class TestExamSettingsView(CourseTestCase, UrlResetMixin): @override_settings( PROCTORING_BACKENDS={ 'DEFAULT': 'test_proctoring_provider', - 'proctortrack': {} + 'test_proctoring_provider': {"requires_escalation_email": True} }, ) @ddt.data( @@ -98,9 +98,9 @@ class TestExamSettingsView(CourseTestCase, UrlResetMixin): An alert should appear if current exam settings are invalid. The exam settings alert should direct users to the exam settings page. """ - # create an error by setting proctortrack as the provider and not setting + # create an error by setting 'requires_escalation_email' as 'True' and not setting # the (required) escalation contact - self.course.proctoring_provider = 'proctortrack' + self.course.proctoring_provider = 'test_proctoring_provider' self.course.enable_proctored_exams = True self.save_course() @@ -123,7 +123,7 @@ class TestExamSettingsView(CourseTestCase, UrlResetMixin): @override_settings( PROCTORING_BACKENDS={ 'DEFAULT': 'test_proctoring_provider', - 'proctortrack': {} + 'test_proctoring_provider': {"requires_escalation_email": True} }, ) @ddt.data( @@ -136,9 +136,9 @@ class TestExamSettingsView(CourseTestCase, UrlResetMixin): The exam settings alert should direct users to the advanced settings page if the exam settings feature is not available. """ - # create an error by setting proctortrack as the provider and not setting + # create an error by setting 'requires_escalation_email' as 'True' and not setting # the (required) escalation contact - self.course.proctoring_provider = 'proctortrack' + self.course.proctoring_provider = 'test_proctoring_provider' self.course.enable_proctored_exams = True self.save_course() @@ -164,7 +164,6 @@ class TestExamSettingsView(CourseTestCase, UrlResetMixin): @override_settings( PROCTORING_BACKENDS={ 'DEFAULT': 'test_proctoring_provider', - 'proctortrack': {}, 'test_proctoring_provider': {}, }, ) diff --git a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py index 78c3935323..747641a184 100644 --- a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py +++ b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py @@ -35,7 +35,6 @@ from xblock.core import XBlock from xblock.fields import Scope from .xblock_helpers import get_block_key_string -from cms.djangoapps.contentstore.config.waffle import SHOW_REVIEW_RULES_FLAG from cms.djangoapps.contentstore.helpers import StaticFileNotices from cms.djangoapps.models.settings.course_grading import CourseGradingModel from cms.lib.ai_aside_summary_config import AiAsideSummaryConfig @@ -49,6 +48,7 @@ from common.djangoapps.student.auth import ( ) from common.djangoapps.util.date_utils import get_default_time_display from common.djangoapps.util.json_request import JsonResponse, expect_json +from common.djangoapps.util.proctoring import show_review_rules from openedx.core.djangoapps.bookmarks import api as bookmarks_api from openedx.core.djangoapps.content_tagging.toggles import is_tagging_feature_disabled from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration @@ -1302,13 +1302,6 @@ def create_xblock_info( # lint-amnesty, pylint: disable=too-many-statements f"Error while getting proctoring exam configuration link: {e}" ) - if course.proctoring_provider == "proctortrack": - show_review_rules = SHOW_REVIEW_RULES_FLAG.is_enabled( - xblock.location.course_key - ) - else: - show_review_rules = True - xblock_info.update( { "is_proctored_exam": xblock.is_proctored_exam, @@ -1323,7 +1316,7 @@ def create_xblock_info( # lint-amnesty, pylint: disable=too-many-statements "default_time_limit_minutes": xblock.default_time_limit_minutes, "proctoring_exam_configuration_link": proctoring_exam_configuration_link, "supports_onboarding": supports_onboarding, - "show_review_rules": show_review_rules, + "show_review_rules": show_review_rules(course.proctoring_provider), } ) diff --git a/cms/djangoapps/models/settings/course_metadata.py b/cms/djangoapps/models/settings/course_metadata.py index 1cd87ed5a0..c66f5b65e4 100644 --- a/cms/djangoapps/models/settings/course_metadata.py +++ b/cms/djangoapps/models/settings/course_metadata.py @@ -14,6 +14,7 @@ from xblock.fields import Scope from cms.djangoapps.contentstore import toggles from common.djangoapps.util.db import MYSQL_MAX_INT, generate_int_id +from common.djangoapps.util.proctoring import requires_escalation_email from common.djangoapps.xblock_django.models import XBlockStudioConfigurationFlag from openedx.core.djangoapps.course_apps.toggles import exams_ida_enabled from openedx.core.djangoapps.discussions.config.waffle_utils import legacy_discussion_experience_enabled @@ -144,10 +145,15 @@ class CourseMetadata: if not COURSE_ENABLE_UNENROLLED_ACCESS_FLAG.is_enabled(course_key=course_key): exclude_list.append('course_visibility') - # Do not show "Proctortrack Exam Escalation Contact" if Proctortrack is not - # an available proctoring backend. - if not settings.PROCTORING_BACKENDS or settings.PROCTORING_BACKENDS.get('proctortrack') is None: - exclude_list.append('proctoring_escalation_email') + # Do not show "Proctoring Exam Escalation Contact" if 'requires_escalation_email' + # is not set on any of the the proctoring backends. + escalation_email_required = False + for provider in get_available_providers(): + if requires_escalation_email(provider): + escalation_email_required = True + break + if not escalation_email_required: + exclude_list.append("proctoring_escalation_email") if not legacy_discussion_experience_enabled(course_key): exclude_list.append('discussion_blackouts') @@ -512,7 +518,7 @@ class CourseMetadata: 'model': proctoring_provider_model }) - # Require a valid escalation email if Proctortrack is chosen as the proctoring provider + # Require a valid escalation email if 'requires_escalation_email' is set in the proctoring backend escalation_email_model = settings_dict.get('proctoring_escalation_email') if escalation_email_model: escalation_email = escalation_email_model.get('value') @@ -520,7 +526,7 @@ class CourseMetadata: escalation_email = block.proctoring_escalation_email missing_escalation_email_msg = 'Provider \'{provider}\' requires an exam escalation contact.' - if proctoring_provider_model and proctoring_provider == 'proctortrack': + if proctoring_provider_model and requires_escalation_email(proctoring_provider): if not escalation_email: message = missing_escalation_email_msg.format(provider=proctoring_provider) errors.append({ @@ -531,7 +537,7 @@ class CourseMetadata: if ( escalation_email_model and not proctoring_provider_model and - proctoring_provider == 'proctortrack' + requires_escalation_email(proctoring_provider) ): if not escalation_email: message = missing_escalation_email_msg.format(provider=proctoring_provider) @@ -548,10 +554,7 @@ class CourseMetadata: else: create_zendesk_tickets = block.create_zendesk_tickets - if ( - (proctoring_provider == 'proctortrack' and create_zendesk_tickets) - or (proctoring_provider == 'software_secure' and not create_zendesk_tickets) - ): + if proctoring_provider == 'software_secure' and not create_zendesk_tickets: LOGGER.info( 'create_zendesk_tickets set to {ticket_value} but proctoring ' 'provider is {provider} for course {course_id}. create_zendesk_tickets ' diff --git a/cms/envs/mock.yml b/cms/envs/mock.yml index 3d6c92d288..980151d3e5 100644 --- a/cms/envs/mock.yml +++ b/cms/envs/mock.yml @@ -638,12 +638,12 @@ PLATFORM_TWITTER_ACCOUNT: '@name' POLICY_CHANGE_GRADES_ROUTING_KEY: edx.lms.core.grades_policy_change PRESS_EMAIL: press@example.com PROCTORING_BACKENDS: - DEFAULT: proctortrack - proctortrack: - base_url: https://testing.verificient.com + DEFAULT: mockprock + mockprock: + base_url: https://testing.mockprock.com client_id: test_client_id client_secret: test_client_secret - integration_specific_email: proctortrack-support@example.com + integration_specific_email: mockprock-support@example.com PROCTORING_SETTINGS: ALLOW_CALLBACK_SIMULATION: true LINK_URLS: diff --git a/common/djangoapps/util/proctoring.py b/common/djangoapps/util/proctoring.py new file mode 100644 index 0000000000..34359d721e --- /dev/null +++ b/common/djangoapps/util/proctoring.py @@ -0,0 +1,26 @@ +""" +Utility methods related to proctoring configurations. +""" + + +from django.conf import settings + + +def requires_escalation_email(proctoring_provider): + """ + Returns the value of 'requires_escalation_email' in the given proctoring backend. + The default value for 'requires_escalation_email' is False. + """ + return settings.PROCTORING_BACKENDS.get(proctoring_provider, {}).get( + "requires_escalation_email", False + ) + + +def show_review_rules(proctoring_provider): + """ + Returns the value of 'show_review_rules' in the given proctoring backend. + The default value for 'show_review_rules' is True. + """ + return settings.PROCTORING_BACKENDS.get(proctoring_provider, {}).get( + "show_review_rules", True + ) diff --git a/docs/decisions/0018-standarize-django-po-files.rst b/docs/decisions/0018-standarize-django-po-files.rst index 4020b7c493..7f008f5545 100644 --- a/docs/decisions/0018-standarize-django-po-files.rst +++ b/docs/decisions/0018-standarize-django-po-files.rst @@ -92,7 +92,6 @@ pofiles into the four files and then combine them into two files respectively: - django-partial.po - mako.po - wiki.po - - edx_proctoring_proctortrack.po platform-js.po: - djangojs-partial.po - djangojs-account-settings-view.po diff --git a/lms/djangoapps/instructor/tests/test_proctoring.py b/lms/djangoapps/instructor/tests/test_proctoring.py index 4b196f307b..6a87538826 100644 --- a/lms/djangoapps/instructor/tests/test_proctoring.py +++ b/lms/djangoapps/instructor/tests/test_proctoring.py @@ -141,7 +141,6 @@ class TestProctoringDashboardViews(SharedModuleStoreTestCase): { 'DEFAULT': 'test_proctoring_provider', 'test_proctoring_provider': {}, - 'proctortrack': {} }, ) @ddt.data( @@ -149,9 +148,9 @@ class TestProctoringDashboardViews(SharedModuleStoreTestCase): ('test_proctoring_provider', 'foo@bar.com') ) @ddt.unpack - def test_non_proctortrack_provider(self, proctoring_provider, escalation_email): + def test_requires_escalation_email_unset(self, proctoring_provider, escalation_email): """ - Escalation email will not be visible if proctortrack is not the proctoring provider, with or without + Escalation email will not be visible if 'requires_escalation_email' is not set, with or without escalation email provided. """ self.setup_course_with_proctoring_backend(proctoring_provider, escalation_email) @@ -164,17 +163,16 @@ class TestProctoringDashboardViews(SharedModuleStoreTestCase): settings.PROCTORING_BACKENDS, { 'DEFAULT': 'test_proctoring_provider', - 'test_proctoring_provider': {}, - 'proctortrack': {} + 'test_proctoring_provider': {"requires_escalation_email": True}, }, ) @patch.dict(settings.FEATURES, {'ENABLE_PROCTORED_EXAMS': True}) - def test_proctortrack_provider_with_email(self): + def test_requires_escalation_email_set_with_email(self): """ - Escalation email will be visible if proctortrack is the proctoring provider, and there + Escalation email will be visible if 'requires_escalation_email' is set, and there is an escalation email provided. """ - self.setup_course_with_proctoring_backend('proctortrack', 'foo@bar.com') + self.setup_course_with_proctoring_backend('test_proctoring_provider', 'foo@bar.com') self.instructor.is_staff = True self.instructor.save() diff --git a/lms/djangoapps/instructor/views/instructor_dashboard.py b/lms/djangoapps/instructor/views/instructor_dashboard.py index 2e1703fa1a..f3b2dc9bc7 100644 --- a/lms/djangoapps/instructor/views/instructor_dashboard.py +++ b/lms/djangoapps/instructor/views/instructor_dashboard.py @@ -34,6 +34,7 @@ from common.djangoapps.student.roles import ( CourseStaffRole ) from common.djangoapps.util.json_request import JsonResponse +from common.djangoapps.util.proctoring import requires_escalation_email from lms.djangoapps.bulk_email.api import is_bulk_email_feature_enabled from lms.djangoapps.bulk_email.models_api import is_bulk_email_disabled_for_course from lms.djangoapps.certificates import api as certs_api @@ -320,7 +321,7 @@ def _section_special_exams(course, access): else: # Only call does_backend_support_onboarding if not using an LTI proctoring provider show_onboarding = does_backend_support_onboarding(course.proctoring_provider) - if proctoring_provider == 'proctortrack': + if requires_escalation_email(course.proctoring_provider): escalation_email = course.proctoring_escalation_email from edx_proctoring.api import is_backend_dashboard_available show_dashboard = is_backend_dashboard_available(course_key) diff --git a/lms/envs/common.py b/lms/envs/common.py index 85b4b8af06..03eb67cd5d 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -3343,14 +3343,6 @@ VIDEO_UPLOAD_PIPELINE = { 'ROOT_PATH': '', } -### Proctoring configuration (redirct URLs and keys shared between systems) #### -PROCTORING_BACKENDS = { - 'DEFAULT': 'null', - # The null key needs to be quoted because - # null is a language independent type in YAML - 'null': {} -} - PROCTORED_EXAM_VIEWABLE_PAST_DUE = False ######################### rate limit for yt_video_metadata api ############## diff --git a/lms/envs/mock.yml b/lms/envs/mock.yml index 9c6b077dbe..47a10eea94 100644 --- a/lms/envs/mock.yml +++ b/lms/envs/mock.yml @@ -977,7 +977,7 @@ PRESS_EMAIL: press@example.com PROCTORING_BACKENDS: DEFAULT: 'null' 'null': {} - proctortrack: + mockprock: base_url: hello client_id: '[encrypted]' client_secret: '[encrypted]' diff --git a/lms/templates/instructor/instructor_dashboard_2/special_exams.html b/lms/templates/instructor/instructor_dashboard_2/special_exams.html index 2658af0bc7..d652842df2 100644 --- a/lms/templates/instructor/instructor_dashboard_2/special_exams.html +++ b/lms/templates/instructor/instructor_dashboard_2/special_exams.html @@ -11,7 +11,7 @@ import pytz % else: % if section_data.get('escalation_email'): -