Merge pull request #29117 from openedx/mikix/drop-learning-sequence-flag

feat: enable using learning sequence outlines by default
This commit is contained in:
Michael Terry
2022-02-07 11:32:59 -05:00
committed by GitHub
10 changed files with 24 additions and 170 deletions

View File

@@ -13,6 +13,7 @@ from django.conf import settings # lint-amnesty, pylint: disable=wrong-import-o
from django.urls import reverse # lint-amnesty, pylint: disable=wrong-import-order
from edx_toggles.toggles.testutils import override_waffle_flag # lint-amnesty, pylint: disable=wrong-import-order
from cms.djangoapps.contentstore.outlines import update_outline_from_modulestore
from common.djangoapps.course_modes.models import CourseMode
from common.djangoapps.student.models import CourseEnrollment
from common.djangoapps.student.roles import CourseInstructorRole
@@ -21,7 +22,6 @@ from lms.djangoapps.course_home_api.tests.utils import BaseCourseHomeTests
from lms.djangoapps.course_home_api.toggles import COURSE_HOME_USE_LEGACY_FRONTEND
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
@@ -261,6 +261,7 @@ class OutlineTabTestViews(BaseCourseHomeTests):
is_onboarding_exam=False,
)
sequence.is_proctored_exam = True
update_outline_from_modulestore(course.id)
mock_summary.return_value = {
'short_description': 'My Exam',
'suggested_icon': 'fa-foo-bar',
@@ -289,6 +290,7 @@ class OutlineTabTestViews(BaseCourseHomeTests):
sequential2 = ItemFactory.create(display_name='Ungraded', category='sequential',
parent_location=chapter.location)
ItemFactory.create(category='problem', parent_location=sequential2.location)
update_outline_from_modulestore(course.id)
url = reverse('course-home:outline-tab', args=[course.id])
CourseEnrollment.enroll(self.user, course.id)
@@ -378,24 +380,22 @@ class OutlineTabTestViews(BaseCourseHomeTests):
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 # pylint: disable=protected-access
)
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
# With a course outline loaded, the same sequence is removed.
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 # pylint: disable=protected-access
)
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
def test_user_has_passing_grade(self):
CourseEnrollment.enroll(self.user, self.course.id)

View File

@@ -40,10 +40,7 @@ from lms.djangoapps.courseware.masquerade import is_masquerading, setup_masquera
from lms.djangoapps.courseware.toggles import course_is_invitation_only
from lms.djangoapps.courseware.views.views import get_cert_data
from lms.djangoapps.grades.course_grade_factory import CourseGradeFactory
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.learning_sequences.api import get_user_course_outline
from openedx.core.djangoapps.content.course_overviews.api import get_course_overview_or_404
from openedx.core.lib.api.authentication import BearerAuthenticationAllowInactiveUser
from openedx.features.course_duration_limits.access import get_access_expiration_data
@@ -294,7 +291,7 @@ class OutlineTabView(RetrieveAPIView):
#
# 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):
if course_blocks:
user_course_outline = get_user_course_outline(
course_key, request.user, datetime.now(tz=timezone.utc)
)

View File

@@ -9,6 +9,7 @@ from django.conf import settings
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
from cms.djangoapps.contentstore.outlines import update_outline_from_modulestore
from common.djangoapps.course_modes.models import CourseMode
from common.djangoapps.course_modes.tests.factories import CourseModeFactory
from lms.djangoapps.courseware.tests.helpers import MasqueradeMixin
@@ -48,6 +49,7 @@ class BaseCourseHomeTests(ModuleStoreTestCase, MasqueradeMixin):
VerificationDeadline.objects.create(course_key=self.course.id, deadline=datetime(2028, 1, 1))
CourseOverviewFactory.create(run='1T2020')
update_outline_from_modulestore(self.course.id)
self.staff_user = self.user
self.user, password = self.create_non_staff_user()

View File

@@ -6,6 +6,5 @@ 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_call_public_api, can_see_all_content
from .permissions import can_see_all_content
from .processors.content_gating import ContentGatingOutlineProcessor
from .processors.enrollment import EnrollmentOutlineProcessor
from .processors.enrollment_track_partition_groups import EnrollmentTrackPartitionGroupsOutlineProcessor
@@ -58,7 +58,6 @@ __all__ = [
'get_user_course_outline',
'get_user_course_outline_details',
'key_supports_outlines',
'public_api_available',
'replace_course_outline',
]
@@ -85,21 +84,6 @@ def key_supports_outlines(opaque_key: OpaqueKey) -> bool:
return False
def public_api_available(course_key: CourseKey) -> 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(course_key)
)
@function_trace('learning_sequences.api.get_course_keys_with_outlines')
def get_course_keys_with_outlines() -> QuerySet:
"""

View File

@@ -10,16 +10,6 @@ from opaque_keys.edx.keys import CourseKey
from lms.djangoapps.courseware.access import has_access
from openedx.core import types
from ..toggles import USE_FOR_OUTLINES
def can_call_public_api(course_key: CourseKey) -> bool:
"""
This is only intended for rollout purposes, and eventually everyone will be
able to call the public API for all courses.
"""
return USE_FOR_OUTLINES.is_enabled(course_key)
def can_see_all_content(requesting_user: types.User, course_key: CourseKey) -> bool:
"""

View File

