Merge pull request #22596 from edx/diana/upgrade-drf
Upgrade DRF to 3.9.4.
This commit is contained in:
@@ -407,7 +407,7 @@ class CommentSerializer(_ContentSerializer):
|
||||
"parent_id does not identify a comment in the thread identified by thread_id."
|
||||
)
|
||||
if is_comment_too_deep(parent):
|
||||
raise ValidationError({"parent_id": ["Comment level is too deep."]})
|
||||
raise ValidationError("Comment level is too deep.")
|
||||
return attrs
|
||||
|
||||
def create(self, validated_data):
|
||||
|
||||
@@ -4,7 +4,7 @@ Experimentation routers
|
||||
|
||||
|
||||
from rest_framework import routers
|
||||
from rest_framework.routers import DynamicDetailRoute, DynamicListRoute, Route
|
||||
from rest_framework.routers import DynamicRoute, Route
|
||||
|
||||
|
||||
class DefaultRouter(routers.DefaultRouter):
|
||||
@@ -19,14 +19,16 @@ class DefaultRouter(routers.DefaultRouter):
|
||||
'put': 'create_or_update',
|
||||
},
|
||||
name='{basename}-list',
|
||||
detail=False,
|
||||
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}',
|
||||
DynamicRoute(
|
||||
url=r'^{prefix}/{lookup}{trailing_slash}$',
|
||||
name='{basename}-list',
|
||||
detail=False,
|
||||
initkwargs={}
|
||||
),
|
||||
# Detail route.
|
||||
@@ -39,13 +41,15 @@ class DefaultRouter(routers.DefaultRouter):
|
||||
'delete': 'destroy'
|
||||
},
|
||||
name='{basename}-detail',
|
||||
detail=True,
|
||||
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}',
|
||||
DynamicRoute(
|
||||
url=r'^{prefix}/{lookup}{trailing_slash}$',
|
||||
name='{basename}-detail',
|
||||
detail=True,
|
||||
initkwargs={}
|
||||
),
|
||||
]
|
||||
|
||||
@@ -165,65 +165,6 @@ class ExperimentDataViewSetTests(APITestCase):
|
||||
response = self.client.patch(url, data)
|
||||
self.assertEqual(response.status_code, 404)
|
||||
|
||||
def test_bulk_upsert_permissions(self):
|
||||
""" Only staff users can access the bulk upsert endpoint. """
|
||||
url = reverse('api_experiments:v0:data-bulk-upsert')
|
||||
data = []
|
||||
|
||||
# Authentication is required
|
||||
response = self.client.put(url, data, format='json')
|
||||
self.assertEqual(response.status_code, 401)
|
||||
|
||||
user = UserFactory()
|
||||
self.client.login(username=user.username, password=UserFactory._DEFAULT_PASSWORD)
|
||||
|
||||
# No access to non-staff users
|
||||
response = self.client.put(url, data, format='json')
|
||||
self.assertEqual(response.status_code, 403)
|
||||
|
||||
user.is_staff = True
|
||||
user.save()
|
||||
response = self.client.put(url, data, format='json')
|
||||
self.assertEqual(response.status_code, 200)
|
||||
|
||||
def test_bulk_upsert(self):
|
||||
""" The endpoint should support creating/updating multiple ExperimentData objects with a single call. """
|
||||
url = reverse('api_experiments:v0:data-bulk-upsert')
|
||||
experiment_id = 1
|
||||
user = UserFactory(is_staff=True)
|
||||
other_user = UserFactory()
|
||||
self.client.login(username=user.username, password=UserFactory._DEFAULT_PASSWORD)
|
||||
|
||||
data = [
|
||||
{
|
||||
'experiment_id': experiment_id,
|
||||
'key': 'foo',
|
||||
'value': 'bar',
|
||||
'user': user.username,
|
||||
},
|
||||
{
|
||||
'experiment_id': experiment_id,
|
||||
'key': 'foo',
|
||||
'value': 'bar',
|
||||
'user': other_user.username,
|
||||
},
|
||||
]
|
||||
response = self.client.put(url, data, format='json')
|
||||
self.assertEqual(response.status_code, 200)
|
||||
kwargs = {
|
||||
'experiment_id': experiment_id,
|
||||
'key': 'foo',
|
||||
'value': 'bar',
|
||||
}
|
||||
ExperimentData.objects.get(user=user, **kwargs)
|
||||
ExperimentData.objects.get(user=other_user, **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)
|
||||
ExperimentData.objects.get(user=user, **kwargs)
|
||||
ExperimentData.objects.get(user=other_user, **kwargs)
|
||||
|
||||
|
||||
def cross_domain_config(func):
|
||||
"""Decorator for configuring a cross-domain request. """
|
||||
@@ -347,54 +288,3 @@ class ExperimentKeyValueViewSetTests(APITestCase):
|
||||
|
||||
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)
|
||||
|
||||
@@ -9,7 +9,6 @@ from django_filters.rest_framework import DjangoFilterBackend
|
||||
from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication
|
||||
from edx_rest_framework_extensions.auth.session.authentication import SessionAuthenticationAllowInactiveUser
|
||||
from rest_framework import permissions, viewsets
|
||||
from rest_framework.decorators import list_route
|
||||
from rest_framework.response import Response
|
||||
|
||||
from experiments import filters, serializers
|
||||
@@ -77,21 +76,6 @@ class ExperimentDataViewSet(viewsets.ModelViewSet):
|
||||
|
||||
return user
|
||||
|
||||
@list_route(methods=['put'], permission_classes=[permissions.IsAdminUser])
|
||||
def bulk_upsert(self, request):
|
||||
upserted = []
|
||||
self._cache_users([datum['user'] for datum in request.data])
|
||||
|
||||
with transaction.atomic():
|
||||
for item in request.data:
|
||||
user = self._get_user(username=item['user'])
|
||||
datum, __ = ExperimentData.objects.update_or_create(
|
||||
user=user, 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)
|
||||
|
||||
|
||||
class ExperimentKeyValueViewSet(viewsets.ModelViewSet):
|
||||
authentication_classes = (JwtAuthentication, ExperimentCrossDomainSessionAuth,)
|
||||
@@ -100,16 +84,3 @@ class ExperimentKeyValueViewSet(viewsets.ModelViewSet):
|
||||
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)
|
||||
|
||||
@@ -13,7 +13,6 @@ from django.utils.translation import ugettext as _
|
||||
from django.utils.translation import ugettext_noop
|
||||
from django_countries import countries
|
||||
from pytz import common_timezones, common_timezones_set, country_timezones
|
||||
from six import text_type
|
||||
|
||||
from openedx.core.lib.time_zone_utils import get_display_time_zone
|
||||
from student.models import User, UserProfile
|
||||
@@ -279,7 +278,9 @@ def update_email_opt_in(user, org, opt_in):
|
||||
if hasattr(settings, 'LMS_SEGMENT_KEY') and settings.LMS_SEGMENT_KEY:
|
||||
_track_update_email_opt_in(user.id, org, opt_in)
|
||||
except IntegrityError as err:
|
||||
log.warning(u"Could not update organization wide preference due to IntegrityError: {}".format(text_type(err)))
|
||||
log.warning(
|
||||
u"Could not update organization wide preference due to IntegrityError: {}".format(six.text_type(err))
|
||||
)
|
||||
|
||||
|
||||
def _track_update_email_opt_in(user_id, organization, opt_in):
|
||||
@@ -386,8 +387,13 @@ def validate_user_preference_serializer(serializer, preference_key, preference_v
|
||||
}
|
||||
})
|
||||
if not serializer.is_valid():
|
||||
errors = serializer.errors
|
||||
# DRF error messages are of type ErrorDetail and serialize out as such. We want to coerce those
|
||||
# messages into the strings only.
|
||||
for key in errors:
|
||||
errors[key] = [six.text_type(el) for el in errors[key]]
|
||||
developer_message = u"Value '{preference_value}' not valid for preference '{preference_key}': {error}".format(
|
||||
preference_key=preference_key, preference_value=preference_value, error=serializer.errors
|
||||
preference_key=preference_key, preference_value=preference_value, error=errors
|
||||
)
|
||||
if "key" in serializer.errors:
|
||||
user_message = _(u"Invalid user preference key '{preference_key}'.").format(
|
||||
|
||||
@@ -13,7 +13,7 @@ from edx_rest_framework_extensions.auth.session.authentication import SessionAut
|
||||
from opaque_keys import InvalidKeyError
|
||||
from opaque_keys.edx.keys import CourseKey
|
||||
from rest_framework import status
|
||||
from rest_framework.exceptions import APIException
|
||||
from rest_framework.exceptions import APIException, ErrorDetail
|
||||
from rest_framework.generics import GenericAPIView
|
||||
from rest_framework.mixins import RetrieveModelMixin, UpdateModelMixin
|
||||
from rest_framework.permissions import IsAuthenticated
|
||||
@@ -132,11 +132,28 @@ def view_auth_classes(is_user=False, is_authenticated=True):
|
||||
return _decorator
|
||||
|
||||
|
||||
def clean_errors(error):
|
||||
"""
|
||||
DRF error messages are of type ErrorDetail and serialize out as such.
|
||||
We want to coerce the strings into the message only.
|
||||
|
||||
This cursively handles the nesting of errors.
|
||||
"""
|
||||
if isinstance(error, ErrorDetail):
|
||||
return text_type(error)
|
||||
if isinstance(error, list):
|
||||
return [clean_errors(el) for el in error]
|
||||
else:
|
||||
# We assume that it's a nested dictionary if it's not a list.
|
||||
return {key: clean_errors(value) for key, value in error.items()}
|
||||
|
||||
|
||||
def add_serializer_errors(serializer, data, field_errors):
|
||||
"""Adds errors from serializer validation to field_errors. data is the original data to deserialize."""
|
||||
if not serializer.is_valid():
|
||||
errors = serializer.errors
|
||||
for key, error in iteritems(errors):
|
||||
error = clean_errors(error)
|
||||
field_errors[key] = {
|
||||
'developer_message': u"Value '{field_value}' is not valid for field '{field_name}': {error}".format(
|
||||
field_value=data.get(key, ''), field_name=key, error=error
|
||||
|
||||
@@ -64,7 +64,7 @@ django-storages
|
||||
django-user-tasks
|
||||
django-waffle==0.12.0
|
||||
django-webpack-loader # Used to wire webpack bundles into the django asset pipeline
|
||||
djangorestframework==3.7.7
|
||||
djangorestframework==3.9.4
|
||||
djangorestframework-jwt
|
||||
drf-yasg # Replacement for django-rest-swagger
|
||||
edx-ace
|
||||
|
||||
@@ -89,7 +89,7 @@ django==1.11.27
|
||||
djangorestframework-jwt==1.11.0
|
||||
git+https://github.com/edx/django-rest-framework-oauth.git@0a43e8525f1e3048efe4bc70c03de308a277197c#egg=djangorestframework-oauth==1.1.1
|
||||
djangorestframework-xml==1.4.0 # via edx-enterprise
|
||||
djangorestframework==3.7.7
|
||||
djangorestframework==3.9.4
|
||||
docopt==0.6.2
|
||||
docutils==0.15.2 # via botocore
|
||||
drf-yasg==1.16
|
||||
|
||||
@@ -102,7 +102,7 @@ django==1.11.27
|
||||
djangorestframework-jwt==1.11.0
|
||||
git+https://github.com/edx/django-rest-framework-oauth.git@0a43e8525f1e3048efe4bc70c03de308a277197c#egg=djangorestframework-oauth==1.1.1
|
||||
djangorestframework-xml==1.4.0
|
||||
djangorestframework==3.7.7
|
||||
djangorestframework==3.9.4
|
||||
docker==4.1.0
|
||||
docopt==0.6.2
|
||||
docutils==0.15.2
|
||||
|
||||
@@ -99,7 +99,7 @@ django-webpack-loader==0.6.0
|
||||
djangorestframework-jwt==1.11.0
|
||||
git+https://github.com/edx/django-rest-framework-oauth.git@0a43e8525f1e3048efe4bc70c03de308a277197c#egg=djangorestframework-oauth==1.1.1
|
||||
djangorestframework-xml==1.4.0
|
||||
djangorestframework==3.7.7
|
||||
djangorestframework==3.9.4
|
||||
docker==4.1.0 # via moto
|
||||
docopt==0.6.2
|
||||
docutils==0.15.2
|
||||
|
||||
Reference in New Issue
Block a user