fix: Use learning_sequences to remove bad sequences from outline.

A common usage pattern is to make an exam that is restricted to a
particular enrollment track. Since there is no UI in Studio to do this
at the sequence/subsection level, course authors instead restrict every
unit in the sequence to that track (e.g. "verified"). The legacy
courseware experience could handle this, but the Course Blocks API does
not. It is likely that we'll want to permit toggling this at the
sequence level, but regardless of whether we do that going forward, we
still have to deal with this kind of content data in existing courses.

An entirely empty sequence is useless, and currently breaks the
Courseware MFE browsing experience. This commit introduces the first
part of that fix, invoking the new Learning Sequences API to remove
sequences that the user shouldn't be allowed to know about. The plan is
to switch over the entire course outline from the Course Blocks API to
the Learning Sequences API, and this is the first small step in that
direction.

This also creates CourseWaffleFlag learning_sequences.use_for_outlines
to control the gradual rollout of this feature. This will start as a
very limited rollout to address courses that show this specific bug
(TNL-8377).

New learning_sequences public API call: public_api_available
This commit is contained in:
David Ormsbee
2021-06-10 14:59:14 -04:00
parent 20d631ff70
commit fe6448303d
8 changed files with 246 additions and 34 deletions

View File

@@ -3,7 +3,7 @@ Tests for Outline Tab API in the Course Home API
"""
import itertools
from datetime import datetime
from datetime import datetime, timezone
from unittest.mock import Mock, patch
import ddt
@@ -18,6 +18,9 @@ from common.djangoapps.student.tests.factories import UserFactory
from lms.djangoapps.course_home_api.tests.utils import BaseCourseHomeTests
from lms.djangoapps.course_home_api.toggles import COURSE_HOME_MICROFRONTEND
from lms.djangoapps.experiments.testutils import override_experiment_waffle_flag
from openedx.core.djangoapps.content.learning_sequences.api import replace_course_outline
from openedx.core.djangoapps.content.learning_sequences.data import CourseOutlineData, CourseVisibility
from openedx.core.djangoapps.content.learning_sequences.toggles import USE_FOR_OUTLINES
from openedx.core.djangoapps.course_date_signals.utils import MIN_DURATION
from openedx.core.djangoapps.user_api.preferences.api import set_user_preference
from openedx.core.djangoapps.user_api.tests.factories import UserCourseTagFactory
@@ -366,3 +369,39 @@ class OutlineTabTestViews(BaseCourseHomeTests):
response = self.client.get(self.url)
assert response.data['verified_mode'] == {'access_expiration_date': (enrollment.created + MIN_DURATION), 'currency': 'USD', 'currency_symbol': '$', 'price': 149, 'sku': 'ABCD1234', 'upgrade_url': '/dashboard'}
@override_experiment_waffle_flag(COURSE_HOME_MICROFRONTEND, active=True)
def test_hide_learning_sequences(self):
"""
Check that Learning Sequences filters out sequences.
"""
CourseEnrollment.enroll(self.user, self.course.id, CourseMode.VERIFIED)
# Normal behavior: the sequence exists
response = self.client.get(self.url)
assert response.status_code == 200
blocks = response.data['course_blocks']['blocks']
seq_block_id = next(
block_id
for block_id, block in blocks.items()
if block['type'] == 'sequential'
)
# With Learning Sequences active and a course outline loaded, the same
# sequence is removed.
with override_waffle_flag(USE_FOR_OUTLINES, active=True):
new_learning_seq_outline = CourseOutlineData(
course_key=self.course.id,
title="Test Course Outline!",
published_at=datetime(2021, 6, 14, tzinfo=timezone.utc),
published_version="5ebece4b69dd593d82fe2022",
entrance_exam_id=None,
days_early_for_beta=None,
sections=[],
self_paced=False,
course_visibility=CourseVisibility.PRIVATE
)
replace_course_outline(new_learning_seq_outline)
response = self.client.get(self.url)
blocks = response.data['course_blocks']['blocks']
assert seq_block_id not in blocks

View File

@@ -1,6 +1,7 @@
"""
Outline Tab Views
"""
from datetime import datetime, timezone
from completion.exceptions import UnavailableCompletionData
from completion.utilities import get_key_to_last_completed_block
@@ -37,6 +38,10 @@ from lms.djangoapps.courseware.courses import get_course_date_blocks, get_course
from lms.djangoapps.courseware.date_summary import TodaysDate
from lms.djangoapps.courseware.masquerade import is_masquerading, setup_masquerade
from lms.djangoapps.courseware.views.views import get_cert_data
from openedx.core.djangoapps.content.learning_sequences.api import (
get_user_course_outline,
public_api_available as learning_sequences_api_available,
)
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
from openedx.core.lib.api.authentication import BearerAuthenticationAllowInactiveUser
from openedx.features.course_duration_limits.access import get_access_expiration_data
@@ -277,6 +282,35 @@ class OutlineTabView(RetrieveAPIView):
elif course.invitation_only:
enroll_alert['can_enroll'] = False
# Sometimes there are sequences returned by Course Blocks that we
# don't actually want to show to the user, such as when a sequence is
# composed entirely of units that the user can't access. The Learning
# Sequences API knows how to roll this up, so we use it determine which
# sequences we should remove from course_blocks.
#
# The long term goal is to remove the Course Blocks API call entirely,
# so this is a tiny first step in that migration.
if course_blocks and learning_sequences_api_available(course_key, request.user):
user_course_outline = get_user_course_outline(
course_key, request.user, datetime.now(tz=timezone.utc)
)
available_seq_ids = {str(usage_key) for usage_key in user_course_outline.sequences}
# 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['children']:
chapter_data['children'] = [
seq_data
for seq_data in chapter_data['children']
if (
seq_data['id'] in available_seq_ids or
# Edge case: Sometimes we have weird course structures.
# We expect only sequentials here, but if there is
# another type, just skip it (don't filter it out).
seq_data['type'] != 'sequential'
)
]
data = {
'access_expiration': access_expiration,
'cert_data': cert_data,

View File

@@ -6,5 +6,6 @@ from .outlines import (
get_user_course_outline,
get_user_course_outline_details,
key_supports_outlines,
public_api_available,
replace_course_outline,
)

View File

@@ -39,7 +39,7 @@ from ..models import (
PublishReport,
UserPartitionGroup
)
from .permissions import can_see_all_content
from .permissions import can_call_public_api, can_see_all_content
from .processors.content_gating import ContentGatingOutlineProcessor
from .processors.enrollment import EnrollmentOutlineProcessor
from .processors.enrollment_track_partition_groups import EnrollmentTrackPartitionGroupsOutlineProcessor
@@ -58,6 +58,7 @@ __all__ = [
'get_user_course_outline',
'get_user_course_outline_details',
'key_supports_outlines',
'public_api_available',
'replace_course_outline',
]
@@ -84,6 +85,21 @@ def key_supports_outlines(opaque_key: OpaqueKey) -> bool:
return False
def public_api_available(course_key: CourseKey, user: types.User) -> bool:
"""
Is the Public API available for this Course to this User?
This only really exists while we do the waffle-flag rollout of this feature,
so that in-process callers from other apps can determine whether they should
trust Learning Sequences API data for a particular user/course.
"""
return (
key_supports_outlines(course_key) and
LearningContext.objects.filter(context_key=course_key).exists() and
can_call_public_api(user, course_key)
)
@function_trace('learning_sequences.api.get_course_keys_with_outlines')
def get_course_keys_with_outlines() -> QuerySet:
"""

