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'): -