Skip to content

Commit

Permalink
Added model and API endpoints for experiment-scoped data
Browse files Browse the repository at this point in the history
  • Loading branch information
Clinton Blackburn authored and clintonb committed Jun 28, 2017
1 parent be51c71 commit beba48a
Show file tree
Hide file tree
Showing 9 changed files with 194 additions and 15 deletions.
11 changes: 10 additions & 1 deletion lms/djangoapps/experiments/factories.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import factory

from experiments.models import ExperimentData
from experiments.models import ExperimentData, ExperimentKeyValue
from student.tests.factories import UserFactory


Expand All @@ -12,3 +12,12 @@ class Meta(object):
experiment_id = factory.fuzzy.FuzzyInteger(0)
key = factory.Sequence(lambda n: n)
value = factory.Faker('word')


class ExperimentKeyValueFactory(factory.DjangoModelFactory):
class Meta(object):
model = ExperimentKeyValue

experiment_id = factory.fuzzy.FuzzyInteger(0)
key = factory.Sequence(lambda n: n)
value = factory.Faker('word')
8 changes: 7 additions & 1 deletion lms/djangoapps/experiments/filters.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,15 @@
import django_filters

from experiments.models import ExperimentData
from experiments.models import ExperimentData, ExperimentKeyValue


class ExperimentDataFilter(django_filters.FilterSet):
class Meta(object):
model = ExperimentData
fields = ['experiment_id', 'key', ]


class ExperimentKeyValueFilter(django_filters.FilterSet):
class Meta(object):
model = ExperimentKeyValue
fields = ['experiment_id', 'key', ]
34 changes: 34 additions & 0 deletions lms/djangoapps/experiments/migrations/0002_auto_20170627_1402.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# -*- coding: utf-8 -*-
from __future__ import unicode_literals

from django.db import migrations, models
import django_extensions.db.fields


class Migration(migrations.Migration):

dependencies = [
('experiments', '0001_initial'),
]

operations = [
migrations.CreateModel(
name='ExperimentKeyValue',
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()),
],
options={
'verbose_name': 'Experiment Data',
'verbose_name_plural': 'Experiment Data',
},
),
migrations.AlterUniqueTogether(
name='experimentkeyvalue',
unique_together=set([('experiment_id', 'key')]),
),
]
15 changes: 15 additions & 0 deletions lms/djangoapps/experiments/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,18 @@ class Meta(object):
unique_together = (
('user', 'experiment_id', 'key'),
)


class ExperimentKeyValue(TimeStampedModel):
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):
verbose_name = 'Experiment Data'
verbose_name_plural = 'Experiment Data'
unique_together = (
('experiment_id', 'key'),
)
7 changes: 7 additions & 0 deletions lms/djangoapps/experiments/permissions.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from rest_framework.permissions import SAFE_METHODS, BasePermission

from openedx.core.lib.api import permissions


Expand All @@ -17,3 +19,8 @@ def has_permission(self, request, view):

# The view will handle filtering for the current user
return True


class IsStaffOrReadOnly(BasePermission):
def has_permission(self, request, view):
return request.user.is_staff or request.method in SAFE_METHODS
8 changes: 7 additions & 1 deletion lms/djangoapps/experiments/serializers.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from django.contrib.auth import get_user_model
from rest_framework import serializers

from .models import ExperimentData
from .models import ExperimentData, ExperimentKeyValue

User = get_user_model() # pylint:disable=invalid-name

Expand All @@ -20,3 +20,9 @@ class ExperimentDataSerializer(serializers.ModelSerializer):

class Meta(ExperimentDataCreateSerializer.Meta):
read_only_fields = ('user',)


class ExperimentKeyValueSerializer(serializers.ModelSerializer):
class Meta(object):
model = ExperimentKeyValue
fields = ('id', 'experiment_id', 'key', 'value', 'created', 'modified',)
85 changes: 83 additions & 2 deletions lms/djangoapps/experiments/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
from django.core.urlresolvers import reverse
from rest_framework.test import APITestCase

from experiments.factories import ExperimentDataFactory
from experiments.models import ExperimentData
from experiments.factories import ExperimentDataFactory, ExperimentKeyValueFactory
from experiments.models import ExperimentData, ExperimentKeyValue
from experiments.serializers import ExperimentDataSerializer
from student.tests.factories import UserFactory

Expand Down Expand Up @@ -208,3 +208,84 @@ def test_bulk_upsert(self):
self.assertEqual(response.status_code, 200)
ExperimentData.objects.get(user=user, **kwargs)
ExperimentData.objects.get(user=other_user, **kwargs)


class ExperimentKeyValueViewSetTests(APITestCase):
def test_permissions(self):
""" Staff access is required for write operations. """
url = reverse('api_experiments:v0:key_value-list')

response = self.client.get(url)
self.assertEqual(response.status_code, 200)

response = self.client.post(url, {})
self.assertEqual(response.status_code, 401)

instance = ExperimentKeyValueFactory()
url = reverse('api_experiments:v0:key_value-detail', kwargs={'pk': instance.id})

response = self.client.get(url)
self.assertEqual(response.status_code, 200)

user = UserFactory(is_staff=False)
self.client.login(username=user.username, password=UserFactory._DEFAULT_PASSWORD)

