diff --git a/AUTHORS b/AUTHORS index f7d8542c31..933a253863 100644 --- a/AUTHORS +++ b/AUTHORS @@ -266,3 +266,4 @@ Kaloian Doganov Sanford Student Florian Haas Leonardo Quiñonez +Dmitry Viskov diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index fa5ba3380c..d697b9e959 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -9,7 +9,8 @@ from django.http import Http404, HttpResponseBadRequest from django.contrib.auth.decorators import login_required from edxmako.shortcuts import render_to_string -from openedx.core.lib.xblock_utils import replace_static_urls, wrap_xblock, wrap_fragment, request_token +from openedx.core.lib.xblock_utils import replace_static_urls, wrap_xblock, wrap_fragment, wrap_xblock_aside,\ + request_token from xmodule.x_module import PREVIEW_VIEWS, STUDENT_VIEW, AUTHOR_VIEW from xmodule.contentstore.django import contentstore from xmodule.error_module import ErrorDescriptor @@ -19,6 +20,7 @@ from xmodule.services import SettingsService from xmodule.modulestore.django import modulestore, ModuleI18nService from xmodule.mixin import wrap_with_license from opaque_keys.edx.keys import UsageKey +from opaque_keys.edx.asides import AsideUsageKeyV1 from xmodule.x_module import ModuleSystem from xblock.runtime import KvsFieldData from xblock.django.request import webob_to_django_response, django_to_webob_request @@ -56,8 +58,18 @@ def preview_handler(request, usage_key_string, handler, suffix=''): """ usage_key = UsageKey.from_string(usage_key_string) - descriptor = modulestore().get_item(usage_key) - instance = _load_preview_module(request, descriptor) + if isinstance(usage_key, AsideUsageKeyV1): + descriptor = modulestore().get_item(usage_key.usage_key) + for aside in descriptor.runtime.get_asides(descriptor): + if aside.scope_ids.block_type == usage_key.aside_type: + asides = [aside] + instance = aside + break + else: + descriptor = modulestore().get_item(usage_key) + instance = _load_preview_module(request, descriptor) + asides = [] + # Let the module handle the AJAX req = django_to_webob_request(request) try: @@ -80,6 +92,7 @@ def preview_handler(request, usage_key_string, handler, suffix=''): log.exception("error processing ajax call") raise + modulestore().update_item(descriptor, request.user.id, asides=asides) return webob_to_django_response(resp) @@ -184,6 +197,15 @@ def _preview_module_system(request, descriptor, field_data): _studio_wrap_xblock, ] + wrappers_asides = [ + partial( + wrap_xblock_aside, + 'PreviewRuntime', + usage_id_serializer=unicode, + request_token=request_token(request) + ) + ] + if settings.FEATURES.get("LICENSING", False): # stick the license wrapper in front wrappers.insert(0, wrap_with_license) @@ -208,6 +230,7 @@ def _preview_module_system(request, descriptor, field_data): # Set up functions to modify the fragment produced by student_view wrappers=wrappers, + wrappers_asides=wrappers_asides, error_descriptor_class=ErrorDescriptor, get_user_role=lambda: get_user_role(request.user, course_id), # Get the raw DescriptorSystem, not the CombinedSystem diff --git a/cms/lib/xblock/tagging.py b/cms/lib/xblock/tagging.py index 6a4ec348ec..7a7710e187 100644 --- a/cms/lib/xblock/tagging.py +++ b/cms/lib/xblock/tagging.py @@ -1,14 +1,17 @@ """ -Example implementation of Structured Tagging based on XBlockAsides +Structured Tagging based on XBlockAsides """ -from xblock.core import XBlockAside +from xblock.core import XBlockAside, XBlock from xblock.fragment import Fragment from xblock.fields import Scope, Dict from xmodule.x_module import STUDENT_VIEW from xmodule.capa_module import CapaModule from abc import ABCMeta, abstractproperty from edxmako.shortcuts import render_to_string +from django.conf import settings +from webob import Response +from collections import OrderedDict _ = lambda text: text @@ -42,24 +45,24 @@ class AbstractTag(object): raise NotImplementedError('Subclasses must implement allowed_values') -class LearningOutcomeTag(AbstractTag): +class DifficultyTag(AbstractTag): """ - Particular implementation tags for learning outcomes + Particular implementation tags for difficulty """ @property def key(self): - """ Identifier for the learning outcome selector """ - return 'learning_outcome_tag' + """ Identifier for the difficulty selector """ + return 'difficulty_tag' @property def name(self): - """ Label for the learning outcome selector """ - return _('Learning outcomes') + """ Label for the difficulty selector """ + return _('Difficulty') @property def allowed_values(self): - """ Allowed values for the learning outcome selector """ - return {'test1': 'Test 1', 'test2': 'Test 2', 'test3': 'Test 3'} + """ Allowed values for the difficulty selector """ + return OrderedDict([('easy', 'Easy'), ('medium', 'Medium'), ('hard', 'Hard')]) class StructuredTagsAside(XBlockAside): @@ -69,10 +72,16 @@ class StructuredTagsAside(XBlockAside): saved_tags = Dict(help=_("Dictionary with the available tags"), scope=Scope.content, default={},) - available_tags = [LearningOutcomeTag()] + available_tags = [DifficultyTag()] + + def _get_studio_resource_url(self, relative_url): + """ + Returns the Studio URL to a static resource. + """ + return settings.STATIC_URL + relative_url @XBlockAside.aside_for(STUDENT_VIEW) - def student_view_aside(self, block, context): + def student_view_aside(self, block, context): # pylint: disable=unused-argument """ Display the tag selector with specific categories and allowed values, depending on the context. @@ -86,7 +95,34 @@ class StructuredTagsAside(XBlockAside): 'values': tag.allowed_values, 'current_value': self.saved_tags.get(tag.key, None), }) - return Fragment(render_to_string('structured_tags_block.html', {'tags': tags})) - #return Fragment(u'
Hello world!!!
') + fragment = Fragment(render_to_string('structured_tags_block.html', {'tags': tags})) + fragment.add_javascript_url(self._get_studio_resource_url('/js/xblock_asides/structured_tags.js')) + fragment.initialize_js('StructuredTagsInit') + return fragment else: return Fragment(u'') + + @XBlock.handler + def save_tags(self, request=None, suffix=None): # pylint: disable=unused-argument + """ + Handler to save choosen tags with connected XBlock + """ + found = False + if 'tag' not in request.params: + return Response("The required parameter 'tag' is not passed", status=400) + + tag = request.params['tag'].split(':') + + for av_tag in self.available_tags: + if av_tag.key == tag[0]: + if tag[1] in av_tag.allowed_values: + self.saved_tags[tag[0]] = tag[1] + found = True + elif tag[1] == '': + self.saved_tags[tag[0]] = None + found = True + + if not found: + return Response("Invalid 'tag' parameter", status=400) + + return Response() diff --git a/cms/lib/xblock/test/test_tagging.py b/cms/lib/xblock/test/test_tagging.py new file mode 100644 index 0000000000..49907e1298 --- /dev/null +++ b/cms/lib/xblock/test/test_tagging.py @@ -0,0 +1,166 @@ +""" +Tests for the Studio Tagging XBlockAside +""" + +from xmodule.modulestore import ModuleStoreEnum +from xmodule.modulestore.django import modulestore +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory +from xblock_config.models import StudioConfig +from cms.lib.xblock.tagging import StructuredTagsAside +from contentstore.views.preview import get_preview_fragment +from contentstore.utils import reverse_usage_url +from contentstore.tests.utils import AjaxEnabledTestClient +from django.test.client import RequestFactory +from student.tests.factories import UserFactory +from opaque_keys.edx.asides import AsideUsageKeyV1 +from datetime import datetime +from pytz import UTC +from lxml import etree +from StringIO import StringIO + + +class StructuredTagsAsideTestCase(ModuleStoreTestCase): + """ + Base class for tests of StructuredTagsAside (tagging.py) + """ + def setUp(self): + """ + Preparation for the test execution + """ + self.user_password = super(StructuredTagsAsideTestCase, self).setUp() + self.aside_name = 'tagging_aside' + self.aside_tag = 'difficulty_tag' + self.aside_tag_value = 'hard' + + course = CourseFactory.create(default_store=ModuleStoreEnum.Type.split) + self.course = ItemFactory.create( + parent_location=course.location, + category="course", + display_name="Test course", + ) + self.chapter = ItemFactory.create( + parent_location=self.course.location, + category='chapter', + display_name="Week 1", + publish_item=True, + start=datetime(2015, 3, 1, tzinfo=UTC), + ) + self.sequential = ItemFactory.create( + parent_location=self.chapter.location, + category='sequential', + display_name="Lesson 1", + publish_item=True, + start=datetime(2015, 3, 1, tzinfo=UTC), + ) + self.vertical = ItemFactory.create( + parent_location=self.sequential.location, + category='vertical', + display_name='Subsection 1', + publish_item=True, + start=datetime(2015, 4, 1, tzinfo=UTC), + ) + self.problem = ItemFactory.create( + category="problem", + parent_location=self.vertical.location, + display_name="A Problem Block", + weight=1, + user_id=self.user.id, + publish_item=False, + ) + self.video = ItemFactory.create( + parent_location=self.vertical.location, + category='video', + display_name='My Video', + user_id=self.user.id + ) + + config = StudioConfig.current() + config.enabled = True + config.save() + + def test_aside_contains_tags(self): + """ + Checks that available_tags list is not empty + """ + self.assertGreater(len(StructuredTagsAside.available_tags), 0, + "StructuredTagsAside should contains at least one available tag") + + def test_preview_html(self): + """ + Checks that html for the StructuredTagsAside is generated correctly + """ + request = RequestFactory().get('/dummy-url') + request.user = UserFactory() + request.session = {} + + # Call get_preview_fragment directly. + context = { + 'reorderable_items': set(), + 'read_only': True + } + problem_html = get_preview_fragment(request, self.problem, context).content + + parser = etree.HTMLParser() + tree = etree.parse(StringIO(problem_html), parser) + + main_div_nodes = tree.xpath('/html/body/div/section/div') + self.assertEquals(len(main_div_nodes), 1) + + div_node = main_div_nodes[0] + self.assertEquals(div_node.get('data-init'), 'StructuredTagsInit') + self.assertEquals(div_node.get('data-runtime-class'), 'PreviewRuntime') + self.assertEquals(div_node.get('data-block-type'), 'tagging_aside') + self.assertEquals(div_node.get('data-runtime-version'), '1') + self.assertIn('xblock_asides-v1', div_node.get('class')) + + select_nodes = div_node.xpath('div/select') + self.assertEquals(len(select_nodes), 1) + + select_node = select_nodes[0] + self.assertEquals(select_node.get('name'), self.aside_tag) + + # Now ensure the acid_aside is not in the result + self.assertNotRegexpMatches(problem_html, r"data-block-type=[\"\']acid_aside[\"\']") + + # Ensure about video don't have asides + video_html = get_preview_fragment(request, self.video, context).content + self.assertNotRegexpMatches(video_html, " % for tag in tags: : - + % for k,v in tag['values'].iteritems(): <% selected = '' diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index 3ddd26181e..278a32fc4a 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -428,7 +428,8 @@ class BlockData(object): 'block_type': self.block_type, 'definition': self.definition, 'defaults': self.defaults, - 'edit_info': self.edit_info.to_storable() + 'asides': self.get_asides(), + 'edit_info': self.edit_info.to_storable(), } def from_storable(self, block_data): @@ -449,9 +450,21 @@ class BlockData(object): # blocks are copied from a library to a course) self.defaults = block_data.get('defaults', {}) + # Additional field data that stored in connected XBlockAsides + self.asides = block_data.get('asides', {}) + # EditInfo object containing all versioning/editing data. self.edit_info = EditInfo(**block_data.get('edit_info', {})) + def get_asides(self): + """ + For the situations if block_data has no asides attribute + (in case it was taken from memcache) + """ + if not hasattr(self, 'asides'): + self.asides = {} # pylint: disable=attribute-defined-outside-init + return self.asides + def __repr__(self): # pylint: disable=bad-continuation, redundant-keyword-arg return ("{classname}(fields={self.fields}, " @@ -459,17 +472,19 @@ class BlockData(object): "definition={self.definition}, " "definition_loaded={self.definition_loaded}, " "defaults={self.defaults}, " + "asides={asides}, " "edit_info={self.edit_info})").format( self=self, classname=self.__class__.__name__, + asides=self.get_asides() ) # pylint: disable=bad-continuation def __eq__(self, block_data): """ Two BlockData objects are equal iff all their attributes are equal. """ - attrs = ['fields', 'block_type', 'definition', 'defaults', 'edit_info'] - return all(getattr(self, attr) == getattr(block_data, attr) for attr in attrs) + attrs = ['fields', 'block_type', 'definition', 'defaults', 'asides', 'edit_info'] + return all(getattr(self, attr, None) == getattr(block_data, attr, None) for attr in attrs) def __neq__(self, block_data): """ diff --git a/common/lib/xmodule/xmodule/modulestore/mixed.py b/common/lib/xmodule/xmodule/modulestore/mixed.py index 3beb3970d5..e104e1b0f2 100644 --- a/common/lib/xmodule/xmodule/modulestore/mixed.py +++ b/common/lib/xmodule/xmodule/modulestore/mixed.py @@ -95,6 +95,40 @@ def strip_key(func): return inner +def prepare_asides(func): + """ + A decorator to handle optional asides param + """ + @functools.wraps(func) + def wrapper(*args, **kwargs): + """ + Supported kwargs: + asides - list with connected asides data for the passed block + """ + if 'asides' in kwargs: + kwargs['asides'] = prepare_asides_to_store(kwargs['asides']) + return func(*args, **kwargs) + return wrapper + + +def prepare_asides_to_store(asides_source): + """ + Convert Asides Xblocks objects to the list of dicts (to store this information in MongoDB) + """ + asides = None + if asides_source: + asides = [] + for asd in asides_source: + aside_fields = {} + for asd_field_key, asd_field_val in asd.fields.iteritems(): + aside_fields[asd_field_key] = asd_field_val.read_from(asd) + asides.append({ + 'aside_type': asd.scope_ids.block_type, + 'fields': aside_fields + }) + return asides + + class MixedModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase): """ ModuleStore knows how to route requests to the right persistence ms @@ -687,6 +721,7 @@ class MixedModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase): )) @strip_key + @prepare_asides def create_item(self, user_id, course_key, block_type, block_id=None, fields=None, **kwargs): """ Creates and saves a new item in a course. @@ -707,6 +742,7 @@ class MixedModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase): return modulestore.create_item(user_id, course_key, block_type, block_id=block_id, fields=fields, **kwargs) @strip_key + @prepare_asides def create_child(self, user_id, parent_usage_key, block_type, block_id=None, fields=None, **kwargs): """ Creates and saves a new xblock that is a child of the specified block @@ -727,6 +763,7 @@ class MixedModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase): return modulestore.create_child(user_id, parent_usage_key, block_type, block_id=block_id, fields=fields, **kwargs) @strip_key + @prepare_asides def import_xblock(self, user_id, course_key, block_type, block_id, fields=None, runtime=None, **kwargs): """ See :py:meth `ModuleStoreDraftAndPublished.import_xblock` @@ -734,7 +771,7 @@ class MixedModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase): Defer to the course's modulestore if it supports this method """ store = self._verify_modulestore_support(course_key, 'import_xblock') - return store.import_xblock(user_id, course_key, block_type, block_id, fields, runtime) + return store.import_xblock(user_id, course_key, block_type, block_id, fields, runtime, **kwargs) @strip_key def copy_from_template(self, source_keys, dest_key, user_id, **kwargs): @@ -745,6 +782,7 @@ class MixedModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase): return store.copy_from_template(source_keys, dest_key, user_id) @strip_key + @prepare_asides def update_item(self, xblock, user_id, allow_not_found=False, **kwargs): """ Update the xblock persisted to be the same as the given for all types of fields diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py index 995ec6fe2c..d7802e9462 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py @@ -3,7 +3,7 @@ import logging from contracts import contract, new_contract from fs.osfs import OSFS from lazy import lazy -from xblock.runtime import KvsFieldData +from xblock.runtime import KvsFieldData, KeyValueStore from xblock.fields import ScopeIds from xblock.core import XBlock from opaque_keys.edx.locator import BlockUsageLocator, LocalId, CourseLocator, LibraryLocator, DefinitionLocator @@ -19,6 +19,7 @@ from xmodule.modulestore.split_mongo import BlockKey, CourseEnvelope from xmodule.modulestore.split_mongo.id_manager import SplitMongoIdManager from xmodule.modulestore.split_mongo.definition_lazy_loader import DefinitionLazyLoader from xmodule.modulestore.split_mongo.split_mongo_kvs import SplitMongoKVS +from xmodule.x_module import XModuleMixin log = logging.getLogger(__name__) @@ -209,12 +210,26 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin): parent = course_key.make_usage_key(parent_key.type, parent_key.id) else: parent = None + + aside_fields = None + + # for the situation if block_data has no asides attribute + # (in case it was taken from memcache) + try: + if block_data.asides: + aside_fields = {block_key.type: {}} + for aside in block_data.asides: + aside_fields[block_key.type].update(aside['fields']) + except AttributeError: + pass + try: kvs = SplitMongoKVS( definition_loader, converted_fields, converted_defaults, parent=parent, + aside_fields=aside_fields, field_decorator=kwargs.get('field_decorator') ) @@ -338,3 +353,30 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin): block_data.edit_info._subtree_edited_on = max_date block_data.edit_info._subtree_edited_by = max_date_by + + def get_aside_of_type(self, block, aside_type): + """ + See `runtime.Runtime.get_aside_of_type` + + This override adds the field data from the block to the aside + """ + asides_cached = block.get_asides() if isinstance(block, XModuleMixin) else None + if asides_cached: + for aside in asides_cached: + if aside.scope_ids.block_type == aside_type: + return aside + + new_aside = super(CachingDescriptorSystem, self).get_aside_of_type(block, aside_type) + new_aside._field_data = block._field_data # pylint: disable=protected-access + + for key, _ in new_aside.fields.iteritems(): + if isinstance(key, KeyValueStore.Key) and block._field_data.has(new_aside, key): # pylint: disable=protected-access + try: + value = block._field_data.get(new_aside, key) # pylint: disable=protected-access + except KeyError: + pass + else: + setattr(new_aside, key, value) + + block.add_aside(new_aside) + return new_aside diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py index 71ebc3351b..965d6cfa81 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py @@ -1604,11 +1604,8 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): serial += 1 @contract(returns='XBlock') - def create_item( - self, user_id, course_key, block_type, block_id=None, - definition_locator=None, fields=None, - force=False, **kwargs - ): + def create_item(self, user_id, course_key, block_type, block_id=None, definition_locator=None, fields=None, + asides=None, force=False, **kwargs): """ Add a descriptor to persistence as an element of the course. Return the resulting post saved version with populated locators. @@ -1695,6 +1692,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): block_fields, definition_locator.definition_id, new_id, + asides=asides )) self.update_structure(course_key, new_structure) @@ -1723,7 +1721,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): # reconstruct the new_item from the cache return self.get_item(item_loc) - def create_child(self, user_id, parent_usage_key, block_type, block_id=None, fields=None, **kwargs): + def create_child(self, user_id, parent_usage_key, block_type, block_id=None, fields=None, asides=None, **kwargs): """ Creates and saves a new xblock that as a child of the specified block @@ -1738,10 +1736,12 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): a new identifier will be generated fields (dict): A dictionary specifying initial values for some or all fields in the newly created block + asides (dict): A dictionary specifying initial values for some or all aside fields + in the newly created block """ with self.bulk_operations(parent_usage_key.course_key): xblock = self.create_item( - user_id, parent_usage_key.course_key, block_type, block_id=block_id, fields=fields, + user_id, parent_usage_key.course_key, block_type, block_id=block_id, fields=fields, asides=asides, **kwargs) # skip attach to parent if xblock has 'detached' tag @@ -1986,10 +1986,8 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): partitioned_fields, descriptor.definition_locator, allow_not_found, force, **kwargs ) or descriptor - def _update_item_from_fields( - self, user_id, course_key, block_key, partitioned_fields, - definition_locator, allow_not_found, force, **kwargs - ): + def _update_item_from_fields(self, user_id, course_key, block_key, partitioned_fields, # pylint: disable=too-many-statements + definition_locator, allow_not_found, force, asides=None, **kwargs): """ Broke out guts of update_item for short-circuited internal use only """ @@ -1999,7 +1997,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): for subfields in partitioned_fields.itervalues(): fields.update(subfields) return self.create_item( - user_id, course_key, block_key.type, fields=fields, force=force + user_id, course_key, block_key.type, fields=fields, asides=asides, force=force ) original_structure = self._lookup_course(course_key).structure @@ -2011,9 +2009,8 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): fields = {} for subfields in partitioned_fields.itervalues(): fields.update(subfields) - return self.create_item( - user_id, course_key, block_key.type, block_id=block_key.id, fields=fields, force=force, - ) + return self.create_item(user_id, course_key, block_key.type, block_id=block_key.id, fields=fields, + asides=asides, force=force) else: raise ItemNotFoundError(course_key.make_usage_key(block_key.type, block_key.id)) @@ -2039,14 +2036,24 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): if is_updated: settings['children'] = serialized_children + asides_data_to_update = None + if asides: + asides_data_to_update, asides_updated = self._get_asides_to_update_from_structure(original_structure, + block_key, asides) + else: + asides_updated = False + # if updated, rev the structure - if is_updated: + if is_updated or asides_updated: new_structure = self.version_structure(course_key, original_structure, user_id) block_data = self._get_block_from_structure(new_structure, block_key) block_data.definition = definition_locator.definition_id block_data.fields = settings + if asides_updated: + block_data.asides = asides_data_to_update + new_id = new_structure['_id'] # source_version records which revision a block was copied from. In this method, we're updating @@ -3215,7 +3222,8 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): self._delete_if_true_orphan(BlockKey(*child), structure) @contract(returns=BlockData) - def _new_block(self, user_id, category, block_fields, definition_id, new_id, raw=False, block_defaults=None): + def _new_block(self, user_id, category, block_fields, definition_id, new_id, raw=False, + asides=None, block_defaults=None): """ Create the core document structure for a block. @@ -3226,10 +3234,13 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): """ if not raw: block_fields = self._serialize_fields(category, block_fields) + if not asides: + asides = [] document = { 'block_type': category, 'definition': definition_id, 'fields': block_fields, + 'asides': asides, 'edit_info': { 'edited_on': datetime.datetime.now(UTC), 'edited_by': user_id, @@ -3249,6 +3260,38 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): """ return structure['blocks'].get(block_key) + @contract(block_key=BlockKey) + def _get_asides_to_update_from_structure(self, structure, block_key, asides): + """ + Get list of aside fields that should be updated/inserted + """ + block = self._get_block_from_structure(structure, block_key) + + if asides: + updated = False + + tmp_new_asides_data = {} + for asd in asides: + aside_type = asd['aside_type'] + tmp_new_asides_data[aside_type] = asd + + result_list = [] + for i, aside in enumerate(block.asides): + if aside['aside_type'] in tmp_new_asides_data: + result_list.append(tmp_new_asides_data.pop(aside['aside_type'])) + updated = True + else: + result_list.append(aside) + + if tmp_new_asides_data: + for _, asd in tmp_new_asides_data.iteritems(): + result_list.append(asd) + updated = True + + return result_list, updated + else: + return block.asides, False + @contract(block_key=BlockKey, content=BlockData) def _update_block_in_structure(self, structure, block_key, content): """ diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py index dc25acf7ac..2b69a6d2a7 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py @@ -137,7 +137,7 @@ class DraftVersioningModuleStore(SplitMongoModuleStore, ModuleStoreDraftAndPubli keys_to_check.extend(children) return new_keys - def update_item(self, descriptor, user_id, allow_not_found=False, force=False, **kwargs): + def update_item(self, descriptor, user_id, allow_not_found=False, force=False, asides=None, **kwargs): old_descriptor_locn = descriptor.location descriptor.location = self._map_revision_to_branch(old_descriptor_locn) emit_signals = descriptor.location.branch == ModuleStoreEnum.BranchName.published \ @@ -149,17 +149,15 @@ class DraftVersioningModuleStore(SplitMongoModuleStore, ModuleStoreDraftAndPubli user_id, allow_not_found=allow_not_found, force=force, + asides=asides, **kwargs ) self._auto_publish_no_children(item.location, item.location.category, user_id, **kwargs) descriptor.location = old_descriptor_locn return item - def create_item( - self, user_id, course_key, block_type, block_id=None, - definition_locator=None, fields=None, - force=False, skip_auto_publish=False, **kwargs - ): + def create_item(self, user_id, course_key, block_type, block_id=None, # pylint: disable=too-many-statements + definition_locator=None, fields=None, asides=None, force=False, skip_auto_publish=False, **kwargs): """ See :py:meth `ModuleStoreDraftAndPublished.create_item` """ @@ -169,7 +167,7 @@ class DraftVersioningModuleStore(SplitMongoModuleStore, ModuleStoreDraftAndPubli with self.bulk_operations(course_key, emit_signals=emit_signals): item = super(DraftVersioningModuleStore, self).create_item( user_id, course_key, block_type, block_id=block_id, - definition_locator=definition_locator, fields=fields, + definition_locator=definition_locator, fields=fields, asides=asides, force=force, **kwargs ) if not skip_auto_publish: @@ -178,13 +176,13 @@ class DraftVersioningModuleStore(SplitMongoModuleStore, ModuleStoreDraftAndPubli def create_child( self, user_id, parent_usage_key, block_type, block_id=None, - fields=None, **kwargs + fields=None, asides=None, **kwargs ): parent_usage_key = self._map_revision_to_branch(parent_usage_key) with self.bulk_operations(parent_usage_key.course_key): item = super(DraftVersioningModuleStore, self).create_child( user_id, parent_usage_key, block_type, block_id=block_id, - fields=fields, **kwargs + fields=fields, asides=asides, **kwargs ) # Publish both the child and the parent, if the child is a direct-only category self._auto_publish_no_children(item.location, item.location.category, user_id, **kwargs) @@ -552,14 +550,16 @@ class DraftVersioningModuleStore(SplitMongoModuleStore, ModuleStoreDraftAndPubli draft_course = course_key.for_branch(ModuleStoreEnum.BranchName.draft) with self.branch_setting(ModuleStoreEnum.Branch.draft_preferred, draft_course): # Importing the block and publishing the block links the draft & published blocks' version history. - draft_block = self.import_xblock(user_id, draft_course, block_type, block_id, fields, runtime) + draft_block = self.import_xblock(user_id, draft_course, block_type, block_id, fields, + runtime, **kwargs) return self.publish(draft_block.location.version_agnostic(), user_id, blacklist=EXCLUDE_ALL, **kwargs) # do the import partitioned_fields = self.partition_fields_by_scope(block_type, fields) course_key = self._map_revision_to_branch(course_key) # cast to branch_setting return self._update_item_from_fields( - user_id, course_key, BlockKey(block_type, block_id), partitioned_fields, None, allow_not_found=True, force=True + user_id, course_key, BlockKey(block_type, block_id), partitioned_fields, None, + allow_not_found=True, force=True, **kwargs ) or self.get_item(new_usage_key) def compute_published_info_internal(self, xblock): diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_mongo_kvs.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_mongo_kvs.py index 8720b7c3d9..da3417236e 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_mongo_kvs.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_mongo_kvs.py @@ -6,6 +6,7 @@ from xblock.exceptions import InvalidScopeError from .definition_lazy_loader import DefinitionLazyLoader from xmodule.modulestore.inheritance import InheritanceKeyValueStore from opaque_keys.edx.locator import BlockUsageLocator +from xblock.core import XBlockAside # id is a BlockUsageLocator, def_id is the definition's guid SplitMongoKVSid = namedtuple('SplitMongoKVSid', 'id, def_id') @@ -21,7 +22,7 @@ class SplitMongoKVS(InheritanceKeyValueStore): VALID_SCOPES = (Scope.parent, Scope.children, Scope.settings, Scope.content) @contract(parent="BlockUsageLocator | None") - def __init__(self, definition, initial_values, default_values, parent, field_decorator=None): + def __init__(self, definition, initial_values, default_values, parent, aside_fields=None, field_decorator=None): """ :param definition: either a lazyloader or definition id for the definition @@ -42,34 +43,52 @@ class SplitMongoKVS(InheritanceKeyValueStore): self.field_decorator = field_decorator self.parent = parent + self.aside_fields = aside_fields if aside_fields else {} def get(self, key): - # load the field, if needed - if key.field_name not in self._fields: - # parent undefined in editing runtime (I think) - if key.scope == Scope.parent: - return self.parent - if key.scope == Scope.children: - # didn't find children in _fields; so, see if there's a default - raise KeyError() - elif key.scope == Scope.settings: - # get default which may be the inherited value - raise KeyError() - elif key.scope == Scope.content: - if isinstance(self._definition, DefinitionLazyLoader): - self._load_definition() - else: - raise KeyError() - else: + if key.block_family == XBlockAside.entry_point: + if key.scope not in [Scope.settings, Scope.content]: raise InvalidScopeError(key, self.VALID_SCOPES) - if key.field_name in self._fields: - field_value = self._fields[key.field_name] + if key.block_scope_id.block_type not in self.aside_fields: + # load the definition to see if it has the aside_fields + self._load_definition() + if key.block_scope_id.block_type not in self.aside_fields: + raise KeyError() + aside_fields = self.aside_fields[key.block_scope_id.block_type] + # load the field, if needed + if key.field_name not in aside_fields: + self._load_definition() - # return the "decorated" field value - return self.field_decorator(field_value) + if key.field_name in aside_fields: + return self.field_decorator(aside_fields[key.field_name]) - return None + raise KeyError() + else: + # load the field, if needed + if key.field_name not in self._fields: + if key.scope == Scope.parent: + return self.parent + if key.scope == Scope.children: + # didn't find children in _fields; so, see if there's a default + raise KeyError() + elif key.scope == Scope.settings: + # get default which may be the inherited value + raise KeyError() + elif key.scope == Scope.content: + if isinstance(self._definition, DefinitionLazyLoader): + self._load_definition() + else: + raise KeyError() + else: + raise InvalidScopeError(key) + + if key.field_name in self._fields: + field_value = self._fields[key.field_name] + # return the "decorated" field value + return self.field_decorator(field_value) + + return None def set(self, key, value): # handle any special cases @@ -78,17 +97,23 @@ class SplitMongoKVS(InheritanceKeyValueStore): if key.scope == Scope.content: self._load_definition() - # set the field - self._fields[key.field_name] = value + if key.block_family == XBlockAside.entry_point: + if key.scope == Scope.children: + raise InvalidScopeError(key) - # This function is currently incomplete: it doesn't handle side effects. - # To complete this function, here is some pseudocode for what should happen: - # - # if key.scope == Scope.children: - # remove inheritance from any exchildren - # add inheritance to any new children - # if key.scope == Scope.settings: - # if inheritable, push down to children + self.aside_fields.setdefault(key.block_scope_id.block_type, {})[key.field_name] = value + else: + # set the field + self._fields[key.field_name] = value + + # This function is currently incomplete: it doesn't handle side effects. + # To complete this function, here is some pseudocode for what should happen: + # + # if key.scope == Scope.children: + # remove inheritance from any exchildren + # add inheritance to any new children + # if key.scope == Scope.settings: + # if inheritable, push down to children def delete(self, key): # handle any special cases @@ -97,9 +122,17 @@ class SplitMongoKVS(InheritanceKeyValueStore): if key.scope == Scope.content: self._load_definition() - # delete the field value - if key.field_name in self._fields: - del self._fields[key.field_name] + if key.block_family == XBlockAside.entry_point: + if key.scope == Scope.children: + raise InvalidScopeError(key) + + if key.block_scope_id.block_type in self.aside_fields \ + and key.field_name in self.aside_fields[key.block_scope_id.block_type]: + del self.aside_fields[key.block_scope_id.block_type][key.field_name] + else: + # delete the field value + if key.field_name in self._fields: + del self._fields[key.field_name] def has(self, key): """ @@ -111,9 +144,16 @@ class SplitMongoKVS(InheritanceKeyValueStore): elif key.scope == Scope.parent: return True - # it's not clear whether inherited values should return True. Right now they don't - # if someone changes it so that they do, then change any tests of field.name in xx._field_data - return key.field_name in self._fields + if key.block_family == XBlockAside.entry_point: + if key.scope == Scope.children: + return False + + b_type = key.block_scope_id.block_type + return b_type in self.aside_fields and key.field_name in self.aside_fields[b_type] + else: + # it's not clear whether inherited values should return True. Right now they don't + # if someone changes it so that they do, then change any tests of field.name in xx._field_data + return key.field_name in self._fields def default(self, key): """ @@ -134,5 +174,10 @@ class SplitMongoKVS(InheritanceKeyValueStore): if persisted_definition is not None: fields = self._definition.field_converter(persisted_definition.get('fields')) self._fields.update(fields) + aside_fields_p = persisted_definition.get('aside_fields') + if aside_fields_p: + aside_fields = self._definition.field_converter(aside_fields_p) + for aside_type, fields in aside_fields.iteritems(): + self.aside_fields.setdefault(aside_type, {}).update(fields) # do we want to cache any of the edit_info? self._definition = None # already loaded diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py index 95c30d92de..dbe4a21f03 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -17,6 +17,7 @@ from mock import patch, Mock, call from django.conf import settings # This import breaks this test file when run separately. Needs to be fixed! (PLAT-449) from nose.plugins.attrib import attr +from nose import SkipTest import pymongo from pytz import UTC from shutil import rmtree @@ -30,6 +31,12 @@ from xmodule.contentstore.content import StaticContent from opaque_keys.edx.keys import CourseKey from xmodule.modulestore.xml_importer import import_course_from_xml from xmodule.modulestore.xml_exporter import export_course_to_xml +from xmodule.modulestore.tests.test_asides import AsideTestType +from xblock.core import XBlockAside +from xblock.fields import Scope, String, ScopeIds +from xblock.fragment import Fragment +from xblock.runtime import DictKeyValueStore, KvsFieldData +from xblock.test.tools import TestRuntime if not settings.configured: settings.configure() @@ -156,7 +163,7 @@ class CommonMixedModuleStoreSetup(CourseComparisonTest): self.user_id = ModuleStoreEnum.UserID.test # pylint: disable=invalid-name - def _create_course(self, course_key): + def _create_course(self, course_key, asides=None): """ Create a course w/ one item in the persistence store using the given course & item location. """ @@ -169,7 +176,8 @@ class CommonMixedModuleStoreSetup(CourseComparisonTest): self.assertEqual(self.course.id, course_key) # create chapter - chapter = self.store.create_child(self.user_id, self.course.location, 'chapter', block_id='Overview') + chapter = self.store.create_child(self.user_id, self.course.location, 'chapter', + block_id='Overview', asides=asides) self.writable_chapter_location = chapter.location def _create_block_hierarchy(self): @@ -296,6 +304,36 @@ class CommonMixedModuleStoreSetup(CourseComparisonTest): self.assertEquals(default, self.store.get_modulestore_type(self.course.id)) +class AsideFoo(XBlockAside): + """ + Test xblock aside class + """ + FRAG_CONTENT = u"

Aside Foo rendered

" + + field11 = String(default="aside1_default_value1", scope=Scope.content) + field12 = String(default="aside1_default_value2", scope=Scope.settings) + + @XBlockAside.aside_for('student_view') + def student_view_aside(self, block, context): # pylint: disable=unused-argument + """Add to the student view""" + return Fragment(self.FRAG_CONTENT) + + +class AsideBar(XBlockAside): + """ + Test xblock aside class + """ + FRAG_CONTENT = u"

Aside Bar rendered

" + + field21 = String(default="aside2_default_value1", scope=Scope.content) + field22 = String(default="aside2_default_value2", scope=Scope.settings) + + @XBlockAside.aside_for('student_view') + def student_view_aside(self, block, context): # pylint: disable=unused-argument + """Add to the student view""" + return Fragment(self.FRAG_CONTENT) + + @ddt.ddt @attr('mongo') class TestMixedModuleStore(CommonMixedModuleStoreSetup): @@ -2978,3 +3016,307 @@ class TestPublishOverExportImport(CommonMixedModuleStoreSetup): with self.store.branch_setting(ModuleStoreEnum.Branch.published_only, source_course_key): component = self.store.get_item(unit.location) self.assertEqual(component.display_name, updated_display_name) + + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) + @XBlockAside.register_temp_plugin(AsideTestType, 'test_aside') + @patch('xmodule.modulestore.split_mongo.caching_descriptor_system.CachingDescriptorSystem.applicable_aside_types', + lambda self, block: ['test_aside']) + def test_aside_crud(self, default_store): + """ + Check that asides could be imported from XML and the modulestores handle asides crud + """ + if default_store == ModuleStoreEnum.Type.mongo: + raise SkipTest("asides not supported in old mongo") + with MongoContentstoreBuilder().build() as contentstore: + self.store = MixedModuleStore( + contentstore=contentstore, + create_modulestore_instance=create_modulestore_instance, + mappings={}, + **self.OPTIONS + ) + self.addCleanup(self.store.close_all_connections) + with self.store.default_store(default_store): + dest_course_key = self.store.make_course_key('edX', "aside_test", "2012_Fall") + courses = import_course_from_xml( + self.store, self.user_id, DATA_DIR, ['aside'], + load_error_modules=False, + static_content_store=contentstore, + target_id=dest_course_key, + create_if_not_present=True, + ) + + # check that the imported blocks have the right asides and values + def check_block(block): + """ + Check whether block has the expected aside w/ its fields and then recurse to the block's children + """ + asides = block.runtime.get_asides(block) + + self.assertEqual(len(asides), 1, "Found {} asides but expected only test_aside".format(asides)) + self.assertIsInstance(asides[0], AsideTestType) + category = block.scope_ids.block_type + self.assertEqual(asides[0].data_field, "{} aside data".format(category)) + self.assertEqual(asides[0].content, "{} Aside".format(category.capitalize())) + + for child in block.get_children(): + check_block(child) + + check_block(courses[0]) + + # create a new block and ensure its aside magically appears with the right fields + new_chapter = self.store.create_child(self.user_id, courses[0].location, 'chapter', 'new_chapter') + asides = new_chapter.runtime.get_asides(new_chapter) + + self.assertEqual(len(asides), 1, "Found {} asides but expected only test_aside".format(asides)) + chapter_aside = asides[0] + self.assertIsInstance(chapter_aside, AsideTestType) + self.assertFalse( + chapter_aside.fields['data_field'].is_set_on(chapter_aside), + "data_field says it's assigned to {}".format(chapter_aside.data_field) + ) + self.assertFalse( + chapter_aside.fields['content'].is_set_on(chapter_aside), + "content says it's assigned to {}".format(chapter_aside.content) + ) + + # now update the values + chapter_aside.data_field = 'new value' + self.store.update_item(new_chapter, self.user_id, asides=[chapter_aside]) + + new_chapter = self.store.get_item(new_chapter.location) + chapter_aside = new_chapter.runtime.get_asides(new_chapter)[0] + self.assertEqual('new value', chapter_aside.data_field) + + # update the values the second time + chapter_aside.data_field = 'another one value' + self.store.update_item(new_chapter, self.user_id, asides=[chapter_aside]) + + new_chapter2 = self.store.get_item(new_chapter.location) + chapter_aside2 = new_chapter2.runtime.get_asides(new_chapter2)[0] + self.assertEqual('another one value', chapter_aside2.data_field) + + +@ddt.ddt +@attr('mongo') +class TestAsidesWithMixedModuleStore(CommonMixedModuleStoreSetup): + """ + Tests of the MixedModulestore interface methods with XBlock asides. + """ + def setUp(self): + """ + Setup environment for testing + """ + super(TestAsidesWithMixedModuleStore, self).setUp() + key_store = DictKeyValueStore() + field_data = KvsFieldData(key_store) + self.runtime = TestRuntime(services={'field-data': field_data}) # pylint: disable=abstract-class-instantiated + + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) + @XBlockAside.register_temp_plugin(AsideFoo, 'test_aside1') + @XBlockAside.register_temp_plugin(AsideBar, 'test_aside2') + @patch('xmodule.modulestore.split_mongo.caching_descriptor_system.CachingDescriptorSystem.applicable_aside_types', + lambda self, block: ['test_aside1', 'test_aside2']) + def test_get_and_update_asides(self, default_store): + """ + Tests that connected asides could be stored, received and updated along with connected course items + """ + if default_store == ModuleStoreEnum.Type.mongo: + raise SkipTest("asides not supported in old mongo") + + self.initdb(default_store) + + block_type1 = 'test_aside1' + def_id = self.runtime.id_generator.create_definition(block_type1) + usage_id = self.runtime.id_generator.create_usage(def_id) + + # the first aside item + aside1 = AsideFoo(scope_ids=ScopeIds('user', block_type1, def_id, usage_id), runtime=self.runtime) + aside1.field11 = 'new_value11' + aside1.field12 = 'new_value12' + + block_type2 = 'test_aside2' + def_id = self.runtime.id_generator.create_definition(block_type1) + usage_id = self.runtime.id_generator.create_usage(def_id) + + # the second aside item + aside2 = AsideBar(scope_ids=ScopeIds('user', block_type2, def_id, usage_id), runtime=self.runtime) + aside2.field21 = 'new_value21' + + # create new item with two asides + published_xblock = self.store.create_item( + self.user_id, + self.course.id, + 'vertical', + block_id='test_vertical', + asides=[aside1, aside2] + ) + + def _check_asides(asides, field11, field12, field21, field22): + """ Helper function to check asides """ + self.assertEqual(len(asides), 2) + self.assertEqual({type(asides[0]), type(asides[1])}, {AsideFoo, AsideBar}) + self.assertEqual(asides[0].field11, field11) + self.assertEqual(asides[0].field12, field12) + self.assertEqual(asides[1].field21, field21) + self.assertEqual(asides[1].field22, field22) + + # get saved item and check asides + component = self.store.get_item(published_xblock.location) + asides = component.runtime.get_asides(component) + _check_asides(asides, 'new_value11', 'new_value12', 'new_value21', 'aside2_default_value2') + + asides[0].field11 = 'other_value11' + + # update the first aside item and check that it was stored correctly + self.store.update_item(component, self.user_id, asides=[asides[0]]) + cached_asides = component.runtime.get_asides(component) + _check_asides(cached_asides, 'other_value11', 'new_value12', 'new_value21', 'aside2_default_value2') + + new_component = self.store.get_item(published_xblock.location) + new_asides = new_component.runtime.get_asides(new_component) + _check_asides(new_asides, 'other_value11', 'new_value12', 'new_value21', 'aside2_default_value2') + + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) + @XBlockAside.register_temp_plugin(AsideFoo, 'test_aside1') + @patch('xmodule.modulestore.split_mongo.caching_descriptor_system.CachingDescriptorSystem.applicable_aside_types', + lambda self, block: ['test_aside1']) + def test_clone_course_with_asides(self, default_store): + """ + Tests that connected asides will be cloned together with the parent courses + """ + if default_store == ModuleStoreEnum.Type.mongo: + raise SkipTest("asides not supported in old mongo") + + with MongoContentstoreBuilder().build() as contentstore: + # initialize the mixed modulestore + self._initialize_mixed(contentstore=contentstore, mappings={}) + + with self.store.default_store(default_store): + block_type1 = 'test_aside1' + def_id = self.runtime.id_generator.create_definition(block_type1) + usage_id = self.runtime.id_generator.create_usage(def_id) + + aside1 = AsideFoo(scope_ids=ScopeIds('user', block_type1, def_id, usage_id), runtime=self.runtime) + aside1.field11 = 'test1' + + source_course_key = self.store.make_course_key("org.source", "course.source", "run.source") + self._create_course(source_course_key, asides=[aside1]) + + dest_course_id = self.store.make_course_key("org.other", "course.other", "run.other") + self.store.clone_course(source_course_key, dest_course_id, self.user_id) + + source_store = self.store._get_modulestore_by_type(default_store) # pylint: disable=protected-access + self.assertCoursesEqual(source_store, source_course_key, source_store, dest_course_id) + + # after clone get connected aside and check that it was cloned correctly + actual_items = source_store.get_items(dest_course_id, + revision=ModuleStoreEnum.RevisionOption.published_only) + chapter_is_found = False + + for block in actual_items: + if block.scope_ids.block_type == 'chapter': + asides = block.runtime.get_asides(block) + self.assertEqual(len(asides), 1) + self.assertEqual(asides[0].field11, 'test1') + self.assertEqual(asides[0].field12, 'aside1_default_value2') + chapter_is_found = True + break + + self.assertTrue(chapter_is_found) + + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) + @XBlockAside.register_temp_plugin(AsideFoo, 'test_aside1') + @patch('xmodule.modulestore.split_mongo.caching_descriptor_system.CachingDescriptorSystem.applicable_aside_types', + lambda self, block: ['test_aside1']) + def test_delete_item_with_asides(self, default_store): + """ + Tests that connected asides will be removed together with the connected items + """ + if default_store == ModuleStoreEnum.Type.mongo: + raise SkipTest("asides not supported in old mongo") + + self.initdb(default_store) + + block_type1 = 'test_aside1' + def_id = self.runtime.id_generator.create_definition(block_type1) + usage_id = self.runtime.id_generator.create_usage(def_id) + + aside1 = AsideFoo(scope_ids=ScopeIds('user', block_type1, def_id, usage_id), runtime=self.runtime) + aside1.field11 = 'new_value11' + aside1.field12 = 'new_value12' + + published_xblock = self.store.create_item( + self.user_id, + self.course.id, + 'vertical', + block_id='test_vertical', + asides=[aside1] + ) + + asides = published_xblock.runtime.get_asides(published_xblock) + self.assertEquals(asides[0].field11, 'new_value11') + self.assertEquals(asides[0].field12, 'new_value12') + + # remove item + self.store.delete_item(published_xblock.location, self.user_id) + + # create item again + published_xblock2 = self.store.create_item( + self.user_id, + self.course.id, + 'vertical', + block_id='test_vertical' + ) + + # check that aside has default values + asides2 = published_xblock2.runtime.get_asides(published_xblock2) + self.assertEquals(asides2[0].field11, 'aside1_default_value1') + self.assertEquals(asides2[0].field12, 'aside1_default_value2') + + @ddt.data((ModuleStoreEnum.Type.mongo, 1, 0), (ModuleStoreEnum.Type.split, 2, 0)) + @XBlockAside.register_temp_plugin(AsideFoo, 'test_aside1') + @patch('xmodule.modulestore.split_mongo.caching_descriptor_system.CachingDescriptorSystem.applicable_aside_types', + lambda self, block: ['test_aside1']) + @ddt.unpack + def test_published_and_unpublish_item_with_asides(self, default_store, max_find, max_send): + """ + Tests that public/unpublish doesn't affect connected stored asides + """ + if default_store == ModuleStoreEnum.Type.mongo: + raise SkipTest("asides not supported in old mongo") + + self.initdb(default_store) + + block_type1 = 'test_aside1' + def_id = self.runtime.id_generator.create_definition(block_type1) + usage_id = self.runtime.id_generator.create_usage(def_id) + + aside1 = AsideFoo(scope_ids=ScopeIds('user', block_type1, def_id, usage_id), runtime=self.runtime) + aside1.field11 = 'new_value11' + aside1.field12 = 'new_value12' + + def _check_asides(item): + """ Helper function to check asides """ + asides = item.runtime.get_asides(item) + self.assertEquals(asides[0].field11, 'new_value11') + self.assertEquals(asides[0].field12, 'new_value12') + + # start off as Private + item = self.store.create_child(self.user_id, self.writable_chapter_location, 'problem', + 'test_compute_publish_state', asides=[aside1]) + item_location = item.location + with check_mongo_calls(max_find, max_send): + self.assertFalse(self.store.has_published_version(item)) + _check_asides(item) + + # Private -> Public + self.store.publish(item_location, self.user_id) + item = self.store.get_item(item_location) + self.assertTrue(self.store.has_published_version(item)) + _check_asides(item) + + # Public -> Private + self.store.unpublish(item_location, self.user_id) + item = self.store.get_item(item_location) + self.assertFalse(self.store.has_published_version(item)) + _check_asides(item) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_semantics.py b/common/lib/xmodule/xmodule/modulestore/tests/test_semantics.py index fab98f1acd..8bf1446300 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_semantics.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_semantics.py @@ -6,6 +6,7 @@ import ddt import itertools from collections import namedtuple from xmodule.course_module import CourseSummary +from mock import patch from xmodule.modulestore.tests.utils import ( PureModulestoreTestCase, MongoModulestoreBuilder, @@ -15,7 +16,10 @@ from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.tests.factories import CourseFactory from xmodule.modulestore.draft_and_published import DIRECT_ONLY_CATEGORIES -from xblock.core import XBlock +from xblock.core import XBlock, XBlockAside +from xblock.fields import Scope, String +from xblock.runtime import DictKeyValueStore, KvsFieldData +from xblock.test.tools import TestRuntime DETACHED_BLOCK_TYPES = dict(XBlock.load_tagged_classes('detached')) @@ -26,6 +30,13 @@ TESTABLE_BLOCK_TYPES.discard('course') TestField = namedtuple('TestField', ['field_name', 'initial', 'updated']) +class AsideTest(XBlockAside): + """ + Test xblock aside class + """ + content = String(default="content", scope=Scope.content) + + @ddt.ddt class DirectOnlyCategorySemantics(PureModulestoreTestCase): """ @@ -43,6 +54,8 @@ class DirectOnlyCategorySemantics(PureModulestoreTestCase): 'course_info': TestField('data', '
test data
', '
different test data
'), } + ASIDE_DATA_FIELD = TestField('content', '
aside test data
', '
aside different test data
') + def setUp(self): super(DirectOnlyCategorySemantics, self).setUp() self.course = CourseFactory.create( @@ -73,7 +86,8 @@ class DirectOnlyCategorySemantics(PureModulestoreTestCase): with self.assertRaises(ItemNotFoundError): self.store.get_item(block_usage_key) - def assertBlockHasContent(self, block_usage_key, field_name, content, draft=None): + def assertBlockHasContent(self, block_usage_key, field_name, content, + aside_field_name=None, aside_content=None, draft=None): """ Assert that the block ``block_usage_key`` has the value ``content`` for ``field_name`` when it is loaded. @@ -82,6 +96,8 @@ class DirectOnlyCategorySemantics(PureModulestoreTestCase): block_usage_key: The xblock to check. field_name (string): The name of the field to check. content: The value to assert is in the field. + aside_field_name (string): The name of the field to check (in connected xblock aside) + aside_content: The value to assert is in the xblock aside field. draft (optional): If omitted, verify both published and draft branches. If True, verify only the draft branch. If False, verify only the published branch. @@ -92,6 +108,10 @@ class DirectOnlyCategorySemantics(PureModulestoreTestCase): block_usage_key, ) self.assertEquals(content, target_block.fields[field_name].read_from(target_block)) + if aside_field_name and aside_content: + aside = self._get_aside(target_block) + self.assertIsNotNone(aside) + self.assertEquals(aside_content, aside.fields[aside_field_name].read_from(aside)) if draft is None or draft: with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred): @@ -99,6 +119,10 @@ class DirectOnlyCategorySemantics(PureModulestoreTestCase): block_usage_key, ) self.assertEquals(content, target_block.fields[field_name].read_from(target_block)) + if aside_field_name and aside_content: + aside = self._get_aside(target_block) + self.assertIsNotNone(aside) + self.assertEquals(aside_content, aside.fields[aside_field_name].read_from(aside)) def assertParentOf(self, parent_usage_key, child_usage_key, draft=None): """ @@ -202,9 +226,29 @@ class DirectOnlyCategorySemantics(PureModulestoreTestCase): def test_create(self, block_type): self._do_create(block_type) + def _prepare_asides(self, scope_ids): + """ + Return list with connected aside xblocks + """ + key_store = DictKeyValueStore() + field_data = KvsFieldData(key_store) + + aside = AsideTest(scope_ids=scope_ids, runtime=TestRuntime(services={'field-data': field_data})) # pylint: disable=abstract-class-instantiated + aside.fields[self.ASIDE_DATA_FIELD.field_name].write_to(aside, self.ASIDE_DATA_FIELD.initial) + return [aside] + + def _get_aside(self, block): + """ + Return connected xblock aside + """ + for aside in block.runtime.get_asides(block): + if isinstance(aside, AsideTest): + return aside + return None + # This function is split out from the test_create method so that it can be called # by other tests - def _do_create(self, block_type): + def _do_create(self, block_type, with_asides=False): """ Create a block of block_type (which should be a DIRECT_ONLY_CATEGORY), and then verify that it was created successfully, and is visible in @@ -228,21 +272,33 @@ class DirectOnlyCategorySemantics(PureModulestoreTestCase): block_id=block_usage_key.block_id ) block.fields[test_data.field_name].write_to(block, initial_field_value) - self.store.update_item(block, ModuleStoreEnum.UserID.test, allow_not_found=True) + asides = [] + if with_asides: + asides = self._prepare_asides(self.course.scope_ids.usage_id) + self.store.update_item(block, ModuleStoreEnum.UserID.test, asides=asides, allow_not_found=True) else: - block = self.store.create_child( + asides = [] + if with_asides: + asides = self._prepare_asides(self.course.scope_ids.usage_id) + self.store.create_child( user_id=ModuleStoreEnum.UserID.test, parent_usage_key=self.course.scope_ids.usage_id, block_type=block_type, block_id=block_usage_key.block_id, fields={test_data.field_name: initial_field_value}, + asides=asides ) if self.is_detached(block_type): self.assertCourseDoesntPointToBlock(block_usage_key) else: self.assertCoursePointsToBlock(block_usage_key) - self.assertBlockHasContent(block_usage_key, test_data.field_name, initial_field_value) + + if with_asides: + self.assertBlockHasContent(block_usage_key, test_data.field_name, initial_field_value, + self.ASIDE_DATA_FIELD.field_name, self.ASIDE_DATA_FIELD.initial) + else: + self.assertBlockHasContent(block_usage_key, test_data.field_name, initial_field_value) return block_usage_key @@ -354,6 +410,7 @@ class DirectOnlyCategorySemantics(PureModulestoreTestCase): self.assertBlockDoesntExist(child_usage_key) +@ddt.ddt class TestSplitDirectOnlyCategorySemantics(DirectOnlyCategorySemantics): """ Verify DIRECT_ONLY_CATEGORY semantics against the SplitMongoModulestore. @@ -361,6 +418,32 @@ class TestSplitDirectOnlyCategorySemantics(DirectOnlyCategorySemantics): MODULESTORE = SPLIT_MODULESTORE_SETUP __test__ = True + @ddt.data(*TESTABLE_BLOCK_TYPES) + @XBlockAside.register_temp_plugin(AsideTest, 'test_aside') + @patch('xmodule.modulestore.split_mongo.caching_descriptor_system.CachingDescriptorSystem.applicable_aside_types', + lambda self, block: ['test_aside']) + def test_create_with_asides(self, block_type): + self._do_create(block_type, with_asides=True) + + @ddt.data(*TESTABLE_BLOCK_TYPES) + @XBlockAside.register_temp_plugin(AsideTest, 'test_aside') + @patch('xmodule.modulestore.split_mongo.caching_descriptor_system.CachingDescriptorSystem.applicable_aside_types', + lambda self, block: ['test_aside']) + def test_update_asides(self, block_type): + block_usage_key = self._do_create(block_type, with_asides=True) + test_data = self.DATA_FIELDS[block_type] + + with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred): + block = self.store.get_item(block_usage_key) + aside = self._get_aside(block) + self.assertIsNotNone(aside) + aside.fields[self.ASIDE_DATA_FIELD.field_name].write_to(aside, self.ASIDE_DATA_FIELD.updated) + + self.store.update_item(block, ModuleStoreEnum.UserID.test, allow_not_found=True, asides=[aside]) + + self.assertBlockHasContent(block_usage_key, test_data.field_name, test_data.initial, + self.ASIDE_DATA_FIELD.field_name, self.ASIDE_DATA_FIELD.updated) + class TestMongoDirectOnlyCategorySemantics(DirectOnlyCategorySemantics): """ diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index 8ef58dbea6..a7bfe69ae0 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -736,10 +736,11 @@ def _update_and_import_module( ) fields = _update_module_references(module, source_course_id, dest_course_id) + asides = module.get_asides() if isinstance(module, XModuleMixin) else None return store.import_xblock( user_id, dest_course_id, module.location.category, - module.location.block_id, fields, runtime + module.location.block_id, fields, runtime, asides=asides ) diff --git a/common/lib/xmodule/xmodule/tests/test_library_content.py b/common/lib/xmodule/xmodule/tests/test_library_content.py index ab967761e3..0f35160057 100644 --- a/common/lib/xmodule/xmodule/tests/test_library_content.py +++ b/common/lib/xmodule/xmodule/tests/test_library_content.py @@ -287,7 +287,7 @@ class TestLibraryContentRender(LibraryContentTest): """ Rendering unit tests for LibraryContentModule """ - def test_preivew_view(self): + def test_preview_view(self): """ Test preview view rendering """ self.lc_block.refresh_children() self.lc_block = self.store.get_item(self.lc_block.location) diff --git a/common/lib/xmodule/xmodule/tests/xml/__init__.py b/common/lib/xmodule/xmodule/tests/xml/__init__.py index f4342b45b6..3beeebf271 100644 --- a/common/lib/xmodule/xmodule/tests/xml/__init__.py +++ b/common/lib/xmodule/xmodule/tests/xml/__init__.py @@ -41,6 +41,7 @@ class InMemorySystem(XMLParsingSystem, MakoDescriptorSystem): # pylint: disable def process_xml(self, xml): # pylint: disable=method-hidden """Parse `xml` as an XBlock, and add it to `self._descriptors`""" + self.get_asides = Mock(return_value=[]) descriptor = self.xblock_from_node( etree.fromstring(xml), None, diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index 8060a71004..c6b54c8e24 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -298,6 +298,7 @@ class XModuleMixin(XModuleFields, XBlock): def __init__(self, *args, **kwargs): self.xmodule_runtime = None + self._asides = [] super(XModuleMixin, self).__init__(*args, **kwargs) @@ -382,6 +383,18 @@ class XModuleMixin(XModuleFields, XBlock): """ return self._field_data + def add_aside(self, aside): + """ + save connected asides + """ + self._asides.append(aside) + + def get_asides(self): + """ + get the list of connected asides + """ + return self._asides + def get_explicitly_set_fields_by_scope(self, scope=Scope.content): """ Get a dictionary of the fields for the given scope which are set explicitly on this xblock. (Including @@ -1194,7 +1207,7 @@ class ConfigurableFragmentWrapper(object): """ Runtime mixin that allows for composition of many `wrap_xblock` wrappers """ - def __init__(self, wrappers=None, **kwargs): + def __init__(self, wrappers=None, wrappers_asides=None, **kwargs): """ :param wrappers: A list of wrappers, where each wrapper is: @@ -1207,6 +1220,10 @@ class ConfigurableFragmentWrapper(object): self.wrappers = wrappers else: self.wrappers = [] + if wrappers_asides is not None: + self.wrappers_asides = wrappers_asides + else: + self.wrappers_asides = [] def wrap_xblock(self, block, view, frag, context): """ @@ -1217,6 +1234,15 @@ class ConfigurableFragmentWrapper(object): return frag + def wrap_aside(self, block, aside, view, frag, context): # pylint: disable=unused-argument + """ + See :func:`Runtime.wrap_child` + """ + for wrapper in self.wrappers_asides: + frag = wrapper(aside, view, frag, context) + + return frag + # This function exists to give applications (LMS/CMS) a place to monkey-patch until # we can refactor modulestore to split out the FieldData half of its interface from @@ -1524,13 +1550,19 @@ class XMLParsingSystem(DescriptorSystem): keys = ScopeIds(None, block_type, def_id, usage_id) block_class = self.mixologist.mix(self.load_block_type(block_type)) - self.parse_asides(node, def_id, usage_id, id_generator) + aside_children = self.parse_asides(node, def_id, usage_id, id_generator) + asides_tags = [x.tag for x in aside_children] block = block_class.parse_xml(node, self, keys, id_generator) self._convert_reference_fields_to_keys(block) # difference from XBlock.runtime block.parent = parent_id block.save() + asides = self.get_asides(block) + for asd in asides: + if asd.scope_ids.block_type in asides_tags: + block.add_aside(asd) + return block def parse_asides(self, node, def_id, usage_id, id_generator): @@ -1547,6 +1579,7 @@ class XMLParsingSystem(DescriptorSystem): for child in aside_children: self._aside_from_xml(child, def_id, usage_id, id_generator) node.remove(child) + return aside_children def _make_usage_key(self, course_key, value): """ diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py index 07a153241e..7e6d439a01 100644 --- a/common/lib/xmodule/xmodule/xml_module.py +++ b/common/lib/xmodule/xmodule/xml_module.py @@ -223,6 +223,7 @@ class XmlParserMixin(object): if filename is None: definition_xml = copy.deepcopy(xml_object) filepath = '' + aside_children = [] else: dog_stats_api.increment( DEPRECATION_VSCOMPAT_EVENT, @@ -250,7 +251,7 @@ class XmlParserMixin(object): definition_xml = cls.load_file(filepath, system.resources_fs, def_id) usage_id = id_generator.create_usage(def_id) - system.parse_asides(definition_xml, def_id, usage_id, id_generator) + aside_children = system.parse_asides(definition_xml, def_id, usage_id, id_generator) # Add the attributes from the pointer node definition_xml.attrib.update(xml_object.attrib) @@ -262,6 +263,9 @@ class XmlParserMixin(object): definition['definition_metadata'] = definition_metadata definition['filename'] = [filepath, filename] + if aside_children: + definition['aside_children'] = aside_children + return definition, children @classmethod @@ -333,6 +337,7 @@ class XmlParserMixin(object): url_name = node.get('url_name', node.get('slug')) def_id = id_generator.create_definition(node.tag, url_name) usage_id = id_generator.create_usage(def_id) + aside_children = [] # VS[compat] -- detect new-style each-in-a-file mode if is_pointer_tag(node): @@ -340,7 +345,7 @@ class XmlParserMixin(object): # read the actual definition file--named using url_name.replace(':','/') filepath = cls._format_filepath(node.tag, name_to_pathname(url_name)) definition_xml = cls.load_file(filepath, runtime.resources_fs, def_id) - runtime.parse_asides(definition_xml, def_id, usage_id, id_generator) + aside_children = runtime.parse_asides(definition_xml, def_id, usage_id, id_generator) else: filepath = None definition_xml = node @@ -370,6 +375,10 @@ class XmlParserMixin(object): log.debug('Error in loading metadata %r', dmdata, exc_info=True) metadata['definition_metadata_err'] = str(err) + definition_aside_children = definition.pop('aside_children', None) + if definition_aside_children: + aside_children.extend(definition_aside_children) + # Set/override any metadata specified by policy cls.apply_policy(metadata, runtime.get_policy(usage_id)) @@ -382,13 +391,22 @@ class XmlParserMixin(object): kvs = InheritanceKeyValueStore(initial_values=field_data) field_data = KvsFieldData(kvs) - return runtime.construct_xblock_from_class( + xblock = runtime.construct_xblock_from_class( cls, # We're loading a descriptor, so student_id is meaningless ScopeIds(None, node.tag, def_id, usage_id), field_data, ) + if aside_children: + asides_tags = [x.tag for x in aside_children] + asides = runtime.get_asides(xblock) + for asd in asides: + if asd.scope_ids.block_type in asides_tags: + xblock.add_aside(asd) + + return xblock + @classmethod def _format_filepath(cls, category, name): return u'{category}/{name}.{ext}'.format(category=category, diff --git a/common/static/js/xblock/core.js b/common/static/js/xblock/core.js index 1c6632c71d..0cbaac3209 100644 --- a/common/static/js/xblock/core.js +++ b/common/static/js/xblock/core.js @@ -92,6 +92,10 @@ var requestToken = requestToken || $element.data('request-token'); var children = XBlock.initializeXBlocks($element, requestToken); + var asides = XBlock.initializeXBlockAsides($element, requestToken); + if (asides) { + children = children.concat(asides); + } $element.prop('xblock_children', children); return constructBlock(element, [initArgs(element)]); @@ -132,8 +136,12 @@ * If neither is available, then use the request tokens of the immediateDescendent xblocks. */ initializeBlocks: function(element, requestToken) { - XBlock.initializeXBlockAsides(element, requestToken); - return XBlock.initializeXBlocks(element, requestToken); + var asides = XBlock.initializeXBlockAsides(element, requestToken); + var xblocks = XBlock.initializeXBlocks(element, requestToken); + if (asides) { + xblocks = xblocks.concat(asides); + } + return xblocks; } }; diff --git a/openedx/core/lib/xblock_utils.py b/openedx/core/lib/xblock_utils.py index b8024abfc5..ab25b27639 100644 --- a/openedx/core/lib/xblock_utils.py +++ b/openedx/core/lib/xblock_utils.py @@ -143,6 +143,72 @@ def wrap_xblock( return wrap_fragment(frag, render_to_string('xblock_wrapper.html', template_context)) +def wrap_xblock_aside( + runtime_class, + aside, + view, + frag, + context, # pylint: disable=unused-argument + usage_id_serializer, + request_token, # pylint: disable=redefined-outer-name + extra_data=None +): + """ + Wraps the results of rendering an XBlockAside view in a standard
with identifying + data so that the appropriate javascript module can be loaded onto it. + + :param runtime_class: The name of the javascript runtime class to use to load this block + :param aside: An XBlockAside + :param view: The name of the view that rendered the fragment being wrapped + :param frag: The :class:`Fragment` to be wrapped + :param context: The context passed to the view being rendered + :param usage_id_serializer: A function to serialize the block's usage_id for use by the + front-end Javascript Runtime. + :param request_token: An identifier that is unique per-request, so that only xblocks + rendered as part of this request will have their javascript initialized. + :param extra_data: A dictionary with extra data values to be set on the wrapper + """ + + if extra_data is None: + extra_data = {} + + data = {} + data.update(extra_data) + + css_classes = [ + 'xblock-{}'.format(markupsafe.escape(view)), + 'xblock-{}-{}'.format( + markupsafe.escape(view), + markupsafe.escape(aside.scope_ids.block_type), + ), + 'xblock_asides-v1' + ] + + if frag.js_init_fn: + data['init'] = frag.js_init_fn + data['runtime-class'] = runtime_class + data['runtime-version'] = frag.js_init_version + + data['block-type'] = aside.scope_ids.block_type + data['usage-id'] = usage_id_serializer(aside.scope_ids.usage_id) + data['request-token'] = request_token + + template_context = { + 'content': frag.content, + 'classes': css_classes, + 'data_attributes': u' '.join(u'data-{}="{}"'.format(markupsafe.escape(key), markupsafe.escape(value)) + for key, value in data.iteritems()), + } + + if hasattr(frag, 'json_init_args') and frag.json_init_args is not None: + # Replace / with \/ so that "" in the data won't break things. + template_context['js_init_parameters'] = json.dumps(frag.json_init_args).replace("/", r"\/") + else: + template_context['js_init_parameters'] = "" + + return wrap_fragment(frag, render_to_string('xblock_wrapper.html', template_context)) + + def replace_jump_to_id_urls(course_id, jump_to_id_base_url, block, view, frag, context): # pylint: disable=unused-argument """ This will replace a link between courseware in the format