diff --git a/lms/djangoapps/course_api/blocks/views.py b/lms/djangoapps/course_api/blocks/views.py index ed7aa3b09e..45a7638e19 100644 --- a/lms/djangoapps/course_api/blocks/views.py +++ b/lms/djangoapps/course_api/blocks/views.py @@ -14,7 +14,6 @@ from rest_framework.generics import ListAPIView from rest_framework.response import Response from lms.djangoapps.course_goals.models import UserActivity -from lms.djangoapps.course_goals.toggles import RECORD_USER_ACTIVITY_FLAG from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, view_auth_classes from xmodule.modulestore.django import modulestore from xmodule.modulestore.exceptions import ItemNotFoundError @@ -311,9 +310,8 @@ class BlocksInCourseView(BlocksView): response = super().list(request, course_usage_key, hide_access_denials=hide_access_denials) - if RECORD_USER_ACTIVITY_FLAG.is_enabled(): - # Record user activity for tracking progress towards a user's course goals (for mobile app) - UserActivity.record_user_activity(request.user, course_key, request=request, only_if_mobile_app=True) + # Record user activity for tracking progress towards a user's course goals (for mobile app) + UserActivity.record_user_activity(request.user, course_key, request=request, only_if_mobile_app=True) calculate_completion = any('completion' in param for param in request.query_params.getlist('requested_fields', [])) diff --git a/lms/djangoapps/course_goals/models.py b/lms/djangoapps/course_goals/models.py index ac6dad98c1..448b9a2178 100644 --- a/lms/djangoapps/course_goals/models.py +++ b/lms/djangoapps/course_goals/models.py @@ -17,6 +17,7 @@ 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 openedx.core.djangoapps.user_api.preferences.api import get_user_preferences from openedx.core.lib.mobile_utils import is_request_from_mobile_app @@ -124,8 +125,11 @@ class UserActivity(models.Model): Once the only_if_mobile_app argument is removed the request argument can be removed as well. The return value is the id of the object that was created, or retrieved. - A return value of None signifies that there was an issue with the parameters (or the user was masquerading). + 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): + return None + if not (user and user.id) or not course_key: return None diff --git a/lms/djangoapps/course_goals/templates/course_goals/edx_ace/goalreminder/email/body.html b/lms/djangoapps/course_goals/templates/course_goals/edx_ace/goalreminder/email/body.html new file mode 100644 index 0000000000..50f681098b --- /dev/null +++ b/lms/djangoapps/course_goals/templates/course_goals/edx_ace/goalreminder/email/body.html @@ -0,0 +1,33 @@ +{% extends 'ace_common/edx_ace/common/base_body.html' %} + +{% load i18n %} +{% load static %} +{% block content %} + + + + +
+

+ {% trans "There's still time to reach your goal" as tmsg %}{{ tmsg | force_escape }} +

+

+ {% filter force_escape %} + {% blocktrans %}You set a goal of learning 3x a week in Rhetoric: The Art of Persuasive Writing and Public Speaking. You're not quite there, but there's still time to reach that goal!{% endblocktrans %} + {% endfilter %} +
+

+ +

+ {% filter force_escape %} + {% blocktrans %}Remember, you can always change your learning goal. The best goal is one that you can stick to. {% endblocktrans %} + {% endfilter %} +
+

+ + {% filter force_escape %} + {% blocktrans asvar course_cta_text %}Adjust my goal{% endblocktrans %} + {% endfilter %} + {% include "ace_common/edx_ace/common/return_to_course_cta.html" with course_cta_text=course_cta_text course_cta_url=reset_link %} +
+{% endblock %} diff --git a/lms/djangoapps/course_goals/tests/test_user_activity.py b/lms/djangoapps/course_goals/tests/test_user_activity.py index d86076c390..d41fca1648 100644 --- a/lms/djangoapps/course_goals/tests/test_user_activity.py +++ b/lms/djangoapps/course_goals/tests/test_user_activity.py @@ -19,7 +19,7 @@ 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 RECORD_USER_ACTIVITY_FLAG +from lms.djangoapps.course_goals.toggles import COURSE_GOALS_NUMBER_OF_DAYS_GOALS from openedx.core.djangoapps.django_comment_common.models import ForumsConfig from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, TEST_DATA_SPLIT_MODULESTORE from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory @@ -28,7 +28,7 @@ User = get_user_model() @ddt.ddt -@override_waffle_flag(RECORD_USER_ACTIVITY_FLAG, active=True) +@override_waffle_flag(COURSE_GOALS_NUMBER_OF_DAYS_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 index cd3fe66261..c20bfcc0e9 100644 --- a/lms/djangoapps/course_goals/toggles.py +++ b/lms/djangoapps/course_goals/toggles.py @@ -19,14 +19,3 @@ WAFFLE_FLAG_NAMESPACE = LegacyWaffleFlagNamespace(name='course_goals') # .. 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__) - -# .. toggle_name: course_goals.record_user_activity -# .. toggle_implementation: CourseWaffleFlag -# .. toggle_default: False -# .. toggle_description: This flag enables populating user activity for tracking a user's progress towards course goals -# .. toggle_warnings: None -# .. toggle_use_cases: temporary -# .. toggle_creation_date: 2021-09-16 -# .. toggle_target_removal_date: 2021-11-16 -# .. toggle_tickets: https://openedx.atlassian.net/browse/AA-905 -RECORD_USER_ACTIVITY_FLAG = CourseWaffleFlag(WAFFLE_FLAG_NAMESPACE, 'record_user_activity', __name__) diff --git a/lms/djangoapps/course_home_api/course_metadata/views.py b/lms/djangoapps/course_home_api/course_metadata/views.py index f397c08000..4132b5f9c3 100644 --- a/lms/djangoapps/course_home_api/course_metadata/views.py +++ b/lms/djangoapps/course_home_api/course_metadata/views.py @@ -13,7 +13,6 @@ from openedx.core.djangoapps.courseware_api.utils import get_celebrations_dict from common.djangoapps.student.models import CourseEnrollment from lms.djangoapps.course_api.api import course_detail -from lms.djangoapps.course_goals.toggles import RECORD_USER_ACTIVITY_FLAG from lms.djangoapps.course_goals.models import UserActivity from lms.djangoapps.course_home_api.course_metadata.serializers import CourseHomeMetadataSerializer from lms.djangoapps.courseware.access import has_access @@ -117,8 +116,7 @@ class CourseHomeMetadataView(RetrieveAPIView): ) # Record course goals user activity for (web) learning mfe course tabs - if RECORD_USER_ACTIVITY_FLAG.is_enabled(): - UserActivity.record_user_activity(request.user, course_key) + UserActivity.record_user_activity(request.user, course_key) data = { 'course_id': course.id, diff --git a/lms/djangoapps/course_home_api/dates/views.py b/lms/djangoapps/course_home_api/dates/views.py index 8b6146c6b4..f7f7fb3209 100644 --- a/lms/djangoapps/course_home_api/dates/views.py +++ b/lms/djangoapps/course_home_api/dates/views.py @@ -13,7 +13,6 @@ from rest_framework.response import Response from common.djangoapps.student.models import CourseEnrollment from lms.djangoapps.course_goals.models import UserActivity -from lms.djangoapps.course_goals.toggles import RECORD_USER_ACTIVITY_FLAG from lms.djangoapps.course_home_api.dates.serializers import DatesTabSerializer from lms.djangoapps.course_home_api.toggles import course_home_legacy_is_active from lms.djangoapps.courseware.access import has_access @@ -95,9 +94,8 @@ class DatesTabView(RetrieveAPIView): reset_masquerade_data=True, ) - if RECORD_USER_ACTIVITY_FLAG.is_enabled(): - # Record user activity for tracking progress towards a user's course goals (for mobile app) - UserActivity.record_user_activity(request.user, course.id, request=request, only_if_mobile_app=True) + # Record user activity for tracking progress towards a user's course goals (for mobile app) + UserActivity.record_user_activity(request.user, course.id, request=request, only_if_mobile_app=True) if not CourseEnrollment.is_enrolled(request.user, course_key) and not is_staff: return Response('User not enrolled.', status=401) diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index b7c7328ae5..361bd97c80 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -55,7 +55,6 @@ from lms.djangoapps.certificates import api as certs_api from lms.djangoapps.certificates.data import CertificateStatuses from lms.djangoapps.commerce.utils import EcommerceService from lms.djangoapps.course_goals.models import UserActivity -from lms.djangoapps.course_goals.toggles import RECORD_USER_ACTIVITY_FLAG from lms.djangoapps.course_home_api.toggles import ( course_home_legacy_is_active, course_home_mfe_progress_tab_is_active @@ -1734,11 +1733,10 @@ def render_xblock(request, usage_key_string, check_if_enrolled=True): staff_access, ) - if RECORD_USER_ACTIVITY_FLAG.is_enabled(): - # Record user activity for tracking progress towards a user's course goals (for mobile app) - UserActivity.record_user_activity( - request.user, usage_key.course_key, request=request, only_if_mobile_app=True - ) + # Record user activity for tracking progress towards a user's course goals (for mobile app) + UserActivity.record_user_activity( + request.user, usage_key.course_key, request=request, only_if_mobile_app=True + ) # get the block, which verifies whether the user has access to the block. recheck_access = request.GET.get('recheck_access') == '1' diff --git a/lms/djangoapps/discussion/rest_api/views.py b/lms/djangoapps/discussion/rest_api/views.py index 945d9704c1..e5614eb7c9 100644 --- a/lms/djangoapps/discussion/rest_api/views.py +++ b/lms/djangoapps/discussion/rest_api/views.py @@ -19,7 +19,6 @@ from rest_framework.viewsets import ViewSet from xmodule.modulestore.django import modulestore from lms.djangoapps.course_goals.models import UserActivity -from lms.djangoapps.course_goals.toggles import RECORD_USER_ACTIVITY_FLAG from lms.djangoapps.instructor.access import update_forum_role from openedx.core.djangoapps.django_comment_common import comment_client from openedx.core.djangoapps.django_comment_common.models import CourseDiscussionSettings, Role @@ -92,9 +91,8 @@ class CourseView(DeveloperErrorViewMixin, APIView): def get(self, request, course_id): """Implements the GET method as described in the class docstring.""" course_key = CourseKey.from_string(course_id) # TODO: which class is right? - if RECORD_USER_ACTIVITY_FLAG.is_enabled(): - # Record user activity for tracking progress towards a user's course goals (for mobile app) - UserActivity.record_user_activity(request.user, course_key, request=request, only_if_mobile_app=True) + # Record user activity for tracking progress towards a user's course goals (for mobile app) + UserActivity.record_user_activity(request.user, course_key, request=request, only_if_mobile_app=True) return Response(get_course(request, course_key)) @@ -137,9 +135,8 @@ class CourseTopicsView(DeveloperErrorViewMixin, APIView): course_key, set(topic_ids.strip(',').split(',')) if topic_ids else None, ) - if RECORD_USER_ACTIVITY_FLAG.is_enabled(): - # Record user activity for tracking progress towards a user's course goals (for mobile app) - UserActivity.record_user_activity(request.user, course_key, request=request, only_if_mobile_app=True) + # Record user activity for tracking progress towards a user's course goals (for mobile app) + UserActivity.record_user_activity(request.user, course_key, request=request, only_if_mobile_app=True) return Response(response) @@ -331,11 +328,10 @@ class ThreadViewSet(DeveloperErrorViewMixin, ViewSet): if not form.is_valid(): raise ValidationError(form.errors) - if RECORD_USER_ACTIVITY_FLAG.is_enabled(): - # Record user activity for tracking progress towards a user's course goals (for mobile app) - UserActivity.record_user_activity( - request.user, form.cleaned_data["course_id"], request=request, only_if_mobile_app=True - ) + # Record user activity for tracking progress towards a user's course goals (for mobile app) + UserActivity.record_user_activity( + request.user, form.cleaned_data["course_id"], request=request, only_if_mobile_app=True + ) return get_thread_list( request, diff --git a/lms/djangoapps/mobile_api/course_info/tests.py b/lms/djangoapps/mobile_api/course_info/tests.py index 6441f9f421..a029c45e6b 100644 --- a/lms/djangoapps/mobile_api/course_info/tests.py +++ b/lms/djangoapps/mobile_api/course_info/tests.py @@ -8,11 +8,12 @@ from django.conf import settings from django.urls import reverse from edx_toggles.toggles.testutils import override_waffle_flag from milestones.tests.utils import MilestonesTestCaseMixin +from mock import patch 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 RECORD_USER_ACTIVITY_FLAG +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 xmodule.html_module import CourseInfoBlock @@ -229,7 +230,7 @@ class TestHandouts(MobileAPITestCase, MobileAuthTestMixin, MobileCourseAccessTes self.login_and_enroll() -@override_waffle_flag(RECORD_USER_ACTIVITY_FLAG, active=True) +@override_waffle_flag(COURSE_GOALS_NUMBER_OF_DAYS_GOALS, active=True) class TestCourseGoalsUserActivityAPI(MobileAPITestCase, SharedModuleStoreTestCase): """ Testing the Course Goals User Activity API. @@ -278,3 +279,21 @@ 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) + @patch('lms.djangoapps.mobile_api.course_info.views.log') + def test_flag_disabled(self, mock_logger): + ''' + Test the API behavior when the goals flag is disabled + ''' + post_data = { + 'user_id': self.user.id, + 'course_key': self.course.id, + } + + response = self.client.post(self.apiUrl, post_data) + assert response.status_code == 200 + mock_logger.warning.assert_called_with( + 'For this mobile request, user activity is not enabled for this user {} and course {}'.format( + str(self.user.id), str(self.course.id)) + ) diff --git a/lms/djangoapps/mobile_api/course_info/views.py b/lms/djangoapps/mobile_api/course_info/views.py index 9c7aec3a96..e12bb953fd 100644 --- a/lms/djangoapps/mobile_api/course_info/views.py +++ b/lms/djangoapps/mobile_api/course_info/views.py @@ -2,6 +2,8 @@ Views for course info API """ +import logging + from django.contrib.auth import get_user_model from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey @@ -12,11 +14,13 @@ 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 ..decorators import mobile_course_access, mobile_view User = get_user_model() +log = logging.getLogger(__name__) @mobile_view() @@ -153,6 +157,12 @@ class CourseGoalsRecordUserActivity(APIView): status=status.HTTP_400_BAD_REQUEST, ) + if not COURSE_GOALS_NUMBER_OF_DAYS_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)) + ) + return Response(status=(200)) + # Populate user activity for tracking progress towards a user's course goals UserActivity.record_user_activity(user, course_key) return Response(status=(200)) diff --git a/lms/djangoapps/mobile_api/decorators.py b/lms/djangoapps/mobile_api/decorators.py index 84d11ae266..b5425a6265 100644 --- a/lms/djangoapps/mobile_api/decorators.py +++ b/lms/djangoapps/mobile_api/decorators.py @@ -11,7 +11,6 @@ from rest_framework import status from rest_framework.response import Response from lms.djangoapps.course_goals.models import UserActivity -from lms.djangoapps.course_goals.toggles import RECORD_USER_ACTIVITY_FLAG from lms.djangoapps.courseware.courses import get_course_with_access from lms.djangoapps.courseware.courseware_access_exception import CoursewareAccessException from lms.djangoapps.courseware.exceptions import CourseAccessRedirect @@ -43,11 +42,10 @@ def mobile_course_access(depth=0): depth=depth, check_if_enrolled=True, ) - if RECORD_USER_ACTIVITY_FLAG.is_enabled(): - # Record user activity for tracking progress towards a user's course goals (for mobile app) - UserActivity.record_user_activity( - request.user, course_id, request=request, only_if_mobile_app=True - ) + # Record user activity for tracking progress towards a user's course goals (for mobile app) + UserActivity.record_user_activity( + request.user, course_id, request=request, only_if_mobile_app=True + ) except CoursewareAccessException as error: return Response(data=error.to_json(), status=status.HTTP_404_NOT_FOUND) except CourseAccessRedirect as error: diff --git a/lms/templates/courseware/course_navigation.html b/lms/templates/courseware/course_navigation.html index 968745d7d8..6f63e76fce 100644 --- a/lms/templates/courseware/course_navigation.html +++ b/lms/templates/courseware/course_navigation.html @@ -7,7 +7,6 @@ from lms.djangoapps.courseware.masquerade import is_masquerading_as_student from lms.djangoapps.courseware.tabs import get_course_tab_list from lms.djangoapps.course_goals.models import UserActivity -from lms.djangoapps.course_goals.toggles import RECORD_USER_ACTIVITY_FLAG from django.conf import settings from django.urls import reverse @@ -44,8 +43,7 @@ masquerading_as_student = is_masquerading_as_student(request.user, course.id) tab_list = get_course_tab_list(request.user, course) # Record course goals user activity for (web) courseware and course tabs that are outside of the learning mfe - if RECORD_USER_ACTIVITY_FLAG.is_enabled(): - UserActivity.record_user_activity(user, course.id) + UserActivity.record_user_activity(user, course.id) %> % if uses_bootstrap: