Merge pull request #1754 from edx/dhm/restful_settings

Restful api for course advanced settings

cms/djangoapps/contentstore/tests/test_contentstore.py

@@ -1660,15 +1660,7 @@ class ContentStoreTest(ModuleStoreTestCase):
         test_get_html('tabs')
         test_get_html('settings/details')
         test_get_html('settings/grading')
-
-        # advanced settings
-        resp = self.client.get_html(reverse('course_advanced_settings',
-                                            kwargs={'org': loc.org,
-                                                    'course': loc.course,
-                                                    'name': loc.name}))
-        self.assertEqual(resp.status_code, 200)
-        # TODO: uncomment when advanced settings not using old locations.
-        # _test_no_locations(self, resp)
+        test_get_html('settings/advanced')
 
         # textbook index
         resp = self.client.get_html(reverse('textbook_index',

cms/djangoapps/contentstore/tests/test_course_settings.py

@@ -9,10 +9,9 @@ import mock
 from django.utils.timezone import UTC
 from django.test.utils import override_settings
 
-from xmodule.modulestore import Location
 from models.settings.course_details import (CourseDetails, CourseSettingsEncoder)
 from models.settings.course_grading import CourseGradingModel
-from contentstore.utils import get_modulestore
+from contentstore.utils import get_modulestore, EXTRA_TAB_PANELS
 from xmodule.modulestore.tests.factories import CourseFactory
 
 
@@ -20,7 +19,8 @@ from models.settings.course_metadata import CourseMetadata
 from xmodule.fields import Date
 
 from .utils import CourseTestCase
-from xmodule.modulestore.django import loc_mapper
+from xmodule.modulestore.django import loc_mapper, modulestore
+from contentstore.views.component import ADVANCED_COMPONENT_POLICY_KEY
 
 
 class CourseDetailsTestCase(CourseTestCase):
@@ -418,15 +418,19 @@ class CourseMetadataEditingTest(CourseTestCase):
     """
     def setUp(self):
         CourseTestCase.setUp(self)
-        CourseFactory.create(org='edX', course='999', display_name='Robot Super Course')
-        self.fullcourse_location = Location(['i4x', 'edX', '999', 'course', 'Robot_Super_Course', None])
+        self.fullcourse = CourseFactory.create(org='edX', course='999', display_name='Robot Super Course')
+        self.course_setting_url = self.course_locator.url_reverse('settings/advanced')
+        self.fullcourse_setting_url = loc_mapper().translate_location(
+            self.fullcourse.location.course_id,
+            self.fullcourse.location, False, True
+        ).url_reverse('settings/advanced')
 
     def test_fetch_initial_fields(self):
-        test_model = CourseMetadata.fetch(self.course.location)
+        test_model = CourseMetadata.fetch(self.course)
         self.assertIn('display_name', test_model, 'Missing editable metadata field')
         self.assertEqual(test_model['display_name'], 'Robot Super Course', "not expected value")
 
-        test_model = CourseMetadata.fetch(self.fullcourse_location)
+        test_model = CourseMetadata.fetch(self.fullcourse)
         self.assertNotIn('graceperiod', test_model, 'blacklisted field leaked in')
         self.assertIn('display_name', test_model, 'full missing editable metadata field')
         self.assertEqual(test_model['display_name'], 'Robot Super Course', "not expected value")
@@ -435,17 +439,18 @@ class CourseMetadataEditingTest(CourseTestCase):
         self.assertIn('xqa_key', test_model, 'xqa_key field ')
 
     def test_update_from_json(self):
-        test_model = CourseMetadata.update_from_json(self.course.location, {
+        test_model = CourseMetadata.update_from_json(self.course, {
             "advertised_start": "start A",
             "testcenter_info": {"c": "test"},
             "days_early_for_beta": 2
         })
         self.update_check(test_model)
         # try fresh fetch to ensure persistence
-        test_model = CourseMetadata.fetch(self.course.location)
+        fresh = modulestore().get_item(self.course_location)
+        test_model = CourseMetadata.fetch(fresh)
         self.update_check(test_model)
         # now change some of the existing metadata
-        test_model = CourseMetadata.update_from_json(self.course.location, {
+        test_model = CourseMetadata.update_from_json(fresh, {
             "advertised_start": "start B",
             "display_name": "jolly roger"}
         )
@@ -465,7 +470,11 @@ class CourseMetadataEditingTest(CourseTestCase):
         self.assertEqual(test_model['days_early_for_beta'], 2, "days_early_for_beta not expected value")
 
     def test_delete_key(self):
-        test_model = CourseMetadata.delete_key(self.fullcourse_location, {'deleteKeys': ['doesnt_exist', 'showanswer', 'xqa_key']})
+        test_model = CourseMetadata.update_from_json(
+            self.fullcourse, {
+                "unsetKeys": ['showanswer', 'xqa_key']
+            }
+        )
         # ensure no harm
         self.assertNotIn('graceperiod', test_model, 'blacklisted field leaked in')
         self.assertIn('display_name', test_model, 'full missing editable metadata field')
@@ -475,6 +484,65 @@ class CourseMetadataEditingTest(CourseTestCase):
         self.assertEqual('finished', test_model['showanswer'], 'showanswer field still in')
         self.assertEqual(None, test_model['xqa_key'], 'xqa_key field still in')
 
+    def test_http_fetch_initial_fields(self):
+        response = self.client.get_json(self.course_setting_url)
+        test_model = json.loads(response.content)
+        self.assertIn('display_name', test_model, 'Missing editable metadata field')
+        self.assertEqual(test_model['display_name'], 'Robot Super Course', "not expected value")
+
+        response = self.client.get_json(self.fullcourse_setting_url)
+        test_model = json.loads(response.content)
+        self.assertNotIn('graceperiod', test_model, 'blacklisted field leaked in')
+        self.assertIn('display_name', test_model, 'full missing editable metadata field')
+        self.assertEqual(test_model['display_name'], 'Robot Super Course', "not expected value")
+        self.assertIn('rerandomize', test_model, 'Missing rerandomize metadata field')
+        self.assertIn('showanswer', test_model, 'showanswer field ')
+        self.assertIn('xqa_key', test_model, 'xqa_key field ')
+
+    def test_http_update_from_json(self):
+        response = self.client.ajax_post(self.course_setting_url, {
+            "advertised_start": "start A",
+            "testcenter_info": {"c": "test"},
+            "days_early_for_beta": 2,
+            "unsetKeys": ['showanswer', 'xqa_key'],
+        })
+        test_model = json.loads(response.content)
+        self.update_check(test_model)
+        self.assertEqual('finished', test_model['showanswer'], 'showanswer field still in')
+        self.assertEqual(None, test_model['xqa_key'], 'xqa_key field still in')
+
+        response = self.client.get_json(self.course_setting_url)
+        test_model = json.loads(response.content)
+        self.update_check(test_model)
+        # now change some of the existing metadata
+        response = self.client.ajax_post(self.course_setting_url, {
+            "advertised_start": "start B",
+            "display_name": "jolly roger"
+        })
+        test_model = json.loads(response.content)
+        self.assertIn('display_name', test_model, 'Missing editable metadata field')
+        self.assertEqual(test_model['display_name'], 'jolly roger', "not expected value")
+        self.assertIn('advertised_start', test_model, 'Missing revised advertised_start metadata field')
+        self.assertEqual(test_model['advertised_start'], 'start B', "advertised_start not expected value")
+
+    def test_advanced_components_munge_tabs(self):
+        """
+        Test that adding and removing specific advanced components adds and removes tabs.
+        """
+        self.assertNotIn(EXTRA_TAB_PANELS.get("open_ended"), self.course.tabs)
+        self.assertNotIn(EXTRA_TAB_PANELS.get("notes"), self.course.tabs)
+        self.client.ajax_post(self.course_setting_url, {
+            ADVANCED_COMPONENT_POLICY_KEY: ["combinedopenended"]
+        })
+        course = modulestore().get_item(self.course_location)
+        self.assertIn(EXTRA_TAB_PANELS.get("open_ended"), course.tabs)
+        self.assertNotIn(EXTRA_TAB_PANELS.get("notes"), course.tabs)
+        self.client.ajax_post(self.course_setting_url, {
+            ADVANCED_COMPONENT_POLICY_KEY: []
+        })
+        course = modulestore().get_item(self.course_location)
+        self.assertNotIn(EXTRA_TAB_PANELS.get("open_ended"), course.tabs)
+
 
 class CourseGraderUpdatesTest(CourseTestCase):
     """

cms/djangoapps/contentstore/views/course.py

@@ -27,8 +27,7 @@ from xmodule.modulestore.exceptions import (
     ItemNotFoundError, InvalidLocationError)
 from xmodule.modulestore import Location
 
-from contentstore.course_info_model import (
-    get_course_updates, update_course_updates, delete_course_update)
+from contentstore.course_info_model import get_course_updates, update_course_updates, delete_course_update
 from contentstore.utils import (
     get_lms_link_for_item, add_extra_panel_tab, remove_extra_panel_tab,
     get_modulestore)
@@ -57,8 +56,8 @@ from contentstore import utils
 __all__ = ['course_info_handler', 'course_handler', 'course_info_update_handler',
            'settings_handler',
            'grading_handler',
-           'course_config_advanced_page',
-           'course_advanced_updates', 'textbook_index', 'textbook_by_id',
+           'advanced_settings_handler',
+           'textbook_index', 'textbook_by_id',
            'create_textbook']
 
 
@@ -175,7 +174,6 @@ def course_index(request, course_id, branch, version_guid, block):
     if not has_access(request.user, location):
         raise PermissionDenied()
 
-
     old_location = loc_mapper().translate_locator_to_location(location)
 
     lms_link = get_lms_link_for_item(old_location)
@@ -228,14 +226,20 @@ def create_new_course(request):
         pass
     if existing_course is not None:
         return JsonResponse({
-            'ErrMsg': _('There is already a course defined with the same '
+            'ErrMsg': _(
+                'There is already a course defined with the same '
                 'organization, course number, and course run. Please '
                 'change either organization or course number to be '
-                'unique.'),
-            'OrgErrMsg': _('Please change either the organization or '
-                'course number so that it is unique.'),
-            'CourseErrMsg': _('Please change either the organization or '
-                'course number so that it is unique.'),
+                'unique.'
+            ),
+            'OrgErrMsg': _(
+                'Please change either the organization or '
+                'course number so that it is unique.'
+            ),
+            'CourseErrMsg': _(
+                'Please change either the organization or '
+                'course number so that it is unique.'
+            ),
         })
 
     # dhm: this query breaks the abstraction, but I'll fix it when I do my suspended refactoring of this
@@ -250,12 +254,15 @@ def create_new_course(request):
     courses = modulestore().collection.find(course_search_location, fields=('_id'))
     if courses.count() > 0:
         return JsonResponse({
-            'ErrMsg': _('There is already a course defined with the same '
+            'ErrMsg': _(
+                'There is already a course defined with the same '
                 'organization and course number. Please '
                 'change at least one field to be unique.'),
-            'OrgErrMsg': _('Please change either the organization or '
+            'OrgErrMsg': _(
+                'Please change either the organization or '
                 'course number so that it is unique.'),
-            'CourseErrMsg': _('Please change either the organization or '
+            'CourseErrMsg': _(
+                'Please change either the organization or '
                 'course number so that it is unique.'),
         })
 
@@ -343,9 +350,8 @@ def course_info_handler(request, tag=None, course_id=None, branch=None, version_
 @ensure_csrf_cookie
 @require_http_methods(("GET", "POST", "PUT", "DELETE"))
 @expect_json
-def course_info_update_handler(
-    request, tag=None, course_id=None, branch=None, version_guid=None, block=None, provided_id=None
-    ):
+def course_info_update_handler(request, tag=None, course_id=None, branch=None, version_guid=None, block=None,
+                               provided_id=None):
     """
     restful CRUD operations on course_info updates.
     provided_id should be none if it's new (create) and index otherwise.
@@ -492,118 +498,104 @@ def grading_handler(request, tag=None, course_id=None, branch=None, version_guid
             return JsonResponse()
 
 
-@login_required
-@ensure_csrf_cookie
-def course_config_advanced_page(request, org, course, name):
-    """
-    Send models and views as well as html for editing the advanced course
-    settings to the client.
-
-    org, course, name: Attributes of the Location for the item to edit
-    """
-    location = get_location_and_verify_access(request, org, course, name)
+# pylint: disable=invalid-name
+def _config_course_advanced_components(request, course_module):
+    """
+    Check to see if the user instantiated any advanced components. This
+    is a hack that does the following :
+    1) adds/removes the open ended panel tab to a course automatically
+    if the user has indicated that they want to edit the
+    combinedopendended or peergrading module
+    2) adds/removes the notes panel tab to a course automatically if
+    the user has indicated that they want the notes module enabled in
+    their course
+    """
+    # TODO refactor the above into distinct advanced policy settings
+    filter_tabs = True  # Exceptional conditions will pull this to False
+    if ADVANCED_COMPONENT_POLICY_KEY in request.json:  # Maps tab types to components
+        tab_component_map = {
+            'open_ended':OPEN_ENDED_COMPONENT_TYPES,
+            'notes':NOTE_COMPONENT_TYPES,
+        }
+        # Check to see if the user instantiated any notes or open ended
+        # components
+        for tab_type in tab_component_map.keys():
+            component_types = tab_component_map.get(tab_type)
+            found_ac_type = False
+            for ac_type in component_types:
+                if ac_type in request.json[ADVANCED_COMPONENT_POLICY_KEY]:
+                    # Add tab to the course if needed
+                    changed, new_tabs = add_extra_panel_tab(tab_type, course_module)
+                    # If a tab has been added to the course, then send the
+                    # metadata along to CourseMetadata.update_from_json
+                    if changed:
+                        course_module.tabs = new_tabs
+                        request.json.update({'tabs': new_tabs})
+                        # Indicate that tabs should not be filtered out of
+                        # the metadata
+                        filter_tabs = False  # Set this flag to avoid the tab removal code below.
+                    found_ac_type = True  #break
 
-    course_module = modulestore().get_item(location)
+            # If we did not find a module type in the advanced settings,
+            # we may need to remove the tab from the course.
+            if not found_ac_type:  # Remove tab from the course if needed
+                changed, new_tabs = remove_extra_panel_tab(tab_type, course_module)
+                if changed:
+                    course_module.tabs = new_tabs
+                    request.json.update({'tabs':new_tabs})
+                    # Indicate that tabs should *not* be filtered out of
+                    # the metadata
+                    filter_tabs = False
 
-    return render_to_response('settings_advanced.html', {
-        'context_course': course_module,
-        'course_location': location,
-        'course_locator': loc_mapper().translate_location(location.course_id, location, False, True),
-        'advanced_dict': json.dumps(CourseMetadata.fetch(location)),
-    })
+    return filter_tabs
 
 
-@require_http_methods(("GET", "POST", "PUT", "DELETE"))
 @login_required
 @ensure_csrf_cookie
+@require_http_methods(("GET", "POST", "PUT"))
 @expect_json
-def course_advanced_updates(request, org, course, name):
+def advanced_settings_handler(request, course_id=None, branch=None, version_guid=None, block=None, tag=None):
     """
-    Restful CRUD operations on metadata. The payload is a json rep of the
-    metadata dicts. For delete, otoh, the payload is either a key or a list of
-    keys to delete.
-
-    org, course: Attributes of the Location for the item to edit
+    Course settings configuration
+    GET
+        html: get the page
+        json: get the model
+    PUT, POST
+        json: update the Course's settings. The payload is a json rep of the
+            metadata dicts. The dict can include a "unsetKeys" entry which is a list
+            of keys whose values to unset: i.e., revert to default
     """
-    location = get_location_and_verify_access(request, org, course, name)
+    locator = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, usage_id=block)
+    if not has_access(request.user, locator):
+        raise PermissionDenied()
 
-    if request.method == 'GET':
-        return JsonResponse(CourseMetadata.fetch(location))
-    elif request.method == 'DELETE':
-        return JsonResponse(CourseMetadata.delete_key(
-            location,
-            json.loads(request.body)
-        ))
-    else:
-        # Whether or not to filter the tabs key out of the settings metadata
-        filter_tabs = True
-
-        # Check to see if the user instantiated any advanced components. This
-        # is a hack that does the following :
-        #   1) adds/removes the open ended panel tab to a course automatically
-        #   if the user has indicated that they want to edit the
-        #   combinedopendended or peergrading module
-        #   2) adds/removes the notes panel tab to a course automatically if
-        #   the user has indicated that they want the notes module enabled in
-        #   their course
-        # TODO refactor the above into distinct advanced policy settings
-        if ADVANCED_COMPONENT_POLICY_KEY in request.json:
-            # Get the course so that we can scrape current tabs
-            course_module = modulestore().get_item(location)
-
-            # Maps tab types to components
-            tab_component_map = {
-                'open_ended': OPEN_ENDED_COMPONENT_TYPES,
-                'notes': NOTE_COMPONENT_TYPES,
-            }
+    course_old_location = loc_mapper().translate_locator_to_location(locator)
+    course_module = modulestore().get_item(course_old_location)
 
-            # Check to see if the user instantiated any notes or open ended
-            # components
-            for tab_type in tab_component_map.keys():
-                component_types = tab_component_map.get(tab_type)
-                found_ac_type = False
-                for ac_type in component_types:
-                    if ac_type in request.json[ADVANCED_COMPONENT_POLICY_KEY]:
-                        # Add tab to the course if needed
-                        changed, new_tabs = add_extra_panel_tab(
-                            tab_type,
-                            course_module
-                        )
-                        # If a tab has been added to the course, then send the
-                        # metadata along to CourseMetadata.update_from_json
-                        if changed:
-                            course_module.tabs = new_tabs
-                            request.json.update({'tabs': new_tabs})
-                            # Indicate that tabs should not be filtered out of
-                            # the metadata
-                            filter_tabs = False
-                        # Set this flag to avoid the tab removal code below.
-                        found_ac_type = True
-                        break
-                # If we did not find a module type in the advanced settings,
-                # we may need to remove the tab from the course.
-                if not found_ac_type:
-                    # Remove tab from the course if needed
-                    changed, new_tabs = remove_extra_panel_tab(
-                        tab_type, course_module
-                    )
-                    if changed:
-                        course_module.tabs = new_tabs
-                        request.json.update({'tabs': new_tabs})
-                        # Indicate that tabs should *not* be filtered out of
-                        # the metadata
-                        filter_tabs = False
-        try:
-            return JsonResponse(CourseMetadata.update_from_json(
-                location,
-                request.json,
-                filter_tabs=filter_tabs
-            ))
-        except (TypeError, ValueError) as err:
-            return HttpResponseBadRequest(
-                "Incorrect setting format. " + str(err),
-                content_type="text/plain"
-            )
+    if 'text/html' in request.META.get('HTTP_ACCEPT', '') and request.method == 'GET':
+
+        return render_to_response('settings_advanced.html', {
+            'context_course': course_module,
+            'advanced_dict': json.dumps(CourseMetadata.fetch(course_module)),
+            'advanced_settings_url': locator.url_reverse('settings/advanced')
+        })
+    elif 'application/json' in request.META.get('HTTP_ACCEPT', ''):
+        if request.method == 'GET':
+            return JsonResponse(CourseMetadata.fetch(course_module))
+        else:
+            # Whether or not to filter the tabs key out of the settings metadata
+            filter_tabs = _config_course_advanced_components(request, course_module)
+            try:
+                return JsonResponse(CourseMetadata.update_from_json(
+                    course_module,
+                    request.json,
+                    filter_tabs=filter_tabs
+                ))
+            except (TypeError, ValueError) as err:
+                return HttpResponseBadRequest(
+                    "Incorrect setting format. {}".format(err),
+                    content_type="text/plain"
+                )
 
 
 class TextbookValidationError(Exception):

cms/djangoapps/models/settings/course_details.py

@@ -32,11 +32,11 @@ class CourseDetails(object):
         self.course_image_asset_path = ""  # URL of the course image
 
     @classmethod
-    def fetch(cls, course_location):
+    def fetch(cls, course_locator):
         """
         Fetch the course details for the given course from persistence and return a CourseDetails model.
         """
-        course_old_location = loc_mapper().translate_locator_to_location(course_location)
+        course_old_location = loc_mapper().translate_locator_to_location(course_locator)
         descriptor = get_modulestore(course_old_location).get_item(course_old_location)
         course = cls(course_old_location.org, course_old_location.course, course_old_location.name)
 
@@ -75,11 +75,11 @@ class CourseDetails(object):
         return course
 
     @classmethod
-    def update_from_json(cls, course_location, jsondict):
+    def update_from_json(cls, course_locator, jsondict):
         """
         Decode the json into CourseDetails and save any changed attrs to the db
         """
-        course_old_location = loc_mapper().translate_locator_to_location(course_location)
+        course_old_location = loc_mapper().translate_locator_to_location(course_locator)
         descriptor = get_modulestore(course_old_location).get_item(course_old_location)
 
         dirty = False
@@ -153,7 +153,7 @@ class CourseDetails(object):
 
         # Could just return jsondict w/o doing any db reads, but I put the reads in as a means to confirm
         # it persisted correctly
-        return CourseDetails.fetch(course_location)
+        return CourseDetails.fetch(course_locator)
 
     @staticmethod
     def parse_video_tag(raw_video):

cms/djangoapps/models/settings/course_grading.py

@@ -18,11 +18,11 @@ class CourseGradingModel(object):
         self.grace_period = CourseGradingModel.convert_set_grace_period(course_descriptor)
 
     @classmethod
-    def fetch(cls, course_location):
+    def fetch(cls, course_locator):
         """
         Fetch the course grading policy for the given course from persistence and return a CourseGradingModel.
         """
-        course_old_location = loc_mapper().translate_locator_to_location(course_location)
+        course_old_location = loc_mapper().translate_locator_to_location(course_locator)
         descriptor = get_modulestore(course_old_location).get_item(course_old_location)
 
         model = cls(descriptor)
@@ -52,12 +52,12 @@ class CourseGradingModel(object):
                     }
 
     @staticmethod
-    def update_from_json(course_location, jsondict):
+    def update_from_json(course_locator, jsondict):
         """
         Decode the json into CourseGradingModel and save any changes. Returns the modified model.
         Probably not the usual path for updates as it's too coarse grained.
         """
-        course_old_location = loc_mapper().translate_locator_to_location(course_location)
+        course_old_location = loc_mapper().translate_locator_to_location(course_locator)
         descriptor = get_modulestore(course_old_location).get_item(course_old_location)
 
         graders_parsed = [CourseGradingModel.parse_grader(jsonele) for jsonele in jsondict['graders']]
@@ -69,9 +69,9 @@ class CourseGradingModel(object):
             course_old_location, descriptor.get_explicitly_set_fields_by_scope(Scope.content)
         )
 
-        CourseGradingModel.update_grace_period_from_json(course_location, jsondict['grace_period'])
+        CourseGradingModel.update_grace_period_from_json(course_locator, jsondict['grace_period'])
 
-        return CourseGradingModel.fetch(course_location)
+        return CourseGradingModel.fetch(course_locator)
 
     @staticmethod
     def update_grader_from_json(course_location, grader):

cms/djangoapps/models/settings/course_metadata.py

-from xmodule.modulestore import Location
+from xblock.fields import Scope
+
 from contentstore.utils import get_modulestore
 from xmodule.modulestore.inheritance import own_metadata
-from xblock.fields import Scope
 from cms.xmodule_namespace import CmsBlockMixin
 
 
@@ -20,21 +20,18 @@ class CourseMetadata(object):
                      'tabs',
                      'graceperiod',
                      'checklists',
-                     'show_timezone'
+                     'show_timezone',
+                     'format',
+                     'graded',
     ]
 
     @classmethod
-    def fetch(cls, course_location):
+    def fetch(cls, descriptor):
         """
         Fetch the key:value editable course details for the given course from
         persistence and return a CourseMetadata model.
         """
-        if not isinstance(course_location, Location):
-            course_location = Location(course_location)
-
-        course = {}
-
-        descriptor = get_modulestore(course_location).get_item(course_location)
+        result = {}
 
         for field in descriptor.fields.values():
             if field.name in CmsBlockMixin.fields:
@@ -46,19 +43,17 @@ class CourseMetadata(object):
             if field.name in cls.FILTERED_LIST:
                 continue
 
-            course[field.name] = field.read_json(descriptor)
+            result[field.name] = field.read_json(descriptor)
 
-        return course
+        return result
 
     @classmethod
-    def update_from_json(cls, course_location, jsondict, filter_tabs=True):
+    def update_from_json(cls, descriptor, jsondict, filter_tabs=True):
         """
         Decode the json into CourseMetadata and save any changed attrs to the db.
 
         Ensures none of the fields are in the blacklist.
         """
-        descriptor = get_modulestore(course_location).get_item(course_location)
-
         dirty = False
 
         # Copy the filtered list to avoid permanently changing the class attribute.
@@ -72,39 +67,17 @@ class CourseMetadata(object):
             if key in filtered_list:
                 continue
 
+            if key == "unsetKeys":
+                dirty = True
+                for unset in val:
+                    descriptor.fields[unset].delete_from(descriptor)
+
             if hasattr(descriptor, key) and getattr(descriptor, key) != val:
                 dirty = True
                 value = descriptor.fields[key].from_json(val)
                 setattr(descriptor, key, value)
 
         if dirty:
-            # Save the data that we've just changed to the underlying
-            # MongoKeyValueStore before we update the mongo datastore.
-            descriptor.save()
-            get_modulestore(course_location).update_metadata(course_location,
-                                                             own_metadata(descriptor))
-
-        # Could just generate and return a course obj w/o doing any db reads,
-        # but I put the reads in as a means to confirm it persisted correctly
-        return cls.fetch(course_location)
-
-    @classmethod
-    def delete_key(cls, course_location, payload):
-        '''
-        Remove the given metadata key(s) from the course. payload can be a
-        single key or [key..]
-        '''
-        descriptor = get_modulestore(course_location).get_item(course_location)
-
-        for key in payload['deleteKeys']:
-            if hasattr(descriptor, key):
-                delattr(descriptor, key)
-
-        # Save the data that we've just changed to the underlying
-        # MongoKeyValueStore before we update the mongo datastore.
-        descriptor.save()
-
-        get_modulestore(course_location).update_metadata(course_location,
-                                                         own_metadata(descriptor))
+            get_modulestore(descriptor.location).update_metadata(descriptor.location, own_metadata(descriptor))
 
-        return cls.fetch(course_location)
+        return cls.fetch(descriptor)

cms/templates/settings.html

@@ -6,7 +6,6 @@
 <%!
   from django.utils.translation import ugettext as _
   from xmodule.modulestore.django import loc_mapper
-  from django.core.urlresolvers import reverse
 %>
 
 <%block name="jsextra">
@@ -293,14 +292,14 @@ require(["domReady!", "jquery", "js/models/settings/course_details", "js/views/s
           <%
             course_team_url = course_locator.url_reverse('course_team/', '')
             grading_config_url = course_locator.url_reverse('settings/grading/')
-            ctx_loc = context_course.location
+            advanced_config_url = course_locator.url_reverse('settings/advanced/')
           %>
         <h3 class="title-3">${_("Other Course Settings")}</h3>
         <nav class="nav-related">
           <ul>
             <li class="nav-item"><a href="${grading_config_url}">${_("Grading")}</a></li>
             <li class="nav-item"><a href="${course_team_url}">${_("Course Team")}</a></li>
-            <li class="nav-item"><a href="${reverse('course_advanced_settings', kwargs={'org' : ctx_loc.org, 'course' : ctx_loc.course, 'name': ctx_loc.name})}">${_("Advanced Settings")}</a></li>
+            <li class="nav-item"><a href="${advanced_config_url}">${_("Advanced Settings")}</a></li>
           </ul>
         </nav>
      % endif

cms/templates/settings_advanced.html

 <%inherit file="base.html" />
 <%namespace name='static' file='static_content.html'/>
 <%!
-  from django.core.urlresolvers import reverse
   from django.utils.translation import ugettext as _
   from contentstore import utils
   from xmodule.modulestore.django import loc_mapper
-  from django.core.urlresolvers import reverse
 %>
 <%block name="title">${_("Advanced Settings")}</%block>
 <%block name="bodyclass">is-signedin course advanced view-settings</%block>
@@ -28,7 +26,7 @@ require(["domReady!", "jquery", "js/models/settings/advanced", "js/views/setting
 
   // proactively populate advanced b/c it has the filtered list and doesn't really follow the model pattern
   var advancedModel = new AdvancedSettingsModel(${advanced_dict | n}, {parse: true});
-  advancedModel.url = "${reverse('course_advanced_settings_updates', kwargs=dict(org=context_course.location.org, course=context_course.location.course, name=context_course.location.name))}";
+  advancedModel.url = "${advanced_settings_url}";
 
   var editor = new AdvancedSettingsView({
     el: $('.settings-advanced'),
@@ -91,13 +89,15 @@ require(["domReady!", "jquery", "js/models/settings/advanced", "js/views/setting
         <%
           ctx_loc = context_course.location
           location = loc_mapper().translate_location(ctx_loc.course_id, ctx_loc, False, True)
+          details_url = location.url_reverse('settings/details/')
+          grading_url = location.url_reverse('settings/grading/')
           course_team_url = location.url_reverse('course_team/', '')
         %>
       <h3 class="title-3">${_("Other Course Settings")}</h3>
         <nav class="nav-related">
           <ul>
-            <li class="nav-item"><a href="${course_locator.url_reverse('settings/details/')}">${_("Details &amp; Schedule")}</a></li>
-            <li class="nav-item"><a href="${course_locator.url_reverse('settings/grading/')}">${_("Grading")}</a></li>
+            <li class="nav-item"><a href="${details_url}">${_("Details &amp; Schedule")}</a></li>
+            <li class="nav-item"><a href="${grading_url}">${_("Grading")}</a></li>
             <li class="nav-item"><a href="${course_team_url}">${_("Course Team")}</a></li>
           </ul>
         </nav>

cms/templates/settings_graders.html

@@ -7,7 +7,6 @@
   from contentstore import utils
   from django.utils.translation import ugettext as _
   from xmodule.modulestore.django import loc_mapper
-  from django.core.urlresolvers import reverse
 %>
 
 <%block name="header_extras">
@@ -139,15 +138,16 @@ require(["domReady!", "jquery", "js/views/settings/grading", "js/models/settings
       <div class="bit">
       % if context_course:
         <%
-          ctx_loc = context_course.location
           course_team_url = course_locator.url_reverse('course_team/')
+          advanced_settings_url = course_locator.url_reverse('settings/advanced/')
+          detailed_settings_url = course_locator.url_reverse('settings/details/')
         %>
         <h3 class="title-3">${_("Other Course Settings")}</h3>
         <nav class="nav-related">
           <ul>
-            <li class="nav-item"><a href="${course_locator.url_reverse('settings/details/')}">${_("Details &amp; Schedule")}</a></li>
+            <li class="nav-item"><a href="${detailed_settings_url}">${_("Details &amp; Schedule")}</a></li>
             <li class="nav-item"><a href="${course_team_url}">${_("Course Team")}</a></li>
-            <li class="nav-item"><a href="${reverse('course_advanced_settings', kwargs={'org' : ctx_loc.org, 'course' : ctx_loc.course, 'name': ctx_loc.name})}">${_("Advanced Settings")}</a></li>
+            <li class="nav-item"><a href="${advanced_settings_url}">${_("Advanced Settings")}</a></li>
           </ul>
         </nav>
       % endif

cms/templates/widgets/header.html

@@ -25,6 +25,7 @@
             export_url = location.url_reverse('export')
             settings_url = location.url_reverse('settings/details/')
             grading_url = location.url_reverse('settings/grading/')
+            advanced_settings_url = location.url_reverse('settings/advanced/')
             tabs_url = location.url_reverse('tabs')
       %>
       <h2 class="info-course">
@@ -80,7 +81,7 @@
                     <a href="${course_team_url}">${_("Course Team")}</a>
                   </li>
                   <li class="nav-item nav-course-settings-advanced">
-                    <a href="${reverse('course_advanced_settings', kwargs={'org' : ctx_loc.org, 'course' : ctx_loc.course, 'name': ctx_loc.name})}">${_("Advanced Settings")}</a>
+                    <a href="${advanced_settings_url}">${_("Advanced Settings")}</a>
                   </li>
                 </ul>
               </div>

cms/urls.py

@@ -23,13 +23,6 @@ urlpatterns = patterns('',  # nopep8
     url(r'^preview/xblock/(?P<usage_id>.*?)/handler/(?P<handler>[^/]*)(?:/(?P<suffix>[^/]*))?$',
         'contentstore.views.preview_handler', name='preview_handler'),
 
-    # This is the URL to initially render the course advanced settings.
-    url(r'^(?P<org>[^/]+)/(?P<course>[^/]+)/settings-advanced/(?P<name>[^/]+)$',
-        'contentstore.views.course_config_advanced_page', name='course_advanced_settings'),
-    # This is the URL used by BackBone for updating and re-fetching the model.
-    url(r'^(?P<org>[^/]+)/(?P<course>[^/]+)/settings-advanced/(?P<name>[^/]+)/update.*$',
-        'contentstore.views.course_advanced_updates', name='course_advanced_settings_updates'),
-
     url(r'^(?P<org>[^/]+)/(?P<course>[^/]+)/textbooks/(?P<name>[^/]+)$',
         'contentstore.views.textbook_index', name='textbook_index'),
     url(r'^(?P<org>[^/]+)/(?P<course>[^/]+)/textbooks/(?P<name>[^/]+)/new$',
@@ -95,6 +88,7 @@ urlpatterns += patterns(
     url(r'(?ix)^tabs/{}$'.format(parsers.URL_RE_SOURCE), 'tabs_handler'),
     url(r'(?ix)^settings/details/{}$'.format(parsers.URL_RE_SOURCE), 'settings_handler'),
     url(r'(?ix)^settings/grading/{}(/)?(?P<grader_index>\d+)?$'.format(parsers.URL_RE_SOURCE), 'grading_handler'),
+    url(r'(?ix)^settings/advanced/{}$'.format(parsers.URL_RE_SOURCE), 'advanced_settings_handler'),
 )
 
 js_info_dict = {

common/djangoapps/util/json_request.py

 from functools import wraps
-import copy
 import json
 from django.core.serializers import serialize
 from django.core.serializers.json import DjangoJSONEncoder