diff --git a/cms/djangoapps/contentstore/course_group_config.py b/cms/djangoapps/contentstore/course_group_config.py index dcbf19b30e..e4d32e80cd 100644 --- a/cms/djangoapps/contentstore/course_group_config.py +++ b/cms/djangoapps/contentstore/course_group_config.py @@ -9,10 +9,11 @@ from util.db import generate_int_id, MYSQL_MAX_INT from django.utils.translation import ugettext as _ from contentstore.utils import reverse_usage_url from xmodule.partitions.partitions import UserPartition +from xmodule.partitions.partitions_service import get_all_partitions_for_course, MINIMUM_STATIC_PARTITION_ID from xmodule.split_test_module import get_split_user_partitions from openedx.core.djangoapps.course_groups.partition_scheme import get_cohorted_user_partition -MINIMUM_GROUP_ID = 100 +MINIMUM_GROUP_ID = MINIMUM_STATIC_PARTITION_ID RANDOM_SCHEME = "random" COHORT_SCHEME = "cohort" @@ -84,7 +85,7 @@ class GroupConfiguration(object): """ Assign ids for the group_configuration's groups. """ - used_ids = [g.id for p in self.course.user_partitions for g in p.groups] + used_ids = [g.id for p in get_all_partitions_for_course(self.course) for g in p.groups] # Assign ids to every group in configuration. for group in self.configuration.get('groups', []): if group.get('id') is None: @@ -96,7 +97,7 @@ class GroupConfiguration(object): """ Return a list of IDs that already in use. """ - return set([p.id for p in course.user_partitions]) + return set([p.id for p in get_all_partitions_for_course(course)]) def get_user_partition(self): """ diff --git a/cms/djangoapps/contentstore/tests/test_utils.py b/cms/djangoapps/contentstore/tests/test_utils.py index c5e679ccb3..a70193474f 100644 --- a/cms/djangoapps/contentstore/tests/test_utils.py +++ b/cms/djangoapps/contentstore/tests/test_utils.py @@ -493,12 +493,12 @@ class GetUserPartitionInfoTest(ModuleStoreTestCase): ] } ] - self.assertEqual(self._get_partition_info(), expected) + self.assertEqual(self._get_partition_info(schemes=["cohort", "random"]), 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) + self.assertEqual(self._get_partition_info(schemes=["cohort", "random"]), expected) def test_deleted_groups(self): # Select a group that is not defined in the partition @@ -546,7 +546,7 @@ class GetUserPartitionInfoTest(ModuleStoreTestCase): ]) # Expect that the inactive scheme is excluded from the results - partitions = self._get_partition_info() + partitions = self._get_partition_info(schemes=["cohort", "verification"]) self.assertEqual(len(partitions), 1) self.assertEqual(partitions[0]["scheme"], "cohort") @@ -572,7 +572,7 @@ class GetUserPartitionInfoTest(ModuleStoreTestCase): ]) # Expect that the partition with no groups is excluded from the results - partitions = self._get_partition_info() + partitions = self._get_partition_info(schemes=["cohort", "verification"]) self.assertEqual(len(partitions), 1) self.assertEqual(partitions[0]["scheme"], "verification") diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 70a1d43740..1154e18174 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -14,6 +14,7 @@ from django_comment_common.utils import seed_permissions_roles from openedx.core.djangoapps.self_paced.models import SelfPacedConfiguration from openedx.core.djangoapps.site_configuration.models import SiteConfiguration +from xmodule.partitions.partitions_service import get_all_partitions_for_course from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore @@ -373,11 +374,11 @@ def get_user_partition_info(xblock, schemes=None, course=None): schemes = set(schemes) partitions = [] - for p in sorted(course.user_partitions, key=lambda p: p.name): + for p in sorted(get_all_partitions_for_course(course, active_only=True), 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): + if p.groups and (schemes is None or p.scheme.name in schemes): # First, add groups defined by the partition groups = [] @@ -408,7 +409,7 @@ def get_user_partition_info(xblock, schemes=None, course=None): # Put together the entire partition dictionary partitions.append({ "id": p.id, - "name": p.name, + "name": unicode(p.name), # Convert into a string in case ugettext_lazy was used "scheme": p.scheme.name, "groups": groups, }) diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index 2050dafaa1..361b6817a5 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -16,6 +16,7 @@ from xmodule.x_module import PREVIEW_VIEWS, STUDENT_VIEW, AUTHOR_VIEW from xmodule.contentstore.django import contentstore from xmodule.error_module import ErrorDescriptor from xmodule.exceptions import NotFoundError, ProcessingError +from xmodule.partitions.partitions_service import PartitionService from xmodule.studio_editable import has_author_view from xmodule.services import SettingsService from xmodule.modulestore.django import modulestore, ModuleI18nService @@ -213,10 +214,24 @@ def _preview_module_system(request, descriptor, field_data): "i18n": ModuleI18nService, "settings": SettingsService(), "user": DjangoXBlockUserService(request.user), + "partitions": StudioPartitionService(course_id=course_id) }, ) +class StudioPartitionService(PartitionService): + """ + A runtime mixin to allow the display and editing of component visibility based on user partitions. + """ + def get_user_group_id_for_partition(self, user, user_partition_id): + """ + Override this method to return None, as the split_test_module calls this + to determine which group a user should see, but is robust to getting a return + value of None meaning that all groups should be shown. + """ + return None + + def _load_preview_module(request, descriptor): """ Return a preview XModule instantiated from the supplied descriptor. Will use mutable fields diff --git a/cms/djangoapps/contentstore/views/tests/test_item.py b/cms/djangoapps/contentstore/views/tests/test_item.py index 3e28f0e205..1bf35d1d73 100644 --- a/cms/djangoapps/contentstore/views/tests/test_item.py +++ b/cms/djangoapps/contentstore/views/tests/test_item.py @@ -8,6 +8,7 @@ from pytz import UTC from pyquery import PyQuery from webob import Response +from django.conf import settings from django.http import Http404 from django.test import TestCase from django.test.client import RequestFactory @@ -44,6 +45,7 @@ from xblock_django.user_service import DjangoXBlockUserService from opaque_keys.edx.keys import UsageKey, CourseKey from opaque_keys.edx.locations import Location from xmodule.partitions.partitions import Group, UserPartition +from xmodule.partitions.partitions_service import ENROLLMENT_TRACK_PARTITION_ID, MINIMUM_STATIC_PARTITION_ID class AsideTest(XBlockAside): @@ -341,15 +343,17 @@ class GetItemTest(ItemTest): ) def test_get_user_partitions_and_groups(self): + # Note about UserPartition and UserPartition Group IDs: these must not conflict with IDs used + # by dynamic user partitions. self.course.user_partitions = [ UserPartition( - id=0, + id=MINIMUM_STATIC_PARTITION_ID, 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"), + Group(id=MINIMUM_STATIC_PARTITION_ID + 1, name="Group A"), # See note above. + Group(id=MINIMUM_STATIC_PARTITION_ID + 2, name="Group B"), # See note above. ], ), ] @@ -365,18 +369,31 @@ class GetItemTest(ItemTest): result = json.loads(resp.content) self.assertEqual(result["user_partitions"], [ { - "id": 0, + "id": ENROLLMENT_TRACK_PARTITION_ID, + "name": "Enrollment Track Partition", + "scheme": "enrollment_track", + "groups": [ + { + "id": settings.COURSE_ENROLLMENT_MODES["audit"], + "name": "Audit", + "selected": False, + "deleted": False, + } + ] + }, + { + "id": MINIMUM_STATIC_PARTITION_ID, "name": "Verification user partition", "scheme": "verification", "groups": [ { - "id": 0, + "id": MINIMUM_STATIC_PARTITION_ID + 1, "name": "Group A", "selected": False, "deleted": False, }, { - "id": 1, + "id": MINIMUM_STATIC_PARTITION_ID + 2, "name": "Group B", "selected": False, "deleted": False, @@ -1704,15 +1721,30 @@ class TestEditSplitModule(ItemTest): def setUp(self): super(TestEditSplitModule, self).setUp() self.user = UserFactory() + + self.first_user_partition_group_1 = Group(unicode(MINIMUM_STATIC_PARTITION_ID + 1), 'alpha') + self.first_user_partition_group_2 = Group(unicode(MINIMUM_STATIC_PARTITION_ID + 2), 'beta') + self.first_user_partition = UserPartition( + MINIMUM_STATIC_PARTITION_ID, 'first_partition', 'First Partition', + [self.first_user_partition_group_1, self.first_user_partition_group_2] + ) + + # There is a test point below (test_create_groups) that purposefully wants the group IDs + # of the 2 partitions to overlap (which is not something that normally happens). + self.second_user_partition_group_1 = Group(unicode(MINIMUM_STATIC_PARTITION_ID + 1), 'Group 1') + self.second_user_partition_group_2 = Group(unicode(MINIMUM_STATIC_PARTITION_ID + 2), 'Group 2') + self.second_user_partition_group_3 = Group(unicode(MINIMUM_STATIC_PARTITION_ID + 3), 'Group 3') + self.second_user_partition = UserPartition( + MINIMUM_STATIC_PARTITION_ID + 10, 'second_partition', 'Second Partition', + [ + self.second_user_partition_group_1, + self.second_user_partition_group_2, + self.second_user_partition_group_3 + ] + ) self.course.user_partitions = [ - UserPartition( - 0, 'first_partition', 'First Partition', - [Group("0", 'alpha'), Group("1", 'beta')] - ), - UserPartition( - 1, 'second_partition', 'Second Partition', - [Group("0", 'Group 0'), Group("1", 'Group 1'), Group("2", 'Group 2')] - ) + self.first_user_partition, + self.second_user_partition ] self.store.update_item(self.course, self.user.id) root_usage_key = self._create_vertical() @@ -1760,8 +1792,8 @@ class TestEditSplitModule(ItemTest): self.assertEqual(-1, split_test.user_partition_id) self.assertEqual(0, len(split_test.children)) - # Set the user_partition_id to 0. - split_test = self._update_partition_id(0) + # Set the user_partition_id to match the first user_partition. + split_test = self._update_partition_id(self.first_user_partition.id) # Verify that child verticals have been set to match the groups self.assertEqual(2, len(split_test.children)) @@ -1769,13 +1801,13 @@ class TestEditSplitModule(ItemTest): vertical_1 = self.get_item_from_modulestore(split_test.children[1], verify_is_draft=True) self.assertEqual("vertical", vertical_0.category) self.assertEqual("vertical", vertical_1.category) - self.assertEqual("Group ID 0", vertical_0.display_name) - self.assertEqual("Group ID 1", vertical_1.display_name) + self.assertEqual("Group ID " + unicode(MINIMUM_STATIC_PARTITION_ID + 1), vertical_0.display_name) + self.assertEqual("Group ID " + unicode(MINIMUM_STATIC_PARTITION_ID + 2), vertical_1.display_name) # Verify that the group_id_to_child mapping is correct. self.assertEqual(2, len(split_test.group_id_to_child)) - self.assertEqual(vertical_0.location, split_test.group_id_to_child['0']) - self.assertEqual(vertical_1.location, split_test.group_id_to_child['1']) + self.assertEqual(vertical_0.location, split_test.group_id_to_child[str(self.first_user_partition_group_1.id)]) + self.assertEqual(vertical_1.location, split_test.group_id_to_child[str(self.first_user_partition_group_2.id)]) def test_split_xblock_info_group_name(self): """ @@ -1785,8 +1817,8 @@ class TestEditSplitModule(ItemTest): # Initially, no user_partition_id is set, and the split_test has no children. self.assertEqual(split_test.user_partition_id, -1) self.assertEqual(len(split_test.children), 0) - # Set the user_partition_id to 0. - split_test = self._update_partition_id(0) + # Set the user_partition_id to match the first user_partition. + split_test = self._update_partition_id(self.first_user_partition.id) # Verify that child verticals have been set to match the groups self.assertEqual(len(split_test.children), 2) @@ -1808,13 +1840,13 @@ class TestEditSplitModule(ItemTest): group configuration. """ # Set to first group configuration. - split_test = self._update_partition_id(0) + split_test = self._update_partition_id(self.first_user_partition.id) self.assertEqual(2, len(split_test.children)) initial_vertical_0_location = split_test.children[0] initial_vertical_1_location = split_test.children[1] # Set to second group configuration - split_test = self._update_partition_id(1) + split_test = self._update_partition_id(self.second_user_partition.id) # We don't remove existing children. self.assertEqual(5, len(split_test.children)) self.assertEqual(initial_vertical_0_location, split_test.children[0]) @@ -1825,9 +1857,9 @@ class TestEditSplitModule(ItemTest): # Verify that the group_id_to child mapping is correct. self.assertEqual(3, len(split_test.group_id_to_child)) - self.assertEqual(vertical_0.location, split_test.group_id_to_child['0']) - self.assertEqual(vertical_1.location, split_test.group_id_to_child['1']) - self.assertEqual(vertical_2.location, split_test.group_id_to_child['2']) + self.assertEqual(vertical_0.location, split_test.group_id_to_child[str(self.second_user_partition_group_1.id)]) + self.assertEqual(vertical_1.location, split_test.group_id_to_child[str(self.second_user_partition_group_2.id)]) + self.assertEqual(vertical_2.location, split_test.group_id_to_child[str(self.second_user_partition_group_3.id)]) self.assertNotEqual(initial_vertical_0_location, vertical_0.location) self.assertNotEqual(initial_vertical_1_location, vertical_1.location) @@ -1836,12 +1868,12 @@ class TestEditSplitModule(ItemTest): Test that nothing happens when the user_partition_id is set to the same value twice. """ # Set to first group configuration. - split_test = self._update_partition_id(0) + split_test = self._update_partition_id(self.first_user_partition.id) self.assertEqual(2, len(split_test.children)) initial_group_id_to_child = split_test.group_id_to_child # Set again to first group configuration. - split_test = self._update_partition_id(0) + split_test = self._update_partition_id(self.first_user_partition.id) self.assertEqual(2, len(split_test.children)) self.assertEqual(initial_group_id_to_child, split_test.group_id_to_child) @@ -1852,7 +1884,7 @@ class TestEditSplitModule(ItemTest): The user_partition_id will be updated, but children and group_id_to_child map will not change. """ # Set to first group configuration. - split_test = self._update_partition_id(0) + split_test = self._update_partition_id(self.first_user_partition.id) self.assertEqual(2, len(split_test.children)) initial_group_id_to_child = split_test.group_id_to_child @@ -1869,13 +1901,14 @@ class TestEditSplitModule(ItemTest): TODO: move tests that can go over to common after the mixed modulestore work is done. # pylint: disable=fixme """ # Set to first group configuration. - split_test = self._update_partition_id(0) + split_test = self._update_partition_id(self.first_user_partition.id) # Add a group to the first group configuration. + new_group_id = "1002" split_test.user_partitions = [ UserPartition( - 0, 'first_partition', 'First Partition', - [Group("0", 'alpha'), Group("1", 'beta'), Group("2", 'pie')] + self.first_user_partition.id, 'first_partition', 'First Partition', + [self.first_user_partition_group_1, self.first_user_partition_group_2, Group(new_group_id, 'pie')] ) ] self.store.update_item(split_test, self.user.id) @@ -1896,7 +1929,7 @@ class TestEditSplitModule(ItemTest): split_test = self._assert_children(3) self.assertNotEqual(group_id_to_child, split_test.group_id_to_child) group_id_to_child = split_test.group_id_to_child - self.assertEqual(split_test.children[2], group_id_to_child["2"]) + self.assertEqual(split_test.children[2], group_id_to_child[new_group_id]) # Call add_missing_groups again -- it should be a no-op. split_test.add_missing_groups(self.request) diff --git a/cms/envs/bok_choy.py b/cms/envs/bok_choy.py index 29a545e562..8dfdf7082a 100644 --- a/cms/envs/bok_choy.py +++ b/cms/envs/bok_choy.py @@ -93,6 +93,8 @@ FEATURES['LICENSING'] = True FEATURES['ENABLE_MOBILE_REST_API'] = True # Enable video bumper in Studio FEATURES['ENABLE_VIDEO_BUMPER'] = True # Enable video bumper in Studio settings +FEATURES['ENABLE_ENROLLMENT_TRACK_USER_PARTITION'] = True + # Enable partner support link in Studio footer PARTNER_SUPPORT_EMAIL = 'partner-support@example.com' diff --git a/cms/envs/common.py b/cms/envs/common.py index 57d7810ebb..31e45bde85 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -88,6 +88,8 @@ from lms.envs.common import ( # File upload defaults FILE_UPLOAD_STORAGE_BUCKET_NAME, FILE_UPLOAD_STORAGE_PREFIX, + + COURSE_ENROLLMENT_MODES ) from path import Path as path from warnings import simplefilter @@ -225,6 +227,9 @@ FEATURES = { # Allow public account creation 'ALLOW_PUBLIC_ACCOUNT_CREATION': True, + + # Whether or not the dynamic EnrollmentTrackUserPartition should be registered. + 'ENABLE_ENROLLMENT_TRACK_USER_PARTITION': False, } ENABLE_JASMINE = False @@ -883,6 +888,9 @@ INSTALLED_APPS = ( # for managing course modes 'course_modes', + # Verified Track Content Cohorting (Beta feature that will hopefully be removed) + 'openedx.core.djangoapps.verified_track_content', + # Dark-launching languages 'openedx.core.djangoapps.dark_lang', diff --git a/cms/envs/test.py b/cms/envs/test.py index 8120678277..097a7bd785 100644 --- a/cms/envs/test.py +++ b/cms/envs/test.py @@ -320,6 +320,7 @@ FEATURES['ENABLE_COURSEWARE_INDEX'] = True FEATURES['ENABLE_LIBRARY_INDEX'] = True SEARCH_ENGINE = "search.tests.mock_search_engine.MockSearchEngine" +FEATURES['ENABLE_ENROLLMENT_TRACK_USER_PARTITION'] = True # teams feature FEATURES['ENABLE_TEAMS'] = True diff --git a/common/djangoapps/course_modes/admin.py b/common/djangoapps/course_modes/admin.py index a500d5cdea..f7898418a4 100644 --- a/common/djangoapps/course_modes/admin.py +++ b/common/djangoapps/course_modes/admin.py @@ -28,22 +28,18 @@ from course_modes.models import CourseMode, CourseModeExpirationConfig # the verification deadline table won't exist. from lms.djangoapps.verify_student import models as verification_models +COURSE_MODE_SLUG_CHOICES = [(mode_slug, mode_slug) for mode_slug in settings.COURSE_ENROLLMENT_MODES] + class CourseModeForm(forms.ModelForm): + """ + Admin form for adding a course mode. + """ class Meta(object): model = CourseMode fields = '__all__' - COURSE_MODE_SLUG_CHOICES = ( - [(CourseMode.DEFAULT_MODE_SLUG, CourseMode.DEFAULT_MODE_SLUG)] + - [(mode_slug, mode_slug) for mode_slug in CourseMode.VERIFIED_MODES] + - [(CourseMode.NO_ID_PROFESSIONAL_MODE, CourseMode.NO_ID_PROFESSIONAL_MODE)] + - [(mode_slug, mode_slug) for mode_slug in CourseMode.CREDIT_MODES] + - # need to keep legacy modes around for awhile - [(CourseMode.DEFAULT_SHOPPINGCART_MODE_SLUG, CourseMode.DEFAULT_SHOPPINGCART_MODE_SLUG)] - ) - mode_slug = forms.ChoiceField(choices=COURSE_MODE_SLUG_CHOICES, label=_("Mode")) # The verification deadline is stored outside the course mode in the verify_student app. diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index 9c39c7b549..85d6778cfe 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -1341,23 +1341,6 @@ class CourseDescriptor(CourseFields, SequenceDescriptor, LicenseMixin): """ 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. diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py index bd631c4724..501b4608c6 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py @@ -47,6 +47,7 @@ from xmodule.modulestore.draft_and_published import ModuleStoreDraftAndPublished from xmodule.modulestore.edit_info import EditInfoRuntimeMixin from xmodule.modulestore.exceptions import ItemNotFoundError, DuplicateCourseError, ReferentialIntegrityError from xmodule.modulestore.inheritance import InheritanceMixin, inherit_metadata, InheritanceKeyValueStore +from xmodule.partitions.partitions_service import PartitionService from xmodule.modulestore.xml import CourseLocationManager from xmodule.modulestore.store_utilities import DETACHED_XBLOCK_TYPES from xmodule.services import SettingsService @@ -934,6 +935,8 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo if self.request_cache: services["request_cache"] = self.request_cache + services["partitions"] = PartitionService(course_key) + system = CachingDescriptorSystem( modulestore=self, course_key=course_key, @@ -1346,6 +1349,8 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo if self.user_service: services["user"] = self.user_service + services["partitions"] = PartitionService(course_key) + runtime = CachingDescriptorSystem( modulestore=self, module_data={}, diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py index 0a0e689d81..3e1a393f52 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py @@ -83,6 +83,7 @@ from xmodule.modulestore import ( from ..exceptions import ItemNotFoundError from .caching_descriptor_system import CachingDescriptorSystem +from xmodule.partitions.partitions_service import PartitionService from xmodule.modulestore.split_mongo.mongo_connection import MongoConnection, DuplicateKeyError from xmodule.modulestore.split_mongo import BlockKey, CourseEnvelope from xmodule.modulestore.store_utilities import DETACHED_XBLOCK_TYPES @@ -3359,6 +3360,9 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): """ Create the proper runtime for this course """ + services = self.services + services["partitions"] = PartitionService(course_entry.course_key) + return CachingDescriptorSystem( modulestore=self, course_entry=course_entry, @@ -3370,7 +3374,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): mixins=self.xblock_mixins, select=self.xblock_select, disabled_xblock_types=self.disabled_xblock_types, - services=self.services, + services=services, ) def ensure_indexes(self): diff --git a/common/lib/xmodule/xmodule/partitions/partitions.py b/common/lib/xmodule/xmodule/partitions/partitions.py index 559dcf81de..ebded62a7c 100644 --- a/common/lib/xmodule/xmodule/partitions/partitions.py +++ b/common/lib/xmodule/xmodule/partitions/partitions.py @@ -127,7 +127,7 @@ class UserPartition(namedtuple("UserPartition", "id name description groups sche try: scheme = UserPartition.scheme_extensions[name].plugin except KeyError: - raise UserPartitionError("Unrecognized scheme {0}".format(name)) + raise UserPartitionError("Unrecognized scheme '{0}'".format(name)) scheme.name = name return scheme @@ -188,15 +188,25 @@ class UserPartition(namedtuple("UserPartition", "id name description groups sche if not scheme: raise TypeError("UserPartition dict {0} has unrecognized scheme {1}".format(value, scheme_id)) - return UserPartition( - value["id"], - value["name"], - value["description"], - groups, - scheme, - parameters, - active, - ) + if hasattr(scheme, "create_user_partition"): + return scheme.create_user_partition( + value["id"], + value["name"], + value["description"], + groups, + parameters, + active, + ) + else: + return UserPartition( + value["id"], + value["name"], + value["description"], + groups, + scheme, + parameters, + active, + ) def get_group(self, group_id): """ @@ -214,5 +224,7 @@ class UserPartition(namedtuple("UserPartition", "id name description groups sche return group raise NoSuchUserPartitionGroupError( - "could not find a Group with ID [{}] in UserPartition [{}]".format(group_id, self.id) + "Could not find a Group with ID [{group_id}] in UserPartition [{partition_id}].".format( + group_id=group_id, partition_id=self.id + ) ) diff --git a/common/lib/xmodule/xmodule/partitions/partitions_service.py b/common/lib/xmodule/xmodule/partitions/partitions_service.py index e5e5c1a0aa..1299a48b0f 100644 --- a/common/lib/xmodule/xmodule/partitions/partitions_service.py +++ b/common/lib/xmodule/xmodule/partitions/partitions_service.py @@ -3,31 +3,122 @@ This is a service-like API that assigns tracks which groups users are in for var user partitions. It uses the user_service key/value store provided by the LMS runtime to persist the assignments. """ -from abc import ABCMeta, abstractproperty +from django.conf import settings +from django.utils.translation import ugettext_lazy as _ +import logging + +from xmodule.partitions.partitions import UserPartition, UserPartitionError +from xmodule.modulestore.django import modulestore + + +log = logging.getLogger(__name__) + + +# UserPartition IDs must be unique. The Cohort and Random UserPartitions (when they are +# created via Studio) choose an unused ID in the range of 100 (historical) to MAX_INT. Therefore the +# dynamic UserPartitionIDs must be under 100, and they have to be hard-coded to ensure +# they are always the same whenever the dynamic partition is added (since the UserPartition +# ID is stored in the xblock group_access dict). +ENROLLMENT_TRACK_PARTITION_ID = 50 + +MINIMUM_STATIC_PARTITION_ID = 100 + + +# settings will not be available when running nosetests. +FEATURES = getattr(settings, 'FEATURES', {}) + + +def get_all_partitions_for_course(course, active_only=False): + """ + A method that returns all `UserPartitions` associated with a course, as a List. + This will include the ones defined in course.user_partitions, but it may also + include dynamically included partitions (such as the `EnrollmentTrackUserPartition`). + + Args: + course: the course for which user partitions should be returned. + active_only: if `True`, only partitions with `active` set to True will be returned. + + Returns: + A List of UserPartitions associated with the course. + """ + all_partitions = course.user_partitions + _get_dynamic_partitions(course) + if active_only: + all_partitions = [partition for partition in all_partitions if partition.active] + return all_partitions + + +def _get_dynamic_partitions(course): + """ + Return the dynamic user partitions for this course. + If none exists, returns an empty array. + """ + enrollment_partition = _create_enrollment_track_partition(course) + return [enrollment_partition] if enrollment_partition else [] + + +def _create_enrollment_track_partition(course): + """ + Create and return the dynamic enrollment track user partition. + If it cannot be created, None is returned. + """ + if not FEATURES.get('ENABLE_ENROLLMENT_TRACK_USER_PARTITION'): + return None + + try: + enrollment_track_scheme = UserPartition.get_scheme("enrollment_track") + except UserPartitionError: + log.warning("No 'enrollment_track' scheme registered, EnrollmentTrackUserPartition will not be created.") + return None + + used_ids = set(p.id for p in course.user_partitions) + if ENROLLMENT_TRACK_PARTITION_ID in used_ids: + # TODO: change to Exception after this has been in production for awhile, see TNL-6796. + log.warning( + "Can't add 'enrollment_track' partition, as ID {id} is assigned to {partition} in course {course}.".format( + id=ENROLLMENT_TRACK_PARTITION_ID, + partition=_get_partition_from_id(course.user_partitions, ENROLLMENT_TRACK_PARTITION_ID).name, + course=unicode(course.id) + ) + ) + return None + + partition = enrollment_track_scheme.create_user_partition( + id=ENROLLMENT_TRACK_PARTITION_ID, + name=_(u"Enrollment Track Partition"), + description=_(u"Partition for segmenting users by enrollment track"), + parameters={"course_id": unicode(course.id)} + ) + return partition class PartitionService(object): """ - This is an XBlock service that assigns tracks which groups users are in for various - user partitions. It uses the provided user_tags service object to - persist the assignments. + This is an XBlock service that returns information about the user partitions associated + with a given course. """ - __metaclass__ = ABCMeta - @abstractproperty - def course_partitions(self): - """ - Return the set of partitions assigned to self._course_id - """ - raise NotImplementedError('Subclasses must implement course_partition') - - def __init__(self, user, course_id, track_function=None, cache=None): - self._user = user + def __init__(self, course_id, track_function=None, cache=None): self._course_id = course_id self._track_function = track_function self._cache = cache - def get_user_group_id_for_partition(self, user_partition_id): + def get_course(self): + """ + Return the course instance associated with this PartitionService. + This default implementation looks up the course from the modulestore. + """ + return modulestore().get_course(self._course_id) + + @property + def course_partitions(self): + """ + Return the set of partitions assigned to self._course_id (both those set directly on the course + through course.user_partitions, and any dynamic partitions that exist). Note: this returns + both active and inactive partitions. + """ + return get_all_partitions_for_course(self.get_course()) + + def get_user_group_id_for_partition(self, user, user_partition_id): """ If the user is already assigned to a group in user_partition_id, return the group_id. @@ -35,9 +126,6 @@ class PartitionService(object): If not, assign them to one of the groups, persist that decision, and return the group_id. - If the group they are assigned to doesn't exist anymore, re-assign to one of - the existing groups and return its id. - Args: user_partition_id -- an id of a partition that's hopefully in the runtime.user_partitions list. @@ -49,7 +137,7 @@ class PartitionService(object): ValueError if the user_partition_id isn't found. """ cache_key = "PartitionService.ugidfp.{}.{}.{}".format( - self._user.id, self._course_id, user_partition_id + user.id, self._course_id, user_partition_id ) if self._cache and (cache_key in self._cache): @@ -62,7 +150,7 @@ class PartitionService(object): "in course {1}".format(user_partition_id, self._course_id) ) - group = self.get_group(user_partition) + group = self.get_group(user, user_partition) group_id = group.id if group else None if self._cache is not None: @@ -73,22 +161,33 @@ class PartitionService(object): def _get_user_partition(self, user_partition_id): """ Look for a user partition with a matching id in the course's partitions. + Note that this method can return an inactive user partition. Returns: A UserPartition, or None if not found. """ - for partition in self.course_partitions: - if partition.id == user_partition_id: - return partition + return _get_partition_from_id(self.course_partitions, user_partition_id) - return None - - def get_group(self, user_partition, assign=True): + def get_group(self, user, user_partition, assign=True): """ Returns the group from the specified user partition to which the user is assigned. If the user has not yet been assigned, a group will be chosen for them based upon the partition's scheme. """ return user_partition.scheme.get_group_for_user( - self._course_id, self._user, user_partition, assign=assign, track_function=self._track_function + self._course_id, user, user_partition, assign=assign, track_function=self._track_function ) + + +def _get_partition_from_id(partitions, user_partition_id): + """ + Look for a user partition with a matching id in the provided list of partitions. + + Returns: + A UserPartition, or None if not found. + """ + for partition in partitions: + if partition.id == user_partition_id: + return partition + + return None diff --git a/common/lib/xmodule/xmodule/partitions/tests/test_partitions.py b/common/lib/xmodule/xmodule/partitions/tests/test_partitions.py index cb8e0fae40..8db3452445 100644 --- a/common/lib/xmodule/xmodule/partitions/tests/test_partitions.py +++ b/common/lib/xmodule/xmodule/partitions/tests/test_partitions.py @@ -6,12 +6,14 @@ Test the partitions and partitions service from unittest import TestCase from mock import Mock -from opaque_keys.edx.locations import SlashSeparatedCourseKey +from opaque_keys.edx.locator import CourseLocator from stevedore.extension import Extension, ExtensionManager from xmodule.partitions.partitions import ( Group, UserPartition, UserPartitionError, NoSuchUserPartitionGroupError, USER_PARTITION_SCHEME_NAMESPACE ) -from xmodule.partitions.partitions_service import PartitionService +from xmodule.partitions.partitions_service import ( + PartitionService, get_all_partitions_for_course, ENROLLMENT_TRACK_PARTITION_ID, FEATURES +) class TestGroup(TestCase): @@ -105,6 +107,15 @@ class MockUserPartitionScheme(object): return groups[0] +class MockEnrollmentTrackUserPartitionScheme(MockUserPartitionScheme): + + def create_user_partition(self, id, name, description, groups=None, parameters=None, active=True): # pylint: disable=redefined-builtin, invalid-name, unused-argument + """ + The EnrollmentTrackPartitionScheme provides this method to return a subclass of UserPartition. + """ + return UserPartition(id, name, description, groups, self, parameters, active) + + class PartitionTestCase(TestCase): """Base class for test cases that require partitions""" TEST_ID = 0 @@ -113,12 +124,14 @@ class PartitionTestCase(TestCase): 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" + ENROLLMENT_TRACK_SCHEME_NAME = "enrollment_track" def setUp(self): super(PartitionTestCase, self).setUp() # Set up two user partition schemes: mock and random self.non_random_scheme = MockUserPartitionScheme(self.TEST_SCHEME_NAME) self.random_scheme = MockUserPartitionScheme("random") + self.enrollment_track_scheme = MockEnrollmentTrackUserPartitionScheme(self.ENROLLMENT_TRACK_SCHEME_NAME) extensions = [ Extension( self.non_random_scheme.name, USER_PARTITION_SCHEME_NAMESPACE, self.non_random_scheme, None @@ -126,6 +139,9 @@ class PartitionTestCase(TestCase): Extension( self.random_scheme.name, USER_PARTITION_SCHEME_NAMESPACE, self.random_scheme, None ), + Extension( + self.enrollment_track_scheme.name, USER_PARTITION_SCHEME_NAMESPACE, self.enrollment_track_scheme, None + ), ] UserPartition.scheme_extensions = ExtensionManager.make_test_instance( extensions, namespace=USER_PARTITION_SCHEME_NAMESPACE @@ -394,45 +410,50 @@ class TestUserPartition(PartitionTestCase): self.assertEqual(partition.name, self.TEST_NAME) -class StaticPartitionService(PartitionService): +class MockPartitionService(PartitionService): """ Mock PartitionService for testing. """ - def __init__(self, partitions, **kwargs): - super(StaticPartitionService, self).__init__(**kwargs) - self._partitions = partitions + def __init__(self, course, **kwargs): + super(MockPartitionService, self).__init__(**kwargs) + self._course = course - @property - def course_partitions(self): - return self._partitions + def get_course(self): + return self._course -class TestPartitionService(PartitionTestCase): +class PartitionServiceBaseClass(PartitionTestCase): """ - Test getting a user's group out of a partition + Base test class for testing the PartitionService. """ def setUp(self): - super(TestPartitionService, self).setUp() - self.course = Mock(id=SlashSeparatedCourseKey('org_0', 'course_0', 'run_0')) + super(PartitionServiceBaseClass, self).setUp() + self.course = Mock(id=CourseLocator('org_0', 'course_0', 'run_0')) self.partition_service = self._create_service("ma") def _create_service(self, username, cache=None): - """Convenience method to generate a StaticPartitionService for a user.""" + """Convenience method to generate a MockPartitionService for a user.""" # Derive a "user_id" from the username, just so we don't have to add an # extra param to this method. Just has to be unique per user. user_id = abs(hash(username)) + self.user = Mock( + username=username, email='{}@edx.org'.format(username), is_staff=False, is_active=True, id=user_id + ) + self.course.user_partitions = [self.user_partition] - return StaticPartitionService( - [self.user_partition], - user=Mock( - username=username, email='{}@edx.org'.format(username), is_staff=False, is_active=True, id=user_id - ), + return MockPartitionService( + self.course, course_id=self.course.id, track_function=Mock(), cache=cache ) + +class TestPartitionService(PartitionServiceBaseClass): + """ + Test getting a user's group out of a partition + """ def test_get_user_group_id_for_partition(self): # assign the first group to be returned user_partition_id = self.user_partition.id @@ -440,12 +461,12 @@ class TestPartitionService(PartitionTestCase): self.user_partition.scheme.current_group = groups[0] # get a group assigned to the user - group1_id = self.partition_service.get_user_group_id_for_partition(user_partition_id) + group1_id = self.partition_service.get_user_group_id_for_partition(self.user, user_partition_id) self.assertEqual(group1_id, groups[0].id) # switch to the second group and verify that it is returned for the user self.user_partition.scheme.current_group = groups[1] - group2_id = self.partition_service.get_user_group_id_for_partition(user_partition_id) + group2_id = self.partition_service.get_user_group_id_for_partition(self.user, user_partition_id) self.assertEqual(group2_id, groups[1].id) def test_caching(self): @@ -453,14 +474,14 @@ class TestPartitionService(PartitionTestCase): user_partition_id = self.user_partition.id shared_cache = {} - # Two StaticPartitionService objects that share the same cache: + # Two MockPartitionService objects that share the same cache: ps_shared_cache_1 = self._create_service(username, shared_cache) ps_shared_cache_2 = self._create_service(username, shared_cache) - # A StaticPartitionService with its own local cache + # A MockPartitionService with its own local cache ps_diff_cache = self._create_service(username, {}) - # A StaticPartitionService that never uses caching. + # A MockPartitionService that never uses caching. ps_uncached = self._create_service(username) # Set the group we expect users to be placed into @@ -472,7 +493,7 @@ class TestPartitionService(PartitionTestCase): for part_svc in [ps_shared_cache_1, ps_diff_cache, ps_uncached]: self.assertEqual( first_group.id, - part_svc.get_user_group_id_for_partition(user_partition_id) + part_svc.get_user_group_id_for_partition(self.user, user_partition_id) ) # Now select a new target group @@ -485,20 +506,20 @@ class TestPartitionService(PartitionTestCase): for part_svc in [ps_shared_cache_1, ps_shared_cache_2, ps_diff_cache]: self.assertEqual( first_group.id, - part_svc.get_user_group_id_for_partition(user_partition_id) + part_svc.get_user_group_id_for_partition(self.user, user_partition_id) ) # Our uncached service should be accurate. self.assertEqual( second_group.id, - ps_uncached.get_user_group_id_for_partition(user_partition_id) + ps_uncached.get_user_group_id_for_partition(self.user, user_partition_id) ) # And a newly created service should see the right thing ps_new_cache = self._create_service(username, {}) self.assertEqual( second_group.id, - ps_new_cache.get_user_group_id_for_partition(user_partition_id) + ps_new_cache.get_user_group_id_for_partition(self.user, user_partition_id) ) def test_get_group(self): @@ -509,10 +530,90 @@ class TestPartitionService(PartitionTestCase): # assign first group and verify that it is returned for the user self.user_partition.scheme.current_group = groups[0] - group1 = self.partition_service.get_group(self.user_partition) + group1 = self.partition_service.get_group(self.user, self.user_partition) self.assertEqual(group1, groups[0]) # switch to the second group and verify that it is returned for the user self.user_partition.scheme.current_group = groups[1] - group2 = self.partition_service.get_group(self.user_partition) + group2 = self.partition_service.get_group(self.user, self.user_partition) self.assertEqual(group2, groups[1]) + + +class TestGetCourseUserPartitions(PartitionServiceBaseClass): + """ + Test the helper method get_all_partitions_for_course. + """ + + def setUp(self): + super(TestGetCourseUserPartitions, self).setUp() + # django.conf.settings is not available when nosetests are run + TestGetCourseUserPartitions._enable_enrollment_track_partition(True) + + @staticmethod + def _enable_enrollment_track_partition(enable): + """ + Enable or disable the feature flag for the enrollment track user partition. + """ + FEATURES['ENABLE_ENROLLMENT_TRACK_USER_PARTITION'] = enable + + def test_enrollment_track_partition_added(self): + """ + Test that the dynamic enrollment track scheme is added if there is no conflict with the user partition ID. + """ + all_partitions = get_all_partitions_for_course(self.course) + self.assertEqual(2, len(all_partitions)) + self.assertEqual(self.TEST_SCHEME_NAME, all_partitions[0].scheme.name) + enrollment_track_partition = all_partitions[1] + self.assertEqual(self.ENROLLMENT_TRACK_SCHEME_NAME, enrollment_track_partition.scheme.name) + self.assertEqual(unicode(self.course.id), enrollment_track_partition.parameters['course_id']) + self.assertEqual(ENROLLMENT_TRACK_PARTITION_ID, enrollment_track_partition.id) + + def test_enrollment_track_partition_not_added_if_conflict(self): + """ + Test that the dynamic enrollment track scheme is NOT added if a UserPartition exists with that ID. + """ + self.user_partition = UserPartition( + ENROLLMENT_TRACK_PARTITION_ID, + self.TEST_NAME, + self.TEST_DESCRIPTION, + self.TEST_GROUPS, + self.non_random_scheme, + self.TEST_PARAMETERS, + ) + self.course.user_partitions = [self.user_partition] + all_partitions = get_all_partitions_for_course(self.course) + self.assertEqual(1, len(all_partitions)) + self.assertEqual(self.TEST_SCHEME_NAME, all_partitions[0].scheme.name) + + def test_enrollment_track_partition_not_added_if_disabled(self): + """ + Test that the dynamic enrollment track scheme is NOT added if the settings FEATURE flag is disabled. + """ + TestGetCourseUserPartitions._enable_enrollment_track_partition(False) + all_partitions = get_all_partitions_for_course(self.course) + self.assertEqual(1, len(all_partitions)) + self.assertEqual(self.TEST_SCHEME_NAME, all_partitions[0].scheme.name) + + def test_filter_inactive_user_partitions(self): + """ + Tests supplying the `active_only` parameter. + """ + self.user_partition = UserPartition( + self.TEST_ID, + self.TEST_NAME, + self.TEST_DESCRIPTION, + self.TEST_GROUPS, + self.non_random_scheme, + self.TEST_PARAMETERS, + active=False + ) + self.course.user_partitions = [self.user_partition] + + all_partitions = get_all_partitions_for_course(self.course, active_only=True) + self.assertEqual(1, len(all_partitions)) + self.assertEqual(self.ENROLLMENT_TRACK_SCHEME_NAME, all_partitions[0].scheme.name) + + all_partitions = get_all_partitions_for_course(self.course, active_only=False) + self.assertEqual(2, len(all_partitions)) + self.assertEqual(self.TEST_SCHEME_NAME, all_partitions[0].scheme.name) + self.assertEqual(self.ENROLLMENT_TRACK_SCHEME_NAME, all_partitions[1].scheme.name) diff --git a/common/lib/xmodule/xmodule/split_test_module.py b/common/lib/xmodule/xmodule/split_test_module.py index 144460590e..58aebaa7f5 100644 --- a/common/lib/xmodule/xmodule/split_test_module.py +++ b/common/lib/xmodule/xmodule/split_test_module.py @@ -98,7 +98,8 @@ def get_split_user_partitions(user_partitions): @XBlock.needs('user_tags') # pylint: disable=abstract-method -@XBlock.wants('partitions') +@XBlock.needs('partitions') +@XBlock.needs('user') class SplitTestModule(SplitTestFields, XModule, StudioEditableModule): """ Show the user the appropriate child. Uses the ExperimentState @@ -193,9 +194,9 @@ class SplitTestModule(SplitTestFields, XModule, StudioEditableModule): Returns the group ID, or None if none is available. """ partitions_service = self.runtime.service(self, 'partitions') - if not partitions_service: - return None - return partitions_service.get_user_group_id_for_partition(self.user_partition_id) + user_service = self.runtime.service(self, 'user') + user = user_service._django_user # pylint: disable=protected-access + return partitions_service.get_user_group_id_for_partition(user, self.user_partition_id) @property def is_configured(self): @@ -370,8 +371,8 @@ class SplitTestModule(SplitTestFields, XModule, StudioEditableModule): @XBlock.needs('user_tags') # pylint: disable=abstract-method -@XBlock.wants('partitions') -@XBlock.wants('user') +@XBlock.needs('partitions') +@XBlock.needs('user') class SplitTestDescriptor(SplitTestFields, SequenceDescriptor, StudioEditableDescriptor): # the editing interface can be the same as for sequences -- just a container module_class = SplitTestModule @@ -641,10 +642,6 @@ class SplitTestDescriptor(SplitTestFields, SequenceDescriptor, StudioEditableDes Called from Studio view. """ - user_service = self.runtime.service(self, 'user') - if user_service is None: - return Response() - user_partition = self.get_selected_partition() changed = False diff --git a/common/lib/xmodule/xmodule/tests/test_split_test_module.py b/common/lib/xmodule/xmodule/tests/test_split_test_module.py index e4b35ec591..67dcddd624 100644 --- a/common/lib/xmodule/xmodule/tests/test_split_test_module.py +++ b/common/lib/xmodule/xmodule/tests/test_split_test_module.py @@ -6,7 +6,7 @@ import lxml from mock import Mock, patch from fs.memoryfs import MemoryFS -from xmodule.partitions.tests.test_partitions import StaticPartitionService, PartitionTestCase, MockUserPartitionScheme +from xmodule.partitions.tests.test_partitions import MockPartitionService, PartitionTestCase, MockUserPartitionScheme from xmodule.tests.xml import factories as xml from xmodule.tests.xml import XModuleXmlImportTest from xmodule.tests import get_test_system @@ -14,6 +14,7 @@ from xmodule.x_module import AUTHOR_VIEW, STUDENT_VIEW from xmodule.validation import StudioValidationMessage from xmodule.split_test_module import SplitTestDescriptor, SplitTestFields, get_split_user_partitions from xmodule.partitions.partitions import Group, UserPartition +from xmodule.partitions.partitions_service import MINIMUM_STATIC_PARTITION_ID class SplitTestModuleFactory(xml.XmlImportFactory): @@ -81,21 +82,30 @@ class SplitTestModuleTest(XModuleXmlImportTest, PartitionTestCase): self.module_system.descriptor_runtime = self.course._runtime # pylint: disable=protected-access self.course.runtime.export_fs = MemoryFS() - user = Mock(username='ma', email='ma@edx.org', is_staff=False, is_active=True) - self.partitions_service = StaticPartitionService( - [ - self.user_partition, - UserPartition( - 1, 'second_partition', 'Second Partition', - [Group("0", 'abel'), Group("1", 'baker'), Group("2", 'charlie')], - MockUserPartitionScheme() - ) - ], - user=user, + # Create mock partition service, as these tests are running with XML in-memory system. + self.course.user_partitions = [ + self.user_partition, + UserPartition( + MINIMUM_STATIC_PARTITION_ID, 'second_partition', 'Second Partition', + [ + Group(unicode(MINIMUM_STATIC_PARTITION_ID + 1), 'abel'), + Group(unicode(MINIMUM_STATIC_PARTITION_ID + 2), 'baker'), Group("103", 'charlie') + ], + MockUserPartitionScheme() + ) + ] + partitions_service = MockPartitionService( + self.course, course_id=self.course.id, track_function=Mock(name='track_function'), ) - self.module_system._services['partitions'] = self.partitions_service # pylint: disable=protected-access + self.module_system._services['partitions'] = partitions_service # pylint: disable=protected-access + + # Mock user_service user + user_service = Mock() + user = Mock(username='ma', email='ma@edx.org', is_staff=False, is_active=True) + user_service._django_user = user + self.module_system._services['user'] = user_service # pylint: disable=protected-access self.split_test_module = self.course_sequence.get_children()[0] self.split_test_module.bind_for_student( @@ -103,6 +113,12 @@ class SplitTestModuleTest(XModuleXmlImportTest, PartitionTestCase): user.id ) + # Create mock modulestore for getting the course. Needed for rendering the HTML + # view, since mock services exist and the rendering code will not short-circuit. + mocked_modulestore = Mock() + mocked_modulestore.get_course.return_value = self.course + self.split_test_module.system.modulestore = mocked_modulestore + @ddt.ddt class SplitTestModuleLMSTest(SplitTestModuleTest): diff --git a/lms/djangoapps/ccx/tests/test_field_override_performance.py b/lms/djangoapps/ccx/tests/test_field_override_performance.py index 4919e762ff..e2eecea374 100644 --- a/lms/djangoapps/ccx/tests/test_field_override_performance.py +++ b/lms/djangoapps/ccx/tests/test_field_override_performance.py @@ -231,18 +231,18 @@ class TestFieldOverrideMongoPerformance(FieldOverridePerformanceTestCase): # # of sql queries to default, # # of mongo queries, # ) - ('no_overrides', 1, True, False): (22, 6), - ('no_overrides', 2, True, False): (22, 6), - ('no_overrides', 3, True, False): (22, 6), - ('ccx', 1, True, False): (22, 6), - ('ccx', 2, True, False): (22, 6), - ('ccx', 3, True, False): (22, 6), - ('no_overrides', 1, False, False): (22, 6), - ('no_overrides', 2, False, False): (22, 6), - ('no_overrides', 3, False, False): (22, 6), - ('ccx', 1, False, False): (22, 6), - ('ccx', 2, False, False): (22, 6), - ('ccx', 3, False, False): (22, 6), + ('no_overrides', 1, True, False): (24, 6), + ('no_overrides', 2, True, False): (24, 6), + ('no_overrides', 3, True, False): (24, 6), + ('ccx', 1, True, False): (24, 6), + ('ccx', 2, True, False): (24, 6), + ('ccx', 3, True, False): (24, 6), + ('no_overrides', 1, False, False): (24, 6), + ('no_overrides', 2, False, False): (24, 6), + ('no_overrides', 3, False, False): (24, 6), + ('ccx', 1, False, False): (24, 6), + ('ccx', 2, False, False): (24, 6), + ('ccx', 3, False, False): (24, 6), } @@ -254,19 +254,19 @@ class TestFieldOverrideSplitPerformance(FieldOverridePerformanceTestCase): __test__ = True TEST_DATA = { - ('no_overrides', 1, True, False): (22, 3), - ('no_overrides', 2, True, False): (22, 3), - ('no_overrides', 3, True, False): (22, 3), - ('ccx', 1, True, False): (22, 3), - ('ccx', 2, True, False): (22, 3), - ('ccx', 3, True, False): (22, 3), - ('ccx', 1, True, True): (23, 3), - ('ccx', 2, True, True): (23, 3), - ('ccx', 3, True, True): (23, 3), - ('no_overrides', 1, False, False): (22, 3), - ('no_overrides', 2, False, False): (22, 3), - ('no_overrides', 3, False, False): (22, 3), - ('ccx', 1, False, False): (22, 3), - ('ccx', 2, False, False): (22, 3), - ('ccx', 3, False, False): (22, 3), + ('no_overrides', 1, True, False): (24, 3), + ('no_overrides', 2, True, False): (24, 3), + ('no_overrides', 3, True, False): (24, 3), + ('ccx', 1, True, False): (24, 3), + ('ccx', 2, True, False): (24, 3), + ('ccx', 3, True, False): (24, 3), + ('ccx', 1, True, True): (25, 3), + ('ccx', 2, True, True): (25, 3), + ('ccx', 3, True, True): (25, 3), + ('no_overrides', 1, False, False): (24, 3), + ('no_overrides', 2, False, False): (24, 3), + ('no_overrides', 3, False, False): (24, 3), + ('ccx', 1, False, False): (24, 3), + ('ccx', 2, False, False): (24, 3), + ('ccx', 3, False, False): (24, 3), } diff --git a/lms/djangoapps/course_api/blocks/tests/test_api.py b/lms/djangoapps/course_api/blocks/tests/test_api.py index ab5d2c667d..9e4d448eb5 100644 --- a/lms/djangoapps/course_api/blocks/tests/test_api.py +++ b/lms/djangoapps/course_api/blocks/tests/test_api.py @@ -146,7 +146,7 @@ class TestGetBlocksQueryCounts(SharedModuleStoreTestCase): self._get_blocks( course, expected_mongo_queries=0, - expected_sql_queries=3 if with_storage_backing else 2, + expected_sql_queries=5 if with_storage_backing else 4, ) @ddt.data( @@ -164,5 +164,5 @@ class TestGetBlocksQueryCounts(SharedModuleStoreTestCase): self._get_blocks( course, expected_mongo_queries, - expected_sql_queries=11 if with_storage_backing else 3, + expected_sql_queries=13 if with_storage_backing else 5, ) diff --git a/lms/djangoapps/course_blocks/transformers/user_partitions.py b/lms/djangoapps/course_blocks/transformers/user_partitions.py index 78930ae879..a01cdf3b54 100644 --- a/lms/djangoapps/course_blocks/transformers/user_partitions.py +++ b/lms/djangoapps/course_blocks/transformers/user_partitions.py @@ -5,6 +5,7 @@ from openedx.core.djangoapps.content.block_structure.transformer import ( BlockStructureTransformer, FilteringTransformerMixin, ) +from xmodule.partitions.partitions_service import get_all_partitions_for_course from .split_test import SplitTestTransformer from .utils import get_field_on_block @@ -46,11 +47,7 @@ class UserPartitionTransformer(FilteringTransformerMixin, BlockStructureTransfor # Because user partitions are course-wide, only store data for # them on the root block. root_block = block_structure.get_xblock(block_structure.root_block_usage_key) - user_partitions = [ - user_partition - for user_partition in getattr(root_block, 'user_partitions', []) - if user_partition.active - ] + user_partitions = get_all_partitions_for_course(root_block, active_only=True) block_structure.set_transformer_data(cls, 'user_partitions', user_partitions) # If there are no user partitions, this transformation is a diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py index dae7390714..e72a129a59 100644 --- a/lms/djangoapps/courseware/access.py +++ b/lms/djangoapps/courseware/access.py @@ -30,7 +30,6 @@ from xmodule.course_module import ( ) from xmodule.error_module import ErrorDescriptor from xmodule.x_module import XModule -from xmodule.split_test_module import get_split_user_partitions from xmodule.partitions.partitions import NoSuchUserPartitionError, NoSuchUserPartitionGroupError from courseware.access_response import ( @@ -466,12 +465,6 @@ def _has_group_access(descriptor, user, course_key): This function returns a boolean indicating whether or not `user` has sufficient group memberships to "load" a block (the `descriptor`) """ - if len(descriptor.user_partitions) == len(get_split_user_partitions(descriptor.user_partitions)): - # Short-circuit the process, since there are no defined user partitions that are not - # user_partitions used by the split_test module. The split_test module handles its own access - # via updating the children of the split_test module. - return ACCESS_GRANTED - # Allow staff and instructors roles group access, as they are not masquerading as a student. if get_user_role(user, course_key) in ['staff', 'instructor']: return ACCESS_GRANTED diff --git a/lms/djangoapps/courseware/tests/test_access.py b/lms/djangoapps/courseware/tests/test_access.py index c0e9b33b41..eda22d5172 100644 --- a/lms/djangoapps/courseware/tests/test_access.py +++ b/lms/djangoapps/courseware/tests/test_access.py @@ -45,6 +45,7 @@ from xmodule.course_module import ( ) from xmodule.error_module import ErrorDescriptor from xmodule.partitions.partitions import Group, UserPartition +from xmodule.partitions.partitions_service import MINIMUM_STATIC_PARTITION_ID from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory @@ -301,9 +302,11 @@ class AccessTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase, MilestonesTes """ Test that a user masquerading as a member of a group sees appropriate content in preview mode. """ - partition_id = 0 - group_0_id = 0 - group_1_id = 1 + # Note about UserPartition and UserPartition Group IDs: these must not conflict with IDs used + # by dynamic user partitions. + partition_id = MINIMUM_STATIC_PARTITION_ID + group_0_id = MINIMUM_STATIC_PARTITION_ID + 1 + group_1_id = MINIMUM_STATIC_PARTITION_ID + 2 user_partition = UserPartition( partition_id, 'Test User Partition', '', [Group(group_0_id, 'Group 1'), Group(group_1_id, 'Group 2')], @@ -314,7 +317,6 @@ class AccessTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase, MilestonesTes chapter = ItemFactory.create(category="chapter", parent_location=self.course.location) chapter.group_access = {partition_id: [group_0_id]} - chapter.user_partitions = self.course.user_partitions modulestore().update_item(self.course, ModuleStoreEnum.UserID.test) @@ -431,6 +433,7 @@ class AccessTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase, MilestonesTes user = Mock() descriptor = Mock(user_partitions=[]) descriptor._class_tags = {} + descriptor.merged_group_access = {} # Always returns true because DISABLE_START_DATES is set in test.py self.assertTrue(access._has_access_descriptor(user, 'load', descriptor)) @@ -457,6 +460,8 @@ class AccessTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase, MilestonesTes mock_unit._class_tags = {} # Needed for detached check in _has_access_descriptor mock_unit.visible_to_staff_only = visible_to_staff_only mock_unit.start = start + mock_unit.merged_group_access = {} + self.verify_access(mock_unit, expected_access, expected_error_type) def test__has_access_descriptor_beta_user(self): @@ -465,6 +470,7 @@ class AccessTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase, MilestonesTes mock_unit.days_early_for_beta = 2 mock_unit.start = self.TOMORROW mock_unit.visible_to_staff_only = False + mock_unit.merged_group_access = {} self.assertTrue(bool(access._has_access_descriptor( self.beta_user, 'load', mock_unit, course_key=self.course.id))) @@ -480,6 +486,8 @@ class AccessTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase, MilestonesTes mock_unit._class_tags = {} # Needed for detached check in _has_access_descriptor mock_unit.visible_to_staff_only = False mock_unit.start = start + mock_unit.merged_group_access = {} + self.verify_access(mock_unit, True) @ddt.data( @@ -499,6 +507,8 @@ class AccessTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase, MilestonesTes mock_unit._class_tags = {} # Needed for detached check in _has_access_descriptor mock_unit.visible_to_staff_only = False mock_unit.start = start + mock_unit.merged_group_access = {} + self.verify_access(mock_unit, expected_access, expected_error_type) def test__has_access_course_can_enroll(self): diff --git a/lms/djangoapps/courseware/tests/test_group_access.py b/lms/djangoapps/courseware/tests/test_group_access.py index 4566948eeb..a762e4d88b 100644 --- a/lms/djangoapps/courseware/tests/test_group_access.py +++ b/lms/djangoapps/courseware/tests/test_group_access.py @@ -406,31 +406,3 @@ class GroupAccessTestCase(ModuleStoreTestCase): self.check_access(self.blue_dog, block_accessed, False) self.check_access(self.gray_worm, block_accessed, False) self.ensure_staff_access(block_accessed) - - def test_group_access_short_circuits(self): - """ - Test that the group_access check short-circuits if there are no user_partitions defined - except user_partitions in use by the split_test module. - """ - # Initially, "red_cat" user can't view the vertical. - self.set_group_access(self.chapter_location, {self.animal_partition.id: [self.dog_group.id]}) - self.check_access(self.red_cat, self.vertical_location, False) - - # Change the vertical's user_partitions value to the empty list. Now red_cat can view the vertical. - self.set_user_partitions(self.vertical_location, []) - self.check_access(self.red_cat, self.vertical_location, True) - - # Change the vertical's user_partitions value to include only "split_test" partitions. - split_test_partition = UserPartition( - 199, - 'split_test partition', - 'nothing to look at here', - [Group(2, 'random group')], - scheme=UserPartition.get_scheme("random"), - ) - self.set_user_partitions(self.vertical_location, [split_test_partition]) - self.check_access(self.red_cat, self.vertical_location, True) - - # Finally, add back in a cohort user_partition - self.set_user_partitions(self.vertical_location, [split_test_partition, self.animal_partition]) - self.check_access(self.red_cat, self.vertical_location, False) diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index fbfda4228d..42865e2c9f 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -206,7 +206,7 @@ class IndexQueryTestCase(ModuleStoreTestCase): NUM_PROBLEMS = 20 @ddt.data( - (ModuleStoreEnum.Type.mongo, 9), + (ModuleStoreEnum.Type.mongo, 10), (ModuleStoreEnum.Type.split, 4), ) @ddt.unpack @@ -1420,17 +1420,17 @@ class ProgressPageTests(ModuleStoreTestCase): """Test that query counts remain the same for self-paced and instructor-paced courses.""" SelfPacedConfiguration(enabled=self_paced_enabled).save() self.setup_course(self_paced=self_paced) - with self.assertNumQueries(39), check_mongo_calls(4): + with self.assertNumQueries(41), check_mongo_calls(4): self._get_progress_page() def test_progress_queries(self): self.setup_course() - with self.assertNumQueries(39), check_mongo_calls(4): + with self.assertNumQueries(41), check_mongo_calls(4): self._get_progress_page() # subsequent accesses to the progress page require fewer queries. for _ in range(2): - with self.assertNumQueries(25), check_mongo_calls(4): + with self.assertNumQueries(27), check_mongo_calls(4): self._get_progress_page() @patch( diff --git a/lms/djangoapps/courseware/testutils.py b/lms/djangoapps/courseware/testutils.py index 1ef13ad45e..c09da8543a 100644 --- a/lms/djangoapps/courseware/testutils.py +++ b/lms/djangoapps/courseware/testutils.py @@ -178,6 +178,8 @@ class RenderXBlockTestMixin(object): @ddt.unpack def test_success_enrolled_staff(self, default_store, mongo_calls): with self.store.default_store(default_store): + if default_store is ModuleStoreEnum.Type.mongo: + mongo_calls = self.get_success_enrolled_staff_mongo_count() self.setup_course(default_store) self.setup_user(admin=True, enroll=True, login=True) @@ -197,6 +199,13 @@ class RenderXBlockTestMixin(object): with check_mongo_calls(mongo_calls): self.verify_response() + def get_success_enrolled_staff_mongo_count(self): + """ + Helper method used by test_success_enrolled_staff because one test + class using this mixin has an increased number of mongo (only) queries. + """ + return 5 + def test_success_unenrolled_staff(self): self.setup_course() self.setup_user(admin=True, enroll=False, login=True) diff --git a/lms/djangoapps/grades/tests/test_tasks.py b/lms/djangoapps/grades/tests/test_tasks.py index d6aa9e221e..82860c18a7 100644 --- a/lms/djangoapps/grades/tests/test_tasks.py +++ b/lms/djangoapps/grades/tests/test_tasks.py @@ -154,10 +154,10 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest self.assertEquals(mock_block_structure_create.call_count, 1) @ddt.data( - (ModuleStoreEnum.Type.mongo, 1, 24, True), - (ModuleStoreEnum.Type.mongo, 1, 21, False), - (ModuleStoreEnum.Type.split, 3, 23, True), - (ModuleStoreEnum.Type.split, 3, 20, False), + (ModuleStoreEnum.Type.mongo, 1, 26, True), + (ModuleStoreEnum.Type.mongo, 1, 23, False), + (ModuleStoreEnum.Type.split, 3, 25, True), + (ModuleStoreEnum.Type.split, 3, 22, False), ) @ddt.unpack def test_query_counts(self, default_store, num_mongo_calls, num_sql_calls, create_multiple_subsections): @@ -169,8 +169,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest self._apply_recalculate_subsection_grade() @ddt.data( - (ModuleStoreEnum.Type.mongo, 1, 24), - (ModuleStoreEnum.Type.split, 3, 23), + (ModuleStoreEnum.Type.mongo, 1, 26), + (ModuleStoreEnum.Type.split, 3, 25), ) @ddt.unpack def test_query_counts_dont_change_with_more_content(self, default_store, num_mongo_calls, num_sql_calls): @@ -215,8 +215,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest ) @ddt.data( - (ModuleStoreEnum.Type.mongo, 1, 9), - (ModuleStoreEnum.Type.split, 3, 8), + (ModuleStoreEnum.Type.mongo, 1, 11), + (ModuleStoreEnum.Type.split, 3, 10), ) @ddt.unpack def test_persistent_grades_not_enabled_on_course(self, default_store, num_mongo_queries, num_sql_queries): @@ -230,8 +230,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest self.assertEqual(len(PersistentSubsectionGrade.bulk_read_grades(self.user.id, self.course.id)), 0) @ddt.data( - (ModuleStoreEnum.Type.mongo, 1, 22), - (ModuleStoreEnum.Type.split, 3, 21), + (ModuleStoreEnum.Type.mongo, 1, 24), + (ModuleStoreEnum.Type.split, 3, 23), ) @ddt.unpack def test_persistent_grades_enabled_on_course(self, default_store, num_mongo_queries, num_sql_queries): @@ -409,8 +409,8 @@ class ComputeGradesForCourseTest(HasCourseWithProblemsMixin, ModuleStoreTestCase @ddt.data(*xrange(1, 12, 3)) def test_database_calls(self, batch_size): - per_user_queries = 16 * min(batch_size, 6) # No more than 6 due to offset - with self.assertNumQueries(3 + 16 * min(batch_size, 6)): + per_user_queries = 18 * min(batch_size, 6) # No more than 6 due to offset + with self.assertNumQueries(3 + per_user_queries): with check_mongo_calls(1): compute_grades_for_course.delay( course_key=six.text_type(self.course.id), diff --git a/lms/djangoapps/instructor/views/instructor_dashboard.py b/lms/djangoapps/instructor/views/instructor_dashboard.py index c1d70f1aef..524565dffb 100644 --- a/lms/djangoapps/instructor/views/instructor_dashboard.py +++ b/lms/djangoapps/instructor/views/instructor_dashboard.py @@ -54,6 +54,7 @@ from class_dashboard.dashboard_data import get_section_display_name, get_array_s from .tools import get_units_with_due_date, title_or_url from opaque_keys.edx.locations import SlashSeparatedCourseKey from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers +from openedx.core.djangoapps.verified_track_content.models import VerifiedTrackCohortedCourse from openedx.core.djangolib.markup import HTML, Text @@ -640,7 +641,6 @@ def _section_send_email(course, access): if is_course_cohorted(course_key): cohorts = get_course_cohorts(course) course_modes = [] - from verified_track_content.models import VerifiedTrackCohortedCourse if not VerifiedTrackCohortedCourse.is_verified_track_cohort_enabled(course_key): course_modes = CourseMode.modes_for_course(course_key, include_expired=True, only_selectable=False) email_editor = fragment.content diff --git a/lms/djangoapps/instructor_task/tasks_helper.py b/lms/djangoapps/instructor_task/tasks_helper.py index 94912b6a09..851d485eb7 100644 --- a/lms/djangoapps/instructor_task/tasks_helper.py +++ b/lms/djangoapps/instructor_task/tasks_helper.py @@ -30,6 +30,7 @@ from lms.djangoapps.verify_student.models import SoftwareSecurePhotoVerification from pytz import UTC from track import contexts from xmodule.modulestore.django import modulestore +from xmodule.partitions.partitions_service import PartitionService from xmodule.split_test_module import get_split_user_partitions from certificates.api import generate_user_certificates @@ -59,7 +60,6 @@ from shoppingcart.models import ( ) from openassessment.data import OraAggregateData from lms.djangoapps.instructor_task.models import ReportStore, InstructorTask, PROGRESS -from lms.djangoapps.lms_xblock.runtime import LmsPartitionService from openedx.core.djangoapps.course_groups.cohorts import get_cohort from openedx.core.djangoapps.course_groups.models import CourseUserGroup from opaque_keys.edx.keys import UsageKey @@ -806,7 +806,7 @@ def upload_grades_csv(_xmodule_instance_args, _entry_id, course_id, _task_input, group_configs_group_names = [] for partition in experiment_partitions: - group = LmsPartitionService(student, course_id).get_group(partition, assign=False) + group = PartitionService(course_id).get_group(student, partition, assign=False) group_configs_group_names.append(group.name if group else '') team_name = [] diff --git a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py index 24d517d7e6..999edb8476 100644 --- a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py +++ b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py @@ -1775,7 +1775,7 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): 'failed': 3, 'skipped': 2 } - with self.assertNumQueries(168): + with self.assertNumQueries(184): self.assertCertificatesGenerated(task_input, expected_results) expected_results = { diff --git a/lms/djangoapps/lms_xblock/mixin.py b/lms/djangoapps/lms_xblock/mixin.py index e4eb865363..b33ac2eadc 100644 --- a/lms/djangoapps/lms_xblock/mixin.py +++ b/lms/djangoapps/lms_xblock/mixin.py @@ -5,6 +5,7 @@ Namespace that defines fields common to all blocks used in the LMS #from django.utils.translation import ugettext_noop as _ from lazy import lazy +from xblock.core import XBlock from xblock.fields import Boolean, Scope, String, XBlockMixin, Dict from xblock.validation import ValidationMessage from xmodule.modulestore.inheritance import UserPartitionList @@ -26,6 +27,7 @@ class GroupAccessDict(Dict): return {unicode(k): access_dict[k] for k in access_dict} +@XBlock.needs('partitions') class LmsBlockMixin(XBlockMixin): """ Mixin that defines fields common to all blocks used in the LMS @@ -128,10 +130,10 @@ class LmsBlockMixin(XBlockMixin): def _get_user_partition(self, user_partition_id): """ - Returns the user partition with the specified id. Raises - `NoSuchUserPartitionError` if the lookup fails. + Returns the user partition with the specified id. Note that this method can return + an inactive user partition. Raises `NoSuchUserPartitionError` if the lookup fails. """ - for user_partition in self.user_partitions: + for user_partition in self.runtime.service(self, 'partitions').course_partitions: if user_partition.id == user_partition_id: return user_partition diff --git a/lms/djangoapps/lms_xblock/runtime.py b/lms/djangoapps/lms_xblock/runtime.py index ec948d2372..58bf0f1f96 100644 --- a/lms/djangoapps/lms_xblock/runtime.py +++ b/lms/djangoapps/lms_xblock/runtime.py @@ -80,22 +80,6 @@ def local_resource_url(block, uri): return xblock_local_resource_url(block, uri) -class LmsPartitionService(PartitionService): - """ - Another runtime mixin that provides access to the student partitions defined on the - course. - - (If and when XBlock directly provides access from one block (e.g. a split_test_module) - to another (e.g. a course_module), this won't be necessary, but for now it seems like - the least messy way to hook things through) - - """ - @property - def course_partitions(self): - course = modulestore().get_course(self._course_id) - return course.user_partitions - - class UserTagsService(object): """ A runtime class that provides an interface to the user service. It handles filling in @@ -154,8 +138,7 @@ class LmsModuleSystem(ModuleSystem): # pylint: disable=abstract-method services['fs'] = xblock.reference.plugins.FSService() services['i18n'] = ModuleI18nService services['library_tools'] = LibraryToolsService(modulestore()) - services['partitions'] = LmsPartitionService( - user=kwargs.get('user'), + services['partitions'] = PartitionService( course_id=kwargs.get('course_id'), track_function=kwargs.get('track_function', None), cache=request_cache_dict diff --git a/lms/djangoapps/lti_provider/tests/test_views.py b/lms/djangoapps/lti_provider/tests/test_views.py index ed7f539585..08c7053877 100644 --- a/lms/djangoapps/lti_provider/tests/test_views.py +++ b/lms/djangoapps/lti_provider/tests/test_views.py @@ -170,6 +170,8 @@ class LtiLaunchTestRender(LtiTestMixin, RenderXBlockTestMixin, ModuleStoreTestCa This class overrides the get_response method, which is used by the tests defined in RenderXBlockTestMixin. """ + SUCCESS_ENROLLED_STAFF_MONGO_COUNT = 9 + def setUp(self): """ Set up tests @@ -212,3 +214,21 @@ class LtiLaunchTestRender(LtiTestMixin, RenderXBlockTestMixin, ModuleStoreTestCa self.setup_course() self.setup_user(admin=False, enroll=True, login=False) self.verify_response() + + def get_success_enrolled_staff_mongo_count(self): + """ + Override because mongo queries are higher for this + particular test. This has not been investigated exhaustively + as mongo is no longer used much, and removing user_partitions + from inheritance fixes the problem. + + # The 9 mongoDB calls include calls for + # Old Mongo: + # (1) fill_in_run + # (2) get_course in get_course_with_access + # (3) get_item for HTML block in get_module_by_usage_id + # (4) get_parent when loading HTML block + # (5)-(8) calls related to the inherited user_partitions field. + # (9) edx_notes descriptor call to get_course + """ + return 9 diff --git a/lms/envs/bok_choy.py b/lms/envs/bok_choy.py index 40988c28db..6542ed85f5 100644 --- a/lms/envs/bok_choy.py +++ b/lms/envs/bok_choy.py @@ -146,6 +146,8 @@ FEATURES['AUTOMATIC_AUTH_FOR_TESTING'] = True # Open up endpoint for faking Software Secure responses FEATURES['ENABLE_SOFTWARE_SECURE_FAKE'] = True +FEATURES['ENABLE_ENROLLMENT_TRACK_USER_PARTITION'] = True + ########################### Entrance Exams ################################# FEATURES['ENTRANCE_EXAMS'] = True diff --git a/lms/envs/common.py b/lms/envs/common.py index 193593eb12..7e64efcd93 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -371,6 +371,9 @@ FEATURES = { # Enable footer banner for cookie consent. # See https://cookieconsent.insites.com/ for more. 'ENABLE_COOKIE_CONSENT': False, + + # Whether or not the dynamic EnrollmentTrackUserPartition should be registered. + 'ENABLE_ENROLLMENT_TRACK_USER_PARTITION': False, } # Ignore static asset files on import which match this pattern @@ -2146,8 +2149,8 @@ INSTALLED_APPS = ( # API access administration 'openedx.core.djangoapps.api_admin', - # Verified Track Content Cohorting - 'verified_track_content', + # Verified Track Content Cohorting (Beta feature that will hopefully be removed) + 'openedx.core.djangoapps.verified_track_content', # Learner's dashboard 'learner_dashboard', @@ -3068,3 +3071,13 @@ DOC_LINK_BASE_URL = None ENTERPRISE_ENROLLMENT_API_URL = LMS_ROOT_URL + "/api/enrollment/v1/" ENTERPRISE_PUBLIC_ENROLLMENT_API_URL = ENTERPRISE_ENROLLMENT_API_URL ENTERPRISE_API_CACHE_TIMEOUT = 3600 # Value is in seconds + +############## Settings for Course Enrollment Modes ###################### +COURSE_ENROLLMENT_MODES = { + "audit": 1, + "verified": 2, + "professional": 3, + "no-id-professional": 4, + "credit": 5, + "honor": 6, +} diff --git a/lms/envs/test.py b/lms/envs/test.py index bea3cbf823..44e4431f42 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -78,6 +78,8 @@ FEATURES['ENABLE_COMBINED_LOGIN_REGISTRATION'] = True # Enable the milestones app in tests to be consistent with it being enabled in production FEATURES['MILESTONES_APP'] = True +FEATURES['ENABLE_ENROLLMENT_TRACK_USER_PARTITION'] = True + # Need wiki for courseware views to work. TODO (vshnayder): shouldn't need it. WIKI_ENABLED = True diff --git a/lms/urls.py b/lms/urls.py index 0dedb36db9..1974bcf6cf 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -553,7 +553,7 @@ urlpatterns += ( r'^courses/{}/verified_track_content/settings'.format( settings.COURSE_KEY_PATTERN, ), - 'verified_track_content.views.cohorting_settings', + 'openedx.core.djangoapps.verified_track_content.views.cohorting_settings', name='verified_track_cohorting', ), url( diff --git a/openedx/core/djangoapps/credit/verification_access.py b/openedx/core/djangoapps/credit/verification_access.py index 5a5c625718..4bdac37274 100644 --- a/openedx/core/djangoapps/credit/verification_access.py +++ b/openedx/core/djangoapps/credit/verification_access.py @@ -141,7 +141,7 @@ def _set_verification_partitions(course_key, icrv_blocks): log.error("Could not find course %s", course_key) return [] - verified_partitions = course.get_user_partitions_for_scheme(scheme) + verified_partitions = [p for p in course.user_partitions if p.scheme == scheme] partition_id_for_location = { p.parameters["location"]: p.id for p in verified_partitions diff --git a/lms/djangoapps/verified_track_content/__init__.py b/openedx/core/djangoapps/verified_track_content/__init__.py similarity index 100% rename from lms/djangoapps/verified_track_content/__init__.py rename to openedx/core/djangoapps/verified_track_content/__init__.py diff --git a/lms/djangoapps/verified_track_content/admin.py b/openedx/core/djangoapps/verified_track_content/admin.py similarity index 61% rename from lms/djangoapps/verified_track_content/admin.py rename to openedx/core/djangoapps/verified_track_content/admin.py index 71ae02219e..b536c596fe 100644 --- a/lms/djangoapps/verified_track_content/admin.py +++ b/openedx/core/djangoapps/verified_track_content/admin.py @@ -4,8 +4,8 @@ Django admin page for verified track configuration from django.contrib import admin -from verified_track_content.forms import VerifiedTrackCourseForm -from verified_track_content.models import VerifiedTrackCohortedCourse +from openedx.core.djangoapps.verified_track_content.forms import VerifiedTrackCourseForm +from openedx.core.djangoapps.verified_track_content.models import VerifiedTrackCohortedCourse @admin.register(VerifiedTrackCohortedCourse) diff --git a/lms/djangoapps/verified_track_content/forms.py b/openedx/core/djangoapps/verified_track_content/forms.py similarity index 94% rename from lms/djangoapps/verified_track_content/forms.py rename to openedx/core/djangoapps/verified_track_content/forms.py index 2de81ee8c6..53eddf9719 100644 --- a/lms/djangoapps/verified_track_content/forms.py +++ b/openedx/core/djangoapps/verified_track_content/forms.py @@ -9,7 +9,7 @@ from xmodule.modulestore.django import modulestore from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey -from verified_track_content.models import VerifiedTrackCohortedCourse +from openedx.core.djangoapps.verified_track_content.models import VerifiedTrackCohortedCourse class VerifiedTrackCourseForm(forms.ModelForm): diff --git a/lms/djangoapps/verified_track_content/migrations/0001_initial.py b/openedx/core/djangoapps/verified_track_content/migrations/0001_initial.py similarity index 100% rename from lms/djangoapps/verified_track_content/migrations/0001_initial.py rename to openedx/core/djangoapps/verified_track_content/migrations/0001_initial.py diff --git a/lms/djangoapps/verified_track_content/migrations/0002_verifiedtrackcohortedcourse_verified_cohort_name.py b/openedx/core/djangoapps/verified_track_content/migrations/0002_verifiedtrackcohortedcourse_verified_cohort_name.py similarity index 100% rename from lms/djangoapps/verified_track_content/migrations/0002_verifiedtrackcohortedcourse_verified_cohort_name.py rename to openedx/core/djangoapps/verified_track_content/migrations/0002_verifiedtrackcohortedcourse_verified_cohort_name.py diff --git a/lms/djangoapps/verified_track_content/migrations/__init__.py b/openedx/core/djangoapps/verified_track_content/migrations/__init__.py similarity index 100% rename from lms/djangoapps/verified_track_content/migrations/__init__.py rename to openedx/core/djangoapps/verified_track_content/migrations/__init__.py diff --git a/lms/djangoapps/verified_track_content/models.py b/openedx/core/djangoapps/verified_track_content/models.py similarity index 97% rename from lms/djangoapps/verified_track_content/models.py rename to openedx/core/djangoapps/verified_track_content/models.py index fcf7f71131..213c53bf5b 100644 --- a/lms/djangoapps/verified_track_content/models.py +++ b/openedx/core/djangoapps/verified_track_content/models.py @@ -8,9 +8,9 @@ from django.db.models.signals import post_save, pre_save from openedx.core.djangoapps.xmodule_django.models import CourseKeyField from student.models import CourseEnrollment -from courseware.courses import get_course_by_id +from lms.djangoapps.courseware.courses import get_course_by_id -from verified_track_content.tasks import sync_cohort_with_mode +from openedx.core.djangoapps.verified_track_content.tasks import sync_cohort_with_mode from openedx.core.djangoapps.course_groups.cohorts import ( get_course_cohorts, CourseCohort, is_course_cohorted, get_random_cohort ) diff --git a/openedx/core/djangoapps/verified_track_content/partition_scheme.py b/openedx/core/djangoapps/verified_track_content/partition_scheme.py new file mode 100644 index 0000000000..b93a014f78 --- /dev/null +++ b/openedx/core/djangoapps/verified_track_content/partition_scheme.py @@ -0,0 +1,138 @@ +""" +UserPartitionScheme for enrollment tracks. +""" +from django.conf import settings + +from courseware.masquerade import ( + get_course_masquerade, + get_masquerading_group_info, + is_masquerading_as_specific_student, +) +from course_modes.models import CourseMode +from student.models import CourseEnrollment +from opaque_keys.edx.keys import CourseKey +from openedx.core.djangoapps.verified_track_content.models import VerifiedTrackCohortedCourse +from xmodule.partitions.partitions import NoSuchUserPartitionGroupError, Group, UserPartition + + +# These IDs must be less than 100 so that they do not overlap with Groups in +# CohortUserPartition or RandomUserPartitionScheme +# (CMS' course_group_config uses a minimum value of 100 for all generated IDs). +ENROLLMENT_GROUP_IDS = settings.COURSE_ENROLLMENT_MODES + + +class EnrollmentTrackUserPartition(UserPartition): + """ + Extends UserPartition to support dynamic groups pulled from the current course Enrollment tracks. + """ + + @property + def groups(self): + """ + Return the groups (based on CourseModes) for the course associated with this + EnrollmentTrackUserPartition instance. + + If a course is using the Verified Track Cohorting pilot feature, this method + returns an empty array regardless of registered CourseModes. + """ + course_key = CourseKey.from_string(self.parameters["course_id"]) + + if is_course_using_cohort_instead(course_key): + return [] + + return [ + Group(ENROLLMENT_GROUP_IDS[mode.slug], unicode(mode.name)) + for mode in CourseMode.modes_for_course(course_key, include_expired=True, only_selectable=False) + ] + + def to_json(self): + """ + Because this partition is dynamic, to_json and from_json are not supported. + + Calling this method will raise a TypeError. + """ + raise TypeError("Because EnrollmentTrackUserPartition is a dynamic partition, 'to_json' is not supported.") + + def from_json(self): + """ + Because this partition is dynamic, to_json and from_json are not supported. + + Calling this method will raise a TypeError. + """ + raise TypeError("Because EnrollmentTrackUserPartition is a dynamic partition, 'from_json' is not supported.") + + +class EnrollmentTrackPartitionScheme(object): + """ + This scheme uses learner enrollment tracks to map learners into partition groups. + """ + + @classmethod + def get_group_for_user(cls, course_key, user, user_partition, **kwargs): # pylint: disable=unused-argument + """ + Returns the Group from the specified user partition to which the user + is assigned, via enrollment mode. + + If a course is using the Verified Track Cohorting pilot feature, this method + returns None regardless of the user's enrollment mode. + """ + if is_course_using_cohort_instead(course_key): + return None + + # NOTE: masquerade code was copied from CohortPartitionScheme, and it may need + # some changes (or if not, code should be refactored out and shared). + # This work will be done in a future story TNL-6739. + + # First, check if we have to deal with masquerading. + # If the current user is masquerading as a specific student, use the + # same logic as normal to return that student's group. If the current + # user is masquerading as a generic student in a specific group, then + # return that group. + if get_course_masquerade(user, course_key) and not is_masquerading_as_specific_student(user, course_key): + group_id, user_partition_id = get_masquerading_group_info(user, course_key) + if user_partition_id == user_partition.id and group_id is not None: + try: + return user_partition.get_group(group_id) + except NoSuchUserPartitionGroupError: + return None + # The user is masquerading as a generic student. We can't show any particular group. + return None + + mode_slug, is_active = CourseEnrollment.enrollment_mode_for_user(user, course_key) + if mode_slug and is_active: + course_mode = CourseMode.mode_for_course( + course_key, + mode_slug, + modes=CourseMode.modes_for_course(course_key, include_expired=True, only_selectable=False), + ) + if not course_mode: + course_mode = CourseMode.DEFAULT_MODE + return Group(ENROLLMENT_GROUP_IDS[course_mode.slug], unicode(course_mode.name)) + else: + return None + + @classmethod + def create_user_partition(cls, id, name, description, groups=None, parameters=None, active=True): # pylint: disable=redefined-builtin, invalid-name, unused-argument + """ + Create a custom UserPartition to support dynamic 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 course ID for this partition scheme. + + 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. + """ + return EnrollmentTrackUserPartition(id, name, description, [], cls, parameters, active) + + +def is_course_using_cohort_instead(course_key): + """ + Returns whether the given course_context is using verified-track cohorts + and therefore shouldn't use a track-based partition. + """ + return VerifiedTrackCohortedCourse.is_verified_track_cohort_enabled(course_key) diff --git a/lms/djangoapps/verified_track_content/tasks.py b/openedx/core/djangoapps/verified_track_content/tasks.py similarity index 100% rename from lms/djangoapps/verified_track_content/tasks.py rename to openedx/core/djangoapps/verified_track_content/tasks.py diff --git a/lms/djangoapps/verified_track_content/tests/__init__.py b/openedx/core/djangoapps/verified_track_content/tests/__init__.py similarity index 100% rename from lms/djangoapps/verified_track_content/tests/__init__.py rename to openedx/core/djangoapps/verified_track_content/tests/__init__.py diff --git a/lms/djangoapps/verified_track_content/tests/test_forms.py b/openedx/core/djangoapps/verified_track_content/tests/test_forms.py similarity index 94% rename from lms/djangoapps/verified_track_content/tests/test_forms.py rename to openedx/core/djangoapps/verified_track_content/tests/test_forms.py index b400c292c0..c80c7db687 100644 --- a/lms/djangoapps/verified_track_content/tests/test_forms.py +++ b/openedx/core/djangoapps/verified_track_content/tests/test_forms.py @@ -4,7 +4,7 @@ Test for forms helpers. from xmodule.modulestore.tests.factories import CourseFactory from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase -from verified_track_content.forms import VerifiedTrackCourseForm +from openedx.core.djangoapps.verified_track_content.forms import VerifiedTrackCourseForm class TestVerifiedTrackCourseForm(SharedModuleStoreTestCase): diff --git a/lms/djangoapps/verified_track_content/tests/test_models.py b/openedx/core/djangoapps/verified_track_content/tests/test_models.py similarity index 92% rename from lms/djangoapps/verified_track_content/tests/test_models.py rename to openedx/core/djangoapps/verified_track_content/tests/test_models.py index 71d2458d74..0c3a4c8d10 100644 --- a/lms/djangoapps/verified_track_content/tests/test_models.py +++ b/openedx/core/djangoapps/verified_track_content/tests/test_models.py @@ -1,6 +1,9 @@ """ Tests for Verified Track Cohorting models """ +# pylint: disable=attribute-defined-outside-init +# pylint: disable=no-member + from django.test import TestCase import mock from mock import patch @@ -12,11 +15,12 @@ from student.models import CourseMode from student.tests.factories import UserFactory, CourseEnrollmentFactory from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory -from verified_track_content.models import VerifiedTrackCohortedCourse, DEFAULT_VERIFIED_COHORT_NAME -from verified_track_content.tasks import sync_cohort_with_mode +from ..models import VerifiedTrackCohortedCourse, DEFAULT_VERIFIED_COHORT_NAME +from ..tasks import sync_cohort_with_mode from openedx.core.djangoapps.course_groups.cohorts import ( set_course_cohort_settings, add_cohort, CourseCohort, DEFAULT_COHORT_NAME ) +from openedx.core.djangolib.testing.utils import skip_unless_lms class TestVerifiedTrackCohortedCourse(TestCase): @@ -48,13 +52,13 @@ class TestVerifiedTrackCohortedCourse(TestCase): self.assertEqual(unicode(config), "Course: {}, enabled: True".format(self.SAMPLE_COURSE)) def test_verified_cohort_name(self): - COHORT_NAME = 'verified cohort' + cohort_name = 'verified cohort' course_key = CourseKey.from_string(self.SAMPLE_COURSE) config = VerifiedTrackCohortedCourse.objects.create( - course_key=course_key, enabled=True, verified_cohort_name=COHORT_NAME + course_key=course_key, enabled=True, verified_cohort_name=cohort_name ) config.save() - self.assertEqual(VerifiedTrackCohortedCourse.verified_cohort_name_for_course(course_key), COHORT_NAME) + self.assertEqual(VerifiedTrackCohortedCourse.verified_cohort_name_for_course(course_key), cohort_name) def test_unset_verified_cohort_name(self): fake_course_id = 'fake/course/key' @@ -62,6 +66,7 @@ class TestVerifiedTrackCohortedCourse(TestCase): self.assertEqual(VerifiedTrackCohortedCourse.verified_cohort_name_for_course(course_key), None) +@skip_unless_lms class TestMoveToVerified(SharedModuleStoreTestCase): """ Tests for the post-save listener. """ @@ -82,12 +87,15 @@ class TestMoveToVerified(SharedModuleStoreTestCase): self.addCleanup(celery_task_patcher.stop) def _enable_cohorting(self): + """ Turn on cohorting in the course. """ set_course_cohort_settings(self.course.id, is_cohorted=True) def _create_verified_cohort(self, name=DEFAULT_VERIFIED_COHORT_NAME): + """ Create a verified cohort. """ add_cohort(self.course.id, name, CourseCohort.MANUAL) def _create_named_random_cohort(self, name): + """ Create a random cohort with the supplied name. """ return add_cohort(self.course.id, name, CourseCohort.RANDOM) def _enable_verified_track_cohorting(self, cohort_name=None): @@ -101,6 +109,7 @@ class TestMoveToVerified(SharedModuleStoreTestCase): config.save() def _enroll_in_course(self): + """ Enroll self.user in self.course. """ self.enrollment = CourseEnrollmentFactory(course_id=self.course.id, user=self.user) def _upgrade_to_verified(self): @@ -108,6 +117,7 @@ class TestMoveToVerified(SharedModuleStoreTestCase): self.enrollment.update_enrollment(mode=CourseMode.VERIFIED) def _verify_no_automatic_cohorting(self): + """ Check that upgrading self.user to verified does not move them into a cohort. """ self._enroll_in_course() self.assertIsNone(get_cohort(self.user, self.course.id, assign=False)) self._upgrade_to_verified() @@ -115,13 +125,15 @@ class TestMoveToVerified(SharedModuleStoreTestCase): self.assertEqual(0, self.mocked_celery_task.call_count) def _unenroll(self): + """ Unenroll self.user from self.course. """ self.enrollment.unenroll(self.user, self.course.id) def _reenroll(self): + """ Re-enroll the learner into mode AUDIT. """ self.enrollment.activate() self.enrollment.change_mode(CourseMode.AUDIT) - @mock.patch('verified_track_content.models.log.error') + @mock.patch('openedx.core.djangoapps.verified_track_content.models.log.error') def test_automatic_cohorting_disabled(self, error_logger): """ If the VerifiedTrackCohortedCourse feature is disabled for a course, enrollment mode changes do not move @@ -136,7 +148,7 @@ class TestMoveToVerified(SharedModuleStoreTestCase): # No logging occurs if feature is disabled for course. self.assertFalse(error_logger.called) - @mock.patch('verified_track_content.models.log.error') + @mock.patch('openedx.core.djangoapps.verified_track_content.models.log.error') def test_cohorting_enabled_course_not_cohorted(self, error_logger): """ If the VerifiedTrackCohortedCourse feature is enabled for a course, but the course is not cohorted, @@ -149,7 +161,7 @@ class TestMoveToVerified(SharedModuleStoreTestCase): self.assertTrue(error_logger.called) self.assertIn("course is not cohorted", error_logger.call_args[0][0]) - @mock.patch('verified_track_content.models.log.error') + @mock.patch('openedx.core.djangoapps.verified_track_content.models.log.error') def test_cohorting_enabled_missing_verified_cohort(self, error_logger): """ If the VerifiedTrackCohortedCourse feature is enabled for a course and the course is cohorted, diff --git a/openedx/core/djangoapps/verified_track_content/tests/test_partition_scheme.py b/openedx/core/djangoapps/verified_track_content/tests/test_partition_scheme.py new file mode 100644 index 0000000000..9e11816788 --- /dev/null +++ b/openedx/core/djangoapps/verified_track_content/tests/test_partition_scheme.py @@ -0,0 +1,182 @@ +""" +Tests for verified_track_content/partition_scheme.py. +""" +from datetime import datetime, timedelta +import pytz + +from ..partition_scheme import EnrollmentTrackPartitionScheme, EnrollmentTrackUserPartition, ENROLLMENT_GROUP_IDS +from ..models import VerifiedTrackCohortedCourse +from course_modes.models import CourseMode + +from student.models import CourseEnrollment +from student.tests.factories import UserFactory +from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory +from xmodule.partitions.partitions import UserPartition +from xmodule.partitions.partitions_service import MINIMUM_STATIC_PARTITION_ID + + +class EnrollmentTrackUserPartitionTest(SharedModuleStoreTestCase): + """ + Tests for the custom EnrollmentTrackUserPartition (dynamic groups). + """ + + @classmethod + def setUpClass(cls): + super(EnrollmentTrackUserPartitionTest, cls).setUpClass() + cls.course = CourseFactory.create() + + def test_only_default_mode(self): + partition = create_enrollment_track_partition(self.course) + groups = partition.groups + self.assertEqual(1, len(groups)) + self.assertEqual("Audit", groups[0].name) + + def test_using_verified_track_cohort(self): + VerifiedTrackCohortedCourse.objects.create(course_key=self.course.id, enabled=True).save() + partition = create_enrollment_track_partition(self.course) + self.assertEqual(0, len(partition.groups)) + + def test_multiple_groups(self): + create_mode(self.course, CourseMode.AUDIT, "Audit Enrollment Track", min_price=0) + # Note that the verified mode is expired-- this is intentional. + create_mode( + self.course, CourseMode.VERIFIED, "Verified Enrollment Track", min_price=1, + expiration_datetime=datetime.now(pytz.UTC) + timedelta(days=-1) + ) + # Note that the credit mode is not selectable-- this is intentional. + create_mode(self.course, CourseMode.CREDIT_MODE, "Credit Mode", min_price=2) + + partition = create_enrollment_track_partition(self.course) + groups = partition.groups + self.assertEqual(3, len(groups)) + self.assertIsNotNone(self.get_group_by_name(partition, "Audit Enrollment Track")) + self.assertIsNotNone(self.get_group_by_name(partition, "Verified Enrollment Track")) + self.assertIsNotNone(self.get_group_by_name(partition, "Credit Mode")) + + def test_to_json_not_supported(self): + user_partition = create_enrollment_track_partition(self.course) + with self.assertRaises(TypeError): + user_partition.to_json() + + def test_from_json_not_supported(self): + with self.assertRaises(TypeError): + EnrollmentTrackUserPartition.from_json() + + def test_group_ids(self): + """ + Test that group IDs are all less than MINIMUM_STATIC_PARTITION_ID (to avoid overlapping + with group IDs associated with cohort and random user partitions). + """ + for mode in ENROLLMENT_GROUP_IDS: + self.assertLess(ENROLLMENT_GROUP_IDS[mode], MINIMUM_STATIC_PARTITION_ID) + + @staticmethod + def get_group_by_name(partition, name): + """ + Return the group in the EnrollmentTrackUserPartition with the given name. + If no such group exists, returns `None`. + """ + for group in partition.groups: + if group.name == name: + return group + return None + + +class EnrollmentTrackPartitionSchemeTest(SharedModuleStoreTestCase): + """ + Tests for EnrollmentTrackPartitionScheme. + """ + + @classmethod + def setUpClass(cls): + super(EnrollmentTrackPartitionSchemeTest, cls).setUpClass() + cls.course = CourseFactory.create() + cls.student = UserFactory() + + def test_get_scheme(self): + """ + Ensure that the scheme extension is correctly plugged in (via entry point in setup.py) + """ + self.assertEquals(UserPartition.get_scheme('enrollment_track'), EnrollmentTrackPartitionScheme) + + def test_create_user_partition(self): + user_partition = UserPartition.get_scheme('enrollment_track').create_user_partition( + 301, "partition", "test partition", parameters={"course_id": unicode(self.course.id)} + ) + self.assertEqual(type(user_partition), EnrollmentTrackUserPartition) + self.assertEqual(user_partition.name, "partition") + + groups = user_partition.groups + self.assertEqual(1, len(groups)) + self.assertEqual("Audit", groups[0].name) + + def test_not_enrolled(self): + self.assertIsNone(self._get_user_group()) + + def test_default_enrollment(self): + CourseEnrollment.enroll(self.student, self.course.id) + self.assertEqual("Audit", self._get_user_group().name) + + def test_enrolled_in_nonexistent_mode(self): + CourseEnrollment.enroll(self.student, self.course.id, mode=CourseMode.VERIFIED) + self.assertEqual("Audit", self._get_user_group().name) + + def test_enrolled_in_verified(self): + create_mode(self.course, CourseMode.VERIFIED, "Verified Enrollment Track", min_price=1) + CourseEnrollment.enroll(self.student, self.course.id, mode=CourseMode.VERIFIED) + self.assertEqual("Verified Enrollment Track", self._get_user_group().name) + + def test_enrolled_in_expired(self): + create_mode( + self.course, CourseMode.VERIFIED, "Verified Enrollment Track", + min_price=1, expiration_datetime=datetime.now(pytz.UTC) + timedelta(days=-1) + ) + CourseEnrollment.enroll(self.student, self.course.id, mode=CourseMode.VERIFIED) + self.assertEqual("Verified Enrollment Track", self._get_user_group().name) + + def test_enrolled_in_non_selectable(self): + create_mode(self.course, CourseMode.CREDIT_MODE, "Credit Enrollment Track", min_price=1) + CourseEnrollment.enroll(self.student, self.course.id, mode=CourseMode.CREDIT_MODE) + self.assertEqual("Credit Enrollment Track", self._get_user_group().name) + + def test_using_verified_track_cohort(self): + VerifiedTrackCohortedCourse.objects.create(course_key=self.course.id, enabled=True).save() + CourseEnrollment.enroll(self.student, self.course.id) + self.assertIsNone(self._get_user_group()) + + def _get_user_group(self): + """ + Gets the group the user is assigned to. + """ + user_partition = create_enrollment_track_partition(self.course) + return user_partition.scheme.get_group_for_user(self.course.id, self.student, user_partition) + + +def create_enrollment_track_partition(course): + """ + Create an EnrollmentTrackUserPartition instance for the given course. + """ + enrollment_track_scheme = UserPartition.get_scheme("enrollment_track") + partition = enrollment_track_scheme.create_user_partition( + id=1, + name="TestEnrollment Track Partition", + description="Test partition for segmenting users by enrollment track", + parameters={"course_id": unicode(course.id)} + ) + return partition + + +def create_mode(course, mode_slug, mode_name, min_price=0, expiration_datetime=None): + """ + Create a new course mode for the given course. + """ + return CourseMode.objects.get_or_create( + course_id=course.id, + mode_display_name=mode_name, + mode_slug=mode_slug, + min_price=min_price, + suggested_prices='', + _expiration_datetime=expiration_datetime, + currency='usd' + ) diff --git a/lms/djangoapps/verified_track_content/tests/test_views.py b/openedx/core/djangoapps/verified_track_content/tests/test_views.py similarity index 89% rename from lms/djangoapps/verified_track_content/tests/test_views.py rename to openedx/core/djangoapps/verified_track_content/tests/test_views.py index 6e3017d817..dfc80affbb 100644 --- a/lms/djangoapps/verified_track_content/tests/test_views.py +++ b/openedx/core/djangoapps/verified_track_content/tests/test_views.py @@ -5,22 +5,21 @@ Tests for verified track content views. import json from nose.plugins.attrib import attr -from unittest import skipUnless from django.http import Http404 from django.test.client import RequestFactory -from django.conf import settings +from openedx.core.djangolib.testing.utils import skip_unless_lms from student.tests.factories import UserFactory, AdminFactory from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory -from verified_track_content.models import VerifiedTrackCohortedCourse -from verified_track_content.views import cohorting_settings +from ..models import VerifiedTrackCohortedCourse +from ..views import cohorting_settings @attr(shard=2) -@skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Tests only valid in LMS') +@skip_unless_lms class CohortingSettingsTestCase(SharedModuleStoreTestCase): """ Tests the `cohort_discussion_topics` view. @@ -65,6 +64,7 @@ class CohortingSettingsTestCase(SharedModuleStoreTestCase): self._verify_cohort_settings_response(expected_response) def _verify_cohort_settings_response(self, expected_response): + """ Verify that the response was successful and matches the expected JSON payload. """ request = RequestFactory().get("dummy_url") request.user = AdminFactory() response = cohorting_settings(request, unicode(self.course.id)) diff --git a/lms/djangoapps/verified_track_content/views.py b/openedx/core/djangoapps/verified_track_content/views.py similarity index 88% rename from lms/djangoapps/verified_track_content/views.py rename to openedx/core/djangoapps/verified_track_content/views.py index e5d776323b..4067a87a34 100644 --- a/lms/djangoapps/verified_track_content/views.py +++ b/openedx/core/djangoapps/verified_track_content/views.py @@ -6,9 +6,9 @@ from util.json_request import expect_json, JsonResponse from django.contrib.auth.decorators import login_required from opaque_keys.edx.keys import CourseKey -from courseware.courses import get_course_with_access +from lms.djangoapps.courseware.courses import get_course_with_access -from verified_track_content.models import VerifiedTrackCohortedCourse +from openedx.core.djangoapps.verified_track_content.models import VerifiedTrackCohortedCourse @expect_json diff --git a/setup.py b/setup.py index 08f2637360..3d6713cef1 100644 --- a/setup.py +++ b/setup.py @@ -42,6 +42,7 @@ setup( "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", + "enrollment_track = openedx.core.djangoapps.verified_track_content.partition_scheme:EnrollmentTrackPartitionScheme", ], "openedx.block_structure_transformer": [ "library_content = lms.djangoapps.course_blocks.transformers.library_content:ContentLibraryTransformer",