fix: Remove debugging flag that is no longer necessary and refactor

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)
-            # 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)
-            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', []))

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
 

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 %}
+<table width="100%" align="left" border="0" cellpadding="0" cellspacing="0" role="presentation">
+    <tr>
+        <td>
+            <h1>
+                {% trans "There's still time to reach your goal" as tmsg %}{{ tmsg | force_escape }}
+            </h1>
+            <p style="color: rgba(0,0,0,.75);">
+                {% 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 %}
+                <br />
+            </p>
+
+            <p style="color: rgba(0,0,0,.75);">
+                {% filter force_escape %}
+                {% blocktrans %}Remember, you can always change your learning goal. The best goal is one that you can stick to.  {% endblocktrans %}
+                {% endfilter %}
+                <br />
+            </p>
+
+            {% 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 %}
+        </td>
+    </tr>
+</table>
+{% endblock %}

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

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__)

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,

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)
-            # 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)
-            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)

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)
-            # Record user activity for tracking progress towards a user's course goals (for mobile app)
+        UserActivity.record_user_activity(
-            UserActivity.record_user_activity(
+            request.user, usage_key.course_key, request=request, only_if_mobile_app=True
-                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'

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)
-            # 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)
-            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)
-                # 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)
-                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)
-            # Record user activity for tracking progress towards a user's course goals (for mobile app)
+        UserActivity.record_user_activity(
-            UserActivity.record_user_activity(
+            request.user, form.cleaned_data["course_id"], request=request, only_if_mobile_app=True
-                request.user, form.cleaned_data["course_id"], request=request, only_if_mobile_app=True
+        )
-            )
 
         return get_thread_list(
             request,

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))
+        )

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))

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)
-                        # Record user activity for tracking progress towards a user's course goals (for mobile app)
+                    UserActivity.record_user_activity(
-                        UserActivity.record_user_activity(
+                        request.user, course_id, request=request, only_if_mobile_app=True
-                            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:

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:
         <nav class="navbar course-tabs pb-0 navbar-expand" aria-label="${_('Course')}">

openedx/core/djangoapps/courseware_api/views.py

@@ -22,7 +22,6 @@ from lms.djangoapps.edxnotes.helpers import is_feature_enabled
 from lms.djangoapps.certificates.api import get_certificate_url
 from lms.djangoapps.certificates.models import GeneratedCertificate
 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.courseware.access import has_access
 from lms.djangoapps.courseware.access_response import (
@@ -456,8 +455,7 @@ class CoursewareInformation(RetrieveAPIView):
             username=username,
         )
         # Record course goals user activity for learning mfe courseware on web
-        if RECORD_USER_ACTIVITY_FLAG.is_enabled():
+        UserActivity.record_user_activity(self.request.user, course_key)
-            UserActivity.record_user_activity(self.request.user, course_key)
 
         return overview
 

openedx/features/course_experience/api/v1/views.py

@@ -21,7 +21,6 @@ from opaque_keys.edx.keys import CourseKey
 
 from lms.djangoapps.course_api.api import course_detail
 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
 from lms.djangoapps.courseware.access import has_access
 from lms.djangoapps.courseware.courses import get_course_with_access
@@ -154,11 +153,10 @@ class CourseDeadlinesMobileView(RetrieveAPIView):
         # Although this course data is not used this method will return 404 if course does not exist
         get_course_with_access(request.user, 'load', course_key)
 
-        if RECORD_USER_ACTIVITY_FLAG.is_enabled():
+        # Record user activity for tracking progress towards a user's course goals (for mobile app)
-            # Record user activity for tracking progress towards a user's course goals (for mobile app)
+        UserActivity.record_user_activity(
-            UserActivity.record_user_activity(
+            request.user, course_key, request=request, only_if_mobile_app=True
-                request.user, course_key, request=request, only_if_mobile_app=True
+        )
-            )
 
         serializer = self.get_serializer({})
         return Response(serializer.data)