Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions cms/djangoapps/contentstore/course_group_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,11 @@
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"
Expand Down Expand Up @@ -84,7 +85,7 @@ def assign_group_ids(self):
"""
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:
Expand All @@ -96,7 +97,7 @@ def get_used_ids(course):
"""
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):
"""
Expand Down
8 changes: 4 additions & 4 deletions cms/djangoapps/contentstore/tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -493,12 +493,12 @@ def test_retrieves_partition_info_with_selected_groups(self):
]
}
]
self.assertEqual(self._get_partition_info(), expected)
self.assertEqual(self._get_partition_info(schemes=["cohort", "random"]), expected)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note to reviewers: exclude the enrollment_track partition.


# 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
Expand Down Expand Up @@ -546,7 +546,7 @@ def test_exclude_inactive_partitions(self):
])

# 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")

Expand All @@ -572,7 +572,7 @@ def test_exclude_partitions_with_no_groups(self):
])

# 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")

Expand Down
7 changes: 4 additions & 3 deletions cms/djangoapps/contentstore/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

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
Expand Down Expand Up @@ -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 = []
Expand Down Expand Up @@ -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

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note to reviewers: I had to add this because the EnrollmentTrackUserPartition uses ugettext_lazy (due to the fact that it lives in common.lib.xmodule.xmodule). When CMS unit tests ran, p.name was a function instead of a string, and JSON encoding failed.

"scheme": p.scheme.name,
"groups": groups,
})
Expand Down
15 changes: 15 additions & 0 deletions cms/djangoapps/contentstore/views/preview.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
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
Expand Down Expand Up @@ -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
Expand Down
101 changes: 67 additions & 34 deletions cms/djangoapps/contentstore/views/tests/test_item.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
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
Expand Down Expand Up @@ -44,6 +45,7 @@
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):
Expand Down Expand Up @@ -341,15 +343,17 @@ def test_invalid_paging(self, page_number, page_size):
)

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.
],
),
]
Expand All @@ -365,18 +369,31 @@ def test_get_user_partitions_and_groups(self):
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,
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -1760,22 +1792,22 @@ def test_create_groups(self):
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))
vertical_0 = self.get_item_from_modulestore(split_test.children[0], verify_is_draft=True)
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):
"""
Expand All @@ -1785,8 +1817,8 @@ def test_split_xblock_info_group_name(self):
# 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)

Expand All @@ -1808,13 +1840,13 @@ def test_change_user_partition_id(self):
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])
Expand All @@ -1825,9 +1857,9 @@ def test_change_user_partition_id(self):

# 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)

Expand All @@ -1836,12 +1868,12 @@ def test_change_same_user_partition_id(self):
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)

Expand All @@ -1852,7 +1884,7 @@ def test_change_non_existent_user_partition_id(self):
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

Expand All @@ -1869,13 +1901,14 @@ def test_add_groups(self):
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)
Expand All @@ -1896,7 +1929,7 @@ def test_add_groups(self):
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)
Expand Down
2 changes: 2 additions & 0 deletions cms/envs/bok_choy.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,8 @@
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

@cahrens cahrens Mar 27, 2017

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note to reviewers: feature flag was introduced because the next story will add the ability to set component visibility based on this new partition. @sstack22 doesn't want course authors seeing this functionality before the other parts (including the ability to segment discussions) are present.

Hopefully this feature flag will be deleted once everything is complete, as our plan is to make the feature invisible for courses where only a single enrollment track exists.

I have enabled the feature flag for all unit and bok choy tests to ensure we are testing in those environments with the EnrollmentTrackUserPartition present (to catch any potential issues earlier).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the way, in the future, consider using a waffle switch in these cases. This way, you wouldn't need to be blocked on a release in order to enable/disable the feature. I've been using it for several features now, and created helper waffle utility classes: https://github.com/edx/edx-platform/blob/master/openedx/core/djangolib/waffle_utils.py

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the pointer-- definitely next time!


# Enable partner support link in Studio footer
PARTNER_SUPPORT_EMAIL = 'partner-support@example.com'

Expand Down
8 changes: 8 additions & 0 deletions cms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,8 @@
# 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
Expand Down Expand Up @@ -225,6 +227,9 @@

# 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
Expand Down Expand Up @@ -883,6 +888,9 @@
# 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',

Expand Down
Loading