Merge pull request #3888 from edx/dhm/bug_get_children

Redirect most studio access through draft modulestore

cms/djangoapps/contentstore/features/video.py

@@ -154,7 +154,8 @@ def xml_only_video(step):
     world.ItemFactory.create(
         parent_location=parent_location,
         category='video',
-        data='<video youtube="1.00:%s"></video>' % youtube_id
+        data='<video youtube="1.00:%s"></video>' % youtube_id,
+        modulestore=store,
     )
 
 

cms/djangoapps/contentstore/tests/test_contentstore.py

@@ -741,22 +741,21 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
 
     def test_illegal_draft_crud_ops(self):
         draft_store = modulestore('draft')
-        direct_store = modulestore('direct')
 
         course = CourseFactory.create(org='MITx', course='999', display_name='Robot Super Course')
 
         location = course.id.make_usage_key('chapter', 'neuvo')
-        # Ensure draft mongo store does not allow us to create chapters either directly or via convert to draft
-        with self.assertRaises(InvalidVersionError):
-            draft_store.create_and_save_xmodule(location)
-        direct_store.create_and_save_xmodule(location)
+        # Ensure draft mongo store does not create drafts for things that shouldn't be draft
+        newobject = draft_store.create_and_save_xmodule(location)
+        self.assertFalse(getattr(newobject, 'is_draft', False))
         with self.assertRaises(InvalidVersionError):
             draft_store.convert_to_draft(location)
         chapter = draft_store.get_item(location)
         chapter.data = 'chapter data'
 
-        with self.assertRaises(InvalidVersionError):
-            draft_store.update_item(chapter, self.user.id)
+        draft_store.update_item(chapter, self.user.id)
+        newobject = draft_store.get_item(chapter.location)
+        self.assertFalse(getattr(newobject, 'is_draft', False))
 
         with self.assertRaises(InvalidVersionError):
             draft_store.unpublish(location)

cms/djangoapps/contentstore/utils.py

@@ -14,7 +14,6 @@ from xmodule.modulestore.django import modulestore
 from xmodule.modulestore.exceptions import ItemNotFoundError
 from xmodule.modulestore.locations import SlashSeparatedCourseKey, Location
 from xmodule.modulestore.store_utilities import delete_course
-from xmodule.modulestore.draft import DIRECT_ONLY_CATEGORIES
 from student.roles import CourseInstructorRole, CourseStaffRole
 
 
@@ -52,15 +51,10 @@ def delete_course_and_groups(course_id, commit=False):
 
 def get_modulestore(category_or_location):
     """
-    Returns the correct modulestore to use for modifying the specified location
+    This function no longer does anything more than just calling `modulestore()`. It used
+    to select 'direct' v 'draft' based on the category.
     """
-    if isinstance(category_or_location, Location):
-        category_or_location = category_or_location.category
-
-    if category_or_location in DIRECT_ONLY_CATEGORIES:
-        return modulestore('direct')
-    else:
-        return modulestore()
+    return modulestore()
 
 
 def get_lms_link_for_item(location, preview=False):

cms/djangoapps/contentstore/views/component.py

@@ -2,7 +2,6 @@ from __future__ import absolute_import
 
 import json
 import logging
-from collections import defaultdict
 
 from django.http import HttpResponseBadRequest, Http404
 from django.contrib.auth.decorators import login_required

cms/djangoapps/contentstore/views/course.py

@@ -53,7 +53,6 @@ from django_comment_common.utils import seed_permissions_roles
 from student.models import CourseEnrollment
 from student.roles import CourseRole, UserBasedRole
 
-from xmodule.html_module import AboutDescriptor
 from xmodule.modulestore.keys import CourseKey
 from course_creators.views import get_course_creator_status, add_user_with_status_unrequested
 from contentstore import utils

cms/djangoapps/contentstore/views/item.py

