From 626f015a2efa37c25111a629124f4d1850543eea Mon Sep 17 00:00:00 2001 From: Clinton Blackburn Date: Tue, 13 Jun 2017 17:34:21 -0400 Subject: [PATCH] Added experiments app and API endpoint --- lms/djangoapps/experiments/__init__.py | 6 + lms/djangoapps/experiments/admin.py | 13 ++ lms/djangoapps/experiments/factories.py | 14 ++ lms/djangoapps/experiments/filters.py | 9 ++ .../experiments/migrations/0001_initial.py | 40 ++++++ .../experiments/migrations/__init__.py | 0 lms/djangoapps/experiments/models.py | 22 +++ lms/djangoapps/experiments/permissions.py | 12 ++ lms/djangoapps/experiments/routers.py | 46 +++++++ lms/djangoapps/experiments/serializers.py | 12 ++ lms/djangoapps/experiments/tests/__init__.py | 0 .../experiments/tests/test_views.py | 127 ++++++++++++++++++ lms/djangoapps/experiments/urls.py | 11 ++ lms/djangoapps/experiments/views.py | 43 ++++++ lms/envs/common.py | 2 + lms/urls.py | 3 +- 16 files changed, 358 insertions(+), 2 deletions(-) create mode 100644 lms/djangoapps/experiments/__init__.py create mode 100644 lms/djangoapps/experiments/admin.py create mode 100644 lms/djangoapps/experiments/factories.py create mode 100644 lms/djangoapps/experiments/filters.py create mode 100644 lms/djangoapps/experiments/migrations/0001_initial.py create mode 100644 lms/djangoapps/experiments/migrations/__init__.py create mode 100644 lms/djangoapps/experiments/models.py create mode 100644 lms/djangoapps/experiments/permissions.py create mode 100644 lms/djangoapps/experiments/routers.py create mode 100644 lms/djangoapps/experiments/serializers.py create mode 100644 lms/djangoapps/experiments/tests/__init__.py create mode 100644 lms/djangoapps/experiments/tests/test_views.py create mode 100644 lms/djangoapps/experiments/urls.py create mode 100644 lms/djangoapps/experiments/views.py diff --git a/lms/djangoapps/experiments/__init__.py b/lms/djangoapps/experiments/__init__.py new file mode 100644 index 0000000000..c706668768 --- /dev/null +++ b/lms/djangoapps/experiments/__init__.py @@ -0,0 +1,6 @@ +""" +This app is used by the Rapid Experiments Team to hold data related to experiments. + +WARNING: Do NOT use this app for anything long-term (or even short-tem) unless you are a member of the team. This app +is subject to change at any time without notice to those outside of the Rapid Experiments Team. +""" diff --git a/lms/djangoapps/experiments/admin.py b/lms/djangoapps/experiments/admin.py new file mode 100644 index 0000000000..feee039a09 --- /dev/null +++ b/lms/djangoapps/experiments/admin.py @@ -0,0 +1,13 @@ +from django.contrib import admin + +from .models import ExperimentData + + +@admin.register(ExperimentData) +class ExperimentDataAdmin(admin.ModelAdmin): + list_display = ('user', 'experiment_id', 'key',) + list_filter = ('experiment_id',) + ordering = ('experiment_id', 'user', 'key',) + raw_id_fields = ('user',) + readonly_fields = ('created', 'modified',) + search_fields = ('experiment_id', 'user', 'key',) diff --git a/lms/djangoapps/experiments/factories.py b/lms/djangoapps/experiments/factories.py new file mode 100644 index 0000000000..b718a246e0 --- /dev/null +++ b/lms/djangoapps/experiments/factories.py @@ -0,0 +1,14 @@ +import factory + +from experiments.models import ExperimentData +from student.tests.factories import UserFactory + + +class ExperimentDataFactory(factory.DjangoModelFactory): + class Meta(object): + model = ExperimentData + + user = factory.SubFactory(UserFactory) + experiment_id = factory.fuzzy.FuzzyInteger(0) + key = factory.Faker('word') + value = factory.Faker('word') diff --git a/lms/djangoapps/experiments/filters.py b/lms/djangoapps/experiments/filters.py new file mode 100644 index 0000000000..cfbf9be11e --- /dev/null +++ b/lms/djangoapps/experiments/filters.py @@ -0,0 +1,9 @@ +import django_filters + +from experiments.models import ExperimentData + + +class ExperimentDataFilter(django_filters.FilterSet): + class Meta(object): + model = ExperimentData + fields = ['experiment_id', 'key', ] diff --git a/lms/djangoapps/experiments/migrations/0001_initial.py b/lms/djangoapps/experiments/migrations/0001_initial.py new file mode 100644 index 0000000000..63aa19d55a --- /dev/null +++ b/lms/djangoapps/experiments/migrations/0001_initial.py @@ -0,0 +1,40 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models +from django.conf import settings +import django_extensions.db.fields + + +class Migration(migrations.Migration): + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.CreateModel( + name='ExperimentData', + fields=[ + ('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)), + ('created', django_extensions.db.fields.CreationDateTimeField(auto_now_add=True, verbose_name='created')), + ('modified', django_extensions.db.fields.ModificationDateTimeField(auto_now=True, verbose_name='modified')), + ('experiment_id', models.PositiveSmallIntegerField(verbose_name=b'Experiment ID', db_index=True)), + ('key', models.CharField(max_length=255)), + ('value', models.TextField()), + ('user', models.ForeignKey(to=settings.AUTH_USER_MODEL)), + ], + options={ + 'verbose_name': 'Experiment Data', + 'verbose_name_plural': 'Experiment Data', + }, + ), + migrations.AlterUniqueTogether( + name='experimentdata', + unique_together=set([('user', 'experiment_id', 'key')]), + ), + migrations.AlterIndexTogether( + name='experimentdata', + index_together=set([('user', 'experiment_id')]), + ), + ] diff --git a/lms/djangoapps/experiments/migrations/__init__.py b/lms/djangoapps/experiments/migrations/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/lms/djangoapps/experiments/models.py b/lms/djangoapps/experiments/models.py new file mode 100644 index 0000000000..008620ef36 --- /dev/null +++ b/lms/djangoapps/experiments/models.py @@ -0,0 +1,22 @@ +from django.conf import settings +from django.db import models +from django_extensions.db.models import TimeStampedModel + + +class ExperimentData(TimeStampedModel): + user = models.ForeignKey(settings.AUTH_USER_MODEL) + experiment_id = models.PositiveSmallIntegerField( + null=False, blank=False, db_index=True, verbose_name='Experiment ID' + ) + key = models.CharField(null=False, blank=False, max_length=255) + value = models.TextField() + + class Meta(object): + index_together = ( + ('user', 'experiment_id'), + ) + verbose_name = 'Experiment Data' + verbose_name_plural = 'Experiment Data' + unique_together = ( + ('user', 'experiment_id', 'key'), + ) diff --git a/lms/djangoapps/experiments/permissions.py b/lms/djangoapps/experiments/permissions.py new file mode 100644 index 0000000000..3e61be1eb5 --- /dev/null +++ b/lms/djangoapps/experiments/permissions.py @@ -0,0 +1,12 @@ +from openedx.core.lib.api import permissions + + +class IsStaffOrOwner(permissions.IsStaffOrOwner): + """ + Permission that allows access to admin users or the owner of an object. + The owner is considered the User object represented by obj.user. + """ + + def has_permission(self, request, view): + # The view will handle filtering for the current user + return True diff --git a/lms/djangoapps/experiments/routers.py b/lms/djangoapps/experiments/routers.py new file mode 100644 index 0000000000..d6161cfdf8 --- /dev/null +++ b/lms/djangoapps/experiments/routers.py @@ -0,0 +1,46 @@ +from rest_framework import routers +from rest_framework.routers import DynamicDetailRoute, DynamicListRoute, Route + + +class DefaultRouter(routers.DefaultRouter): + routes = [ + # List route. + Route( + url=r'^{prefix}{trailing_slash}$', + mapping={ + 'get': 'list', + 'post': 'create', + # Allow PUT as create + 'put': 'create_or_update', + }, + name='{basename}-list', + initkwargs={'suffix': 'List'} + ), + # Dynamically generated list routes. + # Generated using @list_route decorator + # on methods of the viewset. + DynamicListRoute( + url=r'^{prefix}/{methodname}{trailing_slash}$', + name='{basename}-{methodnamehyphen}', + initkwargs={} + ), + # Detail route. + Route( + url=r'^{prefix}/{lookup}{trailing_slash}$', + mapping={ + 'get': 'retrieve', + 'put': 'update', + 'patch': 'partial_update', + 'delete': 'destroy' + }, + name='{basename}-detail', + initkwargs={'suffix': 'Instance'} + ), + # Dynamically generated detail routes. + # Generated using @detail_route decorator on methods of the viewset. + DynamicDetailRoute( + url=r'^{prefix}/{lookup}/{methodname}{trailing_slash}$', + name='{basename}-{methodnamehyphen}', + initkwargs={} + ), + ] diff --git a/lms/djangoapps/experiments/serializers.py b/lms/djangoapps/experiments/serializers.py new file mode 100644 index 0000000000..c50433a692 --- /dev/null +++ b/lms/djangoapps/experiments/serializers.py @@ -0,0 +1,12 @@ +from rest_framework import serializers + +from .models import ExperimentData + + +class ExperimentDataSerializer(serializers.ModelSerializer): + user = serializers.SlugRelatedField(read_only=True, slug_field='username', default=serializers.CurrentUserDefault()) + + class Meta(object): + model = ExperimentData + fields = ('id', 'experiment_id', 'user', 'key', 'value', 'created', 'modified',) + read_only_fields = ('user',) diff --git a/lms/djangoapps/experiments/tests/__init__.py b/lms/djangoapps/experiments/tests/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/lms/djangoapps/experiments/tests/test_views.py b/lms/djangoapps/experiments/tests/test_views.py new file mode 100644 index 0000000000..817d4a8319 --- /dev/null +++ b/lms/djangoapps/experiments/tests/test_views.py @@ -0,0 +1,127 @@ +import urllib + +from django.core.urlresolvers import reverse +from rest_framework.test import APITestCase + +from experiments.factories import ExperimentDataFactory +from experiments.models import ExperimentData +from experiments.serializers import ExperimentDataSerializer +from student.tests.factories import UserFactory + + +class ExperimentDataViewSetTests(APITestCase): + def assert_data_created_for_user(self, user, method='post', status=201): + url = reverse('api_experiments:v0:data-list') + data = { + 'experiment_id': 1, + 'key': 'foo', + 'value': 'bar', + } + self.client.login(username=user.username, password=UserFactory._DEFAULT_PASSWORD) + response = getattr(self.client, method)(url, data) + self.assertEqual(response.status_code, status) + + # This will raise an exception if no data exists + ExperimentData.objects.get(user=user) + + data['user'] = user.username + self.assertDictContainsSubset(data, response.data) + + def test_list_permissions(self): + """ Users should only be able to list their own data. """ + url = reverse('api_experiments:v0:data-list') + user = UserFactory() + + response = self.client.get(url) + self.assertEqual(response.status_code, 401) + + ExperimentDataFactory() + datum = ExperimentDataFactory(user=user) + self.client.login(username=user.username, password=UserFactory._DEFAULT_PASSWORD) + + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + self.assertEqual(response.data['results'], ExperimentDataSerializer([datum], many=True).data) + + def test_list_filtering(self): + """ Users should be able to filter by the experiment_id and key fields. """ + url = reverse('api_experiments:v0:data-list') + user = UserFactory() + self.client.login(username=user.username, password=UserFactory._DEFAULT_PASSWORD) + + experiment_id = 1 + ExperimentDataFactory() + ExperimentDataFactory(user=user) + data = ExperimentDataFactory.create_batch(3, user=user, experiment_id=experiment_id) + + qs = urllib.urlencode({'experiment_id': experiment_id}) + response = self.client.get('{url}?{qs}'.format(url=url, qs=qs)) + self.assertEqual(response.status_code, 200) + self.assertEqual(response.data['results'], ExperimentDataSerializer(data, many=True).data) + + datum = data[0] + qs = urllib.urlencode({'key': datum.key}) + response = self.client.get('{url}?{qs}'.format(url=url, qs=qs)) + self.assertEqual(response.status_code, 200) + self.assertEqual(response.data['results'], ExperimentDataSerializer([datum], many=True).data) + + qs = urllib.urlencode({'experiment_id': experiment_id, 'key': datum.key}) + response = self.client.get('{url}?{qs}'.format(url=url, qs=qs)) + self.assertEqual(response.status_code, 200) + self.assertEqual(response.data['results'], ExperimentDataSerializer([datum], many=True).data) + + def test_read_permissions(self): + """ Users should only be allowed to read their own data. """ + user = UserFactory() + datum = ExperimentDataFactory(user=user) + url = reverse('api_experiments:v0:data-detail', kwargs={'pk': datum.id}) + + response = self.client.get(url) + self.assertEqual(response.status_code, 401) + + self.client.login(username=user.username, password=UserFactory._DEFAULT_PASSWORD) + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + + other_user = UserFactory() + self.client.login(username=other_user.username, password=UserFactory._DEFAULT_PASSWORD) + response = self.client.get(url) + self.assertEqual(response.status_code, 404) + + def test_create_permissions(self): + """ Users should only be allowed to create data for themselves. """ + url = reverse('api_experiments:v0:data-list') + + # Authentication is required + response = self.client.post(url, {}) + self.assertEqual(response.status_code, 401) + + self.assert_data_created_for_user(UserFactory()) + self.assert_data_created_for_user(UserFactory()) + + def test_put_as_create(self): + """ Users should be able to use PUT to create new data. """ + user = UserFactory() + self.assert_data_created_for_user(user, 'put') + + # Subsequent requests should update the data + self.assert_data_created_for_user(user, 'put', 200) + + def test_update_permissions(self): + """ Users should only be allowed to update their own data. """ + user = UserFactory() + other_user = UserFactory() + datum = ExperimentDataFactory(user=user) + url = reverse('api_experiments:v0:data-detail', kwargs={'pk': datum.id}) + data = {} + + response = self.client.patch(url, data) + self.assertEqual(response.status_code, 401) + + self.client.login(username=user.username, password=UserFactory._DEFAULT_PASSWORD) + response = self.client.patch(url, data) + self.assertEqual(response.status_code, 200) + + self.client.login(username=other_user.username, password=UserFactory._DEFAULT_PASSWORD) + response = self.client.patch(url, data) + self.assertEqual(response.status_code, 404) diff --git a/lms/djangoapps/experiments/urls.py b/lms/djangoapps/experiments/urls.py new file mode 100644 index 0000000000..635ea7b7e7 --- /dev/null +++ b/lms/djangoapps/experiments/urls.py @@ -0,0 +1,11 @@ +from django.conf.urls import include, url + +from experiments import routers +from experiments.views import ExperimentDataViewSet + +router = routers.DefaultRouter() +router.register(r'data', ExperimentDataViewSet, base_name='data') + +urlpatterns = [ + url(r'^v0/', include(router.urls, namespace='v0')), +] diff --git a/lms/djangoapps/experiments/views.py b/lms/djangoapps/experiments/views.py new file mode 100644 index 0000000000..4f6bf93d3e --- /dev/null +++ b/lms/djangoapps/experiments/views.py @@ -0,0 +1,43 @@ +from edx_rest_framework_extensions.authentication import JwtAuthentication +from rest_framework import permissions, viewsets +from rest_framework.filters import DjangoFilterBackend + +from experiments import filters +from experiments.models import ExperimentData +from experiments.permissions import IsStaffOrOwner +from experiments.serializers import ExperimentDataSerializer +from openedx.core.lib.api.authentication import SessionAuthenticationAllowInactiveUser + + +class ExperimentDataViewSet(viewsets.ModelViewSet): + authentication_classes = (JwtAuthentication, SessionAuthenticationAllowInactiveUser,) + filter_backends = (DjangoFilterBackend,) + filter_class = filters.ExperimentDataFilter + permission_classes = (permissions.IsAuthenticated, IsStaffOrOwner,) + queryset = ExperimentData.objects.all() + serializer_class = ExperimentDataSerializer + + def filter_queryset(self, queryset): + queryset = queryset.filter(user=self.request.user) + return super(ExperimentDataViewSet, self).filter_queryset(queryset) + + def create_or_update(self, request, *args, **kwargs): + # If we have a primary key, treat this as a regular update request + if self.kwargs.get('pk'): + return self.update(request, *args, **kwargs) + + # If we only have data, check to see if an instance exists in the database. If so, update it. + # Otherwise, create a new instance. + experiment_id = request.data.get('experiment_id') + key = request.data.get('key') + + if experiment_id and key: + try: + obj = self.get_queryset().get(user=self.request.user, experiment_id=experiment_id, key=key) + self.kwargs['pk'] = obj.pk + self.request.data['id'] = obj.pk + return self.update(request, *args, **kwargs) + except ExperimentData.DoesNotExist: + pass + + return self.create(request, *args, **kwargs) diff --git a/lms/envs/common.py b/lms/envs/common.py index 72e0b9939f..84b675ffb9 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -2232,6 +2232,8 @@ INSTALLED_APPS = ( 'openedx.features.course_experience', 'openedx.features.course_search', 'openedx.features.enterprise_support', + + 'experiments', ) ######################### CSRF ######################################### diff --git a/lms/urls.py b/lms/urls.py index 6f12d30e13..ba5e524b53 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -108,10 +108,9 @@ urlpatterns = ( # URLs for API access management url(r'^api-admin/', include('openedx.core.djangoapps.api_admin.urls', namespace='api_admin')), -) -urlpatterns += ( url(r'^dashboard/', include('learner_dashboard.urls')), + url(r'^api/experiments/', include('experiments.urls', namespace='api_experiments')), ) # TODO: This needs to move to a separate urls.py once the student_account and