MA-1624 Unit tests for User Partition Transformer.

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),

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.