From 103ec9e0dc3f2a92dc6d80c6f92e34b888383177 Mon Sep 17 00:00:00 2001 From: Diane Kaplan Date: Wed, 4 Nov 2020 11:21:48 -0500 Subject: [PATCH] REV-1564: add user-metadata API, for use by Optimizely experiments (#25450) --- common/djangoapps/student/views/dashboard.py | 9 +- lms/djangoapps/experiments/permissions.py | 10 ++ .../experiments/tests/test_utils.py | 46 +++++- .../experiments/tests/test_views.py | 59 +++++++- lms/djangoapps/experiments/urls.py | 5 +- lms/djangoapps/experiments/utils.py | 131 ++++++++++++++---- lms/djangoapps/experiments/views.py | 37 ++++- lms/templates/experiments/user_metadata.html | 68 +-------- 8 files changed, 260 insertions(+), 105 deletions(-) diff --git a/common/djangoapps/student/views/dashboard.py b/common/djangoapps/student/views/dashboard.py index 071cd3840d..55e1e2497a 100644 --- a/common/djangoapps/student/views/dashboard.py +++ b/common/djangoapps/student/views/dashboard.py @@ -29,7 +29,7 @@ from edxmako.shortcuts import render_to_response, render_to_string from entitlements.models import CourseEntitlement from lms.djangoapps.commerce.utils import EcommerceService from lms.djangoapps.courseware.access import has_access -from lms.djangoapps.experiments.utils import get_dashboard_course_info +from lms.djangoapps.experiments.utils import get_dashboard_course_info, get_experiment_user_metadata_context from lms.djangoapps.verify_student.services import IDVerificationService from openedx.core.djangoapps.catalog.utils import ( get_programs, @@ -805,6 +805,13 @@ def student_dashboard(request): ) context.update(context_from_plugins) + course = None + context.update( + get_experiment_user_metadata_context( + course, + user, + ) + ) if ecommerce_service.is_enabled(request.user): context.update({ 'use_ecommerce_payment_flow': True, diff --git a/lms/djangoapps/experiments/permissions.py b/lms/djangoapps/experiments/permissions.py index 165a0d4706..8c256725d3 100644 --- a/lms/djangoapps/experiments/permissions.py +++ b/lms/djangoapps/experiments/permissions.py @@ -29,3 +29,13 @@ class IsStaffOrOwner(permissions.IsStaffOrOwner): class IsStaffOrReadOnly(BasePermission): def has_permission(self, request, view): return request.user.is_staff or request.method in SAFE_METHODS + + +class IsStaffOrReadOnlyForSelf(BasePermission): + """ + Grants access to staff or to user reading info about their own user + """ + def has_permission(self, request, view): + username = request.user.username + return request.user.is_staff or (request.method in SAFE_METHODS and ( + username == request.parser_context['kwargs']['username'])) diff --git a/lms/djangoapps/experiments/tests/test_utils.py b/lms/djangoapps/experiments/tests/test_utils.py index 8e3881de68..f347975423 100644 --- a/lms/djangoapps/experiments/tests/test_utils.py +++ b/lms/djangoapps/experiments/tests/test_utils.py @@ -2,21 +2,26 @@ Tests of experiment functionality """ - +from datetime import timedelta from decimal import Decimal +from django.utils.timezone import now from unittest import TestCase from opaque_keys.edx.keys import CourseKey - +from lms.djangoapps.course_blocks.transformers.tests.helpers import ModuleStoreTestCase +from lms.djangoapps.courseware import courses from lms.djangoapps.experiments.utils import ( get_course_entitlement_price_and_sku, + get_experiment_user_metadata_context, get_program_price_and_skus, get_unenrolled_courses, is_enrolled_in_course_run ) +from student.tests.factories import CourseEnrollmentFactory, UserFactory +from xmodule.modulestore.tests.factories import CourseFactory -class ExperimentUtilsTests(TestCase): +class ExperimentUtilsTests(ModuleStoreTestCase, TestCase): """ Tests of experiment functionality """ @@ -116,3 +121,38 @@ class ExperimentUtilsTests(TestCase): self.assertEqual(2, len(skus)) self.assertIn(self.run_a_sku, skus) self.assertIn(self.entitlement_a_sku, skus) + + def test_get_experiment_user_metadata_context(self): + course = CourseFactory.create(start=now() - timedelta(days=30), pacing_type="instructor_paced", course_duration=None, upgrade_price='Free', + upgrade_link=None, enrollment_mode=None, audit_access_deadline=None, program_key_fields=None, schedule_start=None, + enrollment_time=None, dynamic_upgrade_deadline=None, course_upgrade_deadline=None, course_key_fields={'org': 'org.0', 'course': 'course_0', 'run': 'Run_0'}) + user = UserFactory() + context = get_experiment_user_metadata_context(course, user) + CourseEnrollmentFactory(course_id=course.id, user=user) + + user_metadata_expected_result = {'username': user.username, + 'user_id': user.id, + 'course_id': course.id, + 'enrollment_mode': course.enrollment_mode, + 'upgrade_link': course.upgrade_link, + 'upgrade_price': course.upgrade_price, + 'audit_access_deadline': course.audit_access_deadline, + 'course_duration': course.course_duration, + 'pacing_type': course.pacing_type, + 'has_staff_access': False, + 'forum_roles': [], + 'partition_groups': {}, + 'has_non_audit_enrollments': False, + 'program_key_fields': course.program_key_fields, + 'email': user.email, + 'schedule_start': course.schedule_start, + 'enrollment_time': course.enrollment_time, + 'course_start': course.start, + 'course_end': course.end, + 'dynamic_upgrade_deadline': course.dynamic_upgrade_deadline, + 'course_upgrade_deadline': course.course_upgrade_deadline, + 'course_key_fields': course.course_key_fields} + + user_metadata = context.get('user_metadata') + + self.assertTrue(user_metadata, user_metadata_expected_result) diff --git a/lms/djangoapps/experiments/tests/test_views.py b/lms/djangoapps/experiments/tests/test_views.py index 5ab08b0464..e5945dd5aa 100644 --- a/lms/djangoapps/experiments/tests/test_views.py +++ b/lms/djangoapps/experiments/tests/test_views.py @@ -8,10 +8,13 @@ import unittest import six.moves.urllib.error import six.moves.urllib.parse import six.moves.urllib.request +from datetime import timedelta from django.conf import settings from django.core.handlers.wsgi import WSGIRequest +from django.utils.timezone import now from django.test.utils import override_settings from django.urls import reverse +from lms.djangoapps.course_blocks.transformers.tests.helpers import ModuleStoreTestCase from mock import patch from rest_framework.test import APITestCase @@ -20,10 +23,12 @@ from lms.djangoapps.experiments.models import ExperimentData, ExperimentKeyValue from lms.djangoapps.experiments.serializers import ExperimentDataSerializer from student.tests.factories import UserFactory +from xmodule.modulestore.tests.factories import CourseFactory + CROSS_DOMAIN_REFERER = 'https://ecommerce.edx.org' -class ExperimentDataViewSetTests(APITestCase): +class ExperimentDataViewSetTests(APITestCase, ModuleStoreTestCase): def assert_data_created_for_user(self, user, method='post', status=201): url = reverse('api_experiments:v0:data-list') @@ -288,3 +293,55 @@ class ExperimentKeyValueViewSetTests(APITestCase): response = self.client.delete(url) self.assertEqual(response.status_code, 403) + + +class ExperimentUserMetaDataViewTests(APITestCase, ModuleStoreTestCase): + """ Internal user_metadata view/API for use in Optimizely experiments """ + + def test_UserMetaDataView_get_success_same_user(self): + """ Request succeeds when logged-in user makes request for self """ + lookup_user = UserFactory() + lookup_course = CourseFactory.create(start=now() - timedelta(days=30)) + call_args = [lookup_user.username, lookup_course.id] + self.client.login(username=lookup_user.username, password=UserFactory._DEFAULT_PASSWORD) + + response = self.client.get(reverse('api_experiments:user_metadata', args=call_args)) + self.assertEqual(response.status_code, 200) + + def test_UserMetaDataView_get_success_staff_user(self): + """ Request succeeds when logged-in staff user makes request for different user """ + lookup_user = UserFactory() + lookup_course = CourseFactory.create(start=now() - timedelta(days=30)) + call_args = [lookup_user.username, lookup_course.id] + staff_user = UserFactory(is_staff=True) + + self.client.login(username=staff_user.username, password=UserFactory._DEFAULT_PASSWORD) + + response = self.client.get(reverse('api_experiments:user_metadata', args=call_args)) + self.assertEqual(response.status_code, 200) + self.assertTrue(response.json()['course_id']) + self.assertTrue(response.json()['user_id']) + self.assertEqual(response.json()['username'], lookup_user.username) + self.assertEqual(response.json()['email'], lookup_user.email) + + def test_UserMetaDataView_get_different_user(self): + """ Request fails when not logged in for requested user or staff """ + lookup_user = UserFactory() + lookup_course = CourseFactory.create(start=now() - timedelta(days=30)) + call_args = [lookup_user.username, lookup_course.id] + + response = self.client.get(reverse('api_experiments:user_metadata', args=call_args)) + self.assertEqual(response.status_code, 401) + + def test_UserMetaDataView_get_missing_course(self): + """ Request fails when not course not found """ + lookup_user = UserFactory() + lookup_course = CourseFactory.create(start=now() - timedelta(days=30)) + call_args = [lookup_user.username, lookup_course.id] + self.client.login(username=lookup_user.username, password=UserFactory._DEFAULT_PASSWORD) + bogus_course_name = str(lookup_course.id) + '_FOOBAR' + + call_args_with_bogus_course = [lookup_user.username, bogus_course_name] + response = self.client.get(reverse('api_experiments:user_metadata', args=call_args_with_bogus_course)) + self.assertEqual(response.status_code, 404) + self.assertEqual(response.json()['message'], 'Provided course is not found') diff --git a/lms/djangoapps/experiments/urls.py b/lms/djangoapps/experiments/urls.py index ec6af20e1a..005c9ed0d2 100644 --- a/lms/djangoapps/experiments/urls.py +++ b/lms/djangoapps/experiments/urls.py @@ -2,7 +2,7 @@ Experimentation URLs """ - +from django.conf import settings from django.conf.urls import include, url from . import routers, views, views_custom @@ -14,4 +14,7 @@ router.register(r'key-value', views.ExperimentKeyValueViewSet, basename='key_val urlpatterns = [ url(r'^v0/custom/REV-934/', views_custom.Rev934.as_view(), name='rev_934'), url(r'^v0/', include((router.urls, "api"), namespace='v0')), + url(r'^v0/custom/userMetadata/{username},{course_key}$'.format( + username=settings.USERNAME_PATTERN, + course_key=settings.COURSE_ID_PATTERN), views.UserMetaDataView.as_view(), name='user_metadata'), ] diff --git a/lms/djangoapps/experiments/utils.py b/lms/djangoapps/experiments/utils.py index 05cc3ceda0..b38f49cfdd 100644 --- a/lms/djangoapps/experiments/utils.py +++ b/lms/djangoapps/experiments/utils.py @@ -264,46 +264,119 @@ def get_dashboard_course_info(user, dashboard_enrollments): def get_experiment_user_metadata_context(course, user): """ - Return a context dictionary with the keys used by the user_metadata.html. + Return a context dictionary with the keys used for Optimizely experiments, exposed via user_metadata.html: + view from the DOM in those calling views using: JSON.parse($("#user-metadata").text()); + Most views call this function with both parameters, but student dashboard has only a user """ enrollment = None # TODO: clean up as part of REVO-28 (START) user_enrollments = None audit_enrollments = None has_non_audit_enrollments = False - try: - user_enrollments = CourseEnrollment.objects.select_related('course', 'schedule').filter(user_id=user.id) - has_non_audit_enrollments = user_enrollments.exclude(mode__in=CourseMode.UPSELL_TO_VERIFIED_MODES).exists() + context = {} + if course is not None: + try: + user_enrollments = CourseEnrollment.objects.select_related('course', 'schedule').filter(user_id=user.id) + has_non_audit_enrollments = user_enrollments.exclude(mode__in=CourseMode.UPSELL_TO_VERIFIED_MODES).exists() + # TODO: clean up as part of REVO-28 (END) + enrollment = CourseEnrollment.objects.select_related( + 'course', 'schedule' + ).get(user_id=user.id, course_id=course.id) + except CourseEnrollment.DoesNotExist: + pass # Not enrolled, use the default values + + has_entitlements = False + if user.is_authenticated: + has_entitlements = CourseEntitlement.objects.filter(user=user).exists() + + context = get_base_experiment_metadata_context(course, user, enrollment, user_enrollments) + has_staff_access = has_staff_access_to_preview_mode(user, course.id) + forum_roles = [] + if user.is_authenticated: + forum_roles = list(Role.objects.filter(users=user, course_id=course.id).values_list('name').distinct()) + + # get user partition data + if user.is_authenticated: + partition_groups = get_all_partitions_for_course(course) + user_partitions = get_user_partition_groups(course.id, partition_groups, user, 'name') + else: + user_partitions = {} + + # TODO: clean up as part of REVO-28 (START) + context['has_non_audit_enrollments'] = has_non_audit_enrollments or has_entitlements # TODO: clean up as part of REVO-28 (END) - enrollment = CourseEnrollment.objects.select_related( - 'course', 'schedule' - ).get(user_id=user.id, course_id=course.id) - except CourseEnrollment.DoesNotExist: - pass # Not enrolled, use the default values + context['has_staff_access'] = has_staff_access + context['forum_roles'] = forum_roles + context['partition_groups'] = user_partitions - has_entitlements = False - if user.is_authenticated: - has_entitlements = CourseEntitlement.objects.filter(user=user).exists() + user_metadata = { + key: context.get(key) + for key in ( + 'username', + 'user_id', + 'course_id', + 'enrollment_mode', + 'upgrade_link', + 'upgrade_price', + 'audit_access_deadline', + 'course_duration', + 'pacing_type', + 'has_staff_access', + 'forum_roles', + 'partition_groups', + # TODO: clean up as part of REVO-28 (START) + 'has_non_audit_enrollments', + # TODO: clean up as part of REVO-28 (END) + # TODO: clean up as part of REVEM-199 (START) + 'program_key_fields', + # TODO: clean up as part of REVEM-199 (END) + ) + } - context = get_base_experiment_metadata_context(course, user, enrollment, user_enrollments) - has_staff_access = has_staff_access_to_preview_mode(user, course.id) - forum_roles = [] - if user.is_authenticated: - forum_roles = list(Role.objects.filter(users=user, course_id=course.id).values_list('name').distinct()) + if user: + user_metadata['username'] = user.username + user_metadata['user_id'] = user.id + if hasattr(user, 'email'): + user_metadata['email'] = user.email - # get user partition data - if user.is_authenticated: - partition_groups = get_all_partitions_for_course(course) - user_partitions = get_user_partition_groups(course.id, partition_groups, user, 'name') - else: - user_partitions = {} + for datekey in ( + 'schedule_start', + 'enrollment_time', + 'course_start', + 'course_end', + 'dynamic_upgrade_deadline', + 'course_upgrade_deadline', + 'audit_access_deadline', + ): + user_metadata[datekey] = ( + context.get(datekey).isoformat() if context.get(datekey) else None + ) - # TODO: clean up as part of REVO-28 (START) - context['has_non_audit_enrollments'] = has_non_audit_enrollments or has_entitlements - # TODO: clean up as part of REVO-28 (END) - context['has_staff_access'] = has_staff_access - context['forum_roles'] = forum_roles - context['partition_groups'] = user_partitions + for timedeltakey in ( + 'course_duration', + ): + user_metadata[timedeltakey] = ( + context.get(timedeltakey).total_seconds() if context.get(timedeltakey) else None + ) + + course_key = context.get('course_key') + if course and not course_key: + course_key = course.id + + if course_key: + if isinstance(course_key, CourseKey): + user_metadata['course_key_fields'] = { + 'org': course_key.org, + 'course': course_key.course, + 'run': course_key.run, + } + + if not context.get('course_id'): + user_metadata['course_id'] = six.text_type(course_key) + elif isinstance(course_key, six.string_types): + user_metadata['course_id'] = course_key + + context['user_metadata'] = user_metadata return context diff --git a/lms/djangoapps/experiments/views.py b/lms/djangoapps/experiments/views.py index 14606e4933..52ab50250e 100644 --- a/lms/djangoapps/experiments/views.py +++ b/lms/djangoapps/experiments/views.py @@ -6,15 +6,22 @@ Experimentation views from django.contrib.auth import get_user_model from django.db import transaction from django_filters.rest_framework import DjangoFilterBackend +from django.http import Http404 from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication from edx_rest_framework_extensions.auth.session.authentication import SessionAuthenticationAllowInactiveUser +from lms.djangoapps.courseware import courses +from opaque_keys.edx.keys import CourseKey from rest_framework import permissions, viewsets from rest_framework.response import Response +from rest_framework.views import APIView +from util.json_request import JsonResponse -from lms.djangoapps.experiments import filters, serializers -from lms.djangoapps.experiments.models import ExperimentData, ExperimentKeyValue -from lms.djangoapps.experiments.permissions import IsStaffOrOwner, IsStaffOrReadOnly +from experiments import filters, serializers +from experiments.models import ExperimentData, ExperimentKeyValue +from experiments.permissions import IsStaffOrOwner, IsStaffOrReadOnly, IsStaffOrReadOnlyForSelf +from experiments.utils import get_experiment_user_metadata_context from openedx.core.djangoapps.cors_csrf.authentication import SessionAuthenticationCrossDomainCsrf +from student.models import get_user_by_username_or_email User = get_user_model() # pylint: disable=invalid-name @@ -84,3 +91,27 @@ class ExperimentKeyValueViewSet(viewsets.ModelViewSet): permission_classes = (IsStaffOrReadOnly,) queryset = ExperimentKeyValue.objects.all() serializer_class = serializers.ExperimentKeyValueSerializer + + +class UserMetaDataView(APIView): + authentication_classes = (JwtAuthentication, ExperimentCrossDomainSessionAuth,) + permission_classes = (IsStaffOrReadOnlyForSelf,) + + def get(self, request, course_id=None, username=None): + """ Return user-metadata for the given course and user """ + try: + user = get_user_by_username_or_email(username) + except User.DoesNotExist: + # Note: this will only be seen by staff, for administrative de-bugging purposes + message = "Provided user is not found" + return JsonResponse({'message': message}, status=404) + + try: + course = courses.get_course_by_id(CourseKey.from_string(course_id)) + except Http404: + message = "Provided course is not found" + return JsonResponse({'message': message}, status=404) + + context = get_experiment_user_metadata_context(course, user) + user_metadata = context.get('user_metadata') + return JsonResponse(user_metadata) diff --git a/lms/templates/experiments/user_metadata.html b/lms/templates/experiments/user_metadata.html index 8600ec11e1..275893e57b 100644 --- a/lms/templates/experiments/user_metadata.html +++ b/lms/templates/experiments/user_metadata.html @@ -6,73 +6,7 @@ from opaque_keys.edx.keys import CourseKey import six %> <% -user_metadata = { - key: context.get(key) - for key in ( - 'username', - 'user_id', - 'course_id', - 'enrollment_mode', - 'upgrade_link', - 'upgrade_price', - 'audit_access_deadline', - 'course_duration', - 'pacing_type', - 'has_staff_access', - 'forum_roles', - 'partition_groups', - # TODO: clean up as part of REVO-28 (START) - 'has_non_audit_enrollments', - # TODO: clean up as part of REVO-28 (END) - # TODO: clean up as part of REVEM-199 (START) - 'program_key_fields', - # TODO: clean up as part of REVEM-199 (END) - ) -} - -if user: - user_metadata['username'] = user.username - user_metadata['user_id'] = user.id - if hasattr(user, 'email'): - user_metadata['email'] = user.email - -for datekey in ( - 'schedule_start', - 'enrollment_time', - 'course_start', - 'course_end', - 'dynamic_upgrade_deadline', - 'course_upgrade_deadline', - 'audit_access_deadline', -): - user_metadata[datekey] = ( - context.get(datekey).isoformat() if context.get(datekey) else None - ) - -for timedeltakey in ( - 'course_duration', -): - user_metadata[timedeltakey] = ( - context.get(timedeltakey).total_seconds() if context.get(timedeltakey) else None - ) - -course_key = context.get('course_key') -if course and not course_key: - course_key = course.id - -if course_key: - if isinstance(course_key, CourseKey): - user_metadata['course_key_fields'] = { - 'org': course_key.org, - 'course': course_key.course, - 'run': course_key.run, - } - - if not course_id: - user_metadata['course_id'] = six.text_type(course_key) - elif isinstance(course_key, six.string_types): - user_metadata['course_id'] = course_key - +user_metadata = context.get('user_metadata') %>