Incidental functionality improvements from Next Gen Modulestore work

cms/djangoapps/contentstore/tests/test_course_settings.py

@@ -105,7 +105,6 @@ def test_ooc_encoder(self):
         self.assertEqual(jsondetails['string'], 'string')
 
     def test_update_and_fetch(self):
-        # # NOTE: I couldn't figure out how to validly test time setting w/ all the conversions
         jsondetails = CourseDetails.fetch(self.course_location)
         jsondetails.syllabus = "<a href='foo'>bar</a>"
         # encode - decode to convert date fields and other data which changes form
@@ -128,6 +127,11 @@ def test_update_and_fetch(self):
             CourseDetails.update_from_json(jsondetails.__dict__).effort,
             jsondetails.effort, "After set effort"
         )
+        jsondetails.start_date = datetime.datetime(2010, 10, 1, 0, tzinfo=UTC())
+        self.assertEqual(
+            CourseDetails.update_from_json(jsondetails.__dict__).start_date,
+            jsondetails.start_date
+        )
 
     @override_settings(MKTG_URLS={'ROOT': 'dummy-root'})
     def test_marketing_site_fetch(self):
@@ -235,8 +239,7 @@ def compare_date_fields(self, details, encoded, context, field):
                 dt1 = date.from_json(encoded[field])
                 dt2 = details[field]
 
-                expected_delta = datetime.timedelta(0)
-                self.assertEqual(dt1 - dt2, expected_delta, str(dt1) + "!=" + str(dt2) + " at " + context)
+                self.assertEqual(dt1, dt2, msg="{} != {} at {}".format(dt1, dt2, context))
             else:
                 self.fail(field + " missing from encoded but in details at " + context)
         elif field in encoded and encoded[field] is not None:

cms/djangoapps/contentstore/views/access.py

@@ -2,20 +2,21 @@
 from auth.authz import is_user_in_course_group_role
 from django.core.exceptions import PermissionDenied
 from ..utils import get_course_location_for_item
+from xmodule.modulestore import Location
 
 
 def get_location_and_verify_access(request, org, course, name):
     """
-    Create the location tuple verify that the user has permissions
-    to view the location. Returns the location.
+    Create the location, verify that the user has permissions
+    to view the location. Returns the location as a Location
     """
     location = ['i4x', org, course, 'course', name]
 
     # check that logged in user has permissions to this item
     if not has_access(request.user, location):
         raise PermissionDenied()
 
-    return location
+    return Location(location)
 
 
 def has_access(user, location, role=STAFF_ROLE_NAME):

cms/djangoapps/contentstore/views/assets.py

@@ -258,7 +258,7 @@ def import_course(request, org, course, name):
         _module_store, course_items = import_from_xml(modulestore('direct'), settings.GITHUB_REPO_ROOT,
                                                       [course_subdir], load_error_modules=False,
                                                       static_content_store=contentstore(),
-                                                      target_location_namespace=Location(location),
+                                                      target_location_namespace=location,
                                                       draft_store=modulestore())
 
         # we can blow this away when we're done importing.

cms/djangoapps/contentstore/views/checklist.py

@@ -67,7 +67,9 @@ def update_checklist(request, org, course, name, checklist_index=None):
         if checklist_index is not None and 0 <= int(checklist_index) < len(course_module.checklists):
             index = int(checklist_index)
             course_module.checklists[index] = json.loads(request.body)
-            checklists, modified = expand_checklist_action_urls(course_module)
+            # seeming noop which triggers kvs to record that the metadata is not default
+            course_module.checklists = course_module.checklists
+            checklists, _ = expand_checklist_action_urls(course_module)
             modulestore.update_metadata(location, own_metadata(course_module))
             return HttpResponse(json.dumps(checklists[index]), mimetype="application/json")
         else:

cms/djangoapps/contentstore/views/component.py

@@ -38,7 +38,8 @@
 
 log = logging.getLogger(__name__)
 
