diff --git a/cms/djangoapps/contentstore/course_info_model.py b/cms/djangoapps/contentstore/course_info_model.py index cee37c4cf5..8c8aed549d 100644 --- a/cms/djangoapps/contentstore/course_info_model.py +++ b/cms/djangoapps/contentstore/course_info_model.py @@ -1,7 +1,7 @@ from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.modulestore import Location from xmodule.modulestore.django import modulestore -from lxml import html +from lxml import html, etree import re from django.http import HttpResponseBadRequest import logging diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index dcd1f408cd..c20e2e2050 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -109,11 +109,11 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): import_from_xml(modulestore(), 'common/test/data/', ['full']) ms = modulestore('direct') effort = ms.get_item(Location(['i4x', 'edX', 'full', 'about', 'effort', None])) - self.assertEqual(effort.definition['data'], '6 hours') + self.assertEqual(effort.data, '6 hours') # this one should be in a non-override folder effort = ms.get_item(Location(['i4x', 'edX', 'full', 'about', 'end_date', None])) - self.assertEqual(effort.definition['data'], 'TBD') + self.assertEqual(effort.data, 'TBD') def test_remove_hide_progress_tab(self): import_from_xml(modulestore(), 'common/test/data/', ['full']) @@ -123,7 +123,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): source_location = CourseDescriptor.id_to_location('edX/full/6.002_Spring_2012') course = ms.get_item(source_location) - self.assertNotIn('hide_progress_tab', course.metadata) + self.assertFalse(course.hide_progress_tab) def test_clone_course(self): @@ -216,7 +216,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): with fs.open('grading_policy.json','r') as grading_policy: on_disk = loads(grading_policy.read()) course = ms.get_item(location) - self.assertEqual(on_disk, course.definition['data']['grading_policy']) + self.assertEqual(on_disk, course.grading_policy) # remove old course delete_course(ms, cs, location) @@ -407,5 +407,4 @@ class ContentStoreTest(ModuleStoreTestCase): self.assertIsInstance(problem, CapaDescriptor, "New problem is not a CapaDescriptor") context = problem.get_context() self.assertIn('markdown', context, "markdown is missing from context") - self.assertIn('markdown', problem.metadata, "markdown is missing from metadata") self.assertNotIn('markdown', problem.editable_metadata_fields, "Markdown slipped into the editable metadata fields") diff --git a/cms/djangoapps/contentstore/tests/test_course_settings.py b/cms/djangoapps/contentstore/tests/test_course_settings.py index 6a73248b81..efb619571d 100644 --- a/cms/djangoapps/contentstore/tests/test_course_settings.py +++ b/cms/djangoapps/contentstore/tests/test_course_settings.py @@ -251,8 +251,7 @@ class CourseGradingTest(CourseTestCase): test_grader.grace_period = {'hours': 4, 'minutes': 5, 'seconds': 0} altered_grader = CourseGradingModel.update_from_json(test_grader.__dict__) - print test_grader.__dict__ - print altered_grader.__dict__ + print test_grader.grace_period, altered_grader.grace_period self.assertDictEqual(test_grader.__dict__, altered_grader.__dict__, "4 hour grace period") def test_update_grader_from_json(self): diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py index 0d905ffa32..ffb6f857d3 100644 --- a/cms/djangoapps/contentstore/views.py +++ b/cms/djangoapps/contentstore/views.py @@ -281,7 +281,7 @@ def edit_unit(request, location): component_templates[template.location.category].append(( template.lms.display_name, template.location.url(), - 'markdown' in template.metadata, + hasattr(template, 'markdown') and template.markdown != '', template.location.name == 'Empty' )) @@ -944,7 +944,7 @@ def reorder_static_tabs(request): for tab in course.tabs: if tab['type'] == 'static_tab': reordered_tabs.append({'type': 'static_tab', - 'name': tab_items[static_tab_idx].metadata.get('display_name'), + 'name': tab_items[static_tab_idx].lms.display_name, 'url_slug': tab_items[static_tab_idx].location.name}) static_tab_idx += 1 else: @@ -953,7 +953,7 @@ def reorder_static_tabs(request): # OK, re-assemble the static tabs in the new order course.tabs = reordered_tabs - modulestore('direct').update_metadata(course.location, course.metadata) + modulestore('direct').update_metadata(course.location, own_metadata(course)) return HttpResponse() diff --git a/cms/djangoapps/models/settings/course_grading.py b/cms/djangoapps/models/settings/course_grading.py index bf01fb4549..aeb808ec38 100644 --- a/cms/djangoapps/models/settings/course_grading.py +++ b/cms/djangoapps/models/settings/course_grading.py @@ -157,10 +157,10 @@ class CourseGradingModel(object): graceperiodjson = graceperiodjson['grace_period'] # lms requires these to be in a fixed order - grace_rep = "{0[hours]:d} hours {0[minutes]:d} minutes {0[seconds]:d} seconds".format(graceperiodjson) + grace_timedelta = timedelta(**graceperiodjson) descriptor = get_modulestore(course_location).get_item(course_location) - descriptor.lms.graceperiod = grace_rep + descriptor.lms.graceperiod = grace_timedelta get_modulestore(course_location).update_metadata(course_location, descriptor._model_data._kvs._metadata) @staticmethod @@ -236,7 +236,6 @@ class CourseGradingModel(object): @staticmethod def convert_set_grace_period(descriptor): -<<<<<<< HEAD # 5 hours 59 minutes 59 seconds => converted to iso format rawgrace = descriptor.lms.graceperiod if rawgrace: @@ -248,27 +247,19 @@ class CourseGradingModel(object): minutes = int(seconds / 60) seconds -= minutes * 60 - graceperiod = {} + graceperiod = {'hours': 0, 'minutes': 0, 'seconds': 0} if hours > 0: - graceperiod['hours'] = str(hours) + graceperiod['hours'] = hours if minutes > 0: - graceperiod['minutes'] = str(minutes) + graceperiod['minutes'] = minutes if seconds > 0: - graceperiod['seconds'] = str(seconds) + graceperiod['seconds'] = seconds return graceperiod else: return None -======= - # 5 hours 59 minutes 59 seconds => { hours: 5, minutes : 59, seconds : 59} - rawgrace = descriptor.metadata.get('graceperiod', None) - if rawgrace: - parsedgrace = {str(key): int(val) for (val, key) in re.findall('\s*(\d+)\s*(\w+)', rawgrace)} - return parsedgrace - else: return None ->>>>>>> origin/master @staticmethod def parse_grader(json_grader): diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index 92742203c4..f508e2f850 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -116,7 +116,7 @@ class CapaModule(XModule): #log.debug("Then parsed " + grace_period_string + # " to closing date" + str(self.close_date)) else: - self.close_date = self.due + self.close_date = due_date if self.seed is None: if self.rerandomize == 'never': @@ -373,8 +373,11 @@ class CapaModule(XModule): datetime.datetime.utcnow() > self.close_date) def closed(self): - ''' Is the student still allowed to submit answers? ''' - if self.attempts == self.max_attempts: + ''' Is the student prevented from submitting answers? ''' + if self.max_attempts is None: + return False + + if self.attempts >= self.max_attempts: return True if self.is_past_due(): return True @@ -669,6 +672,7 @@ class CapaDescriptor(RawDescriptor): module_class = CapaModule weight = Float(help="How much to weight this problem by", scope=Scope.settings) + markdown = String(help="Markdown source of this module", scope=Scope.settings, default='') stores_state = True has_score = True @@ -690,7 +694,7 @@ class CapaDescriptor(RawDescriptor): def get_context(self): _context = RawDescriptor.get_context(self) - _context.update({'markdown': self.metadata.get('markdown', '')}) + _context.update({'markdown': self.markdown}) return _context @property diff --git a/common/lib/xmodule/xmodule/conditional_module.py b/common/lib/xmodule/xmodule/conditional_module.py index 11d2a58231..7f01a6a030 100644 --- a/common/lib/xmodule/xmodule/conditional_module.py +++ b/common/lib/xmodule/xmodule/conditional_module.py @@ -37,14 +37,15 @@ class ConditionalModule(XModule): condition = String(help="Condition for this module", default='', scope=Scope.settings) - def __init__(self, system, location, definition, descriptor, instance_state=None, shared_state=None, **kwargs): + def __init__(self, *args, **kwargs): """ In addition to the normal XModule init, provide: self.condition = string describing condition required """ - XModule.__init__(self, system, location, definition, descriptor, instance_state, shared_state, **kwargs) + super(ConditionalModule, self).__init__(*args, **kwargs) + self.contents = None self._get_required_modules() children = self.get_display_items() diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index ad8dd06ab5..28110070db 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -409,6 +409,11 @@ class CourseDescriptor(SequenceDescriptor): def grade_cutoffs(self, value): self._grading_policy['GRADE_CUTOFFS'] = value + # XBlock fields don't update after mutation + policy = self.grading_policy + policy['GRADE_CUTOFFS'] = value + self.grading_policy = policy + @property def lowest_passing_grade(self): diff --git a/common/lib/xmodule/xmodule/gst_module.py b/common/lib/xmodule/xmodule/gst_module.py index 91a9828c3e..8ebe77cb7a 100644 --- a/common/lib/xmodule/xmodule/gst_module.py +++ b/common/lib/xmodule/xmodule/gst_module.py @@ -14,6 +14,7 @@ from xmodule.xml_module import XmlDescriptor from xmodule.x_module import XModule from xmodule.stringify import stringify_children from pkg_resources import resource_string +from xblock.core import String, Scope log = logging.getLogger(__name__) @@ -43,6 +44,8 @@ class GraphicalSliderToolModule(XModule): } js_module_name = "GraphicalSliderTool" + render = String(scope=Scope.content) + def __init__(self, system, location, definition, descriptor, instance_state=None, shared_state=None, **kwargs): """ @@ -60,7 +63,7 @@ class GraphicalSliderToolModule(XModule): self.html_class = self.location.category self.configuration_json = self.build_configuration_json() params = { - 'gst_html': self.substitute_controls(self.definition['render']), + 'gst_html': self.substitute_controls(self.render), 'element_id': self.html_id, 'element_class': self.html_class, 'configuration_json': self.configuration_json @@ -146,6 +149,9 @@ class GraphicalSliderToolDescriptor(MakoModuleDescriptor, XmlDescriptor): module_class = GraphicalSliderToolModule template_dir_name = 'graphical_slider_tool' + render = String(scope=Scope.content) + configuration = String(scope=Scope.content) + @classmethod def definition_from_xml(cls, xml_object, system): """ @@ -184,7 +190,7 @@ class GraphicalSliderToolDescriptor(MakoModuleDescriptor, XmlDescriptor): xml_object = etree.Element('graphical_slider_tool') def add_child(k): - child_str = '<{tag}>{body}'.format(tag=k, body=self.definition[k]) + child_str = '<{tag}>{body}'.format(tag=k, body=getattr(self, k)) child_node = etree.fromstring(child_str) xml_object.append(child_node) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/factories.py b/common/lib/xmodule/xmodule/modulestore/tests/factories.py index 5375f3262f..3dd05a59dc 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/factories.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/factories.py @@ -4,6 +4,7 @@ from uuid import uuid4 from xmodule.modulestore import Location from xmodule.modulestore.django import modulestore from xmodule.timeparse import stringify_time +from xmodule.modulestore.inheritance import own_metadata def XMODULE_COURSE_CREATION(class_to_create, **kwargs): @@ -51,7 +52,7 @@ class XModuleCourseFactory(Factory): {"type": "progress", "name": "Progress"}] # Update the data in the mongo datastore - store.update_metadata(new_course.location.url(), new_course.own_metadata) + store.update_metadata(new_course.location.url(), own_metadata(new_course)) return new_course @@ -98,14 +99,11 @@ class XModuleItemFactory(Factory): new_item = store.clone_item(template, dest_location) - # TODO: This needs to be deleted when we have proper storage for static content - new_item.data_dir = parent.data_dir - # replace the display name with an optional parameter passed in from the caller if display_name is not None: new_item.lms.display_name = display_name - store.update_metadata(new_item.location.url(), new_item.own_metadata) + store.update_metadata(new_item.location.url(), own_metadata(new_item)) if new_item.location.category not in DETACHED_CATEGORIES: store.update_children(parent_location, parent.children + [new_item.location.url()]) diff --git a/common/lib/xmodule/xmodule/modulestore/xml_exporter.py b/common/lib/xmodule/xmodule/modulestore/xml_exporter.py index 509a2c7db9..c903fa5bce 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_exporter.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_exporter.py @@ -32,7 +32,7 @@ def export_to_xml(modulestore, contentstore, course_location, root_dir, course_d policies_dir = export_fs.makeopendir('policies') course_run_policy_dir = policies_dir.makeopendir(course.location.name) with course_run_policy_dir.open('grading_policy.json', 'w') as grading_policy: - grading_policy.write(dumps(course.definition['data']['grading_policy'])) + grading_policy.write(dumps(course.grading_policy)) def export_extra_content(export_fs, modulestore, course_location, category_type, dirname, file_suffix=''): @@ -43,4 +43,4 @@ def export_extra_content(export_fs, modulestore, course_location, category_type, item_dir = export_fs.makeopendir(dirname) for item in items: with item_dir.open(item.location.name + file_suffix, 'w') as item_file: - item_file.write(item.definition['data'].encode('utf8')) + item_file.write(item.data.encode('utf8')) diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index d4251360a9..52c695a9eb 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -192,6 +192,7 @@ def import_from_xml(store, data_dir, course_dirs=None, course_dirs=course_dirs, load_error_modules=load_error_modules ) + data_dir = path(data_dir) # NOTE: the XmlModuleStore does not implement get_items() which would be a preferable means # to enumerate the entire collection of course modules. It will be left as a TBD to implement that diff --git a/common/lib/xmodule/xmodule/tests/test_capa_module.py b/common/lib/xmodule/xmodule/tests/test_capa_module.py index a22fcdb5f6..222f5689b1 100644 --- a/common/lib/xmodule/xmodule/tests/test_capa_module.py +++ b/common/lib/xmodule/xmodule/tests/test_capa_module.py @@ -62,37 +62,30 @@ class CapaFactory(object): definition = {'data': CapaFactory.sample_problem_xml, } location = Location(["i4x", "edX", "capa_test", "problem", "SampleProblem{0}".format(CapaFactory.next_num())]) - metadata = {} + model_data = {} if graceperiod is not None: - metadata['graceperiod'] = graceperiod + model_data['graceperiod'] = graceperiod if due is not None: - metadata['due'] = due + model_data['due'] = due if max_attempts is not None: - metadata['attempts'] = max_attempts + model_data['max_attempts'] = int(max_attempts) if showanswer is not None: - metadata['showanswer'] = showanswer + model_data['showanswer'] = showanswer if force_save_button is not None: - metadata['force_save_button'] = force_save_button + model_data['force_save_button'] = force_save_button if rerandomize is not None: - metadata['rerandomize'] = rerandomize + model_data['rerandomize'] = rerandomize descriptor = Mock(weight="1") - instance_state_dict = {} if problem_state is not None: - instance_state_dict = problem_state + model_data.update(problem_state) if attempts is not None: # converting to int here because I keep putting "0" and "1" in the tests # since everything else is a string. - instance_state_dict['attempts'] = int(attempts) - if len(instance_state_dict) > 0: - instance_state = json.dumps(instance_state_dict) - else: - instance_state = None + model_data['attempts'] = int(attempts) - module = CapaModule(test_system, location, - definition, descriptor, - instance_state, None, metadata=metadata) + module = CapaModule(test_system, location, descriptor, model_data) return module diff --git a/common/lib/xmodule/xmodule/tests/test_conditional.py b/common/lib/xmodule/xmodule/tests/test_conditional.py index 361a6ea785..d4d5a1d2e1 100644 --- a/common/lib/xmodule/xmodule/tests/test_conditional.py +++ b/common/lib/xmodule/xmodule/tests/test_conditional.py @@ -75,17 +75,12 @@ class ConditionalModuleTest(unittest.TestCase): print "Course: ", course print "id: ", course.id - instance_states = dict(problem=None) - shared_state = None - def inner_get_module(descriptor): if isinstance(descriptor, Location): location = descriptor descriptor = self.modulestore.get_instance(course.id, location, depth=None) location = descriptor.location - instance_state = instance_states.get(location.category, None) - print "inner_get_module, location=%s, inst_state=%s" % (location, instance_state) - return descriptor.xmodule_constructor(test_system)(instance_state, shared_state) + return descriptor.xmodule(test_system) location = Location(["i4x", "edX", "cond_test", "conditional", "condone"]) @@ -96,7 +91,7 @@ class ConditionalModuleTest(unittest.TestCase): module = inner_get_module(location) print "module: ", module - print "module definition: ", module.definition + print "module.condition: ", module.condition print "module children: ", module.get_children() print "module display items (children): ", module.get_display_items() @@ -110,12 +105,12 @@ class ConditionalModuleTest(unittest.TestCase): print "gdi=", gdi ajax = json.loads(module.handle_ajax('', '')) - self.assertTrue('xmodule.conditional_module' in ajax['html']) print "ajax: ", ajax + self.assertTrue('ConditionalModule' in ajax['html']) # now change state of the capa problem to make it completed - instance_states['problem'] = json.dumps({'attempts': 1}) + inner_get_module(Location('i4x://edX/cond_test/problem/choiceprob')).attempts = 1 ajax = json.loads(module.handle_ajax('', '')) - self.assertTrue('This is a secret' in ajax['html']) print "post-attempt ajax: ", ajax + self.assertTrue('This is a secret' in ajax['html']) diff --git a/common/lib/xmodule/xmodule/tests/test_import.py b/common/lib/xmodule/xmodule/tests/test_import.py index c0ecd7e854..4f446d3c50 100644 --- a/common/lib/xmodule/xmodule/tests/test_import.py +++ b/common/lib/xmodule/xmodule/tests/test_import.py @@ -354,7 +354,7 @@ class ImportTestCase(BaseCourseTestCase): render_string_from_sample_gst_xml = """ \ """.strip() - self.assertEqual(gst_sample.definition['render'], render_string_from_sample_gst_xml) + self.assertEqual(gst_sample.render, render_string_from_sample_gst_xml) def test_cohort_config(self): """ diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index e2d86c6546..9681b3bdfd 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -390,9 +390,9 @@ class XModuleDescriptor(HTMLSnippet, ResourceTemplates, XBlock): def xmodule(self, system): """ - Returns a constructor for an XModule. This constructor takes two - arguments: instance_state and shared_state, and returns a fully - instantiated XModule + Returns an XModule. + + system: Module system """ return self.module_class( system,