From 6c2c896ec33155914357b887f0b771b9bf83a33b Mon Sep 17 00:00:00 2001 From: Matthew Piatetsky Date: Fri, 19 Apr 2019 10:50:06 -0400 Subject: [PATCH] allow overriding FBE group access restrictions for problems at the unit level --- .../content_type_gating/field_override.py | 8 +++ .../content_type_gating/partitions.py | 52 ++----------------- .../tests/test_partitions.py | 22 +++----- 3 files changed, 18 insertions(+), 64 deletions(-) diff --git a/openedx/features/content_type_gating/field_override.py b/openedx/features/content_type_gating/field_override.py index 88f93807e1..c0c6c38903 100644 --- a/openedx/features/content_type_gating/field_override.py +++ b/openedx/features/content_type_gating/field_override.py @@ -46,6 +46,14 @@ class ContentTypeGatingFieldOverride(FieldOverrideProvider): if original_group_access is None: original_group_access = {} + + # For Feature Based Enrollments, we want to inherit group access configurations + # from parent blocks. The use case is to allow granting access + # to all graded problems in a unit at the unit level + merged_group_access = block.get_parent().merged_group_access + if merged_group_access and CONTENT_GATING_PARTITION_ID in merged_group_access: + return original_group_access + original_group_access.setdefault( CONTENT_GATING_PARTITION_ID, [settings.CONTENT_TYPE_GATE_GROUP_IDS['full_access']] diff --git a/openedx/features/content_type_gating/partitions.py b/openedx/features/content_type_gating/partitions.py index ce1b988988..691260e26c 100644 --- a/openedx/features/content_type_gating/partitions.py +++ b/openedx/features/content_type_gating/partitions.py @@ -15,11 +15,6 @@ from django.utils.translation import ugettext_lazy as _ from web_fragments.fragment import Fragment from course_modes.models import CourseMode -from courseware.masquerade import ( - is_masquerading_as_specific_student, - is_masquerading_as_student, - get_course_masquerade, -) from lms.djangoapps.commerce.utils import EcommerceService from xmodule.partitions.partitions import UserPartition, UserPartitionError, ENROLLMENT_TRACK_PARTITION_ID from openedx.core.lib.mobile_utils import is_request_from_mobile_app @@ -82,8 +77,8 @@ class ContentTypeGatingPartition(UserPartition): 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): + if (verified_mode is None or user_group == FULL_ACCESS or + user_group in allowed_groups): return None ecommerce_checkout_link = self._get_checkout_link(user, verified_mode.sku) @@ -99,8 +94,8 @@ class ContentTypeGatingPartition(UserPartition): 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): + if (verified_mode is None or user_group == FULL_ACCESS or + user_group in allowed_groups): return None request = crum.get_current_request() @@ -109,45 +104,6 @@ class ContentTypeGatingPartition(UserPartition): else: return _(u"Graded assessments are available to Verified Track learners. Upgrade to Unlock.") - def _is_audit_enrollment(self, user, course_key): - """ - Checks if user is enrolled in `Audit` track of course or any staff member is - viewing course as in `Audit` enrollment. - """ - if self._is_masquerading_as_generic_student(user, course_key): - return self._is_masquerading_audit_enrollment(user, course_key) - return self._has_active_enrollment_in_audit_mode(user, course_key) - - def _is_masquerading_as_generic_student(self, user, course_key): - """ - Checks if user is masquerading as a generic student. - """ - return ( - is_masquerading_as_student(user, course_key) and - not is_masquerading_as_specific_student(user, course_key) - ) - - def _is_masquerading_audit_enrollment(self, user, course_key): - """ - Checks if user is masquerading as learners in `Audit` enrollment track. - """ - course_masquerade = get_course_masquerade(user, course_key) - if course_masquerade.user_partition_id == ENROLLMENT_TRACK_PARTITION_ID: - audit_mode_id = settings.COURSE_ENROLLMENT_MODES.get(CourseMode.AUDIT, {}).get('id') - return course_masquerade.group_id == audit_mode_id - if course_masquerade.user_partition_id == CONTENT_GATING_PARTITION_ID: - limited_access_group_id = LIMITED_ACCESS.id - return course_masquerade.group_id == limited_access_group_id - return False - - def _has_active_enrollment_in_audit_mode(self, user, course_key): - """ - Checks if user has an audit and active enrollment in the given course. - """ - course_enrollment = apps.get_model('student.CourseEnrollment') - mode_slug, is_active = course_enrollment.enrollment_mode_for_user(user, course_key) - return mode_slug == CourseMode.AUDIT and is_active - def _get_checkout_link(self, user, sku): ecomm_service = EcommerceService() ecommerce_checkout = ecomm_service.is_enabled(user) diff --git a/openedx/features/content_type_gating/tests/test_partitions.py b/openedx/features/content_type_gating/tests/test_partitions.py index 71b10f3d02..1fdd41442d 100644 --- a/openedx/features/content_type_gating/tests/test_partitions.py +++ b/openedx/features/content_type_gating/tests/test_partitions.py @@ -3,12 +3,11 @@ from mock import Mock, patch from django.conf import settings from django.test import RequestFactory -from courseware.masquerade import CourseMasquerade from course_modes.tests.factories import CourseModeFactory from lms.djangoapps.courseware.tests.factories import GlobalStaffFactory from opaque_keys.edx.keys import CourseKey from openedx.core.djangolib.testing.utils import CacheIsolationTestCase -from openedx.features.content_type_gating.helpers import CONTENT_GATING_PARTITION_ID, FULL_ACCESS +from openedx.features.content_type_gating.helpers import CONTENT_GATING_PARTITION_ID, FULL_ACCESS, LIMITED_ACCESS from openedx.features.content_type_gating.partitions import ( create_content_gating_partition, ContentTypeGatingPartition @@ -67,7 +66,7 @@ class TestContentTypeGatingPartition(CacheIsolationTestCase): def test_access_denied_fragment_for_masquerading(self): """ - Test that a global staff sees gated content flag when viewing course as `Learner in Audit` + Test that a global staff sees gated content flag when viewing course as `Learner in Limited Access` Note: Global staff doesn't require to be enrolled in course. """ mock_request = RequestFactory().get('/') @@ -75,8 +74,8 @@ class TestContentTypeGatingPartition(CacheIsolationTestCase): mock_block = Mock(scope_ids=Mock(usage_id=Mock(course_key=mock_course.id))) mock_course_masquerade = Mock( role='student', - user_partition_id=ENROLLMENT_TRACK_PARTITION_ID, - group_id=settings.COURSE_ENROLLMENT_MODES['audit']['id'], + user_partition_id=CONTENT_GATING_PARTITION_ID, + group_id=LIMITED_ACCESS.id, user_name=None ) CourseModeFactory.create(course_id=mock_course.id, mode_slug='verified') @@ -87,16 +86,10 @@ class TestContentTypeGatingPartition(CacheIsolationTestCase): partition = create_content_gating_partition(mock_course) with patch( - 'courseware.masquerade.get_course_masquerade', - return_value=mock_course_masquerade - ), patch( - 'openedx.features.content_type_gating.partitions.get_course_masquerade', - return_value=mock_course_masquerade - ), patch( 'crum.get_current_request', return_value=mock_request ): - fragment = partition.access_denied_fragment(mock_block, global_staff, GroupFactory(), 'test_allowed_group') + fragment = partition.access_denied_fragment(mock_block, global_staff, LIMITED_ACCESS, [FULL_ACCESS]) self.assertIsNotNone(fragment) @@ -144,10 +137,7 @@ class TestContentTypeGatingPartition(CacheIsolationTestCase): with patch( 'crum.get_current_request', return_value=mock_request - ), patch( - 'openedx.features.content_type_gating.partitions.ContentTypeGatingPartition._is_audit_enrollment', - return_value=True ): - fragment = partition.access_denied_fragment(mock_block, global_staff, GroupFactory(), 'test_allowed_group') + fragment = partition.access_denied_fragment(mock_block, global_staff, LIMITED_ACCESS, [FULL_ACCESS]) self.assertIsNotNone(fragment)