From 521568937daeaab9e7a5954079f4fd20d8dd5ebf Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Fri, 13 Nov 2015 00:23:29 -0500 Subject: [PATCH] MA-1624 Unit tests for User Partition Transformer. --- .../tests/test_user_partitions.py | 219 +++++++++++++++++- .../transformers/user_partitions.py | 16 +- 2 files changed, 219 insertions(+), 16 deletions(-) diff --git a/lms/djangoapps/course_blocks/transformers/tests/test_user_partitions.py b/lms/djangoapps/course_blocks/transformers/tests/test_user_partitions.py index 799a1c29a0..c10bca4289 100644 --- a/lms/djangoapps/course_blocks/transformers/tests/test_user_partitions.py +++ b/lms/djangoapps/course_blocks/transformers/tests/test_user_partitions.py @@ -2,6 +2,7 @@ """ Tests for UserPartitionTransformer. """ +from collections import namedtuple import ddt from openedx.core.djangoapps.course_groups.partition_scheme import CohortPartitionScheme @@ -10,10 +11,11 @@ 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 xmodule.modulestore.tests.factories import CourseFactory from ...api import get_course_blocks from ..user_partitions import UserPartitionTransformer, _MergedGroupAccess -from .test_helpers import CourseStructureTestCase +from .test_helpers import CourseStructureTestCase, update_block class UserPartitionTestMixin(object): @@ -42,21 +44,23 @@ class UserPartitionTestMixin(object): user_partition.scheme.name = "cohort" self.user_partitions.append(user_partition) - def setup_chorts(self, course): + def setup_cohorts(self, course): """ Sets up a cohort for each previously created user partition. """ + config_course_cohorts(course, is_cohorted=True) + self.partition_cohorts = [] for user_partition in self.user_partitions: - config_course_cohorts(course, is_cohorted=True) - self.cohorts = [] + partition_cohorts = [] for group in self.groups: cohort = CohortFactory(course_id=course.id) - self.cohorts.append(cohort) + partition_cohorts.append(cohort) link_cohort_to_partition_group( cohort, user_partition.id, group.id, ) + self.partition_cohorts.append(partition_cohorts) @ddt.ddt @@ -84,7 +88,7 @@ class UserPartitionTransformerTestCase(UserPartitionTestMixin, CourseStructureTe CourseEnrollmentFactory.create(user=self.user, course_id=self.course.id, is_active=True) # Set up cohorts. - self.setup_chorts(self.course) + self.setup_cohorts(self.course) self.transformer = UserPartitionTransformer() @@ -194,7 +198,8 @@ class UserPartitionTransformerTestCase(UserPartitionTestMixin, CourseStructureTe @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) + cohort = self.partition_cohorts[self.user_partition.id - 1][group_id - 1] + add_user_to_cohort(cohort, self.user.username) trans_block_structure = get_course_blocks( self.user, @@ -208,11 +213,207 @@ class UserPartitionTransformerTestCase(UserPartitionTestMixin, CourseStructureTe @ddt.ddt -class MergedGroupAccessTestCase(UserPartitionTestMixin, CourseStructureTestCase): +class MergedGroupAccessTestData(UserPartitionTestMixin, CourseStructureTestCase): """ _MergedGroupAccess Test """ - # TODO Test Merged Group Access (MA-1624) + def setUp(self): + """ + Setup course structure and create user for user partition + transformer test. + """ + super(MergedGroupAccessTestData, self).setUp() + + # Set up multiple user partitions and groups. + self.setup_groups_partitions(num_user_partitions=3) + + self.course = CourseFactory.create(user_partitions=self.user_partitions) + CourseEnrollmentFactory.create(user=self.user, course_id=self.course.id, is_active=True) + + # Set up cohorts. + self.setup_cohorts(self.course) + + def get_course_hierarchy(self): + """ + Returns a course hierarchy to test with. + """ + # The block tree is as follows, with the numbers in the brackets + # specifying the group_id for each of the 3 partitions. + # A + # / | \ + # / | \ + # B C D + # [1][3][] [2][2][] [3][1][] + # \ / + # \ / + # E + # + return [ + { + 'org': 'MergedGroupAccess', + 'course': 'MGA101F', + 'run': 'test_run', + 'user_partitions': self.user_partitions, + '#type': 'course', + '#ref': 'A', + }, + { + '#type': 'vertical', + '#ref': 'B', + '#parents': ['A'], + 'metadata': {'group_access': {1: [1], 2:[3], 3:[]}}, + }, + { + '#type': 'vertical', + '#ref': 'C', + '#parents': ['A'], + 'metadata': {'group_access': {1: [2], 2:[2], 3:[]}}, + }, + { + '#type': 'vertical', + '#ref': 'D', + '#parents': ['A'], + 'metadata': {'group_access': {1: [3], 2:[1], 3:[]}}, + }, + { + '#type': 'vertical', + '#ref': 'E', + '#parents': ['B', 'C'], + }, + ] + + AccessTestData = namedtuple( + 'AccessTestData', + ['partition_groups', 'xblock_access', 'merged_parents_list', 'expected_access'], + ) + AccessTestData.__new__.__defaults__ = ({}, None, [], False) + + @ddt.data( + # universal access throughout + AccessTestData(expected_access=True), + AccessTestData(xblock_access={1: None}, expected_access=True), + AccessTestData(xblock_access={1: []}, expected_access=True), + + # partition 1 requiring membership in group 1 + AccessTestData(xblock_access={1: [1]}), + AccessTestData(partition_groups={2: 1, 3: 1}, xblock_access={1: [1]}), + AccessTestData(partition_groups={1: 1, 2: 1, 3: 1}, xblock_access={1: [1]}, expected_access=True), + AccessTestData(partition_groups={1: 1, 2: 1}, xblock_access={1: [1], 2: [], 3: []}, expected_access=True), + + # partitions 1 and 2 requiring membership in group 1 + AccessTestData(xblock_access={1: [1], 2: [1]}), + AccessTestData(partition_groups={2: 1, 3: 1}, xblock_access={1: [1], 2: [1]}), + AccessTestData(partition_groups={1: 1, 2: 1}, xblock_access={1: [1], 2: [1]}, expected_access=True), + + # partitions 1 and 2 requiring membership in different groups + AccessTestData(xblock_access={1: [1], 2: [2]}), + AccessTestData(partition_groups={2: 1, 3: 1}, xblock_access={1: [1], 2: [2]}), + AccessTestData(partition_groups={1: 1, 2: 1, 3: 1}, xblock_access={1: [1], 2: [2]}), + + AccessTestData(partition_groups={1: 1, 2: 2}, xblock_access={1: [1], 2: [2]}, expected_access=True), + + # partitions 1 and 2 requiring membership in list of groups + AccessTestData(partition_groups={1: 3, 2: 3}, xblock_access={1: [1, 2], 2: [1, 2]}), + + AccessTestData(partition_groups={1: 1, 2: 1}, xblock_access={1: [1, 2], 2: [1, 2]}, expected_access=True), + AccessTestData(partition_groups={1: 1, 2: 2}, xblock_access={1: [1, 2], 2: [1, 2]}, expected_access=True), + AccessTestData(partition_groups={1: 2, 2: 1}, xblock_access={1: [1, 2], 2: [1, 2]}, expected_access=True), + AccessTestData(partition_groups={1: 2, 2: 2}, xblock_access={1: [1, 2], 2: [1, 2]}, expected_access=True), + + # parent inheritance + # 1 parent allows + AccessTestData(partition_groups={1: 1, 2: 2}, merged_parents_list=[{1: {1}}], expected_access=True), + + # 2 parents allow + AccessTestData(partition_groups={1: 1, 2: 2}, merged_parents_list=[{1: {1}}, {1: {1}}], expected_access=True), + AccessTestData(partition_groups={1: 1, 2: 2}, merged_parents_list=[{1: {2}}, {1: {1}}], expected_access=True), + AccessTestData( + partition_groups={1: 1, 2: 2}, + merged_parents_list=[{1: {2}, 2: {2}}, {1: {1}, 2: {1}}], + expected_access=True, + ), + + # 1 parent denies + AccessTestData(partition_groups={1: 1, 2: 2}, merged_parents_list=[{1: {}}]), + AccessTestData(partition_groups={1: 1, 2: 2}, merged_parents_list=[{1: {3}}]), + + # 1 parent denies, 1 parent allows all + AccessTestData(partition_groups={1: 1, 2: 2}, merged_parents_list=[{1: {}}, {}], expected_access=True), + AccessTestData(partition_groups={1: 1, 2: 2}, merged_parents_list=[{1: {}}, {1: {}}, {}], expected_access=True), + AccessTestData(partition_groups={1: 1, 2: 2}, merged_parents_list=[{1: {}}, {}, {1: {}}], expected_access=True), + + # 1 parent denies, 1 parent allows + AccessTestData(partition_groups={1: 1, 2: 2}, merged_parents_list=[{1: {3}}, {1: {1}}], expected_access=True), + + # 2 parents deny + AccessTestData(partition_groups={1: 1, 2: 2}, merged_parents_list=[{1: {}}, {1: {}}]), + AccessTestData(partition_groups={1: 1, 2: 2}, merged_parents_list=[{1: {3}}, {1: {3}, 2: {2}}]), + + # intersect with parent + # child denies, 1 parent allows + AccessTestData(partition_groups={1: 1, 2: 2}, xblock_access={1: [3]}, merged_parents_list=[{1: {1}}]), + AccessTestData(partition_groups={1: 1, 2: 2}, xblock_access={1: [2]}, merged_parents_list=[{1: {1}}]), + + # child denies, 2 parents allow + AccessTestData(partition_groups={1: 1, 2: 2}, xblock_access={1: [3]}, merged_parents_list=[{1: {1}}, {2: {2}}]), + AccessTestData(partition_groups={1: 1, 2: 2}, xblock_access={2: [3]}, merged_parents_list=[{1: {1}}, {2: {2}}]), + + # child allows, 1 parent denies + AccessTestData(partition_groups={1: 1, 2: 2}, xblock_access={2: [2]}, merged_parents_list=[{1: {}}]), + AccessTestData(partition_groups={1: 1, 2: 2}, xblock_access={1: [1]}, merged_parents_list=[{1: {2}}]), + AccessTestData(partition_groups={1: 1, 2: 2}, xblock_access={2: [2]}, merged_parents_list=[{1: {2}}]), + + # child allows, 1 parent allows + AccessTestData( + partition_groups={1: 1, 2: 2}, + xblock_access={1: [1]}, + merged_parents_list=[{}], + expected_access=True, + ), + AccessTestData( + partition_groups={1: 1, 2: 2}, xblock_access={2: [2]}, merged_parents_list=[{1: {1}}], expected_access=True + ), + AccessTestData( + partition_groups={1: 1, 2: 2}, + xblock_access={1: [1, 3], 2: [2, 3]}, + merged_parents_list=[{1: {1, 2, 3}}, {2: {1, 2, 3}}], + expected_access=True, + ), + + # child allows, 1 parent allows, 1 parent denies + AccessTestData( + partition_groups={1: 1, 2: 2}, + xblock_access={1: [1]}, + merged_parents_list=[{1: {3}}, {1: {1}}], + expected_access=True, + ), + ) + @ddt.unpack + def test_merged_group_access(self, user_partition_groups, xblock_access, merged_parents_list, expected_access): + # use the course as the block to test + block = self.course + + # update block access + if xblock_access is not None: + block.group_access = xblock_access + update_block(self.course) + + # convert merged_parents_list to _MergedGroupAccess objects + for ind, merged_parent in enumerate(merged_parents_list): + converted_object = _MergedGroupAccess([], block, []) + converted_object._access = merged_parent + merged_parents_list[ind] = converted_object + + merged_group_access = _MergedGroupAccess(self.user_partitions, block, merged_parents_list) + + # convert group_id to groups in user_partition_groups parameter + for partition_id, group_id in user_partition_groups.iteritems(): + user_partition_groups[partition_id] = self.groups[group_id - 1] + + self.assertEquals( + merged_group_access.check_group_access(user_partition_groups), + expected_access, + ) @ddt.data( ([None], None), diff --git a/lms/djangoapps/course_blocks/transformers/user_partitions.py b/lms/djangoapps/course_blocks/transformers/user_partitions.py index 337dc0e51a..91ef4f9918 100644 --- a/lms/djangoapps/course_blocks/transformers/user_partitions.py +++ b/lms/djangoapps/course_blocks/transformers/user_partitions.py @@ -161,19 +161,21 @@ class _MergedGroupAccess(object): 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. + # 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. + # Since this parent chain has no group access + # restrictions for this partition, allow + # unfettered group access for this partition + # and don't bother checking the rest of the + # parents. 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 + xblock_partition_access = set(xblock_group_access.get(partition.id) or []) or None # Compute this block's access by intersecting the block's # own access with the merged access from its parent chains.