feat: Enrollment Tracks OutlineProcessor (MST-685)

NOTE: This will require a forced backfill of course outlines to update
the course content data in learning_sequences:

  python manage.py cms backfill_course_outlines --force

Without this backfill, the learning_sequences API will continue to serve
stale content data that has no user partition group data. It won't cause
errors, but it won't do the exclusions properly.

Commit summary:

* Created EnrollmentTrackPartitionGroupsOutlineProcessor to process the
  enrollment_track User Partition Group, allowing Sequences and Sections
  to be removed based on their group_access settings.
* Added user_partition_groups attribute to CourseLearningSequenceData
  and CourseSectionData in learning_sequences/data.py, along with
  backing model data.
* get_outline_from_modulestore now extracts group_access settings from
  Sections and Sequences. It also bubbles up group_access settings from
  Units, meaning that if a Sequence with no group_access setting has
  Units that are all set to show only to the Verified enrollment track,
  then the Sequence will only show to the Verified enrollment track.

This commit adds model-level support for all user partition groups by
capturing all the content group associations (group_access), but it only
implements the code checks for the enrollment track partition. It's not
clear that we want to generalize, since there's only one other partition
type (A/B testing) that is applicable at the outline level.

It's important to note that there is no way to set the group_access for
a Section or Sequence in Studio today. It's only possible by direct
editing of the OLX for import. That being said, the block structures
framework supports applying course groups at this level, and this commit
moves learning_sequences closer to feature parity.

The bubbling up from Units to the parent Sequence was done to mitigate
confusion when a Sequence is entirely composed of Units that are not
visible to the user because of content group restrictions. It's not
clear whether this is something we want to do in the long term, since it
would simplify the code to always specify group_access at the Sequence
level. This first pass is done partially to collect better data about
places in our courses where this kind of usage is already happening.

Most of the EnrollmentTrackPartitionGroupsOutlineProcessor code and its
tests were written by @schenedx.
This commit is contained in:
David Ormsbee
2021-04-14 12:26:25 -04:00
parent 3ce04b7983
commit dfb80acc11
10 changed files with 1015 additions and 39 deletions

View File

