From 6e52aafc8cae3165d3b448ecbad9b2dae0626e2b Mon Sep 17 00:00:00 2001 From: Michael Terry Date: Fri, 30 Apr 2021 12:38:30 -0400 Subject: [PATCH] fix: properly set contains_gated_content field on xblocks again This has been broken for a couple months, preventing proper display of verified-only assignments on the dates tab and elsewhere. AA-780 --- .../xmodule/modulestore/tests/factories.py | 1 + .../course_api/blocks/serializers.py | 1 + .../outline/v1/tests/test_views.py | 12 ++----- .../tests/test_content_highlights.py | 2 +- .../content_type_gating/block_transformers.py | 4 ++- .../content_type_gating/tests/test_access.py | 31 ++++++++++--------- 6 files changed, 26 insertions(+), 25 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/factories.py b/common/lib/xmodule/xmodule/modulestore/tests/factories.py index 7717581dd9..bfd925f45b 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/factories.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/factories.py @@ -412,6 +412,7 @@ class ItemFactory(XModuleFactory): module.submission_start = submission_start if submission_end: module.submission_end = submission_end + store.update_item(module, user_id) # VS[compat] cdodge: This is a hack because static_tabs also have references from the course module, so # if we add one then we need to also add it to the policy information (i.e. metadata) diff --git a/lms/djangoapps/course_api/blocks/serializers.py b/lms/djangoapps/course_api/blocks/serializers.py index f5f1cc4b43..277bf2a668 100644 --- a/lms/djangoapps/course_api/blocks/serializers.py +++ b/lms/djangoapps/course_api/blocks/serializers.py @@ -103,6 +103,7 @@ FIELDS_ALLOWED_IN_AUTH_DENIED_CONTENT = [ "descendants", "authorization_denial_reason", "authorization_denial_message", + 'contains_gated_content', ] diff --git a/lms/djangoapps/course_home_api/outline/v1/tests/test_views.py b/lms/djangoapps/course_home_api/outline/v1/tests/test_views.py index ad4eb9564a..1cb9833a44 100644 --- a/lms/djangoapps/course_home_api/outline/v1/tests/test_views.py +++ b/lms/djangoapps/course_home_api/outline/v1/tests/test_views.py @@ -304,17 +304,11 @@ class OutlineTabTestViews(BaseCourseHomeTests): chapter = ItemFactory.create(category='chapter', parent_location=course.location) sequential = ItemFactory.create(display_name='Test', category='sequential', graded=True, has_score=True, parent_location=chapter.location) - problem1 = ItemFactory.create(category='problem', graded=True, has_score=True, - parent_location=sequential.location) - problem2 = ItemFactory.create(category='problem', graded=True, has_score=True, - parent_location=sequential.location) + ItemFactory.create(category='problem', graded=True, has_score=True, parent_location=sequential.location) + ItemFactory.create(category='problem', graded=True, has_score=True, parent_location=sequential.location) sequential2 = ItemFactory.create(display_name='Ungraded', category='sequential', parent_location=chapter.location) - problem3 = ItemFactory.create(category='problem', parent_location=sequential2.location) - course.children = [chapter] - chapter.children = [sequential, sequential2] - sequential.children = [problem1, problem2] - sequential2.children = [problem3] + ItemFactory.create(category='problem', parent_location=sequential2.location) url = reverse('course-home-outline-tab', args=[course.id]) CourseEnrollment.enroll(self.user, course.id) diff --git a/openedx/core/djangoapps/schedules/tests/test_content_highlights.py b/openedx/core/djangoapps/schedules/tests/test_content_highlights.py index 3d0fafeb85..20b2a8289d 100644 --- a/openedx/core/djangoapps/schedules/tests/test_content_highlights.py +++ b/openedx/core/djangoapps/schedules/tests/test_content_highlights.py @@ -166,7 +166,7 @@ class TestContentHighlights(ModuleStoreTestCase): # lint-amnesty, pylint: disab mock_get_module.return_value = None with self.store.bulk_operations(self.course_key): - self._create_chapter(highlights='Test highlight') + self._create_chapter(highlights=['Test highlight']) with self.assertRaisesRegex(CourseUpdateDoesNotExist, 'Course module .* not found'): get_week_highlights(self.user, self.course_key, 1) diff --git a/openedx/features/content_type_gating/block_transformers.py b/openedx/features/content_type_gating/block_transformers.py index 6c0213b1c9..94b66eda43 100644 --- a/openedx/features/content_type_gating/block_transformers.py +++ b/openedx/features/content_type_gating/block_transformers.py @@ -62,7 +62,9 @@ class ContentTypeGateTransformer(BlockStructureTransformer): block_key, UserPartitionTransformer, 'merged_group_access', None ) if merged_access: - current_access = merged_access.get_allowed_groups() + # merged_access holds a dictionary of sets, but group_access is a dictionary of lists, so we convert here + # (sets seem like a better format for this, but existing code already expects lists) + current_access = {p: list(g) for (p, g) in merged_access.get_allowed_groups().items()} else: # This fallback code has a bug if UserPartitionTranformer is not being used -- it does not consider # inheritance from parent blocks. This is why our class docstring recommends UserPartitionTranformer. diff --git a/openedx/features/content_type_gating/tests/test_access.py b/openedx/features/content_type_gating/tests/test_access.py index 6d70b9ad75..ddb15ac137 100644 --- a/openedx/features/content_type_gating/tests/test_access.py +++ b/openedx/features/content_type_gating/tests/test_access.py @@ -124,20 +124,23 @@ def _assert_block_is_gated(block, is_gated, user, course, request_factory, has_u assert 'content-paywall' not in content fake_request = request_factory.get('') - with patch('lms.djangoapps.course_api.blocks.api.is_request_from_mobile_app', return_value=False): - requested_fields = ['display_name', 'block_id', 'student_view_url', 'student_view_data'] - blocks = get_blocks(fake_request, course.location, user=user, requested_fields=requested_fields, student_view_data=['html']) - course_api_block = blocks['blocks'][str(block.location)] - if is_gated: - assert 'authorization_denial_reason' in course_api_block - assert "display_name" in course_api_block - assert "block_id" in course_api_block - assert "student_view_url" in course_api_block - assert "student_view_data" not in course_api_block - else: - assert 'authorization_denial_reason' not in course_api_block - if block.category == 'html': - assert 'student_view_data' in course_api_block + requested_fields = ['block_id', 'contains_gated_content', 'display_name', 'student_view_data', 'student_view_url'] + blocks = get_blocks(fake_request, course.location, user=user, requested_fields=requested_fields, + student_view_data=['html']) + course_api_block = blocks['blocks'][str(block.location)] + + assert course_api_block.get('contains_gated_content', False) == is_gated + + if is_gated: + assert 'authorization_denial_reason' in course_api_block + assert 'block_id' in course_api_block + assert 'display_name' in course_api_block + assert 'student_view_data' not in course_api_block + assert 'student_view_url' in course_api_block + else: + assert 'authorization_denial_reason' not in course_api_block + if block.category == 'html': + assert 'student_view_data' in course_api_block def _assert_block_is_empty(block, user_id, course, request_factory):