From a40d2b65d23d8b44f6b0a592cfbddb2231a1819d Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Wed, 28 Oct 2015 19:05:40 -0400 Subject: [PATCH 1/6] Course Blocks App MA-1556 --- lms/djangoapps/course_blocks/__init__.py | 27 ++ lms/djangoapps/course_blocks/api.py | 97 +++++ lms/djangoapps/course_blocks/signals.py | 27 ++ .../course_blocks/transformers/__init__.py | 3 + .../transformers/tests/test_helpers.py | 330 ++++++++++++++++++ .../course_blocks/transformers/utils.py | 16 + lms/djangoapps/course_blocks/usage_info.py | 36 ++ lms/envs/common.py | 4 + lms/envs/dev.py | 5 + lms/envs/test.py | 4 + openedx/core/lib/block_cache/__init__.py | 5 +- openedx/core/lib/block_cache/block_cache.py | 6 + .../core/lib/block_cache/block_structure.py | 17 +- setup.py | 6 + 14 files changed, 575 insertions(+), 8 deletions(-) create mode 100644 lms/djangoapps/course_blocks/__init__.py create mode 100644 lms/djangoapps/course_blocks/api.py create mode 100644 lms/djangoapps/course_blocks/signals.py create mode 100644 lms/djangoapps/course_blocks/transformers/__init__.py create mode 100644 lms/djangoapps/course_blocks/transformers/tests/test_helpers.py create mode 100644 lms/djangoapps/course_blocks/transformers/utils.py create mode 100644 lms/djangoapps/course_blocks/usage_info.py diff --git a/lms/djangoapps/course_blocks/__init__.py b/lms/djangoapps/course_blocks/__init__.py new file mode 100644 index 0000000000..6357a46361 --- /dev/null +++ b/lms/djangoapps/course_blocks/__init__.py @@ -0,0 +1,27 @@ +""" +The Course Blocks app, built upon the Block Cache framework in +openedx.core.lib.block_cache, is a higher layer django app in LMS that +provides additional context of Courses and Users (via usage_info.py) with +implementations for Block Structure Transformers that are related to +block structure course access. + +As described in the Block Cache framework's __init__ module, this +framework provides faster access to course blocks for performance +sensitive features, by caching all transformer-required data so no +modulestore access is necessary during block access. + +It is expected that only Block Access related transformers reside in +this django app, as they are cross-cutting authorization transformers +required across other features. Other higher-level and feature-specific +transformers should be implemented in their own separate apps. + +Note: Currently, some of the implementation is redundant with the +has_access code in courseware/access.py. However, we do have short-term +plans for refactoring the current has_access code to use Course Blocks +instead (https://openedx.atlassian.net/browse/MA-1019). We have +introduced this redundancy in the short-term as an incremental +implementation approach, reducing risk with initial release of this app. +""" + +# Importing signals is necessary to activate the course publish/delete signal handlers. +from . import signals # pylint: disable=unused-import diff --git a/lms/djangoapps/course_blocks/api.py b/lms/djangoapps/course_blocks/api.py new file mode 100644 index 0000000000..995a251b67 --- /dev/null +++ b/lms/djangoapps/course_blocks/api.py @@ -0,0 +1,97 @@ +""" +API entry point to the course_blocks app with top-level +get_course_blocks and clear_course_from_cache functions. +""" +from django.core.cache import cache + +from openedx.core.lib.block_cache.block_cache import get_blocks, clear_block_cache +from xmodule.modulestore.django import modulestore + +from .transformers import ( + library_content, + start_date, + user_partitions, + visibility, +) +from .usage_info import CourseUsageInfo + + +# Default list of transformers for manipulating course block structures +# based on the user's access to the course blocks. +COURSE_BLOCK_ACCESS_TRANSFORMERS = [ + library_content.ContentLibraryTransformer(), + start_date.StartDateTransformer(), + user_partitions.UserPartitionTransformer(), + visibility.VisibilityTransformer(), +] + + +def get_course_blocks( + user, + root_block_usage_key, + transformers=None +): + """ + A higher order function implemented on top of the + block_cache.get_blocks function returning a transformed block + structure for the given user starting at root_block_usage_key. + + Note: The current implementation requires the root_block_usage_key + to be the root block of its corresponding course. However, this + is a short-term limitation, which will be addressed in a coming + ticket (https://openedx.atlassian.net/browse/MA-1604). Once that + ticket is implemented, callers will be able to get course blocks + starting at any arbitrary location within a block structure. + + Arguments: + user (django.contrib.auth.models.User) - User object for + which the block structure is to be transformed. + + root_block_usage_key (UsageKey) - The usage_key for the root + of the block structure that is being accessed. + + transformers ([BlockStructureTransformer]) - The list of + transformers whose transform methods are to be called. + If None, COURSE_BLOCK_ACCESS_TRANSFORMERS is used. + + Returns: + BlockStructureBlockData - A transformed block structure, + starting at root_block_usage_key, that has undergone the + transform methods for the given user and the course + associated with the block structure. If using the default + transformers, the transformed block structure will be + exactly equivalent to the blocks that the given user has + access. + """ + store = modulestore() + if root_block_usage_key != store.make_course_usage_key(root_block_usage_key.course_key): + # Enforce this check for now until MA-1604 is implemented. + # Otherwise, callers will get incorrect block data after a + # new version of the course is published, since + # clear_course_from_cache only clears the cached block + # structures starting at the root block of the course. + raise NotImplementedError + + return get_blocks( + cache, + store, + CourseUsageInfo(root_block_usage_key.course_key, user), + root_block_usage_key, + COURSE_BLOCK_ACCESS_TRANSFORMERS if transformers is None else transformers, + ) + + +def clear_course_from_cache(course_key): + """ + A higher order function implemented on top of the + block_cache.clear_block_cache function that clears the block + structure from the cache for the block structure starting at the + root block of the course for the given course_key. + + Note: See Note in get_course_blocks. Even after MA-1604 is + implemented, this implementation should still be valid since the + entire block structure of the course is cached, even though + arbitrary access to an intermediate block will be supported. + """ + course_usage_key = modulestore().make_course_usage_key(course_key) + return clear_block_cache(cache, course_usage_key) diff --git a/lms/djangoapps/course_blocks/signals.py b/lms/djangoapps/course_blocks/signals.py new file mode 100644 index 0000000000..2767b56a1a --- /dev/null +++ b/lms/djangoapps/course_blocks/signals.py @@ -0,0 +1,27 @@ +""" +Signal handlers for invalidating cached data. +""" +from django.dispatch.dispatcher import receiver + +from xmodule.modulestore.django import SignalHandler + +from .api import clear_course_from_cache + + +@receiver(SignalHandler.course_published) +def _listen_for_course_publish(sender, course_key, **kwargs): # pylint: disable=unused-argument + """ + Catches the signal that a course has been published in the module + store and invalidates the corresponding cache entry if one exists. + """ + clear_course_from_cache(course_key) + + +@receiver(SignalHandler.course_deleted) +def _listen_for_course_delete(sender, course_key, **kwargs): # pylint: disable=unused-argument + """ + Catches the signal that a course has been deleted from the + module store and invalidates the corresponding cache entry if one + exists. + """ + clear_course_from_cache(course_key) diff --git a/lms/djangoapps/course_blocks/transformers/__init__.py b/lms/djangoapps/course_blocks/transformers/__init__.py new file mode 100644 index 0000000000..9ab8802aec --- /dev/null +++ b/lms/djangoapps/course_blocks/transformers/__init__.py @@ -0,0 +1,3 @@ +""" +Module container for all Course Block Access Transformers. +""" diff --git a/lms/djangoapps/course_blocks/transformers/tests/test_helpers.py b/lms/djangoapps/course_blocks/transformers/tests/test_helpers.py new file mode 100644 index 0000000000..6cc5072b60 --- /dev/null +++ b/lms/djangoapps/course_blocks/transformers/tests/test_helpers.py @@ -0,0 +1,330 @@ +""" +Test helpers for testing course block transformers. +""" +from student.tests.factories import CourseEnrollmentFactory, UserFactory +from xmodule.modulestore.django import modulestore +from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from lms.djangoapps.courseware.access import has_access + +from ...api import get_course_blocks + + +class CourseStructureTestCase(ModuleStoreTestCase): + """ + Helper for test cases that need to build course structures. + """ + def setUp(self): + """ + Create users. + """ + super(CourseStructureTestCase, self).setUp() + # Set up users. + self.password = 'test' + self.user = UserFactory.create(password=self.password) + self.staff = UserFactory.create(password=self.password, is_staff=True) + + def create_block_id(self, block_type, block_ref): + """ + Returns the block id (display name) that is used in the test + course structures for the given block type and block reference + string. + """ + return '{}_{}'.format(block_type, block_ref) + + def build_xblock(self, block_hierarchy, block_map, parent): + """ + Build an XBlock, add it to block_map, and call build_xblock on + the children defined in block_dict. + + Arguments: + block_hierarchy (BlockStructureDict): Definition of + hierarchy, from this block down. + block_map (dict[str: XBlock]): Mapping from '#ref' values to + their XBlocks. + parent (XBlock): Parent block for this xBlock. + """ + block_type = block_hierarchy['#type'] + block_ref = block_hierarchy['#ref'] + factory = (CourseFactory if block_type == 'course' else ItemFactory) + kwargs = {key: value for key, value in block_hierarchy.iteritems() if key[0] != '#'} + + if block_type != 'course': + kwargs['category'] = block_type + if parent: + kwargs['parent'] = parent + + xblock = factory.create( + display_name=self.create_block_id(block_type, block_ref), + publish_item=True, + **kwargs + ) + block_map[block_ref] = xblock + + for child_hierarchy in block_hierarchy.get('#children', []): + self.build_xblock(child_hierarchy, block_map, xblock) + + def add_parents(self, block_hierarchy, block_map): + """ + Recursively traverse the block_hierarchy and add additional + parents. This method is expected to be called only after all + blocks have been created. + + The additional parents are obtained from the '#parents' field + and is expected to be a list of '#ref' values of the parents. + + Note: if a '#parents' field is found, the block is removed from + the course block since it is expected to not belong to the root. + If the block is meant to be a direct child of the course as + well, the course should be explicitly listed in '#parents'. + + Arguments: + block_hierarchy (BlockStructureDict): + Definition of block hierarchy. + block_map (dict[str: XBlock]): + Mapping from '#ref' values to their XBlocks. + + """ + parents = block_hierarchy.get('#parents', []) + if parents: + block_key = block_map[block_hierarchy['#ref']].location + + # First remove the block from the course. + # It would be re-added to the course if the course was + # explicitly listed in parents. + course = modulestore().get_item(block_map['course'].location) + course.children.remove(block_key) + block_map['course'] = update_block(course) + + # Add this to block to each listed parent. + for parent_ref in parents: + parent_block = modulestore().get_item(block_map[parent_ref].location) + parent_block.children.append(block_key) + block_map[parent_ref] = update_block(parent_block) + + # recursively call the children + for child_hierarchy in block_hierarchy.get('#children', []): + self.add_parents(child_hierarchy, block_map) + + def build_course(self, course_hierarchy): + """ + Build a hierarchy of XBlocks. + + Arguments: + course_hierarchy (BlockStructureDict): Definition of course + hierarchy. + + where a BlockStructureDict is a list of dicts in the form { + 'key1': 'value1', + ... + 'keyN': 'valueN', + '#type': block_type, + '#ref': short_string_for_referencing_block, + '#children': list[BlockStructureDict], + '#parents': list['#ref' values] + } + + Special keys start with '#'; the rest just get passed as + kwargs to Factory.create. + + Note: the caller has a choice of whether to create + (1) a nested block structure with children blocks embedded + within their parents, or + (2) a flat block structure with children blocks defined + alongside their parents and attached via the #parents + field, or + (3) a combination of both #1 and #2 used for whichever + blocks. + + Note 2: When the #parents field is used in addition to the + nested pattern for a block, it specifies additional parents + that aren't already implied by having the block exist within + another block's #children field. + + Returns: + dict[str: XBlock]: + Mapping from '#ref' values to their XBlocks. + """ + block_map = {} + + # build the course tree + for block_hierarchy in course_hierarchy: + self.build_xblock(block_hierarchy, block_map, parent=None) + + # add additional parents if the course is a DAG or built + # linearly (without specifying '#children' values) + for block_hierarchy in course_hierarchy: + self.add_parents(block_hierarchy, block_map) + + return block_map + + def get_block_key_set(self, blocks, *refs): + """ + Gets the set of usage keys that correspond to the list of + #ref values as defined on blocks. + + Returns: set[UsageKey] + """ + xblocks = (blocks[ref] for ref in refs) + return set([xblock.location for xblock in xblocks]) + + +class BlockParentsMapTestCase(ModuleStoreTestCase): + """ + Test helper class for creating a test course of + a graph of vertical blocks based on a parents_map. + """ + + # Tree formed by parent_map: + # 0 + # / \ + # 1 2 + # / \ / \ + # 3 4 / 5 + # \ / + # 6 + # Note the parents must always have lower indices than their + # children. + parents_map = [[], [0], [0], [1], [1], [2], [2, 4]] + + def setUp(self, **kwargs): + super(BlockParentsMapTestCase, self).setUp(**kwargs) + + # create the course + self.course = CourseFactory.create() + + # an ordered list of block locations, where the index + # corresponds to the block's index in the parents_map. + self.xblock_keys = [self.course.location] + + # create all other blocks in the course + for i, parents_index in enumerate(self.parents_map): + if i == 0: + continue # course already created + + # create the block as a vertical + self.xblock_keys.append( + ItemFactory.create( + parent=self.get_block(parents_index[0]), + category="vertical", + ).location + ) + + # add additional parents + if len(parents_index) > 1: + for index in range(1, len(parents_index)): + parent_index = parents_index[index] + parent_block = self.get_block(parent_index) + parent_block.children.append(self.xblock_keys[i]) + update_block(parent_block) + + self.password = 'test' + self.student = UserFactory.create(is_staff=False, username='test_student', password=self.password) + self.staff = UserFactory.create(is_staff=True, username='test_staff', password=self.password) + CourseEnrollmentFactory.create(is_active=True, mode='honor', user=self.student, course_id=self.course.id) + + def assert_transform_results( + self, + test_user, + expected_user_accessible_blocks, + blocks_with_differing_access, + transformers=None, + ): + """ + Verifies the results of transforming the blocks in the course. + + Arguments: + test_user (User): The non-staff user that is being tested. + For example, self.student. + + expected_user_accessible_blocks (set(int)): Set of blocks + (indices) that a student user is expected to have access + to after the transformers are executed. + + blocks_with_differing_access (set(int)): Set of + blocks (indices) whose access will differ from the + transformers result and the current implementation of + has_access. + + transformers (BlockStructureTransformer): An optional list + of transformer that are to be executed. If not + provided, the default value used by get_course_blocks + is used. + """ + def check_results(user, expected_accessible_blocks, blocks_with_differing_access): + """ + Verifies the results of transforming the blocks in the + course for the given user. + """ + + self.client.login(username=user.username, password=self.password) + block_structure = get_course_blocks(user, self.course.location, transformers=transformers) + + # Enumerate through all the blocks that were created in the + # course + for i, xblock_key in enumerate(self.xblock_keys): + + # verify existence of the block + block_structure_result = block_structure.has_block(xblock_key) + has_access_result = bool(has_access(user, 'load', self.get_block(i), course_key=self.course.id)) + + # compare with expected value + self.assertEquals( + block_structure_result, + i in expected_accessible_blocks, + "block_structure return value {0} not equal to expected value for block {1} for user {2}".format( + block_structure_result, i, user.username + ) + ) + + # compare with has_access result + if i in blocks_with_differing_access: + self.assertNotEqual( + block_structure_result, + has_access_result, + "block structure ({0}) & has_access ({1}) results are equal for block {2} for user {3}".format( + block_structure_result, has_access_result, i, user.username + ) + ) + else: + self.assertEquals( + block_structure_result, + has_access_result, + "block structure ({0}) & has_access ({1}) results not equal for block {2} for user {3}".format( + block_structure_result, has_access_result, i, user.username + ) + ) + + self.client.logout() + + # verify given test user has access to expected blocks + check_results( + test_user, + expected_user_accessible_blocks, + blocks_with_differing_access + ) + + # verify staff has access to all blocks + check_results(self.staff, set(range(len(self.parents_map))), {}) + + def get_block(self, block_index): + """ + Helper method to retrieve the requested block (index) from the + modulestore + """ + return modulestore().get_item(self.xblock_keys[block_index]) + + +def update_block(block): + """ + Helper method to update the block in the modulestore + """ + return modulestore().update_item(block, 'test_user') + + +def create_location(org, course, run, block_type, block_id): + """ + Returns the usage key for the given key parameters using the + default modulestore + """ + return modulestore().make_course_key(org, course, run).make_usage_key(block_type, block_id) diff --git a/lms/djangoapps/course_blocks/transformers/utils.py b/lms/djangoapps/course_blocks/transformers/utils.py new file mode 100644 index 0000000000..9884951251 --- /dev/null +++ b/lms/djangoapps/course_blocks/transformers/utils.py @@ -0,0 +1,16 @@ +""" +Common Helper utilities for transformers +""" + + +def get_field_on_block(block, field_name, default_value=None): + """ + Get the field value that is directly set on the xblock. + Do not get the inherited value since field inheritance + returns value from only a single parent chain + (e.g., doesn't take a union in DAGs). + """ + if block.fields[field_name].is_set_on(block): + return getattr(block, field_name) + else: + return default_value diff --git a/lms/djangoapps/course_blocks/usage_info.py b/lms/djangoapps/course_blocks/usage_info.py new file mode 100644 index 0000000000..f07d3d62c3 --- /dev/null +++ b/lms/djangoapps/course_blocks/usage_info.py @@ -0,0 +1,36 @@ +""" +Declares CourseUsageInfo class to be used by the transform method in +Transformers. +""" +from lms.djangoapps.courseware.access import _has_access_to_course + + +class CourseUsageInfo(object): + ''' + A class object that encapsulates the course and user context to be + used as currency across block structure transformers, by passing + an instance of it in calls to BlockStructureTransformer.transform + methods. + ''' + def __init__(self, course_key, user): + # Course identifier (opaque_keys.edx.keys.CourseKey) + self.course_key = course_key + + # User object (django.contrib.auth.models.User) + self.user = user + + # Cached value of whether the user has staff access (bool/None) + self._has_staff_access = None + + @property + def has_staff_access(self): + ''' + Returns whether the user has staff access to the course + associated with this CourseUsageInfo instance. + + For performance reasons (minimizing multiple SQL calls), the + value is cached within this instance. + ''' + if self._has_staff_access is None: + self._has_staff_access = _has_access_to_course(self.user, 'staff', self.course_key) + return self._has_staff_access diff --git a/lms/envs/common.py b/lms/envs/common.py index 1aa953a7a7..6072e7087e 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -1954,8 +1954,12 @@ INSTALLED_APPS = ( 'lms.djangoapps.lms_xblock', + # Course data caching 'openedx.core.djangoapps.content.course_overviews', 'openedx.core.djangoapps.content.course_structures', + 'lms.djangoapps.course_blocks', + + # Old course structure API 'course_structure_api', # Mailchimp Syncing diff --git a/lms/envs/dev.py b/lms/envs/dev.py index 0dc306b5c1..f8b00de05e 100644 --- a/lms/envs/dev.py +++ b/lms/envs/dev.py @@ -97,6 +97,11 @@ CACHES = { 'BACKEND': 'django.core.cache.backends.locmem.LocMemCache', 'LOCATION': 'edx_course_structure_mem_cache', }, + 'lms.course_blocks': { + 'BACKEND': 'django.core.cache.backends.locmem.LocMemCache', + 'KEY_FUNCTION': 'util.memcache.safe_key', + 'LOCATION': 'lms_course_blocks_cache', + }, } diff --git a/lms/envs/test.py b/lms/envs/test.py index ba943ab5e6..91fe133dd9 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -223,6 +223,10 @@ CACHES = { 'BACKEND': 'django.core.cache.backends.locmem.LocMemCache', 'LOCATION': 'edx_location_block_cache', }, + 'lms.course_blocks': { + 'BACKEND': 'django.core.cache.backends.locmem.LocMemCache', + 'LOCATION': 'edx_location_course_blocks', + }, } # Dummy secret key for dev diff --git a/openedx/core/lib/block_cache/__init__.py b/openedx/core/lib/block_cache/__init__.py index 28ba7eaae0..4cc1cb613e 100644 --- a/openedx/core/lib/block_cache/__init__.py +++ b/openedx/core/lib/block_cache/__init__.py @@ -51,7 +51,10 @@ Registry. Transformers are registered using the platform's PluginManager (e.g., Stevedore). This is currently done by updating setup.py. Only registered transformers are called during the Collect Phase. And only registered transformers can be used during the -Transform phase. +Transform phase. Exceptions to this rule are any nested transformers +that are contained within higher-order transformers - as long as the +higher-order transformers are registered and appropriately call the +contained transformers within them. Note: A partial subset (as an ordered list) of the registered transformers can be requested during the Transform phase, allowing diff --git a/openedx/core/lib/block_cache/block_cache.py b/openedx/core/lib/block_cache/block_cache.py index dba7adc608..7914cec3e3 100644 --- a/openedx/core/lib/block_cache/block_cache.py +++ b/openedx/core/lib/block_cache/block_cache.py @@ -36,6 +36,12 @@ def get_blocks(cache, modulestore, usage_info, root_block_usage_key, transformer transformers whose transform methods are to be called. This list should be a subset of the list of registered transformers in the Transformer Registry. + + Returns: + BlockStructureBlockData - A transformed block structure, + starting at root_block_usage_key, that has undergone the + transform methods in the given transformers with the + given usage_info. """ # Verify that all requested transformers are registered in the diff --git a/openedx/core/lib/block_cache/block_structure.py b/openedx/core/lib/block_cache/block_structure.py index f6a0817daa..a8db0497e7 100644 --- a/openedx/core/lib/block_cache/block_structure.py +++ b/openedx/core/lib/block_cache/block_structure.py @@ -118,6 +118,16 @@ class BlockStructure(object): """ return usage_key in self._block_relations + def get_block_keys(self): + """ + Returns the block keys in the block structure. + + Returns: + iterator(UsageKey) - An iterator of the usage + keys of all the blocks in the block structure. + """ + return self._block_relations.iterkeys() + #--- Block structure traversal methods ---# def topological_traversal( @@ -198,13 +208,6 @@ class BlockStructure(object): # Replace this structure's relations with the newly pruned one. self._block_relations = pruned_block_relations - def _get_block_keys(self): - """ - Returns an iterator of all the block keys in the block - structure. - """ - return self._block_relations.iterkeys() - def _add_relation(self, parent_key, child_key): """ Adds a parent to child relationship in this block structure. diff --git a/setup.py b/setup.py index 6bae27c051..b59b5fd156 100644 --- a/setup.py +++ b/setup.py @@ -48,5 +48,11 @@ setup( "cohort = openedx.core.djangoapps.course_groups.partition_scheme:CohortPartitionScheme", "verification = openedx.core.djangoapps.credit.partition_schemes:VerificationPartitionScheme", ], + "openedx.block_structure_transformer": [ + "library_content = lms.djangoapps.course_blocks.transformers.library_content:ContentLibraryTransformer", + "start_date = lms.djangoapps.course_blocks.transformers.start_date:StartDateTransformer", + "user_partitions = lms.djangoapps.course_blocks.transformers.user_partitions:UserPartitionTransformer", + "visibility = lms.djangoapps.course_blocks.transformers.visibility:VisibilityTransformer", + ], } ) From 3f6baa992d59da1c157fd502aa32d61299c65298 Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Wed, 28 Oct 2015 19:06:32 -0400 Subject: [PATCH 2/6] Transformer: VisibilityTransformer --- .../transformers/tests/__init__.py | 0 .../transformers/tests/test_visibility.py | 43 ++++++++++ .../course_blocks/transformers/visibility.py | 80 +++++++++++++++++++ 3 files changed, 123 insertions(+) create mode 100644 lms/djangoapps/course_blocks/transformers/tests/__init__.py create mode 100644 lms/djangoapps/course_blocks/transformers/tests/test_visibility.py create mode 100644 lms/djangoapps/course_blocks/transformers/visibility.py diff --git a/lms/djangoapps/course_blocks/transformers/tests/__init__.py b/lms/djangoapps/course_blocks/transformers/tests/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/lms/djangoapps/course_blocks/transformers/tests/test_visibility.py b/lms/djangoapps/course_blocks/transformers/tests/test_visibility.py new file mode 100644 index 0000000000..c6841e707a --- /dev/null +++ b/lms/djangoapps/course_blocks/transformers/tests/test_visibility.py @@ -0,0 +1,43 @@ +""" +Tests for VisibilityTransformer. +""" +import ddt + +from ..visibility import VisibilityTransformer +from .test_helpers import BlockParentsMapTestCase, update_block + + +@ddt.ddt +class VisibilityTransformerTestCase(BlockParentsMapTestCase): + """ + VisibilityTransformer Test + """ + # Following test cases are based on BlockParentsMapTestCase.parents_map + @ddt.data( + ({}, {0, 1, 2, 3, 4, 5, 6}, {}), + ({0}, {}, {1, 2, 3, 4, 5, 6}), + ({1}, {0, 2, 5, 6}, {3, 4}), + ({2}, {0, 1, 3, 4, 6}, {5}), + ({3}, {0, 1, 2, 4, 5, 6}, {}), + ({4}, {0, 1, 2, 3, 5, 6}, {}), + ({5}, {0, 1, 2, 3, 4, 6}, {}), + ({6}, {0, 1, 2, 3, 4, 5}, {}), + ({1, 2}, {0}, {3, 4, 5, 6}), + ({2, 4}, {0, 1, 3}, {5, 6}), + ({1, 2, 3, 4, 5, 6}, {0}, {}), + ) + @ddt.unpack + def test_block_visibility( + self, staff_only_blocks, expected_visible_blocks, blocks_with_differing_access + ): + for idx, _ in enumerate(self.parents_map): + block = self.get_block(idx) + block.visible_to_staff_only = (idx in staff_only_blocks) + update_block(block) + + self.assert_transform_results( + self.student, + expected_visible_blocks, + blocks_with_differing_access, + [VisibilityTransformer()], + ) diff --git a/lms/djangoapps/course_blocks/transformers/visibility.py b/lms/djangoapps/course_blocks/transformers/visibility.py new file mode 100644 index 0000000000..cde91c1ad3 --- /dev/null +++ b/lms/djangoapps/course_blocks/transformers/visibility.py @@ -0,0 +1,80 @@ +""" +Visibility Transformer implementation. +""" +from openedx.core.lib.block_cache.transformer import BlockStructureTransformer + + +class VisibilityTransformer(BlockStructureTransformer): + """ + A transformer that enforces the visible_to_staff_only field on + blocks by removing blocks from the block structure for which the + user does not have access. The visible_to_staff_only field on a + block is percolated down to its descendants, so that all blocks + enforce the visibility settings from their ancestors. + + For a block with multiple parents, access is denied only if + visibility is denied for all its parents. + + Staff users are exempted from visibility rules. + """ + VERSION = 1 + + MERGED_VISIBLE_TO_STAFF_ONLY = 'merged_visible_to_staff_only' + + @classmethod + def name(cls): + """ + Unique identifier for the transformer's class; + same identifier used in setup.py. + """ + return "visibility" + + @classmethod + def get_visible_to_staff_only(cls, block_structure, block_key): + """ + Returns whether the block with the given block_key in the + given block_structure should be visible to staff only per + computed value from ancestry chain. + """ + return block_structure.get_transformer_block_field( + block_key, cls, cls.MERGED_VISIBLE_TO_STAFF_ONLY, False + ) + + @classmethod + def collect(cls, block_structure): + """ + Collects any information that's necessary to execute this + transformer's transform method. + """ + for block_key in block_structure.topological_traversal(): + + # compute merged value of visible_to_staff_only from all parents + parents = block_structure.get_parents(block_key) + all_parents_visible_to_staff_only = all( # pylint: disable=invalid-name + cls.get_visible_to_staff_only(block_structure, parent_key) + for parent_key in parents + ) if parents else False + + # set the merged value for this block + block_structure.set_transformer_block_field( + block_key, + cls, + cls.MERGED_VISIBLE_TO_STAFF_ONLY, + # merge visible_to_staff_only from all parents and this block + ( + all_parents_visible_to_staff_only or + block_structure.get_xblock(block_key).visible_to_staff_only + ) + ) + + def transform(self, usage_info, block_structure): + """ + Mutates block_structure based on the given usage_info. + """ + # Users with staff access bypass the Visibility check. + if usage_info.has_staff_access: + return + + block_structure.remove_block_if( + lambda block_key: self.get_visible_to_staff_only(block_structure, block_key) + ) From dcdbd53e99bcd714064e5b47e4f594d9c4ef15d0 Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Wed, 28 Oct 2015 19:07:06 -0400 Subject: [PATCH 3/6] Transformer: StartDateTransformer --- .../course_blocks/transformers/start_date.py | 101 +++++++++++++++ .../transformers/tests/test_start_date.py | 118 ++++++++++++++++++ 2 files changed, 219 insertions(+) create mode 100644 lms/djangoapps/course_blocks/transformers/start_date.py create mode 100644 lms/djangoapps/course_blocks/transformers/tests/test_start_date.py diff --git a/lms/djangoapps/course_blocks/transformers/start_date.py b/lms/djangoapps/course_blocks/transformers/start_date.py new file mode 100644 index 0000000000..2838931943 --- /dev/null +++ b/lms/djangoapps/course_blocks/transformers/start_date.py @@ -0,0 +1,101 @@ +""" +Start Date Transformer implementation. +""" +from openedx.core.lib.block_cache.transformer import BlockStructureTransformer +from lms.djangoapps.courseware.access_utils import check_start_date +from xmodule.course_metadata_utils import DEFAULT_START_DATE + +from .utils import get_field_on_block + + +class StartDateTransformer(BlockStructureTransformer): + """ + A transformer that enforces the 'start' and 'days_early_for_beta' + fields on blocks by removing blocks from the block structure for + which the user does not have access. The 'start' field on a + block is percolated down to its descendants, so that all blocks + enforce the 'start' field from their ancestors. The assumed + 'start' value for a block is then the maximum of its parent and its + own. + + For a block with multiple parents, the assumed parent start date + value is a computed minimum of the start dates of all its parents. + So as long as one parent chain allows access, the block has access. + + Staff users are exempted from visibility rules. + """ + VERSION = 1 + MERGED_START_DATE = 'merged_start_date' + + @classmethod + def name(cls): + """ + Unique identifier for the transformer's class; + same identifier used in setup.py. + """ + return "start_date" + + @classmethod + def get_merged_start_date(cls, block_structure, block_key): + """ + Returns the merged value for the start date for the block with + the given block_key in the given block_structure. + """ + return block_structure.get_transformer_block_field( + block_key, cls, cls.MERGED_START_DATE, False + ) + + @classmethod + def collect(cls, block_structure): + """ + Collects any information that's necessary to execute this + transformer's transform method. + """ + block_structure.request_xblock_fields('days_early_for_beta') + + for block_key in block_structure.topological_traversal(): + + # compute merged value of start date from all parents + parents = block_structure.get_parents(block_key) + min_all_parents_start_date = min( + cls.get_merged_start_date(block_structure, parent_key) + for parent_key in parents + ) if parents else None + + # set the merged value for this block + block_start = get_field_on_block(block_structure.get_xblock(block_key), 'start') + if min_all_parents_start_date is None: + # no parents so just use value on block or default + merged_start_value = block_start or DEFAULT_START_DATE + + elif not block_start: + # no value on this block so take value from parents + merged_start_value = min_all_parents_start_date + + else: + # max of merged-start-from-all-parents and this block + merged_start_value = max(min_all_parents_start_date, block_start) + + block_structure.set_transformer_block_field( + block_key, + cls, + cls.MERGED_START_DATE, + merged_start_value + ) + + def transform(self, usage_info, block_structure): + """ + Mutates block_structure based on the given usage_info. + """ + # Users with staff access bypass the Start Date check. + if usage_info.has_staff_access: + return + + block_structure.remove_block_if( + lambda block_key: not check_start_date( + usage_info.user, + block_structure.get_xblock_field(block_key, 'days_early_for_beta'), + self.get_merged_start_date(block_structure, block_key), + usage_info.course_key, + ) + ) diff --git a/lms/djangoapps/course_blocks/transformers/tests/test_start_date.py b/lms/djangoapps/course_blocks/transformers/tests/test_start_date.py new file mode 100644 index 0000000000..88d1d744d9 --- /dev/null +++ b/lms/djangoapps/course_blocks/transformers/tests/test_start_date.py @@ -0,0 +1,118 @@ +""" +Tests for StartDateTransformer. +""" +import ddt +from datetime import timedelta +from django.utils.timezone import now +from mock import patch + +from courseware.tests.factories import BetaTesterFactory +from ..start_date import StartDateTransformer, DEFAULT_START_DATE +from .test_helpers import BlockParentsMapTestCase, update_block + + +@ddt.ddt +class StartDateTransformerTestCase(BlockParentsMapTestCase): + """ + StartDateTransformer Test + """ + STUDENT = 1 + BETA_USER = 2 + + class StartDateType(object): + """ + Use constant enum types for deterministic ddt test method names (rather than dynamically generated timestamps) + """ + released = 1, + future = 2, + default = 3 + + TODAY = now() + LAST_MONTH = TODAY - timedelta(days=30) + NEXT_MONTH = TODAY + timedelta(days=30) + + @classmethod + def start(cls, enum_value): + """ + Returns a start date for the given enum value + """ + if enum_value == cls.released: + return cls.LAST_MONTH + elif enum_value == cls.future: + return cls.NEXT_MONTH + else: + return DEFAULT_START_DATE + + def setUp(self, **kwargs): + super(StartDateTransformerTestCase, self).setUp(**kwargs) + self.beta_user = BetaTesterFactory(course_key=self.course.id, username='beta_tester', password=self.password) + course = self.get_block(0) + course.days_early_for_beta = 33 + update_block(course) + + # Following test cases are based on BlockParentsMapTestCase.parents_map: + # 0 + # / \ + # 1 2 + # / \ / \ + # 3 4 / 5 + # \ / + # 6 + @patch.dict('django.conf.settings.FEATURES', {'DISABLE_START_DATES': False}) + @ddt.data( + (STUDENT, {}, {}, {}), + (STUDENT, {0: StartDateType.default}, {}, {}), + (STUDENT, {0: StartDateType.future}, {}, {}), + (STUDENT, {0: StartDateType.released}, {0, 1, 2, 3, 4, 5, 6}, {}), + + # has_access checks on block directly and doesn't follow negative access set on parent/ancestor (i.e., 0) + (STUDENT, {1: StartDateType.released}, {}, {1, 3, 4, 6}), + (STUDENT, {2: StartDateType.released}, {}, {2, 5, 6}), + (STUDENT, {1: StartDateType.released, 2: StartDateType.released}, {}, {1, 2, 3, 4, 5, 6}), + + # DAG conflicts: has_access relies on field inheritance so it takes only the value from the first parent-chain + (STUDENT, {0: StartDateType.released, 4: StartDateType.future}, {0, 1, 2, 3, 5, 6}, {6}), + ( + STUDENT, + {0: StartDateType.released, 2: StartDateType.released, 4: StartDateType.future}, + {0, 1, 2, 3, 5, 6}, + {6}, + ), + (STUDENT, {0: StartDateType.released, 2: StartDateType.future, 4: StartDateType.released}, {0, 1, 3, 4, 6}, {}), + + # beta user cases + (BETA_USER, {}, {}, {}), + (BETA_USER, {0: StartDateType.default}, {}, {}), + (BETA_USER, {0: StartDateType.future}, {0, 1, 2, 3, 4, 5, 6}, {}), + (BETA_USER, {0: StartDateType.released}, {0, 1, 2, 3, 4, 5, 6}, {}), + + ( + BETA_USER, + {0: StartDateType.released, 2: StartDateType.default, 5: StartDateType.future}, + {0, 1, 3, 4, 6}, + {5}, + ), + (BETA_USER, {1: StartDateType.released, 2: StartDateType.default}, {}, {1, 3, 4, 6}), + (BETA_USER, {0: StartDateType.released, 4: StartDateType.future}, {0, 1, 2, 3, 4, 5, 6}, {}), + (BETA_USER, {0: StartDateType.released, 4: StartDateType.default}, {0, 1, 2, 3, 5, 6}, {6}), + ) + @ddt.unpack + # pylint: disable=invalid-name + def test_block_start_date( + self, + user_type, + start_date_type_values, + expected_student_visible_blocks, + blocks_with_differing_student_access + ): + for idx, start_date_type in start_date_type_values.iteritems(): + block = self.get_block(idx) + block.start = self.StartDateType.start(start_date_type) + update_block(block) + + self.assert_transform_results( + self.beta_user if user_type == self.BETA_USER else self.student, + expected_student_visible_blocks, + blocks_with_differing_student_access, + [StartDateTransformer()], + ) From a1c7c5f7e94f5ceb61b1dbe087226c6f2ffcac05 Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Wed, 28 Oct 2015 19:08:28 -0400 Subject: [PATCH 4/6] Transformer: UserPartitionTransformer --- .../tests/test_user_partitions.py | 233 ++++++++++++++++ .../transformers/user_partitions.py | 263 ++++++++++++++++++ 2 files changed, 496 insertions(+) create mode 100644 lms/djangoapps/course_blocks/transformers/tests/test_user_partitions.py create mode 100644 lms/djangoapps/course_blocks/transformers/user_partitions.py diff --git a/lms/djangoapps/course_blocks/transformers/tests/test_user_partitions.py b/lms/djangoapps/course_blocks/transformers/tests/test_user_partitions.py new file mode 100644 index 0000000000..799a1c29a0 --- /dev/null +++ b/lms/djangoapps/course_blocks/transformers/tests/test_user_partitions.py @@ -0,0 +1,233 @@ +# pylint: disable=attribute-defined-outside-init, protected-access +""" +Tests for UserPartitionTransformer. +""" +import ddt + +from openedx.core.djangoapps.course_groups.partition_scheme import CohortPartitionScheme +from openedx.core.djangoapps.course_groups.tests.helpers import CohortFactory, config_course_cohorts +from openedx.core.djangoapps.course_groups.cohorts import add_user_to_cohort +from openedx.core.djangoapps.course_groups.views import link_cohort_to_partition_group +from student.tests.factories import CourseEnrollmentFactory +from xmodule.partitions.partitions import Group, UserPartition + +from ...api import get_course_blocks +from ..user_partitions import UserPartitionTransformer, _MergedGroupAccess +from .test_helpers import CourseStructureTestCase + + +class UserPartitionTestMixin(object): + """ + Helper Mixin for testing user partitions. + """ + def setup_groups_partitions(self, num_user_partitions=1, num_groups=4): + """ + Sets up groups and user partitions for testing. + """ + # Set up groups + self.groups = [] + for group_num in range(1, num_groups + 1): + self.groups.append(Group(group_num, 'Group ' + unicode(group_num))) + + # Set up user partitions + self.user_partitions = [] + for user_partition_num in range(1, num_user_partitions + 1): + user_partition = UserPartition( + id=user_partition_num, + name='Partition ' + unicode(user_partition_num), + description='This is partition ' + unicode(user_partition_num), + groups=self.groups, + scheme=CohortPartitionScheme + ) + user_partition.scheme.name = "cohort" + self.user_partitions.append(user_partition) + + def setup_chorts(self, course): + """ + Sets up a cohort for each previously created user partition. + """ + for user_partition in self.user_partitions: + config_course_cohorts(course, is_cohorted=True) + self.cohorts = [] + for group in self.groups: + cohort = CohortFactory(course_id=course.id) + self.cohorts.append(cohort) + link_cohort_to_partition_group( + cohort, + user_partition.id, + group.id, + ) + + +@ddt.ddt +class UserPartitionTransformerTestCase(UserPartitionTestMixin, CourseStructureTestCase): + """ + UserPartitionTransformer Test + """ + def setUp(self): + """ + Setup course structure and create user for user partition + transformer test. + """ + super(UserPartitionTransformerTestCase, self).setUp() + + # Set up user partitions and groups. + self.setup_groups_partitions() + self.user_partition = self.user_partitions[0] + + # Build course. + self.course_hierarchy = self.get_course_hierarchy() + self.blocks = self.build_course(self.course_hierarchy) + self.course = self.blocks['course'] + + # Enroll user in course. + CourseEnrollmentFactory.create(user=self.user, course_id=self.course.id, is_active=True) + + # Set up cohorts. + self.setup_chorts(self.course) + + self.transformer = UserPartitionTransformer() + + def get_course_hierarchy(self): + """ + Returns a course hierarchy to test with. + """ + # course + # / \ + # / \ + # A[1, 2, 3] B + # / | \ | + # / | \ | + # / | \ | + # C[1, 2] D[2, 3] E / + # / | \ | / \ / + # / | \ | / \ / + # / | \ | / \ / + # F G[1] H[2] I J K[4] / + # / \ / / \ / + # / \ / / \ / + # / \ / / \/ + # L[1, 2] M[1, 2, 3] N O + # + return [ + { + 'org': 'UserPartitionTransformer', + 'course': 'UP101F', + 'run': 'test_run', + 'user_partitions': [self.user_partition], + '#type': 'course', + '#ref': 'course', + '#children': [ + { + '#type': 'vertical', + '#ref': 'A', + 'metadata': {'group_access': {self.user_partition.id: [0, 1, 2, 3]}}, + }, + {'#type': 'vertical', '#ref': 'B'}, + ], + }, + { + '#type': 'vertical', + '#ref': 'C', + '#parents': ['A'], + 'metadata': {'group_access': {self.user_partition.id: [1, 2]}}, + '#children': [ + {'#type': 'vertical', '#ref': 'F'}, + { + '#type': 'vertical', + '#ref': 'G', + 'metadata': {'group_access': {self.user_partition.id: [1]}}, + }, + { + '#type': 'vertical', + '#ref': 'H', + 'metadata': {'group_access': {self.user_partition.id: [2]}}, + }, + ], + }, + { + '#type': 'vertical', + '#ref': 'D', + '#parents': ['A'], + 'metadata': {'group_access': {self.user_partition.id: [2, 3]}}, + '#children': [{'#type': 'vertical', '#ref': 'I'}], + }, + { + '#type': 'vertical', + '#ref': 'E', + '#parents': ['A'], + '#children': [{'#type': 'vertical', '#ref': 'J'}], + }, + { + '#type': 'vertical', + '#ref': 'K', + '#parents': ['E'], + 'metadata': {'group_access': {self.user_partition.id: [4]}}, + '#children': [{'#type': 'vertical', '#ref': 'N'}], + }, + { + '#type': 'vertical', + '#ref': 'L', + '#parents': ['G'], + 'metadata': {'group_access': {self.user_partition.id: [1, 2]}}, + }, + { + '#type': 'vertical', + '#ref': 'M', + '#parents': ['G', 'H'], + 'metadata': {'group_access': {self.user_partition.id: [1, 2, 3]}}, + }, + { + '#type': 'vertical', + '#ref': 'O', + '#parents': ['K', 'B'], + }, + ] + + @ddt.data( + (None, ('course', 'B', 'O')), + (1, ('course', 'A', 'B', 'C', 'E', 'F', 'G', 'J', 'L', 'M', 'O')), + (2, ('course', 'A', 'B', 'C', 'D', 'E', 'F', 'H', 'I', 'J', 'M', 'O')), + (3, ('course', 'A', 'B', 'D', 'E', 'I', 'J', 'O')), + (4, ('course', 'B', 'O')), + ) + @ddt.unpack + def test_transform(self, group_id, expected_blocks): + if group_id: + add_user_to_cohort(self.cohorts[group_id - 1], self.user.username) + + trans_block_structure = get_course_blocks( + self.user, + self.course.location, + transformers={self.transformer} + ) + self.assertSetEqual( + set(trans_block_structure.get_block_keys()), + self.get_block_key_set(self.blocks, *expected_blocks) + ) + + +@ddt.ddt +class MergedGroupAccessTestCase(UserPartitionTestMixin, CourseStructureTestCase): + """ + _MergedGroupAccess Test + """ + # TODO Test Merged Group Access (MA-1624) + + @ddt.data( + ([None], None), + ([{1}, None], {1}), + ([None, {1}], {1}), + ([None, {1}, {1, 2}], {1}), + ([None, {1, 2}, {1, 2}], {1, 2}), + ([{1, 2, 3}, {1, 2}, None], {1, 2}), + ([{1, 2}, {1, 2, 3, 4}, None], {1, 2}), + ([{1}, {2}, None], set()), + ([None, {1}, {2}, None], set()), + ) + @ddt.unpack + def test_intersection_method(self, input_value, expected_result): + self.assertEquals( + _MergedGroupAccess._intersection(*input_value), + expected_result, + ) diff --git a/lms/djangoapps/course_blocks/transformers/user_partitions.py b/lms/djangoapps/course_blocks/transformers/user_partitions.py new file mode 100644 index 0000000000..337dc0e51a --- /dev/null +++ b/lms/djangoapps/course_blocks/transformers/user_partitions.py @@ -0,0 +1,263 @@ +""" +User Partitions Transformer +""" +from openedx.core.lib.block_cache.transformer import BlockStructureTransformer + +from .split_test import SplitTestTransformer +from .utils import get_field_on_block + + +class UserPartitionTransformer(BlockStructureTransformer): + """ + A transformer that enforces the group access rules on course blocks, + by honoring their user_partitions and group_access fields, and + removing all blocks in the block structure to which the user does + not have group access. + + Staff users are *not* exempted from user partition pathways. + """ + VERSION = 1 + + @classmethod + def name(cls): + """ + Unique identifier for the transformer's class; + same identifier used in setup.py. + """ + return "user_partitions" + + @classmethod + def collect(cls, block_structure): + """ + Computes any information for each XBlock that's necessary to + execute this transformer's transform method. + + Arguments: + block_structure (BlockStructureCollectedData) + """ + # First have the split test transformer setup its group access + # data for each block. + SplitTestTransformer.collect(block_structure) + + # Because user partitions are course-wide, only store data for + # them on the root block. + root_block = block_structure.get_xblock(block_structure.root_block_usage_key) + user_partitions = getattr(root_block, 'user_partitions', []) or [] + block_structure.set_transformer_data(cls, 'user_partitions', user_partitions) + + # If there are no user partitions, this transformation is a + # no-op, so there is nothing to collect. + if not user_partitions: + return + + # For each block, compute merged group access. Because this is a + # topological sort, we know a block's parents are guaranteed to + # already have merged group access computed before the block + # itself. + for block_key in block_structure.topological_traversal(): + xblock = block_structure.get_xblock(block_key) + parent_keys = block_structure.get_parents(block_key) + merged_parent_access_list = [ + block_structure.get_transformer_block_field(parent_key, cls, 'merged_group_access') + for parent_key in parent_keys + ] + merged_group_access = _MergedGroupAccess(user_partitions, xblock, merged_parent_access_list) + block_structure.set_transformer_block_field(block_key, cls, 'merged_group_access', merged_group_access) + + def transform(self, usage_info, block_structure): + """ + Mutates block_structure and block_data based on the given + usage_info. + + Arguments: + usage_info (object) + block_structure (BlockStructureCollectedData) + """ + SplitTestTransformer().transform(usage_info, block_structure) + + user_partitions = block_structure.get_transformer_data(self, 'user_partitions') + + if not user_partitions: + return + + user_groups = _get_user_partition_groups( + usage_info.course_key, user_partitions, usage_info.user + ) + block_structure.remove_block_if( + lambda block_key: not block_structure.get_transformer_block_field( + block_key, self, 'merged_group_access' + ).check_group_access(user_groups) + ) + + +class _MergedGroupAccess(object): + """ + A class object to represent the computed access value for a block, + merged from the inherited values from its ancestors. + + Note: The implementation assumes that the block structure is + topologically traversed so that all parents' merged accesses are + computed before a block's. + + How group access restrictions are represented within an XBlock: + - group_access not defined + => No group access restrictions. + - For each partition: + - partition.id not in group_access + => All groups have access for this partition + - group_access[partition_id] is None + => All groups have access for this partition + - group_access[partition_id] == [] + => All groups have access for this partition + - group_access[partition_id] == [group1..groupN] + => groups 1..N have access for this partition + + We internally represent the restrictions in a simplified way: + - self._access == {} + => No group access restrictions. + - For each partition: + - partition.id not in _access + => All groups have access for this partition + - _access[partition_id] == set() + => No groups have access for this partition + - _access[partition_id] == set(group1..groupN) + => groups 1..N have access for this partition + + Note that a user must have access to all partitions in group_access + or _access in order to access a block. + """ + def __init__(self, user_partitions, xblock, merged_parent_access_list): + """ + Arguments: + user_partitions (list[UserPartition]) + xblock (XBlock) + merged_parent_access_list (list[_MergedGroupAccess]) + """ + + # { partition.id: set(IDs of groups that can access partition) } + # If partition id is absent in this dict, no group access + # restrictions exist for that partition. + self._access = {} + + # Get the group_access value that is directly set on the xblock. + # Do not get the inherited value since field inheritance doesn't + # take a union of them for DAGs. + xblock_group_access = get_field_on_block(xblock, 'group_access', default_value={}) + + for partition in user_partitions: + # Running list of all groups that have access to this + # block, computed as a "union" from all parent chains. + # + # Set the default to universal access, for the case when + # there are no parents. + merged_parent_group_ids = None + + if merged_parent_access_list: + # Set the default to most restrictive as we iterate + # through all the parent chains. + merged_parent_group_ids = set() + + # Loop through parent_access from each parent-chain + for merged_parent_access in merged_parent_access_list: + # pylint: disable=protected-access + if partition.id in merged_parent_access._access: + # Since this parent has group access restrictions, + # merge it with the running list of + # parent-introduced restrictions. + merged_parent_group_ids.update(merged_parent_access._access[partition.id]) + else: + # Since at least one parent chain has no group + # access restrictions for this partition, allow + # unfettered group access or this partition. + merged_parent_group_ids = None + break + + # Group access for this partition as stored on the xblock + xblock_partition_access = set(xblock_group_access.get(partition.id, [])) or None + + # Compute this block's access by intersecting the block's + # own access with the merged access from its parent chains. + merged_group_ids = _MergedGroupAccess._intersection(xblock_partition_access, merged_parent_group_ids) + + # Add this partition's access only if group restrictions + # exist. + if merged_group_ids is not None: + self._access[partition.id] = merged_group_ids + + @staticmethod + def _intersection(*sets): + """ + Compute an intersection of sets, interpreting None as the + Universe set. + + This makes __init__ a bit more elegant. + + Arguments: + sets (list[set or None]), where None represents the Universe + set. + + Returns: + set or None, where None represents the Universe set. + """ + non_universe_sets = [set_ for set_ in sets if set_ is not None] + if non_universe_sets: + first, rest = non_universe_sets[0], non_universe_sets[1:] + return first.intersection(*rest) + else: + return None + + def check_group_access(self, user_groups): + """ + Arguments: + dict[int: Group]: Given a user, a mapping from user + partition IDs to the group to which the user belongs in + each partition. + + Returns: + bool: Whether said user has group access. + """ + for partition_id, allowed_group_ids in self._access.iteritems(): + + # If the user is not assigned to a group for this partition, + # deny access. + if partition_id not in user_groups: + return False + + # If the user belongs to one of the allowed groups for this + # partition, then move and check the next partition. + elif user_groups[partition_id].id in allowed_group_ids: + continue + + # Else, deny access. + else: + return False + + # The user has access for every partition, grant access. + return True + + +def _get_user_partition_groups(course_key, user_partitions, user): + """ + Collect group ID for each partition in this course for this user. + + Arguments: + course_key (CourseKey) + user_partitions (list[UserPartition]) + user (User) + + Returns: + dict[int: Group]: Mapping from user partitions to the group to + which the user belongs in each partition. If the user isn't + in a group for a particular partition, then that partition's + ID will not be in the dict. + """ + partition_groups = {} + for partition in user_partitions: + group = partition.scheme.get_group_for_user( + course_key, + user, + partition, + ) + if group is not None: + partition_groups[partition.id] = group + return partition_groups From d1674ca85fb4003cfd2db65a1f26c14bd21d182e Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Wed, 28 Oct 2015 19:08:48 -0400 Subject: [PATCH 5/6] Transformer: SplitTestTransformer --- .../course_blocks/transformers/split_test.py | 81 +++++++ .../transformers/tests/test_split_test.py | 226 ++++++++++++++++++ 2 files changed, 307 insertions(+) create mode 100644 lms/djangoapps/course_blocks/transformers/split_test.py create mode 100644 lms/djangoapps/course_blocks/transformers/tests/test_split_test.py diff --git a/lms/djangoapps/course_blocks/transformers/split_test.py b/lms/djangoapps/course_blocks/transformers/split_test.py new file mode 100644 index 0000000000..85e0a15561 --- /dev/null +++ b/lms/djangoapps/course_blocks/transformers/split_test.py @@ -0,0 +1,81 @@ +""" +Split Test Block Transformer +""" +from openedx.core.lib.block_cache.transformer import BlockStructureTransformer + + +class SplitTestTransformer(BlockStructureTransformer): + """ + A nested transformer of the UserPartitionTransformer that honors the + block structure pathways created by split_test modules. + + To avoid code duplication, the implementation transforms its block + access representation to the representation used by user_partitions. + Namely, the 'group_id_to_child' field on a split_test module is + transformed into the, now standard, 'group_access' fields in the + split_test module's children. + + The implementation therefore relies on the UserPartitionTransformer + to actually enforce the access using the 'user_partitions' and + 'group_access' fields. + """ + VERSION = 1 + + @classmethod + def name(cls): + """ + Unique identifier for the transformer's class; + same identifier used in setup.py. + """ + return "split_test" + + @classmethod + def collect(cls, block_structure): + """ + Collects any information that's necessary to execute this + transformer's transform method. + """ + + root_block = block_structure.get_xblock(block_structure.root_block_usage_key) + user_partitions = getattr(root_block, 'user_partitions', []) + + for block_key in block_structure.topological_traversal( + filter_func=lambda block_key: block_key.block_type == 'split_test', + yield_descendants_of_unyielded=True, + ): + xblock = block_structure.get_xblock(block_key) + partition_for_this_block = next( + ( + partition for partition in user_partitions + if partition.id == xblock.user_partition_id + ), + None + ) + if not partition_for_this_block: + continue + + # Create dict of child location to group_id, using the + # group_id_to_child field on the split_test module. + child_to_group = { + xblock.group_id_to_child.get(unicode(group.id), None): group.id + for group in partition_for_this_block.groups + } + + # Set group access for each child using its group_access + # field so the user partitions transformer enforces it. + for child_location in xblock.children: + child = block_structure.get_xblock(child_location) + group = child_to_group.get(child_location, None) + child.group_access[partition_for_this_block.id] = [group] if group else [] + + def transform(self, usage_info, block_structure): # pylint: disable=unused-argument + """ + Mutates block_structure based on the given usage_info. + """ + + # The UserPartitionTransformer will enforce group access, so + # go ahead and remove all extraneous split_test modules. + block_structure.remove_block_if( + lambda block_key: block_key.block_type == 'split_test', + keep_descendants=True, + ) diff --git a/lms/djangoapps/course_blocks/transformers/tests/test_split_test.py b/lms/djangoapps/course_blocks/transformers/tests/test_split_test.py new file mode 100644 index 0000000000..f211047d1b --- /dev/null +++ b/lms/djangoapps/course_blocks/transformers/tests/test_split_test.py @@ -0,0 +1,226 @@ +""" +Tests for SplitTestTransformer. +""" +import ddt + +import openedx.core.djangoapps.user_api.course_tag.api as course_tag_api +from openedx.core.djangoapps.user_api.partition_schemes import RandomUserPartitionScheme +from student.tests.factories import CourseEnrollmentFactory +from xmodule.partitions.partitions import Group, UserPartition +from xmodule.modulestore.tests.factories import check_mongo_calls, check_mongo_calls_range + +from ...api import get_course_blocks +from ..user_partitions import UserPartitionTransformer, _get_user_partition_groups +from .test_helpers import CourseStructureTestCase, create_location + + +@ddt.ddt +class SplitTestTransformerTestCase(CourseStructureTestCase): + """ + SplitTestTransformer Test + """ + TEST_PARTITION_ID = 0 + + def setUp(self): + """ + Setup course structure and create user for split test transformer test. + """ + super(SplitTestTransformerTestCase, self).setUp() + + # Set up user partitions and groups. + self.groups = [Group(1, 'Group 1'), Group(2, 'Group 2'), Group(3, 'Group 3')] + self.split_test_user_partition_id = self.TEST_PARTITION_ID + self.split_test_user_partition = UserPartition( + id=self.split_test_user_partition_id, + name='Split Partition', + description='This is split partition', + groups=self.groups, + scheme=RandomUserPartitionScheme + ) + self.split_test_user_partition.scheme.name = "random" + + # Build course. + self.course_hierarchy = self.get_course_hierarchy() + self.blocks = self.build_course(self.course_hierarchy) + self.course = self.blocks['course'] + + # Enroll user in course. + CourseEnrollmentFactory.create(user=self.user, course_id=self.course.id, is_active=True) + + self.transformer = UserPartitionTransformer() + + def get_course_hierarchy(self): + """ + Get a course hierarchy to test with. + + Assumes self.split_test_user_partition has already been initialized. + + Returns: dict[course_structure] + """ + + org_name = 'SplitTestTransformer' + course_name = 'ST101F' + run_name = 'test_run' + + def location(block_ref, block_type='vertical'): + """ + Returns the usage key for the given block_type and block reference string in the test course. + """ + return create_location( + org_name, course_name, run_name, block_type, self.create_block_id(block_type, block_ref) + ) + + # course + # / | \ + # / | \ + # A BSplit CSplit + # / \ / | \ | \ + # / \ / | \ | \ + # D E[1] F[2] G[3] H[1] I[2] + # / \ \ | + # / \ \ | + # J KSplit \ L + # / | \ / \ + # / | \ / \ + # M[2] N[3] O P + # + return [ + { + 'org': org_name, + 'course': course_name, + 'run': run_name, + 'user_partitions': [self.split_test_user_partition], + '#type': 'course', + '#ref': 'course', + }, + { + '#type': 'vertical', + '#ref': 'A', + '#children': [{'#type': 'vertical', '#ref': 'D'}], + }, + { + '#type': 'split_test', + '#ref': 'BSplit', + 'metadata': {'category': 'split_test'}, + 'user_partition_id': self.TEST_PARTITION_ID, + 'group_id_to_child': { + '1': location('E'), + '2': location('F'), + '3': location('G'), + }, + '#children': [{'#type': 'vertical', '#ref': 'G'}], + }, + { + '#type': 'vertical', + '#ref': 'E', + '#parents': ['A', 'BSplit'], + }, + { + '#type': 'vertical', + '#ref': 'F', + '#parents': ['BSplit'], + '#children': [ + {'#type': 'vertical', '#ref': 'J'}, + ], + }, + { + '#type': 'split_test', + '#ref': 'KSplit', + 'metadata': {'category': 'split_test'}, + 'user_partition_id': self.TEST_PARTITION_ID, + 'group_id_to_child': { + '2': location('M'), + '3': location('N'), + }, + '#parents': ['F'], + '#children': [ + {'#type': 'vertical', '#ref': 'M'}, + {'#type': 'vertical', '#ref': 'N'}, + ], + }, + { + '#type': 'split_test', + '#ref': 'CSplit', + 'metadata': {'category': 'split_test'}, + 'user_partition_id': self.TEST_PARTITION_ID, + 'group_id_to_child': { + '1': location('H'), + '2': location('I'), + }, + '#children': [ + {'#type': 'vertical', '#ref': 'I'}, + { + '#type': 'vertical', + '#ref': 'H', + '#children': [ + { + '#type': 'vertical', + '#ref': 'L', + '#children': [{'#type': 'vertical', '#ref': 'P'}], + }, + ], + }, + ], + }, + { + '#type': 'vertical', + '#ref': 'O', + '#parents': ['G', 'L'], + }, + ] + + @ddt.data( + # Note: Theoretically, block E should be accessible by users + # not in Group 1, since there's an open path through block A. + # Since the split_test transformer automatically sets the block + # access on its children, it bypasses the paths via other + # parents. However, we don't think this is a use case we need to + # support for split_test components (since they are now deprecated + # in favor of content groups and user partitions). + (1, ('course', 'A', 'D', 'E', 'H', 'L', 'O', 'P',)), + (2, ('course', 'A', 'D', 'F', 'J', 'M', 'I',)), + (3, ('course', 'A', 'D', 'G', 'O',)), + ) + @ddt.unpack + def test_user(self, group_id, expected_blocks): + course_tag_api.set_course_tag( + self.user, + self.course.id, + RandomUserPartitionScheme.key_for_partition(self.split_test_user_partition), + group_id, + ) + + block_structure1 = get_course_blocks( + self.user, + self.course.location, + transformers={self.transformer}, + ) + self.assertEqual( + set(block_structure1.get_block_keys()), + set(self.get_block_key_set(self.blocks, *expected_blocks)), + ) + + def test_user_randomly_assigned(self): + # user was randomly assigned to one of the groups + user_groups = _get_user_partition_groups( # pylint: disable=protected-access + self.course.id, [self.split_test_user_partition], self.user + ) + self.assertEquals(len(user_groups), 1) + + # calling twice should result in the same block set + with check_mongo_calls_range(min_finds=1): + block_structure1 = get_course_blocks( + self.user, + self.course.location, + transformers={self.transformer}, + ) + with check_mongo_calls(0): + block_structure2 = get_course_blocks( + self.user, + self.course.location, + transformers={self.transformer}, + ) + self.assertEqual( + set(block_structure1.get_block_keys()), + set(block_structure2.get_block_keys()), + ) From a78b94d83a6f6453c5bf6e1dc71dda826e8ca4fd Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Wed, 28 Oct 2015 19:09:39 -0400 Subject: [PATCH 6/6] Transformer: ContentLibraryTransformer --- .../xmodule/xmodule/library_content_module.py | 170 ++++++++++++----- .../transformers/library_content.py | 177 ++++++++++++++++++ .../tests/test_library_content.py | 165 ++++++++++++++++ 3 files changed, 464 insertions(+), 48 deletions(-) create mode 100644 lms/djangoapps/course_blocks/transformers/library_content.py create mode 100644 lms/djangoapps/course_blocks/transformers/tests/test_library_content.py diff --git a/common/lib/xmodule/xmodule/library_content_module.py b/common/lib/xmodule/xmodule/library_content_module.py index 033f34b069..97c84da980 100644 --- a/common/lib/xmodule/xmodule/library_content_module.py +++ b/common/lib/xmodule/xmodule/library_content_module.py @@ -134,8 +134,65 @@ class LibraryContentModule(LibraryContentFields, XModule, StudioEditableModule): any particular student. """ + @classmethod + def make_selection(cls, selected, children, max_count, mode): + """ + Dynamically selects block_ids indicating which of the possible children are displayed to the current user. + + Arguments: + selected - list of (block_type, block_id) tuples assigned to this student + children - children of this block + max_count - number of components to display to each student + mode - how content is drawn from the library + + Returns: + A dict containing the following keys: + + 'selected' (set) of (block_type, block_id) tuples assigned to this student + 'invalid' (set) of dropped (block_type, block_id) tuples that are no longer valid + 'overlimit' (set) of dropped (block_type, block_id) tuples that were previously selected + 'added' (set) of newly added (block_type, block_id) tuples + """ + selected = set(tuple(k) for k in selected) # set of (block_type, block_id) tuples assigned to this student + + # Determine which of our children we will show: + valid_block_keys = set([(c.block_type, c.block_id) for c in children]) + # Remove any selected blocks that are no longer valid: + invalid_block_keys = (selected - valid_block_keys) + if invalid_block_keys: + selected -= invalid_block_keys + + # If max_count has been decreased, we may have to drop some previously selected blocks: + overlimit_block_keys = set() + while len(selected) > max_count: + overlimit_block_keys.add(selected.pop()) + + # Do we have enough blocks now? + num_to_add = max_count - len(selected) + + added_block_keys = None + if num_to_add > 0: + # We need to select [more] blocks to display to this user: + pool = valid_block_keys - selected + if mode == "random": + num_to_add = min(len(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. + else: + raise NotImplementedError("Unsupported mode.") + selected |= added_block_keys + + return { + 'selected': selected, + 'invalid': invalid_block_keys, + 'overlimit': overlimit_block_keys, + 'added': added_block_keys, + } + def _publish_event(self, event_name, result, **kwargs): - """ Helper method to publish an event for analytics purposes """ + """ + Helper method to publish an event for analytics purposes + """ event_data = { "location": unicode(self.location), "result": result, @@ -146,6 +203,61 @@ class LibraryContentModule(LibraryContentFields, XModule, StudioEditableModule): self.runtime.publish(self, "edx.librarycontentblock.content.{}".format(event_name), event_data) self._last_event_result_count = len(result) # pylint: disable=attribute-defined-outside-init + @classmethod + def publish_selected_children_events(cls, block_keys, format_block_keys, publish_event): + """ + Helper method for publishing events when children blocks are + selected/updated for a user. This helper is also used by + the ContentLibraryTransformer. + + Arguments: + + block_keys - + A dict describing which events to publish (add or + remove), see `make_selection` above for format details. + + format_block_keys - + A function to convert block keys to the format expected + by publish_event. Must have the signature: + + [(block_type, block_id)] -> T + + Where T is a collection of block keys as accepted by + `publish_event`. + + publish_event - + Function that handles the actual publishing. Must have + the signature: + + <'removed'|'assigned'> -> result:T -> removed:T -> reason:basestring -> None + + Where T is a collection of block_keys as returned by + `format_block_keys`. + """ + if block_keys['invalid']: + # reason "invalid" means deleted from library or a different library is now being used. + publish_event( + "removed", + result=format_block_keys(block_keys['selected']), + removed=format_block_keys(block_keys['invalid']), + reason="invalid" + ) + + if block_keys['overlimit']: + publish_event( + "removed", + result=format_block_keys(block_keys['selected']), + removed=format_block_keys(block_keys['overlimit']), + reason="overlimit" + ) + + if block_keys['added']: + publish_event( + "assigned", + result=format_block_keys(block_keys['selected']), + added=format_block_keys(block_keys['added']) + ) + def selected_children(self): """ Returns a set() of block_ids indicating which of the possible children @@ -161,61 +273,23 @@ class LibraryContentModule(LibraryContentFields, XModule, StudioEditableModule): # Already done: 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 + block_keys = self.make_selection(self.selected, self.children, self.max_count, "random") # pylint: disable=no-member + # Publish events for analytics purposes: lib_tools = self.runtime.service(self, 'library_tools') format_block_keys = lambda keys: lib_tools.create_block_analytics_summary(self.location.course_key, keys) + self.publish_selected_children_events( + block_keys, + format_block_keys, + self._publish_event, + ) - # Determine which of our children we will show: - 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: - 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. - self._publish_event( - "removed", - result=format_block_keys(selected), - removed=format_block_keys(invalid_block_keys), - reason="invalid" - ) - # 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: - overlimit_block_keys.add(selected.pop()) - if overlimit_block_keys: - # Publish an event for analytics purposes: - self._publish_event( - "removed", - result=format_block_keys(selected), - removed=format_block_keys(overlimit_block_keys), - reason="overlimit" - ) - # Do we have enough blocks now? - num_to_add = self.max_count - len(selected) - if num_to_add > 0: - added_block_keys = None - # We need to select [more] blocks to display to this user: - pool = valid_block_keys - selected - if self.mode == "random": - num_to_add = min(len(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. - else: - raise NotImplementedError("Unsupported mode.") - selected |= added_block_keys - if added_block_keys: - # Publish an event for analytics purposes: - self._publish_event( - "assigned", - result=format_block_keys(selected), - added=format_block_keys(added_block_keys) - ) # Save our selections to the user state, to ensure consistency: + selected = block_keys['selected'] self.selected = list(selected) # TODO: this doesn't save from the LMS "Progress" page. # Cache the results self._selected_set = selected # pylint: disable=attribute-defined-outside-init + return selected def _get_selected_child_blocks(self): diff --git a/lms/djangoapps/course_blocks/transformers/library_content.py b/lms/djangoapps/course_blocks/transformers/library_content.py new file mode 100644 index 0000000000..48a7ce74c1 --- /dev/null +++ b/lms/djangoapps/course_blocks/transformers/library_content.py @@ -0,0 +1,177 @@ +""" +Content Library Transformer. +""" +import json +from courseware.models import StudentModule +from openedx.core.lib.block_cache.transformer import BlockStructureTransformer +from xmodule.library_content_module import LibraryContentModule +from xmodule.modulestore.django import modulestore +from eventtracking import tracker + + +class ContentLibraryTransformer(BlockStructureTransformer): + """ + A transformer that manipulates the block structure by removing all + blocks within a library_content module to which a user should not + have access. + + Staff users are *not* exempted from library content pathways. + """ + VERSION = 1 + + @classmethod + def name(cls): + """ + Unique identifier for the transformer's class; + same identifier used in setup.py. + """ + return "library_content" + + @classmethod + def collect(cls, block_structure): + """ + Collects any information that's necessary to execute this + transformer's transform method. + """ + block_structure.request_xblock_fields('mode') + block_structure.request_xblock_fields('max_count') + block_structure.request_xblock_fields('category') + store = modulestore() + + # needed for analytics purposes + def summarize_block(usage_key): + """ Basic information about the given block """ + orig_key, orig_version = 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, + } + + # For each block check if block is library_content. + # If library_content add children array to content_library_children field + for block_key in block_structure.topological_traversal( + filter_func=lambda block_key: block_key.block_type == 'library_content', + yield_descendants_of_unyielded=True, + ): + xblock = block_structure.get_xblock(block_key) + for child_key in xblock.children: + summary = summarize_block(child_key) + block_structure.set_transformer_block_field(child_key, cls, 'block_analytics_summary', summary) + + def transform(self, usage_info, block_structure): + """ + Mutates block_structure based on the given usage_info. + """ + + all_library_children = set() + all_selected_children = set() + for block_key in block_structure.topological_traversal( + filter_func=lambda block_key: block_key.block_type == 'library_content', + yield_descendants_of_unyielded=True, + ): + library_children = block_structure.get_children(block_key) + if library_children: + all_library_children.update(library_children) + selected = [] + mode = block_structure.get_xblock_field(block_key, 'mode') + max_count = block_structure.get_xblock_field(block_key, 'max_count') + + # Retrieve "selected" json from LMS MySQL database. + module = self._get_student_module(usage_info.user, usage_info.course_key, block_key) + if module: + state_dict = json.loads(module.state) + # Add all selected entries for this user for this + # library module to the selected list. + for state in state_dict['selected']: + usage_key = usage_info.course_key.make_usage_key(state[0], state[1]) + if usage_key in library_children: + selected.append((state[0], state[1])) + + # update selected + previous_count = len(selected) + block_keys = LibraryContentModule.make_selection(selected, library_children, max_count, mode) + selected = block_keys['selected'] + + # publish events for analytics + self._publish_events(block_structure, block_key, previous_count, max_count, block_keys) + all_selected_children.update(usage_info.course_key.make_usage_key(s[0], s[1]) for s in selected) + + def check_child_removal(block_key): + """ + Return True if selected block should be removed. + + Block is removed if it is part of library_content, but has + not been selected for current user. + """ + if block_key not in all_library_children: + return False + if block_key in all_selected_children: + return False + return True + + # Check and remove all non-selected children from course + # structure. + block_structure.remove_block_if( + check_child_removal + ) + + @classmethod + def _get_student_module(cls, user, course_key, block_key): + """ + Get the student module for the given user for the given block. + + Arguments: + user (User) + course_key (CourseLocator) + block_key (BlockUsageLocator) + + Returns: + StudentModule if exists, or None. + """ + try: + return StudentModule.objects.get( + student=user, + course_id=course_key, + module_state_key=block_key, + state__contains='"selected": [[' + ) + except StudentModule.DoesNotExist: + return None + + @classmethod + def _publish_events(cls, block_structure, location, previous_count, max_count, block_keys): + """ + Helper method to publish events for analytics purposes + """ + + def format_block_keys(keys): + """ + Helper function to format block keys + """ + json_result = [] + for key in keys: + info = block_structure.get_transformer_block_field( + key, ContentLibraryTransformer, 'block_analytics_summary' + ) + json_result.append(info) + return json_result + + def publish_event(event_name, result, **kwargs): + """ + Helper function to publish an event for analytics purposes + """ + event_data = { + "location": unicode(location), + "previous_count": previous_count, + "result": result, + "max_count": max_count + } + event_data.update(kwargs) + tracker.emit("edx.librarycontentblock.content.{}".format(event_name), event_data) + + LibraryContentModule.publish_selected_children_events( + block_keys, + format_block_keys, + publish_event, + ) diff --git a/lms/djangoapps/course_blocks/transformers/tests/test_library_content.py b/lms/djangoapps/course_blocks/transformers/tests/test_library_content.py new file mode 100644 index 0000000000..b712ef33d4 --- /dev/null +++ b/lms/djangoapps/course_blocks/transformers/tests/test_library_content.py @@ -0,0 +1,165 @@ +""" +Tests for ContentLibraryTransformer. +""" +import mock +from student.tests.factories import CourseEnrollmentFactory + +from course_blocks.transformers.library_content import ContentLibraryTransformer +from course_blocks.api import get_course_blocks, clear_course_from_cache +from lms.djangoapps.course_blocks.transformers.tests.test_helpers import CourseStructureTestCase + + +class MockedModule(object): + """ + Object with mocked selected modules for user. + """ + def __init__(self, state): + """ + Set state attribute on initialize. + """ + self.state = state + + +class ContentLibraryTransformerTestCase(CourseStructureTestCase): + """ + ContentLibraryTransformer Test + """ + + def setUp(self): + """ + Setup course structure and create user for content library transformer test. + """ + super(ContentLibraryTransformerTestCase, self).setUp() + + # Build course. + self.course_hierarchy = self.get_course_hierarchy() + self.blocks = self.build_course(self.course_hierarchy) + self.course = self.blocks['course'] + clear_course_from_cache(self.course.id) + + # Enroll user in course. + CourseEnrollmentFactory.create(user=self.user, course_id=self.course.id, is_active=True) + + self.selected_module = MockedModule('{"selected": [["vertical", "vertical_vertical2"]]}') + self.transformer = ContentLibraryTransformer() + + def get_course_hierarchy(self): + """ + Get a course hierarchy to test with. + """ + return [{ + 'org': 'ContentLibraryTransformer', + 'course': 'CL101F', + 'run': 'test_run', + '#type': 'course', + '#ref': 'course', + '#children': [ + { + '#type': 'chapter', + '#ref': 'chapter1', + '#children': [ + { + '#type': 'sequential', + '#ref': 'lesson1', + '#children': [ + { + '#type': 'vertical', + '#ref': 'vertical1', + '#children': [ + { + 'metadata': {'category': 'library_content'}, + '#type': 'library_content', + '#ref': 'library_content1', + '#children': [ + { + 'metadata': {'display_name': "CL Vertical 2"}, + '#type': 'vertical', + '#ref': 'vertical2', + '#children': [ + { + 'metadata': {'display_name': "HTML1"}, + '#type': 'html', + '#ref': 'html1', + } + ] + }, + { + 'metadata': {'display_name': "CL Vertical 3"}, + '#type': 'vertical', + '#ref': 'vertical3', + '#children': [ + { + 'metadata': {'display_name': "HTML2"}, + '#type': 'html', + '#ref': 'html2', + } + ] + } + ] + } + ], + } + ], + } + ], + } + ] + }] + + def test_content_library(self): + """ + Test when course has content library section. + First test user can't see any content library section, + and after that mock response from MySQL db. + Check user can see mocked sections in content library. + """ + raw_block_structure = get_course_blocks( + self.user, + self.course.location, + transformers={} + ) + self.assertEqual(len(list(raw_block_structure.get_block_keys())), len(self.blocks)) + + clear_course_from_cache(self.course.id) + trans_block_structure = get_course_blocks( + self.user, + self.course.location, + transformers={self.transformer} + ) + + # Should dynamically assign a block to student + trans_keys = set(trans_block_structure.get_block_keys()) + block_key_set = self.get_block_key_set( + self.blocks, 'course', 'chapter1', 'lesson1', 'vertical1', 'library_content1' + ) + for key in block_key_set: + self.assertIn(key, trans_keys) + + vertical2_selected = self.get_block_key_set(self.blocks, 'vertical2').pop() in trans_keys + vertical3_selected = self.get_block_key_set(self.blocks, 'vertical3').pop() in trans_keys + self.assertTrue(vertical2_selected or vertical3_selected) + + # Check course structure again, with mocked selected modules for a user. + with mock.patch( + 'course_blocks.transformers.library_content.ContentLibraryTransformer._get_student_module', + return_value=self.selected_module + ): + clear_course_from_cache(self.course.id) + trans_block_structure = get_course_blocks( + self.user, + self.course.location, + transformers={self.transformer} + ) + self.assertEqual( + set(trans_block_structure.get_block_keys()), + self.get_block_key_set( + self.blocks, + 'course', + 'chapter1', + 'lesson1', + 'vertical1', + 'library_content1', + 'vertical2', + 'html1' + ) + )