From 32bb24b1782472500ccbf3b9ffd7d881cef240d3 Mon Sep 17 00:00:00 2001 From: Moncef Abboud Date: Wed, 6 Sep 2023 16:15:55 +0200 Subject: [PATCH] fix: hide sequences & sections when access is restricted in units through cohorts --- .../course_home_api/outline/views.py | 13 +- .../learning_sequences/api/outlines.py | 2 + .../api/processors/cohort_partition_groups.py | 88 +++++++++ .../api/tests/test_outlines.py | 186 ++++++++++++++++++ .../content/learning_sequences/data.py | 10 +- .../core/djangoapps/course_groups/cohorts.py | 14 ++ 6 files changed, 310 insertions(+), 3 deletions(-) create mode 100644 openedx/core/djangoapps/content/learning_sequences/api/processors/cohort_partition_groups.py diff --git a/lms/djangoapps/course_home_api/outline/views.py b/lms/djangoapps/course_home_api/outline/views.py index 7650c2ae5320..813bde793f88 100644 --- a/lms/djangoapps/course_home_api/outline/views.py +++ b/lms/djangoapps/course_home_api/outline/views.py @@ -303,9 +303,20 @@ def get(self, request, *args, **kwargs): # pylint: disable=too-many-statements ) available_seq_ids = {str(usage_key) for usage_key in user_course_outline.sequences} + available_section_ids = {str(section.usage_key) for section in user_course_outline.sections} + + # course_blocks is a reference to the root of the course, + # so we go through the chapters (sections) and keep only those + # which are part of the outline. + course_blocks['children'] = [ + chapter_data + for chapter_data in course_blocks.get('children', []) + if chapter_data['id'] in available_section_ids + ] + # course_blocks is a reference to the root of the course, so we go # through the chapters (sections) to look for sequences to remove. - for chapter_data in course_blocks.get('children', []): + for chapter_data in course_blocks['children']: chapter_data['children'] = [ seq_data for seq_data in chapter_data['children'] diff --git a/openedx/core/djangoapps/content/learning_sequences/api/outlines.py b/openedx/core/djangoapps/content/learning_sequences/api/outlines.py index 800b085e5000..9af14d6140ef 100644 --- a/openedx/core/djangoapps/content/learning_sequences/api/outlines.py +++ b/openedx/core/djangoapps/content/learning_sequences/api/outlines.py @@ -40,6 +40,7 @@ UserPartitionGroup ) from .permissions import can_see_all_content +from .processors.cohort_partition_groups import CohortPartitionGroupsOutlineProcessor from .processors.content_gating import ContentGatingOutlineProcessor from .processors.enrollment import EnrollmentOutlineProcessor from .processors.enrollment_track_partition_groups import EnrollmentTrackPartitionGroupsOutlineProcessor @@ -328,6 +329,7 @@ def _get_user_course_outline_and_processors(course_key: CourseKey, # lint-amnes ('visibility', VisibilityOutlineProcessor), ('enrollment', EnrollmentOutlineProcessor), ('enrollment_track_partitions', EnrollmentTrackPartitionGroupsOutlineProcessor), + ('cohorts_partitions', CohortPartitionGroupsOutlineProcessor), ] # Run each OutlineProcessor in order to figure out what items we have to diff --git a/openedx/core/djangoapps/content/learning_sequences/api/processors/cohort_partition_groups.py b/openedx/core/djangoapps/content/learning_sequences/api/processors/cohort_partition_groups.py new file mode 100644 index 000000000000..fa921902c833 --- /dev/null +++ b/openedx/core/djangoapps/content/learning_sequences/api/processors/cohort_partition_groups.py @@ -0,0 +1,88 @@ +# lint-amnesty, pylint: disable=missing-module-docstring +import logging +from datetime import datetime +from typing import Union + +from opaque_keys.edx.keys import CourseKey + +from openedx.core import types +from openedx.core.djangoapps.course_groups.cohorts import ( + get_cohort, + get_cohorted_user_partition_id, + get_group_info_for_cohort, +) + +from .base import OutlineProcessor + +log = logging.getLogger(__name__) + + +class CohortPartitionGroupsOutlineProcessor(OutlineProcessor): + """ + Processor for applying cohort user partition groups. + + """ + def __init__(self, course_key: CourseKey, user: types.User, at_time: datetime): + super().__init__(course_key, user, at_time) + self.user_cohort_group_id: Union[int, None] = None + self.cohorted_partition_id: Union[int, None] = None + + def load_data(self, full_course_outline) -> None: + """ + Load the cohorted partition id and the user's group id. + """ + + # It is possible that a cohort is not linked to any content group/partition. + # This is why the cohorted_partition_id needs to be set independently + # of a particular user's cohort. + self.cohorted_partition_id = get_cohorted_user_partition_id(self.course_key) + + if self.cohorted_partition_id: + user_cohort = get_cohort(self.user, self.course_key) + + if user_cohort: + self.user_cohort_group_id, _ = get_group_info_for_cohort(user_cohort) + + def _is_user_excluded_by_partition_group(self, user_partition_groups) -> bool: + """ + Is the user part of the group to which the block is restricting content? + """ + if not user_partition_groups: + return False + + groups = user_partition_groups.get(self.cohorted_partition_id) + if not groups: + return False + + if self.user_cohort_group_id not in groups: + # If the user's group (cohort) does not belong + # to the partition of the block or the user's cohort + # is not linked to a content group (user_cohort_group_id is None), + # the block should be removed + return True + return False + + def usage_keys_to_remove(self, full_course_outline): + """ + Content group exclusions remove the content entirely. + + Remove sections and sequences inacessible by the user's + cohort. + """ + if not self.cohorted_partition_id: + return frozenset() + + removed_usage_keys = set() + for section in full_course_outline.sections: + remove_all_children = False + if self._is_user_excluded_by_partition_group( + section.user_partition_groups + ): + removed_usage_keys.add(section.usage_key) + remove_all_children = True + for seq in section.sequences: + if remove_all_children or self._is_user_excluded_by_partition_group( + seq.user_partition_groups + ): + removed_usage_keys.add(seq.usage_key) + return removed_usage_keys diff --git a/openedx/core/djangoapps/content/learning_sequences/api/tests/test_outlines.py b/openedx/core/djangoapps/content/learning_sequences/api/tests/test_outlines.py index 805066d1d6db..61ba39954b30 100644 --- a/openedx/core/djangoapps/content/learning_sequences/api/tests/test_outlines.py +++ b/openedx/core/djangoapps/content/learning_sequences/api/tests/test_outlines.py @@ -19,6 +19,8 @@ import pytest from openedx.core.djangoapps.course_apps.toggles import EXAMS_IDA +from openedx.core.djangoapps.course_groups.models import CourseCohortsSettings, CourseUserGroupPartitionGroup +from openedx.core.djangoapps.course_groups.tests.helpers import CohortFactory from openedx.core.djangolib.testing.utils import CacheIsolationTestCase from openedx.features.course_experience import COURSE_ENABLE_UNENROLLED_ACCESS_FLAG from common.djangoapps.course_modes.models import CourseMode @@ -1717,6 +1719,190 @@ def test_processor_only( assert len(removed_usage_keys) == expected_values_dict[learner_to_verify.username] +@ddt.ddt +class CohortPartitionGroupsTestCase(OutlineProcessorTestCase): + """Tests for cohort partitions outline processor that affects outlines""" + + @classmethod + def setUpTestData(cls): + super().setUpTestData() + cls.visibility = VisibilityData( + hide_from_toc=False, + visible_to_staff_only=False + ) + + def _create_and_enroll_learner(self, username, is_staff=False): + """ + Helper function to create the learner based on the username, + then enroll the learner into the test course with VERIFIED + mode. + Returns the created learner + """ + learner = UserFactory.create( + username=username, email='{}@example.com'.format(username), is_staff=is_staff + ) + learner.courseenrollment_set.create(course_id=self.course_key, is_active=True, mode=CourseMode.VERIFIED) + return learner + + def _setup_course_outline_with_sections( + self, + course_sections, + course_start_date=datetime(2021, 3, 26, tzinfo=timezone.utc) + ): + """ + Helper function to update the course outline under test with + the course sections passed in. + Returns the newly constructed course outline + """ + set_dates_for_course( + self.course_key, + [ + ( + self.course_key.make_usage_key('course', 'course'), + {'start': course_start_date} + ) + ] + ) + + new_outline = CourseOutlineData( + course_key=self.course_key, + title="Cohort User Partition Test Course", + published_at=course_start_date, + published_version="8ebece4b69dd593d82fe2023", + sections=course_sections, + self_paced=False, + days_early_for_beta=None, + entrance_exam_id=None, + course_visibility=CourseVisibility.PRIVATE, + ) + + replace_course_outline(new_outline) + + return new_outline + + @ddt.data( + ( + None, + None, + ['student1', 'student2'], + {'student1': 1, 'student2': 1} + ), + ( + set([1001]), + None, + ['student1', 'student2'], + {'student1': 1, 'student2': 0} + ), + ( + set([1002]), + None, + ['student1', 'student2'], + {'student1': 0, 'student2': 1} + ), + ( + set([1001, 1002]), + None, + ['student1', 'student2'], + {'student1': 1, 'student2': 1} + ), + ( + None, + set([1001]), + ['student1', 'student2'], + {'student1': 1, 'student2': 0} + ), + ( + None, + set([1002]), + ['student1', 'student2'], + {'student1': 0, 'student2': 1} + ), + ( + None, + set([1001, 1002]), + ['student1', 'student2'], + {'student1': 1, 'student2': 1} + ), + ) + @ddt.unpack + def test_cohort_partition_on_outline( + self, + section_visible_groups, + sequence_visible_groups, + learners, + expected_values_dict + ): + + section_user_partition_groups = None + sequence_user_partition_groups = None + if section_visible_groups: + section_user_partition_groups = { + 1000: frozenset(section_visible_groups) + } + if sequence_visible_groups: + sequence_user_partition_groups = { + 1000: frozenset(sequence_visible_groups) + } + + CourseCohortsSettings.objects.create(course_id=self.course_key, is_cohorted=True) + + # Enroll students in the course + learners_to_verify = [] + for username in learners: + learners_to_verify.append( + self._create_and_enroll_learner(username) + ) + + # Create cohorts and corresponding GroupPartitions + cohort_1 = CohortFactory( + course_id=self.course_key, + name='Test Cohort 1', + users=[learners_to_verify[0]] + ) + + CourseUserGroupPartitionGroup( + course_user_group=cohort_1, + partition_id=1000, + group_id=1001 + ).save() + + cohort_2 = CohortFactory( + course_id=self.course_key, + name='Test Cohort 2', + users=[learners_to_verify[1]] + ) + + CourseUserGroupPartitionGroup( + course_user_group=cohort_2, + partition_id=1000, + group_id=1002 + ).save() + + self._setup_course_outline_with_sections( + [ + CourseSectionData( + usage_key=self.course_key.make_usage_key('chapter', '0'), + title="Section 0", + user_partition_groups=section_user_partition_groups, + sequences=[ + CourseLearningSequenceData( + usage_key=self.course_key.make_usage_key('subsection', '0'), + title='Subsection 0', + visibility=self.visibility, + user_partition_groups=sequence_user_partition_groups, + ), + ] + ) + ] + ) + + check_date = datetime(2021, 3, 27, tzinfo=timezone.utc) + + for learner_to_verify in learners_to_verify: + learner_details = get_user_course_outline_details(self.course_key, learner_to_verify, check_date) + assert len(learner_details.outline.accessible_sequences) == expected_values_dict[learner_to_verify.username] + + class ContentErrorTestCase(CacheIsolationTestCase): """Test error collection and reporting.""" diff --git a/openedx/core/djangoapps/content/learning_sequences/data.py b/openedx/core/djangoapps/content/learning_sequences/data.py index d7b706bb431a..c13b451490ab 100644 --- a/openedx/core/djangoapps/content/learning_sequences/data.py +++ b/openedx/core/djangoapps/content/learning_sequences/data.py @@ -250,10 +250,16 @@ def remove(self, usage_keys): """ keys_to_remove = set(usage_keys) - # If we remove a Section, we also remove all Sequences in that Section. for section in self.sections: + section_sequences_keys = {seq.usage_key for seq in section.sequences} + + # If we remove a Section, we also remove all Sequences in that Section. if section.usage_key in keys_to_remove: - keys_to_remove |= {seq.usage_key for seq in section.sequences} + keys_to_remove |= section_sequences_keys + + # If a Section is empty or about to be, we remove it. + elif section_sequences_keys.issubset(keys_to_remove): + keys_to_remove.add(section.usage_key) return attr.evolve( self, diff --git a/openedx/core/djangoapps/course_groups/cohorts.py b/openedx/core/djangoapps/course_groups/cohorts.py index a7298161f2fe..cb3dfc5b9e88 100644 --- a/openedx/core/djangoapps/course_groups/cohorts.py +++ b/openedx/core/djangoapps/course_groups/cohorts.py @@ -619,3 +619,17 @@ def _get_cohort_settings_from_modulestore(course): 'cohorted_discussions': list(course.cohorted_discussions), 'always_cohort_inline_discussions': course.always_cohort_inline_discussions } + + +def get_cohorted_user_partition_id(course_key): + """ + Returns the partition id to which cohorts are linked or None if there is no cohort linked + to a content group. + """ + course_user_group_partition_group = CourseUserGroupPartitionGroup.objects.filter( + course_user_group__course_id=course_key + ).first() + + if course_user_group_partition_group: + return course_user_group_partition_group.partition_id + return None