response = self.client.put(url, {})
self.assertEqual(response.status_code, 403)

response = self.client.patch(url, {})
self.assertEqual(response.status_code, 403)

response = self.client.delete(url)
self.assertEqual(response.status_code, 403)

def test_bulk_upsert_permissions(self):
""" Non-staff users should not be allowed to access the endpoint. """
data = []
url = reverse('api_experiments:v0:key_value-bulk-upsert')
user = UserFactory(is_staff=False)

# Authentication required
response = self.client.put(url, data, format='json')
self.assertEqual(response.status_code, 401)

# Staff permission required
self.client.login(username=user.username, password=UserFactory._DEFAULT_PASSWORD)
response = self.client.put(url, data, format='json')
self.assertEqual(response.status_code, 403)

def test_bulk_upsert(self):
""" The endpoint should support creating/updating multiple ExperimentData objects with a single call. """
url = reverse('api_experiments:v0:key_value-bulk-upsert')
experiment_id = 1
user = UserFactory(is_staff=True)
data = [
{
'experiment_id': experiment_id,
'key': 'foo',
'value': 'bar',
},
{
'experiment_id': experiment_id,
'key': 'foo1',
'value': 'bar',
},
]

self.client.login(username=user.username, password=UserFactory._DEFAULT_PASSWORD)

# New data should be created
response = self.client.put(url, data, format='json')
self.assertEqual(response.status_code, 200)
kwargs = {
'experiment_id': experiment_id,
'value': 'bar',
}
ExperimentKeyValue.objects.get(key='foo', **kwargs)
ExperimentKeyValue.objects.get(key='foo1', **kwargs)

# Subsequent calls should update the existing data rather than create more
response = self.client.put(url, data, format='json')
self.assertEqual(response.status_code, 200)
ExperimentKeyValue.objects.get(key='foo', **kwargs)
ExperimentKeyValue.objects.get(key='foo1', **kwargs)
6 changes: 3 additions & 3 deletions lms/djangoapps/experiments/urls.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
from django.conf.urls import include, url

from experiments import routers
from experiments.views import ExperimentDataViewSet
from experiments import routers, views

router = routers.DefaultRouter()
router.register(r'data', ExperimentDataViewSet, base_name='data')
router.register(r'data', views.ExperimentDataViewSet, base_name='data')
router.register(r'key-value', views.ExperimentKeyValueViewSet, base_name='key_value')

urlpatterns = [
url(r'^v0/', include(router.urls, namespace='v0')),
Expand Down
35 changes: 28 additions & 7 deletions lms/djangoapps/experiments/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,9 @@
from rest_framework.filters import DjangoFilterBackend
from rest_framework.response import Response

from experiments import filters
from experiments.models import ExperimentData
from experiments.permissions import IsStaffOrOwner
from experiments.serializers import ExperimentDataCreateSerializer, ExperimentDataSerializer
from experiments import filters, serializers
from experiments.models import ExperimentData, ExperimentKeyValue
from experiments.permissions import IsStaffOrOwner, IsStaffOrReadOnly
from openedx.core.lib.api.authentication import SessionAuthenticationAllowInactiveUser

User = get_user_model() # pylint: disable=invalid-name
Expand All @@ -21,7 +20,7 @@ class ExperimentDataViewSet(viewsets.ModelViewSet):
filter_class = filters.ExperimentDataFilter
permission_classes = (permissions.IsAuthenticated, IsStaffOrOwner,)
queryset = ExperimentData.objects.all()
serializer_class = ExperimentDataSerializer
serializer_class = serializers.ExperimentDataSerializer
_cached_users = {}

def filter_queryset(self, queryset):
Expand All @@ -30,8 +29,8 @@ def filter_queryset(self, queryset):

def get_serializer_class(self):
if self.action == 'create':
return ExperimentDataCreateSerializer
return ExperimentDataSerializer
return serializers.ExperimentDataCreateSerializer
return serializers.ExperimentDataSerializer

def create_or_update(self, request, *args, **kwargs):
# If we have a primary key, treat this as a regular update request
Expand Down Expand Up @@ -82,3 +81,25 @@ def bulk_upsert(self, request):

serializer = self.get_serializer(upserted, many=True)
return Response(serializer.data)


class ExperimentKeyValueViewSet(viewsets.ModelViewSet):
authentication_classes = (JwtAuthentication, SessionAuthenticationAllowInactiveUser,)
filter_backends = (DjangoFilterBackend,)
filter_class = filters.ExperimentKeyValueFilter
permission_classes = (IsStaffOrReadOnly,)
queryset = ExperimentKeyValue.objects.all()
serializer_class = serializers.ExperimentKeyValueSerializer

@list_route(methods=['put'], permission_classes=[permissions.IsAdminUser])
def bulk_upsert(self, request):
upserted = []

with transaction.atomic():
for item in request.data:
datum, __ = ExperimentKeyValue.objects.update_or_create(
experiment_id=item['experiment_id'], key=item['key'], defaults={'value': item['value']})
upserted.append(datum)

serializer = self.get_serializer(upserted, many=True)
return Response(serializer.data)

0 comments on commit beba48a

Please sign in to comment.