-COMPONENT_TYPES = ['customtag', 'discussion', 'html', 'problem', 'video']
+# NOTE: edit_unit assumes this list is disjoint from ADVANCED_COMPONENT_TYPES
+COMPONENT_TYPES = ['discussion', 'html', 'problem', 'video']
 
 OPEN_ENDED_COMPONENT_TYPES = ["combinedopenended", "peergrading"]
 NOTE_COMPONENT_TYPES = ['notes']
@@ -220,7 +221,7 @@ def edit_unit(request, location):
         'section': containing_section,
         'create_new_unit_template': Location('i4x', 'edx', 'templates', 'vertical', 'Empty'),
         'unit_state': unit_state,
-        'published_date': item.cms.published_date.strftime('%B %d, %Y') if item.cms.published_date is not None else None,
+        'published_date': get_default_time_display(item.cms.published_date) if item.cms.published_date is not None else None
     })
 
 

cms/djangoapps/contentstore/views/course.py

@@ -153,7 +153,7 @@ def course_info(request, org, course, name, provided_id=None):
     course_module = modulestore().get_item(location)
 
     # get current updates
-    location = ['i4x', org, course, 'course_info', "updates"]
+    location = Location(['i4x', org, course, 'course_info', "updates"])
 
     return render_to_response('course_info.html', {
         'active_tab': 'courseinfo-tab',

cms/djangoapps/contentstore/views/session_kv_store.py

@@ -25,4 +25,4 @@ def delete(self, key):
             del self._session[tuple(key)]
 
     def has(self, key):
-        return key in self._descriptor_model_data or key in self._session
+        return key.field_name in self._descriptor_model_data or tuple(key) in self._session

cms/djangoapps/models/settings/course_details.py

@@ -74,7 +74,7 @@ def update_from_json(cls, jsondict):
         Decode the json into CourseDetails and save any changed attrs to the db
         """
         # TODO make it an error for this to be undefined & for it to not be retrievable from modulestore
-        course_location = jsondict['course_location']
+        course_location = Location(jsondict['course_location'])
         # Will probably want to cache the inflight courses because every blur generates an update
         descriptor = get_modulestore(course_location).get_item(course_location)
 

cms/envs/acceptance.py

@@ -28,15 +28,15 @@
 
 MODULESTORE = {
     'default': {
-        'ENGINE': 'xmodule.modulestore.mongo.DraftMongoModuleStore',
+        'ENGINE': 'xmodule.modulestore.draft.DraftModuleStore',
         'OPTIONS': MODULESTORE_OPTIONS
     },
     'direct': {
         'ENGINE': 'xmodule.modulestore.mongo.MongoModuleStore',
         'OPTIONS': MODULESTORE_OPTIONS
     },
     'draft': {
-        'ENGINE': 'xmodule.modulestore.mongo.DraftMongoModuleStore',
+        'ENGINE': 'xmodule.modulestore.draft.DraftModuleStore',
         'OPTIONS': MODULESTORE_OPTIONS
     }
 }

cms/envs/dev.py

@@ -27,7 +27,7 @@
 
 MODULESTORE = {
     'default': {
-        'ENGINE': 'xmodule.modulestore.mongo.DraftMongoModuleStore',
+        'ENGINE': 'xmodule.modulestore.draft.DraftModuleStore',
         'OPTIONS': modulestore_options
     },
     'direct': {

cms/envs/test.py

@@ -53,15 +53,15 @@
 
 MODULESTORE = {
     'default': {
-        'ENGINE': 'xmodule.modulestore.mongo.DraftMongoModuleStore',
+        'ENGINE': 'xmodule.modulestore.draft.DraftModuleStore',
         'OPTIONS': MODULESTORE_OPTIONS
     },
     'direct': {
         'ENGINE': 'xmodule.modulestore.mongo.MongoModuleStore',
         'OPTIONS': MODULESTORE_OPTIONS
     },
     'draft': {
-        'ENGINE': 'xmodule.modulestore.mongo.DraftMongoModuleStore',
+        'ENGINE': 'xmodule.modulestore.draft.DraftModuleStore',
         'OPTIONS': MODULESTORE_OPTIONS
     }
 }

cms/templates/edit_subsection.html

@@ -1,7 +1,7 @@
 <%inherit file="base.html" />
 <%!
    import logging
-   from xmodule.util.date_utils import get_default_time_display
+   from xmodule.util.date_utils import get_default_time_display, almost_same_datetime
 %>
 
 <%! from django.core.urlresolvers import reverse %>
@@ -47,9 +47,10 @@ <h4 class="header">Subsection Settings</h4>
                 placeholder="HH:MM" class="time" size='10' autocomplete="off"/>
           </div>
         </div>
-        % if subsection.lms.start != parent_item.lms.start and subsection.lms.start:
+        % if subsection.lms.start and not almost_same_datetime(subsection.lms.start, parent_item.lms.start):
             % if parent_item.lms.start is None:
-            <p class="notice">The date above differs from the release date of ${parent_item.display_name_with_default}, which is unset.
+            <p class="notice">The date above differs from the release date of 
+            ${parent_item.display_name_with_default}, which is unset.
             % else:
             <p class="notice">The date above differs from the release date of ${parent_item.display_name_with_default} –
                 ${get_default_time_display(parent_item.lms.start)}.

common/djangoapps/util/json_request.py

@@ -15,8 +15,7 @@ def expect_json_with_cloned_request(request, *args, **kwargs):
         # e.g. 'charset', so we can't do a direct string compare
         if request.META.get('CONTENT_TYPE', '').lower().startswith("application/json"):
             cloned_request = copy.copy(request)
-            cloned_request.POST = cloned_request.POST.copy()
-            cloned_request.POST.update(json.loads(request.body))
+            cloned_request.POST = json.loads(request.body)
             return view_function(cloned_request, *args, **kwargs)
         else:
             return view_function(request, *args, **kwargs)

common/lib/xmodule/xmodule/contentstore/mongo.py

@@ -2,7 +2,8 @@
 import gridfs
 from gridfs.errors import NoFile
 
-from xmodule.modulestore.mongo import location_to_query, Location
+from xmodule.modulestore import Location
+from xmodule.modulestore.mongo.base import location_to_query
 from xmodule.contentstore.content import XASSET_LOCATION_TAG
 
 import logging

common/lib/xmodule/xmodule/modulestore/__init__.py

@@ -16,16 +16,7 @@
 
 
 URL_RE = re.compile("""
-    (?P<tag>[^:]+)://
-    (?P<org>[^/]+)/
-    (?P<course>[^/]+)/
-    (?P<category>[^/]+)/
-    (?P<name>[^@]+)
-    (@(?P<revision>[^/]+))?
-    """, re.VERBOSE)
-
-MISSING_SLASH_URL_RE = re.compile("""
-    (?P<tag>[^:]+):/
+    (?P<tag>[^:]+)://?
     (?P<org>[^/]+)/
     (?P<course>[^/]+)/
     (?P<category>[^/]+)/
@@ -180,13 +171,8 @@ def check(val, regexp):
         if isinstance(location, basestring):
             match = URL_RE.match(location)
             if match is None:
-                # cdodge:
-                # check for a dropped slash near the i4x:// element of the location string. This can happen with some
-                # redirects (e.g. edx.org -> www.edx.org which I think happens in Nginx)
-                match = MISSING_SLASH_URL_RE.match(location)
-                if match is None:
-                    log.debug('location is instance of %s but no URL match' % basestring)
-                    raise InvalidLocationError(location)
+                log.debug('location is instance of %s but no URL match' % basestring)
+                raise InvalidLocationError(location)
             groups = match.groupdict()
             check_dict(groups)
             return _LocationBase.__new__(_cls, **groups)