Commit 16dc83df by Braden MacDonald

Merge pull request #6492 from open-craft/content_libraries/13-analytics-enhancements

Content libraries analytics enhancements (SOL-121)
parents 195d5b57 05fc6738
...@@ -34,9 +34,8 @@ def i_create_a_course(step): ...@@ -34,9 +34,8 @@ def i_create_a_course(step):
create_a_course() create_a_course()
# pylint: disable=invalid-name
@step('I click the course link in Studio Home$') @step('I click the course link in Studio Home$')
def i_click_the_course_link_in_studio_home(step): def i_click_the_course_link_in_studio_home(step): # pylint: disable=invalid-name
course_css = 'a.course-link' course_css = 'a.course-link'
world.css_click(course_css) world.css_click(course_css)
......
...@@ -28,28 +28,27 @@ GLOBAL_WAIT_FOR_TIMEOUT = 60 ...@@ -28,28 +28,27 @@ GLOBAL_WAIT_FOR_TIMEOUT = 60
REQUIREJS_WAIT = { REQUIREJS_WAIT = {
# Settings - Schedule & Details # Settings - Schedule & Details
re.compile('^Schedule & Details Settings \|'): [ re.compile(r'^Schedule & Details Settings \|'): [
"jquery", "js/base", "js/models/course", "jquery", "js/base", "js/models/course",
"js/models/settings/course_details", "js/views/settings/main"], "js/models/settings/course_details", "js/views/settings/main"],
# Settings - Advanced Settings # Settings - Advanced Settings
re.compile('^Advanced Settings \|'): [ re.compile(r'^Advanced Settings \|'): [
"jquery", "js/base", "js/models/course", "js/models/settings/advanced", "jquery", "js/base", "js/models/course", "js/models/settings/advanced",
"js/views/settings/advanced", "codemirror"], "js/views/settings/advanced", "codemirror"],
# Unit page # Unit page
re.compile('^Unit \|'): [ re.compile(r'^Unit \|'): [
"jquery", "js/base", "js/models/xblock_info", "js/views/pages/container", "jquery", "js/base", "js/models/xblock_info", "js/views/pages/container",
"js/collections/component_template", "xmodule", "coffee/src/main", "xblock/cms.runtime.v1"], "js/collections/component_template", "xmodule", "coffee/src/main", "xblock/cms.runtime.v1"],
# Content - Outline # Content - Outline
# Note that calling your org, course number, or display name, 'course' will mess this up # Note that calling your org, course number, or display name, 'course' will mess this up
re.compile('^Course Outline \|'): [ re.compile(r'^Course Outline \|'): [
"js/base", "js/models/course", "js/models/location", "js/models/section"], "js/base", "js/models/course", "js/models/location", "js/models/section"],
# Dashboard # Dashboard
# pylint: disable=anomalous-backslash-in-string re.compile(r'^Studio Home \|'): [
re.compile('^Studio Home \|'): [
"js/sock", "gettext", "js/base", "js/sock", "gettext", "js/base",
"jquery.ui", "coffee/src/main", "underscore"], "jquery.ui", "coffee/src/main", "underscore"],
...@@ -60,7 +59,7 @@ REQUIREJS_WAIT = { ...@@ -60,7 +59,7 @@ REQUIREJS_WAIT = {
], ],
# Pages # Pages
re.compile('^Pages \|'): [ re.compile(r'^Pages \|'): [
'js/models/explicit_url', 'coffee/src/views/tabs', 'js/models/explicit_url', 'coffee/src/views/tabs',
'xmodule', 'coffee/src/main', 'xblock/cms.runtime.v1' 'xmodule', 'coffee/src/main', 'xblock/cms.runtime.v1'
], ],
......
...@@ -220,25 +220,56 @@ class LibraryContentModule(LibraryContentFields, XModule, StudioEditableModule): ...@@ -220,25 +220,56 @@ class LibraryContentModule(LibraryContentFields, XModule, StudioEditableModule):
if hasattr(self, "_selected_set"): if hasattr(self, "_selected_set"):
# Already done: # Already done:
return self._selected_set # pylint: disable=access-member-before-definition return self._selected_set # pylint: disable=access-member-before-definition
selected = set(tuple(k) for k in self.selected) # set of (block_type, block_id) tuples assigned to this student
previous_count = len(selected)
lib_tools = self.runtime.service(self, 'library_tools')
format_block_keys = lambda keys: lib_tools.create_block_analytics_summary(self.location.course_key, keys)
def publish_event(event_name, **kwargs):
""" Publish an event for analytics purposes """
event_data = {
"location": unicode(self.location),
"result": format_block_keys(selected),
"previous_count": previous_count,
"max_count": self.max_count,
}
event_data.update(kwargs)
self.runtime.publish(self, "edx.librarycontentblock.content.{}".format(event_name), event_data)
# Determine which of our children we will show: # Determine which of our children we will show:
selected = set(tuple(k) for k in self.selected) # set of (block_type, block_id) tuples
valid_block_keys = set([(c.block_type, c.block_id) for c in self.children]) # pylint: disable=no-member valid_block_keys = set([(c.block_type, c.block_id) for c in self.children]) # pylint: disable=no-member
# Remove any selected blocks that are no longer valid: # Remove any selected blocks that are no longer valid:
selected -= (selected - valid_block_keys) invalid_block_keys = (selected - valid_block_keys)
if invalid_block_keys:
selected -= invalid_block_keys
# Publish an event for analytics purposes:
# reason "invalid" means deleted from library or a different library is now being used.
publish_event("removed", removed=format_block_keys(invalid_block_keys), reason="invalid")
# If max_count has been decreased, we may have to drop some previously selected blocks: # If max_count has been decreased, we may have to drop some previously selected blocks:
overlimit_block_keys = set()
while len(selected) > self.max_count: while len(selected) > self.max_count:
selected.pop() overlimit_block_keys.add(selected.pop())
if overlimit_block_keys:
# Publish an event for analytics purposes:
publish_event("removed", removed=format_block_keys(overlimit_block_keys), reason="overlimit")
# Do we have enough blocks now? # Do we have enough blocks now?
num_to_add = self.max_count - len(selected) num_to_add = self.max_count - len(selected)
if num_to_add > 0: if num_to_add > 0:
added_block_keys = None
# We need to select [more] blocks to display to this user: # We need to select [more] blocks to display to this user:
pool = valid_block_keys - selected
if self.mode == "random": if self.mode == "random":
pool = valid_block_keys - selected
num_to_add = min(len(pool), num_to_add) num_to_add = min(len(pool), num_to_add)
selected |= set(random.sample(pool, num_to_add)) added_block_keys = set(random.sample(pool, num_to_add))
# We now have the correct n random children to show for this user. # We now have the correct n random children to show for this user.
else: else:
raise NotImplementedError("Unsupported mode.") raise NotImplementedError("Unsupported mode.")
selected |= added_block_keys
if added_block_keys:
# Publish an event for analytics purposes:
publish_event("assigned", added=format_block_keys(added_block_keys))
# Save our selections to the user state, to ensure consistency: # Save our selections to the user state, to ensure consistency:
self.selected = list(selected) # TODO: this doesn't save from the LMS "Progress" page. self.selected = list(selected) # TODO: this doesn't save from the LMS "Progress" page.
# Cache the results # Cache the results
......
...@@ -44,6 +44,44 @@ class LibraryToolsService(object): ...@@ -44,6 +44,44 @@ class LibraryToolsService(object):
return library.location.library_key.version_guid return library.location.library_key.version_guid
return None return None
def create_block_analytics_summary(self, course_key, block_keys):
"""
Given a CourseKey and a list of (block_type, block_id) pairs,
prepare the JSON-ready metadata needed for analytics logging.
This is [
{"usage_key": x, "original_usage_key": y, "original_usage_version": z, "descendants": [...]}
]
where the main list contains all top-level blocks, and descendants contains a *flat* list of all
descendants of the top level blocks, if any.
"""
def summarize_block(usage_key):
""" Basic information about the given block """
orig_key, orig_version = self.store.get_block_original_usage(usage_key)
return {
"usage_key": unicode(usage_key),
"original_usage_key": unicode(orig_key) if orig_key else None,
"original_usage_version": unicode(orig_version) if orig_version else None,
}
result_json = []
for block_key in block_keys:
key = course_key.make_usage_key(*block_key)
info = summarize_block(key)
info['descendants'] = []
try:
block = self.store.get_item(key, depth=None) # Load the item and all descendants
children = list(getattr(block, "children", []))
while children:
child_key = children.pop()
child = self.store.get_item(child_key)
info['descendants'].append(summarize_block(child_key))
children.extend(getattr(child, "children", []))
except ItemNotFoundError:
pass # The block has been deleted
result_json.append(info)
return result_json
def _filter_child(self, usage_key, capa_type): def _filter_child(self, usage_key, capa_type):
""" """
Filters children by CAPA problem type, if configured Filters children by CAPA problem type, if configured
......
...@@ -509,6 +509,18 @@ class MixedModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase): ...@@ -509,6 +509,18 @@ class MixedModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase):
store = self._get_modulestore_for_courseid(location.course_key) store = self._get_modulestore_for_courseid(location.course_key)
return store.get_parent_location(location, **kwargs) return store.get_parent_location(location, **kwargs)
def get_block_original_usage(self, usage_key):
"""
If a block was inherited into another structure using copy_from_template,
this will return the original block usage locator from which the
copy was inherited.
"""
try:
store = self._verify_modulestore_support(usage_key.course_key, 'get_block_original_usage')
return store.get_block_original_usage(usage_key)
except NotImplementedError:
return None, None
def get_modulestore_type(self, course_id): def get_modulestore_type(self, course_id):
""" """
Returns a type which identifies which modulestore is servicing the given course_id. Returns a type which identifies which modulestore is servicing the given course_id.
......
...@@ -454,12 +454,17 @@ class SplitBulkWriteMixin(BulkOperationsMixin): ...@@ -454,12 +454,17 @@ class SplitBulkWriteMixin(BulkOperationsMixin):
if block_info['edit_info'].get('update_version') == update_version: if block_info['edit_info'].get('update_version') == update_version:
return return
original_usage = block_info['edit_info'].get('original_usage')
original_usage_version = block_info['edit_info'].get('original_usage_version')
block_info['edit_info'] = { block_info['edit_info'] = {
'edited_on': datetime.datetime.now(UTC), 'edited_on': datetime.datetime.now(UTC),
'edited_by': user_id, 'edited_by': user_id,
'previous_version': block_info['edit_info']['update_version'], 'previous_version': block_info['edit_info']['update_version'],
'update_version': update_version, 'update_version': update_version,
} }
if original_usage:
block_info['edit_info']['original_usage'] = original_usage
block_info['edit_info']['original_usage_version'] = original_usage_version
def find_matching_course_indexes(self, branch=None, search_targets=None): def find_matching_course_indexes(self, branch=None, search_targets=None):
""" """
...@@ -1254,6 +1259,21 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): ...@@ -1254,6 +1259,21 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
# TODO implement # TODO implement
pass pass
def get_block_original_usage(self, usage_key):
"""
If a block was inherited into another structure using copy_from_template,
this will return the original block usage locator and version from
which the copy was inherited.
Returns usage_key, version if the data is available, otherwise returns (None, None)
"""
blocks = self._lookup_course(usage_key.course_key).structure['blocks']
block = blocks.get(BlockKey.from_usage_key(usage_key))
if block and 'original_usage' in block['edit_info']:
usage_key = BlockUsageLocator.from_string(block['edit_info']['original_usage'])
return usage_key, block['edit_info'].get('original_usage_version')
return None, None
def create_definition_from_data(self, course_key, new_def_data, category, user_id): def create_definition_from_data(self, course_key, new_def_data, category, user_id):
""" """
Pull the definition fields out of descriptor and save to the db as a new definition Pull the definition fields out of descriptor and save to the db as a new definition
...@@ -2214,6 +2234,8 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): ...@@ -2214,6 +2234,8 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
# Setting it to the source_block_info structure version here breaks split_draft's has_changes() method. # Setting it to the source_block_info structure version here breaks split_draft's has_changes() method.
new_block_info['edit_info']['edited_by'] = user_id new_block_info['edit_info']['edited_by'] = user_id
new_block_info['edit_info']['edited_on'] = datetime.datetime.now(UTC) new_block_info['edit_info']['edited_on'] = datetime.datetime.now(UTC)
new_block_info['edit_info']['original_usage'] = unicode(usage_key.replace(branch=None, version_guid=None))
new_block_info['edit_info']['original_usage_version'] = source_block_info['edit_info'].get('update_version')
dest_structure['blocks'][new_block_key] = new_block_info dest_structure['blocks'][new_block_key] = new_block_info
children = source_block_info['fields'].get('children') children = source_block_info['fields'].get('children')
......
...@@ -268,6 +268,15 @@ class DraftVersioningModuleStore(SplitMongoModuleStore, ModuleStoreDraftAndPubli ...@@ -268,6 +268,15 @@ class DraftVersioningModuleStore(SplitMongoModuleStore, ModuleStoreDraftAndPubli
location = self._map_revision_to_branch(location, revision=revision) location = self._map_revision_to_branch(location, revision=revision)
return super(DraftVersioningModuleStore, self).get_parent_location(location, **kwargs) return super(DraftVersioningModuleStore, self).get_parent_location(location, **kwargs)
def get_block_original_usage(self, usage_key):
"""
If a block was inherited into another structure using copy_from_template,
this will return the original block usage locator from which the
copy was inherited.
"""
usage_key = self._map_revision_to_branch(usage_key)
return super(DraftVersioningModuleStore, self).get_block_original_usage(usage_key)
def get_orphans(self, course_key, **kwargs): def get_orphans(self, course_key, **kwargs):
course_key = self._map_revision_to_branch(course_key) course_key = self._map_revision_to_branch(course_key)
return super(DraftVersioningModuleStore, self).get_orphans(course_key, **kwargs) return super(DraftVersioningModuleStore, self).get_orphans(course_key, **kwargs)
......
...@@ -119,6 +119,7 @@ class MixedSplitTestCase(TestCase): ...@@ -119,6 +119,7 @@ class MixedSplitTestCase(TestCase):
extra.update(kwargs) extra.update(kwargs)
return ItemFactory.create( return ItemFactory.create(
category=category, category=category,
parent=parent_block,
parent_location=parent_block.location, parent_location=parent_block.location,
modulestore=self.store, modulestore=self.store,
**extra **extra
......
...@@ -755,6 +755,7 @@ def _invoke_xblock_handler(request, course_id, usage_id, handler, suffix, user): ...@@ -755,6 +755,7 @@ def _invoke_xblock_handler(request, course_id, usage_id, handler, suffix, user):
try: try:
descriptor = modulestore().get_item(usage_key) descriptor = modulestore().get_item(usage_key)
descriptor_orig_usage_key, descriptor_orig_version = modulestore().get_block_original_usage(usage_key)
except ItemNotFoundError: except ItemNotFoundError:
log.warn( log.warn(
"Invalid location for course id {course_id}: {usage_key}".format( "Invalid location for course id {course_id}: {usage_key}".format(
...@@ -768,8 +769,13 @@ def _invoke_xblock_handler(request, course_id, usage_id, handler, suffix, user): ...@@ -768,8 +769,13 @@ def _invoke_xblock_handler(request, course_id, usage_id, handler, suffix, user):
tracking_context = { tracking_context = {
'module': { 'module': {
'display_name': descriptor.display_name_with_default, 'display_name': descriptor.display_name_with_default,
'usage_key': unicode(descriptor.location),
} }
} }
# For blocks that are inherited from a content library, we add some additional metadata:
if descriptor_orig_usage_key is not None:
tracking_context['module']['original_usage_key'] = unicode(descriptor_orig_usage_key)
tracking_context['module']['original_usage_version'] = unicode(descriptor_orig_version)
field_data_cache = FieldDataCache.cache_for_descriptor_descendents( field_data_cache = FieldDataCache.cache_for_descriptor_descendents(
course_id, course_id,
......
...@@ -5,6 +5,7 @@ Test for lms courseware app, module render unit ...@@ -5,6 +5,7 @@ Test for lms courseware app, module render unit
from functools import partial from functools import partial
import json import json
from bson import ObjectId
import ddt import ddt
from django.http import Http404, HttpResponse from django.http import Http404, HttpResponse
from django.core.urlresolvers import reverse from django.core.urlresolvers import reverse
...@@ -13,6 +14,7 @@ from django.test.client import RequestFactory ...@@ -13,6 +14,7 @@ from django.test.client import RequestFactory
from django.test.utils import override_settings from django.test.utils import override_settings
from django.contrib.auth.models import AnonymousUser from django.contrib.auth.models import AnonymousUser
from mock import MagicMock, patch, Mock from mock import MagicMock, patch, Mock
from opaque_keys.edx.keys import UsageKey
from opaque_keys.edx.locations import SlashSeparatedCourseKey from opaque_keys.edx.locations import SlashSeparatedCourseKey
from xblock.field_data import FieldData from xblock.field_data import FieldData
from xblock.runtime import Runtime from xblock.runtime import Runtime
...@@ -971,12 +973,13 @@ class TestModuleTrackingContext(ModuleStoreTestCase): ...@@ -971,12 +973,13 @@ class TestModuleTrackingContext(ModuleStoreTestCase):
def test_context_contains_display_name(self, mock_tracker): def test_context_contains_display_name(self, mock_tracker):
problem_display_name = u'Option Response Problem' problem_display_name = u'Option Response Problem'
actual_display_name = self.handle_callback_and_get_display_name_from_event(mock_tracker, problem_display_name) module_info = self.handle_callback_and_get_module_info(mock_tracker, problem_display_name)
self.assertEquals(problem_display_name, actual_display_name) self.assertEquals(problem_display_name, module_info['display_name'])
def handle_callback_and_get_display_name_from_event(self, mock_tracker, problem_display_name=None): def handle_callback_and_get_module_info(self, mock_tracker, problem_display_name=None):
""" """
Creates a fake module, invokes the callback and extracts the display name from the emitted problem_check event. Creates a fake module, invokes the callback and extracts the 'module'
metadata from the emitted problem_check event.
""" """
descriptor_kwargs = { descriptor_kwargs = {
'category': 'problem', 'category': 'problem',
...@@ -1000,12 +1003,28 @@ class TestModuleTrackingContext(ModuleStoreTestCase): ...@@ -1000,12 +1003,28 @@ class TestModuleTrackingContext(ModuleStoreTestCase):
event = mock_call[1][0] event = mock_call[1][0]
self.assertEquals(event['event_type'], 'problem_check') self.assertEquals(event['event_type'], 'problem_check')
return event['context']['module']['display_name'] return event['context']['module']
def test_missing_display_name(self, mock_tracker): def test_missing_display_name(self, mock_tracker):
actual_display_name = self.handle_callback_and_get_display_name_from_event(mock_tracker) actual_display_name = self.handle_callback_and_get_module_info(mock_tracker)['display_name']
self.assertTrue(actual_display_name.startswith('problem')) self.assertTrue(actual_display_name.startswith('problem'))
def test_library_source_information(self, mock_tracker):
"""
Check that XBlocks that are inherited from a library include the
information about their library block source in events.
We patch the modulestore to avoid having to create a library.
"""
original_usage_key = UsageKey.from_string(u'block-v1:A+B+C+type@problem+block@abcd1234')
original_usage_version = ObjectId()
mock_get_original_usage = lambda _, key: (original_usage_key, original_usage_version)
with patch('xmodule.modulestore.mixed.MixedModuleStore.get_block_original_usage', mock_get_original_usage):
module_info = self.handle_callback_and_get_module_info(mock_tracker)
self.assertIn('original_usage_key', module_info)
self.assertEqual(module_info['original_usage_key'], unicode(original_usage_key))
self.assertIn('original_usage_version', module_info)
self.assertEqual(module_info['original_usage_version'], unicode(original_usage_version))
class TestXmoduleRuntimeEvent(TestSubmittingProblems): class TestXmoduleRuntimeEvent(TestSubmittingProblems):
""" """
......
...@@ -10,6 +10,7 @@ from django.conf import settings ...@@ -10,6 +10,7 @@ from django.conf import settings
from lms.djangoapps.lms_xblock.models import XBlockAsidesConfig from lms.djangoapps.lms_xblock.models import XBlockAsidesConfig
from openedx.core.djangoapps.user_api.api import course_tag as user_course_tag_api from openedx.core.djangoapps.user_api.api import course_tag as user_course_tag_api
from xmodule.modulestore.django import modulestore from xmodule.modulestore.django import modulestore
from xmodule.library_tools import LibraryToolsService
from xmodule.x_module import ModuleSystem from xmodule.x_module import ModuleSystem
from xmodule.partitions.partitions_service import PartitionService from xmodule.partitions.partitions_service import PartitionService
...@@ -199,6 +200,7 @@ class LmsModuleSystem(LmsHandlerUrls, ModuleSystem): # pylint: disable=abstract ...@@ -199,6 +200,7 @@ class LmsModuleSystem(LmsHandlerUrls, ModuleSystem): # pylint: disable=abstract
course_id=kwargs.get('course_id'), course_id=kwargs.get('course_id'),
track_function=kwargs.get('track_function', None), track_function=kwargs.get('track_function', None),
) )
services['library_tools'] = LibraryToolsService(modulestore())
services['fs'] = xblock.reference.plugins.FSService() services['fs'] = xblock.reference.plugins.FSService()
self.request_token = kwargs.pop('request_token', None) self.request_token = kwargs.pop('request_token', None)
super(LmsModuleSystem, self).__init__(**kwargs) super(LmsModuleSystem, self).__init__(**kwargs)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment