From dddcadb70602d2beef8d3fcf8f69c8425370a8e2 Mon Sep 17 00:00:00 2001 From: Chris Deery <3932645+cdeery@users.noreply.github.com> Date: Tue, 11 Jan 2022 09:02:03 -0500 Subject: [PATCH] feat: [AA-922] remove deprecated Goals backend (#29705) * feat: [AA-922] remove deprecated Goals backend While the new Weekly Learning Goals were being rolled out, the previous goal setting feature still existed behind a waffle flag. The Weekly Learning Goals now become the one and only learning goal feature. This change does not remove the old goals feature from the legacy backend, and therefore it does not remove any of the data that was used by the old goals feature. The goals are now driven by the single pre-existing Waffle flag ENABLE_COURSE_GOALS - Removed COURSE_GOALS_NUMBER_OF_DAYS_GOALS waffle flag, replacing it where needed with the existing ENABLE_COURSE_GOALS - modified the API to remove the old goal_options, keeping the redundant weekly_learning_goal_enabled flag - updated tests - refactor tests to fit 50 line limit in lint --- .../commands/goal_reminder_email.py | 4 +- .../tests/test_goal_reminder_email.py | 10 +- lms/djangoapps/course_goals/models.py | 11 +- .../course_goals/tests/test_user_activity.py | 4 +- lms/djangoapps/course_goals/toggles.py | 21 --- .../course_home_api/outline/serializers.py | 1 - .../outline/tests/test_goals.py | 2 - .../outline/tests/test_view.py | 39 +---- .../course_home_api/outline/views.py | 83 +++-------- .../mobile_api/course_info/tests.py | 6 +- .../mobile_api/course_info/views.py | 5 +- .../courseware_api/tests/test_views.py | 138 ++++++++++-------- .../core/djangoapps/courseware_api/views.py | 47 +++--- 13 files changed, 140 insertions(+), 231 deletions(-) delete mode 100644 lms/djangoapps/course_goals/toggles.py diff --git a/lms/djangoapps/course_goals/management/commands/goal_reminder_email.py b/lms/djangoapps/course_goals/management/commands/goal_reminder_email.py index 78054edb3a..9ba82b7602 100644 --- a/lms/djangoapps/course_goals/management/commands/goal_reminder_email.py +++ b/lms/djangoapps/course_goals/management/commands/goal_reminder_email.py @@ -16,7 +16,6 @@ from lms.djangoapps.certificates.api import get_certificate_for_user_id from lms.djangoapps.certificates.data import CertificateStatuses from lms.djangoapps.courseware.context_processor import get_user_timezone_or_last_seen_timezone_or_utc from lms.djangoapps.course_goals.models import CourseGoal, CourseGoalReminderStatus, UserActivity -from lms.djangoapps.course_goals.toggles import COURSE_GOALS_NUMBER_OF_DAYS_GOALS from openedx.core.djangoapps.ace_common.template_context import get_base_template_context from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.djangoapps.lang_pref import LANGUAGE_KEY @@ -24,6 +23,7 @@ from openedx.core.djangoapps.site_configuration import helpers as configuration_ from openedx.core.djangoapps.user_api.preferences.api import get_user_preference from openedx.core.lib.celery.task_utils import emulate_http_request from openedx.features.course_duration_limits.access import get_user_course_expiration_date +from openedx.features.course_experience import ENABLE_COURSE_GOALS from openedx.features.course_experience.url_helpers import get_learning_mfe_home_url log = logging.getLogger(__name__) @@ -145,7 +145,7 @@ class Command(BaseCommand): @staticmethod def handle_goal(goal, today, sunday_date, monday_date): """Sends an email reminder for a single CourseGoal, if it passes all our checks""" - if not COURSE_GOALS_NUMBER_OF_DAYS_GOALS.is_enabled(goal.course_key): + if not ENABLE_COURSE_GOALS.is_enabled(goal.course_key): return False enrollment = CourseEnrollment.get_enrollment(goal.user, goal.course_key, select_related=['course']) diff --git a/lms/djangoapps/course_goals/management/commands/tests/test_goal_reminder_email.py b/lms/djangoapps/course_goals/management/commands/tests/test_goal_reminder_email.py index 487418b0e2..68372be5eb 100644 --- a/lms/djangoapps/course_goals/management/commands/tests/test_goal_reminder_email.py +++ b/lms/djangoapps/course_goals/management/commands/tests/test_goal_reminder_email.py @@ -17,10 +17,10 @@ from lms.djangoapps.course_goals.models import CourseGoalReminderStatus from lms.djangoapps.course_goals.tests.factories import ( CourseGoalFactory, CourseGoalReminderStatusFactory, UserActivityFactory, ) -from lms.djangoapps.course_goals.toggles import COURSE_GOALS_NUMBER_OF_DAYS_GOALS from lms.djangoapps.certificates.data import CertificateStatuses from lms.djangoapps.certificates.tests.factories import GeneratedCertificateFactory from openedx.core.djangolib.testing.utils import skip_unless_lms +from openedx.features.course_experience import ENABLE_COURSE_GOALS # Some constants just for clarity of tests (assuming week starts on a Monday, as March 2021 used below does) MONDAY = 0 @@ -34,7 +34,7 @@ SUNDAY = 6 @ddt.ddt @skip_unless_lms -@override_waffle_flag(COURSE_GOALS_NUMBER_OF_DAYS_GOALS, active=True) +@override_waffle_flag(ENABLE_COURSE_GOALS, active=True) class TestGoalReminderEmailCommand(TestCase): """ Test goal_reminder_email management command. @@ -117,15 +117,15 @@ class TestGoalReminderEmailCommand(TestCase): def test_feature_disabled(self): self.make_valid_goal() - with override_waffle_flag(COURSE_GOALS_NUMBER_OF_DAYS_GOALS, active=False): + with override_waffle_flag(ENABLE_COURSE_GOALS, active=False): self.call_command(expect_sent=False) def test_feature_enabled_for_user(self): goal = self.make_valid_goal() - with override_waffle_flag(COURSE_GOALS_NUMBER_OF_DAYS_GOALS, active=None): + with override_waffle_flag(ENABLE_COURSE_GOALS, active=None): # We want to ensure that when we set up a fake request # it works correctly if the flag is only enabled for specific users - flag = get_waffle_flag_model().get(COURSE_GOALS_NUMBER_OF_DAYS_GOALS.name) + flag = get_waffle_flag_model().get(ENABLE_COURSE_GOALS.name) flag.users.add(goal.user) self.call_command(expect_sent=True) diff --git a/lms/djangoapps/course_goals/models.py b/lms/djangoapps/course_goals/models.py index b56b19e15f..f2dfc8e4ef 100644 --- a/lms/djangoapps/course_goals/models.py +++ b/lms/djangoapps/course_goals/models.py @@ -16,9 +16,9 @@ from opaque_keys.edx.django.models import CourseKeyField from simple_history.models import HistoricalRecords from lms.djangoapps.courseware.masquerade import is_masquerading -from lms.djangoapps.course_goals.toggles import COURSE_GOALS_NUMBER_OF_DAYS_GOALS from lms.djangoapps.courseware.context_processor import get_user_timezone_or_last_seen_timezone_or_utc from openedx.core.lib.mobile_utils import is_request_from_mobile_app +from openedx.features.course_experience import ENABLE_COURSE_GOALS # Each goal is represented by a goal key and a string description. GOAL_KEY_CHOICES = Choices( @@ -126,7 +126,7 @@ class UserActivity(models.Model): The return value is the id of the object that was created, or retrieved. A return value of None signifies that a user activity record was not stored or retrieved ''' - if not COURSE_GOALS_NUMBER_OF_DAYS_GOALS.is_enabled(course_key): + if not ENABLE_COURSE_GOALS.is_enabled(course_key): return None if not (user and user.id) or not course_key: @@ -146,13 +146,6 @@ class UserActivity(models.Model): cached_value = TieredCache.get_cached_response(cache_key) if cached_value.is_found: - # Temporary debugging log for testing mobile app connection - if request: - log.info( - 'Retrieved cached value with request {} for user and course combination {} {}'.format( - str(request.build_absolute_uri()), str(user.id), str(course_key) - ) - ) return cached_value.value, False activity_object, __ = cls.objects.get_or_create(user=user, course_key=course_key, date=date) diff --git a/lms/djangoapps/course_goals/tests/test_user_activity.py b/lms/djangoapps/course_goals/tests/test_user_activity.py index 74d1853e0b..6e6733b928 100644 --- a/lms/djangoapps/course_goals/tests/test_user_activity.py +++ b/lms/djangoapps/course_goals/tests/test_user_activity.py @@ -19,8 +19,8 @@ from common.djangoapps.student.models import CourseEnrollment from common.djangoapps.student.tests.factories import UserFactory from common.djangoapps.util.testing import UrlResetMixin from lms.djangoapps.course_goals.models import UserActivity -from lms.djangoapps.course_goals.toggles import COURSE_GOALS_NUMBER_OF_DAYS_GOALS from openedx.core.djangoapps.django_comment_common.models import ForumsConfig +from openedx.features.course_experience import ENABLE_COURSE_GOALS from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, TEST_DATA_SPLIT_MODULESTORE # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory # lint-amnesty, pylint: disable=wrong-import-order @@ -28,7 +28,7 @@ User = get_user_model() @ddt.ddt -@override_waffle_flag(COURSE_GOALS_NUMBER_OF_DAYS_GOALS, active=True) +@override_waffle_flag(ENABLE_COURSE_GOALS, active=True) class UserActivityTests(UrlResetMixin, ModuleStoreTestCase): """ Testing Course Goals User Activity diff --git a/lms/djangoapps/course_goals/toggles.py b/lms/djangoapps/course_goals/toggles.py deleted file mode 100644 index c20bfcc0e9..0000000000 --- a/lms/djangoapps/course_goals/toggles.py +++ /dev/null @@ -1,21 +0,0 @@ -""" -Toggles for course goals -""" - -from edx_toggles.toggles import LegacyWaffleFlagNamespace - -from openedx.core.djangoapps.waffle_utils import CourseWaffleFlag - -WAFFLE_FLAG_NAMESPACE = LegacyWaffleFlagNamespace(name='course_goals') - -# .. toggle_name: course_goals.number_of_days_goals -# .. toggle_implementation: CourseWaffleFlag -# .. toggle_default: False -# .. toggle_description: This flag enables the use of the new version of course goals where users -# .. set a goal for the number of days they want to learn -# .. toggle_warnings: None -# .. toggle_use_cases: temporary -# .. toggle_creation_date: 2021-07-27 -# .. toggle_target_removal_date: 2021-09-01 -# .. toggle_tickets: https://openedx.atlassian.net/browse/AA-859 -COURSE_GOALS_NUMBER_OF_DAYS_GOALS = CourseWaffleFlag(WAFFLE_FLAG_NAMESPACE, 'number_of_days_goals', __name__) diff --git a/lms/djangoapps/course_home_api/outline/serializers.py b/lms/djangoapps/course_home_api/outline/serializers.py index 63c059c160..922a64b494 100644 --- a/lms/djangoapps/course_home_api/outline/serializers.py +++ b/lms/djangoapps/course_home_api/outline/serializers.py @@ -66,7 +66,6 @@ class CourseGoalsSerializer(serializers.Serializer): """ Serializer for Course Goal data """ - goal_options = serializers.ListField(default=[]) selected_goal = serializers.DictField() weekly_learning_goal_enabled = serializers.BooleanField(default=False) diff --git a/lms/djangoapps/course_home_api/outline/tests/test_goals.py b/lms/djangoapps/course_home_api/outline/tests/test_goals.py index 999fb983ed..aa7a97585f 100644 --- a/lms/djangoapps/course_home_api/outline/tests/test_goals.py +++ b/lms/djangoapps/course_home_api/outline/tests/test_goals.py @@ -15,7 +15,6 @@ from common.djangoapps.student.models import CourseEnrollment from common.djangoapps.student.tests.factories import UserFactory from edx_toggles.toggles.testutils import override_waffle_flag # lint-amnesty, pylint: disable=wrong-import-order from lms.djangoapps.course_goals.models import CourseGoal -from lms.djangoapps.course_goals.toggles import COURSE_GOALS_NUMBER_OF_DAYS_GOALS from lms.djangoapps.course_home_api.tests.utils import BaseCourseHomeTests from openedx.features.course_experience import ENABLE_COURSE_GOALS from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase # lint-amnesty, pylint: disable=wrong-import-order @@ -28,7 +27,6 @@ User = get_user_model() @override_waffle_flag(ENABLE_COURSE_GOALS, active=True) -@override_waffle_flag(COURSE_GOALS_NUMBER_OF_DAYS_GOALS, active=True) class TestCourseGoalsAPI(SharedModuleStoreTestCase): """ Testing the Course Goals API. 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 3e0ab93856..6df555304e 100644 --- a/lms/djangoapps/course_home_api/outline/tests/test_view.py +++ b/lms/djangoapps/course_home_api/outline/tests/test_view.py @@ -19,7 +19,6 @@ from common.djangoapps.student.roles import CourseInstructorRole 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_USE_LEGACY_FRONTEND -from lms.djangoapps.course_goals.toggles import COURSE_GOALS_NUMBER_OF_DAYS_GOALS 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 @@ -54,16 +53,6 @@ class OutlineTabTestViews(BaseCourseHomeTests): response = self.client.get(self.url) assert response.status_code == 200 - course_goals = response.data.get('course_goals') - goal_options = course_goals['goal_options'] - if enrollment_mode == CourseMode.VERIFIED: - assert goal_options == [] - else: - assert len(goal_options) > 0 - - selected_goal = course_goals['selected_goal'] - assert selected_goal is None - course_tools = response.data.get('course_tools') assert course_tools assert course_tools[0]['analytics_id'] == 'edx.bookmarks' @@ -88,9 +77,6 @@ class OutlineTabTestViews(BaseCourseHomeTests): response = self.client.get(self.url) assert response.status_code == 200 - course_goals = response.data.get('course_goals') - assert course_goals['goal_options'] == [] - course_tools = response.data.get('course_tools') assert len(course_tools) == 0 @@ -221,26 +207,6 @@ class OutlineTabTestViews(BaseCourseHomeTests): assert response.data['access_expiration']['expiration_date'] == deadline @override_waffle_flag(ENABLE_COURSE_GOALS, active=True) - def test_post_course_goal_deprecated(self): - CourseEnrollment.enroll(self.user, self.course.id, CourseMode.AUDIT) - - post_data = { - 'course_id': self.course.id, - 'goal_key': 'certify' - } - post_course_goal_response = self.client.post(reverse('course-home:save-course-goal'), post_data) - assert post_course_goal_response.status_code == 200 - - response = self.client.get(self.url) - assert response.status_code == 200 - - course_goals = response.data.get('course_goals') - selected_goal = course_goals['selected_goal'] - assert selected_goal is not None - assert selected_goal['key'] == 'certify' - - @override_waffle_flag(ENABLE_COURSE_GOALS, active=True) - @override_waffle_flag(COURSE_GOALS_NUMBER_OF_DAYS_GOALS, active=True) def test_post_course_goal(self): """ Test that the api returns the correct response when saving a goal """ CourseEnrollment.enroll(self.user, self.course.id, CourseMode.AUDIT) @@ -262,12 +228,11 @@ class OutlineTabTestViews(BaseCourseHomeTests): course_goals = response.json()['course_goals'] expected_course_goals = { - 'goal_options': [], 'selected_goal': { 'days_per_week': 1, - 'subscribed_to_reminders': True + 'subscribed_to_reminders': True, }, - 'weekly_learning_goal_enabled': True + 'weekly_learning_goal_enabled': True, } assert course_goals == expected_course_goals diff --git a/lms/djangoapps/course_home_api/outline/views.py b/lms/djangoapps/course_home_api/outline/views.py index cddf939ded..df031e259a 100644 --- a/lms/djangoapps/course_home_api/outline/views.py +++ b/lms/djangoapps/course_home_api/outline/views.py @@ -26,14 +26,9 @@ from common.djangoapps.student.models import CourseEnrollment from common.djangoapps.util.views import expose_header from lms.djangoapps.course_goals.api import ( add_course_goal, - add_course_goal_deprecated, get_course_goal, - get_course_goal_text, - has_course_goal_permission, - valid_course_goals_ordered ) from lms.djangoapps.course_goals.models import CourseGoal -from lms.djangoapps.course_goals.toggles import COURSE_GOALS_NUMBER_OF_DAYS_GOALS from lms.djangoapps.course_home_api.outline.serializers import OutlineTabSerializer from lms.djangoapps.course_home_api.toggles import ( course_home_legacy_is_active, @@ -113,6 +108,7 @@ class OutlineTabView(RetrieveAPIView): selected_goal: days_per_week: (int) The number of days the learner wants to learn per week subscribed_to_reminders: (bool) Whether the learner wants email reminders about their goal + weekly_learning_goal_enabled: Flag indicating if this feature is enabled for this call course_tools: List of serialized Course Tool objects. Each serialization has the following fields: analytics_id: (str) The unique id given to the tool. title: (str) The display title of the tool. @@ -214,8 +210,8 @@ class OutlineTabView(RetrieveAPIView): cert_data = None course_blocks = None course_goals = { - 'goal_options': [], - 'selected_goal': None + 'selected_goal': None, + 'weekly_learning_goal_enabled': False, } course_tools = CourseToolsPluginManager.get_enabled_course_tools(request, course_key) dates_widget = { @@ -253,36 +249,14 @@ class OutlineTabView(RetrieveAPIView): enable_proctored_exams = course_overview.enable_proctored_exams - if COURSE_GOALS_NUMBER_OF_DAYS_GOALS.is_enabled(): - if (is_enrolled and ENABLE_COURSE_GOALS.is_enabled(course_key)): - - course_goals = { - 'selected_goal': None, - 'weekly_learning_goal_enabled': True, + if (is_enrolled and ENABLE_COURSE_GOALS.is_enabled(course_key)): + course_goals['weekly_learning_goal_enabled'] = True + selected_goal = get_course_goal(request.user, course_key) + if selected_goal: + course_goals['selected_goal'] = { + 'days_per_week': selected_goal.days_per_week, + 'subscribed_to_reminders': selected_goal.subscribed_to_reminders, } - selected_goal = get_course_goal(request.user, course_key) - if selected_goal: - course_goals['selected_goal'] = { - 'days_per_week': selected_goal.days_per_week, - 'subscribed_to_reminders': selected_goal.subscribed_to_reminders, - } - else: - # Only show the set course goal message for enrolled, unverified - # users in a course that allows for verified statuses. - is_already_verified = CourseEnrollment.is_enrolled_as_verified(request.user, course_key) - if not is_already_verified and has_course_goal_permission(request, course_key_string, - {'is_enrolled': is_enrolled}): - course_goals = { - 'goal_options': valid_course_goals_ordered(include_unsure=True), - 'selected_goal': None - } - - selected_goal = get_course_goal(request.user, course_key) - if selected_goal: - course_goals['selected_goal'] = { - 'key': selected_goal.goal_key, - 'text': get_course_goal_text(selected_goal.goal_key), - } try: resume_block = get_key_to_last_completed_block(request.user, course.id) @@ -423,33 +397,18 @@ def save_course_goal(request): # pylint: disable=missing-function-docstring if not course_id: raise ParseError("'course_id' is required.") - if COURSE_GOALS_NUMBER_OF_DAYS_GOALS.is_enabled(): - # If body doesn't contain the required goals fields, return 400 to client. - if subscribed_to_reminders is None: - raise ParseError("'subscribed_to_reminders' is required.") + # If body doesn't contain the required goals fields, return 400 to client. + if subscribed_to_reminders is None: + raise ParseError("'subscribed_to_reminders' is required.") - try: - add_course_goal(request.user, course_id, subscribed_to_reminders, days_per_week) - return Response({ - 'header': _('Your course goal has been successfully set.'), - 'message': _('Course goal updated successfully.'), - }) - except Exception: - raise UnableToSaveCourseGoal # pylint: disable=raise-missing-from - - else: - # If body doesn't contain 'goal', return 400 to client. - if not goal_key: - raise ParseError("'goal_key' is required.") - - try: - add_course_goal_deprecated(request.user, course_id, goal_key) - return Response({ - 'header': _('Your course goal has been successfully set.'), - 'message': _('Course goal updated successfully.'), - }) - except Exception: - raise UnableToSaveCourseGoal # pylint: disable=raise-missing-from + try: + add_course_goal(request.user, course_id, subscribed_to_reminders, days_per_week) + return Response({ + 'header': _('Your course goal has been successfully set.'), + 'message': _('Course goal updated successfully.'), + }) + except Exception as exc: + raise UnableToSaveCourseGoal from exc @api_view(['POST']) diff --git a/lms/djangoapps/mobile_api/course_info/tests.py b/lms/djangoapps/mobile_api/course_info/tests.py index c6c0a17c9f..1a02d93dbf 100644 --- a/lms/djangoapps/mobile_api/course_info/tests.py +++ b/lms/djangoapps/mobile_api/course_info/tests.py @@ -13,9 +13,9 @@ from rest_framework.test import APIClient # pylint: disable=unused-import from common.djangoapps.student.models import CourseEnrollment # pylint: disable=unused-import from common.djangoapps.student.tests.factories import UserFactory # pylint: disable=unused-import -from lms.djangoapps.course_goals.toggles import COURSE_GOALS_NUMBER_OF_DAYS_GOALS from lms.djangoapps.mobile_api.testutils import MobileAPITestCase, MobileAuthTestMixin, MobileCourseAccessTestMixin from lms.djangoapps.mobile_api.utils import API_V1, API_V05 +from openedx.features.course_experience import ENABLE_COURSE_GOALS from xmodule.html_module import CourseInfoBlock # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore import ModuleStoreEnum # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order @@ -230,7 +230,7 @@ class TestHandouts(MobileAPITestCase, MobileAuthTestMixin, MobileCourseAccessTes self.login_and_enroll() -@override_waffle_flag(COURSE_GOALS_NUMBER_OF_DAYS_GOALS, active=True) +@override_waffle_flag(ENABLE_COURSE_GOALS, active=True) class TestCourseGoalsUserActivityAPI(MobileAPITestCase, SharedModuleStoreTestCase): """ Testing the Course Goals User Activity API. @@ -280,7 +280,7 @@ class TestCourseGoalsUserActivityAPI(MobileAPITestCase, SharedModuleStoreTestCas response = self.client.post(self.apiUrl, post_data) assert response.status_code == 400 - @override_waffle_flag(COURSE_GOALS_NUMBER_OF_DAYS_GOALS, active=False) + @override_waffle_flag(ENABLE_COURSE_GOALS, active=False) @patch('lms.djangoapps.mobile_api.course_info.views.log') def test_flag_disabled(self, mock_logger): ''' diff --git a/lms/djangoapps/mobile_api/course_info/views.py b/lms/djangoapps/mobile_api/course_info/views.py index e12bb953fd..fb0f91cc59 100644 --- a/lms/djangoapps/mobile_api/course_info/views.py +++ b/lms/djangoapps/mobile_api/course_info/views.py @@ -14,9 +14,8 @@ from rest_framework.views import APIView from common.djangoapps.static_replace import make_static_urls_absolute from lms.djangoapps.courseware.courses import get_course_info_section_module from lms.djangoapps.course_goals.models import UserActivity -from lms.djangoapps.course_goals.toggles import COURSE_GOALS_NUMBER_OF_DAYS_GOALS from openedx.core.lib.xblock_utils import get_course_update_items - +from openedx.features.course_experience import ENABLE_COURSE_GOALS from ..decorators import mobile_course_access, mobile_view User = get_user_model() @@ -157,7 +156,7 @@ class CourseGoalsRecordUserActivity(APIView): status=status.HTTP_400_BAD_REQUEST, ) - if not COURSE_GOALS_NUMBER_OF_DAYS_GOALS.is_enabled(course_key): + if not ENABLE_COURSE_GOALS.is_enabled(course_key): log.warning('For this mobile request, user activity is not enabled for this user {} and course {}'.format( str(user_id), str(course_key)) ) diff --git a/openedx/core/djangoapps/courseware_api/tests/test_views.py b/openedx/core/djangoapps/courseware_api/tests/test_views.py index af3ae97145..0976e89a3a 100644 --- a/openedx/core/djangoapps/courseware_api/tests/test_views.py +++ b/openedx/core/djangoapps/courseware_api/tests/test_views.py @@ -127,19 +127,16 @@ class CourseApiTestViews(BaseCoursewareTests, MasqueradeMixin): ) @ddt.data( - (True, None, ACCESS_DENIED), - (True, 'audit', ACCESS_DENIED), - (True, 'verified', ACCESS_DENIED), - (False, None, ACCESS_DENIED), - (False, None, ACCESS_GRANTED), + (True, 'audit'), + (True, 'verified'), ) @ddt.unpack @mock.patch.dict('django.conf.settings.FEATURES', {'CERTIFICATES_HTML_VIEW': True}) @mock.patch('openedx.core.djangoapps.courseware_api.views.CoursewareMeta.is_microfrontend_enabled_for_user') - def test_course_metadata(self, logged_in, enrollment_mode, enable_anonymous, is_microfrontend_enabled_for_user): + def test_enrolled_course_metadata(self, logged_in, enrollment_mode, is_microfrontend_enabled_for_user): is_microfrontend_enabled_for_user.return_value = True check_public_access = mock.Mock() - check_public_access.return_value = enable_anonymous + check_public_access.return_value = ACCESS_DENIED with mock.patch('lms.djangoapps.courseware.access_utils.check_public_access', check_public_access): if not logged_in: self.client.logout() @@ -155,68 +152,89 @@ class CourseApiTestViews(BaseCoursewareTests, MasqueradeMixin): response = self.client.get(self.url) assert response.status_code == 200 - if enrollment_mode: - enrollment = response.data['enrollment'] - assert enrollment_mode == enrollment['mode'] - assert enrollment['is_active'] - assert len(response.data['tabs']) == 5 - found = False - for tab in response.data['tabs']: - if tab['type'] == 'external_link': - assert tab['url'] != 'http://hidden.com', "Hidden tab is not hidden" - if tab['url'] == 'http://zombo.com': - found = True - assert found, 'external link not in course tabs' - assert not response.data['user_has_passing_grade'] + enrollment = response.data['enrollment'] + assert enrollment_mode == enrollment['mode'] + assert enrollment['is_active'] + assert len(response.data['tabs']) == 5 + found = False + for tab in response.data['tabs']: + if tab['type'] == 'external_link': + assert tab['url'] != 'http://hidden.com', "Hidden tab is not hidden" + if tab['url'] == 'http://zombo.com': + found = True + assert found, 'external link not in course tabs' - # This import errors in cms if it is imported at the top level - from lms.djangoapps.course_goals.api import get_course_goal - selected_goal = get_course_goal(self.user, self.course.id) - if selected_goal: - assert response.data['course_goals']['selected_goal'] == { - 'days_per_week': selected_goal.days_per_week, - 'subscribed_to_reminders': selected_goal.subscribed_to_reminders, - } + assert not response.data['user_has_passing_grade'] - if enrollment_mode == 'audit': - assert response.data['verify_identity_url'] is None - assert response.data['verification_status'] == 'none' # lint-amnesty, pylint: disable=literal-comparison - assert response.data['linkedin_add_to_profile_url'] is None - else: - assert response.data['certificate_data']['cert_status'] == 'earned_but_not_available' - expected_verify_identity_url = IDVerificationService.get_verify_location( - course_id=self.course.id + # This import errors in cms if it is imported at the top level + from lms.djangoapps.course_goals.api import get_course_goal + selected_goal = get_course_goal(self.user, self.course.id) + if selected_goal: + assert response.data['course_goals']['selected_goal'] == { + 'days_per_week': selected_goal.days_per_week, + 'subscribed_to_reminders': selected_goal.subscribed_to_reminders, + } + + if enrollment_mode == 'audit': + assert response.data['verify_identity_url'] is None + assert response.data['verification_status'] == 'none' # lint-amnesty, pylint: disable=literal-comparison + assert response.data['linkedin_add_to_profile_url'] is None + else: + assert response.data['certificate_data']['cert_status'] == 'earned_but_not_available' + expected_verify_identity_url = IDVerificationService.get_verify_location( + course_id=self.course.id + ) + # The response contains an absolute URL so this is only checking the path of the final + assert expected_verify_identity_url in response.data['verify_identity_url'] + assert response.data['verification_status'] == 'none' # lint-amnesty, pylint: disable=literal-comparison + + request = RequestFactory().request() + cert_url = get_certificate_url(course_id=self.course.id, uuid=cert.verify_uuid) + linkedin_url_params = { + 'name': '{platform_name} Verified Certificate for {course_name}'.format( + platform_name=settings.PLATFORM_NAME, course_name=self.course.display_name, + ), + 'certUrl': request.build_absolute_uri(cert_url), + # default value from the LinkedInAddToProfileConfigurationFactory company_identifier + 'organizationId': 1337, + 'certId': cert.verify_uuid, + 'issueYear': cert.created_date.year, + 'issueMonth': cert.created_date.month, + } + expected_linkedin_url = ( + 'https://www.linkedin.com/profile/add?startTask=CERTIFICATION_NAME&{params}'.format( + params=urlencode(linkedin_url_params) ) - # The response contains an absolute URL so this is only checking the path of the final - assert expected_verify_identity_url in response.data['verify_identity_url'] - assert response.data['verification_status'] == 'none' # lint-amnesty, pylint: disable=literal-comparison + ) + assert response.data['linkedin_add_to_profile_url'] == expected_linkedin_url - request = RequestFactory().request() - cert_url = get_certificate_url(course_id=self.course.id, uuid=cert.verify_uuid) - linkedin_url_params = { - 'name': '{platform_name} Verified Certificate for {course_name}'.format( - platform_name=settings.PLATFORM_NAME, course_name=self.course.display_name, - ), - 'certUrl': request.build_absolute_uri(cert_url), - # default value from the LinkedInAddToProfileConfigurationFactory company_identifier - 'organizationId': 1337, - 'certId': cert.verify_uuid, - 'issueYear': cert.created_date.year, - 'issueMonth': cert.created_date.month, - } - expected_linkedin_url = ( - 'https://www.linkedin.com/profile/add?startTask=CERTIFICATION_NAME&{params}'.format( - params=urlencode(linkedin_url_params) - ) - ) - assert response.data['linkedin_add_to_profile_url'] == expected_linkedin_url - elif enable_anonymous and not logged_in: + @ddt.data( + (True, ACCESS_DENIED), + (False, ACCESS_DENIED), + (False, ACCESS_GRANTED), + ) + @ddt.unpack + @mock.patch.dict('django.conf.settings.FEATURES', {'CERTIFICATES_HTML_VIEW': True}) + @mock.patch('openedx.core.djangoapps.courseware_api.views.CoursewareMeta.is_microfrontend_enabled_for_user') + def test_unenrolled_course_metadata(self, logged_in, enable_anonymous, is_microfrontend_enabled_for_user): + is_microfrontend_enabled_for_user.return_value = True + check_public_access = mock.Mock() + check_public_access.return_value = enable_anonymous + with mock.patch('lms.djangoapps.courseware.access_utils.check_public_access', check_public_access): + if not logged_in: + self.client.logout() + + response = self.client.get(self.url) + assert response.status_code == 200 + + if enable_anonymous and not logged_in: # multiple checks use this handler check_public_access.assert_called() assert response.data['enrollment']['mode'] is None assert response.data['course_access']['has_access'] - assert response.data['course_goals'] is None + assert response.data['course_goals']['selected_goal'] is None + assert response.data['course_goals']['weekly_learning_goal_enabled'] is False else: assert not response.data['course_access']['has_access'] diff --git a/openedx/core/djangoapps/courseware_api/views.py b/openedx/core/djangoapps/courseware_api/views.py index a5dc501194..aa5831f542 100644 --- a/openedx/core/djangoapps/courseware_api/views.py +++ b/openedx/core/djangoapps/courseware_api/views.py @@ -25,7 +25,6 @@ from lms.djangoapps.certificates.models import GeneratedCertificate from lms.djangoapps.course_api.api import course_detail from lms.djangoapps.course_goals.models import UserActivity from lms.djangoapps.course_goals.api import get_course_goal -from lms.djangoapps.course_goals.toggles import COURSE_GOALS_NUMBER_OF_DAYS_GOALS from lms.djangoapps.courseware.access import has_access from lms.djangoapps.courseware.access_response import ( CoursewareMicrofrontendDisabledAccessError, @@ -228,20 +227,20 @@ class CoursewareMeta: """ Returns a dict of course goals """ - if COURSE_GOALS_NUMBER_OF_DAYS_GOALS.is_enabled(): - course_goals = { - 'goal_options': [], - 'selected_goal': None - } - user_is_enrolled = CourseEnrollment.is_enrolled(self.effective_user, self.course_key) - if (user_is_enrolled and ENABLE_COURSE_GOALS.is_enabled(self.course_key)): - selected_goal = get_course_goal(self.effective_user, self.course_key) - if selected_goal: - course_goals['selected_goal'] = { - 'days_per_week': selected_goal.days_per_week, - 'subscribed_to_reminders': selected_goal.subscribed_to_reminders, - } - return course_goals + course_goals = { + 'selected_goal': None, + 'weekly_learning_goal_enabled': False, + } + user_is_enrolled = CourseEnrollment.is_enrolled(self.effective_user, self.course_key) + if (user_is_enrolled and ENABLE_COURSE_GOALS.is_enabled(self.course_key)): + course_goals['weekly_learning_goal_enabled'] = True + selected_goal = get_course_goal(self.effective_user, self.course_key) + if selected_goal: + course_goals['selected_goal'] = { + 'days_per_week': selected_goal.days_per_week, + 'subscribed_to_reminders': selected_goal.subscribed_to_reminders, + } + return course_goals @property def user_has_passing_grade(self): @@ -408,10 +407,11 @@ class CoursewareInformation(RetrieveAPIView): * masquerading_expired_course: (bool) Whether this course is expired for the masqueraded user * upgrade_deadline: (str) Last chance to upgrade, in ISO 8601 notation (or None if can't upgrade anymore) * upgrade_url: (str) Upgrade linke (or None if can't upgrade anymore) - course_goals: - selected_goal: - days_per_week: (int) The number of days the learner wants to learn per week - subscribed_to_reminders: (bool) Whether the learner wants email reminders about their goal + * course_goals: + * selected_goal: + * days_per_week: (int) The number of days the learner wants to learn per week + * subscribed_to_reminders: (bool) Whether the learner wants email reminders about their goal + * weekly_learning_goal_enabled: Flag indicating if this feature is enabled for this call * effort: A textual description of the weekly hours of effort expected in the course. * end: Date the course ends, in ISO 8601 notation @@ -596,9 +596,8 @@ class SequenceMetadata(DeveloperErrorViewMixin, APIView): """ try: usage_key = UsageKey.from_string(usage_key_string) - except InvalidKeyError: - raise NotFound(f"Invalid usage key: '{usage_key_string}'.") # lint-amnesty, pylint: disable=raise-missing-from - + except InvalidKeyError as exc: + raise NotFound(f"Invalid usage key: '{usage_key_string}'.") from exc _, request.user = setup_masquerade( request, usage_key.course_key, @@ -657,7 +656,7 @@ class Resume(DeveloperErrorViewMixin, APIView): JwtAuthentication, SessionAuthenticationAllowInactiveUser, ) - permission_classes = (IsAuthenticated, ) + permission_classes = (IsAuthenticated,) def get(self, request, course_key_string, *args, **kwargs): # lint-amnesty, pylint: disable=unused-argument """ @@ -711,7 +710,7 @@ class Celebration(DeveloperErrorViewMixin, APIView): BearerAuthenticationAllowInactiveUser, SessionAuthenticationAllowInactiveUser, ) - permission_classes = (IsAuthenticated, ) + permission_classes = (IsAuthenticated,) http_method_names = ['post'] def post(self, request, course_key_string, *args, **kwargs): # lint-amnesty, pylint: disable=unused-argument