diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index 1f49435ca2..b8a85595dc 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -670,6 +670,11 @@ class CapaDescriptor(RawDescriptor): # actually use type and points? metadata_attributes = RawDescriptor.metadata_attributes + ('type', 'points') + # The capa format specifies that what we call max_attempts in the code + # is the attribute `attempts`. This will do that conversion + metadata_translations = dict(RawDescriptor.metadata_translations) + metadata_translations['attempts'] = 'max_attempts' + # VS[compat] # TODO (cpennington): Delete this method once all fall 2012 course are being # edited in the cms diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index c4dc63c1b4..b2a1917912 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -33,6 +33,9 @@ class CourseDescriptor(SequenceDescriptor): show_calculator = Boolean(help="Whether to show the calculator in this course", default=False, scope=Scope.settings) start = Date(help="Start time when this module is visible", scope=Scope.settings) display_name = String(help="Display name for this module", scope=Scope.settings) + tabs = List(help="List of tabs to enable in this course", scope=Scope.settings) + end_of_course_survey_url = String(help="Url for the end-of-course survey", scope=Scope.settings) + discussion_blackouts = List(help="List of pairs of start/end dates for discussion blackouts", scope=Scope.settings, default=[]) has_children = True info_sidebar_name = String(scope=Scope.settings, default='Course Handouts') @@ -128,7 +131,7 @@ class CourseDescriptor(SequenceDescriptor): if self.start is None: msg = "Course loaded without a valid start date. id = %s" % self.id # hack it -- start in 1970 - self.metadata['start'] = stringify_time(time.gmtime(0)) + self.lms.start = time.gmtime(0) log.critical(msg) system.error_tracker(msg) @@ -198,8 +201,6 @@ class CourseDescriptor(SequenceDescriptor): grading_policy['GRADER'] = grader_from_conf(grading_policy['GRADER']) self._grading_policy = grading_policy - - @classmethod def read_grading_policy(cls, paths, system): """Load a grading policy from the specified paths, in order, if it exists.""" @@ -221,14 +222,13 @@ class CourseDescriptor(SequenceDescriptor): return policy_str - @classmethod def from_xml(cls, xml_data, system, org=None, course=None): instance = super(CourseDescriptor, cls).from_xml(xml_data, system, org, course) # bleh, have to parse the XML here to just pull out the url_name attribute course_file = StringIO(xml_data) - xml_obj = etree.parse(course_file,parser=edx_xml_parser).getroot() + xml_obj = etree.parse(course_file, parser=edx_xml_parser).getroot() policy_dir = None url_name = xml_obj.get('url_name', xml_obj.get('slug')) @@ -241,7 +241,7 @@ class CourseDescriptor(SequenceDescriptor): paths = [policy_dir + '/grading_policy.json'] + paths policy = json.loads(cls.read_grading_policy(paths, system)) - + # cdodge: import the grading policy information that is on disk and put into the # descriptor 'definition' bucket as a dictionary so that it is persisted in the DB instance.grading_policy = policy @@ -250,7 +250,6 @@ class CourseDescriptor(SequenceDescriptor): instance.set_grading_policy(policy) return instance - @classmethod def definition_from_xml(cls, xml_object, system): @@ -334,17 +333,6 @@ class CourseDescriptor(SequenceDescriptor): self.definition['data'].setdefault('grading_policy',{})['GRADE_CUTOFFS'] = value - @property - def tabs(self): - """ - Return the tabs config, as a python object, or None if not specified. - """ - return self.metadata.get('tabs') - - @tabs.setter - def tabs(self, value): - self.metadata['tabs'] = value - @lazyproperty def grading_context(self): """ @@ -383,14 +371,14 @@ class CourseDescriptor(SequenceDescriptor): for c in self.get_children(): sections = [] for s in c.get_children(): - if s.metadata.get('graded', False): + if s.lms.graded: xmoduledescriptors = list(yield_descriptor_descendents(s)) xmoduledescriptors.append(s) # The xmoduledescriptors included here are only the ones that have scores. section_description = { 'section_descriptor' : s, 'xmoduledescriptors' : filter(lambda child: child.has_score, xmoduledescriptors) } - section_format = s.metadata.get('format', "") + section_format = s.lms.format graded_sections[ section_format ] = graded_sections.get( section_format, [] ) + [section_description] all_descriptors.extend(xmoduledescriptors) @@ -447,7 +435,7 @@ class CourseDescriptor(SequenceDescriptor): try: blackout_periods = [(parse_time(start), parse_time(end)) for start, end - in self.metadata.get('discussion_blackouts', [])] + in self.discussion_blackouts] now = time.gmtime() for start, end in blackout_periods: if start <= now <= end: @@ -457,17 +445,6 @@ class CourseDescriptor(SequenceDescriptor): return True - - @property - def end_of_course_survey_url(self): - """ - Pull from policy. Once we have our own survey module set up, can change this to point to an automatically - created survey for each class. - - Returns None if no url specified. - """ - return self.metadata.get('end_of_course_survey_url') - @property def title(self): return self.display_name diff --git a/common/lib/xmodule/xmodule/discussion_module.py b/common/lib/xmodule/xmodule/discussion_module.py index a193604278..19a2b4300f 100644 --- a/common/lib/xmodule/xmodule/discussion_module.py +++ b/common/lib/xmodule/xmodule/discussion_module.py @@ -12,6 +12,10 @@ class DiscussionModule(XModule): } js_module_name = "InlineDiscussion" + discussion_id = String(scope=Scope.settings) + discussion_category = String(scope=Scope.settings) + discussion_target = String(scope=Scope.settings) + data = String(help="XML definition of inline discussion", scope=Scope.content) def get_html(self): @@ -31,3 +35,10 @@ class DiscussionModule(XModule): class DiscussionDescriptor(RawDescriptor): module_class = DiscussionModule template_dir_name = "discussion" + + # The discussion XML format uses `id` and `for` attributes, + # but these would overload other module attributes, so we prefix them + # for actual use in the code + metadata_translations = dict(RawDescriptor.metadata_translations) + metadata_translations['id'] = 'discussion_id' + metadata_translations['for'] = 'discussion_target' diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index 677b2e938e..ce6d9df093 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -430,10 +430,10 @@ class XMLModuleStore(ModuleStoreBase): NOTE: This means that there is no such thing as lazy loading at the moment--this accesses all the children.""" for child in descriptor.get_children(): - inherit_metadata(child, descriptor.metadata) + inherit_metadata(child, descriptor._model_data) compute_inherited_metadata(child) - def inherit_metadata(descriptor, metadata): + def inherit_metadata(descriptor, model_data): """ Updates this module with metadata inherited from a containing module. Only metadata specified in self.inheritable_metadata will @@ -441,11 +441,10 @@ class XMLModuleStore(ModuleStoreBase): """ # Set all inheritable metadata from kwargs that are # in self.inheritable_metadata and aren't already set in metadata - for attr in self.inheritable_metadata: - if attr not in self.metadata and attr in metadata: - self._inherited_metadata.add(attr) - self.metadata[attr] = metadata[attr] - + for attr in descriptor.inheritable_metadata: + if attr not in descriptor._model_data and attr in model_data: + descriptor._inherited_metadata.add(attr) + descriptor._model_data[attr] = model_data[attr] compute_inherited_metadata(course_descriptor) @@ -485,7 +484,7 @@ class XMLModuleStore(ModuleStoreBase): if category == "static_tab": for tab in course_descriptor.tabs or []: if tab.get('url_slug') == slug: - module.display_name = tab['name'] + module.lms.display_name = tab['name'] module.data_dir = course_dir self.modules[course_descriptor.id][module.location] = module except Exception, e: diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py index 0bd2311021..c6342e9d13 100644 --- a/lms/djangoapps/courseware/access.py +++ b/lms/djangoapps/courseware/access.py @@ -155,7 +155,7 @@ def _has_access_course_desc(user, course, action): if settings.MITX_FEATURES.get('ACCESS_REQUIRE_STAFF_FOR_COURSE'): # if this feature is on, only allow courses that have ispublic set to be # seen by non-staff - if course.metadata.get('ispublic'): + if course.lms.ispublic: debug("Allow: ACCESS_REQUIRE_STAFF_FOR_COURSE and ispublic") return True return _has_staff_access_to_descriptor(user, course) diff --git a/lms/djangoapps/courseware/grades.py b/lms/djangoapps/courseware/grades.py index 1283844ade..f6fd7bda88 100644 --- a/lms/djangoapps/courseware/grades.py +++ b/lms/djangoapps/courseware/grades.py @@ -179,12 +179,12 @@ def grade(student, request, course, student_module_cache=None, keep_raw_scores=F else: correct = total - graded = module_descriptor.metadata.get("graded", False) + graded = module_descriptor.lms.graded if not total > 0: #We simply cannot grade a problem that is 12/0, because we might need it as a percentage graded = False - scores.append(Score(correct, total, graded, module_descriptor.metadata.get('display_name'))) + scores.append(Score(correct, total, graded, module_descriptor.lms.display_name)) section_total, graded_total = graders.aggregate_scores(scores, section_name) if keep_raw_scores: @@ -288,7 +288,7 @@ def progress_summary(student, request, course, student_module_cache): continue # Same for sections - graded = section_module.metadata.get('graded', False) + graded = section_module.lms.graded scores = [] module_creator = lambda descriptor : section_module.system.get_module(descriptor.location) @@ -301,20 +301,20 @@ def progress_summary(student, request, course, student_module_cache): continue scores.append(Score(correct, total, graded, - module_descriptor.metadata.get('display_name'))) + module_descriptor.lms.display_name)) scores.reverse() section_total, graded_total = graders.aggregate_scores( - scores, section_module.metadata.get('display_name')) + scores, section_module.lms.display_name) - format = section_module.metadata.get('format', "") + format = section_module.lms.format sections.append({ 'display_name': section_module.display_name, 'url_name': section_module.url_name, 'scores': scores, 'section_total': section_total, 'format': format, - 'due': section_module.metadata.get("due", ""), + 'due': section_module.due, 'graded': graded, }) diff --git a/lms/djangoapps/courseware/tests/tests.py b/lms/djangoapps/courseware/tests/tests.py index 5af79d4983..7a6d498660 100644 --- a/lms/djangoapps/courseware/tests/tests.py +++ b/lms/djangoapps/courseware/tests/tests.py @@ -488,8 +488,8 @@ class TestViewAuth(PageLoader): # Make courses start in the future tomorrow = time.time() + 24*3600 - self.toy.metadata['start'] = stringify_time(time.gmtime(tomorrow)) - self.full.metadata['start'] = stringify_time(time.gmtime(tomorrow)) + self.toy.lms.start = time.gmtime(tomorrow) + self.full.lms.start = time.gmtime(tomorrow) self.assertFalse(self.toy.has_started()) self.assertFalse(self.full.has_started()) diff --git a/lms/djangoapps/django_comment_client/utils.py b/lms/djangoapps/django_comment_client/utils.py index b3a1626d22..174805a999 100644 --- a/lms/djangoapps/django_comment_client/utils.py +++ b/lms/djangoapps/django_comment_client/utils.py @@ -142,9 +142,9 @@ def initialize_discussion_info(course): for location, module in all_modules.items(): if location.category == 'discussion': - id = module.metadata['id'] - category = module.metadata['discussion_category'] - title = module.metadata['for'] + id = module.discussion_id + category = module.discussion_category + title = module.discussion_target sort_key = module.metadata.get('sort_key', title) category = " / ".join([x.strip() for x in category.split("/")]) last_category = category.split("/")[-1] diff --git a/lms/xmodule_namespace.py b/lms/xmodule_namespace.py index 7f30108b73..c78052ed1b 100644 --- a/lms/xmodule_namespace.py +++ b/lms/xmodule_namespace.py @@ -15,7 +15,7 @@ class LmsNamespace(Namespace): format = String( help="What format this module is in (used for deciding which " "grader to apply, and what to show in the TOC)", - scope=Scope.settings + scope=Scope.settings, ) display_name = String( @@ -29,3 +29,4 @@ class LmsNamespace(Namespace): source_file = String(help="DO NOT USE", scope=Scope.settings) giturl = String(help="DO NOT USE", scope=Scope.settings, default='https://github.com/MITx') xqa_key = String(help="DO NOT USE", scope=Scope.settings) + ispublic = Boolean(help="Whether this course is open to the public, or only to admins", scope=Scope.settings)