Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions lms/djangoapps/course_api/blocks/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
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
Expand Down Expand Up @@ -309,6 +311,10 @@ def list(self, request, hide_access_denials=False): # pylint: disable=arguments
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)

calculate_completion = any('completion' in param
for param in request.query_params.getlist('requested_fields', []))
if not calculate_completion:
Expand Down
70 changes: 70 additions & 0 deletions lms/djangoapps/course_goals/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,22 @@
"""

import uuid
import logging
import pytz
from datetime import datetime, timedelta

from django.contrib.auth import get_user_model
from django.db import models
from django.utils.translation import ugettext_lazy as _
from edx_django_utils.cache import TieredCache
from model_utils import Choices
from opaque_keys.edx.django.models import CourseKeyField
from simple_history.models import HistoricalRecords

from lms.djangoapps.courseware.masquerade import is_masquerading
from openedx.core.djangoapps.user_api.preferences.api import get_user_preferences
from openedx.core.lib.mobile_utils import is_request_from_mobile_app

# Each goal is represented by a goal key and a string description.
GOAL_KEY_CHOICES = Choices(
('certify', _('Earn a certificate')),
Expand All @@ -20,6 +28,7 @@
)

User = get_user_model()
log = logging.getLogger(__name__)


class CourseGoal(models.Model):
Expand Down Expand Up @@ -84,3 +93,64 @@ class Meta:
user = models.ForeignKey(User, on_delete=models.CASCADE)
course_key = CourseKeyField(max_length=255)
date = models.DateField()

@classmethod
def record_user_activity(cls, user, course_key, request=None, only_if_mobile_app=False):
'''
Update the user activity table with a record for this activity.

Since we store one activity per date, we don't need to query the database
for every activity on a given date.
To avoid unnecessary queries, we store a record in a cache once we have an activity for the date,
which times out at the end of that date (in the user's timezone).

The request argument is only used to check if the request is coming from a mobile app.
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).
'''
if not (user and user.id) or not course_key:
return None

if only_if_mobile_app and request and not is_request_from_mobile_app(request):
return None

if is_masquerading(user, course_key):
return None

user_preferences = get_user_preferences(user)
timezone = pytz.timezone(user_preferences.get('time_zone', 'UTC'))
now = datetime.now(timezone)
date = now.date()

cache_key = 'goals_user_activity_{}_{}_{}'.format(str(user.id), str(course_key), str(date))

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)

# Cache result until the end of the day to avoid unnecessary database requests
tomorrow = now + timedelta(days=1)
midnight = datetime(year=tomorrow.year, month=tomorrow.month,
day=tomorrow.day, hour=0, minute=0, second=0, tzinfo=timezone)
seconds_until_midnight = (midnight - now).seconds
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: you could also just pass in 24 hours, right? I might be more worried about having fussy logic than holding a cached item for slightly longer than expected.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm - true, i guess the cache key has the date in it, so we shouldn't run into issues of missing data, but i'm also not sure if i'm concerned about this specific logic?


TieredCache.set_all_tiers(cache_key, activity_object.id, seconds_until_midnight)
# Temporary debugging log for testing mobile app connection
if request:
log.info(
'Set cached value with request {} for user and course combination {} {}'.format(
str(request.build_absolute_uri()), str(user.id), str(course_key)
)
)
return activity_object.id
205 changes: 205 additions & 0 deletions lms/djangoapps/course_goals/tests/test_user_activity.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,205 @@
"""
Unit tests for user activity methods.
"""

from datetime import datetime, timedelta


import ddt
from django.contrib.auth import get_user_model
from django.test.client import RequestFactory
from django.urls import reverse
from edx_django_utils.cache import TieredCache
from edx_toggles.toggles.testutils import override_waffle_flag
from freezegun import freeze_time
from mock import patch

from common.djangoapps.edxmako.shortcuts import render_to_response
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 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

User = get_user_model()


@ddt.ddt
@override_waffle_flag(RECORD_USER_ACTIVITY_FLAG, active=True)
class UserActivityTests(UrlResetMixin, ModuleStoreTestCase):
"""
Testing Course Goals User Activity
"""

MODULESTORE = TEST_DATA_SPLIT_MODULESTORE

@patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True})
def setUp(self):
super().setUp()
self.course = CourseFactory.create(
start=datetime(2020, 1, 1),
end=datetime(2028, 1, 1),
enrollment_start=datetime(2020, 1, 1),
enrollment_end=datetime(2028, 1, 1),
emit_signals=True,
modulestore=self.store,
discussion_topics={"Test Topic": {"id": "test_topic"}},
)
chapter = ItemFactory(parent=self.course, category='chapter')
ItemFactory(parent=chapter, category='sequential')

self.client.login(username=self.user.username, password=self.user_password)
CourseEnrollment.enroll(self.user, self.course.id)

self.request = RequestFactory().get('foo')
self.request.user = self.user

config = ForumsConfig.current()
config.enabled = True
config.save()

def test_mfe_tabs_call_user_activity(self):
'''
New style tabs call one of two metadata endpoints
These in turn call get_course_tab_list, which records user activity
'''
url = reverse('course-home:course-metadata', args=[self.course.id])
with patch.object(UserActivity, 'record_user_activity') as record_user_activity_mock:
self.client.get(url)
record_user_activity_mock.assert_called_once()

with patch.object(UserActivity, 'record_user_activity') as record_user_activity_mock:
url = f'/api/courseware/course/{self.course.id}'
self.client.get(url)
record_user_activity_mock.assert_called_once()

def test_non_mfe_tabs_call_user_activity(self):
'''
Tabs that are not yet part of the learning microfrontend all include the course_navigation.html file
This file calls the get_course_tab_list function, which records user activity
'''
with patch.object(UserActivity, 'record_user_activity') as record_user_activity_mock:
render_to_response('courseware/course_navigation.html', {'course': self.course, 'request': self.request})
record_user_activity_mock.assert_called_once()

def test_when_record_user_activity_does_not_perform_updates(self):
'''
Ensure that record user activity is not called when:
1. user or course are not defined
2. we have already recorded user activity for this user/course on this date
and have a record in the cache
'''
with patch.object(TieredCache, 'set_all_tiers', wraps=TieredCache.set_all_tiers) as activity_cache_set:
UserActivity.record_user_activity(self.user, None)
activity_cache_set.assert_not_called()

UserActivity.record_user_activity(None, self.course.id)
activity_cache_set.assert_not_called()

cache_key = 'goals_user_activity_{}_{}_{}'.format(
str(self.user.id), str(self.course.id), str(datetime.now().date())
)
TieredCache.set_all_tiers(cache_key, 'test', 3600)

with patch.object(TieredCache, 'set_all_tiers', wraps=TieredCache.set_all_tiers) as activity_cache_set:
UserActivity.record_user_activity(self.user, self.course.id)
activity_cache_set.assert_not_called()

# Test that the happy path works to ensure that the measurement in this test isn't broken
user2 = UserFactory()
UserActivity.record_user_activity(user2, self.course.id)
activity_cache_set.assert_called_once()

def test_that_user_activity_cache_works_properly(self):
'''
Ensure that the cache for user activity works properly
1. user or course are not defined
2. we have already recorded user activity for this user/course on this date
and have a record in the cache
'''
with patch.object(TieredCache, 'set_all_tiers', wraps=TieredCache.set_all_tiers) as activity_cache_set:
UserActivity.record_user_activity(self.user, self.course.id)
activity_cache_set.assert_called_once()

with patch.object(TieredCache, 'set_all_tiers', wraps=TieredCache.set_all_tiers) as activity_cache_set:
UserActivity.record_user_activity(self.user, self.course.id)
activity_cache_set.assert_not_called()

now_plus_1_day = datetime.now() + timedelta(days=1)
with freeze_time(now_plus_1_day):
UserActivity.record_user_activity(self.user, self.course.id)
activity_cache_set.assert_called_once()

def test_mobile_argument(self):
'''
Method only records activity if the request is coming from the mobile app
when the only_if_mobile_app argument is true
'''
with patch.object(TieredCache, 'set_all_tiers', wraps=TieredCache.set_all_tiers) as activity_cache_set:
UserActivity.record_user_activity(
self.user, self.course.id, request=self.request, only_if_mobile_app=True
)
activity_cache_set.assert_not_called()

with patch('lms.djangoapps.course_goals.models.is_request_from_mobile_app', return_value=True):
UserActivity.record_user_activity(
self.user, self.course.id, request=self.request, only_if_mobile_app=True
)
activity_cache_set.assert_called_once()

def test_masquerading(self):
'''
Method only records activity if the user is not masquerading
'''
with patch.object(TieredCache, 'set_all_tiers', wraps=TieredCache.set_all_tiers) as activity_cache_set:
UserActivity.record_user_activity(self.user, self.course.id)
activity_cache_set.assert_called_once()

with patch.object(TieredCache, 'set_all_tiers', wraps=TieredCache.set_all_tiers) as activity_cache_set:
with patch('lms.djangoapps.course_goals.models.is_masquerading', return_value=True):
UserActivity.record_user_activity(self.user, self.course.id)
activity_cache_set.assert_not_called()

@ddt.data(
'/api/course_home/v1/dates/{COURSE_ID}',
'/api/mobile/v0.5/course_info/{COURSE_ID}/handouts',
'/api/mobile/v0.5/course_info/{COURSE_ID}/updates',
'/api/course_experience/v1/course_deadlines_info/{COURSE_ID}',
'/api/course_home/v1/dates/{COURSE_ID}',
'/api/courseware/course/{COURSE_ID}',
'/api/discussion/v1/courses/{COURSE_ID}/',
'/api/discussion/v1/course_topics/{COURSE_ID}',
)
def test_mobile_app_user_activity_calls(self, url):
url = url.replace('{COURSE_ID}', str(self.course.id))
with patch.object(UserActivity, 'record_user_activity') as record_user_activity_mock:
with patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}):
self.client.get(url)
record_user_activity_mock.assert_called_once()

def test_mobile_app_user_activity_other_calls(self):
# thread view call
with patch.object(UserActivity, 'record_user_activity') as record_user_activity_mock:
try:
self.client.get(reverse("thread-list"), {'course_id': str(self.course.id)})
except: # pylint: disable=bare-except
pass
record_user_activity_mock.assert_called_once()

# blocks call
with patch.object(UserActivity, 'record_user_activity') as record_user_activity_mock:
url = '/api/courses/v2/blocks/'
self.client.get(url, {'course_id': str(self.course.id), 'username': self.user.username})
record_user_activity_mock.assert_called_once()

# xblock call
with patch.object(UserActivity, 'record_user_activity') as record_user_activity_mock:
url = '/xblock/' + str(self.course.scope_ids.usage_id)
try:
self.client.get(url)
except: # pylint: disable=bare-except
pass
record_user_activity_mock.assert_called_once()
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
"""
Unit tests for course_goals.api methods.
Unit tests for course_goals.views methods.
"""


from unittest import mock

from django.contrib.auth import get_user_model
from django.test.utils import override_settings
from django.urls import reverse
from rest_framework.test import APIClient
Expand All @@ -19,8 +18,6 @@
EVENT_NAME_ADDED = 'edx.course.goal.added'
EVENT_NAME_UPDATED = 'edx.course.goal.updated'

User = get_user_model()


class TestCourseGoalsAPI(SharedModuleStoreTestCase):
"""
Expand Down
11 changes: 11 additions & 0 deletions lms/djangoapps/course_goals/toggles.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,14 @@
# .. 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__)
6 changes: 6 additions & 0 deletions lms/djangoapps/course_home_api/course_metadata/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@

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
from lms.djangoapps.courseware.context_processor import user_timezone_locale_prefs
Expand Down Expand Up @@ -114,6 +116,10 @@ def get(self, request, *args, **kwargs):
request.user, enrollment, course, user_timezone if not None else browser_timezone
)

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

data = {
'course_id': course.id,
'username': username,
Expand Down
6 changes: 6 additions & 0 deletions lms/djangoapps/course_home_api/dates/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
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
Expand Down Expand Up @@ -93,6 +95,10 @@ def get(self, request, *args, **kwargs):
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)

if not CourseEnrollment.is_enrolled(request.user, course_key) and not is_staff:
return Response('User not enrolled.', status=401)

Expand Down
Loading