Remove references to .definition and .metadata in modules

cms/djangoapps/models/settings/course_grading.py

@@ -135,7 +135,7 @@ class CourseGradingModel(object):
 
         descriptor = get_modulestore(course_location).get_item(course_location)
         descriptor.grade_cutoffs = cutoffs
-        get_modulestore(course_location).update_item(course_location, descriptor.definition['data'])
+        get_modulestore(course_location).update_item(course_location, descriptor._model_data._kvs._data)
 
         return cutoffs
 
@@ -203,8 +203,8 @@ class CourseGradingModel(object):
             course_location = Location(course_location)
 
         descriptor = get_modulestore(course_location).get_item(course_location)
-        if 'graceperiod' in descriptor.metadata: del descriptor.metadata['graceperiod']
-        get_modulestore(course_location).update_metadata(course_location, descriptor._model_data._kvs._data)
+        del descriptor.lms.graceperiod
+        get_modulestore(course_location).update_metadata(course_location, descriptor._model_data._kvs._metadata)
 
     @staticmethod
     def get_section_grader_type(location):
@@ -213,7 +213,7 @@ class CourseGradingModel(object):
 
         descriptor = get_modulestore(location).get_item(location)
         return {
-                "graderType": descriptor.metadata.get('format', u"Not Graded"),
+                "graderType": descriptor.lms.format or 'Not Graded',
                 "location": location,
                 "id": 99   # just an arbitrary value to
                 }

common/djangoapps/terrain/factories.py

@@ -7,13 +7,14 @@ from xmodule.modulestore.django import modulestore
 from time import gmtime
 from uuid import uuid4
 from xmodule.timeparse import stringify_time
+from xmodule.modulestore.inheritance import own_metadata
 
 
 class GroupFactory(Factory):
     FACTORY_FOR = Group
 
     name = 'staff_MITx/999/Robot_Super_Course'
-    
+
 
 class UserProfileFactory(Factory):
     FACTORY_FOR = UserProfile
@@ -81,18 +82,18 @@ class XModuleCourseFactory(Factory):
 
         # This metadata code was copied from cms/djangoapps/contentstore/views.py
         if display_name is not None:
-            new_course.metadata['display_name'] = display_name
+            new_course.lms.display_name = display_name
 
