Skip to content
Closed
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
2 changes: 1 addition & 1 deletion cms/djangoapps/contentstore/course_group_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,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 self.course.user_partitions 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 Down
2 changes: 1 addition & 1 deletion cms/djangoapps/contentstore/courseware_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ def prepare_item_index(item, skip_index=False, groups_usage_info=None):
split_partition = item.get_selected_partition()
for split_test_child in item.get_children():
if split_partition:
for group in split_partition.groups:
for group in split_partition.groups():
group_id = unicode(group.id)
child_location = item.group_id_to_child.get(group_id, None)
if child_location == split_test_child.location:
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
10 changes: 8 additions & 2 deletions cms/djangoapps/contentstore/tests/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -299,8 +299,14 @@ def assertCoursesEqual(self, course1_id, course2_id):
store = self.store._get_modulestore_for_courselike(course2_id) # pylint: disable=protected-access
new_name = 'course' if isinstance(store, SplitMongoModuleStore) else course2_item_loc.run
course2_item_loc = course2_item_loc.replace(name=new_name)
course2_item = self.store.get_item(course2_item_loc)

course2_item = self.store.get_item(course2_item_loc)
# The EnrollmentTrackuserPartition stores the course_id, which will vary split vs. old mongo.
# Also at least one test calls this comparison method with a purposefully different course.
# Clear stored course_id so that own_metadata call below does not fail.
course1_item.user_partitions[0].parameters['course_id'] = None
course2_item.user_partitions[0].parameters['course_id'] = None
else:
course2_item = self.store.get_item(course2_item_loc)
# compare published state
self.assertEqual(
self.store.has_published_version(course1_item),
Expand Down
43 changes: 18 additions & 25 deletions cms/djangoapps/contentstore/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ def delete_course_and_groups(course_key, user_id):
def get_lms_link_for_item(location, preview=False):
"""
Returns an LMS link to the course with a jump_to to the provided location.

:param location: the location to jump to
:param preview: True if the preview version of LMS should be returned. Default value is false.
"""
Expand Down Expand Up @@ -296,32 +295,26 @@ def get_group_display_name(user_partitions, xblock_display_name):
"""
for user_partition in user_partitions:
for group in user_partition['groups']:
if str(group['id']) in xblock_display_name:
# Temporary hack until TNL-6731 is addressed.

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: this issue will be fixed before this PR merges.

if group['id'] > 99 and str(group['id']) in xblock_display_name:
return group['name']


def get_user_partition_info(xblock, schemes=None, course=None):
"""
Retrieve user partition information for an XBlock for display in editors.

* If a partition has been disabled, it will be excluded from the results.

* If a group within a partition is referenced by the XBlock, but the group has been deleted,
the group will be marked as deleted in the results.

Arguments:
xblock (XBlock): The courseware component being edited.

Keyword Arguments:
schemes (iterable of str): If provided, filter partitions to include only
schemes with the provided names.

course (XBlock): The course descriptor. If provided, uses this to look up the user partitions
instead of loading the course. This is useful if we're calling this function multiple
times for the same course want to minimize queries to the modulestore.

Returns: list

Example Usage:
>>> get_user_partition_info(block, schemes=["cohort", "verification"])
[
Expand Down Expand Up @@ -358,7 +351,6 @@ def get_user_partition_info(xblock, schemes=None, course=None):
]
}
]

"""
course = course or modulestore().get_course(xblock.location.course_key)

Expand All @@ -377,11 +369,11 @@ def get_user_partition_info(xblock, schemes=None, course=None):

# 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.active and p.groups() and (schemes is None or p.scheme.name in schemes):

# First, add groups defined by the partition
groups = []
for g in p.groups:
for g in p.groups():

# Falsey group access for a partition mean that all groups
# are selected. In the UI, though, we don't show the particular
Expand All @@ -395,7 +387,7 @@ def get_user_partition_info(xblock, schemes=None, course=None):
})

# Next, add any groups set on the XBlock that have been deleted
all_groups = set(g.id for g in p.groups)
all_groups = set(g.id for g in p.groups())
missing_group_ids = selected_groups - all_groups
for gid in missing_group_ids:
groups.append({
Expand All @@ -419,38 +411,39 @@ def get_user_partition_info(xblock, schemes=None, course=None):
def get_visibility_partition_info(xblock):
"""
Retrieve user partition information for the component visibility editor.