View File

@@ -1,8 +1,37 @@
"""
This is a dummy placeholder for now, as access to this endpoint is currently
limited to global staff.
Simple permissions for Learning Sequences.
Most access rules determining what a user will see are determined within the
outline processors themselves. This is where we'd put permissions that are used
to determine whether those processors even need to be run to filter the results.
"""
from common.djangoapps.student.roles import (
GlobalStaff,
CourseInstructorRole,
CourseStaffRole,
)
from ..toggles import USE_FOR_OUTLINES
def can_see_all_content(requesting_user, _course_key):
return requesting_user.is_staff
def can_call_public_api(requesting_user, course_key):
"""
Global staff can always call the public API. Otherwise, check waffle flag.
This is only intended for rollout purposes, and eventually everyone will be
able to call the public API for all courses.
"""
return GlobalStaff().has_user(requesting_user) or USE_FOR_OUTLINES.is_enabled(course_key)
def can_see_all_content(requesting_user, course_key):
"""
Global staff, course staff, and instructors can see everything.
There's no need to run processors to restrict results for these users.
"""
return (
GlobalStaff().has_user(requesting_user) or
CourseStaffRole(course_key).has_user(requesting_user) or
CourseInstructorRole(course_key).has_user(requesting_user)
)

View File

@@ -3,20 +3,22 @@ Top level API tests. Tests API public contracts only. Do not import/create/mock
models for this app.
"""
from datetime import datetime, timezone
from unittest import TestCase
from unittest.mock import patch
import unittest
import ddt
import pytest
import attr
from django.conf import settings
from django.contrib.auth.models import AnonymousUser, User # lint-amnesty, pylint: disable=imported-auth-user
from django.db.models import signals
from edx_proctoring.exceptions import ProctoredExamNotFoundException
from edx_toggles.toggles.testutils import override_waffle_flag
from edx_when.api import set_dates_for_course
from opaque_keys.edx.keys import CourseKey
from opaque_keys.edx.locator import LibraryLocator
from edx_toggles.toggles.testutils import override_waffle_flag
import attr
import ddt
import django.test
import pytest
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
@@ -38,19 +40,21 @@ from ...data import (
VisibilityData,
)
from ...toggles import USE_FOR_OUTLINES
from ..outlines import (
get_content_errors,
get_course_outline,
get_user_course_outline,
get_user_course_outline_details,
key_supports_outlines,
public_api_available,
replace_course_outline,
)
from ..processors.enrollment_track_partition_groups import EnrollmentTrackPartitionGroupsOutlineProcessor
from .test_data import generate_sections
class OutlineSupportTestCase(TestCase):
class OutlineSupportTestCase(unittest.TestCase):
"""
Make sure we know what kinds of course-like keys we support for outlines.
"""
@@ -63,6 +67,62 @@ class OutlineSupportTestCase(TestCase):
assert not key_supports_outlines(LibraryLocator(org="edX", library="100"))
class PublicApiAvailableTestCase(django.test.TestCase):
"""
Test API availability checks.
"""
@classmethod
def setUpTestData(cls): # lint-amnesty, pylint: disable=super-method-not-called
"""
Create an empty course outline.
"""
cls.course_key = CourseKey.from_string("course-v1:OpenEdX+Learn+Roundtrip")
cls.course_outline = CourseOutlineData(
course_key=cls.course_key,
title="PublicApiAvailableTestCase Test Course!",
published_at=datetime(2021, 6, 14, tzinfo=timezone.utc),
published_version="5ebece4b69dd593d82fe2021",
entrance_exam_id=None,
days_early_for_beta=None,
sections=[],
self_paced=False,
course_visibility=CourseVisibility.PRIVATE
)
replace_course_outline(cls.course_outline)
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.fake_course_1 = CourseKey.from_string("course-v1:Not+Really+Here")
cls.fake_course_2 = CourseKey.from_string("Also/Not/Here")
def test_flag_inactive(self):
# Old Mongo and non-existent courses are always unavailable
for user in [self.global_staff, self.student]:
assert not public_api_available(self.fake_course_1, user)
assert not public_api_available(self.fake_course_2, user)
# Since the waffle flag is off, only global staff can use the Learning
# Sequences API.
assert public_api_available(self.course_key, self.global_staff)
assert not public_api_available(self.course_key, self.student)
@override_waffle_flag(USE_FOR_OUTLINES, active=True)
def test_flag_active(self):
# Old Mongo and non-existent courses are always unavailable
for user in [self.global_staff, self.student]:
assert not public_api_available(self.fake_course_1, user)
assert not public_api_available(self.fake_course_2, user)
# Since the waffle flag is on, both global staff and students can use
# the Learning Sequences API.
assert public_api_available(self.course_key, self.global_staff)
assert public_api_available(self.course_key, self.student)
class CourseOutlineTestCase(CacheIsolationTestCase):
"""
Simple tests around reading and writing CourseOutlineData. No user info.

