From 53fe1dd7bcf0165ad8fe00bed66237df0e7c616d Mon Sep 17 00:00:00 2001 From: Ahmad Bilal Khalid Date: Wed, 1 Jul 2020 14:15:32 +0500 Subject: [PATCH] Adds Enrollment Processor which is responsible for respecting course's outline access. Moved data.py out into the parent's dirctory i.e learning_sequences directory to avoid circular import issue which is occuring when we try to import data.py from models.py. A new CourseContext model is created, which is responsible for housing course specific fields e.g course_visibility. --- .../learning_sequences/api/outlines.py | 100 +++++++---- .../learning_sequences/api/processors/base.py | 2 - .../api/processors/enrollment.py | 46 +++++ .../api/processors/schedule.py | 2 +- .../api/processors/visibility.py | 4 + .../learning_sequences/api/tests/test_data.py | 7 +- .../api/tests/test_outlines.py | 168 ++++++++++++++++-- .../learning_sequences/{api => }/data.py | 10 +- ...urse_context_for_course_specific_models.py | 60 +++++++ .../content/learning_sequences/models.py | 29 ++- .../content/learning_sequences/tasks.py | 17 +- .../learning_sequences/tests/test_views.py | 7 +- .../content/learning_sequences/views.py | 2 - 13 files changed, 379 insertions(+), 75 deletions(-) create mode 100644 openedx/core/djangoapps/content/learning_sequences/api/processors/enrollment.py rename openedx/core/djangoapps/content/learning_sequences/{api => }/data.py (97%) create mode 100644 openedx/core/djangoapps/content/learning_sequences/migrations/0003_create_course_context_for_course_specific_models.py diff --git a/openedx/core/djangoapps/content/learning_sequences/api/outlines.py b/openedx/core/djangoapps/content/learning_sequences/api/outlines.py index 70bc8c15d1..b1d23b0758 100644 --- a/openedx/core/djangoapps/content/learning_sequences/api/outlines.py +++ b/openedx/core/djangoapps/content/learning_sequences/api/outlines.py @@ -15,16 +15,18 @@ from edx_django_utils.cache import TieredCache, get_cache_key from edx_django_utils.monitoring import function_trace from opaque_keys.edx.keys import CourseKey, UsageKey -from .data import ( +from ..data import ( CourseOutlineData, CourseSectionData, CourseLearningSequenceData, UserCourseOutlineData, UserCourseOutlineDetailsData, VisibilityData, + CourseVisibility ) from ..models import ( - CourseSection, CourseSectionSequence, LearningContext, LearningSequence + CourseSection, CourseSectionSequence, CourseContext, LearningContext, LearningSequence ) from .permissions import can_see_all_content from .processors.schedule import ScheduleOutlineProcessor from .processors.visibility import VisibilityOutlineProcessor +from .processors.enrollment import EnrollmentOutlineProcessor User = get_user_model() log = logging.getLogger(__name__) @@ -46,11 +48,11 @@ def get_course_outline(course_key: CourseKey) -> CourseOutlineData: See the definition of CourseOutlineData for details about the data returned. """ - learning_context = _get_learning_context_for_outline(course_key) + course_context = _get_course_context_for_outline(course_key) # Check to see if it's in the cache. cache_key = "learning_sequences.api.get_course_outline.v1.{}.{}".format( - learning_context.context_key, learning_context.published_version + course_context.learning_context.context_key, course_context.learning_context.published_version ) outline_cache_result = TieredCache.get_cached_response(cache_key) if outline_cache_result.is_found: @@ -60,10 +62,10 @@ def get_course_outline(course_key: CourseKey) -> CourseOutlineData: # represented (so query CourseSection explicitly instead of relying only on # select_related from CourseSectionSequence). section_models = CourseSection.objects \ - .filter(learning_context=learning_context) \ + .filter(course_context=course_context) \ .order_by('ordering') section_sequence_models = CourseSectionSequence.objects \ - .filter(learning_context=learning_context) \ + .filter(course_context=course_context) \ .order_by('ordering') \ .select_related('sequence') @@ -97,31 +99,37 @@ def get_course_outline(course_key: CourseKey) -> CourseOutlineData: ] outline_data = CourseOutlineData( - course_key=learning_context.context_key, - title=learning_context.title, - published_at=learning_context.published_at, - published_version=learning_context.published_version, + course_key=course_context.learning_context.context_key, + title=course_context.learning_context.title, + published_at=course_context.learning_context.published_at, + published_version=course_context.learning_context.published_version, sections=sections_data, + course_visibility=CourseVisibility(course_context.course_visibility), ) TieredCache.set_all_tiers(cache_key, outline_data, 300) return outline_data -def _get_learning_context_for_outline(course_key: CourseKey) -> LearningContext: +def _get_course_context_for_outline(course_key: CourseKey) -> CourseContext: + """ + Get Course Context for given param:course_key + """ if course_key.deprecated: raise ValueError( "Learning Sequence API does not support Old Mongo courses: {}" .format(course_key), ) try: - learning_context = LearningContext.objects.get(context_key=course_key) + course_context = ( + LearningContext.objects.select_related('course_context').get(context_key=course_key).course_context + ) except LearningContext.DoesNotExist: # Could happen if it hasn't been published. raise CourseOutlineData.DoesNotExist( "No CourseOutlineData for {}".format(course_key) ) - return learning_context + return course_context def get_user_course_outline(course_key: CourseKey, @@ -173,6 +181,7 @@ def _get_user_course_outline_and_processors(course_key: CourseKey, processor_classes = [ ('schedule', ScheduleOutlineProcessor), ('visibility', VisibilityOutlineProcessor), + ('enrollment', EnrollmentOutlineProcessor), # Future: # ('content_gating', ContentGatingOutlineProcessor), # ('milestones', MilestonesOutlineProcessor), @@ -209,7 +218,7 @@ def _get_user_course_outline_and_processors(course_key: CourseKey, accessible_sequences=accessible_sequences, **{ name: getattr(trimmed_course_outline, name) - for name in ['course_key', 'title', 'published_at', 'published_version', 'sections'] + for name in ['course_key', 'title', 'published_at', 'published_version', 'sections', 'course_visibility'] } ) @@ -229,40 +238,51 @@ def replace_course_outline(course_outline: CourseOutlineData): ) with transaction.atomic(): - # Update or create the basic LearningContext... - learning_context = _update_learning_context(course_outline) + # Update or create the basic CourseContext... + course_context = _update_course_context(course_outline) # Wipe out the CourseSectionSequences join+ordering table so we can # delete CourseSection and LearningSequence objects more easily. - learning_context.section_sequences.all().delete() + course_context.section_sequences.all().delete() - _update_sections(course_outline, learning_context) - _update_sequences(course_outline, learning_context) - _update_course_section_sequences(course_outline, learning_context) + _update_sections(course_outline, course_context) + _update_sequences(course_outline, course_context) + _update_course_section_sequences(course_outline, course_context) -def _update_learning_context(course_outline: CourseOutlineData): - learning_context, created = LearningContext.objects.update_or_create( +def _update_course_context(course_outline: CourseOutlineData): + """ + Update CourseContext with given param:course_outline data. + """ + learning_context, _ = LearningContext.objects.update_or_create( context_key=course_outline.course_key, defaults={ 'title': course_outline.title, 'published_at': course_outline.published_at, - 'published_version': course_outline.published_version + 'published_version': course_outline.published_version, + } + ) + course_context, created = CourseContext.objects.update_or_create( + learning_context=learning_context, + defaults={ + 'course_visibility': course_outline.course_visibility.value, } ) if created: - log.info("Created new LearningContext for %s", course_outline.course_key) + log.info("Created new CourseContext for %s", course_outline.course_key) else: - log.info("Found LearningContext for %s, updating...", course_outline.course_key) + log.info("Found CourseContext for %s, updating...", course_outline.course_key) - return learning_context + return course_context -def _update_sections(course_outline: CourseOutlineData, learning_context: LearningContext): - # Add/update relevant sections... +def _update_sections(course_outline: CourseOutlineData, course_context: CourseContext): + """ + Add/Update relevant sections + """ for ordering, section_data in enumerate(course_outline.sections): CourseSection.objects.update_or_create( - learning_context=learning_context, + course_context=course_context, usage_key=section_data.usage_key, defaults={ 'title': section_data.title, @@ -276,42 +296,48 @@ def _update_sections(course_outline: CourseOutlineData, learning_context: Learni section_data.usage_key for section_data in course_outline.sections ] CourseSection.objects \ - .filter(learning_context=learning_context) \ + .filter(course_context=course_context) \ .exclude(usage_key__in=section_usage_keys_to_keep) \ .delete() -def _update_sequences(course_outline: CourseOutlineData, learning_context: LearningContext): +def _update_sequences(course_outline: CourseOutlineData, course_context: CourseContext): + """ + Add/Update relevant sequences + """ for section_data in course_outline.sections: for sequence_data in section_data.sequences: LearningSequence.objects.update_or_create( - learning_context=learning_context, + learning_context=course_context.learning_context, usage_key=sequence_data.usage_key, defaults={'title': sequence_data.title} ) LearningSequence.objects \ - .filter(learning_context=learning_context) \ + .filter(learning_context=course_context.learning_context) \ .exclude(usage_key__in=course_outline.sequences) \ .delete() -def _update_course_section_sequences(course_outline: CourseOutlineData, learning_context: LearningContext): +def _update_course_section_sequences(course_outline: CourseOutlineData, course_context: CourseContext): + """ + Add/Update relevant course section and sequences + """ section_models = { section_model.usage_key: section_model for section_model - in CourseSection.objects.filter(learning_context=learning_context).all() + in CourseSection.objects.filter(course_context=course_context).all() } sequence_models = { sequence_model.usage_key: sequence_model for sequence_model - in LearningSequence.objects.filter(learning_context=learning_context).all() + in LearningSequence.objects.filter(learning_context=course_context.learning_context).all() } ordering = 0 for section_data in course_outline.sections: for sequence_data in section_data.sequences: CourseSectionSequence.objects.update_or_create( - learning_context=learning_context, + course_context=course_context, section=section_models[section_data.usage_key], sequence=sequence_models[sequence_data.usage_key], defaults={ diff --git a/openedx/core/djangoapps/content/learning_sequences/api/processors/base.py b/openedx/core/djangoapps/content/learning_sequences/api/processors/base.py index f19d188b7c..af98cb984c 100644 --- a/openedx/core/djangoapps/content/learning_sequences/api/processors/base.py +++ b/openedx/core/djangoapps/content/learning_sequences/api/processors/base.py @@ -8,8 +8,6 @@ from datetime import datetime from django.contrib.auth import get_user_model from opaque_keys.edx.keys import CourseKey, UsageKey -from ..data import ScheduleData, ScheduleItemData - User = get_user_model() log = logging.getLogger(__name__) diff --git a/openedx/core/djangoapps/content/learning_sequences/api/processors/enrollment.py b/openedx/core/djangoapps/content/learning_sequences/api/processors/enrollment.py new file mode 100644 index 0000000000..e89ad72dd6 --- /dev/null +++ b/openedx/core/djangoapps/content/learning_sequences/api/processors/enrollment.py @@ -0,0 +1,46 @@ +""" +Simple OutlineProcessor that removes items based on Enrollment and course visibility setting. +""" +from openedx.features.course_experience import COURSE_ENABLE_UNENROLLED_ACCESS_FLAG +from student.models import CourseEnrollment + +from .base import OutlineProcessor +from ...data import CourseVisibility + + +class EnrollmentOutlineProcessor(OutlineProcessor): + """ + Simple OutlineProcessor that removes items based on Enrollment and course visibility setting. + """ + def usage_keys_to_remove(self, full_course_outline): + """ + Return sequences/sections to be removed + """ + # Public outlines and courses don't need to hide anything from the outline. + is_unenrolled_access_enabled = COURSE_ENABLE_UNENROLLED_ACCESS_FLAG.is_enabled(self.course_key) + is_course_outline_publicly_visible = ( + full_course_outline.course_visibility in [CourseVisibility.PUBLIC, CourseVisibility.PUBLIC_OUTLINE] + ) + + if is_unenrolled_access_enabled and is_course_outline_publicly_visible: + return frozenset() + + # Students who are enrolled can see the full outline. + if CourseEnrollment.is_enrolled(self.user, self.course_key): + return frozenset() + + # Otherwise remove everything: + seqs_to_remove = set(full_course_outline.sequences) + sections_to_remove = set(sec.usage_key for sec in full_course_outline.sections) + + return frozenset(seqs_to_remove | sections_to_remove) + + def inaccessible_sequences(self, full_course_outline): + """ + Return a set/frozenset of Sequence UsageKeys that are not accessible. + """ + is_public_outline = full_course_outline.course_visibility == CourseVisibility.PUBLIC_OUTLINE + is_enrolled_in_course = CourseEnrollment.is_enrolled(self.user, self.course_key) + if is_public_outline and not is_enrolled_in_course: + return frozenset(full_course_outline.sequences) + return frozenset() diff --git a/openedx/core/djangoapps/content/learning_sequences/api/processors/schedule.py b/openedx/core/djangoapps/content/learning_sequences/api/processors/schedule.py index 1952640b0a..f6a0d94699 100644 --- a/openedx/core/djangoapps/content/learning_sequences/api/processors/schedule.py +++ b/openedx/core/djangoapps/content/learning_sequences/api/processors/schedule.py @@ -6,7 +6,7 @@ from django.contrib.auth import get_user_model from edx_when.api import get_dates_for_course from opaque_keys.edx.keys import CourseKey, UsageKey -from ..data import ScheduleData, ScheduleItemData, UserCourseOutlineData +from ...data import ScheduleData, ScheduleItemData, UserCourseOutlineData from .base import OutlineProcessor User = get_user_model() diff --git a/openedx/core/djangoapps/content/learning_sequences/api/processors/visibility.py b/openedx/core/djangoapps/content/learning_sequences/api/processors/visibility.py index 605e4c6ee3..14a4662498 100644 --- a/openedx/core/djangoapps/content/learning_sequences/api/processors/visibility.py +++ b/openedx/core/djangoapps/content/learning_sequences/api/processors/visibility.py @@ -1,3 +1,7 @@ +""" +Simple OutlineProcessor that removes items based on VisibilityData. +""" + from .base import OutlineProcessor diff --git a/openedx/core/djangoapps/content/learning_sequences/api/tests/test_data.py b/openedx/core/djangoapps/content/learning_sequences/api/tests/test_data.py index 46f016e8ec..f8bdc34bc1 100644 --- a/openedx/core/djangoapps/content/learning_sequences/api/tests/test_data.py +++ b/openedx/core/djangoapps/content/learning_sequences/api/tests/test_data.py @@ -4,8 +4,8 @@ from unittest import TestCase from opaque_keys.edx.keys import CourseKey import attr -from ..data import ( - CourseOutlineData, CourseSectionData, CourseLearningSequenceData, VisibilityData +from ...data import ( + CourseOutlineData, CourseSectionData, CourseLearningSequenceData, VisibilityData, CourseVisibility ) @@ -31,7 +31,8 @@ class TestCourseOutlineData(TestCase): title="Exciting Test Course!", published_at=datetime(2020, 5, 19, tzinfo=timezone.utc), published_version="5ebece4b69dd593d82fe2014", - sections=generate_sections(cls.course_key, [3, 2]) + sections=generate_sections(cls.course_key, [3, 2]), + course_visibility=CourseVisibility.PRIVATE ) def test_deprecated_course_key(self): 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 a720dbcc84..83233bca55 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 @@ -4,20 +4,24 @@ models for this app. """ from datetime import datetime, timezone -from django.contrib.auth.models import User, AnonymousUser +import attr +from django.contrib.auth.models import AnonymousUser, User from edx_when.api import set_dates_for_course from opaque_keys.edx.keys import CourseKey, UsageKey from opaque_keys.edx.locator import BlockUsageLocator -import attr +from openedx.core.djangoapps.waffle_utils.testutils import override_waffle_flag from openedx.core.djangolib.testing.utils import CacheIsolationTestCase -from ..data import ( - CourseOutlineData, CourseSectionData, CourseLearningSequenceData, - VisibilityData +from openedx.features.course_experience import COURSE_ENABLE_UNENROLLED_ACCESS_FLAG + +from ...data import ( + CourseLearningSequenceData, CourseOutlineData, CourseSectionData, VisibilityData, CourseVisibility ) from ..outlines import ( - get_course_outline, get_user_course_outline, - get_user_course_outline_details, replace_course_outline + get_course_outline, + get_user_course_outline, + get_user_course_outline_details, + replace_course_outline ) from .test_data import generate_sections @@ -39,6 +43,7 @@ class CourseOutlineTestCase(CacheIsolationTestCase): published_at=datetime(2020, 5, 20, tzinfo=timezone.utc), published_version="5ebece4b69dd593d82fe2015", sections=generate_sections(cls.course_key, [2, 2]), + course_visibility=CourseVisibility.PRIVATE ) def test_deprecated_course_key(self): @@ -118,23 +123,23 @@ class UserCourseOutlineTestCase(CacheIsolationTestCase): cls.student = User.objects.create_user( 'student', email='student@example.com', is_staff=False ) - # TODO: Add AnonymousUser here. + cls.anonymous_user = AnonymousUser() # Seed with data cls.course_key = CourseKey.from_string("course-v1:OpenEdX+Outline+T1") - normal_visibility = VisibilityData( - hide_from_toc=False, - visible_to_staff_only=False - ) cls.simple_outline = CourseOutlineData( course_key=cls.course_key, title="User Outline Test Course!", published_at=datetime(2020, 5, 20, tzinfo=timezone.utc), published_version="5ebece4b69dd593d82fe2020", - sections=generate_sections(cls.course_key, [2, 1, 3]) + sections=generate_sections(cls.course_key, [2, 1, 3]), + course_visibility=CourseVisibility.PRIVATE ) replace_course_outline(cls.simple_outline) + # Enroll student in the course + cls.student.courseenrollment_set.create(course_id=cls.course_key, is_active=True, mode="audit") + def test_simple_outline(self): """This outline is the same for everyone.""" at_time = datetime(2020, 5, 21, tzinfo=timezone.utc) @@ -177,7 +182,7 @@ class ScheduleTestCase(CacheIsolationTestCase): cls.student = User.objects.create_user( 'student', email='student@example.com', is_staff=False ) - # TODO: Add AnonymousUser here. + cls.anonymous_user = AnonymousUser() cls.course_key = CourseKey.from_string("course-v1:OpenEdX+Outline+T1") @@ -249,6 +254,7 @@ class ScheduleTestCase(CacheIsolationTestCase): title="User Outline Test Course!", published_at=datetime(2020, 5, 20, tzinfo=timezone.utc), published_version="5ebece4b69dd593d82fe2020", + course_visibility=CourseVisibility.PRIVATE, sections=[ CourseSectionData( usage_key=cls.section_key, @@ -286,6 +292,9 @@ class ScheduleTestCase(CacheIsolationTestCase): ) replace_course_outline(cls.outline) + # Enroll student in the course + cls.student.courseenrollment_set.create(course_id=cls.course_key, is_active=True, mode="audit") + def get_details(self, at_time): staff_details = get_user_course_outline_details(self.course_key, self.global_staff, at_time) student_details = get_user_course_outline_details(self.course_key, self.student, at_time) @@ -385,7 +394,8 @@ class VisbilityTestCase(CacheIsolationTestCase): cls.student = User.objects.create_user( 'student', email='student@example.com', is_staff=False ) - # TODO: Add AnonymousUser here. + cls.anonymous_user = AnonymousUser() + cls.course_key = CourseKey.from_string("course-v1:OpenEdX+Outline+T1") # The UsageKeys we're going to set up for date tests. @@ -416,6 +426,7 @@ class VisbilityTestCase(CacheIsolationTestCase): title="User Outline Test Course!", published_at=datetime(2020, 5, 20, tzinfo=timezone.utc), published_version="5ebece4b69dd593d82fe2020", + course_visibility=CourseVisibility.PRIVATE, sections=[ CourseSectionData( usage_key=cls.normal_section_key, @@ -448,6 +459,9 @@ class VisbilityTestCase(CacheIsolationTestCase): ) replace_course_outline(cls.outline) + # Enroll student in the course + cls.student.courseenrollment_set.create(course_id=cls.course_key, is_active=True, mode="audit") + def test_visibility(self): at_time = datetime(2020, 5, 21, tzinfo=timezone.utc) # Exact value doesn't matter staff_details = get_user_course_outline_details(self.course_key, self.global_staff, at_time) @@ -461,3 +475,127 @@ class VisbilityTestCase(CacheIsolationTestCase): assert len(staff_details.outline.sequences) == 4 assert len(student_details.outline.sequences) == 1 assert self.normal_in_normal_key in student_details.outline.sequences + + +class SequentialVisibilityTestCase(CacheIsolationTestCase): + """ + Tests sequentials visibility under different course visibility settings i.e public, public_outline, private + and different types of users e.g unenrolled, enrolled, anonymous, staff + """ + + @classmethod + def setUpTestData(cls): + super(SequentialVisibilityTestCase, cls).setUpTestData() + + cls.global_staff = User.objects.create_user('global_staff', email='gstaff@example.com', is_staff=True) + cls.student = User.objects.create_user('student', email='student@example.com', is_staff=False) + cls.unenrolled_student = User.objects.create_user('unenrolled', email='unenrolled@example.com', is_staff=False) + cls.anonymous_user = AnonymousUser() + + # Handy variable as we almost always need to test with all types of users + cls.all_users = [cls.global_staff, cls.student, cls.unenrolled_student, cls.anonymous_user] + + cls.course_access_time = datetime(2020, 5, 21, tzinfo=timezone.utc) # Some random time in past + + # Create course, set it start date to some time in past and attach outline to it + cls.course_key = CourseKey.from_string("course-v1:OpenEdX+Outline+T0") + set_dates_for_course( + cls.course_key, [(cls.course_key.make_usage_key('course', 'course'), {'start': cls.course_access_time})] + ) + cls.course_outline = CourseOutlineData( + course_key=cls.course_key, + title="User Outline Test Course!", + published_at=datetime(2020, 5, 20, tzinfo=timezone.utc), + published_version="5ebece4b69dd593d82fe2020", + sections=generate_sections(cls.course_key, [2, 1, 3]), + course_visibility=CourseVisibility.PRIVATE + ) + replace_course_outline(cls.course_outline) + + # enroll student into the course + cls.student.courseenrollment_set.create(course_id=cls.course_key, is_active=True, mode="audit") + + @override_waffle_flag(COURSE_ENABLE_UNENROLLED_ACCESS_FLAG, active=True) + def test_public_course_outline(self): + """Test that public course outline is the same for everyone.""" + course_outline = attr.evolve(self.course_outline, course_visibility=CourseVisibility.PUBLIC) + replace_course_outline(course_outline) + + for user in self.all_users: + with self.subTest(user=user): + user_course_outline = get_user_course_outline(self.course_key, user, self.course_access_time) + + self.assertEqual(len(user_course_outline.sections), 3) + self.assertEqual(len(user_course_outline.sequences), 6) + self.assertTrue( + all([ + seq.usage_key in user_course_outline.accessible_sequences + for seq in user_course_outline.sequences.values() + ]), + "Sequences should be accessible to all users for a public course" + ) + + @override_waffle_flag(COURSE_ENABLE_UNENROLLED_ACCESS_FLAG, active=True) + def test_public_outline_course_outline(self): + """ + Test that a course with public_outline access has same outline for everyone + except that the links are not accessible for non-enrolled and anonymous user. + """ + course_outline = attr.evolve(self.course_outline, course_visibility=CourseVisibility.PUBLIC_OUTLINE) + replace_course_outline(course_outline) + + for user in self.all_users: + with self.subTest(user=user): + user_course_outline = get_user_course_outline(self.course_key, user, self.course_access_time) + + self.assertEqual(len(user_course_outline.sections), 3) + self.assertEqual(len(user_course_outline.sequences), 6) + + is_sequence_accessible = [ + seq.usage_key in user_course_outline.accessible_sequences + for seq in user_course_outline.sequences.values() + ] + + if user in [self.anonymous_user, self.unenrolled_student]: + self.assertTrue( + all(not is_accessible for is_accessible in is_sequence_accessible), + "Sequences shouldn't be accessible to anonymous or non-enrolled students " + "for a public_outline course" + ) + else: + self.assertTrue( + all(is_sequence_accessible), + "Sequences should be accessible to enrolled, staff users for a public_outline course" + ) + + @override_waffle_flag(COURSE_ENABLE_UNENROLLED_ACCESS_FLAG, active=True) + def test_private_course_outline(self): + """ + Test that the outline of a course with private access is only accessible/visible + to enrolled user or staff. + """ + course_outline = attr.evolve(self.course_outline, course_visibility=CourseVisibility.PRIVATE) + replace_course_outline(course_outline) + + for user in self.all_users: + with self.subTest(user=user): + user_course_outline = get_user_course_outline(self.course_key, user, self.course_access_time) + + is_sequence_accessible = [ + seq.usage_key in user_course_outline.accessible_sequences + for seq in user_course_outline.sequences.values() + ] + + if user in [self.anonymous_user, self.unenrolled_student]: + self.assertTrue( + len(user_course_outline.sections) == len(user_course_outline.sequences) == 0, + "No section of a private course should be visible to anonymous or non-enrolled student" + ) + else: + # Enrolled or Staff User + self.assertEqual(len(user_course_outline.sections), 3) + self.assertEqual(len(user_course_outline.sequences), 6) + self.assertTrue( + all(is_sequence_accessible), + "Sequences should be accessible to enrolled, staff users for a public_outline course" + ) diff --git a/openedx/core/djangoapps/content/learning_sequences/api/data.py b/openedx/core/djangoapps/content/learning_sequences/data.py similarity index 97% rename from openedx/core/djangoapps/content/learning_sequences/api/data.py rename to openedx/core/djangoapps/content/learning_sequences/data.py index edb319b8aa..556e763f54 100644 --- a/openedx/core/djangoapps/content/learning_sequences/api/data.py +++ b/openedx/core/djangoapps/content/learning_sequences/data.py @@ -25,17 +25,23 @@ TODO: Validate all datetimes to be UTC. """ import logging from datetime import datetime, timezone +from enum import Enum from typing import Dict, List, Optional, Set import attr from django.contrib.auth import get_user_model from opaque_keys.edx.keys import CourseKey, UsageKey - User = get_user_model() log = logging.getLogger(__name__) +class CourseVisibility(Enum): + PRIVATE = "private" + PUBLIC_OUTLINE = "public_outline" + PUBLIC = "public" + + class ObjectDoesNotExist(Exception): """ Imitating Django model conventions, we put a subclass of this in some of our @@ -132,6 +138,8 @@ class CourseOutlineData: # derived from what you pass into `sections`. Do not set this directly. sequences = attr.ib(type=Dict[UsageKey, CourseLearningSequenceData], init=False) + course_visibility = attr.ib(validator=attr.validators.in_(CourseVisibility)) + def __attrs_post_init__(self): """Post-init hook that validates and inits the `sequences` field.""" sequences = {} diff --git a/openedx/core/djangoapps/content/learning_sequences/migrations/0003_create_course_context_for_course_specific_models.py b/openedx/core/djangoapps/content/learning_sequences/migrations/0003_create_course_context_for_course_specific_models.py new file mode 100644 index 0000000000..ca7ece7c42 --- /dev/null +++ b/openedx/core/djangoapps/content/learning_sequences/migrations/0003_create_course_context_for_course_specific_models.py @@ -0,0 +1,60 @@ +# Generated by Django 2.2.14 on 2020-07-22 18:27 + +from django.db import migrations, models +import django.db.models.deletion +import django.utils.timezone +import model_utils.fields + + +class Migration(migrations.Migration): + + dependencies = [ + ('learning_sequences', '0002_coursesectionsequence_inaccessible_after_due'), + ] + + operations = [ + migrations.CreateModel( + name='CourseContext', + fields=[ + ('created', model_utils.fields.AutoCreatedField(default=django.utils.timezone.now, editable=False, verbose_name='created')), + ('modified', model_utils.fields.AutoLastModifiedField(default=django.utils.timezone.now, editable=False, verbose_name='modified')), + ('learning_context', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, primary_key=True, related_name='course_context', serialize=False, to='learning_sequences.LearningContext')), + ('course_visibility', models.CharField(choices=[('private', 'private'), ('public_outline', 'public_outline'), ('public', 'public')], max_length=32)), + ], + options={ + 'abstract': False, + }, + ), + migrations.AddField( + model_name='coursesection', + name='course_context', + field=models.ForeignKey(default=None, on_delete=django.db.models.deletion.CASCADE, related_name='sections', to='learning_sequences.CourseContext'), + preserve_default=False, + ), + migrations.AddField( + model_name='coursesectionsequence', + name='course_context', + field=models.ForeignKey(default=None, on_delete=django.db.models.deletion.CASCADE, related_name='section_sequences', to='learning_sequences.CourseContext'), + preserve_default=False, + ), + migrations.AlterUniqueTogether( + name='coursesection', + unique_together={('course_context', 'usage_key')}, + ), + migrations.AlterUniqueTogether( + name='coursesectionsequence', + unique_together={('course_context', 'ordering')}, + ), + migrations.AlterIndexTogether( + name='coursesection', + index_together={('course_context', 'ordering')}, + ), + migrations.RemoveField( + model_name='coursesection', + name='learning_context', + ), + migrations.RemoveField( + model_name='coursesectionsequence', + name='learning_context', + ), + ] diff --git a/openedx/core/djangoapps/content/learning_sequences/models.py b/openedx/core/djangoapps/content/learning_sequences/models.py index 02149d4656..ac0b54c50c 100644 --- a/openedx/core/djangoapps/content/learning_sequences/models.py +++ b/openedx/core/djangoapps/content/learning_sequences/models.py @@ -43,6 +43,7 @@ from model_utils.models import TimeStampedModel from opaque_keys.edx.django.models import ( CourseKeyField, LearningContextKeyField, UsageKeyField ) +from .data import CourseVisibility class LearningContext(TimeStampedModel): @@ -67,6 +68,20 @@ class LearningContext(TimeStampedModel): ] +class CourseContext(TimeStampedModel): + """ + A model containing course specific information e.g course_visibility + """ + learning_context = models.OneToOneField( + LearningContext, on_delete=models.CASCADE, primary_key=True, related_name="course_context" + ) + # Please note, the tuple is intentionally use value for both actual value and display value + # because the name of enum constant is written in upper while the values are lower + course_visibility = models.CharField( + max_length=32, choices=[(constant.value, constant.value) for constant in CourseVisibility] + ) + + class LearningSequence(TimeStampedModel): """ The reason why this model doesn't have a direct foreign key to CourseSection @@ -128,8 +143,8 @@ class CourseSection(CourseContentVisibilityMixin, TimeStampedModel): re-created on course publish. """ id = models.BigAutoField(primary_key=True) - learning_context = models.ForeignKey( - LearningContext, on_delete=models.CASCADE, related_name='sections' + course_context = models.ForeignKey( + CourseContext, on_delete=models.CASCADE, related_name='sections' ) usage_key = UsageKeyField(max_length=255) title = models.CharField(max_length=1000) @@ -139,10 +154,10 @@ class CourseSection(CourseContentVisibilityMixin, TimeStampedModel): class Meta: unique_together = [ - ['learning_context', 'usage_key'], + ['course_context', 'usage_key'], ] index_together = [ - ['learning_context', 'ordering'], + ['course_context', 'ordering'], ] @@ -162,8 +177,8 @@ class CourseSectionSequence(CourseContentVisibilityMixin, TimeStampedModel): re-created on course publish. """ id = models.BigAutoField(primary_key=True) - learning_context = models.ForeignKey( - LearningContext, on_delete=models.CASCADE, related_name='section_sequences' + course_context = models.ForeignKey( + CourseContext, on_delete=models.CASCADE, related_name='section_sequences' ) section = models.ForeignKey(CourseSection, on_delete=models.CASCADE) sequence = models.ForeignKey(LearningSequence, on_delete=models.CASCADE) @@ -177,5 +192,5 @@ class CourseSectionSequence(CourseContentVisibilityMixin, TimeStampedModel): class Meta: unique_together = [ - ['learning_context', 'ordering'], + ['course_context', 'ordering'], ] diff --git a/openedx/core/djangoapps/content/learning_sequences/tasks.py b/openedx/core/djangoapps/content/learning_sequences/tasks.py index 91d9733846..1c456e8044 100644 --- a/openedx/core/djangoapps/content/learning_sequences/tasks.py +++ b/openedx/core/djangoapps/content/learning_sequences/tasks.py @@ -9,8 +9,9 @@ from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore from .api import replace_course_outline -from .api.data import ( - CourseOutlineData, CourseSectionData, CourseLearningSequenceData, VisibilityData +from .data import ( + CourseOutlineData, CourseSectionData, CourseLearningSequenceData, VisibilityData, + CourseVisibility ) @@ -22,6 +23,14 @@ def update_from_modulestore(course_key): We should move this out so that learning_sequences does not depend on ModuleStore. """ + course_outline_data = get_outline_from_modulestore(course_key) + replace_course_outline(course_outline_data) + + +def get_outline_from_modulestore(course_key): + """ + Get CourseOutlineData corresponding to param:course_key + """ def _make_section_data(section): sequences_data = [] for sequence in section.get_children(): @@ -63,6 +72,6 @@ def update_from_modulestore(course_key): published_at=course.subtree_edited_on, published_version=str(course.course_version), # .course_version is a BSON obj sections=sections_data, + course_visibility=CourseVisibility(course.course_visibility), ) - - replace_course_outline(course_outline_data) + return course_outline_data diff --git a/openedx/core/djangoapps/content/learning_sequences/tests/test_views.py b/openedx/core/djangoapps/content/learning_sequences/tests/test_views.py index efb0e0b879..56d8ae7f8e 100644 --- a/openedx/core/djangoapps/content/learning_sequences/tests/test_views.py +++ b/openedx/core/djangoapps/content/learning_sequences/tests/test_views.py @@ -23,7 +23,7 @@ from openedx.core.djangolib.testing.utils import CacheIsolationTestCase from student.tests.factories import UserFactory from ..api import replace_course_outline -from ..api.data import CourseOutlineData +from ..data import CourseOutlineData, CourseVisibility from ..api.tests.test_data import generate_sections @@ -44,7 +44,8 @@ class CourseOutlineViewTest(CacheIsolationTestCase, APITestCase): title="Views Test Course!", published_at=datetime(2020, 5, 20, tzinfo=timezone.utc), published_version="5ebece4b69dd593d82fe2020", - sections=generate_sections(cls.course_key, [2, 2]) + sections=generate_sections(cls.course_key, [2, 2]), + course_visibility=CourseVisibility.PUBLIC ) replace_course_outline(cls.outline) @@ -60,7 +61,7 @@ class CourseOutlineViewTest(CacheIsolationTestCase, APITestCase): """ For now, make sure you need staff access bits to use the API. - This is a temporary safeguard until the API is more complete. + This is a temporary safeguard until the API is more complete """ self.client.login(username='student', password='student_pass') result = self.client.get(self.course_url) diff --git a/openedx/core/djangoapps/content/learning_sequences/views.py b/openedx/core/djangoapps/content/learning_sequences/views.py index 223f0e8524..a1d9dfd522 100644 --- a/openedx/core/djangoapps/content/learning_sequences/views.py +++ b/openedx/core/djangoapps/content/learning_sequences/views.py @@ -18,8 +18,6 @@ import attr from openedx.core.lib.api.permissions import IsStaff from .api import get_user_course_outline_details -from .api.data import ScheduleData, UserCourseOutlineData - User = get_user_model() log = logging.getLogger(__name__)