@@ -79,6 +79,114 @@ def _error_for_not_sequence(section, not_sequence):
)
def _bubbled_up_groups_from_units(group_access_from_units):
"""
Return {user_partition_id: [group_ids]} to bubble up from Units to Sequence.
This is to handle a special case: If *all* of the Units in a sequence have
the exact same group for a given user partition, bubble that value up to the
Sequence as a whole. For example, say that every Unit in a Sequence has a
group_access that looks like: { ENROLLMENT: [MASTERS] } (where both
constants are ints). In this case, an Audit user has nothing to see in the
Sequence at all, and it's not useful to give them an empty shell. So we'll
act as if the Sequence as a whole had that group setting. Note that there is
currently no way to set the group_access setting at the sequence level in
Studio, so course teams can only manipulate it for individual Units.
"""
# If there are no Units, there's nothing to bubble up.
if not group_access_from_units:
return {}
def _normalize_group_access_dict(group_access):
return {
user_partition_id: sorted(group_ids) # sorted for easier comparison
for user_partition_id, group_ids in group_access.items()
}
normalized_group_access_dicts = [
_normalize_group_access_dict(group_access) for group_access in group_access_from_units
]
first_unit_group_access = normalized_group_access_dicts[0]
rest_of_seq_group_access_list = normalized_group_access_dicts[1:]
# If there's only a single Unit, bubble up its group_access.
if not rest_of_seq_group_access_list:
return first_unit_group_access
# Otherwise, go through the user partitions and groups in our first unit
# and compare them to all the other group_access dicts from the units in the
# rest of the sequence. Only keep the ones that match exactly and do not
# have empty groups.
common_group_access = {
user_partition_id: group_ids
for user_partition_id, group_ids in first_unit_group_access.items()
if group_ids and all(
group_ids == group_access.get(user_partition_id)
for group_access in rest_of_seq_group_access_list
)
}
return common_group_access
def _make_user_partition_groups(usage_key, group_access):
"""
Return a (Dict, Optional[ContentErrorData]) of user partition groups.
The Dict is a mapping of user partition ID to list of group IDs. If any
empty groups are encountered, we create a ContentErrorData about that. If
there are no empty groups, we pass back (Dict, None).
"""
empty_partitions = sorted(
part_id for part_id, group_ids in group_access.items() if not group_ids
)
empty_partitions_txt = ", ".join([str(part_id) for part_id in empty_partitions])
if empty_partitions:
error = ContentErrorData(
message=(
f'<{usage_key.block_type}> with url_name="{usage_key.block_id}"'
f' has the following empty group_access user partitions: '
f'{empty_partitions_txt}. This would make this content '
f'unavailable to anyone except course staff. Ignoring these '
f'group_access settings when building outline.'
),
usage_key=_remove_version_info(usage_key),
)
else:
error = None
user_partition_groups = {
part_id: group_ids for part_id, group_ids in group_access.items() if group_ids
}
return user_partition_groups, error
def _make_bubbled_up_error(seq_usage_key, user_partition_id, group_ids):
return ContentErrorData(
message=(
f'<{seq_usage_key.block_type}> with url_name="{seq_usage_key.block_id}"'
f' was assigned group_ids {group_ids} for user_partition_id '
f'{user_partition_id} because all of its child Units had that '
f'group_access setting. This is permitted, but is an unusual usage '
f'that may cause unexpected behavior while browsing the course.'
),
usage_key=_remove_version_info(seq_usage_key),
)
def _make_not_bubbled_up_error(seq_usage_key, seq_group_access, user_partition_id, group_ids):
return ContentErrorData(
message=(
f'<{seq_usage_key.block_type}> with url_name="{seq_usage_key.block_id}" '
f'has children with only group_ids {group_ids} for user_partition_id '
f'{user_partition_id}, but its own group_access setting is '
f'{seq_group_access}, which takes precedence. This is permitted, '
f'but probably not intended, since it means that the content is '
f'effectively unusable by anyone except staff.'
),
usage_key=_remove_version_info(seq_usage_key),
)
def _make_section_data(section):
"""
Return a (CourseSectionData, List[ContentDataError]) from a SectionBlock.
@@ -105,6 +213,13 @@ def _make_section_data(section):
section_errors.append(_error_for_not_section(section))
return (None, section_errors)
section_user_partition_groups, error = _make_user_partition_groups(
section.location, section.group_access
)
# Invalid user partition errors aren't fatal. Just log and continue on.
if error:
section_errors.append(error)
# We haven't officially killed off problemset and videosequence yet, so
# treat them as equivalent to sequential for now.
valid_sequence_tags = ['sequential', 'problemset', 'videosequence']
@@ -115,6 +230,33 @@ def _make_section_data(section):
section_errors.append(_error_for_not_sequence(section, sequence))
continue
seq_user_partition_groups, error = _make_user_partition_groups(
sequence.location, sequence.group_access
)
if error:
section_errors.append(error)
# Bubble up User Partition Group settings from Units if appropriate.
sequence_upg_from_units = _bubbled_up_groups_from_units(
[unit.group_access for unit in sequence.get_children()]
)
for user_partition_id, group_ids in sequence_upg_from_units.items():
# If there's an existing user partition ID set at the sequence
# level, we respect it, even if it seems nonsensical. The hack of
# bubbling things up from the Unit level is only done if there's
# no conflicting value set at the Sequence level.
if user_partition_id not in seq_user_partition_groups:
section_errors.append(
_make_bubbled_up_error(sequence.location, user_partition_id, group_ids)
)
seq_user_partition_groups[user_partition_id] = group_ids
else:
section_errors.append(
_make_not_bubbled_up_error(
sequence.location, sequence.group_access, user_partition_id, group_ids
)
)
sequences_data.append(
CourseLearningSequenceData(
usage_key=_remove_version_info(sequence.location),
@@ -129,6 +271,7 @@ def _make_section_data(section):
hide_from_toc=sequence.hide_from_toc,
visible_to_staff_only=sequence.visible_to_staff_only,
),
user_partition_groups=seq_user_partition_groups,
)
)
@@ -140,6 +283,7 @@ def _make_section_data(section):
hide_from_toc=section.hide_from_toc,
visible_to_staff_only=section.visible_to_staff_only,
),
user_partition_groups=section_user_partition_groups,
)
return section_data, section_errors
@@ -158,7 +302,8 @@ def get_outline_from_modulestore(course_key) -> Tuple[CourseOutlineData, List[Co
content_errors = []
with store.branch_setting(ModuleStoreEnum.Branch.published_only, course_key):
course = store.get_course(course_key, depth=2)
# Pull course with depth=3 so we prefetch Section -> Sequence -> Unit
course = store.get_course(course_key, depth=3)
sections_data = []
for section in course.get_children():
section_data, section_errors = _make_section_data(section)

View File

@@ -273,6 +273,152 @@ class OutlineFromModuleStoreTestCase(ModuleStoreTestCase):
assert outline.sections[0].title == section.url_name
assert outline.sections[0].sequences[0].title == sequence.url_name
def test_empty_user_partition_groups(self):
"""
Ignore user partition setting if no groups are associated.
If we didn't ignore it, we would be creating content that can never be
seen by any student.
"""
with self.store.bulk_operations(self.course_key):
section = ItemFactory.create(
parent_location=self.draft_course.location,
category='chapter',
display_name='Ch 1',
group_access={
49: [],
50: [1, 2],
51: [],
}
)
ItemFactory.create(
parent_location=section.location,
category='sequential',
display_name='Seq 1',
group_access={
49: [],
}
)
outline, errs = get_outline_from_modulestore(self.course_key)
assert len(outline.sections) == 1
assert len(outline.sequences) == 1
assert outline.sections[0].user_partition_groups == {50: [1, 2]}
assert outline.sections[0].sequences[0].user_partition_groups == {}
assert len(errs) == 2
def test_bubbled_up_user_partition_groups_no_children(self):
"""Testing empty case to make sure bubble-up code doesn't break."""
with self.store.bulk_operations(self.course_key):
section = ItemFactory.create(
parent_location=self.draft_course.location,
category='chapter',
display_name='Ch 0',
)
# Bubble up with no children (nothing happens)
ItemFactory.create(
parent_location=section.location,
category='sequential',
display_name='Seq 0',
group_access={}
)
outline, _errs = get_outline_from_modulestore(self.course_key)
seq_data = outline.sections[0].sequences[0]
assert seq_data.user_partition_groups == {}
def test_bubbled_up_user_partition_groups_one_child(self):
"""Group settings should bubble up from Unit to Seq. if only one unit"""
with self.store.bulk_operations(self.course_key):
section = ItemFactory.create(
parent_location=self.draft_course.location,
category='chapter',
display_name='Ch 0',
)
# Bubble up with 1 child (grabs the setting from child)
seq_1 = ItemFactory.create(
parent_location=section.location,
category='sequential',
display_name='Seq 1',
group_access={}
)
ItemFactory.create(
parent_location=seq_1.location,
category='vertical',
display_name='Single Vertical',
group_access={50: [1, 2]},
)
outline, errs = get_outline_from_modulestore(self.course_key)
seq_data = outline.sections[0].sequences[0]
assert seq_data.user_partition_groups == {50: [1, 2]}
assert len(errs) == 1
def test_bubbled_up_user_partition_groups_multiple_children(self):
"""If all Units have the same group_access, bubble up to Sequence."""
with self.store.bulk_operations(self.course_key):
section = ItemFactory.create(
parent_location=self.draft_course.location,
category='chapter',
display_name='Ch 0',
)
# Bubble up with n children, all matching for one group
seq_n = ItemFactory.create(
parent_location=section.location,
category='sequential',
display_name='Seq N',
group_access={}
)
for i in range(4):
ItemFactory.create(
parent_location=seq_n.location,
category='vertical',
display_name=f'vertical {i}',
group_access={50: [3, 4], 51: [i]} # Only 50 should get bubbled up
)
ItemFactory.create(
parent_location=seq_n.location,
category='vertical',
display_name='vertical 5',
group_access={50: [4, 3], 51: [5]} # Ordering should be normalized
)
outline, errs = get_outline_from_modulestore(self.course_key)
seq_data = outline.sections[0].sequences[0]
assert seq_data.user_partition_groups == {50: [3, 4]}
assert len(errs) == 1
def test_not_bubbled_up(self):
"""Don't bubble up from Unit if Seq has a conflicting group_access."""
with self.store.bulk_operations(self.course_key):
section = ItemFactory.create(
parent_location=self.draft_course.location,
category='chapter',
display_name='Ch 0',
)
# Bubble up with 1 child (grabs the setting from child)
seq_1 = ItemFactory.create(
parent_location=section.location,
category='sequential',
display_name='Seq 1',
group_access={50: [3, 4]}
)
ItemFactory.create(
parent_location=seq_1.location,
category='vertical',
display_name='Single Vertical',
group_access={50: [1, 2]},
)
outline, errs = get_outline_from_modulestore(self.course_key)
seq_data = outline.sections[0].sequences[0]
assert seq_data.user_partition_groups == {50: [3, 4]} # Kept the seq settings
assert len(errs) == 1
def _outline_seq_data(self, modulestore_seq):
"""
(CourseLearningSequenceData, UsageKey) for a Modulestore sequence.