-        new_course.metadata['data_dir'] = uuid4().hex
-        new_course.metadata['start'] = stringify_time(gmtime())
+        new_course.data_dir = uuid4().hex
+        new_course.lms.start = gmtime()
         new_course.tabs = [{"type": "courseware"},
-            {"type": "course_info", "name": "Course Info"}, 
+            {"type": "course_info", "name": "Course Info"},
             {"type": "discussion", "name": "Discussion"},
             {"type": "wiki", "name": "Wiki"},
             {"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
 
@@ -140,16 +141,16 @@ 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.metadata['data_dir'] = parent.metadata['data_dir']
+        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.metadata['display_name'] = display_name
+            new_item.lms.display_name = display_name
 
         store.update_metadata(new_item.location.url(), new_item.own_metadata)
 
         if new_item.location.category not in DETACHED_CATEGORIES:
-            store.update_children(parent_location, parent.definition.get('children', []) + [new_item.location.url()])
+            store.update_children(parent_location, parent.children + [new_item.location.url()])
 
         return new_item
 

common/lib/xmodule/xmodule/course_module.py

@@ -175,6 +175,9 @@ class CourseDescriptor(SequenceDescriptor):
     no_grade = Boolean(help="True if this course isn't graded", default=False, scope=Scope.settings)
     disable_progress_graph = Boolean(help="True if this course shouldn't display the progress graph", default=False, scope=Scope.settings)
     pdf_textbooks = List(help="List of dictionaries containing pdf_textbook configuration", default=None, scope=Scope.settings)
+    remote_gradebook = String(scope=Scope.settings, default='')
+    allow_anonymous = Boolean(scope=Scope.settings, default=True)
+    allow_anonymous_to_peers = Boolean(scope=Scope.settings, default=False)
     has_children = True
 
     info_sidebar_name = String(scope=Scope.settings, default='Course Handouts')

common/lib/xmodule/xmodule/foldit_module.py

@@ -7,6 +7,7 @@ from pkg_resources import resource_string
 from xmodule.editing_module import EditingDescriptor
 from xmodule.x_module import XModule
 from xmodule.xml_module import XmlDescriptor
+from xblock.core import Scope, Integer, String
 
 log = logging.getLogger(__name__)
 
@@ -14,10 +15,16 @@ class FolditModule(XModule):
 
     css = {'scss': [resource_string(__name__, 'css/foldit/leaderboard.scss')]}
 
-    def __init__(self, system, location, definition, descriptor,
-                 instance_state=None, shared_state=None, **kwargs):
-        XModule.__init__(self, system, location, definition, descriptor,
-                         instance_state, shared_state, **kwargs)
+    # default to what Spring_7012x uses
+    required_level = Integer(default=4, scope=Scope.settings)
+    required_sublevel = Integer(default=5, scope=Scope.settings)
+    due = String(help="Date that this problem is due by", scope=Scope.settings, default='')
+
+    show_basic_score = String(scope=Scope.settings, default='false')
+    show_leaderboard = String(scope=Scope.settings, default='false')
+
+    def __init__(self, *args, **kwargs):
+        XModule.__init__(self, *args, **kwargs)
         """
 
         Example:
@@ -26,25 +33,17 @@ class FolditModule(XModule):
             required_sublevel="3"
             show_leaderboard="false"/>
         """
-        req_level = self.metadata.get("required_level")
-        req_sublevel = self.metadata.get("required_sublevel")
-
-        # default to what Spring_7012x uses
-        self.required_level = req_level if req_level else 4
-        self.required_sublevel = req_sublevel if req_sublevel else 5
-
         def parse_due_date():
             """
             Pull out the date, or None
             """
-            s = self.metadata.get("due")
+            s = self.due
             if s:
                 return parser.parse(s)
             else:
                 return None
 
-        self.due_str = self.metadata.get("due", "None")
-        self.due = parse_due_date()
+        self.due_time = parse_due_date()
 
     def is_complete(self):
         """
@@ -59,7 +58,7 @@ class FolditModule(XModule):
             self.system.anonymous_student_id,
             self.required_level,
             self.required_sublevel,
-            self.due)
+            self.due_time)
         return complete
 
     def completed_puzzles(self):
@@ -96,10 +95,10 @@ class FolditModule(XModule):
             self.required_level,
             self.required_sublevel)
 
-        showbasic = (self.metadata.get("show_basic_score").lower() == "true")
-        showleader = (self.metadata.get("show_leaderboard").lower() == "true")
+        showbasic = (self.show_basic_score.lower() == "true")
+        showleader = (self.show_leaderboard.lower() == "true")
         context = {
-            'due': self.due_str,
+            'due': self.due,
             'success': self.is_complete(),
             'goal_level': goal_level,
             'completed': self.completed_puzzles(),
@@ -121,7 +120,7 @@ class FolditModule(XModule):
             self.required_sublevel)
 
         context = {
-            'due': self.due_str,
+            'due': self.due,
             'success': self.is_complete(),
             'goal_level': goal_level,
             'completed': self.completed_puzzles(),
@@ -170,9 +169,4 @@ class FolditDescriptor(XmlDescriptor, EditingDescriptor):
     # so always need to actually check.
     always_recalculate_grades = True
 
-    @classmethod
-    def definition_from_xml(cls, xml_object, system):
-        """
-        Get the xml_object's attributes.
-        """
-        return {'metadata': xml_object.attrib}
+    metadata_attributes = XmlDescriptor.metadata_attributes + ('required_level', 'required_sublevel')

common/lib/xmodule/xmodule/modulestore/xml_importer.py

@@ -109,8 +109,8 @@ def import_module_from_xml(modulestore, static_content_store, course_data_path,
                 course=target_location_namespace.course, name=target_location_namespace.name)
 
         # then remap children pointers since they too will be re-namespaced
-        children_locs = module.definition.get('children')
-        if children_locs is not None:
+        if module.has_children:
+            children_locs = module.children
             new_locs = []
             for child in children_locs:
                 child_loc = Location(child)
@@ -119,7 +119,7 @@ def import_module_from_xml(modulestore, static_content_store, course_data_path,
 
                 new_locs.append(new_child_loc.url())
 
-            module.definition['children'] = new_locs
+            module.children = new_locs
 
     if hasattr(module, 'data'):
         # cdodge: now go through any link references to '/static/' and make sure we've imported
@@ -237,7 +237,7 @@ def validate_category_hierarchy(module_store, course_id, parent_category, expect
             parents.append(module)
 
     for parent in parents:
-        for child_loc in [Location(child) for child in parent.definition.get('children', [])]:
+        for child_loc in [Location(child) for child in parent.children]:
             if child_loc.category != expected_child_category:
                 err_cnt += 1
                 print 'ERROR: child {0} of parent {1} was expected to be category of {2} but was {3}'.format(

common/lib/xmodule/xmodule/videoalpha_module.py

@@ -11,6 +11,7 @@ from xmodule.raw_module import RawDescriptor
 from xmodule.modulestore.mongo import MongoModuleStore
 from xmodule.modulestore.django import modulestore
 from xmodule.contentstore.content import StaticContent
+from xblock.core import Integer, Scope, String
 
 import datetime
 import time
@@ -46,11 +47,13 @@ class VideoAlphaModule(XModule):
     css = {'scss': [resource_string(__name__, 'css/videoalpha/display.scss')]}
     js_module_name = "VideoAlpha"
 
-    def __init__(self, system, location, definition, descriptor,
-                 instance_state=None, shared_state=None, **kwargs):
-        XModule.__init__(self, system, location, definition, descriptor,
-                         instance_state, shared_state, **kwargs)
-        xmltree = etree.fromstring(self.definition['data'])
+    data = String(help="XML data for the problem", scope=Scope.content)
+    position = Integer(help="Current position in the video", scope=Scope.student_state, default=0)
+    display_name = String(help="Display name for this module", scope=Scope.settings)
+
+    def __init__(self, *args, **kwargs):
+        XModule.__init__(self, *args, **kwargs)
+        xmltree = etree.fromstring(self.data)
         self.youtube_streams = xmltree.get('youtube')
         self.sub = xmltree.get('sub')
         self.position = 0
@@ -64,11 +67,6 @@ class VideoAlphaModule(XModule):
         self.track = self._get_track(xmltree)
         self.start_time, self.end_time = self._get_timeframe(xmltree)
 
-        if instance_state is not None:
-            state = json.loads(instance_state)
-            if 'position' in state:
-                self.position = int(float(state['position']))
-
     def _get_source(self, xmltree, exts=None):
         """Find the first valid source, which ends with one of `exts`."""
         exts = ['mp4', 'ogv', 'avi', 'webm'] if exts is None else exts
@@ -131,7 +129,7 @@ class VideoAlphaModule(XModule):
         else:
             # VS[compat]
             # cdodge: filesystem static content support.
-            caption_asset_path = "/static/{0}/subs/".format(self.metadata['data_dir'])
+            caption_asset_path = "/static/{0}/subs/".format(getattr(self, 'data_dir', None))
 
         return self.system.render_template('videoalpha.html', {
             'youtube_streams': self.youtube_streams,
@@ -141,7 +139,7 @@ class VideoAlphaModule(XModule):
             'track': self.track,
             'display_name': self.display_name,
             # TODO (cpennington): This won't work when we move to data that isn't on the filesystem
-            'data_dir': self.metadata['data_dir'],
+            'data_dir': getattr(self, 'data_dir', None),
             'caption_asset_path': caption_asset_path,
             'show_captions': self.show_captions,
             'start': self.start_time,

lms/djangoapps/courseware/courses.py

@@ -250,7 +250,7 @@ def get_course_syllabus_section(course, section_key):
             with fs.open(filepath) as htmlFile:
                 return replace_static_urls(
                     htmlFile.read().decode('utf-8'),
-                    course.metadata['data_dir'],
+                    getattr(course, 'data_dir', None),
                     course_namespace=course.location
                 )
         except ResourceNotFoundError:

lms/djangoapps/courseware/features/courses.py

@@ -26,8 +26,8 @@ def get_courseware_with_tabs(course_id):
     top three levels of navigation. Same as get_courseware() except include
     the tabs on the right hand main navigation page.
 
-    This hides the appropriate courseware as defined by the XML flag test:
-    chapter.metadata.get('hide_from_toc','false').lower() == 'true'
+    This hides the appropriate courseware as defined by the hide_from_toc field:
+    chapter.lms.hide_from_toc
 
     Example:
 
@@ -80,14 +80,14 @@ def get_courseware_with_tabs(course_id):
     """
 
     course = get_course_by_id(course_id)
-    chapters = [chapter for chapter in course.get_children() if chapter.metadata.get('hide_from_toc', 'false').lower() != 'true']
+    chapters = [chapter for chapter in course.get_children() if not chapter.lms.hide_from_toc]
     courseware = [{'chapter_name': c.display_name,
                     'sections': [{'section_name': s.display_name,
                                 'clickable_tab_count': len(s.get_children()) if (type(s) == seq_module.SequenceDescriptor) else 0,
                                 'tabs': [{'children_count': len(t.get_children()) if (type(t) == vertical_module.VerticalDescriptor) else 0,
                                         'class': t.__class__.__name__}
                                         for t in s.get_children()]}
-                                for s in c.get_children() if s.metadata.get('hide_from_toc', 'false').lower() != 'true']}
+                                for s in c.get_children() if not s.lms.hide_from_toc]}
                     for c in chapters]
 
     return courseware