This pre-processes partition information to simplify the template.

Arguments:
xblock (XBlock): The component being edited.

Returns: dict

"""
user_partitions = get_user_partition_info(xblock, schemes=["verification", "cohort"])
user_partitions = get_user_partition_info(xblock, schemes=["verification", "enrollment_track", "cohort"])
cohort_partitions = []
enrollment_mode_partitions = []
verification_partitions = []
has_selected_groups = False
has_selected_content_groups = False
has_selected_enrollment_modes = False
selected_verified_partition_id = None

# Pre-process the partitions to make it easier to display the UI
for p in user_partitions:
has_selected = any(g["selected"] for g in p["groups"])
has_selected_groups = has_selected_groups or has_selected

if p["scheme"] == "cohort":
cohort_partitions.append(p)
has_selected_content_groups = any(g["selected"] for g in p["groups"])
elif p["scheme"] == "verification":
verification_partitions.append(p)
if has_selected:
selected_verified_partition_id = p["id"]
# if has_selected:
# selected_verified_partition_id = p["id"]
elif p["scheme"] == "enrollment_track":
enrollment_mode_partitions.append(p)
has_selected_enrollment_modes = any(g["selected"] for g in p["groups"])

return {
"user_partitions": user_partitions,
"cohort_partitions": cohort_partitions,
"enrollment_mode_partitions": enrollment_mode_partitions,
"verification_partitions": verification_partitions,
"has_selected_groups": has_selected_groups,
"has_selected_content_groups": has_selected_content_groups,
"has_selected_enrollment_modes": has_selected_enrollment_modes,
"selected_verified_partition_id": selected_verified_partition_id,
}

Expand Down
6 changes: 3 additions & 3 deletions cms/djangoapps/contentstore/views/course.py
Original file line number Diff line number Diff line change
Expand Up @@ -1482,10 +1482,10 @@ def remove_content_or_experiment_group(request, store, course, configuration, gr
status=400
)

matching_groups = [group for group in configuration.groups if group.id == group_id]
matching_groups = [group for group in configuration.groups() if group.id == group_id]
if matching_groups:
group_index = configuration.groups.index(matching_groups[0])
configuration.groups.pop(group_index)
group_index = configuration.groups().index(matching_groups[0])
configuration.groups().pop(group_index)
else:
return JsonResponse(status=404)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,10 @@ def _add_user_partitions(self, count=1, scheme_id="random"):
self.course.user_partitions = partitions
self.save_course()

def _get_non_enrollment_track_partitions(self):
""" Return the user_partitions that are not of type "enrollment_track". """
return [p for p in self.course.user_partitions if p.scheme.name != "enrollment_track"]


# pylint: disable=no-member
class GroupConfigurationsBaseTestCase(object):
Expand All @@ -146,7 +150,7 @@ def _remove_ids(self, content):
Returns a tuple that contains removed group configuration ID and group IDs.
"""
configuration_id = content.pop("id")
group_ids = [group.pop("id") for group in content["groups"]]
group_ids = [group.pop("id") for group in content['groups']]

return (configuration_id, group_ids)