View File

@@ -0,0 +1,24 @@
"""
Rollout waffle flags for the learning_sequences API.
"""
from openedx.core.djangoapps.waffle_utils import CourseWaffleFlag
WAFFLE_NAMESPACE = 'learning_sequences'
# .. toggle_name: learning_sequences.use_for_outlines
# .. toggle_implementation: CourseWaffleFlag
# .. toggle_description: Waffle flag to enable the use of the Learning Sequences
# Course Outline API (/api/learning_sequences/v1/course_outline/{course_key}).
# Staff can always use this endpoint. If you are a student and this endpoint
# is not enabled, it will return a 403 error. The Courseware MFE should know
# how to detect this condition.
# This flag is also used to determine what is returned by the
# public_api_available learning_sequences API function, though other apps
# calling this API are always able to ignore this result and call any
# learning_sequences API directly (e.g. get_course_outline).
# .. toggle_default: False
# .. toggle_use_cases: temporary, open_edx
# .. toggle_creation_date: 2021-06-07
# .. toggle_target_removal_date: 2020-08-01
USE_FOR_OUTLINES = CourseWaffleFlag(WAFFLE_NAMESPACE, 'use_for_outlines', __name__)

View File

@@ -12,12 +12,12 @@ from edx_rest_framework_extensions.auth.session.authentication import SessionAut
from opaque_keys import InvalidKeyError
from opaque_keys.edx.keys import CourseKey
from rest_framework import serializers
from rest_framework.exceptions import NotFound
from rest_framework.exceptions import NotFound, PermissionDenied
from rest_framework.response import Response
from rest_framework.views import APIView
from openedx.core.lib.api.permissions import IsStaff
from .api import get_user_course_outline_details
from .api.permissions import can_call_public_api
from .data import CourseOutlineData
User = get_user_model()
@@ -31,9 +31,6 @@ class CourseOutlineView(APIView):
# We want to eventually allow unauthenticated users to use this as well...
authentication_classes = (JwtAuthentication, SessionAuthenticationAllowInactiveUser)
# For early testing, restrict this to only global staff...
permission_classes = (IsStaff,)
class UserCourseOutlineDataSerializer(serializers.BaseSerializer): # lint-amnesty, pylint: disable=abstract-method
"""
Read-only serializer for CourseOutlineData for this endpoint.
@@ -154,36 +151,35 @@ class CourseOutlineView(APIView):
"""
The CourseOutline, customized for a given user.
Currently restricted to global staff.
TODO: Swagger docs of API. For an exemplar to imitate, see:
https://github.com/edx/edx-platform/blob/master/lms/djangoapps/program_enrollments/rest_api/v1/views.py#L792-L820
"""
# Translate input params and do any substitutions...
# Translate input params and do course key validation (will cause HTTP
# 400 error if an invalid CourseKey was entered, instead of 404).
course_key = self._validate_course_key(course_key_str)
at_time = datetime.now(timezone.utc)
user = self._determine_user(request)
# Grab the user's outline and send our response...
if not can_call_public_api(request.user, course_key):
raise PermissionDenied()
try:
user_course_outline_details = get_user_course_outline_details(course_key, user, at_time)
# Grab the user's outline and send our response...
outline_user = self._determine_user(request)
user_course_outline_details = get_user_course_outline_details(course_key, outline_user, at_time)
except CourseOutlineData.DoesNotExist as does_not_exist_err:
raise NotFound() from does_not_exist_err
serializer = self.UserCourseOutlineDataSerializer(user_course_outline_details)
return Response(serializer.data)
def _validate_course_key(self, course_key_str): # lint-amnesty, pylint: disable=missing-function-docstring
def _validate_course_key(self, course_key_str):
"""Validate the Course Key and raise a ValidationError if it fails."""
try:
course_key = CourseKey.from_string(course_key_str)
except InvalidKeyError:
raise serializers.ValidationError( # lint-amnesty, pylint: disable=raise-missing-from
f"{course_key_str} is not a valid CourseKey"
)
except InvalidKeyError as err:
raise serializers.ValidationError(f"{course_key_str} is not a valid CourseKey") from err
if course_key.deprecated:
raise serializers.ValidationError(
"Deprecated CourseKeys (Org/Course/Run) are not supported."
)
raise serializers.ValidationError("Deprecated CourseKeys (Org/Course/Run) are not supported.")
return course_key
def _determine_user(self, request):
@@ -193,7 +189,20 @@ class CourseOutlineView(APIView):
until we have more full fledged permissions/masquerading.
"""
requested_username = request.GET.get("user")
if request.user.is_staff and requested_username:
return User.objects.get(username=requested_username)
return request.user
# Simple case: No username passed in, so it's just the request.user
if not requested_username:
return request.user
# If we're here, then the requesting user is asking for someone else's
# course outline. Right now, only global staff is allowed to do that.
if request.user.is_staff:
try:
user = User.objects.get(username=requested_username)
return user
except User.DoesNotExist as err:
raise NotFound("User {requested_username} does not exist.") from err
raise PermissionDenied(
"User {request.user.username} does not have permission to view course outline as {requested_username}"
)