Change content_type_gating enabled_for_enrollment to return false if a course has no verified mode or a user is not enrolled in the audit mode

This commit is contained in:
Matthew Piatetsky
2019-04-10 14:07:14 -04:00
parent 05a682f55e
commit 62b9bb89c8
16 changed files with 112 additions and 88 deletions

View File

@@ -239,7 +239,7 @@ class TestFieldOverrideMongoPerformance(FieldOverridePerformanceTestCase):
__test__ = True
# TODO: decrease query count as part of REVO-28
QUERY_COUNT = 36
QUERY_COUNT = 30
TEST_DATA = {
# (providers, course_width, enable_ccx, view_as_ccx): (
# # of sql queries to default,
@@ -268,7 +268,7 @@ class TestFieldOverrideSplitPerformance(FieldOverridePerformanceTestCase):
__test__ = True
# TODO: decrease query count as part of REVO-28
QUERY_COUNT = 36
QUERY_COUNT = 30
TEST_DATA = {
('no_overrides', 1, True, False): (QUERY_COUNT, 3),
('no_overrides', 2, True, False): (QUERY_COUNT, 3),

View File

@@ -430,8 +430,8 @@ class SelfPacedCourseInfoTestCase(LoginEnrollmentTestCase, SharedModuleStoreTest
def test_num_queries_instructor_paced(self):
# TODO: decrease query count as part of REVO-28
self.fetch_course_info_with_queries(self.instructor_paced_course, 44, 3)
self.fetch_course_info_with_queries(self.instructor_paced_course, 41, 3)
def test_num_queries_self_paced(self):
# TODO: decrease query count as part of REVO-28
self.fetch_course_info_with_queries(self.self_paced_course, 44, 3)
self.fetch_course_info_with_queries(self.self_paced_course, 41, 3)

View File

@@ -213,8 +213,8 @@ class IndexQueryTestCase(ModuleStoreTestCase):
NUM_PROBLEMS = 20
@ddt.data(
(ModuleStoreEnum.Type.mongo, 10, 179),
(ModuleStoreEnum.Type.split, 4, 173),
(ModuleStoreEnum.Type.mongo, 10, 176),
(ModuleStoreEnum.Type.split, 4, 170),
)
@ddt.unpack
def test_index_query_counts(self, store_type, expected_mongo_query_count, expected_mysql_query_count):
@@ -1465,8 +1465,8 @@ class ProgressPageTests(ProgressPageBaseTests):
self.assertContains(resp, u"Download Your Certificate")
@ddt.data(
(True, 56),
(False, 55)
(True, 50),
(False, 49)
)
@ddt.unpack
def test_progress_queries_paced_courses(self, self_paced, query_count):
@@ -1479,8 +1479,8 @@ class ProgressPageTests(ProgressPageBaseTests):
@patch.dict(settings.FEATURES, {'ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS': False})
@ddt.data(
(False, 64, 44),
(True, 55, 39)
(False, 58, 38),
(True, 49, 33)
)
@ddt.unpack
def test_progress_queries(self, enable_waffle, initial, subsequent):

View File

@@ -95,7 +95,7 @@ class TestCourseGradeFactory(GradeTestBase):
[self.sequence.display_name, self.sequence2.display_name]
)
with self.assertNumQueries(5), mock_get_score(1, 2):
with self.assertNumQueries(4), mock_get_score(1, 2):
_assert_read(expected_pass=False, expected_percent=0) # start off with grade of 0
num_queries = 47
@@ -313,7 +313,7 @@ class TestGradeIteration(SharedModuleStoreTestCase):
else mock_course_grade.return_value
for student in self.students
]
with self.assertNumQueries(7):
with self.assertNumQueries(8):
all_course_grades, all_errors = self._course_grades_and_errors_for(self.course, self.students)
self.assertEqual(
{student: text_type(all_errors[student]) for student in all_errors},

View File

@@ -413,7 +413,7 @@ class TestInstructorGradeReport(InstructorGradeReportTestCase):
RequestCache.clear_all_namespaces()
expected_query_count = 48
expected_query_count = 49
with patch('lms.djangoapps.instructor_task.tasks_helper.runner._get_current_task'):
with check_mongo_calls(mongo_count):
with self.assertNumQueries(expected_query_count):

View File

@@ -44,27 +44,25 @@ class FeatureBasedEnrollmentsSupportView(View):
course = CourseOverview.objects.values('display_name').get(id=key)
duration_config = CourseDurationLimitConfig.current(course_key=key)
gating_config = ContentTypeGatingConfig.current(course_key=key)
partially_enabled = bool(duration_config.enabled) != bool(gating_config.enabled)
duration_enabled = CourseDurationLimitConfig.enabled_for_course(course_key=key)
gating_enabled = ContentTypeGatingConfig.enabled_for_course(course_key=key)
if partially_enabled:
if duration_config.enabled:
enabled = 'Course Duration Limits Only'
enabled_as_of = str(duration_config.enabled_as_of) if duration_config.enabled_as_of else 'N/A'
reason = 'Course duration limits are enabled for this course, but content type gating is disabled.'
elif gating_config.enabled:
enabled = 'Content Type Gating Only'
enabled_as_of = str(gating_config.enabled_as_of) if gating_config.enabled_as_of else 'N/A'
reason = 'Content type gating is enabled for this course, but course duration limits are disabled.'
else:
enabled = duration_config.enabled or False
enabled_as_of = str(duration_config.enabled_as_of) if duration_config.enabled_as_of else 'N/A'
reason = duration_config.provenances['enabled']
gating_dict = {
'enabled': gating_enabled,
'enabled_as_of': str(gating_config.enabled_as_of) if gating_config.enabled_as_of else 'N/A',
'reason': gating_config.provenances['enabled'].value
}
duration_dict = {
'enabled': duration_enabled,
'enabled_as_of': str(duration_config.enabled_as_of) if duration_config.enabled_as_of else 'N/A',
'reason': duration_config.provenances['enabled'].value
}
return {
'course_id': course_key,
'course_name': course.get('display_name'),
'gating_config': gating_config,
'duration_config': duration_config,
'gating_config': gating_dict,
'duration_config': duration_dict,
}
except (ObjectDoesNotExist, InvalidKeyError):

View File

@@ -49,17 +49,17 @@ ${_("Feature Based Enrollments")}
<td>${results['course_id']}</td>
<td>${results['course_name']}</td>
<td class="fb-enrollments-gating-col">
${results['gating_config'].enabled if results['gating_config'].enabled is not None else 'Unknown'}
${results['gating_config']['enabled']}
</td>
<td class="fb-enrollments-gating-col">
${results['gating_config'].enabled_as_of if results['gating_config'].enabled_as_of is not None else 'N/A'}
${results['gating_config'].get('enabled_as_of', 'N/A') if results['gating_config']['enabled'] else 'N/A'}
</td>
<td class="fb-enrollments-gating-col">
${results['gating_config'].provenances['enabled'].value}
${results['gating_config']['reason']}
</td>
<td>${results['duration_config'].enabled if results['duration_config'].enabled is not None else 'Unknown'}</td>
<td>${results['duration_config'].enabled_as_of if results['duration_config'].enabled_as_of is not None else 'N/A'}</td>
<td>${results['duration_config'].provenances['enabled'].value}</td>
<td>${results['duration_config']['enabled']}</td>
<td>${results['duration_config'].get('enabled_as_of', 'N/A') if results['duration_config']['enabled'] else 'N/A'}</td>
<td>${results['duration_config']['reason']}</td>
</tr>
</tbody>
</table>

View File

@@ -1,8 +1,10 @@
"""
Helper functions used by both content_type_gating and course_duration_limits.
"""
import logging
from xmodule.partitions.partitions import Group
from course_modes.models import CourseMode
# Studio generates partition IDs starting at 100. There is already a manually generated
# partition for Enrollment Track that uses ID 50, so we'll use 51.
@@ -14,3 +16,42 @@ CONTENT_TYPE_GATE_GROUP_IDS = {
}
LIMITED_ACCESS = Group(CONTENT_TYPE_GATE_GROUP_IDS['limited_access'], 'Limited-access Users')
FULL_ACCESS = Group(CONTENT_TYPE_GATE_GROUP_IDS['full_access'], 'Full-access Users')
LOG = logging.getLogger(__name__)
def correct_modes_for_fbe(course_key, enrollment=None, user=None):
"""
If CONTENT_TYPE_GATING is enabled use the following logic to determine whether
enabled_for_enrollment should be false
"""
if course_key is None:
return True
modes = CourseMode.modes_for_course(course_key, include_expired=True, only_selectable=False)
modes_dict = {mode.slug: mode for mode in modes}
# If there is no audit mode or no verified mode, FBE will not be enabled
if (CourseMode.AUDIT not in modes_dict) or (CourseMode.VERIFIED not in modes_dict):
return False
if enrollment and user:
mode_slug = enrollment.mode
if enrollment.is_active:
course_mode = CourseMode.mode_for_course(
course_key,
mode_slug,
modes=modes,
)
if course_mode is None:
LOG.error(
u"User %s is in an unknown CourseMode '%s'"
u" for course %s. Granting full access to content for this user",
user.username,
mode_slug,
course_key,
)
return False
if mode_slug != CourseMode.AUDIT:
return False
return True

View File

@@ -20,7 +20,7 @@ from lms.djangoapps.courseware.masquerade import (
)
from openedx.core.djangoapps.config_model_utils.models import StackedConfigurationModel
from openedx.core.djangoapps.config_model_utils.utils import is_in_holdback
from openedx.features.content_type_gating.helpers import FULL_ACCESS, LIMITED_ACCESS
from openedx.features.content_type_gating.helpers import FULL_ACCESS, LIMITED_ACCESS, correct_modes_for_fbe
from student.models import CourseEnrollment
from student.role_helpers import has_staff_roles
from xmodule.partitions.partitions import ENROLLMENT_TRACK_PARTITION_ID
@@ -137,14 +137,18 @@ class ContentTypeGatingConfig(StackedConfigurationModel):
if user_variable_represents_correct_user and is_in_holdback(user):
return False
if not correct_modes_for_fbe(course_key, enrollment, user):
return False
# enrollment might be None if the user isn't enrolled. In that case,
# return enablement as if the user enrolled today
# Also, ignore enrollment creation date if the user is masquerading.
if enrollment is None or course_masquerade:
return cls.enabled_for_course(course_key=course_key, target_datetime=timezone.now())
target_datetime = timezone.now()
else:
current_config = cls.current(course_key=enrollment.course_id)
return current_config.enabled_as_of_datetime(target_datetime=enrollment.created)
target_datetime = enrollment.created
current_config = cls.current(course_key=course_key)
return current_config.enabled_as_of_datetime(target_datetime=target_datetime)
@classmethod
def enabled_for_course(cls, course_key, target_datetime=None):
@@ -161,6 +165,8 @@ class ContentTypeGatingConfig(StackedConfigurationModel):
course_key: The CourseKey of the course being queried.
target_datetime: The datetime to checked enablement as of. Defaults to the current date and time.
"""
if not correct_modes_for_fbe(course_key):
return False
if target_datetime is None:
target_datetime = timezone.now()

View File

@@ -177,51 +177,10 @@ class ContentTypeGatingPartitionScheme(object):
"""
Returns the Group for the specified user.
"""
# For now, treat everyone as a Full-access user, until we have the rest of the
# feature gating logic in place.
if not ContentTypeGatingConfig.enabled_for_enrollment(user=user, course_key=course_key,
user_partition=user_partition):
return FULL_ACCESS
# If CONTENT_TYPE_GATING is enabled use the following logic to determine whether a user should have FULL_ACCESS
# or LIMITED_ACCESS
course_mode = apps.get_model('course_modes.CourseMode')
modes = course_mode.modes_for_course(course_key, include_expired=True, only_selectable=False)
modes_dict = {mode.slug: mode for mode in modes}
# If there is no verified mode, all users are granted FULL_ACCESS
if not course_mode.has_verified_mode(modes_dict):
return FULL_ACCESS
course_enrollment = apps.get_model('student.CourseEnrollment')
mode_slug, is_active = course_enrollment.enrollment_mode_for_user(user, course_key)
if mode_slug and is_active:
course_mode = course_mode.mode_for_course(
course_key,
mode_slug,
modes=modes,
)
if course_mode is None:
LOG.error(
u"User %s is in an unknown CourseMode '%s'"
u" for course %s. Granting full access to content for this user",
user.username,
mode_slug,
course_key,
)
return FULL_ACCESS
if mode_slug == CourseMode.AUDIT:
return LIMITED_ACCESS
else:
return FULL_ACCESS
else:
# Unenrolled users don't get gated content
return LIMITED_ACCESS
@classmethod

View File

@@ -365,7 +365,7 @@ class TestProblemTypeAccess(SharedModuleStoreTestCase):
category=component_type,
display_name=component_type,
graded=True,
metadata={} if component_type == 'html' else METADATA
metadata={} if (component_type == 'html' or len(modes) == 1) else METADATA
)
blocks_dict[component_type] = block

View File

@@ -8,6 +8,8 @@ import pytz
from edx_django_utils.cache import RequestCache
from opaque_keys.edx.locator import CourseLocator
from course_modes.tests.factories import CourseModeFactory
from openedx.core.djangoapps.config_model_utils.models import Provenance
from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory
from openedx.core.djangoapps.site_configuration.tests.factories import SiteConfigurationFactory
@@ -24,6 +26,8 @@ class TestContentTypeGatingConfig(CacheIsolationTestCase):
def setUp(self):
self.course_overview = CourseOverviewFactory.create()
CourseModeFactory.create(course_id=self.course_overview.id, mode_slug='audit')
CourseModeFactory.create(course_id=self.course_overview.id, mode_slug='verified')
self.user = UserFactory.create()
super(TestContentTypeGatingConfig, self).setUp()
@@ -73,9 +77,9 @@ class TestContentTypeGatingConfig(CacheIsolationTestCase):
user = self.user
course_key = self.course_overview.id
query_count = 6
query_count = 7
if not already_enrolled or not pass_enrollment and already_enrolled:
query_count = 7
query_count = 8
with self.assertNumQueries(query_count):
enabled = ContentTypeGatingConfig.enabled_for_enrollment(

View File

@@ -25,6 +25,8 @@ class TestContentTypeGatingPartition(CacheIsolationTestCase):
def test_create_content_gating_partition_happy_path(self):
mock_course = Mock(id=self.course_key, user_partitions={})
CourseModeFactory.create(course_id=mock_course.id, mode_slug='audit')
CourseModeFactory.create(course_id=mock_course.id, mode_slug='verified')
ContentTypeGatingConfig.objects.create(enabled=True, enabled_as_of=datetime(2018, 1, 1))
with patch('openedx.features.content_type_gating.partitions.ContentTypeGatingPartitionScheme.create_user_partition') as mock_create:
@@ -133,6 +135,7 @@ class TestContentTypeGatingPartition(CacheIsolationTestCase):
mock_request = None
mock_course = Mock(id=self.course_key, user_partitions={})
mock_block = Mock(scope_ids=Mock(usage_id=Mock(course_key=mock_course.id)))
CourseModeFactory.create(course_id=mock_course.id, mode_slug='audit')
CourseModeFactory.create(course_id=mock_course.id, mode_slug='verified')
global_staff = GlobalStaffFactory.create()
ContentTypeGatingConfig.objects.create(enabled=True, enabled_as_of=datetime(2018, 1, 1))

View File

@@ -20,7 +20,11 @@ from lms.djangoapps.courseware.masquerade import (
)
from openedx.core.djangoapps.config_model_utils.models import StackedConfigurationModel
from openedx.core.djangoapps.config_model_utils.utils import is_in_holdback
from openedx.features.content_type_gating.helpers import CONTENT_GATING_PARTITION_ID, CONTENT_TYPE_GATE_GROUP_IDS
from openedx.features.content_type_gating.helpers import (
CONTENT_GATING_PARTITION_ID,
CONTENT_TYPE_GATE_GROUP_IDS,
correct_modes_for_fbe
)
from student.models import CourseEnrollment
from student.role_helpers import has_staff_roles
from xmodule.partitions.partitions import ENROLLMENT_TRACK_PARTITION_ID
@@ -136,10 +140,14 @@ class CourseDurationLimitConfig(StackedConfigurationModel):
# When masquerading as a specific learner, course duration limits
# will be on if they are currently on for the learner.
if enrollment is None or not_student_masquerade:
return cls.enabled_for_course(course_key=course_key, target_datetime=timezone.now())
# we bypass enabled_for_course here and use enabled_as_of_datetime directly
# because the correct_modes_for_fbe for FBE check contained in enabled_for_course
# is redundant with checks done upstream of this code
target_datetime = timezone.now()
else:
current_config = cls.current(course_key=enrollment.course_id)
return current_config.enabled_as_of_datetime(target_datetime=enrollment.created)
target_datetime = enrollment.created
current_config = cls.current(course_key=course_key)
return current_config.enabled_as_of_datetime(target_datetime=target_datetime)
@classmethod
def enabled_for_course(cls, course_key, target_datetime=None):
@@ -156,6 +164,8 @@ class CourseDurationLimitConfig(StackedConfigurationModel):
course_key: The CourseKey of the course being queried.
target_datetime: The datetime to checked enablement as of. Defaults to the current date and time.
"""
if not correct_modes_for_fbe(course_key):
return False
if target_datetime is None:
target_datetime = timezone.now()

View File

@@ -11,6 +11,7 @@ from mock import Mock
import pytz
from edx_django_utils.cache import RequestCache
from course_modes.tests.factories import CourseModeFactory
from opaque_keys.edx.locator import CourseLocator
from openedx.core.djangoapps.config_model_utils.models import Provenance
from openedx.core.djangoapps.site_configuration.tests.factories import SiteConfigurationFactory
@@ -31,6 +32,8 @@ class TestCourseDurationLimitConfig(CacheIsolationTestCase):
def setUp(self):
self.course_overview = CourseOverviewFactory.create()
CourseModeFactory.create(course_id=self.course_overview.id, mode_slug='audit')
CourseModeFactory.create(course_id=self.course_overview.id, mode_slug='verified')
self.user = UserFactory.create()
super(TestCourseDurationLimitConfig, self).setUp()

View File

@@ -130,7 +130,7 @@ class TestCourseUpdatesPage(SharedModuleStoreTestCase):
# Fetch the view and verify that the query counts haven't changed
# TODO: decrease query count as part of REVO-28
with self.assertNumQueries(53, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST):
with self.assertNumQueries(50, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST):
with check_mongo_calls(4):
url = course_updates_url(self.course)
self.client.get(url)