Expand Down Expand Up @@ -282,23 +286,23 @@ def test_can_create_group_configuration(self):
self.assertEqual(len(group_ids), 2)
self.reload_course()
# Verify that user_partitions in the course contains the new group configuration.
user_partititons = self.course.user_partitions
self.assertEqual(len(user_partititons), 1)
self.assertEqual(user_partititons[0].name, u'Test name')
self.assertEqual(len(user_partititons[0].groups), 2)
self.assertEqual(user_partititons[0].groups[0].name, u'Group A')
self.assertEqual(user_partititons[0].groups[1].name, u'Group B')
self.assertEqual(user_partititons[0].parameters, {})
user_partitions = self._get_non_enrollment_track_partitions()
self.assertEqual(len(user_partitions), 1)
self.assertEqual(user_partitions[0].name, u'Test name')
self.assertEqual(len(user_partitions[0].groups()), 2)
self.assertEqual(user_partitions[0].groups()[0].name, u'Group A')
self.assertEqual(user_partitions[0].groups()[1].name, u'Group B')
self.assertEqual(user_partitions[0].parameters, {})

def test_lazily_creates_cohort_configuration(self):
"""
Test that a cohort schemed user partition is NOT created by
default for the user.
"""
self.assertEqual(len(self.course.user_partitions), 0)
self.assertEqual(len(self._get_non_enrollment_track_partitions()), 0)
self.client.get(self._url())
self.reload_course()
self.assertEqual(len(self.course.user_partitions), 0)
self.assertEqual(len(self._get_non_enrollment_track_partitions()), 0)


class GroupConfigurationsDetailHandlerTestCase(CourseTestCase, GroupConfigurationsBaseTestCase, HelperMethods):
Expand Down Expand Up @@ -348,12 +352,12 @@ def test_can_create_new_content_group_if_it_does_not_exist(self):
self.assertEqual(content, expected)
self.reload_course()
# Verify that user_partitions in the course contains the new group configuration.
user_partitions = self.course.user_partitions
user_partitions = self._get_non_enrollment_track_partitions()
self.assertEqual(len(user_partitions), 1)
self.assertEqual(user_partitions[0].name, u'Test name')
self.assertEqual(len(user_partitions[0].groups), 2)
self.assertEqual(user_partitions[0].groups[0].name, u'Group A')
self.assertEqual(user_partitions[0].groups[1].name, u'Group B')
self.assertEqual(len(user_partitions[0].groups()), 2)
self.assertEqual(user_partitions[0].groups()[0].name, u'Group A')
self.assertEqual(user_partitions[0].groups()[1].name, u'Group B')
self.assertEqual(user_partitions[0].parameters, {})

def test_can_edit_content_group(self):
Expand Down Expand Up @@ -389,13 +393,13 @@ def test_can_edit_content_group(self):
self.reload_course()

# Verify that user_partitions is properly updated in the course.
user_partititons = self.course.user_partitions
user_partititons = self._get_non_enrollment_track_partitions()

self.assertEqual(len(user_partititons), 1)
self.assertEqual(user_partititons[0].name, u'New Test name')
self.assertEqual(len(user_partititons[0].groups), 2)
self.assertEqual(user_partititons[0].groups[0].name, u'New Group Name')
self.assertEqual(user_partititons[0].groups[1].name, u'Group C')
self.assertEqual(len(user_partititons[0].groups()), 2)
self.assertEqual(user_partititons[0].groups()[0].name, u'New Group Name')
self.assertEqual(user_partititons[0].groups()[1].name, u'Group C')
self.assertEqual(user_partititons[0].parameters, {})

def test_can_delete_content_group(self):
Expand All @@ -415,11 +419,11 @@ def test_can_delete_content_group(self):
self.assertEqual(response.status_code, 204)
self.reload_course()
# Verify that group and partition is properly updated in the course.
user_partititons = self.course.user_partitions
self.assertEqual(len(user_partititons), 1)
self.assertEqual(user_partititons[0].name, 'Name 0')
self.assertEqual(len(user_partititons[0].groups), 2)
self.assertEqual(user_partititons[0].groups[1].name, 'Group C')
user_partitions = self._get_non_enrollment_track_partitions()
self.assertEqual(len(user_partitions), 1)
self.assertEqual(user_partitions[0].name, 'Name 0')
self.assertEqual(len(user_partitions[0].groups()), 2)
self.assertEqual(user_partitions[0].groups()[1].name, 'Group C')

