diff --git a/cms/djangoapps/contentstore/tests/test_course_settings.py b/cms/djangoapps/contentstore/tests/test_course_settings.py index 35f1dcaed01178152d53507af40df881a46e006e..fe7d5033b92c40f2b5c79711540f68a630a109c9 100644 --- a/cms/djangoapps/contentstore/tests/test_course_settings.py +++ b/cms/djangoapps/contentstore/tests/test_course_settings.py @@ -1242,6 +1242,31 @@ class CourseMetadataEditingTest(CourseTestCase): ) self.assertIsNone(test_model) + @ddt.data(True, False) + @override_waffle_flag(ENABLE_PROCTORING_PROVIDER_OVERRIDES, False) + def test_validate_update_allows_changes_to_settings_when_proctoring_provider_disabled(self, staff_user): + """ + Course staff can modify Advanced Settings when the proctoring_provider settings is not available (i.e. when + the ENABLE_PROCTORING_PROVIDER_OVERRIDES is not enabled for the course). This ensures that our restrictions + on changing the proctoring_provider do not inhibit users from changing Advanced Settings when the + proctoring_provider setting is not available. + """ + # It doesn't matter what the field is - just check that we can change any field. + field_name = "enable_proctored_exams" + course = CourseFactory.create(start=datetime.datetime.now(UTC) - datetime.timedelta(days=1)) + user = UserFactory.create(is_staff=staff_user) + + did_validate, errors, test_model = CourseMetadata.validate_and_update_from_json( + course, + { + field_name: {"value": True}, + }, + user=user + ) + self.assertTrue(did_validate) + self.assertEqual(len(errors), 0) + self.assertIn(field_name, test_model) + @override_settings( PROCTORING_BACKENDS={ 'DEFAULT': 'test_proctoring_provider', diff --git a/cms/djangoapps/models/settings/course_metadata.py b/cms/djangoapps/models/settings/course_metadata.py index 8d1a900188247a85896c25639e428c82c118237a..5630d40d360ae4cf2901bf596e8f1ab9c4bf6184 100644 --- a/cms/djangoapps/models/settings/course_metadata.py +++ b/cms/djangoapps/models/settings/course_metadata.py @@ -250,9 +250,15 @@ class CourseMetadata(object): # Disallow updates to the proctoring provider after course start proctoring_provider_model = filtered_dict.get('proctoring_provider') + + # If the user is not edX staff, the user has requested a change to the proctoring_provider + # Advanced Setting, and and it is after course start, prevent the user from changing the + # proctoring provider. if ( not user.is_staff and - proctoring_provider_model != descriptor.proctoring_provider and + cls._has_requested_proctoring_provider_changed( + descriptor.proctoring_provider, proctoring_provider_model + ) and datetime.now(pytz.UTC) > descriptor.start ): did_validate = False @@ -260,7 +266,7 @@ class CourseMetadata(object): 'The proctoring provider cannot be modified after a course has started.' ' Contact {support_email} for assistance' ).format(support_email=settings.PARTNER_SUPPORT_EMAIL or 'support') - errors.append({'message': message, 'model': filtered_dict.get('proctoring_provider')}) + errors.append({'message': message, 'model': proctoring_provider_model}) # If did validate, go ahead and update the metadata if did_validate: @@ -268,6 +274,21 @@ class CourseMetadata(object): return did_validate, errors, updated_data + @staticmethod + def _has_requested_proctoring_provider_changed(current_provider, requested_provider): + """ + Return whether the requested proctoring provider is different than the current proctoring provider, indicating + that the user has requested a change to the proctoring_provider Advanced Setting. + + The requested_provider will be None if the proctoring_provider setting is not available (e.g. if the + ENABLE_PROCTORING_PROVIDER_OVERRIDES waffle flag is not enabled for the course). In this case, we consider + that there is no change in the requested proctoring provider. + """ + if requested_provider is None: + return False + else: + return current_provider != requested_provider + @classmethod def update_from_dict(cls, key_values, descriptor, user, save=True): """