@@ -381,8 +381,7 @@ def _save_item(request, usage_key, data=None, children=None, metadata=None, null
             # interface for publishing. However, as of now, only the DraftMongoModulestore
             # does, so we have to check for the attribute explicitly.
             store = get_modulestore(block.location)
-            if hasattr(store, 'publish'):
-                store.publish(block.location, request.user.id)
+            store.publish(block.location, request.user.id)
 
         _xmodule_recurse(
             existing_item,

common/lib/xmodule/xmodule/modulestore/mongo/draft.py

@@ -75,25 +75,27 @@ class DraftModuleStore(MongoModuleStore):
             in the request. The depth is counted in the number of calls to
             get_children() to cache. None indicates to cache all descendents
         """
-
-        try:
-            return wrap_draft(super(DraftModuleStore, self).get_item(as_draft(usage_key), depth=depth))
-        except ItemNotFoundError:
-            return wrap_draft(super(DraftModuleStore, self).get_item(usage_key, depth=depth))
+        if usage_key.category not in DIRECT_ONLY_CATEGORIES:
+            try:
+                return wrap_draft(super(DraftModuleStore, self).get_item(as_draft(usage_key), depth=depth))
+            except ItemNotFoundError:
+                return wrap_draft(super(DraftModuleStore, self).get_item(usage_key, depth=depth))
+        else:
+            return super(DraftModuleStore, self).get_item(usage_key, depth=depth)
 
     def create_xmodule(self, location, definition_data=None, metadata=None, system=None, fields={}):
         """
-        Create the new xmodule but don't save it. Returns the new module with a draft locator
+        Create the new xmodule but don't save it. Returns the new module with a draft locator if
+        the category allows drafts. If the category does not allow drafts, just creates a published module.
 
         :param location: a Location--must have a category
         :param definition_data: can be empty. The initial definition_data for the kvs
         :param metadata: can be empty, the initial metadata for the kvs
         :param system: if you already have an xmodule from the course, the xmodule.system value
         """
-        if location.category in DIRECT_ONLY_CATEGORIES:
-            raise InvalidVersionError(location)
-        draft_loc = as_draft(location)
-        return super(DraftModuleStore, self).create_xmodule(draft_loc, definition_data, metadata, system, fields)
+        if location.category not in DIRECT_ONLY_CATEGORIES:
+            location = as_draft(location)
+        return super(DraftModuleStore, self).create_xmodule(location, definition_data, metadata, system, fields)
 
     def get_items(self, course_key, settings=None, content=None, revision=None, **kwargs):
         """
@@ -139,12 +141,12 @@ class DraftModuleStore(MongoModuleStore):
 
         :param source: the location of the source (its revision must be None)
         """
-        original = self.collection.find_one({'_id': source_location.to_deprecated_son()})
-        draft_location = as_draft(source_location)
-        if draft_location.category in DIRECT_ONLY_CATEGORIES:
+        if source_location.category in DIRECT_ONLY_CATEGORIES:
             raise InvalidVersionError(source_location)
+        original = self.collection.find_one({'_id': source_location.to_deprecated_son()})
         if not original:
             raise ItemNotFoundError(source_location)
+        draft_location = as_draft(source_location)
         original['_id'] = draft_location.to_deprecated_son()
         try:
             self.collection.insert(original)
@@ -161,6 +163,9 @@ class DraftModuleStore(MongoModuleStore):
         In addition to the superclass's behavior, this method converts the unit to draft if it's not
         already draft.
         """
+        if xblock.location.category in DIRECT_ONLY_CATEGORIES:
+            return super(DraftModuleStore, self).update_item(xblock, user_id, allow_not_found)
+
         draft_loc = as_draft(xblock.location)
         try:
             if not self.has_item(draft_loc):
@@ -180,6 +185,9 @@ class DraftModuleStore(MongoModuleStore):
 
         location: Something that can be passed to Location
         """
+        if location.category in DIRECT_ONLY_CATEGORIES:
+            return super(DraftModuleStore, self).delete_item(as_published(location))
+
         super(DraftModuleStore, self).delete_item(as_draft(location))
         if delete_all_versions:
             super(DraftModuleStore, self).delete_item(as_published(location))
@@ -190,6 +198,10 @@ class DraftModuleStore(MongoModuleStore):
         """
         Save a current draft to the underlying modulestore
         """
+        if location.category in DIRECT_ONLY_CATEGORIES:
+            # ignore noop attempt to publish something that can't be draft.
+            # ignoring v raising exception b/c bok choy tests always pass make_public which calls publish
+            return
         try:
             original_published = super(DraftModuleStore, self).get_item(location)
         except ItemNotFoundError: