From ec28a75f14f035d3848d25ef815f46b142997a9b Mon Sep 17 00:00:00 2001 From: zubair-arbi Date: Fri, 31 Jul 2015 19:23:46 +0500 Subject: [PATCH] In-course reverification access control * Automatically create user partitions on course publish for each ICRV checkpoint. * Disable partitions for ICRV checkpoints that have been deleted. * Skip partitions that have been disabled when checking access. * Add verification access control UI to visibility settings. * Add verification access control UI to sequential and vertical settings. * Add partition scheme for verification partition groups. * Cache information used by verification partition scheme and invalidate the cache on update. * Add location parameter to UserPartition so the partition scheme can find the associated checkpoint. * Refactor GroupConfiguration to allow multiple user partitions. * Add special messaging to ICRV for students in the honor track. Authors: Zubair Arbi, Awais Qureshi, Aamir Khan, Will Daly --- .../contentstore/course_group_config.py | 78 +++-- .../contentstore/tests/test_utils.py | 208 ++++++++++++++ cms/djangoapps/contentstore/utils.py | 165 ++++++++++- cms/djangoapps/contentstore/views/item.py | 24 +- .../views/tests/test_group_configurations.py | 69 +++++ .../contentstore/views/tests/test_item.py | 60 +++- cms/lib/xblock/test/test_authoring_mixin.py | 31 ++ cms/static/js/models/xblock_info.js | 29 +- .../js/views/modals/course_outline_modals.js | 113 +++++++- cms/static/js/xblock/authoring.js | 49 +++- cms/static/sass/elements/_modal-window.scss | 29 +- cms/templates/course_outline.html | 2 +- .../js/verification-access-editor.underscore | 44 +++ cms/templates/visibility_editor.html | 113 +++++--- common/djangoapps/student/models.py | 47 +++ common/lib/xmodule/xmodule/course_module.py | 36 +++ .../xmodule/xmodule/modulestore/__init__.py | 17 ++ .../lib/xmodule/xmodule/modulestore/django.py | 2 + .../xmodule/xmodule/partitions/partitions.py | 51 +++- .../partitions/tests/test_partitions.py | 109 ++++++- .../pages/studio/component_editor.py | 8 +- .../tests/test_field_override_performance.py | 40 +-- lms/djangoapps/courseware/access.py | 28 +- lms/djangoapps/courseware/testutils.py | 2 +- lms/djangoapps/lms_xblock/mixin.py | 12 +- lms/djangoapps/verify_student/models.py | 99 ++++++- lms/djangoapps/verify_student/services.py | 25 +- .../verify_student/tests/test_models.py | 22 +- .../verify_student/tests/test_services.py | 20 ++ .../courseware/course_navigation.html | 2 +- .../cohort_management.html | 2 +- .../course_groups/partition_scheme.py | 4 +- .../tests/test_partition_scheme.py | 4 +- .../djangoapps/credit/partition_schemes.py | 136 +++++++++ openedx/core/djangoapps/credit/signals.py | 24 +- openedx/core/djangoapps/credit/tasks.py | 26 +- .../djangoapps/credit/tests/test_partition.py | 181 ++++++++++++ .../djangoapps/credit/tests/test_tasks.py | 2 +- .../credit/tests/test_verification_access.py | 270 ++++++++++++++++++ openedx/core/djangoapps/credit/utils.py | 38 +++ .../djangoapps/credit/verification_access.py | 187 ++++++++++++ requirements/edx/github.txt | 2 +- setup.py | 4 +- 43 files changed, 2174 insertions(+), 240 deletions(-) create mode 100644 cms/templates/js/verification-access-editor.underscore create mode 100644 openedx/core/djangoapps/credit/partition_schemes.py create mode 100644 openedx/core/djangoapps/credit/tests/test_partition.py create mode 100644 openedx/core/djangoapps/credit/tests/test_verification_access.py create mode 100644 openedx/core/djangoapps/credit/utils.py create mode 100644 openedx/core/djangoapps/credit/verification_access.py diff --git a/cms/djangoapps/contentstore/course_group_config.py b/cms/djangoapps/contentstore/course_group_config.py index 69b2051a72..2056ce91cf 100644 --- a/cms/djangoapps/contentstore/course_group_config.py +++ b/cms/djangoapps/contentstore/course_group_config.py @@ -199,6 +199,8 @@ class GroupConfiguration(object): """ Returns all units names and their urls. + This will return only groups for the cohort user partition. + Returns: {'group_id': [ @@ -214,25 +216,22 @@ class GroupConfiguration(object): } """ usage_info = {} - for item in items: - if hasattr(item, 'group_access') and item.group_access: - (__, group_ids), = item.group_access.items() - for group_id in group_ids: - if group_id not in usage_info: - usage_info[group_id] = [] + for item, group_id in GroupConfiguration._iterate_items_and_content_group_ids(course, items): + if group_id not in usage_info: + usage_info[group_id] = [] - unit = item.get_parent() - if not unit: - log.warning("Unable to find parent for component %s", item.location) - continue + unit = item.get_parent() + if not unit: + log.warning("Unable to find parent for component %s", item.location) + continue - usage_info = GroupConfiguration._get_usage_info( - course, - unit=unit, - item=item, - usage_info=usage_info, - group_id=group_id - ) + usage_info = GroupConfiguration._get_usage_info( + course, + unit=unit, + item=item, + usage_info=usage_info, + group_id=group_id + ) return usage_info @@ -250,6 +249,8 @@ class GroupConfiguration(object): """ Returns all items names and their urls. + This will return only groups for the cohort user partition. + Returns: {'group_id': [ @@ -265,23 +266,38 @@ class GroupConfiguration(object): } """ usage_info = {} - for item in items: - if hasattr(item, 'group_access') and item.group_access: - (__, group_ids), = item.group_access.items() - for group_id in group_ids: - if group_id not in usage_info: - usage_info[group_id] = [] + for item, group_id in GroupConfiguration._iterate_items_and_content_group_ids(course, items): + if group_id not in usage_info: + usage_info[group_id] = [] - usage_info = GroupConfiguration._get_usage_info( - course, - unit=item, - item=item, - usage_info=usage_info, - group_id=group_id - ) + usage_info = GroupConfiguration._get_usage_info( + course, + unit=item, + item=item, + usage_info=usage_info, + group_id=group_id + ) return usage_info + @staticmethod + def _iterate_items_and_content_group_ids(course, items): + """ + Iterate through items and content group IDs in a course. + + This will yield group IDs *only* for cohort user partitions. + + Yields: tuple of (item, group_id) + """ + content_group_configuration = get_cohorted_user_partition(course) + if content_group_configuration is not None: + for item in items: + if hasattr(item, 'group_access') and item.group_access: + group_ids = item.group_access.get(content_group_configuration.id, []) + + for group_id in group_ids: + yield item, group_id + @staticmethod def update_usage_info(store, course, configuration): """ @@ -329,7 +345,7 @@ class GroupConfiguration(object): the client explicitly creates a group within the partition and POSTs back. """ - content_group_configuration = get_cohorted_user_partition(course.id) + content_group_configuration = get_cohorted_user_partition(course) if content_group_configuration is None: content_group_configuration = UserPartition( id=generate_int_id(MINIMUM_GROUP_ID, MYSQL_MAX_INT, GroupConfiguration.get_used_ids(course)), diff --git a/cms/djangoapps/contentstore/tests/test_utils.py b/cms/djangoapps/contentstore/tests/test_utils.py index 69b8fc6a69..2ed22b6a3c 100644 --- a/cms/djangoapps/contentstore/tests/test_utils.py +++ b/cms/djangoapps/contentstore/tests/test_utils.py @@ -12,6 +12,7 @@ from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from opaque_keys.edx.locations import SlashSeparatedCourseKey from xmodule.modulestore.django import modulestore +from xmodule.partitions.partitions import UserPartition, Group from contentstore import utils from contentstore.tests.utils import CourseTestCase @@ -413,6 +414,43 @@ class GroupVisibilityTest(CourseTestCase): self.html = self.store.get_item(html.location) self.problem = self.store.get_item(problem.location) + # Add partitions to the course + self.course.user_partitions = [ + UserPartition( + id=0, + name="Partition 0", + description="Partition 0", + scheme=UserPartition.get_scheme("random"), + groups=[ + Group(id=0, name="Group A"), + Group(id=1, name="Group B"), + ], + ), + UserPartition( + id=1, + name="Partition 1", + description="Partition 1", + scheme=UserPartition.get_scheme("random"), + groups=[ + Group(id=0, name="Group C"), + Group(id=1, name="Group D"), + ], + ), + UserPartition( + id=2, + name="Partition 2", + description="Partition 2", + scheme=UserPartition.get_scheme("random"), + groups=[ + Group(id=0, name="Group E"), + Group(id=1, name="Group F"), + Group(id=2, name="Group G"), + Group(id=3, name="Group H"), + ], + ), + ] + self.course = self.store.update_item(self.course, ModuleStoreEnum.UserID.test) + def set_group_access(self, xblock, value): """ Sets group_access to specified value and calls update_item to persist the change. """ xblock.group_access = value @@ -452,3 +490,173 @@ class GroupVisibilityTest(CourseTestCase): self.assertFalse(utils.is_visible_to_specific_content_groups(self.vertical)) self.assertFalse(utils.is_visible_to_specific_content_groups(self.html)) self.assertTrue(utils.is_visible_to_specific_content_groups(self.problem)) + + +class GetUserPartitionInfoTest(ModuleStoreTestCase): + """ + Tests for utility function that retrieves user partition info + and formats it for consumption by the editing UI. + """ + + def setUp(self): + """Create a dummy course. """ + super(GetUserPartitionInfoTest, self).setUp() + self.course = CourseFactory() + self.block = ItemFactory.create(category="problem", parent_location=self.course.location) # pylint: disable=no-member + + # Set up some default partitions + self._set_partitions([ + UserPartition( + id=0, + name="Cohort user partition", + scheme=UserPartition.get_scheme("cohort"), + description="Cohorted user partition", + groups=[ + Group(id=0, name="Group A"), + Group(id=1, name="Group B"), + ], + ), + UserPartition( + id=1, + name="Random user partition", + scheme=UserPartition.get_scheme("random"), + description="Random user partition", + groups=[ + Group(id=0, name="Group C"), + ], + ), + ]) + + def test_retrieves_partition_info_with_selected_groups(self): + # Initially, no group access is set on the block, so no groups should + # be marked as selected. + expected = [ + { + "id": 0, + "name": "Cohort user partition", + "scheme": "cohort", + "groups": [ + { + "id": 0, + "name": "Group A", + "selected": False, + "deleted": False, + }, + { + "id": 1, + "name": "Group B", + "selected": False, + "deleted": False, + }, + ] + }, + { + "id": 1, + "name": "Random user partition", + "scheme": "random", + "groups": [ + { + "id": 0, + "name": "Group C", + "selected": False, + "deleted": False, + }, + ] + } + ] + self.assertEqual(self._get_partition_info(), expected) + + # Update group access and expect that now one group is marked as selected. + self._set_group_access({0: [1]}) + expected[0]["groups"][1]["selected"] = True + self.assertEqual(self._get_partition_info(), expected) + + def test_deleted_groups(self): + # Select a group that is not defined in the partition + self._set_group_access({0: [3]}) + + # Expect that the group appears as selected but is marked as deleted + partitions = self._get_partition_info() + groups = partitions[0]["groups"] + self.assertEqual(len(groups), 3) + self.assertEqual(groups[2], { + "id": 3, + "name": "Deleted group", + "selected": True, + "deleted": True + }) + + def test_filter_by_partition_scheme(self): + partitions = self._get_partition_info(schemes=["random"]) + self.assertEqual(len(partitions), 1) + self.assertEqual(partitions[0]["scheme"], "random") + + def test_exclude_inactive_partitions(self): + # Include an inactive verification scheme + self._set_partitions([ + UserPartition( + id=0, + name="Cohort user partition", + scheme=UserPartition.get_scheme("cohort"), + description="Cohorted user partition", + groups=[ + Group(id=0, name="Group A"), + Group(id=1, name="Group B"), + ], + ), + UserPartition( + id=1, + name="Verification user partition", + scheme=UserPartition.get_scheme("verification"), + description="Verification user partition", + groups=[ + Group(id=0, name="Group C"), + ], + active=False, + ), + ]) + + # Expect that the inactive scheme is excluded from the results + partitions = self._get_partition_info() + self.assertEqual(len(partitions), 1) + self.assertEqual(partitions[0]["scheme"], "cohort") + + def test_exclude_partitions_with_no_groups(self): + # The cohort partition has no groups defined + self._set_partitions([ + UserPartition( + id=0, + name="Cohort user partition", + scheme=UserPartition.get_scheme("cohort"), + description="Cohorted user partition", + groups=[], + ), + UserPartition( + id=1, + name="Verification user partition", + scheme=UserPartition.get_scheme("verification"), + description="Verification user partition", + groups=[ + Group(id=0, name="Group C"), + ], + ), + ]) + + # Expect that the partition with no groups is excluded from the results + partitions = self._get_partition_info() + self.assertEqual(len(partitions), 1) + self.assertEqual(partitions[0]["scheme"], "verification") + + def _set_partitions(self, partitions): + """Set the user partitions of the course descriptor. """ + self.course.user_partitions = partitions + self.course = self.store.update_item(self.course, ModuleStoreEnum.UserID.test) + + def _set_group_access(self, group_access): + """Set group access of the block. """ + self.block.group_access = group_access + self.block = self.store.update_item(self.block, ModuleStoreEnum.UserID.test) + + def _get_partition_info(self, schemes=None): + """Retrieve partition info and selected groups. """ + return utils.get_user_partition_info(self.block, schemes=schemes) diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 9534f34d28..eba7ecc678 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -10,6 +10,7 @@ from pytz import UTC from django.conf import settings from django.core.urlresolvers import reverse +from django.utils.translation import ugettext as _ from django_comment_common.models import assign_default_role from django_comment_common.utils import seed_permissions_roles @@ -213,12 +214,11 @@ def is_visible_to_specific_content_groups(xblock): """ if not xblock.group_access: return False - for __, value in xblock.group_access.iteritems(): - # value should be a list of group IDs. If it is an empty list or None, the xblock is visible - # to all groups in that particular partition. So if value is a truthy value, the xblock is - # restricted in some way. - if value: + + for partition in get_user_partition_info(xblock): + if any(g["selected"] for g in partition["groups"]): return True + return False @@ -331,3 +331,158 @@ def has_active_web_certificate(course): cert_config = True break return cert_config + + +def get_user_partition_info(xblock, schemes=None, course=None): + """ + Retrieve user partition information for an XBlock for display in editors. + + * If a partition has been disabled, it will be excluded from the results. + + * If a group within a partition is referenced by the XBlock, but the group has been deleted, + the group will be marked as deleted in the results. + + Arguments: + xblock (XBlock): The courseware component being edited. + + Keyword Arguments: + schemes (iterable of str): If provided, filter partitions to include only + schemes with the provided names. + + course (XBlock): The course descriptor. If provided, uses this to look up the user partitions + instead of loading the course. This is useful if we're calling this function multiple + times for the same course want to minimize queries to the modulestore. + + Returns: list + + Example Usage: + >>> get_user_partition_info(block, schemes=["cohort", "verification"]) + [ + { + "id": 12345, + "name": "Cohorts" + "scheme": "cohort", + "groups": [ + { + "id": 7890, + "name": "Foo", + "selected": True, + "deleted": False, + } + ] + }, + { + "id": 7292, + "name": "Midterm A", + "scheme": "verification", + "groups": [ + { + "id": 1, + "name": "Completed verification at Midterm A", + "selected": False, + "deleted": False + }, + { + "id": 0, + "name": "Did not complete verification at Midterm A", + "selected": False, + "deleted": False, + } + ] + } + ] + + """ + course = course or modulestore().get_course(xblock.location.course_key) + + if course is None: + log.warning( + "Could not find course %s to retrieve user partition information", + xblock.location.course_key + ) + return [] + + if schemes is not None: + schemes = set(schemes) + + partitions = [] + for p in sorted(course.user_partitions, key=lambda p: p.name): + + # Exclude disabled partitions, partitions with no groups defined + # Also filter by scheme name if there's a filter defined. + if p.active and p.groups and (schemes is None or p.scheme.name in schemes): + + # First, add groups defined by the partition + groups = [] + for g in p.groups: + + # Falsey group access for a partition mean that all groups + # are selected. In the UI, though, we don't show the particular + # groups selected, since there's a separate option for "all users". + selected_groups = set(xblock.group_access.get(p.id, []) or []) + groups.append({ + "id": g.id, + "name": g.name, + "selected": g.id in selected_groups, + "deleted": False, + }) + + # Next, add any groups set on the XBlock that have been deleted + all_groups = set(g.id for g in p.groups) + missing_group_ids = selected_groups - all_groups + for gid in missing_group_ids: + groups.append({ + "id": gid, + "name": _("Deleted group"), + "selected": True, + "deleted": True, + }) + + # Put together the entire partition dictionary + partitions.append({ + "id": p.id, + "name": p.name, + "scheme": p.scheme.name, + "groups": groups, + }) + + return partitions + + +def get_visibility_partition_info(xblock): + """ + Retrieve user partition information for the component visibility editor. + + This pre-processes partition information to simplify the template. + + Arguments: + xblock (XBlock): The component being edited. + + Returns: dict + + """ + user_partitions = get_user_partition_info(xblock, schemes=["verification", "cohort"]) + cohort_partitions = [] + verification_partitions = [] + has_selected_groups = False + selected_verified_partition_id = None + + # Pre-process the partitions to make it easier to display the UI + for p in user_partitions: + has_selected = any(g["selected"] for g in p["groups"]) + has_selected_groups = has_selected_groups or has_selected + + if p["scheme"] == "cohort": + cohort_partitions.append(p) + elif p["scheme"] == "verification": + verification_partitions.append(p) + if has_selected: + selected_verified_partition_id = p["id"] + + return { + "user_partitions": user_partitions, + "cohort_partitions": cohort_partitions, + "verification_partitions": verification_partitions, + "has_selected_groups": has_selected_groups, + "selected_verified_partition_id": selected_verified_partition_id, + } diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 04bf559e65..debc327ab5 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -40,8 +40,11 @@ from util.date_utils import get_default_time_display from util.json_request import expect_json, JsonResponse from student.auth import has_studio_write_access, has_studio_read_access -from contentstore.utils import find_release_date_source, find_staff_lock_source, is_currently_visible_to_students, \ - ancestor_has_staff_lock, has_children_visible_to_specific_content_groups +from contentstore.utils import ( + find_release_date_source, find_staff_lock_source, is_currently_visible_to_students, + ancestor_has_staff_lock, has_children_visible_to_specific_content_groups, + get_user_partition_info, +) from contentstore.views.helpers import is_unit, xblock_studio_url, xblock_primary_child_category, \ xblock_type_display_name, get_parent_xblock, create_xblock, usage_key_with_run from contentstore.views.preview import get_preview_fragment @@ -756,7 +759,7 @@ def _get_module_info(xblock, rewrite_static_links=True, include_ancestor_info=Fa def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=False, include_child_info=False, course_outline=False, include_children_predicate=NEVER, parent_xblock=None, graders=None, - user=None): + user=None, course=None): """ Creates the information needed for client-side XBlockInfo. @@ -788,6 +791,11 @@ def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=F # Filter the graders data as needed graders = _filter_entrance_exam_grader(graders) + # We need to load the course in order to retrieve user partition information. + # For this reason, we load the course once and re-use it when recursively loading children. + if course is None: + course = modulestore().get_course(xblock.location.course_key) + # Compute the child info first so it can be included in aggregate information for the parent should_visit_children = include_child_info and (course_outline and not is_xblock_unit or not course_outline) if should_visit_children and xblock.has_children: @@ -796,7 +804,8 @@ def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=F course_outline, graders, include_children_predicate=include_children_predicate, - user=user + user=user, + course=course ) else: child_info = None @@ -850,6 +859,8 @@ def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=F "has_changes": has_changes, "actions": xblock_actions, "explanatory_message": explanatory_message, + "group_access": xblock.group_access, + "user_partitions": get_user_partition_info(xblock, course=course), } # update xblock_info with proctored_exam information if the feature flag is enabled @@ -1023,7 +1034,7 @@ def _create_xblock_ancestor_info(xblock, course_outline): } -def _create_xblock_child_info(xblock, course_outline, graders, include_children_predicate=NEVER, user=None): +def _create_xblock_child_info(xblock, course_outline, graders, include_children_predicate=NEVER, user=None, course=None): # pylint: disable=line-too-long """ Returns information about the children of an xblock, as well as about the primary category of xblock expected as children. @@ -1042,7 +1053,8 @@ def _create_xblock_child_info(xblock, course_outline, graders, include_children_ include_children_predicate=include_children_predicate, parent_xblock=xblock, graders=graders, - user=user + user=user, + course=course, ) for child in xblock.get_children() ] return child_info diff --git a/cms/djangoapps/contentstore/views/tests/test_group_configurations.py b/cms/djangoapps/contentstore/views/tests/test_group_configurations.py index f6ea5f21a9..a498ac86f8 100644 --- a/cms/djangoapps/contentstore/views/tests/test_group_configurations.py +++ b/cms/djangoapps/contentstore/views/tests/test_group_configurations.py @@ -262,6 +262,8 @@ class GroupConfigurationsListHandlerTestCase(CourseTestCase, GroupConfigurations {u'name': u'Group A', u'version': 1}, {u'name': u'Group B', u'version': 1}, ], + u'parameters': {}, + u'active': True } response = self.client.ajax_post( self._url(), @@ -283,6 +285,7 @@ class GroupConfigurationsListHandlerTestCase(CourseTestCase, GroupConfigurations self.assertEqual(len(user_partititons[0].groups), 2) self.assertEqual(user_partititons[0].groups[0].name, u'Group A') self.assertEqual(user_partititons[0].groups[1].name, u'Group B') + self.assertEqual(user_partititons[0].parameters, {}) def test_lazily_creates_cohort_configuration(self): """ @@ -327,6 +330,8 @@ class GroupConfigurationsDetailHandlerTestCase(CourseTestCase, GroupConfiguratio {u'id': 0, u'name': u'Group A', u'version': 1, u'usage': []}, {u'id': 1, u'name': u'Group B', u'version': 1, u'usage': []}, ], + u'parameters': {}, + u'active': True, } response = self.client.put( self._url(cid=666), @@ -346,6 +351,7 @@ class GroupConfigurationsDetailHandlerTestCase(CourseTestCase, GroupConfiguratio self.assertEqual(len(user_partitions[0].groups), 2) self.assertEqual(user_partitions[0].groups[0].name, u'Group A') self.assertEqual(user_partitions[0].groups[1].name, u'Group B') + self.assertEqual(user_partitions[0].parameters, {}) def test_can_edit_content_group(self): """ @@ -364,6 +370,8 @@ class GroupConfigurationsDetailHandlerTestCase(CourseTestCase, GroupConfiguratio {u'id': 0, u'name': u'New Group Name', u'version': 1, u'usage': []}, {u'id': 2, u'name': u'Group C', u'version': 1, u'usage': []}, ], + u'parameters': {}, + u'active': True, } response = self.client.put( @@ -385,6 +393,7 @@ class GroupConfigurationsDetailHandlerTestCase(CourseTestCase, GroupConfiguratio self.assertEqual(len(user_partititons[0].groups), 2) self.assertEqual(user_partititons[0].groups[0].name, u'New Group Name') self.assertEqual(user_partititons[0].groups[1].name, u'Group C') + self.assertEqual(user_partititons[0].parameters, {}) def test_can_delete_content_group(self): """ @@ -466,6 +475,8 @@ class GroupConfigurationsDetailHandlerTestCase(CourseTestCase, GroupConfiguratio {u'id': 1, u'name': u'Group B', u'version': 1}, ], u'usage': [], + u'parameters': {}, + u'active': True, } response = self.client.put( @@ -485,6 +496,7 @@ class GroupConfigurationsDetailHandlerTestCase(CourseTestCase, GroupConfiguratio self.assertEqual(len(user_partitions[0].groups), 2) self.assertEqual(user_partitions[0].groups[0].name, u'Group A') self.assertEqual(user_partitions[0].groups[1].name, u'Group B') + self.assertEqual(user_partitions[0].parameters, {}) def test_can_edit_group_configuration(self): """ @@ -504,6 +516,8 @@ class GroupConfigurationsDetailHandlerTestCase(CourseTestCase, GroupConfiguratio {u'id': 2, u'name': u'Group C', u'version': 1}, ], u'usage': [], + u'parameters': {}, + u'active': True, } response = self.client.put( @@ -525,6 +539,7 @@ class GroupConfigurationsDetailHandlerTestCase(CourseTestCase, GroupConfiguratio self.assertEqual(len(user_partititons[0].groups), 2) self.assertEqual(user_partititons[0].groups[0].name, u'New Group Name') self.assertEqual(user_partititons[0].groups[1].name, u'Group C') + self.assertEqual(user_partititons[0].parameters, {}) def test_can_delete_group_configuration(self): """ @@ -609,6 +624,8 @@ class GroupConfigurationsUsageInfoTestCase(CourseTestCase, HelperMethods): {'id': 1, 'name': 'Group B', 'version': 1, 'usage': usage_for_group}, {'id': 2, 'name': 'Group C', 'version': 1, 'usage': []}, ], + u'parameters': {}, + u'active': True, } def test_content_group_not_used(self): @@ -701,6 +718,8 @@ class GroupConfigurationsUsageInfoTestCase(CourseTestCase, HelperMethods): {'id': 2, 'name': 'Group C', 'version': 1}, ], 'usage': [], + 'parameters': {}, + 'active': True, }] self.assertEqual(actual, expected) @@ -730,6 +749,8 @@ class GroupConfigurationsUsageInfoTestCase(CourseTestCase, HelperMethods): 'label': 'Test Unit 0 / Test Content Experiment 0', 'validation': None, }], + 'parameters': {}, + 'active': True, }, { 'id': 1, 'name': 'Name 1', @@ -742,6 +763,8 @@ class GroupConfigurationsUsageInfoTestCase(CourseTestCase, HelperMethods): {'id': 2, 'name': 'Group C', 'version': 1}, ], 'usage': [], + 'parameters': {}, + 'active': True, }] self.assertEqual(actual, expected) @@ -772,6 +795,8 @@ class GroupConfigurationsUsageInfoTestCase(CourseTestCase, HelperMethods): 'label': u"Test Unit 0 / Test Content Experiment 0JOSÉ ANDRÉS", 'validation': None, }], + 'parameters': {}, + 'active': True, }] self.assertEqual(actual, expected) @@ -807,6 +832,8 @@ class GroupConfigurationsUsageInfoTestCase(CourseTestCase, HelperMethods): 'label': 'Test Unit 1 / Test Content Experiment 1', 'validation': None, }], + 'parameters': {}, + 'active': True, }] self.assertEqual(actual, expected) @@ -829,6 +856,48 @@ class GroupConfigurationsUsageInfoTestCase(CourseTestCase, HelperMethods): actual = GroupConfiguration.get_content_experiment_usage_info(self.store, self.course) self.assertEqual(actual, {0: []}) + def test_can_handle_multiple_partitions(self): + # Create the user partitions + self.course.user_partitions = [ + UserPartition( + id=0, + name='Cohort user partition', + scheme=UserPartition.get_scheme('cohort'), + description='Cohorted user partition', + groups=[ + Group(id=0, name="Group A"), + Group(id=1, name="Group B"), + ], + ), + UserPartition( + id=1, + name='Random user partition', + scheme=UserPartition.get_scheme('random'), + description='Random user partition', + groups=[ + Group(id=0, name="Group A"), + Group(id=1, name="Group B"), + ], + ), + ] + self.store.update_item(self.course, ModuleStoreEnum.UserID.test) + + # Assign group access rules for multiple partitions, one of which is a cohorted partition + __, problem = self._create_problem_with_content_group(0, 1) + problem.group_access = { + 0: [0], + 1: [1], + } + self.store.update_item(problem, ModuleStoreEnum.UserID.test) + + # This used to cause an exception since the code assumed that + # only one partition would be available. + actual = GroupConfiguration.get_content_groups_usage_info(self.store, self.course) + self.assertEqual(actual.keys(), [0]) + + actual = GroupConfiguration.get_content_groups_items_usage_info(self.store, self.course) + self.assertEqual(actual.keys(), [0]) + class GroupConfigurationsValidationTestCase(CourseTestCase, HelperMethods): """ diff --git a/cms/djangoapps/contentstore/views/tests/test_item.py b/cms/djangoapps/contentstore/views/tests/test_item.py index fec27e64d6..f633e0b170 100644 --- a/cms/djangoapps/contentstore/views/tests/test_item.py +++ b/cms/djangoapps/contentstore/views/tests/test_item.py @@ -119,9 +119,9 @@ class GetItemTest(ItemTest): return resp @ddt.data( - (1, 16, 14, 15, 11), - (2, 16, 14, 15, 11), - (3, 16, 14, 15, 11), + (1, 17, 15, 16, 12), + (2, 17, 15, 16, 12), + (3, 17, 15, 16, 12), ) @ddt.unpack def test_get_query_count(self, branching_factor, chapter_queries, section_queries, unit_queries, problem_queries): @@ -137,9 +137,9 @@ class GetItemTest(ItemTest): self.client.get(reverse_usage_url('xblock_handler', self.populated_usage_keys['problem'][-1])) @ddt.data( - (1, 26), - (2, 28), - (3, 30), + (1, 30), + (2, 32), + (3, 34), ) @ddt.unpack def test_container_get_query_count(self, branching_factor, unit_queries,): @@ -310,6 +310,52 @@ class GetItemTest(ItemTest): content_contains="Couldn't parse paging parameters" ) + def test_get_user_partitions_and_groups(self): + self.course.user_partitions = [ + UserPartition( + id=0, + name="Verification user partition", + scheme=UserPartition.get_scheme("verification"), + description="Verification user partition", + groups=[ + Group(id=0, name="Group A"), + Group(id=1, name="Group B"), + ], + ), + ] + self.store.update_item(self.course, self.user.id) + + # Create an item and retrieve it + resp = self.create_xblock(category='vertical') + usage_key = self.response_usage_key(resp) + resp = self.client.get(reverse_usage_url('xblock_handler', usage_key)) + self.assertEqual(resp.status_code, 200) + + # Check that the partition and group information was returned + result = json.loads(resp.content) + self.assertEqual(result["user_partitions"], [ + { + "id": 0, + "name": "Verification user partition", + "scheme": "verification", + "groups": [ + { + "id": 0, + "name": "Group A", + "selected": False, + "deleted": False, + }, + { + "id": 1, + "name": "Group B", + "selected": False, + "deleted": False, + }, + ] + } + ]) + self.assertEqual(result["group_access"], {}) + @ddt.ddt class DeleteItem(ItemTest): @@ -1414,7 +1460,7 @@ class TestXBlockInfo(ItemTest): @ddt.data( (ModuleStoreEnum.Type.split, 5, 5), - (ModuleStoreEnum.Type.mongo, 4, 6), + (ModuleStoreEnum.Type.mongo, 5, 7), ) @ddt.unpack def test_xblock_outline_handler_mongo_calls(self, store_type, chapter_queries, chapter_queries_1): diff --git a/cms/lib/xblock/test/test_authoring_mixin.py b/cms/lib/xblock/test/test_authoring_mixin.py index 26c41dc480..f9eb62ff5c 100644 --- a/cms/lib/xblock/test/test_authoring_mixin.py +++ b/cms/lib/xblock/test/test_authoring_mixin.py @@ -56,6 +56,26 @@ class AuthoringMixinTestCase(ModuleStoreTestCase): self.course.user_partitions = [self.content_partition] self.store.update_item(self.course, self.user.id) + def create_verification_user_partitions(self, checkpoint_names): + """ + Create user partitions for verification checkpoints. + """ + scheme = UserPartition.get_scheme("verification") + self.course.user_partitions = [ + UserPartition( + id=0, + name=checkpoint_name, + description="Verification checkpoint", + scheme=scheme, + groups=[ + Group(scheme.ALLOW, "Completed verification at {}".format(checkpoint_name)), + Group(scheme.DENY, "Did not complete verification at {}".format(checkpoint_name)), + ], + ) + for checkpoint_name in checkpoint_names + ] + self.store.update_item(self.course, self.user.id) + def set_staff_only(self, item_location): """Make an item visible to staff only.""" item = self.store.get_item(item_location) @@ -129,3 +149,14 @@ class AuthoringMixinTestCase(ModuleStoreTestCase): 'Content group no longer exists.' ] ) + + def test_html_verification_checkpoints(self): + self.create_verification_user_partitions(["Midterm A", "Midterm B"]) + self.verify_visibility_view_contains( + self.video_location, + [ + "Verification Checkpoint", + "Midterm A", + "Midterm B", + ] + ) diff --git a/cms/static/js/models/xblock_info.js b/cms/static/js/models/xblock_info.js index 639ae1e2ed..7c7ef81fba 100644 --- a/cms/static/js/models/xblock_info.js +++ b/cms/static/js/models/xblock_info.js @@ -144,8 +144,18 @@ function(Backbone, _, str, ModuleUtils) { /** * Optional explanatory message about the xblock. */ - 'explanatory_message': null - + 'explanatory_message': null, + /** + * The XBlock's group access rules. This is a dictionary keyed to user partition IDs, + * where the values are lists of group IDs. + */ + 'group_access': null, + /** + * User partition dictionary. This is pre-processed by Studio, so it contains + * some additional fields that are not stored in the course descriptor + * (for example, which groups are selected for this particular XBlock). + */ + 'user_partitions': null, }, initialize: function () { @@ -238,7 +248,20 @@ function(Backbone, _, str, ModuleUtils) { */ isEditableOnCourseOutline: function() { return this.isSequential() || this.isChapter() || this.isVertical(); - } + }, + + /* + * Check whether any verification checkpoints are defined in the course. + * Verification checkpoints are defined if there exists a user partition + * that uses the verification partition scheme. + */ + hasVerifiedCheckpoints: function() { + var partitions = this.get("user_partitions") || []; + + return Boolean(_.find(partitions, function(p) { + return p.scheme === "verification"; + })); + } }); return XBlockInfo; }); diff --git a/cms/static/js/views/modals/course_outline_modals.js b/cms/static/js/views/modals/course_outline_modals.js index 2fa152dd7a..5b00400447 100644 --- a/cms/static/js/views/modals/course_outline_modals.js +++ b/cms/static/js/views/modals/course_outline_modals.js @@ -13,7 +13,8 @@ define(['jquery', 'backbone', 'underscore', 'gettext', 'js/views/baseview', ) { 'use strict'; var CourseOutlineXBlockModal, SettingsXBlockModal, PublishXBlockModal, AbstractEditor, BaseDateEditor, - ReleaseDateEditor, DueDateEditor, GradingEditor, PublishEditor, StaffLockEditor, TimedExaminationPreferenceEditor; + ReleaseDateEditor, DueDateEditor, GradingEditor, PublishEditor, StaffLockEditor, + VerificationAccessEditor, TimedExaminationPreferenceEditor; CourseOutlineXBlockModal = BaseModal.extend({ events : { @@ -427,7 +428,7 @@ define(['jquery', 'backbone', 'underscore', 'gettext', 'js/views/baseview', }, hasChanges: function() { - return this.isModelLocked() != this.isLocked(); + return this.isModelLocked() !== this.isLocked(); }, getRequestData: function() { @@ -443,7 +444,110 @@ define(['jquery', 'backbone', 'underscore', 'gettext', 'js/views/baseview', return { hasExplicitStaffLock: this.isModelLocked(), ancestorLocked: this.isAncestorLocked() + }; + } + }); + + VerificationAccessEditor = AbstractEditor.extend({ + templateName: 'verification-access-editor', + className: 'edit-verification-access', + + // This constant MUST match the group ID + // defined by VerificationPartitionScheme on the backend! + ALLOW_GROUP_ID: 1, + + getSelectedPartition: function() { + var hasRestrictions = $("#verification-access-checkbox").is(":checked"), + selectedPartitionID = null; + + if (hasRestrictions) { + selectedPartitionID = $("#verification-partition-select").val(); } + + return parseInt(selectedPartitionID, 10); + }, + + getGroupAccess: function() { + var groupAccess = _.clone(this.model.get('group_access')) || [], + userPartitions = this.model.get('user_partitions') || [], + selectedPartition = this.getSelectedPartition(), + that = this; + + // We display a simplified UI to course authors. + // On the backend, each verification checkpoint is associated + // with a user partition that has two groups. For example, + // if two checkpoints were defined, they might look like: + // + // Midterm A: |-- ALLOW --|-- DENY --| + // Midterm B: |-- ALLOW --|-- DENY --| + // + // To make life easier for course authors, we display + // *one* option for each checkpoint: + // + // [X] Must complete verification checkpoint + // Dropdown: + // * Midterm A + // * Midterm B + // + // This is where we map the simplified UI to + // the underlying user partition. If the user checked + // the box, that means there *is* a restriction, + // so only the "ALLOW" group for the selected partition has access. + // Otherwise, all groups in the partition have access. + // + _.each(userPartitions, function(partition) { + if (partition.scheme === "verification") { + if (selectedPartition === partition.id) { + groupAccess[partition.id] = [that.ALLOW_GROUP_ID]; + } else { + delete groupAccess[partition.id]; + } + } + }); + + return groupAccess; + }, + + getRequestData: function() { + var groupAccess = this.getGroupAccess(), + hasChanges = !_.isEqual(groupAccess, this.model.get('group_access')); + + return hasChanges ? { + publish: 'republish', + metadata: { + group_access: groupAccess, + } + } : {}; + }, + + getContext: function() { + var partitions = this.model.get("user_partitions"), + hasRestrictions = false, + verificationPartitions = [], + isSelected = false; + + // Display a simplified version of verified partition schemes. + // Although there are two groups defined (ALLOW and DENY), + // we show only the ALLOW group. + // To avoid searching all the groups, we're assuming that the editor + // either sets the ALLOW group or doesn't set any groups (implicitly allow all). + _.each(partitions, function(item) { + if (item.scheme === "verification") { + isSelected = _.any(_.pluck(item.groups, "selected")); + hasRestrictions = hasRestrictions || isSelected; + + verificationPartitions.push({ + "id": item.id, + "name": item.name, + "selected": isSelected, + }); + } + }); + + return { + "hasVerificationRestrictions": hasRestrictions, + "verificationPartitions": verificationPartitions, + }; } }); @@ -472,8 +576,13 @@ define(['jquery', 'backbone', 'underscore', 'gettext', 'js/views/baseview', } editors.push(StaffLockEditor); + } else if (xblockInfo.isVertical()) { editors = [StaffLockEditor]; + + if (xblockInfo.hasVerifiedCheckpoints()) { + editors.push(VerificationAccessEditor); + } } return new SettingsXBlockModal($.extend({ editors: editors, diff --git a/cms/static/js/xblock/authoring.js b/cms/static/js/xblock/authoring.js index dfc8e7cc0d..d9abe7d6a6 100644 --- a/cms/static/js/xblock/authoring.js +++ b/cms/static/js/xblock/authoring.js @@ -6,29 +6,56 @@ function VisibilityEditorView(runtime, element) { this.getGroupAccess = function() { - var groupAccess, userPartitionId, selectedGroupIds; + var groupAccess = {}, + checkboxValues, + partitionId, + groupId, + + // This constant MUST match the group ID + // defined by VerificationPartitionScheme on the backend! + ALLOW_GROUP_ID = 1; + if (element.find('.visibility-level-all').prop('checked')) { return {}; } - userPartitionId = element.find('.wrapper-visibility-specific').data('user-partition-id').toString(); - selectedGroupIds = []; + + // Cohort partitions (user is allowed to select more than one) element.find('.field-visibility-content-group input:checked').each(function(index, input) { - selectedGroupIds.push(parseInt($(input).val())); + checkboxValues = $(input).val().split("-"); + partitionId = parseInt(checkboxValues[0], 10); + groupId = parseInt(checkboxValues[1], 10); + + if (groupAccess.hasOwnProperty(partitionId)) { + groupAccess[partitionId].push(groupId); + } else { + groupAccess[partitionId] = [groupId]; + } }); - groupAccess = {}; - groupAccess[userPartitionId] = selectedGroupIds; + + // Verification partitions (user can select exactly one) + if (element.find('#verification-access-checkbox').prop('checked')) { + partitionId = parseInt($('#verification-access-dropdown').val(), 10); + groupAccess[partitionId] = [ALLOW_GROUP_ID]; + } + return groupAccess; }; + // When selecting "all students and staff", uncheck the specific groups element.find('.field-visibility-level input').change(function(event) { if ($(event.target).hasClass('visibility-level-all')) { - element.find('.field-visibility-content-group input').prop('checked', false); + element.find('.field-visibility-content-group input, .field-visibility-verification input') + .prop('checked', false); } }); - element.find('.field-visibility-content-group input').change(function(event) { - element.find('.visibility-level-all').prop('checked', false); - element.find('.visibility-level-specific').prop('checked', true); - }); + + // When selecting a specific group, deselect "all students and staff" and + // select "specific content groups" instead.` + element.find('.field-visibility-content-group input, .field-visibility-verification input') + .change(function() { + element.find('.visibility-level-all').prop('checked', false); + element.find('.visibility-level-specific').prop('checked', true); + }); } VisibilityEditorView.prototype.collectFieldData = function collectFieldData() { diff --git a/cms/static/sass/elements/_modal-window.scss b/cms/static/sass/elements/_modal-window.scss index e7d06cf9c0..53f6835a20 100644 --- a/cms/static/sass/elements/_modal-window.scss +++ b/cms/static/sass/elements/_modal-window.scss @@ -465,6 +465,15 @@ } } + .field-visibility-verification { + .note { + @extend %t-copy-sub2; + @extend %t-regular; + margin: 14px 0 0 24px; + display: block; + } + } + // CASE: content group has been removed .field-visibility-content-group.was-removed { @@ -629,8 +638,12 @@ } } + .edit-staff-lock { + margin-bottom: $baseline; + } + // UI: staff lock section - .edit-staff-lock, .edit-settings-timed-examination { + .edit-staff-lock, .edit-settings-timed-examination, .edit-verification-access { .checkbox-cosmetic .input-checkbox { @extend %cont-text-sr; @@ -653,6 +666,20 @@ .checkbox-cosmetic .label { margin-bottom: 0; } + + .note { + @extend %t-copy-sub2; + @extend %t-regular; + margin: 14px 0 0 21px; + display: block; + } + } + + .verification-access { + .checkbox-cosmetic .label { + @include float(left); + margin: 2px 6px 0 0; + } } // UI: timed and proctored exam section diff --git a/cms/templates/course_outline.html b/cms/templates/course_outline.html index b8896b0d5e..839689d6ef 100644 --- a/cms/templates/course_outline.html +++ b/cms/templates/course_outline.html @@ -21,7 +21,7 @@ from microsite_configuration import microsite <%block name="header_extras"> -% for template_name in ['course-outline', 'xblock-string-field-editor', 'basic-modal', 'modal-button', 'course-outline-modal', 'due-date-editor', 'release-date-editor', 'grading-editor', 'publish-editor', 'staff-lock-editor', 'timed-examination-preference-editor']: +% for template_name in ['course-outline', 'xblock-string-field-editor', 'basic-modal', 'modal-button', 'course-outline-modal', 'due-date-editor', 'release-date-editor', 'grading-editor', 'publish-editor', 'staff-lock-editor', 'verification-access-editor', 'timed-examination-preference-editor']: diff --git a/cms/templates/js/verification-access-editor.underscore b/cms/templates/js/verification-access-editor.underscore new file mode 100644 index 0000000000..d5dd25468f --- /dev/null +++ b/cms/templates/js/verification-access-editor.underscore @@ -0,0 +1,44 @@ +
+
+ +
+
+
+ checked="checked" <% } %> + aria-describedby="verification-help-text" + > + + + + + + +
+ <%= gettext("Learners who require verification must pass the selected checkpoint to see the content in this unit. Learners who do not require verification see this content by default.") %> +
+
+
+
+
+
diff --git a/cms/templates/visibility_editor.html b/cms/templates/visibility_editor.html index b5b9d9bd22..3dfaa54173 100644 --- a/cms/templates/visibility_editor.html +++ b/cms/templates/visibility_editor.html @@ -1,19 +1,20 @@ <% from django.utils.translation import ugettext as _ +from openedx.core.djangoapps.credit.partition_schemes import VerificationPartitionScheme +from contentstore.utils import ancestor_has_staff_lock, get_visibility_partition_info -from openedx.core.djangoapps.course_groups.partition_scheme import get_cohorted_user_partition -from contentstore.utils import ancestor_has_staff_lock +partition_info = get_visibility_partition_info(xblock) +user_partitions = partition_info["user_partitions"] +cohort_partitions = partition_info["cohort_partitions"] +verification_partitions = partition_info["verification_partitions"] +has_selected_groups = partition_info["has_selected_groups"] +selected_verified_partition_id = partition_info["selected_verified_partition_id"] -cohorted_user_partition = get_cohorted_user_partition(xblock.location.course_key) -unsorted_groups = cohorted_user_partition.groups if cohorted_user_partition else [] -groups = sorted(unsorted_groups, key=lambda group: group.name) -selected_group_ids = xblock.group_access.get(cohorted_user_partition.id, []) if cohorted_user_partition else [] -has_selected_groups = len(selected_group_ids) > 0 is_staff_locked = ancestor_has_staff_lock(xblock) %>
- % if len(groups) == 0: + % if len(user_partitions) == 0:

