Revert "REVMI-62 Return gated blocks in the course blocks API response with an...

Revert "REVMI-62 Return gated blocks in the course blocks API response with an authorization denial reason"

common/lib/xmodule/xmodule/partitions/partitions.py

@@ -250,13 +250,13 @@ class UserPartition(namedtuple("UserPartition", "id name description groups sche
             )
         )
 
-    def access_denied_message(self, block_key, user, user_group, allowed_groups):
+    def access_denied_message(self, block, user, user_group, allowed_groups):
         """
         Return a message that should be displayed to the user when they are not allowed to access
         content managed by this partition, or None if there is no applicable message.
 
         Arguments:
-            block_key (:class:`.BlockUsageLocator`): The content being managed
+            block (:class:`.XBlock`): The content being managed
             user (:class:`.User`): The user who was denied access
             user_group (:class:`.Group`): The current Group the user is in
             allowed_groups (list of :class:`.Group`): The groups who are allowed to see the content

common/lib/xmodule/xmodule/partitions/partitions_service.py

@@ -104,7 +104,7 @@ def _create_enrollment_track_partition(course):
         log.warning(
             "Can't add 'enrollment_track' partition, as ID {id} is assigned to {partition} in course {course}.".format(
                 id=ENROLLMENT_TRACK_PARTITION_ID,
-                partition=get_partition_from_id(course.user_partitions, ENROLLMENT_TRACK_PARTITION_ID).name,
+                partition=_get_partition_from_id(course.user_partitions, ENROLLMENT_TRACK_PARTITION_ID).name,
                 course=unicode(course.id)
             )
         )
@@ -193,7 +193,7 @@ class PartitionService(object):
         Returns:
             A UserPartition, or None if not found.
         """
-        return get_partition_from_id(self.course_partitions, user_partition_id)
+        return _get_partition_from_id(self.course_partitions, user_partition_id)
 
     def get_group(self, user, user_partition, assign=True):
         """
@@ -206,7 +206,7 @@ class PartitionService(object):
         )
 
 
-def get_partition_from_id(partitions, user_partition_id):
+def _get_partition_from_id(partitions, user_partition_id):
     """
     Look for a user partition with a matching id in the provided list of partitions.
 

lms/djangoapps/course_api/blocks/api.py

@@ -5,7 +5,6 @@ API function for retrieving course blocks data
 import lms.djangoapps.course_blocks.api as course_blocks_api
 from lms.djangoapps.course_blocks.transformers.hidden_content import HiddenContentTransformer
 from lms.djangoapps.course_blocks.transformers.hide_empty import HideEmptyTransformer
-from lms.djangoapps.course_blocks.transformers.access_denied_filter import AccessDeniedMessageFilterTransformer
 from openedx.core.djangoapps.content.block_structure.transformers import BlockStructureTransformers
 from openedx.core.lib.mobile_utils import is_request_from_mobile_app
 
@@ -26,7 +25,6 @@ def get_blocks(
         student_view_data=None,
         return_type='dict',
         block_types_filter=None,
-        hide_access_denials=False,
 ):
     """
     Return a serialized representation of the course blocks.
@@ -53,9 +51,6 @@ def get_blocks(
             the format for returning the blocks.
         block_types_filter (list): Optional list of block type names used to filter
             the final result of returned blocks.
-        hide_access_denials (bool): When True, filter out any blocks that were
-            denied access to the user, even if they have access denial messages
-            attached.
     """
     # create ordered list of transformers, adding BlocksAPITransformer at end.
     transformers = BlockStructureTransformers()
@@ -75,9 +70,6 @@ def get_blocks(
             HiddenContentTransformer()
         ]
 
-    if hide_access_denials:
-        transformers += [AccessDeniedMessageFilterTransformer()]
-
     # TODO: Remove this after REVE-52 lands and old-mobile-app traffic falls to < 5% of mobile traffic
     if is_request_from_mobile_app(request):
         transformers += [HideEmptyTransformer()]

lms/djangoapps/course_api/blocks/serializers.py

