Permalink
Browse files

[bug 933786] Fix strip error

DRF allows nulls as valid values for CharField. This creates
NoNullsCharField which restores sanity to the world.

This fixes the strip error by raising a ValidationError which will send
an error to the client rather than letting the value through and having
it throw an AttributeError where it shouldn't be.
  • Loading branch information...
1 parent f5fc069 commit 2324aa34f5c23d8b1b19166733a108a925c104ae @willkg willkg committed Feb 11, 2014
Showing with 42 additions and 8 deletions.
  1. +22 −8 fjord/feedback/models.py
  2. +20 −0 fjord/feedback/tests/test_api.py
View
@@ -1,6 +1,7 @@
from datetime import datetime
from django.core.cache import cache
+from django.core.exceptions import ValidationError
from django.db import models
from elasticutils.contrib.django import Indexable
@@ -239,6 +240,19 @@ class ResponseEmail(ModelBase):
email = models.EmailField()
+class NoNullsCharField(serializers.CharField):
+ """Further restricts CharField so it doesn't accept nulls
+
+ DRF lets CharFields take nulls which is not what I want. This
+ raises a ValidationError if the value is a null.
+
+ """
+ def from_native(self, value):
+ if value is None:
+ raise ValidationError('Value cannot be null')
+ return super(NoNullsCharField, self).from_native(value)
+
+
class ResponseSerializer(serializers.Serializer):
"""This handles incoming feedback
@@ -254,16 +268,16 @@ class ResponseSerializer(serializers.Serializer):
# browser since those don't make sense.
# product, channel, version, locale, platform
- product = serializers.CharField(max_length=20, required=True)
- channel = serializers.CharField(max_length=30, required=False, default=u'')
- version = serializers.CharField(max_length=30, required=False, default=u'')
- locale = serializers.CharField(max_length=8, required=False, default=u'')
- platform = serializers.CharField(max_length=30, required=False, default=u'')
- country = serializers.CharField(max_length=4, required=False, default=u'')
+ product = NoNullsCharField(max_length=20, required=True)
+ channel = NoNullsCharField(max_length=30, required=False, default=u'')
+ version = NoNullsCharField(max_length=30, required=False, default=u'')
+ locale = NoNullsCharField(max_length=8, required=False, default=u'')
+ platform = NoNullsCharField(max_length=30, required=False, default=u'')
+ country = NoNullsCharField(max_length=4, required=False, default=u'')
# device information
- manufacturer = serializers.CharField(required=False, default=u'')
- device = serializers.CharField(required=False, default=u'')
+ manufacturer = NoNullsCharField(required=False, default=u'')
+ device = NoNullsCharField(required=False, default=u'')
# user's email address
email = serializers.EmailField(required=False)
@@ -102,6 +102,26 @@ def test_with_email(self):
email = models.ResponseEmail.objects.latest(field_name='id')
eq_(email.email, data['email'])
+ def test_null_device_returns_400(self):
+ data = {
+ 'happy': True,
+ 'description': u'Great!',
+ 'product': u'Firefox OS',
+ 'device': None
+ }
+
+ r = self.client.post(reverse('api-post-feedback'), data)
+ eq_(r.status_code, 201)
+
+ feedback = models.Response.objects.latest(field_name='id')
+ eq_(feedback.happy, True)
+ eq_(feedback.description, data['description'])
+ eq_(feedback.product, data['product'])
+
+ # Fills in defaults
+ eq_(feedback.url, u'')
+ eq_(feedback.user_agent, u'api')
+
def test_invalid_email_address_returns_400(self):
data = {
'happy': True,

0 comments on commit 2324aa3

Please sign in to comment.