lms/djangoapps/courseware/tests/test_access.py

@@ -88,7 +88,6 @@ class AccessTestCase(TestCase):
         yesterday = time.gmtime(time.time() - 86400)
         tomorrow = time.gmtime(time.time() + 86400)
         c = Mock(enrollment_start=yesterday, enrollment_end=tomorrow)
-        c.metadata.get = 'is_public'
 
         # User can enroll if it is between the start and end dates
         self.assertTrue(access._has_access_course_desc(u, c, 'enroll'))
@@ -99,7 +98,6 @@ class AccessTestCase(TestCase):
         u.is_authenticated.return_value = True
 
         c = Mock(enrollment_start=tomorrow, enrollment_end=tomorrow, id='edX/test/2012_Fall')
-        c.metadata.get = 'is_public'
 
         allowed = CourseEnrollmentAllowedFactory(email=u.email, course_id=c.id)
 
@@ -110,7 +108,6 @@ class AccessTestCase(TestCase):
         u.is_authenticated.return_value = True
 
         c = Mock(enrollment_start=tomorrow, enrollment_end=tomorrow, id='edX/test/Whenever')
-        c.metadata.get = 'is_public'
         self.assertTrue(access._has_access_course_desc(u, c, 'enroll'))
 
         # TODO:

lms/djangoapps/courseware/tests/tests.py