@@ -35,20 +35,6 @@ class BlockSerializer(serializers.Serializer):  # pylint: disable=abstract-metho
         Return a serializable representation of the requested block
         """
         # create response data dict for basic fields
-
-        block_structure = self.context['block_structure']
-        authorization_denial_reason = block_structure.get_xblock_field(block_key, 'authorization_denial_reason')
-        authorization_denial_message = block_structure.get_xblock_field(block_key, 'authorization_denial_message')
-
-        if authorization_denial_reason and authorization_denial_message:
-            data = {
-                'id': unicode(block_key),
-                'block_id': unicode(block_key.block_id),
-                'authorization_denial_reason': authorization_denial_reason,
-                'authorization_denial_message': authorization_denial_message
-            }
-            return data
-
         data = {
             'id': unicode(block_key),
             'block_id': unicode(block_key.block_id),
@@ -85,7 +71,7 @@ class BlockSerializer(serializers.Serializer):  # pylint: disable=abstract-metho
                     data[supported_field.serializer_field_name] = field_value
 
         if 'children' in self.context['requested_fields']:
-            children = block_structure.get_children(block_key)
+            children = self.context['block_structure'].get_children(block_key)
             if children:
                 data['children'] = [unicode(child) for child in children]
 

lms/djangoapps/course_api/blocks/urls.py

@@ -11,7 +11,6 @@ urlpatterns = [
     url(
         r'^v1/blocks/{}'.format(settings.USAGE_KEY_PATTERN),
         BlocksView.as_view(),
-        kwargs={'hide_access_denials': True},
         name="blocks_in_block_tree"
     ),
 
@@ -19,20 +18,6 @@ urlpatterns = [
     url(
         r'^v1/blocks/',
         BlocksInCourseView.as_view(),
-        kwargs={'hide_access_denials': True},
-        name="blocks_in_course"
-    ),
-    # This endpoint requires the usage_key for the starting block.
-    url(
-        r'^v2/blocks/{}'.format(settings.USAGE_KEY_PATTERN),
-        BlocksView.as_view(),
-        name="blocks_in_block_tree"
-    ),
-
-    # This endpoint is an alternative to the above, but requires course_id as a parameter.
-    url(
-        r'^v2/blocks/',
-        BlocksInCourseView.as_view(),
         name="blocks_in_course"
     ),
 ]

lms/djangoapps/course_api/blocks/views.py

@@ -183,7 +183,7 @@ class BlocksView(DeveloperErrorViewMixin, ListAPIView):
             Returned only if "show_correctness" is included in the "requested_fields" parameter.
     """
 
-    def list(self, request, usage_key_string, hide_access_denials=False):  # pylint: disable=arguments-differ
+    def list(self, request, usage_key_string):  # pylint: disable=arguments-differ
         """
         REST API endpoint for listing all the blocks information in the course,
         while regarding user access and roles.
@@ -213,7 +213,6 @@ class BlocksView(DeveloperErrorViewMixin, ListAPIView):
                     params.cleaned_data.get('student_view_data', []),
                     params.cleaned_data['return_type'],
                     params.cleaned_data.get('block_types_filter', None),
-                    hide_access_denials=hide_access_denials,
                 )
             )
         except ItemNotFoundError as exception:
@@ -261,7 +260,7 @@ class BlocksInCourseView(BlocksView):
         with a message indicating that the course_id is not valid.
     """
 