@@ -16,7 +16,6 @@ from opaque_keys.edx.keys import CourseKey
from opaque_keys.edx.locator import LibraryLocator
import attr
import ddt
import django.test
import pytest
from openedx.core.djangolib.testing.utils import CacheIsolationTestCase
@@ -40,14 +39,12 @@ 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
@@ -67,56 +64,6 @@ class OutlineSupportTestCase(unittest.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 = UserFactory.create(
username='global_staff', email='gstaff@example.com', is_staff=True
)
cls.student = UserFactory.create(
username='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
assert not public_api_available(self.fake_course_1)
assert not public_api_available(self.fake_course_2)
# Waffle-flag controlled
assert not public_api_available(self.course_key)
@override_waffle_flag(USE_FOR_OUTLINES, active=True)
def test_flag_active(self):
# Old Mongo and non-existent courses are always unavailable
assert not public_api_available(self.fake_course_1)
assert not public_api_available(self.fake_course_2)
# Waffle-flag controlled
assert public_api_available(self.course_key)
class CourseOutlineTestCase(CacheIsolationTestCase):
"""
Simple tests around reading and writing CourseOutlineData. No user info.

View File

@@ -16,7 +16,6 @@ from this app, edx-when's set_dates_for_course).
from datetime import datetime, timezone
import ddt
from edx_toggles.toggles.testutils import override_waffle_flag
from opaque_keys.edx.keys import CourseKey
from rest_framework.test import APITestCase, APIClient
@@ -30,11 +29,9 @@ from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, skip_un
from ..api import replace_course_outline
from ..api.tests.test_data import generate_sections
from ..data import CourseOutlineData, CourseVisibility
from ..toggles import USE_FOR_OUTLINES
@skip_unless_lms
@override_waffle_flag(USE_FOR_OUTLINES, active=True)
class CourseOutlineViewTest(CacheIsolationTestCase, APITestCase):
"""
General tests for the CourseOutline.
@@ -122,23 +119,6 @@ class CourseOutlineViewTest(CacheIsolationTestCase, APITestCase):
assert len(data['outline']['sections'][1]['sequence_ids']) == 2
assert len(data['outline']['sequences']) == 4
@override_waffle_flag(USE_FOR_OUTLINES, active=False)
def test_override_rollout(self):
"""
Test that we can still access the API by sending force_on
This lets us manually test the outline rendering on live courses that
haven't been rolled out to yet.
TODO: Remove this test entirely after rollout.
"""
self.client.login(username='staff', password='staff_pass')
result = self.client.get(self.course_url)
assert result.status_code == 403
result = self.client.get(self.course_url, {'force_on': '1'})
assert result.status_code == 200
@ddt.ddt
@skip_unless_lms
@@ -203,7 +183,6 @@ class CourseOutlineViewTargetUserTest(CacheIsolationTestCase, APITestCase):
super().setUp()
self.client = APIClient()
@override_waffle_flag(USE_FOR_OUTLINES, active=True)
def test_global_staff(self):
"""Global staff can successfuly masquerade in both courses."""
self.client.login(username='global_staff', password='global_staff_pass')
@@ -212,7 +191,6 @@ class CourseOutlineViewTargetUserTest(CacheIsolationTestCase, APITestCase):
assert result.status_code == 200
assert result.data['username'] == 'student'
@override_waffle_flag(USE_FOR_OUTLINES, active=True)
@ddt.data(
('course_instructor', 'course_instructor_pass'),
('course_staff', 'course_staff_pass'),
@@ -243,7 +221,6 @@ class CourseOutlineViewTargetUserTest(CacheIsolationTestCase, APITestCase):
)
assert other_course_as_student.status_code == 403
@override_waffle_flag(USE_FOR_OUTLINES, active=True)
def test_student(self):
"""Students have no ability to masquerade."""
self.client.login(username='student', password='student_pass')
@@ -268,7 +245,6 @@ class CourseOutlineViewTargetUserTest(CacheIsolationTestCase, APITestCase):
@ddt.ddt
@skip_unless_lms
@override_waffle_flag(USE_FOR_OUTLINES, active=True)
class CourseOutlineViewMasqueradingTest(MasqueradeMixin, CacheIsolationTestCase):
"""
Tests permissions of session masquerading.

View File

@@ -1,25 +0,0 @@
"""
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}).
# If this endpoint is not enabled for a given course, it will return a 403
# error. The Courseware MFE should know how to detect this condition. To
# see the value of this API for a course before it has officially been rolled
# out for it, you can bypass this check by passing force_on=1 as a querystring
# parameter. 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

@@ -22,7 +22,6 @@ from openedx.core import types
from openedx.core.lib.api.view_utils import validate_course_key
from .api import get_user_course_outline_details
from .api.permissions import can_call_public_api
from .data import CourseOutlineData
User = get_user_model()
@@ -167,21 +166,6 @@ class CourseOutlineView(APIView):
# Get target user (and override request user for the benefit of any waffle checks)
request.user = self._determine_user(request, course_key)
# We use can_call_public_api to slowly roll this feature out, and be
# able to turn it off for a course. But it's not really a permissions
# thing in that it doesn't give them elevated access. If I had it to do
# over again, I'd call it something else, but all this code is supposed
# to go away when rollout is completed anyway.
#
# The force_on param just means, "Yeah, never mind whether you're turned
# on by default for the purposes of the MFE. I want to see production
# data using this API." The MFE should _never_ pass this parameter. It's
# just a way to peek at the API while it's techincally dark for rollout
# purposes. TODO: REMOVE THIS PARAM AFTER FULL ROLLOUT OF THIS FEATURE.
force_on = request.GET.get("force_on")
if (not force_on) and (not can_call_public_api(course_key)):
raise PermissionDenied()
try:
# Grab the user's outline and send our response...
user_course_outline_details = get_user_course_outline_details(course_key, request.user, at_time)