From 7a238935578c958f5cbba47554466825049ce8aa Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Tue, 26 Mar 2013 16:40:28 -0400 Subject: [PATCH 1/9] wip --- cms/djangoapps/contentstore/utils.py | 6 +- cms/djangoapps/contentstore/views.py | 2 +- cms/templates/widgets/units.html | 2 +- cms/xmodule_namespace.py | 1 - .../xmodule/xmodule/modulestore/__init__.py | 11 +++ .../lib/xmodule/xmodule/modulestore/draft.py | 69 +++++++++++++++---- .../lib/xmodule/xmodule/modulestore/mongo.py | 20 ++---- .../xmodule/modulestore/store_utilities.py | 1 + 8 files changed, 77 insertions(+), 35 deletions(-) diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 63dfe5bf5f..1660b227f6 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -1,3 +1,4 @@ +import logging from django.conf import settings from xmodule.modulestore import Location from xmodule.modulestore.django import modulestore @@ -127,7 +128,7 @@ class UnitState(object): public = 'public' -def compute_unit_state(unit): +def compute_unit_state(unit, subsection=None): """ Returns whether this unit is 'draft', 'public', or 'private'. @@ -137,7 +138,8 @@ def compute_unit_state(unit): 'private' content is editabled and not visible in the LMS """ - if unit.cms.is_draft: + logging.debug('****** is_draft = {0}'.format(getattr(unit, 'is_draft', False))) + if getattr(unit, 'is_draft', False): try: modulestore('direct').get_item(unit.location) return UnitState.draft diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py index 561708c833..edbaed3afa 100644 --- a/cms/djangoapps/contentstore/views.py +++ b/cms/djangoapps/contentstore/views.py @@ -188,7 +188,7 @@ def course_index(request, org, course, name): 'coursename': name }) - course = modulestore().get_item(location) + course = modulestore().get_item(location, depth=3) sections = course.get_children() return render_to_response('overview.html', { diff --git a/cms/templates/widgets/units.html b/cms/templates/widgets/units.html index 5ac05e79eb..c7dbf88341 100644 --- a/cms/templates/widgets/units.html +++ b/cms/templates/widgets/units.html @@ -13,7 +13,7 @@ This def will enumerate through a passed in subsection and list all of the units % for unit in subsection_units:
  • <% - unit_state = compute_unit_state(unit) + unit_state = compute_unit_state(unit, subsection=subsection) if unit.location == selected: selected_class = 'editing' else: diff --git a/cms/xmodule_namespace.py b/cms/xmodule_namespace.py index cad3110574..c9bb8f4c6e 100644 --- a/cms/xmodule_namespace.py +++ b/cms/xmodule_namespace.py @@ -40,7 +40,6 @@ class CmsNamespace(Namespace): """ Namespace with fields common to all blocks in Studio """ - is_draft = Boolean(help="Whether this module is a draft", default=False, scope=Scope.settings) published_date = DateTuple(help="Date when the module was published", scope=Scope.settings) published_by = String(help="Id of the user who published this module", scope=Scope.settings) empty = StringyBoolean(help="Whether this is an empty template", scope=Scope.settings, default=False) diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index 022e016a58..2593b04472 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -10,6 +10,7 @@ from collections import namedtuple from .exceptions import InvalidLocationError, InsufficientSpecificationError from xmodule.errortracker import ErrorLog, make_error_tracker +from bson.son import SON log = logging.getLogger('mitx.' + 'modulestore') @@ -457,3 +458,13 @@ class ModuleStoreBase(ModuleStore): if c.id == course_id: return c return None + + +def namedtuple_to_son(namedtuple, prefix=''): + """ + Converts a namedtuple into a SON object with the same key order + """ + son = SON() + for idx, field_name in enumerate(namedtuple._fields): + son[prefix + field_name] = namedtuple[idx] + return son diff --git a/common/lib/xmodule/xmodule/modulestore/draft.py b/common/lib/xmodule/xmodule/modulestore/draft.py index 71922c08df..0c647159ed 100644 --- a/common/lib/xmodule/xmodule/modulestore/draft.py +++ b/common/lib/xmodule/xmodule/modulestore/draft.py @@ -1,7 +1,8 @@ from datetime import datetime -from . import ModuleStoreBase, Location +from . import ModuleStoreBase, Location, namedtuple_to_son from .exceptions import ItemNotFoundError +import logging DRAFT = 'draft' @@ -15,11 +16,11 @@ def as_draft(location): def wrap_draft(item): """ - Sets `item.cms.is_draft` to `True` if the item is a + Sets `item.is_draft` to `True` if the item is a draft, and `False` otherwise. Sets the item's location to the non-draft location in either case """ - item.cms.is_draft = item.location.revision == DRAFT + setattr(item, 'is_draft', item.location.revision == DRAFT) item.location = item.location._replace(revision=None) return item @@ -55,11 +56,10 @@ class DraftModuleStore(ModuleStoreBase): get_children() to cache. None indicates to cache all descendents """ - # cdodge: we're forcing depth=0 here as the Draft store is not handling caching well try: - return wrap_draft(super(DraftModuleStore, self).get_item(as_draft(location), depth=0)) + return wrap_draft(super(DraftModuleStore, self).get_item(as_draft(location), depth=depth)) except ItemNotFoundError: - return wrap_draft(super(DraftModuleStore, self).get_item(location, depth=0)) + return wrap_draft(super(DraftModuleStore, self).get_item(location, depth=depth)) def get_instance(self, course_id, location, depth=0): """ @@ -67,11 +67,10 @@ class DraftModuleStore(ModuleStoreBase): TODO (vshnayder): this may want to live outside the modulestore eventually """ - # cdodge: we're forcing depth=0 here as the Draft store is not handling caching well try: - return wrap_draft(super(DraftModuleStore, self).get_instance(course_id, as_draft(location), depth=0)) + return wrap_draft(super(DraftModuleStore, self).get_instance(course_id, as_draft(location), depth=depth)) except ItemNotFoundError: - return wrap_draft(super(DraftModuleStore, self).get_instance(course_id, location, depth=0)) + return wrap_draft(super(DraftModuleStore, self).get_instance(course_id, location, depth=depth)) def get_items(self, location, course_id=None, depth=0): """ @@ -88,9 +87,8 @@ class DraftModuleStore(ModuleStoreBase): """ draft_loc = as_draft(location) - # cdodge: we're forcing depth=0 here as the Draft store is not handling caching well - draft_items = super(DraftModuleStore, self).get_items(draft_loc, course_id=course_id, depth=0) - items = super(DraftModuleStore, self).get_items(location, course_id=course_id, depth=0) + draft_items = super(DraftModuleStore, self).get_items(draft_loc, course_id=course_id, depth=depth) + items = super(DraftModuleStore, self).get_items(location, course_id=course_id, depth=depth) draft_locs_found = set(item.location._replace(revision=None) for item in draft_items) non_draft_items = [ @@ -118,7 +116,7 @@ class DraftModuleStore(ModuleStoreBase): """ draft_loc = as_draft(location) draft_item = self.get_item(location) - if not draft_item.cms.is_draft: + if not getattr(draft_item, 'is_draft', False): self.clone_item(location, draft_loc) return super(DraftModuleStore, self).update_item(draft_loc, data) @@ -133,7 +131,7 @@ class DraftModuleStore(ModuleStoreBase): """ draft_loc = as_draft(location) draft_item = self.get_item(location) - if not draft_item.cms.is_draft: + if not getattr(draft_item, 'is_draft', False): self.clone_item(location, draft_loc) return super(DraftModuleStore, self).update_children(draft_loc, children) @@ -149,7 +147,7 @@ class DraftModuleStore(ModuleStoreBase): draft_loc = as_draft(location) draft_item = self.get_item(location) - if not draft_item.cms.is_draft: + if not getattr(draft_item, 'is_draft', False): self.clone_item(location, draft_loc) if 'is_draft' in metadata: @@ -192,3 +190,44 @@ class DraftModuleStore(ModuleStoreBase): """ super(DraftModuleStore, self).clone_item(location, as_draft(location)) super(DraftModuleStore, self).delete_item(location) + + def _cache_children(self, items, depth=0): + """ + Returns a dictionary mapping Location -> item data, populated with json data + for all descendents of items up to the specified depth. + (0 = no descendents, 1 = children, 2 = grandchildren, etc) + If depth is None, will load all the children. + This will make a number of queries that is linear in the depth. + """ + + data = {} + to_process = list(items) + while to_process and depth is None or depth >= 0: + children = [] + for item in to_process: + self._clean_item_data(item) + children.extend(item.get('definition', {}).get('children', [])) + data[Location(item['location'])] = item + + # Load all children by id. See + # http://www.mongodb.org/display/DOCS/Advanced+Queries#AdvancedQueries-%24or + # for or-query syntax + if children: + query = { + '_id': {'$in': [namedtuple_to_son(Location(child)) for child in children]} + } + to_process = list(self.collection.find(query)) + + query = { + '_id': {'$in': [namedtuple_to_son(as_draft(Location(child))) for child in children]} + } + to_process.extend(list(self.collection.find(query))) + logging.debug('**** depth = {0}'.format(depth)) + logging.debug('**** to_process = {0}'.format(to_process)) + else: + to_process = [] + # If depth is None, then we just recurse until we hit all the descendents + if depth is not None: + depth -= 1 + + return data \ No newline at end of file diff --git a/common/lib/xmodule/xmodule/modulestore/mongo.py b/common/lib/xmodule/xmodule/modulestore/mongo.py index b76251bb99..7fa2d9a5c0 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo.py @@ -3,7 +3,6 @@ import sys import logging import copy -from bson.son import SON from collections import namedtuple from fs.osfs import OSFS from itertools import repeat @@ -18,7 +17,7 @@ from xmodule.error_module import ErrorDescriptor from xblock.runtime import DbModel, KeyValueStore, InvalidScopeError from xblock.core import Scope -from . import ModuleStoreBase, Location +from . import ModuleStoreBase, Location, namedtuple_to_son from .draft import DraftModuleStore from .exceptions import (ItemNotFoundError, DuplicateItemError) @@ -196,16 +195,6 @@ def location_to_query(location, wildcard=True): return query -def namedtuple_to_son(namedtuple, prefix=''): - """ - Converts a namedtuple into a SON object with the same key order - """ - son = SON() - for idx, field_name in enumerate(namedtuple._fields): - son[prefix + field_name] = namedtuple[idx] - return son - - class MongoModuleStore(ModuleStoreBase): """ A Mongodb backed ModuleStore @@ -372,13 +361,14 @@ class MongoModuleStore(ModuleStoreBase): # Load all children by id. See # http://www.mongodb.org/display/DOCS/Advanced+Queries#AdvancedQueries-%24or # for or-query syntax - if children: + if children and depth > 0: query = { '_id': {'$in': [namedtuple_to_son(Location(child)) for child in children]} } - to_process = self.collection.find(query) + to_process = list(self.collection.find(query)) else: - to_process = [] + break + # If depth is None, then we just recurse until we hit all the descendents if depth is not None: depth -= 1 diff --git a/common/lib/xmodule/xmodule/modulestore/store_utilities.py b/common/lib/xmodule/xmodule/modulestore/store_utilities.py index cb3cd375a7..2935069090 100644 --- a/common/lib/xmodule/xmodule/modulestore/store_utilities.py +++ b/common/lib/xmodule/xmodule/modulestore/store_utilities.py @@ -136,3 +136,4 @@ def delete_course(modulestore, contentstore, source_location, commit = False): modulestore.delete_item(source_location) return True + From 3cdd973af404dc339400a907a3a61a1e86d40481 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Thu, 28 Mar 2013 09:28:19 -0400 Subject: [PATCH 2/9] get _cache_children to queyr both non-draft and draft versions of the children, then overwrite all non-drafts with the draft version, if available. This conforms with the semantics of the DraftMongoModuleStore --- cms/djangoapps/contentstore/utils.py | 1 - .../lib/xmodule/xmodule/modulestore/draft.py | 35 +++++++++++++++---- 2 files changed, 29 insertions(+), 7 deletions(-) diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 1660b227f6..4a8b1fe269 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -138,7 +138,6 @@ def compute_unit_state(unit, subsection=None): 'private' content is editabled and not visible in the LMS """ - logging.debug('****** is_draft = {0}'.format(getattr(unit, 'is_draft', False))) if getattr(unit, 'is_draft', False): try: modulestore('direct').get_item(unit.location) diff --git a/common/lib/xmodule/xmodule/modulestore/draft.py b/common/lib/xmodule/xmodule/modulestore/draft.py index 0c647159ed..a663889c95 100644 --- a/common/lib/xmodule/xmodule/modulestore/draft.py +++ b/common/lib/xmodule/xmodule/modulestore/draft.py @@ -209,23 +209,46 @@ class DraftModuleStore(ModuleStoreBase): children.extend(item.get('definition', {}).get('children', [])) data[Location(item['location'])] = item + if depth == 0: + break; + # Load all children by id. See # http://www.mongodb.org/display/DOCS/Advanced+Queries#AdvancedQueries-%24or # for or-query syntax + to_process = [] if children: + # first get non-draft in a round-trip query = { '_id': {'$in': [namedtuple_to_son(Location(child)) for child in children]} } - to_process = list(self.collection.find(query)) + to_process_non_drafts = list(self.collection.find(query)) + to_process_dict = {} + for non_draft in to_process_non_drafts: + to_process_dict[Location(non_draft["_id"])] = non_draft + + # now query all draft content in a round-trip query = { '_id': {'$in': [namedtuple_to_son(as_draft(Location(child))) for child in children]} } - to_process.extend(list(self.collection.find(query))) - logging.debug('**** depth = {0}'.format(depth)) - logging.debug('**** to_process = {0}'.format(to_process)) - else: - to_process = [] + to_process_drafts = list(self.collection.find(query)) + + # now we have to go through all drafts and replace the non-draft + # with the draft. This is because the semantics of the DraftStore is to + # always return the draft - if available + for draft in to_process_drafts: + draft_loc = Location(draft["_id"]) + draft_as_non_draft_loc = draft_loc._replace(revision=None) + + # does non-draft exist in the collection + # if so, replace it + if draft_as_non_draft_loc in to_process_dict: + to_process_dict[draft_as_non_draft_loc] = draft + + # convert the dict - which is used for look ups - back into a list + for key, value in to_process_dict.iteritems(): + to_process.append(value) + # If depth is None, then we just recurse until we hit all the descendents if depth is not None: depth -= 1 From c7bafddace1b3e35e48886f1d34dee8d6f1e8ff7 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Thu, 28 Mar 2013 09:49:55 -0400 Subject: [PATCH 3/9] DRY things out a bit and share as much code between MongoModuleStore and DraftMongoModuleStore --- .../lib/xmodule/xmodule/modulestore/draft.py | 83 ++++++------------- .../lib/xmodule/xmodule/modulestore/mongo.py | 19 +++-- 2 files changed, 37 insertions(+), 65 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/draft.py b/common/lib/xmodule/xmodule/modulestore/draft.py index a663889c95..cfce5eb7db 100644 --- a/common/lib/xmodule/xmodule/modulestore/draft.py +++ b/common/lib/xmodule/xmodule/modulestore/draft.py @@ -191,66 +191,35 @@ class DraftModuleStore(ModuleStoreBase): super(DraftModuleStore, self).clone_item(location, as_draft(location)) super(DraftModuleStore, self).delete_item(location) - def _cache_children(self, items, depth=0): - """ - Returns a dictionary mapping Location -> item data, populated with json data - for all descendents of items up to the specified depth. - (0 = no descendents, 1 = children, 2 = grandchildren, etc) - If depth is None, will load all the children. - This will make a number of queries that is linear in the depth. - """ + def _query_children_for_cache_children(self, items): + # first get non-draft in a round-trip + queried_children = [] + to_process_non_drafts = super(DraftModuleStore, self)._query_children_for_cache_children(items) - data = {} - to_process = list(items) - while to_process and depth is None or depth >= 0: - children = [] - for item in to_process: - self._clean_item_data(item) - children.extend(item.get('definition', {}).get('children', [])) - data[Location(item['location'])] = item + to_process_dict = {} + for non_draft in to_process_non_drafts: + to_process_dict[Location(non_draft["_id"])] = non_draft - if depth == 0: - break; + # now query all draft content in another round-trip + query = { + '_id': {'$in': [namedtuple_to_son(as_draft(Location(item))) for item in items]} + } + to_process_drafts = list(self.collection.find(query)) - # Load all children by id. See - # http://www.mongodb.org/display/DOCS/Advanced+Queries#AdvancedQueries-%24or - # for or-query syntax - to_process = [] - if children: - # first get non-draft in a round-trip - query = { - '_id': {'$in': [namedtuple_to_son(Location(child)) for child in children]} - } - to_process_non_drafts = list(self.collection.find(query)) + # now we have to go through all drafts and replace the non-draft + # with the draft. This is because the semantics of the DraftStore is to + # always return the draft - if available + for draft in to_process_drafts: + draft_loc = Location(draft["_id"]) + draft_as_non_draft_loc = draft_loc._replace(revision=None) - to_process_dict = {} - for non_draft in to_process_non_drafts: - to_process_dict[Location(non_draft["_id"])] = non_draft + # does non-draft exist in the collection + # if so, replace it + if draft_as_non_draft_loc in to_process_dict: + to_process_dict[draft_as_non_draft_loc] = draft - # now query all draft content in a round-trip - query = { - '_id': {'$in': [namedtuple_to_son(as_draft(Location(child))) for child in children]} - } - to_process_drafts = list(self.collection.find(query)) + # convert the dict - which is used for look ups - back into a list + for key, value in to_process_dict.iteritems(): + queried_children.append(value) - # now we have to go through all drafts and replace the non-draft - # with the draft. This is because the semantics of the DraftStore is to - # always return the draft - if available - for draft in to_process_drafts: - draft_loc = Location(draft["_id"]) - draft_as_non_draft_loc = draft_loc._replace(revision=None) - - # does non-draft exist in the collection - # if so, replace it - if draft_as_non_draft_loc in to_process_dict: - to_process_dict[draft_as_non_draft_loc] = draft - - # convert the dict - which is used for look ups - back into a list - for key, value in to_process_dict.iteritems(): - to_process.append(value) - - # If depth is None, then we just recurse until we hit all the descendents - if depth is not None: - depth -= 1 - - return data \ No newline at end of file + return queried_children diff --git a/common/lib/xmodule/xmodule/modulestore/mongo.py b/common/lib/xmodule/xmodule/modulestore/mongo.py index 8f8f4577cc..36b97e5f64 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo.py @@ -363,6 +363,13 @@ class MongoModuleStore(ModuleStoreBase): item['location'] = item['_id'] del item['_id'] + def _query_children_for_cache_children(self, items): + # first get non-draft in a round-trip + query = { + '_id': {'$in': [namedtuple_to_son(Location(item)) for item in items]} + } + return list(self.collection.find(query)) + def _cache_children(self, items, depth=0): """ Returns a dictionary mapping Location -> item data, populated with json data @@ -382,18 +389,14 @@ class MongoModuleStore(ModuleStoreBase): data[Location(item['location'])] = item if depth == 0: - break + break; # Load all children by id. See # http://www.mongodb.org/display/DOCS/Advanced+Queries#AdvancedQueries-%24or # for or-query syntax - if children and depth > 0: - query = { - '_id': {'$in': [namedtuple_to_son(Location(child)) for child in children]} - } - to_process = list(self.collection.find(query)) - else: - break + to_process = [] + if children: + to_process = self._query_children_for_cache_children(children) # If depth is None, then we just recurse until we hit all the descendents if depth is not None: From 197f52539f791ec4b70e707864792ee5f0cc7eaa Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Thu, 28 Mar 2013 11:37:53 -0400 Subject: [PATCH 4/9] add some unit tests --- .../contentstore/tests/test_contentstore.py | 38 +++++++++++++++++++ cms/envs/test.py | 4 ++ 2 files changed, 42 insertions(+) diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index ce5bf36559..7448e2e435 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -85,6 +85,44 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): def test_edit_unit_full(self): self.check_edit_unit('full') + def _get_draft_counts(self, item): + cnt = 1 if getattr(item, 'is_draft', False) else 0 + print "Checking {0}. Result = {1}".format(item.location, cnt) + for child in item.get_children(): + cnt = cnt + self._get_draft_counts(child) + + return cnt + + def test_get_depth_with_drafts(self): + import_from_xml(modulestore(), 'common/test/data/', ['simple']) + + course = modulestore('draft').get_item(Location(['i4x', 'edX', 'simple', + 'course', '2012_Fall', None]), depth=None) + + # make sure no draft items have been returned + num_drafts = self._get_draft_counts(course) + self.assertEqual(num_drafts, 0) + + problem = modulestore('draft').get_item(Location(['i4x', 'edX', 'simple', + 'problem', 'ps01-simple', None])) + + # put into draft + modulestore('draft').clone_item(problem.location, problem.location) + + # make sure we can query that item and verify that it is a draft + draft_problem = modulestore('draft').get_item(Location(['i4x', 'edX', 'simple', + 'problem', 'ps01-simple', None])) + self.assertTrue(getattr(draft_problem,'is_draft', False)) + + #now requery with depth + course = modulestore('draft').get_item(Location(['i4x', 'edX', 'simple', + 'course', '2012_Fall', None]), depth=None) + + # make sure no draft items have been returned + num_drafts = self._get_draft_counts(course) + self.assertEqual(num_drafts, 1) + + def test_static_tab_reordering(self): import_from_xml(modulestore(), 'common/test/data/', ['full']) diff --git a/cms/envs/test.py b/cms/envs/test.py index d7992cb471..59664bfd40 100644 --- a/cms/envs/test.py +++ b/cms/envs/test.py @@ -58,6 +58,10 @@ MODULESTORE = { 'direct': { 'ENGINE': 'xmodule.modulestore.mongo.MongoModuleStore', 'OPTIONS': modulestore_options + }, + 'draft': { + 'ENGINE': 'xmodule.modulestore.mongo.DraftMongoModuleStore', + 'OPTIONS': modulestore_options } } From 1c2a8a97cdf7e1ff35c882873235c5542d962807 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Thu, 28 Mar 2013 11:38:22 -0400 Subject: [PATCH 5/9] remove unnecessary debug log message --- cms/djangoapps/contentstore/tests/test_contentstore.py | 1 - 1 file changed, 1 deletion(-) diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 7448e2e435..7a5c3364bd 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -87,7 +87,6 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): def _get_draft_counts(self, item): cnt = 1 if getattr(item, 'is_draft', False) else 0 - print "Checking {0}. Result = {1}".format(item.location, cnt) for child in item.get_children(): cnt = cnt + self._get_draft_counts(child) From ee5076bda99aa021af983819f4c9342be41d803b Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Thu, 28 Mar 2013 14:48:12 -0400 Subject: [PATCH 6/9] fix incorrect comment --- cms/djangoapps/contentstore/tests/test_contentstore.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 7a5c3364bd..355b840fdf 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -117,7 +117,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): course = modulestore('draft').get_item(Location(['i4x', 'edX', 'simple', 'course', '2012_Fall', None]), depth=None) - # make sure no draft items have been returned + # make sure just one draft item have been returned num_drafts = self._get_draft_counts(course) self.assertEqual(num_drafts, 1) From bbb53a17f8186df5ef2a8c6e7ed559d3147354f8 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 29 Mar 2013 09:58:03 -0400 Subject: [PATCH 7/9] add some depth optimziations for edit subsection and unit pages as well --- cms/djangoapps/contentstore/views.py | 29 +++++++++------------------- cms/envs/dev.py | 2 +- 2 files changed, 10 insertions(+), 21 deletions(-) diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py index edbaed3afa..945216d1db 100644 --- a/cms/djangoapps/contentstore/views.py +++ b/cms/djangoapps/contentstore/views.py @@ -208,19 +208,14 @@ def course_index(request, org, course, name): @login_required def edit_subsection(request, location): # check that we have permissions to edit this item - if not has_access(request.user, location): + course = get_course_for_item(location) + if not has_access(request.user, course.location): raise PermissionDenied() - item = modulestore().get_item(location) + item = modulestore().get_item(location, depth=1) - # TODO: we need a smarter way to figure out what course an item is in - for course in modulestore().get_courses(): - if (course.location.org == item.location.org and - course.location.course == item.location.course): - break - - lms_link = get_lms_link_for_item(location) - preview_link = get_lms_link_for_item(location, preview=True) + lms_link = get_lms_link_for_item(location, course_id=course.location.course_id) + preview_link = get_lms_link_for_item(location, course_id=course.location.course_id, preview=True) # make sure that location references a 'sequential', otherwise return BadRequest if item.location.category != 'sequential': @@ -277,19 +272,13 @@ def edit_unit(request, location): id: A Location URL """ - # check that we have permissions to edit this item - if not has_access(request.user, location): + course = get_course_for_item(location) + if not has_access(request.user, course.location): raise PermissionDenied() - item = modulestore().get_item(location) + item = modulestore().get_item(location, depth=1) - # TODO: we need a smarter way to figure out what course an item is in - for course in modulestore().get_courses(): - if (course.location.org == item.location.org and - course.location.course == item.location.course): - break - - lms_link = get_lms_link_for_item(item.location) + lms_link = get_lms_link_for_item(item.location, course_id=course.location.course_id) component_templates = defaultdict(list) diff --git a/cms/envs/dev.py b/cms/envs/dev.py index 5612db1396..b8d4d14b9e 100644 --- a/cms/envs/dev.py +++ b/cms/envs/dev.py @@ -142,4 +142,4 @@ DEBUG_TOOLBAR_CONFIG = { # To see stacktraces for MongoDB queries, set this to True. # Stacktraces slow down page loads drastically (for pages with lots of queries). -DEBUG_TOOLBAR_MONGO_STACKTRACES = False +DEBUG_TOOLBAR_MONGO_STACKTRACES = True From 60e295895eef2515beb5bbc450838d368ea5375d Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 29 Mar 2013 15:26:21 -0400 Subject: [PATCH 8/9] remove unused parameter --- cms/djangoapps/contentstore/utils.py | 2 +- cms/templates/widgets/units.html | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 4a8b1fe269..bd820fd489 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -128,7 +128,7 @@ class UnitState(object): public = 'public' -def compute_unit_state(unit, subsection=None): +def compute_unit_state(unit): """ Returns whether this unit is 'draft', 'public', or 'private'. diff --git a/cms/templates/widgets/units.html b/cms/templates/widgets/units.html index c7dbf88341..5ac05e79eb 100644 --- a/cms/templates/widgets/units.html +++ b/cms/templates/widgets/units.html @@ -13,7 +13,7 @@ This def will enumerate through a passed in subsection and list all of the units % for unit in subsection_units:
  • <% - unit_state = compute_unit_state(unit, subsection=subsection) + unit_state = compute_unit_state(unit) if unit.location == selected: selected_class = 'editing' else: From 599ca4d429c80409adc311667ac242873ffe647e Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 29 Mar 2013 15:31:37 -0400 Subject: [PATCH 9/9] oops. I'm not programming in C# any longer --- common/lib/xmodule/xmodule/modulestore/mongo.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/lib/xmodule/xmodule/modulestore/mongo.py b/common/lib/xmodule/xmodule/modulestore/mongo.py index 36b97e5f64..da8e0f5040 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo.py @@ -389,7 +389,7 @@ class MongoModuleStore(ModuleStoreBase): data[Location(item['location'])] = item if depth == 0: - break; + break # Load all children by id. See # http://www.mongodb.org/display/DOCS/Advanced+Queries#AdvancedQueries-%24or