From 1f6cd7aa5ab8653bc6eae184479d4117f78d1d4c Mon Sep 17 00:00:00 2001 From: Matthew Piatetsky Date: Thu, 28 Mar 2019 14:44:05 -0400 Subject: [PATCH 1/2] Revert "Revert "REVMI-62 Return gated blocks in the course blocks API response with an authorization denial reason"" --- .../xmodule/xmodule/partitions/partitions.py | 4 +- .../xmodule/partitions/partitions_service.py | 6 +- lms/djangoapps/course_api/blocks/api.py | 8 ++ .../course_api/blocks/serializers.py | 16 +++- lms/djangoapps/course_api/blocks/urls.py | 15 +++ lms/djangoapps/course_api/blocks/views.py | 7 +- .../course_api/tests/test_serializers.py | 2 +- .../transformers/access_denied_filter.py | 43 +++++++++ .../transformers/tests/test_split_test.py | 2 +- .../tests/test_user_partitions.py | 33 ++++++- .../transformers/user_partitions.py | 92 +++++++++++++------ lms/djangoapps/courseware/access.py | 3 +- .../content_type_gating/partitions.py | 13 ++- .../tests/test_partitions.py | 2 +- setup.py | 1 + 15 files changed, 203 insertions(+), 44 deletions(-) create mode 100644 lms/djangoapps/course_blocks/transformers/access_denied_filter.py diff --git a/common/lib/xmodule/xmodule/partitions/partitions.py b/common/lib/xmodule/xmodule/partitions/partitions.py index 49aa36e2ef..9c62c957ac 100644 --- a/common/lib/xmodule/xmodule/partitions/partitions.py +++ b/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, user, user_group, allowed_groups): + def access_denied_message(self, block_key, 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 (:class:`.XBlock`): The content being managed + block_key (:class:`.BlockUsageLocator`): 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 diff --git a/common/lib/xmodule/xmodule/partitions/partitions_service.py b/common/lib/xmodule/xmodule/partitions/partitions_service.py index e742373288..3fb0d8740d 100644 --- a/common/lib/xmodule/xmodule/partitions/partitions_service.py +++ b/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. diff --git a/lms/djangoapps/course_api/blocks/api.py b/lms/djangoapps/course_api/blocks/api.py index 8c0c586db4..a99fc4121d 100644 --- a/lms/djangoapps/course_api/blocks/api.py +++ b/lms/djangoapps/course_api/blocks/api.py @@ -5,6 +5,7 @@ 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 @@ -25,6 +26,7 @@ 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. @@ -51,6 +53,9 @@ 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() @@ -70,6 +75,9 @@ 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()] diff --git a/lms/djangoapps/course_api/blocks/serializers.py b/lms/djangoapps/course_api/blocks/serializers.py index f154b2234d..3bbd41aa35 100644 --- a/lms/djangoapps/course_api/blocks/serializers.py +++ b/lms/djangoapps/course_api/blocks/serializers.py @@ -35,6 +35,20 @@ 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), @@ -71,7 +85,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 = self.context['block_structure'].get_children(block_key) + children = block_structure.get_children(block_key) if children: data['children'] = [unicode(child) for child in children] diff --git a/lms/djangoapps/course_api/blocks/urls.py b/lms/djangoapps/course_api/blocks/urls.py index 7fb3ca83f9..ea71b1581e 100644 --- a/lms/djangoapps/course_api/blocks/urls.py +++ b/lms/djangoapps/course_api/blocks/urls.py @@ -11,6 +11,7 @@ urlpatterns = [ url( r'^v1/blocks/{}'.format(settings.USAGE_KEY_PATTERN), BlocksView.as_view(), + kwargs={'hide_access_denials': True}, name="blocks_in_block_tree" ), @@ -18,6 +19,20 @@ 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" ), ] diff --git a/lms/djangoapps/course_api/blocks/views.py b/lms/djangoapps/course_api/blocks/views.py index 02783f3b78..8f40a223df 100644 --- a/lms/djangoapps/course_api/blocks/views.py +++ b/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): # pylint: disable=arguments-differ + def list(self, request, usage_key_string, hide_access_denials=False): # pylint: disable=arguments-differ """ REST API endpoint for listing all the blocks information in the course, while regarding user access and roles. @@ -213,6 +213,7 @@ 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: @@ -260,7 +261,7 @@ class BlocksInCourseView(BlocksView): with a message indicating that the course_id is not valid. """ - def list(self, request): # pylint: disable=arguments-differ + def list(self, request, hide_access_denials=False): # 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 @@ -280,4 +281,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) + return super(BlocksInCourseView, self).list(request, course_usage_key, hide_access_denials=hide_access_denials) diff --git a/lms/djangoapps/course_api/tests/test_serializers.py b/lms/djangoapps/course_api/tests/test_serializers.py index 59d073b93a..45197a6422 100644 --- a/lms/djangoapps/course_api/tests/test_serializers.py +++ b/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/v1/blocks/?course_id=edX%2Ftoy%2F2012_Fall', + 'blocks_url': u'http://testserver/api/courses/v2/blocks/?course_id=edX%2Ftoy%2F2012_Fall', 'effort': u'6 hours', 'pacing': 'instructor', 'mobile_available': True, diff --git a/lms/djangoapps/course_blocks/transformers/access_denied_filter.py b/lms/djangoapps/course_blocks/transformers/access_denied_filter.py new file mode 100644 index 0000000000..5ca3125857 --- /dev/null +++ b/lms/djangoapps/course_blocks/transformers/access_denied_filter.py @@ -0,0 +1,43 @@ +""" +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 diff --git a/lms/djangoapps/course_blocks/transformers/tests/test_split_test.py b/lms/djangoapps/course_blocks/transformers/tests/test_split_test.py index 1e8873e0f9..9cf0f8b071 100644 --- a/lms/djangoapps/course_blocks/transformers/tests/test_split_test.py +++ b/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',)), ) 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 67a8ded484..d4ad5d6e0b 100644 --- a/lms/djangoapps/course_blocks/transformers/tests/test_user_partitions.py +++ b/lms/djangoapps/course_blocks/transformers/tests/test_user_partitions.py @@ -4,13 +4,18 @@ 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 @@ -173,7 +178,7 @@ class UserPartitionTransformerTestCase(UserPartitionTestMixin, CourseStructureTe '#type': 'vertical', '#ref': 'K', '#parents': ['E'], - 'metadata': {'group_access': {self.user_partition.id: [4]}}, + 'metadata': {'group_access': {self.user_partition.id: [4, 51]}}, '#children': [{'#type': 'vertical', '#ref': 'N'}], }, { @@ -219,6 +224,32 @@ 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. diff --git a/lms/djangoapps/course_blocks/transformers/user_partitions.py b/lms/djangoapps/course_blocks/transformers/user_partitions.py index 4efd932b2a..19519b01f2 100644 --- a/lms/djangoapps/course_blocks/transformers/user_partitions.py +++ b/lms/djangoapps/course_blocks/transformers/user_partitions.py @@ -6,7 +6,11 @@ from openedx.core.djangoapps.content.block_structure.transformer import ( BlockStructureTransformer, FilteringTransformerMixin ) -from xmodule.partitions.partitions_service import get_user_partition_groups, get_all_partitions_for_course +from xmodule.partitions.partitions_service import ( + get_all_partitions_for_course, + get_partition_from_id, + get_user_partition_groups +) from .split_test import SplitTestTransformer from .utils import get_field_on_block @@ -79,15 +83,38 @@ class UserPartitionTransformer(FilteringTransformerMixin, BlockStructureTransfor return [block_structure.create_universal_filter()] user_groups = get_user_partition_groups(usage_info.course_key, user_partitions, user, 'id') - group_access_filter = block_structure.create_removal_filter( - 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 + + 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 ) ) - result_list.append(group_access_filter) return result_list @@ -153,7 +180,6 @@ 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. @@ -188,6 +214,9 @@ 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): """ @@ -210,6 +239,34 @@ class _MergedGroupAccess(object): else: return None + def get_access_denying_partition(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: Which partition is denying 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. + if partition_id not in user_groups: + return partition_id + + # 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: + return partition_id + + # The user has access for every partition, return none + return None + def check_group_access(self, user_groups): """ Arguments: @@ -220,21 +277,4 @@ class _MergedGroupAccess(object): 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 + return self.get_access_denying_partition(user_groups) is None diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py index ccdac8abc3..60391f3fb0 100644 --- a/lms/djangoapps/courseware/access.py +++ b/lms/djangoapps/courseware/access.py @@ -503,11 +503,12 @@ 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(descriptor, user, user_group, allowed_groups), + user_message=partition.access_denied_message(block_key, user, user_group, allowed_groups), user_fragment=partition.access_denied_fragment(descriptor, user, user_group, allowed_groups), ) diff --git a/openedx/features/content_type_gating/partitions.py b/openedx/features/content_type_gating/partitions.py index 3b2e03b5f1..b640a19fab 100644 --- a/openedx/features/content_type_gating/partitions.py +++ b/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) + 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,14 +95,19 @@ class ContentTypeGatingPartition(UserPartition): })) return frag - def access_denied_message(self, block, user, user_group, allowed_groups): - course_key = self._get_course_key_from_course_block(block) + def access_denied_message(self, block_key, user, user_group, allowed_groups): + course_key = block_key.course_key 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 - return _(u"Graded assessments are available to Verified Track learners. Upgrade to Unlock.") + + 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.") def _is_audit_enrollment(self, user, course_key): """ diff --git a/openedx/features/content_type_gating/tests/test_partitions.py b/openedx/features/content_type_gating/tests/test_partitions.py index 641ab1a25a..de08ef914e 100644 --- a/openedx/features/content_type_gating/tests/test_partitions.py +++ b/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, global_staff, FULL_ACCESS, 'test_allowed_group') + message = partition.access_denied_message(mock_block.scope_ids.usage_id, global_staff, FULL_ACCESS, 'test_allowed_group') self.assertIsNone(message) def test_acess_denied_fragment_for_null_request(self): diff --git a/setup.py b/setup.py index 26e0426552..8e9822aaba 100644 --- a/setup.py +++ b/setup.py @@ -63,6 +63,7 @@ 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" From c77ba984c99a80f0eac212acf47b3caa4c205205 Mon Sep 17 00:00:00 2001 From: Matthew Piatetsky Date: Thu, 28 Mar 2019 14:53:39 -0400 Subject: [PATCH 2/2] fix traversal bug and keep current behavior as default --- lms/djangoapps/course_api/blocks/api.py | 2 +- .../course_blocks/transformers/access_denied_filter.py | 5 +---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/lms/djangoapps/course_api/blocks/api.py b/lms/djangoapps/course_api/blocks/api.py index a99fc4121d..905e1bd9d7 100644 --- a/lms/djangoapps/course_api/blocks/api.py +++ b/lms/djangoapps/course_api/blocks/api.py @@ -26,7 +26,7 @@ def get_blocks( student_view_data=None, return_type='dict', block_types_filter=None, - hide_access_denials=False, + hide_access_denials=True, ): """ Return a serialized representation of the course blocks. diff --git a/lms/djangoapps/course_blocks/transformers/access_denied_filter.py b/lms/djangoapps/course_blocks/transformers/access_denied_filter.py index 5ca3125857..adbcd5f740 100644 --- a/lms/djangoapps/course_blocks/transformers/access_denied_filter.py +++ b/lms/djangoapps/course_blocks/transformers/access_denied_filter.py @@ -37,7 +37,4 @@ class AccessDeniedMessageFilterTransformer(BlockStructureTransformer): 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 + block_structure.remove_block_traversal(_filter)