From f5294358a6118a893d309bf72b8844b25783bbda Mon Sep 17 00:00:00 2001 From: Oleg Marshev Date: Wed, 15 Oct 2014 11:44:10 +0300 Subject: [PATCH] Do not deprecate giturl. --- .../tests/test_course_settings.py | 76 +++++++++++++++++++ .../models/settings/course_metadata.py | 26 +++++-- .../xmodule/modulestore/inheritance.py | 3 +- 3 files changed, 98 insertions(+), 7 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_course_settings.py b/cms/djangoapps/contentstore/tests/test_course_settings.py index 63926053e3..ed1deb3ca3 100644 --- a/cms/djangoapps/contentstore/tests/test_course_settings.py +++ b/cms/djangoapps/contentstore/tests/test_course_settings.py @@ -5,9 +5,11 @@ import datetime import json import copy import mock +from mock import patch from django.utils.timezone import UTC from django.test.utils import override_settings +from django.conf import settings from models.settings.course_details import (CourseDetails, CourseSettingsEncoder) from models.settings.course_grading import CourseGradingModel @@ -476,6 +478,80 @@ class CourseMetadataEditingTest(CourseTestCase): self.assertIn('showanswer', test_model, 'showanswer field ') self.assertIn('xqa_key', test_model, 'xqa_key field ') + @patch.dict(settings.FEATURES, {'ENABLE_EXPORT_GIT': True}) + def test_fetch_giturl_present(self): + """ + If feature flag ENABLE_EXPORT_GIT is on, show the setting as a non-deprecated Advanced Setting. + """ + test_model = CourseMetadata.fetch(self.fullcourse) + self.assertIn('giturl', test_model) + + @patch.dict(settings.FEATURES, {'ENABLE_EXPORT_GIT': False}) + def test_fetch_giturl_not_present(self): + """ + If feature flag ENABLE_EXPORT_GIT is off, don't show the setting at all on the Advanced Settings page. + """ + test_model = CourseMetadata.fetch(self.fullcourse) + self.assertNotIn('giturl', test_model) + + @patch.dict(settings.FEATURES, {'ENABLE_EXPORT_GIT': False}) + def test_validate_update_filtered_off(self): + """ + If feature flag is off, then giturl must be filtered. + """ + # pylint: disable=unused-variable + is_valid, errors, test_model = CourseMetadata.validate_and_update_from_json( + self.course, + { + "giturl": {"value": "http://example.com"}, + }, + user=self.user + ) + self.assertNotIn('giturl', test_model) + + @patch.dict(settings.FEATURES, {'ENABLE_EXPORT_GIT': True}) + def test_validate_update_filtered_on(self): + """ + If feature flag is on, then giturl must not be filtered. + """ + # pylint: disable=unused-variable + is_valid, errors, test_model = CourseMetadata.validate_and_update_from_json( + self.course, + { + "giturl": {"value": "http://example.com"}, + }, + user=self.user + ) + self.assertIn('giturl', test_model) + + @patch.dict(settings.FEATURES, {'ENABLE_EXPORT_GIT': True}) + def test_update_from_json_filtered_on(self): + """ + If feature flag is on, then giturl must be updated. + """ + test_model = CourseMetadata.update_from_json( + self.course, + { + "giturl": {"value": "http://example.com"}, + }, + user=self.user + ) + self.assertIn('giturl', test_model) + + @patch.dict(settings.FEATURES, {'ENABLE_EXPORT_GIT': False}) + def test_update_from_json_filtered_off(self): + """ + If feature flag is on, then giturl must not be updated. + """ + test_model = CourseMetadata.update_from_json( + self.course, + { + "giturl": {"value": "http://example.com"}, + }, + user=self.user + ) + self.assertNotIn('giturl', test_model) + def test_validate_and_update_from_json_correct_inputs(self): is_valid, errors, test_model = CourseMetadata.validate_and_update_from_json( self.course, diff --git a/cms/djangoapps/models/settings/course_metadata.py b/cms/djangoapps/models/settings/course_metadata.py index 27d5e9faa9..9aeb1cb1a9 100644 --- a/cms/djangoapps/models/settings/course_metadata.py +++ b/cms/djangoapps/models/settings/course_metadata.py @@ -1,6 +1,7 @@ from xblock.fields import Scope from xmodule.modulestore.django import modulestore from django.utils.translation import ugettext as _ +from django.conf import settings class CourseMetadata(object): @@ -11,6 +12,8 @@ class CourseMetadata(object): editable metadata. ''' # The list of fields that wouldn't be shown in Advanced Settings. + # Should not be used directly. Instead the filtered_list method should be used if the field needs to be filtered + # depending on the feature flag. FILTERED_LIST = ['xml_attributes', 'start', 'end', @@ -30,6 +33,20 @@ class CourseMetadata(object): 'visible_to_staff_only' ] + @classmethod + def filtered_list(cls): + """ + Filter fields based on feature flag, i.e. enabled, disabled. + """ + # Copy the filtered list to avoid permanently changing the class attribute. + filtered_list = list(cls.FILTERED_LIST) + + # Do not show giturl if feature is not enabled. + if not settings.FEATURES.get('ENABLE_EXPORT_GIT'): + filtered_list.append('giturl') + + return filtered_list + @classmethod def fetch(cls, descriptor): """ @@ -42,7 +59,7 @@ class CourseMetadata(object): if field.scope != Scope.settings: continue - if field.name in cls.FILTERED_LIST: + if field.name in cls.filtered_list(): continue result[field.name] = { @@ -61,8 +78,7 @@ class CourseMetadata(object): Ensures none of the fields are in the blacklist. """ - # Copy the filtered list to avoid permanently changing the class attribute. - filtered_list = list(cls.FILTERED_LIST) + filtered_list = cls.filtered_list() # Don't filter on the tab attribute if filter_tabs is False. if not filter_tabs: filtered_list.remove("tabs") @@ -97,10 +113,10 @@ class CourseMetadata(object): errors: list of error objects result: the updated course metadata or None if error """ - - filtered_list = list(cls.FILTERED_LIST) + filtered_list = cls.filtered_list() if not filter_tabs: filtered_list.remove("tabs") + filtered_dict = dict((k, v) for k, v in jsondict.iteritems() if k not in filtered_list) did_validate = True errors = [] diff --git a/common/lib/xmodule/xmodule/modulestore/inheritance.py b/common/lib/xmodule/xmodule/modulestore/inheritance.py index ecab07facd..68e7641e58 100644 --- a/common/lib/xmodule/xmodule/modulestore/inheritance.py +++ b/common/lib/xmodule/xmodule/modulestore/inheritance.py @@ -66,8 +66,7 @@ class InheritanceMixin(XBlockMixin): giturl = String( display_name=_("GIT URL"), help=_("Enter the URL for the course data GIT repository."), - scope=Scope.settings, - deprecated=True # Deprecated because GIT workflow users do not use Studio. + scope=Scope.settings ) xqa_key = String( display_name=_("XQA Key"),