diff --git a/lms/djangoapps/course_home_api/outline/tests/test_view.py b/lms/djangoapps/course_home_api/outline/tests/test_view.py index 6df555304e..2714f3ece5 100644 --- a/lms/djangoapps/course_home_api/outline/tests/test_view.py +++ b/lms/djangoapps/course_home_api/outline/tests/test_view.py @@ -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) diff --git a/lms/djangoapps/course_home_api/outline/views.py b/lms/djangoapps/course_home_api/outline/views.py index 703d55211b..f17d9b0f46 100644 --- a/lms/djangoapps/course_home_api/outline/views.py +++ b/lms/djangoapps/course_home_api/outline/views.py @@ -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) ) diff --git a/lms/djangoapps/course_home_api/tests/utils.py b/lms/djangoapps/course_home_api/tests/utils.py index f1d2731bb0..8edc270fa7 100644 --- a/lms/djangoapps/course_home_api/tests/utils.py +++ b/lms/djangoapps/course_home_api/tests/utils.py @@ -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() diff --git a/openedx/core/djangoapps/content/learning_sequences/api/__init__.py b/openedx/core/djangoapps/content/learning_sequences/api/__init__.py index bbede54893..2ab8283a6e 100644 --- a/openedx/core/djangoapps/content/learning_sequences/api/__init__.py +++ b/openedx/core/djangoapps/content/learning_sequences/api/__init__.py @@ -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, ) diff --git a/openedx/core/djangoapps/content/learning_sequences/api/outlines.py b/openedx/core/djangoapps/content/learning_sequences/api/outlines.py index cf8525b08b..800b085e50 100644 --- a/openedx/core/djangoapps/content/learning_sequences/api/outlines.py +++ b/openedx/core/djangoapps/content/learning_sequences/api/outlines.py @@ -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: """ diff --git a/openedx/core/djangoapps/content/learning_sequences/api/permissions.py b/openedx/core/djangoapps/content/learning_sequences/api/permissions.py index 747e9e6e17..6fafa1ca26 100644 --- a/openedx/core/djangoapps/content/learning_sequences/api/permissions.py +++ b/openedx/core/djangoapps/content/learning_sequences/api/permissions.py @@ -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: """ 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 476c98b0e9..9eb45e64ad 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 @@ -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. 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 c50e1cdff1..4cd8bf0c7d 100644 --- a/openedx/core/djangoapps/content/learning_sequences/tests/test_views.py +++ b/openedx/core/djangoapps/content/learning_sequences/tests/test_views.py @@ -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. diff --git a/openedx/core/djangoapps/content/learning_sequences/toggles.py b/openedx/core/djangoapps/content/learning_sequences/toggles.py deleted file mode 100644 index d40150b482..0000000000 --- a/openedx/core/djangoapps/content/learning_sequences/toggles.py +++ /dev/null @@ -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__) diff --git a/openedx/core/djangoapps/content/learning_sequences/views.py b/openedx/core/djangoapps/content/learning_sequences/views.py index c410592473..1ab79f64c9 100644 --- a/openedx/core/djangoapps/content/learning_sequences/views.py +++ b/openedx/core/djangoapps/content/learning_sequences/views.py @@ -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)