From 7f91ce40ae582f8fafa4a79e4f1b617db1c599aa Mon Sep 17 00:00:00 2001 From: Don Mitchell Date: Thu, 21 Nov 2013 10:25:20 -0500 Subject: [PATCH] Restful api for course advanced settings STUD-948 --- .../contentstore/tests/test_contentstore.py | 10 +- .../tests/test_course_settings.py | 90 ++++++- cms/djangoapps/contentstore/views/course.py | 238 +++++++++--------- .../models/settings/course_details.py | 10 +- .../models/settings/course_grading.py | 12 +- .../models/settings/course_metadata.py | 61 ++--- cms/templates/settings.html | 5 +- cms/templates/settings_advanced.html | 10 +- cms/templates/settings_graders.html | 8 +- cms/templates/widgets/header.html | 3 +- cms/urls.py | 8 +- common/djangoapps/util/json_request.py | 1 - 12 files changed, 237 insertions(+), 219 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index aaf2cf7660..0aaf2dfb29 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/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', diff --git a/cms/djangoapps/contentstore/tests/test_course_settings.py b/cms/djangoapps/contentstore/tests/test_course_settings.py index bbd386bd58..0e24dd497a 100644 --- a/cms/djangoapps/contentstore/tests/test_course_settings.py +++ b/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): """ diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index f96c476c44..044ef79473 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/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): +# pylint: disable=invalid-name +def _config_course_advanced_components(request, course_module): """ - 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 + 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 """ - location = get_location_and_verify_access(request, org, course, name) - - course_module = modulestore().get_item(location) - - 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)), - }) - - -@require_http_methods(("GET", "POST", "PUT", "DELETE")) -@login_required -@ensure_csrf_cookie -@expect_json -def course_advanced_updates(request, org, course, name): - """ - 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 - """ - location = get_location_and_verify_access(request, org, course, name) - - 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, - } - - # 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 - ) + # 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 + # 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" - ) + 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 + + return filter_tabs + + +@login_required +@ensure_csrf_cookie +@require_http_methods(("GET", "POST", "PUT")) +@expect_json +def advanced_settings_handler(request, course_id=None, branch=None, version_guid=None, block=None, tag=None): + """ + 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 + """ + locator = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, usage_id=block) + if not has_access(request.user, locator): + raise PermissionDenied() + + course_old_location = loc_mapper().translate_locator_to_location(locator) + course_module = modulestore().get_item(course_old_location) + + 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): diff --git a/cms/djangoapps/models/settings/course_details.py b/cms/djangoapps/models/settings/course_details.py index acce1f8079..dd8582ba76 100644 --- a/cms/djangoapps/models/settings/course_details.py +++ b/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): diff --git a/cms/djangoapps/models/settings/course_grading.py b/cms/djangoapps/models/settings/course_grading.py index 8af4c44eef..fbbb37450c 100644 --- a/cms/djangoapps/models/settings/course_grading.py +++ b/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): diff --git a/cms/djangoapps/models/settings/course_metadata.py b/cms/djangoapps/models/settings/course_metadata.py index 603865b884..ddb4814511 100644 --- a/cms/djangoapps/models/settings/course_metadata.py +++ b/cms/djangoapps/models/settings/course_metadata.py @@ -1,7 +1,7 @@ -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)) + get_modulestore(descriptor.location).update_metadata(descriptor.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)) - - return cls.fetch(course_location) + return cls.fetch(descriptor) diff --git a/cms/templates/settings.html b/cms/templates/settings.html index 59e0ef0018..744f9cb112 100644 --- a/cms/templates/settings.html +++ b/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/') %>

${_("Other Course Settings")}

% endif diff --git a/cms/templates/settings_advanced.html b/cms/templates/settings_advanced.html index 73328ee327..0735992a9a 100644 --- a/cms/templates/settings_advanced.html +++ b/cms/templates/settings_advanced.html @@ -1,11 +1,9 @@ <%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 name="bodyclass">is-signedin course advanced view-settings @@ -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/', '') %>

${_("Other Course Settings")}

diff --git a/cms/templates/settings_graders.html b/cms/templates/settings_graders.html index 132125019f..a892dca473 100644 --- a/cms/templates/settings_graders.html +++ b/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
% 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/') %>

${_("Other Course Settings")}

% endif diff --git a/cms/templates/widgets/header.html b/cms/templates/widgets/header.html index 6e4581b469..b5d6c80d9c 100644 --- a/cms/templates/widgets/header.html +++ b/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') %>

@@ -80,7 +81,7 @@ ${_("Course Team")}
  • - ${_("Advanced Settings")} + ${_("Advanced Settings")}

    • diff --git a/cms/urls.py b/cms/urls.py index ac50c623b8..e17d4c729a 100644 --- a/cms/urls.py +++ b/cms/urls.py @@ -23,13 +23,6 @@ urlpatterns = patterns('', # nopep8 url(r'^preview/xblock/(?P.*?)/handler/(?P[^/]*)(?:/(?P[^/]*))?$', 'contentstore.views.preview_handler', name='preview_handler'), - # This is the URL to initially render the course advanced settings. - url(r'^(?P[^/]+)/(?P[^/]+)/settings-advanced/(?P[^/]+)$', - '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[^/]+)/(?P[^/]+)/settings-advanced/(?P[^/]+)/update.*$', - 'contentstore.views.course_advanced_updates', name='course_advanced_settings_updates'), - url(r'^(?P[^/]+)/(?P[^/]+)/textbooks/(?P[^/]+)$', 'contentstore.views.textbook_index', name='textbook_index'), url(r'^(?P[^/]+)/(?P[^/]+)/textbooks/(?P[^/]+)/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\d+)?$'.format(parsers.URL_RE_SOURCE), 'grading_handler'), + url(r'(?ix)^settings/advanced/{}$'.format(parsers.URL_RE_SOURCE), 'advanced_settings_handler'), ) js_info_dict = { diff --git a/common/djangoapps/util/json_request.py b/common/djangoapps/util/json_request.py index fea91488d4..b390c8c11c 100644 --- a/common/djangoapps/util/json_request.py +++ b/common/djangoapps/util/json_request.py @@ -1,5 +1,4 @@ from functools import wraps -import copy import json from django.core.serializers import serialize from django.core.serializers.json import DjangoJSONEncoder