@@ -325,7 +325,7 @@ class PageLoader(ActivateLoginTestCase):
                     num_bad += 1
                 elif isinstance(descriptor, ErrorDescriptor):
                     msg = "ERROR error descriptor loaded: "
-                    msg = msg + descriptor.definition['data']['error_msg']
+                    msg = msg + descriptor.error_msg
                     all_ok = False
                     num_bad += 1
 

lms/djangoapps/courseware/views.py

@@ -194,7 +194,7 @@ def check_for_active_timelimit_module(request, course_id, course):
                 # This value should be in milliseconds.
                 remaining_time = timelimit_module.get_remaining_time_in_ms()
                 context['timer_expiration_duration'] = remaining_time
-                context['suppress_toplevel_navigation'] = timelimit_descriptor.metadata.suppress_toplevel_navigation
+                context['suppress_toplevel_navigation'] = timelimit_descriptor.suppress_toplevel_navigation
                 return_url = reverse('jump_to', kwargs={'course_id': course_id, 'location': location})
                 context['timer_navigation_return_url'] = return_url
     return context

lms/djangoapps/django_comment_client/base/views.py

@@ -72,17 +72,17 @@ def create_thread(request, course_id, commentable_id):
     course = get_course_with_access(request.user, course_id, 'load')
     post = request.POST
 