-    def list(self, request, hide_access_denials=False):  # pylint: disable=arguments-differ
+    def list(self, request):  # pylint: disable=arguments-differ
         """
         Retrieves the usage_key for the requested course, and then returns the
         same information that would be returned by BlocksView.list, called with
@@ -281,4 +280,4 @@ class BlocksInCourseView(BlocksView):
             course_usage_key = modulestore().make_course_usage_key(course_key)
         except InvalidKeyError:
             raise ValidationError(u"'{}' is not a valid course key.".format(unicode(course_key_string)))
-        return super(BlocksInCourseView, self).list(request, course_usage_key, hide_access_denials=hide_access_denials)
+        return super(BlocksInCourseView, self).list(request, course_usage_key)

lms/djangoapps/course_api/tests/test_serializers.py

@@ -65,7 +65,7 @@ class TestCourseSerializer(CourseApiFactoryMixin, ModuleStoreTestCase):
             'end': u'2015-09-19T18:00:00Z',
             'enrollment_start': u'2015-06-15T00:00:00Z',
             'enrollment_end': u'2015-07-15T00:00:00Z',
-            'blocks_url': u'http://testserver/api/courses/v2/blocks/?course_id=edX%2Ftoy%2F2012_Fall',
+            'blocks_url': u'http://testserver/api/courses/v1/blocks/?course_id=edX%2Ftoy%2F2012_Fall',
             'effort': u'6 hours',
             'pacing': 'instructor',
             'mobile_available': True,

lms/djangoapps/course_blocks/transformers/access_denied_filter.py

-"""
-Access Denied Message Filter Transformer implementation.
-"""
-# TODO: Remove this file after REVE-52 lands and old-mobile-app traffic falls to < 5% of mobile traffic
-from openedx.core.djangoapps.content.block_structure.transformer import (
-    BlockStructureTransformer
-)
-
-
-class AccessDeniedMessageFilterTransformer(BlockStructureTransformer):
-    """
-    A transformer that removes any block from the course that has an
-    authorization_denial_reason or an authorization_denial_message.
-    """
-    WRITE_VERSION = 1
-    READ_VERSION = 1
-
-    @classmethod
-    def name(cls):
-        """
-        Unique identifier for the transformer's class;
-        same identifier used in setup.py.
-        """
-        return "access_denied_message_filter"
-
-    @classmethod
-    def collect(cls, block_structure):
-        """
-        Collects any information that's necessary to execute this
-        transformer's transform method.
-        """
-        block_structure.request_xblock_fields('authorization_denial_reason', 'authorization_denial_message')
-
-    def transform(self, usage_info, block_structure):
-        def _filter(block_key):
-            reason = block_structure.get_xblock_field(block_key, 'authorization_denial_reason')
-            message = block_structure.get_xblock_field(block_key, 'authorization_denial_message')
-            return reason and message
-
-        for _ in block_structure.post_order_traversal(
-            filter_func=block_structure.create_removal_filter(_filter)
-        ):
-            pass

lms/djangoapps/course_blocks/transformers/tests/test_split_test.py

@@ -177,7 +177,7 @@ class SplitTestTransformerTestCase(CourseStructureTestCase):
         #  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).
-        (0, ('course', 'A', 'D', 'E', 'H', 'L', 'O', 'P', )),
+        (0, ('course', 'A', 'D', 'E', 'H', 'L', 'O', 'P',)),
         (1, ('course', 'A', 'D', 'F', 'J', 'M', 'I',)),
         (2, ('course', 'A', 'D', 'G', 'O',)),
     )

lms/djangoapps/course_blocks/transformers/tests/test_user_partitions.py

@@ -4,18 +4,13 @@ Tests for UserPartitionTransformer.
 """
 import string
 from collections import namedtuple
-from datetime import datetime
 
 import ddt
-from mock import patch
 
-from course_modes.tests.factories import CourseModeFactory
 from openedx.core.djangoapps.course_groups.cohorts import add_user_to_cohort
 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.views import link_cohort_to_partition_group
-from openedx.features.content_type_gating.models import ContentTypeGatingConfig
-from openedx.features.content_type_gating.partitions import create_content_gating_partition
 from student.tests.factories import CourseEnrollmentFactory
 from xmodule.modulestore.tests.factories import CourseFactory
 from xmodule.partitions.partitions import Group, UserPartition
