Merge pull request #5585 from edx/oleg/giturl-not-deprecate

GIT URL should not be a deprecated setting (TNL-396).

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,

cms/djangoapps/models/settings/course_metadata.py

 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',
@@ -31,6 +34,20 @@ class CourseMetadata(object):
     ]
 
     @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):
         """
         Fetch the key:value editable course details for the given course from
@@ -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 = []

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"),