${_('No content groups exist')}

@@ -42,7 +43,7 @@ is_staff_locked = ancestor_has_staff_lock(xblock) % endif
-% if len(groups) > 0: +% if len(user_partitions) > 0:
@@ -51,49 +52,83 @@ is_staff_locked = ancestor_has_staff_lock(xblock)
-
    -
  • +
    +
    -
    • +
-
  • +
    -
    • - +
    +
    -
    +

    ${_('Content Groups')}

    -
      - <% - missing_group_ids = set(selected_group_ids) - %> - % for group in groups: - <% - is_group_selected = group.id in selected_group_ids - if is_group_selected: - missing_group_ids.remove(group.id) - %> -
    • - - -
      • +
      + % for partition in cohort_partitions: + % for group in partition["groups"]: +
      + + % if group["deleted"]: + + % else: + + % endif +
      % endfor + % endfor - % for group_id in missing_group_ids: -
    • - -
      • - % endfor -
    + + + + + +
    + ${_("Learners who require verification must pass the selected checkpoint to see the content in this component. Learners who do not require verification see this content by default.")} +
    +
    +
    + % endif +
    diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 3cb3f3ab9a..c65c7b3243 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -35,6 +35,7 @@ from django.db.models.signals import pre_save, post_save from django.dispatch import receiver, Signal from django.core.exceptions import ObjectDoesNotExist from django.utils.translation import ugettext_noop +from django.core.cache import cache from django_countries.fields import CountryField import dogstats_wrapper as dog_stats_api from eventtracking import tracker @@ -846,6 +847,9 @@ class CourseEnrollment(models.Model): # Maintain a history of requirement status updates for auditing purposes history = HistoricalRecords() + # cache key format e.g enrollment...mode = 'honor' + COURSE_ENROLLMENT_CACHE_KEY = u"enrollment.{}.{}.mode" + class Meta(object): # pylint: disable=missing-docstring unique_together = (('user', 'course_id'),) ordering = ('user', 'course_id') @@ -1338,6 +1342,49 @@ class CourseEnrollment(models.Model): """ return CourseMode.is_verified_slug(self.mode) + @classmethod + def is_enrolled_as_verified(cls, user, course_key): + """ + Check whether the course enrollment is for a verified mode. + + Arguments: + user (User): The user object. + course_key (CourseKey): The identifier for the course. + + Returns: bool + + """ + enrollment = cls.get_enrollment(user, course_key) + return ( + enrollment is not None and + enrollment.is_active and + enrollment.is_verified_enrollment() + ) + + @classmethod + def cache_key_name(cls, user_id, course_key): + """Return cache key name to be used to cache current configuration. + Args: + user_id(int): Id of user. + course_key(unicode): Unicode of course key + + Returns: + Unicode cache key + """ + return cls.COURSE_ENROLLMENT_CACHE_KEY.format(user_id, unicode(course_key)) + + +@receiver(models.signals.post_save, sender=CourseEnrollment) +@receiver(models.signals.post_delete, sender=CourseEnrollment) +def invalidate_enrollment_mode_cache(sender, instance, **kwargs): # pylint: disable=unused-argument, invalid-name + """Invalidate the cache of CourseEnrollment model. """ + + cache_key = CourseEnrollment.cache_key_name( + instance.user.id, + unicode(instance.course_id) + ) + cache.delete(cache_key) + class ManualEnrollmentAudit(models.Model): """ diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index 405000c06b..01cf4de5cd 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -1533,3 +1533,39 @@ class CourseDescriptor(CourseFields, SequenceDescriptor, LicenseMixin): Returns the topics that have been configured for teams for this course, else None. """ return self.teams_configuration.get('topics', None) + + def get_user_partitions_for_scheme(self, scheme): + """ + Retrieve all user partitions defined in the course for a particular + partition scheme. + + Arguments: + scheme (object): The user partition scheme. + + Returns: + list of `UserPartition` + + """ + return [ + p for p in self.user_partitions + if p.scheme == scheme + ] + + def set_user_partitions_for_scheme(self, partitions, scheme): + """ + Set the user partitions for a particular scheme. + + Preserves partitions associated with other schemes. + + Arguments: + scheme (object): The user partition scheme. + + Returns: + list of `UserPartition` + + """ + other_partitions = [ + p for p in self.user_partitions # pylint: disable=access-member-before-definition + if p.scheme != scheme + ] + self.user_partitions = other_partitions + partitions # pylint: disable=attribute-defined-outside-init diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index 42aec05184..cb64b0ba6f 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -101,6 +101,9 @@ class ModuleStoreEnum(object): # user ID to use for tests that do not have a django user available test = -3 + # user ID for automatic update by the system + system = -4 + class SortOrder(object): """ Values for sorting asset metadata. @@ -264,6 +267,12 @@ class BulkOperationsMixin(object): if not bulk_ops_record.active: return + # Send the pre-publish signal within the context of the bulk operation. + # Writes performed by signal handlers will be persisted when the bulk + # operation ends. + if emit_signals and bulk_ops_record.is_root: + self.send_pre_publish_signal(bulk_ops_record, structure_key) + bulk_ops_record.unnest() # If this wasn't the outermost context, then don't close out the @@ -293,6 +302,14 @@ class BulkOperationsMixin(object): """ return self._get_bulk_ops_record(course_key, ignore_case).active + def send_pre_publish_signal(self, bulk_ops_record, course_id): + """ + Send a signal just before items are published in the course. + """ + signal_handler = getattr(self, "signal_handler", None) + if signal_handler and bulk_ops_record.has_publish_item: + signal_handler.send("pre_publish", course_key=course_id) + def send_bulk_published_signal(self, bulk_ops_record, course_id): """ Sends out the signal that items have been published from within this course. diff --git a/common/lib/xmodule/xmodule/modulestore/django.py b/common/lib/xmodule/xmodule/modulestore/django.py index 8b91916b45..9f61a4e8d6 100644 --- a/common/lib/xmodule/xmodule/modulestore/django.py +++ b/common/lib/xmodule/xmodule/modulestore/django.py @@ -86,11 +86,13 @@ class SignalHandler(object): almost no work. Its main job is to kick off the celery task that will do the actual work. """ + pre_publish = django.dispatch.Signal(providing_args=["course_key"]) course_published = django.dispatch.Signal(providing_args=["course_key"]) course_deleted = django.dispatch.Signal(providing_args=["course_key"]) library_updated = django.dispatch.Signal(providing_args=["library_key"]) _mapping = { + "pre_publish": pre_publish, "course_published": course_published, "course_deleted": course_deleted, "library_updated": library_updated, diff --git a/common/lib/xmodule/xmodule/partitions/partitions.py b/common/lib/xmodule/xmodule/partitions/partitions.py index 31d8b79456..190e11062e 100644 --- a/common/lib/xmodule/xmodule/partitions/partitions.py +++ b/common/lib/xmodule/xmodule/partitions/partitions.py @@ -84,18 +84,23 @@ class Group(namedtuple("Group", "id name")): USER_PARTITION_SCHEME_NAMESPACE = 'openedx.user_partition_scheme' -class UserPartition(namedtuple("UserPartition", "id name description groups scheme")): - """ - A named way to partition users into groups, primarily intended for running - experiments. It is expected that each user will be in at most one group in a - partition. +class UserPartition(namedtuple("UserPartition", "id name description groups scheme parameters active")): + """A named way to partition users into groups, primarily intended for + running experiments. It is expected that each user will be in at most one + group in a partition. - A Partition has an id, name, scheme, description, and a list of groups. - The id is intended to be unique within the context where these are used. (e.g. for - partitions of users within a course, the ids should be unique per-course). - The scheme is used to assign users into groups. + A Partition has an id, name, scheme, description, parameters, and a list + of groups. The id is intended to be unique within the context where these + are used. (e.g., for partitions of users within a course, the ids should + be unique per-course). The scheme is used to assign users into groups. + The parameters field is used to save extra parameters e.g., location of + the block in case of VerificationPartitionScheme. + + Partitions can be marked as inactive by setting the "active" flag to False. + Any group access rule referencing inactive partitions will be ignored + when performing access checks. """ - VERSION = 2 + VERSION = 3 # The collection of user partition scheme extensions. scheme_extensions = None @@ -103,11 +108,14 @@ class UserPartition(namedtuple("UserPartition", "id name description groups sche # The default scheme to be used when upgrading version 1 partitions. VERSION_1_SCHEME = "random" - def __new__(cls, id, name, description, groups, scheme=None, scheme_id=VERSION_1_SCHEME): + def __new__(cls, id, name, description, groups, scheme=None, parameters=None, active=True, scheme_id=VERSION_1_SCHEME): # pylint: disable=line-too-long # pylint: disable=super-on-old-class if not scheme: scheme = UserPartition.get_scheme(scheme_id) - return super(UserPartition, cls).__new__(cls, int(id), name, description, groups, scheme) + if parameters is None: + parameters = {} + + return super(UserPartition, cls).__new__(cls, int(id), name, description, groups, scheme, parameters, active) @staticmethod def get_scheme(name): @@ -137,7 +145,9 @@ class UserPartition(namedtuple("UserPartition", "id name description groups sche "name": self.name, "scheme": self.scheme.name, "description": self.description, + "parameters": self.parameters, "groups": [g.to_json() for g in self.groups], + "active": bool(self.active), "version": UserPartition.VERSION } @@ -165,13 +175,16 @@ class UserPartition(namedtuple("UserPartition", "id name description groups sche # Version changes should be backwards compatible in case the code # gets rolled back. If we see a version number greater than the current # version, we should try to read it rather than raising an exception. - elif value["version"] >= UserPartition.VERSION: + elif value["version"] >= 2: if "scheme" not in value: raise TypeError("UserPartition dict {0} missing value key 'scheme'".format(value)) + scheme_id = value["scheme"] else: raise TypeError("UserPartition dict {0} has unexpected version".format(value)) + parameters = value.get("parameters", {}) + active = value.get("active", True) groups = [Group.from_json(g) for g in value["groups"]] scheme = UserPartition.get_scheme(scheme_id) if not scheme: @@ -183,12 +196,22 @@ class UserPartition(namedtuple("UserPartition", "id name description groups sche value["description"], groups, scheme, + parameters, + active, ) def get_group(self, group_id): """ - Returns the group with the specified id. Raises NoSuchUserPartitionGroupError if not found. + Returns the group with the specified id. + + Arguments: + group_id (int): ID of the partition group. + + Raises: + NoSuchUserPartitionGroupError: The specified group could not be found. + """ + # pylint: disable=no-member for group in self.groups: if group.id == group_id: return group diff --git a/common/lib/xmodule/xmodule/partitions/tests/test_partitions.py b/common/lib/xmodule/xmodule/partitions/tests/test_partitions.py index b5db22a238..af2bf96919 100644 --- a/common/lib/xmodule/xmodule/partitions/tests/test_partitions.py +++ b/common/lib/xmodule/xmodule/partitions/tests/test_partitions.py @@ -110,6 +110,7 @@ class PartitionTestCase(TestCase): TEST_ID = 0 TEST_NAME = "Mock Partition" TEST_DESCRIPTION = "for testing purposes" + TEST_PARAMETERS = {"location": "block-v1:edX+DemoX+Demo+type@block@uuid"} TEST_GROUPS = [Group(0, 'Group 1'), Group(1, 'Group 2')] TEST_SCHEME_NAME = "mock" @@ -136,7 +137,8 @@ class PartitionTestCase(TestCase): self.TEST_NAME, self.TEST_DESCRIPTION, self.TEST_GROUPS, - extensions[0].plugin + extensions[0].plugin, + self.TEST_PARAMETERS, ) # Make sure the names are set on the schemes (which happens normally in code, but may not happen in tests). @@ -149,17 +151,28 @@ class TestUserPartition(PartitionTestCase): def test_construct(self): user_partition = UserPartition( - self.TEST_ID, self.TEST_NAME, self.TEST_DESCRIPTION, self.TEST_GROUPS, MockUserPartitionScheme() + self.TEST_ID, + self.TEST_NAME, + self.TEST_DESCRIPTION, + self.TEST_GROUPS, + MockUserPartitionScheme(), + self.TEST_PARAMETERS, ) self.assertEqual(user_partition.id, self.TEST_ID) self.assertEqual(user_partition.name, self.TEST_NAME) - self.assertEqual(user_partition.description, self.TEST_DESCRIPTION) - self.assertEqual(user_partition.groups, self.TEST_GROUPS) - self.assertEquals(user_partition.scheme.name, self.TEST_SCHEME_NAME) + self.assertEqual(user_partition.description, self.TEST_DESCRIPTION) # pylint: disable=no-member + self.assertEqual(user_partition.groups, self.TEST_GROUPS) # pylint: disable=no-member + self.assertEquals(user_partition.scheme.name, self.TEST_SCHEME_NAME) # pylint: disable=no-member + self.assertEquals(user_partition.parameters, self.TEST_PARAMETERS) # pylint: disable=no-member def test_string_id(self): user_partition = UserPartition( - "70", self.TEST_NAME, self.TEST_DESCRIPTION, self.TEST_GROUPS + "70", + self.TEST_NAME, + self.TEST_DESCRIPTION, + self.TEST_GROUPS, + MockUserPartitionScheme(), + self.TEST_PARAMETERS, ) self.assertEqual(user_partition.id, 70) @@ -169,9 +182,11 @@ class TestUserPartition(PartitionTestCase): "id": self.TEST_ID, "name": self.TEST_NAME, "description": self.TEST_DESCRIPTION, + "parameters": self.TEST_PARAMETERS, "groups": [group.to_json() for group in self.TEST_GROUPS], "version": self.user_partition.VERSION, - "scheme": self.TEST_SCHEME_NAME + "scheme": self.TEST_SCHEME_NAME, + "active": True, } self.assertEqual(jsonified, act_jsonified) @@ -180,22 +195,26 @@ class TestUserPartition(PartitionTestCase): "id": self.TEST_ID, "name": self.TEST_NAME, "description": self.TEST_DESCRIPTION, + "parameters": self.TEST_PARAMETERS, "groups": [group.to_json() for group in self.TEST_GROUPS], "version": UserPartition.VERSION, "scheme": "mock", } user_partition = UserPartition.from_json(jsonified) - self.assertEqual(user_partition.id, self.TEST_ID) - self.assertEqual(user_partition.name, self.TEST_NAME) - self.assertEqual(user_partition.description, self.TEST_DESCRIPTION) - for act_group in user_partition.groups: + self.assertEqual(user_partition.id, self.TEST_ID) # pylint: disable=no-member + self.assertEqual(user_partition.name, self.TEST_NAME) # pylint: disable=no-member + self.assertEqual(user_partition.description, self.TEST_DESCRIPTION) # pylint: disable=no-member + self.assertEqual(user_partition.parameters, self.TEST_PARAMETERS) # pylint: disable=no-member + + for act_group in user_partition.groups: # pylint: disable=no-member self.assertIn(act_group.id, [0, 1]) exp_group = self.TEST_GROUPS[act_group.id] self.assertEqual(exp_group.id, act_group.id) self.assertEqual(exp_group.name, act_group.name) def test_version_upgrade(self): - # Version 1 partitions did not have a scheme specified + # Test that version 1 partitions did not have a scheme specified + # and have empty parameters jsonified = { "id": self.TEST_ID, "name": self.TEST_NAME, @@ -204,13 +223,61 @@ class TestUserPartition(PartitionTestCase): "version": 1, } user_partition = UserPartition.from_json(jsonified) - self.assertEqual(user_partition.scheme.name, "random") + self.assertEqual(user_partition.scheme.name, "random") # pylint: disable=no-member + self.assertEqual(user_partition.parameters, {}) + self.assertTrue(user_partition.active) + + def test_version_upgrade_2_to_3(self): + # Test that version 3 user partition raises error if 'scheme' field is + # not provided (same behavior as version 2) + jsonified = { + 'id': self.TEST_ID, + "name": self.TEST_NAME, + "description": self.TEST_DESCRIPTION, + "parameters": self.TEST_PARAMETERS, + "groups": [group.to_json() for group in self.TEST_GROUPS], + "version": 2, + } + with self.assertRaisesRegexp(TypeError, "missing value key 'scheme'"): + UserPartition.from_json(jsonified) + + # Test that version 3 partitions have a scheme specified + # and a field 'parameters' (optional while setting user partition but + # always present in response) + jsonified = { + "id": self.TEST_ID, + "name": self.TEST_NAME, + "description": self.TEST_DESCRIPTION, + "groups": [group.to_json() for group in self.TEST_GROUPS], + "version": 2, + "scheme": self.TEST_SCHEME_NAME, + } + user_partition = UserPartition.from_json(jsonified) + self.assertEqual(user_partition.scheme.name, self.TEST_SCHEME_NAME) + self.assertEqual(user_partition.parameters, {}) + self.assertTrue(user_partition.active) + + # now test that parameters dict is present in response with same value + # as provided + jsonified = { + "id": self.TEST_ID, + "name": self.TEST_NAME, + "description": self.TEST_DESCRIPTION, + "groups": [group.to_json() for group in self.TEST_GROUPS], + "parameters": self.TEST_PARAMETERS, + "version": 3, + "scheme": self.TEST_SCHEME_NAME, + } + user_partition = UserPartition.from_json(jsonified) + self.assertEqual(user_partition.parameters, self.TEST_PARAMETERS) + self.assertTrue(user_partition.active) def test_from_json_broken(self): # Missing field jsonified = { "name": self.TEST_NAME, "description": self.TEST_DESCRIPTION, + "parameters": self.TEST_PARAMETERS, "groups": [group.to_json() for group in self.TEST_GROUPS], "version": UserPartition.VERSION, "scheme": self.TEST_SCHEME_NAME, @@ -223,6 +290,7 @@ class TestUserPartition(PartitionTestCase): 'id': self.TEST_ID, "name": self.TEST_NAME, "description": self.TEST_DESCRIPTION, + "parameters": self.TEST_PARAMETERS, "groups": [group.to_json() for group in self.TEST_GROUPS], "version": UserPartition.VERSION, } @@ -234,6 +302,7 @@ class TestUserPartition(PartitionTestCase): 'id': self.TEST_ID, "name": self.TEST_NAME, "description": self.TEST_DESCRIPTION, + "parameters": self.TEST_PARAMETERS, "groups": [group.to_json() for group in self.TEST_GROUPS], "version": UserPartition.VERSION, "scheme": "no_such_scheme", @@ -246,6 +315,7 @@ class TestUserPartition(PartitionTestCase): 'id': self.TEST_ID, "name": self.TEST_NAME, "description": self.TEST_DESCRIPTION, + "parameters": self.TEST_PARAMETERS, "groups": [group.to_json() for group in self.TEST_GROUPS], "version": -1, "scheme": self.TEST_SCHEME_NAME, @@ -258,6 +328,7 @@ class TestUserPartition(PartitionTestCase): 'id': self.TEST_ID, "name": self.TEST_NAME, "description": self.TEST_DESCRIPTION, + "parameters": self.TEST_PARAMETERS, "groups": [group.to_json() for group in self.TEST_GROUPS], "version": UserPartition.VERSION, "scheme": "mock", @@ -266,6 +337,18 @@ class TestUserPartition(PartitionTestCase): user_partition = UserPartition.from_json(jsonified) self.assertNotIn("programmer", user_partition.to_json()) + # No error on missing parameters key (which is optional) + jsonified = { + 'id': self.TEST_ID, + "name": self.TEST_NAME, + "description": self.TEST_DESCRIPTION, + "groups": [group.to_json() for group in self.TEST_GROUPS], + "version": UserPartition.VERSION, + "scheme": "mock", + } + user_partition = UserPartition.from_json(jsonified) + self.assertEqual(user_partition.parameters, {}) + def test_get_group(self): """ UserPartition.get_group correctly returns the group referenced by the diff --git a/common/test/acceptance/pages/studio/component_editor.py b/common/test/acceptance/pages/studio/component_editor.py index 03ad24a994..2448e84eb8 100644 --- a/common/test/acceptance/pages/studio/component_editor.py +++ b/common/test/acceptance/pages/studio/component_editor.py @@ -108,19 +108,19 @@ class ComponentVisibilityEditorView(BaseComponentEditorView): """ A :class:`.PageObject` representing the rendered view of a component visibility editor. """ - OPTION_SELECTOR = '.modal-section-content li.field' + OPTION_SELECTOR = '.modal-section-content .field' @property def all_options(self): """ - Return all visibility 'li' options. + Return all visibility options. """ return self.q(css=self._bounded_selector(self.OPTION_SELECTOR)).results @property def selected_options(self): """ - Return all selected visibility 'li' options. + Return all selected visibility options. """ results = [] for option in self.all_options: @@ -131,7 +131,7 @@ class ComponentVisibilityEditorView(BaseComponentEditorView): def select_option(self, label_text, save=True): """ - Click the first li which has a label matching `label_text`. + Click the first option which has a label matching `label_text`. Arguments: label_text (str): Text of a label accompanying the input diff --git a/lms/djangoapps/ccx/tests/test_field_override_performance.py b/lms/djangoapps/ccx/tests/test_field_override_performance.py index 4b92a2b5f3..ed0f7a2782 100644 --- a/lms/djangoapps/ccx/tests/test_field_override_performance.py +++ b/lms/djangoapps/ccx/tests/test_field_override_performance.py @@ -24,7 +24,7 @@ from xblock.core import XBlock from xmodule.modulestore.django import modulestore from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, \ TEST_DATA_SPLIT_MODULESTORE, TEST_DATA_MONGO_MODULESTORE -from xmodule.modulestore.tests.factories import check_mongo_calls, CourseFactory, check_sum_of_calls +from xmodule.modulestore.tests.factories import check_mongo_calls_range, CourseFactory, check_sum_of_calls from xmodule.modulestore.tests.utils import ProceduralCourseTestMixin from ccx_keys.locator import CCXLocator from ccx.tests.factories import CcxFactory @@ -142,7 +142,7 @@ class FieldOverridePerformanceTestCase(ProceduralCourseTestMixin, Assert that mongodb is queried ``calls`` times in the surrounded context. """ - return check_mongo_calls(calls) + return check_mongo_calls_range(max_finds=calls) def assertXBlockInstantiations(self, instantiations): """ @@ -214,24 +214,24 @@ class TestFieldOverrideMongoPerformance(FieldOverridePerformanceTestCase): TEST_DATA = { # (providers, course_width, enable_ccx, view_as_ccx): # of sql queries, # of mongo queries, # of xblocks - ('no_overrides', 1, True, False): (24, 7, 14), - ('no_overrides', 2, True, False): (69, 7, 85), - ('no_overrides', 3, True, False): (264, 7, 336), - ('ccx', 1, True, False): (24, 7, 14), - ('ccx', 2, True, False): (69, 7, 85), - ('ccx', 3, True, False): (264, 7, 336), - ('ccx', 1, True, True): (24, 7, 14), - ('ccx', 2, True, True): (69, 7, 85), - ('ccx', 3, True, True): (264, 7, 336), - ('no_overrides', 1, False, False): (24, 7, 14), - ('no_overrides', 2, False, False): (69, 7, 85), - ('no_overrides', 3, False, False): (264, 7, 336), - ('ccx', 1, False, False): (24, 7, 14), - ('ccx', 2, False, False): (69, 7, 85), - ('ccx', 3, False, False): (264, 7, 336), - ('ccx', 1, False, True): (24, 7, 14), - ('ccx', 2, False, True): (69, 7, 85), - ('ccx', 3, False, True): (264, 7, 336), + ('no_overrides', 1, True, False): (24, 6, 13), + ('no_overrides', 2, True, False): (69, 6, 84), + ('no_overrides', 3, True, False): (264, 6, 335), + ('ccx', 1, True, False): (24, 6, 13), + ('ccx', 2, True, False): (69, 6, 84), + ('ccx', 3, True, False): (264, 6, 335), + ('ccx', 1, True, True): (24, 6, 13), + ('ccx', 2, True, True): (69, 6, 84), + ('ccx', 3, True, True): (264, 6, 335), + ('no_overrides', 1, False, False): (24, 6, 13), + ('no_overrides', 2, False, False): (69, 6, 84), + ('no_overrides', 3, False, False): (264, 6, 335), + ('ccx', 1, False, False): (24, 6, 13), + ('ccx', 2, False, False): (69, 6, 84), + ('ccx', 3, False, False): (264, 6, 335), + ('ccx', 1, False, True): (24, 6, 13), + ('ccx', 2, False, True): (69, 6, 84), + ('ccx', 3, False, True): (264, 6, 335), } diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py index 4f2378cc9e..a7e199e450 100644 --- a/lms/djangoapps/courseware/access.py +++ b/lms/djangoapps/courseware/access.py @@ -487,16 +487,24 @@ def _has_group_access(descriptor, user, course_key): # resolve the partition IDs in group_access to actual # partition objects, skipping those which contain empty group directives. - # if a referenced partition could not be found, access will be denied. - try: - partitions = [ - descriptor._get_user_partition(partition_id) # pylint: disable=protected-access - for partition_id, group_ids in merged_access.items() - if group_ids is not None - ] - except NoSuchUserPartitionError: - log.warning("Error looking up user partition, access will be denied.", exc_info=True) - return ACCESS_DENIED + # If a referenced partition could not be found, it will be denied + # If the partition is found but is no longer active (meaning it's been disabled) + # then skip the access check for that partition. + partitions = [] + for partition_id, group_ids in merged_access.items(): + try: + partition = descriptor._get_user_partition(partition_id) # pylint: disable=protected-access + if partition.active: + if group_ids is not None: + partitions.append(partition) + else: + log.debug( + "Skipping partition with ID %s in course %s because it is no longer active", + partition.id, course_key + ) + except NoSuchUserPartitionError: + log.warning("Error looking up user partition, access will be denied.", exc_info=True) + return ACCESS_DENIED # next resolve the group IDs specified within each partition partition_groups = [] diff --git a/lms/djangoapps/courseware/testutils.py b/lms/djangoapps/courseware/testutils.py index 40aae1c188..b0e4e8b31f 100644 --- a/lms/djangoapps/courseware/testutils.py +++ b/lms/djangoapps/courseware/testutils.py @@ -93,7 +93,7 @@ class RenderXBlockTestMixin(object): return response @ddt.data( - (ModuleStoreEnum.Type.mongo, 8), + (ModuleStoreEnum.Type.mongo, 7), (ModuleStoreEnum.Type.split, 5), ) @ddt.unpack diff --git a/lms/djangoapps/lms_xblock/mixin.py b/lms/djangoapps/lms_xblock/mixin.py index 9c7b2e3475..493dd9624e 100644 --- a/lms/djangoapps/lms_xblock/mixin.py +++ b/lms/djangoapps/lms_xblock/mixin.py @@ -151,11 +151,13 @@ class LmsBlockMixin(XBlockMixin): except NoSuchUserPartitionError: has_invalid_user_partitions = True else: - for group_id in group_ids: - try: - user_partition.get_group(group_id) - except NoSuchUserPartitionGroupError: - has_invalid_groups = True + # Skip the validation check if the partition has been disabled + if user_partition.active: + for group_id in group_ids: + try: + user_partition.get_group(group_id) + except NoSuchUserPartitionGroupError: + has_invalid_groups = True if has_invalid_user_partitions: validation.add( diff --git a/lms/djangoapps/verify_student/models.py b/lms/djangoapps/verify_student/models.py index 0b25c7e96a..4202538f33 100644 --- a/lms/djangoapps/verify_student/models.py +++ b/lms/djangoapps/verify_student/models.py @@ -1126,11 +1126,16 @@ class VerificationStatus(models.Model): A verification status represents a user’s progress through the verification process for a particular checkpoint. """ + SUBMITTED_STATUS = "submitted" + APPROVED_STATUS = "approved" + DENIED_STATUS = "denied" + ERROR_STATUS = "error" + VERIFICATION_STATUS_CHOICES = ( - ("submitted", "submitted"), - ("approved", "approved"), - ("denied", "denied"), - ("error", "error") + (SUBMITTED_STATUS, SUBMITTED_STATUS), + (APPROVED_STATUS, APPROVED_STATUS), + (DENIED_STATUS, DENIED_STATUS), + (ERROR_STATUS, ERROR_STATUS) ) checkpoint = models.ForeignKey(VerificationCheckpoint, related_name="checkpoint_status") @@ -1199,15 +1204,15 @@ class VerificationStatus(models.Model): return None @classmethod - def get_user_attempts(cls, user_id, course_key, related_assessment_location): + def get_user_attempts(cls, user_id, course_key, checkpoint_location): """ Get re-verification attempts against a user for a given 'checkpoint' and 'course_id'. Arguments: - user_id(str): User Id string - course_key(str): A CourseKey of a course - related_assessment_location(str): Verification checkpoint location + user_id (str): User Id string + course_key (str): A CourseKey of a course + checkpoint_location (str): Verification checkpoint location Returns: Count of re-verification attempts @@ -1216,8 +1221,8 @@ class VerificationStatus(models.Model): return cls.objects.filter( user_id=user_id, checkpoint__course_id=course_key, - checkpoint__checkpoint_location=related_assessment_location, - status="submitted" + checkpoint__checkpoint_location=checkpoint_location, + status=cls.SUBMITTED_STATUS ).count() @classmethod @@ -1236,6 +1241,49 @@ class VerificationStatus(models.Model): except cls.DoesNotExist: return "" + @classmethod + def get_all_checkpoints(cls, user_id, course_key): + """Return dict of all the checkpoints with their status. + Args: + user_id(int): Id of user. + course_key(unicode): Unicode of course key + + Returns: + dict: {checkpoint:status} + """ + all_checks_points = cls.objects.filter( + user_id=user_id, checkpoint__course_id=course_key + ) + check_points = {} + for check in all_checks_points: + check_points[check.checkpoint.checkpoint_location] = check.status + + return check_points + + @classmethod + def cache_key_name(cls, user_id, course_key): + """Return the name of the key to use to cache the current configuration + Args: + user_id(int): Id of user. + course_key(unicode): Unicode of course key + + Returns: + Unicode cache key + """ + return u"verification.{}.{}".format(user_id, unicode(course_key)) + + +@receiver(models.signals.post_save, sender=VerificationStatus) +@receiver(models.signals.post_delete, sender=VerificationStatus) +def invalidate_verification_status_cache(sender, instance, **kwargs): # pylint: disable=unused-argument, invalid-name + """Invalidate the cache of VerificationStatus model. """ + + cache_key = VerificationStatus.cache_key_name( + instance.user.id, + unicode(instance.checkpoint.course_id) + ) + cache.delete(cache_key) + # DEPRECATED: this feature has been permanently enabled. # Once the application code has been updated in production, @@ -1283,15 +1331,40 @@ class SkippedReverification(models.Model): cls.objects.create(checkpoint=checkpoint, user_id=user_id, course_id=course_id) @classmethod - def check_user_skipped_reverification_exists(cls, user, course_id): + def check_user_skipped_reverification_exists(cls, user_id, course_id): """Check existence of a user's skipped re-verification attempt for a specific course. Arguments: - user(User): user object + user_id(str): user id course_id(CourseKey): CourseKey Returns: Boolean """ - return cls.objects.filter(user=user, course_id=course_id).exists() + has_skipped = cls.objects.filter(user_id=user_id, course_id=course_id).exists() + return has_skipped + + @classmethod + def cache_key_name(cls, user_id, course_key): + """Return the name of the key to use to cache the current configuration + Arguments: + user(User): user object + course_key(CourseKey): CourseKey + + Returns: + string: cache key name + """ + return u"skipped_reverification.{}.{}".format(user_id, unicode(course_key)) + + +@receiver(models.signals.post_save, sender=SkippedReverification) +@receiver(models.signals.post_delete, sender=SkippedReverification) +def invalidate_skipped_verification_cache(sender, instance, **kwargs): # pylint: disable=unused-argument, invalid-name + """Invalidate the cache of skipped verification model. """ + + cache_key = SkippedReverification.cache_key_name( + instance.user.id, + unicode(instance.course_id) + ) + cache.delete(cache_key) diff --git a/lms/djangoapps/verify_student/services.py b/lms/djangoapps/verify_student/services.py index 64f615bc63..cdf0a38df4 100644 --- a/lms/djangoapps/verify_student/services.py +++ b/lms/djangoapps/verify_student/services.py @@ -10,6 +10,7 @@ from django.db import IntegrityError from opaque_keys.edx.keys import CourseKey +from student.models import User, CourseEnrollment from verify_student.models import VerificationCheckpoint, VerificationStatus, SkippedReverification @@ -21,24 +22,28 @@ class ReverificationService(object): Reverification XBlock service """ + SKIPPED_STATUS = "skipped" + NON_VERIFIED_TRACK = "not-verified" + def get_status(self, user_id, course_id, related_assessment_location): """Get verification attempt status against a user for a given 'checkpoint' and 'course_id'. Args: - user_id(str): User Id string - course_id(str): A string of course id - related_assessment_location(str): Location of Reverification XBlock + user_id (str): User Id string + course_id (str): A string of course id + related_assessment_location (str): Location of Reverification XBlock - Returns: - "skipped" if the user has skipped the re-verification or - Verification Status string if the user has submitted photo - verification attempt else None + Returns: str or None """ + user = User.objects.get(id=user_id) course_key = CourseKey.from_string(course_id) - has_skipped = SkippedReverification.check_user_skipped_reverification_exists(user_id, course_key) - if has_skipped: - return "skipped" + + if not CourseEnrollment.is_enrolled_as_verified(user, course_key): + return self.NON_VERIFIED_TRACK + elif SkippedReverification.check_user_skipped_reverification_exists(user_id, course_key): + return self.SKIPPED_STATUS + try: checkpoint_status = VerificationStatus.objects.filter( user_id=user_id, diff --git a/lms/djangoapps/verify_student/tests/test_models.py b/lms/djangoapps/verify_student/tests/test_models.py index 8d7b4cb914..86f78d19ab 100644 --- a/lms/djangoapps/verify_student/tests/test_models.py +++ b/lms/djangoapps/verify_student/tests/test_models.py @@ -688,14 +688,12 @@ class VerificationStatusTest(ModuleStoreTestCase): status='submitted' ) - self.assertEqual( - VerificationStatus.get_user_attempts( - user_id=self.user.id, - course_key=self.course.id, - related_assessment_location=self.first_checkpoint_location - ), - 1 + actual_attempts = VerificationStatus.get_user_attempts( + self.user.id, + self.course.id, + self.first_checkpoint_location ) + self.assertEqual(actual_attempts, 1) class SkippedReverificationTest(ModuleStoreTestCase): @@ -763,12 +761,18 @@ class SkippedReverificationTest(ModuleStoreTestCase): checkpoint=self.checkpoint, user_id=self.user.id, course_id=unicode(self.course.id) ) self.assertTrue( - SkippedReverification.check_user_skipped_reverification_exists(course_id=self.course.id, user=self.user) + SkippedReverification.check_user_skipped_reverification_exists( + user_id=self.user.id, + course_id=self.course.id + ) ) user2 = UserFactory.create() self.assertFalse( - SkippedReverification.check_user_skipped_reverification_exists(course_id=self.course.id, user=user2) + SkippedReverification.check_user_skipped_reverification_exists( + user_id=user2.id, + course_id=self.course.id + ) ) diff --git a/lms/djangoapps/verify_student/tests/test_services.py b/lms/djangoapps/verify_student/tests/test_services.py index 8ff66d4de9..85568c3d9f 100644 --- a/lms/djangoapps/verify_student/tests/test_services.py +++ b/lms/djangoapps/verify_student/tests/test_services.py @@ -4,7 +4,9 @@ Tests of re-verification service. import ddt +from course_modes.models import CourseMode from course_modes.tests.factories import CourseModeFactory +from student.models import CourseEnrollment from student.tests.factories import UserFactory from verify_student.models import VerificationCheckpoint, VerificationStatus, SkippedReverification from verify_student.services import ReverificationService @@ -35,6 +37,9 @@ class TestReverificationService(ModuleStoreTestCase): org=self.course_key.org, course=self.course_key.course ) + # Enroll in a verified mode + self.enrollment = CourseEnrollment.enroll(self.user, self.course_key, mode=CourseMode.VERIFIED) + @ddt.data('final', 'midterm') def test_start_verification(self, checkpoint_name): """Test the 'start_verification' service method. @@ -107,6 +112,12 @@ class TestReverificationService(ModuleStoreTestCase): 1 ) + # testing service for skipped attempt. + self.assertEqual( + reverification_service.get_status(self.user.id, unicode(self.course_key), self.final_checkpoint_location), + 'skipped' + ) + def test_get_attempts(self): """Check verification attempts count against a user for a given 'checkpoint' and 'course_id'. @@ -129,3 +140,12 @@ class TestReverificationService(ModuleStoreTestCase): reverification_service.get_attempts(self.user.id, course_id, self.final_checkpoint_location), 1 ) + + def test_not_in_verified_track(self): + # No longer enrolled in a verified track + self.enrollment.update_enrollment(mode=CourseMode.HONOR) + + # Should be marked as "skipped" (opted out) + service = ReverificationService() + status = service.get_status(self.user.id, unicode(self.course_key), self.final_checkpoint_location) + self.assertEqual(status, service.NON_VERIFIED_TRACK) diff --git a/lms/templates/courseware/course_navigation.html b/lms/templates/courseware/course_navigation.html index 691b94b57b..cc76636ea8 100644 --- a/lms/templates/courseware/course_navigation.html +++ b/lms/templates/courseware/course_navigation.html @@ -25,7 +25,7 @@ def selected(is_selected): return "selected" if is_selected else "" show_preview_menu = not disable_preview_menu and staff_access and active_page in ["courseware", "info"] -cohorted_user_partition = get_cohorted_user_partition(course.id) +cohorted_user_partition = get_cohorted_user_partition(course) masquerade_user_name = masquerade.user_name if masquerade else None masquerade_group_id = masquerade.group_id if masquerade else None staff_selected = selected(not masquerade or masquerade.role != "student") diff --git a/lms/templates/instructor/instructor_dashboard_2/cohort_management.html b/lms/templates/instructor/instructor_dashboard_2/cohort_management.html index db7d3e7dc5..7a46a00d93 100644 --- a/lms/templates/instructor/instructor_dashboard_2/cohort_management.html +++ b/lms/templates/instructor/instructor_dashboard_2/cohort_management.html @@ -20,7 +20,7 @@ from openedx.core.djangoapps.course_groups.partition_scheme import get_cohorted_ <%block name="js_extra"> <%static:require_module module_name="js/groups/views/cohorts_dashboard_factory" class_name="CohortsFactory"> <% - cohorted_user_partition = get_cohorted_user_partition(course.id) + cohorted_user_partition = get_cohorted_user_partition(course) content_groups = cohorted_user_partition.groups if cohorted_user_partition else [] %> var cohortUserPartitionId = ${cohorted_user_partition.id if cohorted_user_partition else 'null'}, diff --git a/openedx/core/djangoapps/course_groups/partition_scheme.py b/openedx/core/djangoapps/course_groups/partition_scheme.py index 346528e825..05f1e5015c 100644 --- a/openedx/core/djangoapps/course_groups/partition_scheme.py +++ b/openedx/core/djangoapps/course_groups/partition_scheme.py @@ -3,7 +3,6 @@ Provides a UserPartition driver for cohorts. """ import logging -from courseware import courses from courseware.masquerade import ( # pylint: disable=import-error get_course_masquerade, get_masquerading_group_info, @@ -100,13 +99,12 @@ class CohortPartitionScheme(object): return None -def get_cohorted_user_partition(course_key): +def get_cohorted_user_partition(course): """ Returns the first user partition from the specified course which uses the CohortPartitionScheme, or None if one is not found. Note that it is currently recommended that each course have only one cohorted user partition. """ - course = courses.get_course_by_id(course_key) for user_partition in course.user_partitions: if user_partition.scheme == CohortPartitionScheme: return user_partition diff --git a/openedx/core/djangoapps/course_groups/tests/test_partition_scheme.py b/openedx/core/djangoapps/course_groups/tests/test_partition_scheme.py index 55f558780d..093eca653f 100644 --- a/openedx/core/djangoapps/course_groups/tests/test_partition_scheme.py +++ b/openedx/core/djangoapps/course_groups/tests/test_partition_scheme.py @@ -317,14 +317,14 @@ class TestGetCohortedUserPartition(ModuleStoreTestCase): self.course.user_partitions.append(self.random_user_partition) self.course.user_partitions.append(self.cohort_user_partition) self.course.user_partitions.append(self.second_cohort_user_partition) - self.assertEqual(self.cohort_user_partition, get_cohorted_user_partition(self.course_key)) + self.assertEqual(self.cohort_user_partition, get_cohorted_user_partition(self.course)) def test_no_cohort_user_partitions(self): """ Test get_cohorted_user_partition returns None when there are no cohorted user partitions. """ self.course.user_partitions.append(self.random_user_partition) - self.assertIsNone(get_cohorted_user_partition(self.course_key)) + self.assertIsNone(get_cohorted_user_partition(self.course)) class TestMasqueradedGroup(StaffMasqueradeTestCase): diff --git a/openedx/core/djangoapps/credit/partition_schemes.py b/openedx/core/djangoapps/credit/partition_schemes.py new file mode 100644 index 0000000000..db364a1011 --- /dev/null +++ b/openedx/core/djangoapps/credit/partition_schemes.py @@ -0,0 +1,136 @@ +""" +Partition scheme for in-course reverification. + +This is responsible for placing users into one of two groups, +ALLOW or DENY, for a partition associated with a particular +in-course reverification checkpoint. + +NOTE: This really should be defined in the verify_student app, +which owns the verification and reverification process. +It isn't defined there now because (a) we need access to this in both Studio +and the LMS, but verify_student is specific to the LMS, and +(b) in-course reverification checkpoints currently have messaging that's +specific to credit requirements. + +""" +import logging + +from django.core.cache import cache + +from lms.djangoapps.verify_student.models import SkippedReverification, VerificationStatus +from student.models import CourseEnrollment +from xmodule.partitions.partitions import NoSuchUserPartitionGroupError + + +log = logging.getLogger(__name__) + + +class VerificationPartitionScheme(object): + """ + Assign users to groups for a particular verification checkpoint. + + Users in the ALLOW group can see gated content; + users in the DENY group cannot. + """ + + DENY = 0 + ALLOW = 1 + + @classmethod + def get_group_for_user(cls, course_key, user, user_partition, **kwargs): # pylint: disable=unused-argument + """ + Return the user's group depending their enrollment and verification + status. + + Args: + course_key (CourseKey): CourseKey + user (User): user object + user_partition (UserPartition): The user partition object. + + Returns: + string of allowed access group + """ + checkpoint = user_partition.parameters['location'] + + # Retrieve all information we need to determine the user's group + # as a multi-get from the cache. + is_verified, has_skipped, has_completed = _get_user_statuses(user, course_key, checkpoint) + + # Decide whether the user should have access to content gated by this checkpoint. + # Intuitively, we allow access if the user doesn't need to do anything at the checkpoint, + # either because the user is in a non-verified track or the user has already submitted. + # + # Note that we do NOT wait the user's reverification attempt to be approved, + # since this can take some time and the user might miss an assignment deadline. + partition_group = cls.DENY + if not is_verified or has_skipped or has_completed: + partition_group = cls.ALLOW + + # Return matching user partition group if it exists + try: + return user_partition.get_group(partition_group) + except NoSuchUserPartitionGroupError: + log.error( + ( + u"Could not find group with ID %s for verified partition " + "with ID %s in course %s. The user will not be assigned a group." + ), + partition_group, + user_partition.id, + course_key + ) + return None + + +def _get_user_statuses(user, course_key, checkpoint): + """ + Retrieve all the information we need to determine the user's group. + + This will retrieve the information as a multi-get from the cache. + + Args: + user (User): User object + course_key (CourseKey): Identifier for the course. + checkpoint (unicode): Location of the checkpoint in the course (serialized usage key) + + Returns: + tuple of booleans of the form (is_verified, has_skipped, has_completed) + + """ + enrollment_cache_key = CourseEnrollment.cache_key_name(user.id, unicode(course_key)) + has_skipped_cache_key = SkippedReverification.cache_key_name(user.id, unicode(course_key)) + verification_status_cache_key = VerificationStatus.cache_key_name(user.id, unicode(course_key)) + + # Try a multi-get from the cache + cache_values = cache.get_many([ + enrollment_cache_key, + has_skipped_cache_key, + verification_status_cache_key + ]) + + # Retrieve whether the user is enrolled in a verified mode. + is_verified = cache_values.get(enrollment_cache_key) + if is_verified is None: + is_verified = CourseEnrollment.is_enrolled_as_verified(user, course_key) + cache.set(enrollment_cache_key, is_verified) + + # Retrieve whether the user has skipped any checkpoints in this course + has_skipped = cache_values.get(has_skipped_cache_key) + if has_skipped is None: + has_skipped = SkippedReverification.check_user_skipped_reverification_exists(user, course_key) + cache.set(has_skipped_cache_key, has_skipped) + + # Retrieve the user's verification status for each checkpoint in the course. + verification_statuses = cache_values.get(verification_status_cache_key) + if verification_statuses is None: + verification_statuses = VerificationStatus.get_all_checkpoints(user.id, course_key) + cache.set(verification_status_cache_key, verification_statuses) + + # Check whether the user has completed this checkpoint + # "Completion" here means *any* submission, regardless of its status + # since we want to show the user the content if they've submitted + # photos. + checkpoint = verification_statuses.get(checkpoint) + has_completed_check = bool(checkpoint) + + return (is_verified, has_skipped, has_completed_check) diff --git a/openedx/core/djangoapps/credit/signals.py b/openedx/core/djangoapps/credit/signals.py index aed3ef8032..fbec5c9bd8 100644 --- a/openedx/core/djangoapps/credit/signals.py +++ b/openedx/core/djangoapps/credit/signals.py @@ -8,14 +8,15 @@ from django.dispatch import receiver from django.utils import timezone from opaque_keys.edx.keys import CourseKey -from xmodule.modulestore.django import SignalHandler from openedx.core.djangoapps.signals.signals import GRADES_UPDATED +from openedx.core.djangoapps.credit.verification_access import update_verification_partitions +from xmodule.modulestore.django import SignalHandler log = logging.getLogger(__name__) -def on_course_publish(course_key): # pylint: disable=unused-argument +def on_course_publish(course_key): """ Will receive a delegated 'course_published' signal from cms/djangoapps/contentstore/signals.py and kick off a celery task to update the credit course requirements. @@ -33,6 +34,25 @@ def on_course_publish(course_key): # pylint: disable=unused-argument log.info(u'Added task to update credit requirements for course "%s" to the task queue', course_key) +@receiver(SignalHandler.pre_publish) +def on_pre_publish(sender, course_key, **kwargs): # pylint: disable=unused-argument + """ + Create user partitions for verification checkpoints. + + This is a pre-publish step since we need to write to the course descriptor. + """ + from openedx.core.djangoapps.credit import api + if api.is_credit_course(course_key): + # For now, we are tagging content with in-course-reverification access groups + # only in credit courses on publish. In the long run, this is not where we want to put this. + # This really should be a transformation on the course structure performed as a pre-processing + # step by the LMS, and the transformation should be owned by the verify_student app. + # Since none of that infrastructure currently exists, we're doing it this way instead. + log.info(u"Starting to update in-course reverification access rules") + update_verification_partitions(course_key) + log.info(u"Finished updating in-course reverification access rules") + + @receiver(GRADES_UPDATED) def listen_for_grade_calculation(sender, username, grade_summary, course_key, deadline, **kwargs): # pylint: disable=unused-argument """Receive 'MIN_GRADE_REQUIREMENT_STATUS' signal and update minimum grade diff --git a/openedx/core/djangoapps/credit/tasks.py b/openedx/core/djangoapps/credit/tasks.py index 2e8908f91a..51c2af2874 100644 --- a/openedx/core/djangoapps/credit/tasks.py +++ b/openedx/core/djangoapps/credit/tasks.py @@ -15,8 +15,9 @@ from opaque_keys.edx.keys import CourseKey from .api import set_credit_requirements from openedx.core.djangoapps.credit.exceptions import InvalidCreditRequirements from openedx.core.djangoapps.credit.models import CreditCourse -from xmodule.modulestore.django import modulestore +from openedx.core.djangoapps.credit.utils import get_course_blocks from xmodule.modulestore import ModuleStoreEnum +from xmodule.modulestore.django import modulestore from xmodule.modulestore.exceptions import ItemNotFoundError LOGGER = get_task_logger(__name__) @@ -139,34 +140,13 @@ def _get_credit_course_requirement_xblocks(course_key): # pylint: disable=inval return requirements -def _is_in_course_tree(block): - """ - Check that the XBlock is in the course tree. - - It's possible that the XBlock is not in the course tree - if its parent has been deleted and is now an orphan. - """ - ancestor = block.get_parent() - while ancestor is not None and ancestor.location.category != "course": - ancestor = ancestor.get_parent() - - return ancestor is not None - - def _get_xblocks(course_key, category): """ Retrieve all XBlocks in the course for a particular category. Returns only XBlocks that are published and haven't been deleted. """ - xblocks = [ - block for block in modulestore().get_items( - course_key, - qualifiers={"category": category}, - revision=ModuleStoreEnum.RevisionOption.published_only, - ) - if _is_in_course_tree(block) - ] + xblocks = get_course_blocks(course_key, category) # Secondary sort on credit requirement name xblocks = sorted(xblocks, key=lambda block: block.get_credit_requirement_display_name()) diff --git a/openedx/core/djangoapps/credit/tests/test_partition.py b/openedx/core/djangoapps/credit/tests/test_partition.py new file mode 100644 index 0000000000..f6d74f51b0 --- /dev/null +++ b/openedx/core/djangoapps/credit/tests/test_partition.py @@ -0,0 +1,181 @@ +# -*- coding: utf-8 -*- +""" +Tests for In-Course Reverification Access Control Partition scheme +""" + +import ddt +import unittest + +from django.conf import settings + +from lms.djangoapps.verify_student.models import ( + VerificationCheckpoint, + VerificationStatus, + SkippedReverification, +) +from openedx.core.djangoapps.credit.partition_schemes import VerificationPartitionScheme +from student.models import CourseEnrollment +from student.tests.factories import UserFactory +from xmodule.partitions.partitions import UserPartition, Group +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory + + +@ddt.ddt +@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') +class ReverificationPartitionTest(ModuleStoreTestCase): + """Tests for the Reverification Partition Scheme. """ + + SUBMITTED = "submitted" + APPROVED = "approved" + DENIED = "denied" + + def setUp(self): + super(ReverificationPartitionTest, self).setUp() + + # creating course, checkpoint location and user partition mock object. + self.course = CourseFactory.create() + self.checkpoint_location = u'i4x://{org}/{course}/edx-reverification-block/first_uuid'.format( + org=self.course.id.org, course=self.course.id.course + ) + + scheme = UserPartition.get_scheme("verification") + self.user_partition = UserPartition( + id=0, + name=u"Verification Checkpoint", + description=u"Verification Checkpoint", + scheme=scheme, + parameters={"location": self.checkpoint_location}, + groups=[ + Group(scheme.ALLOW, "Allow access to content"), + Group(scheme.DENY, "Deny access to content"), + ] + ) + + self.first_checkpoint = VerificationCheckpoint.objects.create( + course_id=self.course.id, + checkpoint_location=self.checkpoint_location + ) + + def create_user_and_enroll(self, enrollment_type): + """Create and enroll users with provided enrollment type.""" + + user = UserFactory.create() + CourseEnrollment.objects.create( + user=user, + course_id=self.course.id, + mode=enrollment_type, + is_active=True + ) + return user + + def add_verification_status(self, user, status): + """Adding the verification status for a user.""" + + VerificationStatus.add_status_from_checkpoints( + checkpoints=[self.first_checkpoint], + user=user, + status=status + ) + + @ddt.data( + ("verified", SUBMITTED, VerificationPartitionScheme.ALLOW), + ("verified", APPROVED, VerificationPartitionScheme.ALLOW), + ("verified", DENIED, VerificationPartitionScheme.ALLOW), + ("verified", None, VerificationPartitionScheme.DENY), + ("honor", None, VerificationPartitionScheme.ALLOW), + ) + @ddt.unpack + def test_get_group_for_user(self, enrollment_type, verification_status, expected_group): + # creating user and enroll them. + user = self.create_user_and_enroll(enrollment_type) + if verification_status: + self.add_verification_status(user, verification_status) + + self._assert_group_assignment(user, expected_group) + + def test_get_group_for_user_with_skipped(self): + # Check that a user is in verified allow group if that user has skipped + # any ICRV block. + user = self.create_user_and_enroll('verified') + + SkippedReverification.add_skipped_reverification_attempt( + checkpoint=self.first_checkpoint, + user_id=user.id, + course_id=self.course.id + ) + + self._assert_group_assignment(user, VerificationPartitionScheme.ALLOW) + + def test_cache_with_skipped_icrv(self): + # Check that a user is in verified allow group if that user has skipped + # any ICRV block. + user = self.create_user_and_enroll('verified') + SkippedReverification.add_skipped_reverification_attempt( + checkpoint=self.first_checkpoint, + user_id=user.id, + course_id=self.course.id + ) + # this will warm the cache. + with self.assertNumQueries(3): + self._assert_group_assignment(user, VerificationPartitionScheme.ALLOW) + + # no db queries this time. + with self.assertNumQueries(0): + self._assert_group_assignment(user, VerificationPartitionScheme.ALLOW) + + def test_cache_with_submitted_status(self): + # Check that a user is in verified allow group if that user has approved status at + # any ICRV block. + user = self.create_user_and_enroll('verified') + self.add_verification_status(user, VerificationStatus.APPROVED_STATUS) + # this will warm the cache. + with self.assertNumQueries(4): + self._assert_group_assignment(user, VerificationPartitionScheme.ALLOW) + + # no db queries this time. + with self.assertNumQueries(0): + self._assert_group_assignment(user, VerificationPartitionScheme.ALLOW) + + def test_cache_with_denied_status(self): + # Check that a user is in verified allow group if that user has denied at + # any ICRV block. + user = self.create_user_and_enroll('verified') + self.add_verification_status(user, VerificationStatus.DENIED_STATUS) + + # this will warm the cache. + with self.assertNumQueries(4): + self._assert_group_assignment(user, VerificationPartitionScheme.ALLOW) + + # no db queries this time. + with self.assertNumQueries(0): + self._assert_group_assignment(user, VerificationPartitionScheme.ALLOW) + + def test_cache_with_honor(self): + # Check that a user is in honor mode. + # any ICRV block. + user = self.create_user_and_enroll('honor') + # this will warm the cache. + with self.assertNumQueries(3): + self._assert_group_assignment(user, VerificationPartitionScheme.ALLOW) + + # no db queries this time. + with self.assertNumQueries(0): + self._assert_group_assignment(user, VerificationPartitionScheme.ALLOW) + + def test_cache_with_verified_deny_group(self): + # Check that a user is in verified mode. But not perform any action + + user = self.create_user_and_enroll('verified') + # this will warm the cache. + with self.assertNumQueries(3): + self._assert_group_assignment(user, VerificationPartitionScheme.DENY) + + # no db queries this time. + with self.assertNumQueries(0): + self._assert_group_assignment(user, VerificationPartitionScheme.DENY) + + def _assert_group_assignment(self, user, expected_group_id): + """Check that the user was assigned to a group. """ + actual_group = VerificationPartitionScheme.get_group_for_user(self.course.id, user, self.user_partition) + self.assertEqual(actual_group.id, expected_group_id) diff --git a/openedx/core/djangoapps/credit/tests/test_tasks.py b/openedx/core/djangoapps/credit/tests/test_tasks.py index 32e5aa099c..70f838d7c7 100644 --- a/openedx/core/djangoapps/credit/tests/test_tasks.py +++ b/openedx/core/djangoapps/credit/tests/test_tasks.py @@ -208,7 +208,7 @@ class TestTaskExecution(ModuleStoreTestCase): self.add_credit_course(self.course.id) self.add_icrv_xblock() - with check_mongo_calls_range(max_finds=7): + with check_mongo_calls_range(max_finds=11): on_course_publish(self.course.id) def test_remove_icrv_requirement(self): diff --git a/openedx/core/djangoapps/credit/tests/test_verification_access.py b/openedx/core/djangoapps/credit/tests/test_verification_access.py new file mode 100644 index 0000000000..e7a0db47e6 --- /dev/null +++ b/openedx/core/djangoapps/credit/tests/test_verification_access.py @@ -0,0 +1,270 @@ +""" +Tests for in-course reverification user partition creation. + +This should really belong to the verify_student app, +but we can't move it there because it's in the LMS and we're +currently applying these rules on publish from Studio. + +In the future, this functionality should be a course transformation +defined in the verify_student app, and these tests should be moved +into verify_student. + +""" + +from mock import patch + +from django.conf import settings + +from openedx.core.djangoapps.credit.models import CreditCourse +from openedx.core.djangoapps.credit.partition_schemes import VerificationPartitionScheme +from openedx.core.djangoapps.credit.verification_access import update_verification_partitions +from openedx.core.djangoapps.credit.signals import on_pre_publish +from xmodule.modulestore import ModuleStoreEnum +from xmodule.modulestore.django import SignalHandler +from xmodule.modulestore.exceptions import ItemNotFoundError +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, TEST_DATA_SPLIT_MODULESTORE +from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, check_mongo_calls_range +from xmodule.partitions.partitions import Group, UserPartition + + +class CreateVerificationPartitionTest(ModuleStoreTestCase): + """ + Tests for applying verification access rules. + """ + + # Run the tests in split modulestore + # While verification access will work in old-Mongo, it's not something + # we're committed to supporting, since this feature is meant for use + # in new courses. + MODULESTORE = TEST_DATA_SPLIT_MODULESTORE + + @patch.dict(settings.FEATURES, {"ENABLE_COURSEWARE_INDEX": False}) + def setUp(self): + super(CreateVerificationPartitionTest, self).setUp() + + # Disconnect the signal receiver -- we'll invoke the update code ourselves + SignalHandler.pre_publish.disconnect(receiver=on_pre_publish) + self.addCleanup(SignalHandler.pre_publish.connect, receiver=on_pre_publish) + + # Create a dummy course with a single verification checkpoint + # Because we need to check "exam" content surrounding the ICRV checkpoint, + # we need to create a fairly large course structure, with multiple sections, + # subsections, verticals, units, and items. + self.course = CourseFactory() + self.sections = [ + ItemFactory.create(parent=self.course, category='chapter', display_name='Test Section A'), + ItemFactory.create(parent=self.course, category='chapter', display_name='Test Section B'), + ] + self.subsections = [ + ItemFactory.create(parent=self.sections[0], category='sequential', display_name='Test Subsection A 1'), + ItemFactory.create(parent=self.sections[0], category='sequential', display_name='Test Subsection A 2'), + ItemFactory.create(parent=self.sections[1], category='sequential', display_name='Test Subsection B 1'), + ItemFactory.create(parent=self.sections[1], category='sequential', display_name='Test Subsection B 2'), + ] + self.verticals = [ + ItemFactory.create(parent=self.subsections[0], category='vertical', display_name='Test Unit A 1 a'), + ItemFactory.create(parent=self.subsections[0], category='vertical', display_name='Test Unit A 1 b'), + ItemFactory.create(parent=self.subsections[1], category='vertical', display_name='Test Unit A 2 a'), + ItemFactory.create(parent=self.subsections[1], category='vertical', display_name='Test Unit A 2 b'), + ItemFactory.create(parent=self.subsections[2], category='vertical', display_name='Test Unit B 1 a'), + ItemFactory.create(parent=self.subsections[2], category='vertical', display_name='Test Unit B 1 b'), + ItemFactory.create(parent=self.subsections[3], category='vertical', display_name='Test Unit B 2 a'), + ItemFactory.create(parent=self.subsections[3], category='vertical', display_name='Test Unit B 2 b'), + ] + self.icrv = ItemFactory.create(parent=self.verticals[0], category='edx-reverification-block') + self.sibling_problem = ItemFactory.create(parent=self.verticals[0], category='problem') + + def test_creates_user_partitions(self): + self._update_partitions() + + # Check that a new user partition was created for the ICRV block + self.assertEqual(len(self.course.user_partitions), 1) + partition = self.course.user_partitions[0] + self.assertEqual(partition.scheme.name, "verification") + self.assertEqual(partition.parameters["location"], unicode(self.icrv.location)) + + # Check that the groups for the partition were created correctly + self.assertEqual(len(partition.groups), 2) + self.assertItemsEqual( + [g.id for g in partition.groups], + [ + VerificationPartitionScheme.ALLOW, + VerificationPartitionScheme.DENY, + ] + ) + + @patch.dict(settings.FEATURES, {"ENABLE_COURSEWARE_INDEX": False}) + def test_removes_deleted_user_partitions(self): + self._update_partitions() + + # Delete the reverification block, then update the partitions + self.store.delete_item( + self.icrv.location, + ModuleStoreEnum.UserID.test, + revision=ModuleStoreEnum.RevisionOption.published_only + ) + self._update_partitions() + + # Check that the user partition was marked as inactive + self.assertEqual(len(self.course.user_partitions), 1) + partition = self.course.user_partitions[0] + self.assertFalse(partition.active) + self.assertEqual(partition.scheme.name, "verification") + + @patch.dict(settings.FEATURES, {"ENABLE_COURSEWARE_INDEX": False}) + def test_preserves_partition_id_for_verified_partitions(self): + self._update_partitions() + partition_id = self.course.user_partitions[0].id + self._update_partitions() + new_partition_id = self.course.user_partitions[0].id + self.assertEqual(partition_id, new_partition_id) + + @patch.dict(settings.FEATURES, {"ENABLE_COURSEWARE_INDEX": False}) + def test_preserves_existing_user_partitions(self): + # Add other, non-verified partition to the course + self.course.user_partitions = [ + UserPartition( + id=0, + name='Cohort user partition', + scheme=UserPartition.get_scheme('cohort'), + description='Cohorted user partition', + groups=[ + Group(id=0, name="Group A"), + Group(id=1, name="Group B"), + ], + ), + UserPartition( + id=1, + name='Random user partition', + scheme=UserPartition.get_scheme('random'), + description='Random user partition', + groups=[ + Group(id=0, name="Group A"), + Group(id=1, name="Group B"), + ], + ), + ] + self.course = self.store.update_item(self.course, ModuleStoreEnum.UserID.test) + + # Update the verification partitions. + # The existing partitions should still be available + self._update_partitions() + partition_ids = [p.id for p in self.course.user_partitions] + self.assertEqual(len(partition_ids), 3) + self.assertIn(0, partition_ids) + self.assertIn(1, partition_ids) + + def test_multiple_reverification_blocks(self): + # Add an additional ICRV block in another section + other_icrv = ItemFactory.create(parent=self.verticals[3], category='edx-reverification-block') + self._update_partitions() + + # Expect that both ICRV blocks have corresponding partitions + self.assertEqual(len(self.course.user_partitions), 2) + partition_locations = [p.parameters.get("location") for p in self.course.user_partitions] + self.assertIn(unicode(self.icrv.location), partition_locations) + self.assertIn(unicode(other_icrv.location), partition_locations) + + # Delete the first ICRV block and update partitions + icrv_location = self.icrv.location + self.store.delete_item( + self.icrv.location, + ModuleStoreEnum.UserID.test, + revision=ModuleStoreEnum.RevisionOption.published_only + ) + self._update_partitions() + + # Expect that the correct partition is marked as inactive + self.assertEqual(len(self.course.user_partitions), 2) + partitions_by_loc = { + p.parameters["location"]: p + for p in self.course.user_partitions + } + self.assertFalse(partitions_by_loc[unicode(icrv_location)].active) + self.assertTrue(partitions_by_loc[unicode(other_icrv.location)].active) + + def test_query_counts_with_no_reverification_blocks(self): + # Delete the ICRV block, so the number of ICRV blocks is zero + self.store.delete_item( + self.icrv.location, + ModuleStoreEnum.UserID.test, + revision=ModuleStoreEnum.RevisionOption.published_only + ) + + # 2 calls: get the course (definitions + structures) + # 2 calls: look up ICRV blocks in the course (definitions + structures) + with check_mongo_calls_range(max_finds=4, max_sends=2): + self._update_partitions(reload_items=False) + + def test_query_counts_with_one_reverification_block(self): + # One ICRV block created in the setup method + # Additional call to load the ICRV block + with check_mongo_calls_range(max_finds=5, max_sends=3): + self._update_partitions(reload_items=False) + + def test_query_counts_with_multiple_reverification_blocks(self): + # Total of two ICRV blocks (one created in setup method) + # Additional call to load each ICRV block + ItemFactory.create(parent=self.verticals[3], category='edx-reverification-block') + with check_mongo_calls_range(max_finds=6, max_sends=3): + self._update_partitions(reload_items=False) + + def _update_partitions(self, reload_items=True): + """Update user partitions in the course descriptor, then reload the content. """ + update_verification_partitions(self.course.id) # pylint: disable=no-member + + # Reload each component so we can see the changes + if reload_items: + self.course = self.store.get_course(self.course.id) # pylint: disable=no-member + self.sections = [self._reload_item(section.location) for section in self.sections] + self.subsections = [self._reload_item(subsection.location) for subsection in self.subsections] + self.verticals = [self._reload_item(vertical.location) for vertical in self.verticals] + self.icrv = self._reload_item(self.icrv.location) + self.sibling_problem = self._reload_item(self.sibling_problem.location) + + def _reload_item(self, location): + """Safely reload an item from the moduelstore. """ + try: + return self.store.get_item(location) + except ItemNotFoundError: + return None + + +class WriteOnPublishTest(ModuleStoreTestCase): + """ + Verify that updates to the course descriptor's + user partitions are written automatically on publish. + """ + MODULESTORE = TEST_DATA_SPLIT_MODULESTORE + + @patch.dict(settings.FEATURES, {"ENABLE_COURSEWARE_INDEX": False}) + def setUp(self): + super(WriteOnPublishTest, self).setUp() + + # Create a dummy course with an ICRV block + self.course = CourseFactory() + self.section = ItemFactory.create(parent=self.course, category='chapter', display_name='Test Section') + self.subsection = ItemFactory.create(parent=self.section, category='sequential', display_name='Test Subsection') + self.vertical = ItemFactory.create(parent=self.subsection, category='vertical', display_name='Test Unit') + self.icrv = ItemFactory.create(parent=self.vertical, category='edx-reverification-block') + + # Mark the course as credit + CreditCourse.objects.create(course_key=self.course.id, enabled=True) # pylint: disable=no-member + + @patch.dict(settings.FEATURES, {"ENABLE_COURSEWARE_INDEX": False}) + def test_can_write_on_publish_signal(self): + # Sanity check -- initially user partitions should be empty + self.assertEqual(self.course.user_partitions, []) + + # Make and publish a change to a block, which should trigger the publish signal + with self.store.bulk_operations(self.course.id): # pylint: disable=no-member + self.icrv.display_name = "Updated display name" + self.store.update_item(self.icrv, ModuleStoreEnum.UserID.test) + self.store.publish(self.icrv.location, ModuleStoreEnum.UserID.test) + + # Within the test, the course pre-publish signal should have fired synchronously + # Since the course is marked as credit, the in-course verification partitions + # should have been created. + # We need to verify that these changes were actually persisted to the modulestore. + retrieved_course = self.store.get_course(self.course.id) # pylint: disable=no-member + self.assertEqual(len(retrieved_course.user_partitions), 1) diff --git a/openedx/core/djangoapps/credit/utils.py b/openedx/core/djangoapps/credit/utils.py new file mode 100644 index 0000000000..ad4c6cb486 --- /dev/null +++ b/openedx/core/djangoapps/credit/utils.py @@ -0,0 +1,38 @@ +""" +Utilities for the credit app. +""" +from xmodule.modulestore import ModuleStoreEnum +from xmodule.modulestore.django import modulestore + + +def get_course_blocks(course_key, category): + """ + Retrieve all XBlocks in the course for a particular category. + + Returns only XBlocks that are published and haven't been deleted. + """ + # Note: we need to check if found components have been orphaned + # due to a bug in split modulestore (PLAT-799). Once that bug + # is resolved, we can skip the `_is_in_course_tree()` check entirely. + return [ + block for block in modulestore().get_items( + course_key, + qualifiers={"category": category}, + revision=ModuleStoreEnum.RevisionOption.published_only, + ) + if _is_in_course_tree(block) + ] + + +def _is_in_course_tree(block): + """ + Check that the XBlock is in the course tree. + + It's possible that the XBlock is not in the course tree + if its parent has been deleted and is now an orphan. + """ + ancestor = block.get_parent() + while ancestor is not None and ancestor.location.category != "course": + ancestor = ancestor.get_parent() + + return ancestor is not None diff --git a/openedx/core/djangoapps/credit/verification_access.py b/openedx/core/djangoapps/credit/verification_access.py new file mode 100644 index 0000000000..5a5c625718 --- /dev/null +++ b/openedx/core/djangoapps/credit/verification_access.py @@ -0,0 +1,187 @@ +""" +Create in-course reverification access groups in a course. + +We model the rules as a set of user partitions, one for each +verification checkpoint in a course. + +For example, suppose that a course has two verification checkpoints, +one at midterm A and one at the midterm B. + +Then the user partitions would look like this: + +Midterm A: |-- ALLOW --|-- DENY --| +Midterm B: |-- ALLOW --|-- DENY --| + +where the groups are defined as: + +* ALLOW: The user has access to content gated by the checkpoint. +* DENY: The user does not have access to content gated by the checkpoint. + +""" +import logging + +from util.db import generate_int_id +from openedx.core.djangoapps.credit.utils import get_course_blocks +from xmodule.modulestore.django import modulestore +from xmodule.modulestore import ModuleStoreEnum +from xmodule.partitions.partitions import Group, UserPartition + + +log = logging.getLogger(__name__) + + +VERIFICATION_SCHEME_NAME = "verification" +VERIFICATION_BLOCK_CATEGORY = "edx-reverification-block" + + +def update_verification_partitions(course_key): + """ + Create a user partition for each verification checkpoint in the course. + + This will modify the published version of the course descriptor. + It ensures that any in-course reverification XBlocks in the course + have an associated user partition. Other user partitions (e.g. cohorts) + will be preserved. Partitions associated with deleted reverification checkpoints + will be marked as inactive and will not be used to restrict access. + + Arguments: + course_key (CourseKey): identifier for the course. + + Returns: + None + """ + # Batch all the queries we're about to do and suppress + # the "publish" signal to avoid an infinite call loop. + with modulestore().bulk_operations(course_key, emit_signals=False): + + # Retrieve all in-course reverification blocks in the course + icrv_blocks = get_course_blocks(course_key, VERIFICATION_BLOCK_CATEGORY) + + # Update the verification definitions in the course descriptor + # This will also clean out old verification partitions if checkpoints + # have been deleted. + _set_verification_partitions(course_key, icrv_blocks) + + +def _unique_partition_id(course): + """Return a unique user partition ID for the course. """ + # Exclude all previously used IDs, even for partitions that have been disabled + # (e.g. if the course author deleted an in-course reverifification block but + # there are courseware components that reference the disabled partition). + used_ids = set(p.id for p in course.user_partitions) + return generate_int_id(used_ids=used_ids) + + +def _other_partitions(verified_partitions, exclude_partitions, course_key): + """ + Retrieve all partitions NOT associated with the current set of ICRV blocks. + + Any partition associated with a deleted ICRV block will be marked as inactive + so its access rules will no longer be enforced. + + Arguments: + all_partitions (list of UserPartition): All verified partitions defined in the course. + exclude_partitions (list of UserPartition): Partitions to exclude (e.g. the ICRV partitions already added) + course_key (CourseKey): Identifier for the course (used for logging). + + Returns: list of `UserPartition`s + + """ + results = [] + partition_by_id = { + p.id: p for p in verified_partitions + } + other_partition_ids = set(p.id for p in verified_partitions) - set(p.id for p in exclude_partitions) + + for pid in other_partition_ids: + partition = partition_by_id[pid] + results.append( + UserPartition( + id=partition.id, + name=partition.name, + description=partition.description, + scheme=partition.scheme, + parameters=partition.parameters, + groups=partition.groups, + active=False, + ) + ) + log.info( + ( + "Disabled partition %s in course %s because the " + "associated in-course-reverification checkpoint does not exist." + ), + partition.id, course_key + ) + + return results + + +def _set_verification_partitions(course_key, icrv_blocks): + """ + Create or update user partitions in the course. + + Ensures that each ICRV block in the course has an associated user partition + with the groups ALLOW and DENY. + + Arguments: + course_key (CourseKey): Identifier for the course. + icrv_blocks (list of XBlock): In-course reverification blocks, e.g. reverification checkpoints. + + Returns: + list of UserPartition + """ + scheme = UserPartition.get_scheme(VERIFICATION_SCHEME_NAME) + if scheme is None: + log.error("Could not retrieve user partition scheme with ID %s", VERIFICATION_SCHEME_NAME) + return [] + + course = modulestore().get_course(course_key) + if course is None: + log.error("Could not find course %s", course_key) + return [] + + verified_partitions = course.get_user_partitions_for_scheme(scheme) + partition_id_for_location = { + p.parameters["location"]: p.id + for p in verified_partitions + if "location" in p.parameters + } + + partitions = [] + for block in icrv_blocks: + partition = UserPartition( + id=partition_id_for_location.get( + unicode(block.location), + _unique_partition_id(course) + ), + name=block.related_assessment, + description=u"Verification checkpoint at {}".format(block.related_assessment), + scheme=scheme, + parameters={"location": unicode(block.location)}, + groups=[ + Group(scheme.ALLOW, "Completed verification at {}".format(block.related_assessment)), + Group(scheme.DENY, "Did not complete verification at {}".format(block.related_assessment)), + ] + ) + partitions.append(partition) + + log.info( + ( + "Configured partition %s for course %s using a verified partition scheme " + "for the in-course-reverification checkpoint at location %s" + ), + partition.id, + course_key, + partition.parameters["location"] + ) + + # Preserve existing, non-verified partitions from the course + # Mark partitions for deleted in-course reverification as disabled. + partitions += _other_partitions(verified_partitions, partitions, course_key) + course.set_user_partitions_for_scheme(partitions, scheme) + modulestore().update_item(course, ModuleStoreEnum.UserID.system) + + log.info("Saved updated partitions for the course %s", course_key) + + return partitions diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index 1357eb107f..7adcab832f 100644 --- a/requirements/edx/github.txt +++ b/requirements/edx/github.txt @@ -53,7 +53,7 @@ git+https://github.com/edx/edx-oauth2-provider.git@0.5.6#egg=oauth2-provider==0. git+https://github.com/edx/edx-lint.git@ed8c8d2a0267d4d42f43642d193e25f8bd575d9b#egg=edx_lint==0.2.3 -e git+https://github.com/edx/xblock-utils.git@213a97a50276d6a2504d8133650b2930ead357a0#egg=xblock-utils -e git+https://github.com/edx-solutions/xblock-google-drive.git@138e6fa0bf3a2013e904a085b9fed77dab7f3f21#egg=xblock-google-drive --e git+https://github.com/edx/edx-reverification-block.git@1e8f5a7fd589951a90bd31a0824a2c01ac9598ce#egg=edx-reverification-block +-e git+https://github.com/edx/edx-reverification-block.git@30fcf2fea305ed6649adcee9c831afaefba635c5#egg=edx-reverification-block git+https://github.com/edx/ecommerce-api-client.git@1.1.0#egg=ecommerce-api-client==1.1.0 -e git+https://github.com/edx/edx-user-state-client.git@30c0ad4b9f57f8d48d6943eb585ec8a9205f4469#egg=edx-user-state-client -e git+https://github.com/edx/edx-organizations.git@release-2015-08-03#egg=edx-organizations diff --git a/setup.py b/setup.py index 1b2ef6ec30..6bae27c051 100644 --- a/setup.py +++ b/setup.py @@ -6,13 +6,14 @@ from setuptools import setup setup( name="Open edX", - version="0.4", + version="0.5", install_requires=["setuptools"], requires=[], # NOTE: These are not the names we should be installing. This tree should # be reorganized to be a more conventional Python tree. packages=[ "openedx.core.djangoapps.course_groups", + "openedx.core.djangoapps.credit", "openedx.core.djangoapps.user_api", "lms", "cms", @@ -45,6 +46,7 @@ setup( "openedx.user_partition_scheme": [ "random = openedx.core.djangoapps.user_api.partition_schemes:RandomUserPartitionScheme", "cohort = openedx.core.djangoapps.course_groups.partition_scheme:CohortPartitionScheme", + "verification = openedx.core.djangoapps.credit.partition_schemes:VerificationPartitionScheme", ], } )