def test_cannot_delete_used_content_group(self):
"""
Expand All @@ -440,10 +444,10 @@ def test_cannot_delete_used_content_group(self):
self.assertTrue(content['error'])
self.reload_course()
# Verify that user_partitions and groups are still the same.
user_partititons = self.course.user_partitions
user_partititons = self._get_non_enrollment_track_partitions()
self.assertEqual(len(user_partititons), 1)
self.assertEqual(len(user_partititons[0].groups), 3)
self.assertEqual(user_partititons[0].groups[1].name, 'Group B')
self.assertEqual(len(user_partititons[0].groups()), 3)
self.assertEqual(user_partititons[0].groups()[1].name, 'Group B')

def test_cannot_delete_non_existent_content_group(self):
"""
Expand All @@ -461,7 +465,7 @@ def test_cannot_delete_non_existent_content_group(self):
# Verify that user_partitions is still the same.
user_partititons = self.course.user_partitions
self.assertEqual(len(user_partititons), 1)
self.assertEqual(len(user_partititons[0].groups), 3)
self.assertEqual(len(user_partititons[0].groups()), 3)

def test_can_create_new_group_configuration_if_it_does_not_exist(self):
"""
Expand Down Expand Up @@ -493,12 +497,12 @@ def test_can_create_new_group_configuration_if_it_does_not_exist(self):
self.assertEqual(content, expected)
self.reload_course()
# Verify that user_partitions in the course contains the new group configuration.
user_partitions = self.course.user_partitions
user_partitions = self._get_non_enrollment_track_partitions()
self.assertEqual(len(user_partitions), 1)
self.assertEqual(user_partitions[0].name, u'Test name')
self.assertEqual(len(user_partitions[0].groups), 2)
self.assertEqual(user_partitions[0].groups[0].name, u'Group A')
self.assertEqual(user_partitions[0].groups[1].name, u'Group B')
self.assertEqual(len(user_partitions[0].groups()), 2)
self.assertEqual(user_partitions[0].groups()[0].name, u'Group A')
self.assertEqual(user_partitions[0].groups()[1].name, u'Group B')
self.assertEqual(user_partitions[0].parameters, {})

def test_can_edit_group_configuration(self):
Expand Down Expand Up @@ -535,13 +539,13 @@ def test_can_edit_group_configuration(self):
self.reload_course()

# Verify that user_partitions is properly updated in the course.
user_partititons = self.course.user_partitions
user_partititons = self._get_non_enrollment_track_partitions()

self.assertEqual(len(user_partititons), 1)
self.assertEqual(user_partititons[0].name, u'New Test name')
self.assertEqual(len(user_partititons[0].groups), 2)
self.assertEqual(user_partititons[0].groups[0].name, u'New Group Name')
self.assertEqual(user_partititons[0].groups[1].name, u'Group C')
self.assertEqual(len(user_partititons[0].groups()), 2)
self.assertEqual(user_partititons[0].groups()[0].name, u'New Group Name')
self.assertEqual(user_partititons[0].groups()[1].name, u'Group C')
self.assertEqual(user_partititons[0].parameters, {})

def test_can_delete_group_configuration(self):
Expand All @@ -560,7 +564,7 @@ def test_can_delete_group_configuration(self):
self.assertEqual(response.status_code, 204)
self.reload_course()
# Verify that user_partitions is properly updated in the course.
user_partititons = self.course.user_partitions
user_partititons = self._get_non_enrollment_track_partitions()
self.assertEqual(len(user_partititons), 1)
self.assertEqual(user_partititons[0].name, 'Name 1')

Expand All @@ -582,7 +586,7 @@ def test_cannot_delete_used_group_configuration(self):
self.assertTrue(content['error'])
self.reload_course()
# Verify that user_partitions is still the same.
user_partititons = self.course.user_partitions
user_partititons = self._get_non_enrollment_track_partitions()
self.assertEqual(len(user_partititons), 2)
self.assertEqual(user_partititons[0].name, 'Name 0')

Expand Down
Loading