@@ -178,7 +173,7 @@ class UserPartitionTransformerTestCase(UserPartitionTestMixin, CourseStructureTe
                 '#type': 'vertical',
                 '#ref': 'K',
                 '#parents': ['E'],
-                'metadata': {'group_access': {self.user_partition.id: [4, 51]}},
+                'metadata': {'group_access': {self.user_partition.id: [4]}},
                 '#children': [{'#type': 'vertical', '#ref': 'N'}],
             },
             {
@@ -224,32 +219,6 @@ class UserPartitionTransformerTestCase(UserPartitionTestMixin, CourseStructureTe
             self.get_block_key_set(self.blocks, *expected_blocks)
         )
 
-    def test_transform_with_content_gating_partition(self):
-        self.setup_partitions_and_course()
-        CourseModeFactory.create(course_id=self.course.id, mode_slug='audit')
-        CourseModeFactory.create(course_id=self.course.id, mode_slug='verified')
-        ContentTypeGatingConfig.objects.create(enabled=True, enabled_as_of=datetime(2018, 1, 1))
-        partition = create_content_gating_partition(self.course)
-        self.user_partitions.append(partition)
-        cohort = self.partition_cohorts[0][1]
-        add_user_to_cohort(cohort, self.user.username)
-
-        with patch(
-            'lms.djangoapps.course_blocks.transformers.user_partitions.get_partition_from_id',
-            return_value=partition
-        ), patch(
-            'lms.djangoapps.course_blocks.transformers.user_partitions._MergedGroupAccess.get_allowed_groups',
-            return_value={51: set([])}
-        ):
-            trans_block_structure = get_course_blocks(
-                self.user,
-                self.course.location,
-                self.transformers,
-            )
-            xblocks_denial_reason = [trans_block_structure.get_xblock_field(b, 'authorization_denial_reason')
-                                     for b in trans_block_structure.get_block_keys()]
-            self.assertSetEqual(set(xblocks_denial_reason), set([u'Feature-based Enrollments']))
-
     def test_transform_on_inactive_partition(self):
         """
         Tests UserPartitionTransformer for inactive UserPartition.

lms/djangoapps/course_blocks/transformers/user_partitions.py

@@ -6,11 +6,7 @@ from openedx.core.djangoapps.content.block_structure.transformer import (
     BlockStructureTransformer,
     FilteringTransformerMixin
 )
-from xmodule.partitions.partitions_service import (
-    get_all_partitions_for_course,
-    get_partition_from_id,
-    get_user_partition_groups
-)
+from xmodule.partitions.partitions_service import get_user_partition_groups, get_all_partitions_for_course
 
 from .split_test import SplitTestTransformer
 from .utils import get_field_on_block
@@ -83,38 +79,15 @@ class UserPartitionTransformer(FilteringTransformerMixin, BlockStructureTransfor
             return [block_structure.create_universal_filter()]
 
         user_groups = get_user_partition_groups(usage_info.course_key, user_partitions, user, 'id')
-
-        for block_key in block_structure.topological_traversal():
-            transformer_block_field = block_structure.get_transformer_block_field(
-                block_key, self, 'merged_group_access'
-            )
-            access_denying_partition_id = transformer_block_field.get_access_denying_partition(
-                user_groups
-            )
-            access_denying_partition = get_partition_from_id(user_partitions, access_denying_partition_id)
-
-            if not has_access(user, 'staff', block_key) and access_denying_partition:
-                user_group = user_groups.get(access_denying_partition.id)
-                allowed_groups = transformer_block_field.get_allowed_groups()[access_denying_partition.id]
-                access_denied_message = access_denying_partition.access_denied_message(
-                    block_key, user, user_group, allowed_groups
-                )
-                block_structure.override_xblock_field(
-                    block_key, 'authorization_denial_reason', access_denying_partition.name
-                )
-                block_structure.override_xblock_field(
-                    block_key, 'authorization_denial_message', access_denied_message
-                )
-
         group_access_filter = block_structure.create_removal_filter(
-            lambda block_key: (
-                not has_access(user, 'staff', block_key) and
-                block_structure.get_transformer_block_field(
-                    block_key, self, 'merged_group_access'
-                ).get_access_denying_partition(user_groups) is not None and
-                block_structure.get_xblock_field(block_key, 'authorization_denial_message') is None
+            lambda block_key: not (
+                has_access(user, 'staff', block_key) or
+                block_structure.get_transformer_block_field(block_key, self, 'merged_group_access').check_group_access(
+                    user_groups
+                )
             )
         )
+
         result_list.append(group_access_filter)
         return result_list
 
@@ -180,6 +153,7 @@ class _MergedGroupAccess(object):
             # 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.
@@ -214,9 +188,6 @@ class _MergedGroupAccess(object):
             if merged_group_ids is not None:
                 self._access[partition.id] = merged_group_ids
 
-    def get_allowed_groups(self):
-        return self._access
-
     @staticmethod
     def _intersection(*sets):
         """
@@ -239,7 +210,7 @@ class _MergedGroupAccess(object):
         else:
             return None
 
-    def get_access_denying_partition(self, user_groups):
+    def check_group_access(self, user_groups):
         """
         Arguments:
             dict[int: Group]: Given a user, a mapping from user
@@ -247,34 +218,23 @@ class _MergedGroupAccess(object):
                 each partition.
 
         Returns:
-            bool: Which partition is denying access
+            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,
-            # return partition that would deny access.
+            # deny access.
             if partition_id not in user_groups:
-                return partition_id
+                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, return partition that would deny access.
+            # Else, deny access.
             else:
-                return partition_id
-
-        # The user has access for every partition, return none
-        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.
+                return False
 
-        Returns:
-            bool: Whether said user has group access.
-        """
-        return self.get_access_denying_partition(user_groups) is None
+        # The user has access for every partition, grant access.
+        return True

lms/djangoapps/courseware/access.py

@@ -503,12 +503,11 @@ def _has_group_access(descriptor, user, course_key):
 
     if missing_groups:
         partition, user_group, allowed_groups = missing_groups[0]
-        block_key = descriptor.scope_ids.usage_id
         return IncorrectPartitionGroupError(
             partition=partition,
             user_group=user_group,
             allowed_groups=allowed_groups,
-            user_message=partition.access_denied_message(block_key, user, user_group, allowed_groups),
+            user_message=partition.access_denied_message(descriptor, user, user_group, allowed_groups),
             user_fragment=partition.access_denied_fragment(descriptor, user, user_group, allowed_groups),
         )
 

openedx/features/content_type_gating/partitions.py

@@ -85,8 +85,8 @@ class ContentTypeGatingPartition(UserPartition):
         if (verified_mode is None or not self._is_audit_enrollment(user, course_key) or
                 user_group == FULL_ACCESS):
             return None
-
         ecommerce_checkout_link = self._get_checkout_link(user, verified_mode.sku)
+
         request = crum.get_current_request()
         frag = Fragment(render_to_string('content_type_gating/access_denied_message.html', {
             'mobile_app': request and is_request_from_mobile_app(request),
@@ -95,19 +95,14 @@ class ContentTypeGatingPartition(UserPartition):
         }))
         return frag
 
-    def access_denied_message(self, block_key, user, user_group, allowed_groups):
-        course_key = block_key.course_key
+    def access_denied_message(self, block, user, user_group, allowed_groups):
+        course_key = self._get_course_key_from_course_block(block)
         modes = CourseMode.modes_for_course_dict(course_key)
         verified_mode = modes.get(CourseMode.VERIFIED)
         if (verified_mode is None or not self._is_audit_enrollment(user, course_key) or
                 user_group == FULL_ACCESS):
             return None
-
-        request = crum.get_current_request()
-        if request and is_request_from_mobile_app(request):
-            return _(u"Graded assessments are available to Verified Track learners.")
-        else:
-            return _(u"Graded assessments are available to Verified Track learners. Upgrade to Unlock.")
+        return _(u"Graded assessments are available to Verified Track learners. Upgrade to Unlock.")
 
     def _is_audit_enrollment(self, user, course_key):
         """

openedx/features/content_type_gating/tests/test_partitions.py

@@ -119,7 +119,7 @@ class TestContentTypeGatingPartition(CacheIsolationTestCase):
         ):
             fragment = partition.access_denied_fragment(mock_block, global_staff, FULL_ACCESS, 'test_allowed_group')
             self.assertIsNone(fragment)
-            message = partition.access_denied_message(mock_block.scope_ids.usage_id, global_staff, FULL_ACCESS, 'test_allowed_group')
+            message = partition.access_denied_message(mock_block, global_staff, FULL_ACCESS, 'test_allowed_group')
             self.assertIsNone(message)
 
     def test_acess_denied_fragment_for_null_request(self):

setup.py

@@ -63,7 +63,6 @@ setup(
             "completion = lms.djangoapps.course_api.blocks.transformers.block_completion:BlockCompletionTransformer",
             "load_override_data = lms.djangoapps.course_blocks.transformers.load_override_data:OverrideDataTransformer",
             "content_type_gate = openedx.features.content_type_gating.block_transformers:ContentTypeGateTransformer",
-            "access_denied_message_filter = lms.djangoapps.course_blocks.transformers.access_denied_filter:AccessDeniedMessageFilterTransformer",
         ],
         "openedx.ace.policy": [
             "bulk_email_optout = lms.djangoapps.bulk_email.policies:CourseEmailOptout"