-    if course.metadata.get("allow_anonymous", True):
+    if course.allow_anonymous:
         anonymous = post.get('anonymous', 'false').lower() == 'true'
     else:
         anonymous = False
 
-    if course.metadata.get("allow_anonymous_to_peers", False):
+    if course.allow_anonymous_to_peers:
         anonymous_to_peers = post.get('anonymous_to_peers', 'false').lower() == 'true'
     else:
         anonymous_to_peers = False
 
-    thread = cc.Thread(**extract(post, ['body', 'title', 'tags']))
+    thread = cc.Thread(**extract(post,/ ['body', 'title', 'tags']))
     thread.update_attributes(**{
         'anonymous': anonymous,
         'anonymous_to_peers': anonymous_to_peers,
@@ -96,7 +96,7 @@ def create_thread(request, course_id, commentable_id):
     #kevinchugh because the new requirement is that all groups will be determined
     #by the group id in the request this all goes away
     #not anymore, only for admins
-    
+
     # Cohort the thread if the commentable is cohorted.
     if is_commentable_cohorted(course_id, commentable_id):
         user_group_id = get_cohort_id(user, course_id)
@@ -113,7 +113,7 @@ def create_thread(request, course_id, commentable_id):
 
         if group_id:
             thread.update_attributes(group_id=group_id)
-    
+
     thread.save()
 
     if post.get('auto_subscribe', 'false').lower() == 'true':
@@ -147,12 +147,12 @@ def _create_comment(request, course_id, thread_id=None, parent_id=None):
     comment = cc.Comment(**extract(post, ['body']))
 
     course = get_course_with_access(request.user, course_id, 'load')
-    if course.metadata.get("allow_anonymous", True):
+    if course.allow_anonymous:
         anonymous = post.get('anonymous', 'false').lower() == 'true'
     else:
         anonymous = False
 
-    if course.metadata.get("allow_anonymous_to_peers", False):
+    if course.allow_anonymous_to_peers:
         anonymous_to_peers = post.get('anonymous_to_peers', 'false').lower() == 'true'
     else:
         anonymous_to_peers = False

lms/djangoapps/django_comment_client/forum/views.py

@@ -124,31 +124,31 @@ def inline_discussion(request, course_id, discussion_id):
 
     annotated_content_info = utils.get_metadata_for_threads(course_id, threads, request.user, user_info)
 
-    allow_anonymous = course.metadata.get("allow_anonymous", True)
-    allow_anonymous_to_peers = course.metadata.get("allow_anonymous_to_peers", False)
+    allow_anonymous = course.allow_anonymous
+    allow_anonymous_to_peers = course.allow_anonymous_to_peers
 
     #since inline is all one commentable, only show or allow the choice of cohorts
     #if the commentable is cohorted, otherwise everything is not cohorted
     #and no one has the option of choosing a cohort
     is_cohorted = is_course_cohorted(course_id) and is_commentable_cohorted(course_id, discussion_id)
     is_moderator = cached_has_permission(request.user, "see_all_cohorts", course_id)
-    
+
     cohorts_list = list()
-    
+
     if is_cohorted:
         cohorts_list.append({'name':'All Groups','id':None})
-        
+
         #if you're a mod, send all cohorts and let you pick
-        
+
         if is_moderator:
             cohorts = get_course_cohorts(course_id)
             for c in cohorts:
                 cohorts_list.append({'name':c.name, 'id':c.id})
-                
+
         else:
             #students don't get to choose
             cohorts_list = None
-          
+
     return utils.JsonResponse({
         'discussion_data': map(utils.safe_content, threads),
         'user_info': user_info,

lms/djangoapps/django_comment_client/tests.py

@@ -62,7 +62,7 @@ from courseware.tests.tests import PageLoader, TEST_DATA_XML_MODULESTORE
 #        #self.assertEqual(resp.status_code, 200)
 #        #self.assertEqual(my_save.something, "expected", "complaint if not true")
 #
-#        self.toy.metadata["cohort_config"] = {"cohorted": True}
+#        self.toy.cohort_config = {"cohorted": True}
 #
 #        # call the view again ...
 #

lms/djangoapps/django_comment_client/utils.py

@@ -173,8 +173,8 @@ def initialize_discussion_info(course):
     path_to_locations = {}
     for module in all_modules:
         skip_module = False
-        for key in ('id', 'discussion_category', 'for'):
-            if key not in module.metadata:
+        for key in ('discussion_id', 'discussion_category', 'discussion_target'):
+            if getattr(module, key) is None:
                 log.warning("Required key '%s' not in discussion %s, leaving out of category map" % (key, module.location))
                 skip_module = True
 

lms/djangoapps/instructor/views.py

@@ -136,7 +136,7 @@ def instructor_dashboard(request, course_id):
 
     if settings.MITX_FEATURES['ENABLE_MANUAL_GIT_RELOAD']:
         if 'GIT pull' in action:
-            data_dir = course.metadata['data_dir']
+            data_dir = getattr(course, 'data_dir')
             log.debug('git pull {0}'.format(data_dir))
             gdir = settings.DATA_DIR / data_dir
             if not os.path.exists(gdir):
@@ -150,7 +150,7 @@ def instructor_dashboard(request, course_id):
         if 'Reload course' in action:
             log.debug('reloading {0} ({1})'.format(course_id, course))
             try:
-                data_dir = course.metadata['data_dir']
+                data_dir = getattr(course, 'data_dir')
                 modulestore().try_load_course(data_dir)
                 msg += "<br/><p>Course reloaded from {0}</p>".format(data_dir)
                 track.views.server_track(request, 'reload {0}'.format(data_dir), {}, page='idashboard')
@@ -404,7 +404,7 @@ def instructor_dashboard(request, course_id):
         def getdat(u):
             p = u.profile
             return [u.username, u.email] + [getattr(p,x,'') for x in profkeys]
-        
+
         datatable['data'] = [getdat(u) for u in enrolled_students]
         datatable['title'] = 'Student profile data for course %s' % course_id
         return return_csv('profiledata_%s.csv' % course_id, datatable)
@@ -426,7 +426,7 @@ def instructor_dashboard(request, course_id):
             msg+="<font color='red'>Couldn't find module with that urlname.  </font>"
             msg += "<pre>%s</pre>" % escape(err)
             smdat = []
-        
+
         if smdat:
             datatable = {'header': ['username', 'state']}
             datatable['data'] = [ [x.student.username, x.state] for x in smdat ]
@@ -621,7 +621,7 @@ def _do_remote_gradebook(user, course, action, args=None, files=None):
     '''
     Perform remote gradebook action.  Returns msg, datatable.
     '''
-    rg = course.metadata.get('remote_gradebook', '')
+    rg = course.remote_gradebook
     if not rg:
         msg = "No remote gradebook defined in course metadata"
         return msg, {}

lms/djangoapps/staticbook/views.py

@@ -46,7 +46,7 @@ def pdf_index(request, course_id, book_index, chapter=None, page=None):
         input_url = "'" + original_url + "'"
         output_url = replace_static_urls(
                     input_url,
-                    course.metadata['data_dir'],
+                    getattr(course, 'data_dir', None),
                     course_namespace=course.location
                 )
         # strip off the quotes again...
@@ -57,12 +57,12 @@ def pdf_index(request, course_id, book_index, chapter=None, page=None):
     # then remap all the chapter URLs as well, if they are provided.
     if 'chapters' in textbook:
         for entry in textbook['chapters']:
-            entry['url'] = remap_static_url(entry['url'], course)            
+            entry['url'] = remap_static_url(entry['url'], course)
 
 
     return render_to_response('static_pdfbook.html',
-                              {'book_index': book_index, 
-                               'course': course, 
+                              {'book_index': book_index,
+                               'course': course,
                                'textbook': textbook,
                                'chapter': chapter,
                                'page': page,