EDUCATOR-782 | Django 1.11 upgrade.

.gitignore

@@ -66,5 +66,6 @@ logs/*/*.log*
 .vagrant
 
 venv/
+venvs/
 
 video-images/

.travis.yml

 language: python
 python:
-  - "2.7"
+- '2.7'
 install:
-  - "pip install -r requirements.txt"
-  - "pip install -r test-requirements.txt"
-  - "pip install coveralls"
+- pip install -r requirements.txt
+- pip install -r test-requirements.txt
+- pip install coveralls
 script:
-  - "python manage.py test"
+- tox
 branches:
-    only:
-      - master
-after_success:
-  coveralls
+  only:
+  - master
+after_success: coveralls
+# Set password via "travis encrypt --add deploy.password"; for details, see
+# https://docs.travis-ci.com/user/deployment/pypi
+deploy:
+  provider: pypi
+  user: edx
+  distributions: sdist bdist_wheel
+  skip_upload_docs: true
+  on:
+    tags: true
+  password:
+    secure: JGKw0H2Hl1i3OuJ+54hGX/YImhWe+MvkkeFZDynpBKir8XD+E/fbz2lvvmggIrMsawkE5qlOjK8f4d4LMzwL51Ln0EmuIkhUceXLDihrEymZu97uAWbT5qR55RrfwZk/qaYV8JHAszBD3yLA5VOyDLTiDt9s2ilZDQqQqecX514=

MANIFEST.in

 include requirements.txt
 include test-requirements.txt
+include django-requirements.txt
 include LICENSE
 include AUTHORS
 include README.rst

README.md

@@ -30,3 +30,16 @@ Returns (dict):
 }
 ```
  
+Developing
+-----------------
+First, create a virtual environment:
+```
+virtualenv venvs/val
+source venvs/val/bin/activate
+```
+
+To run tests:
+```
+pip install -r test-requirements.txt
+tox  # to run only a single environment, do e.g. tox -e django18
+```

django-requirements.txt

+Django>=1.8,<2

edxval/models.py

@@ -206,13 +206,19 @@ class ListField(models.TextField):
     """
     ListField use to store and retrieve list data.
     """
-    __metaclass__ = models.SubfieldBase
-
     def get_prep_value(self, value):
         """
         Converts a list to its json represetation to store in database as text.
         """
-        return json.dumps(value)
+        if value and not isinstance(value, list):
+            raise ValidationError(u'ListField value {} is not a list.'.format(value))
+        return json.dumps(self.validate(value) or [])
+
+    def from_db_value(self, value, expression, connection, context):
+        """
+        Converts a json list representation in a database to a python object.
+        """
+        return self.to_python(value)
 
     def to_python(self, value):
         """

edxval/tests/test_api.py

@@ -2,6 +2,7 @@
 """
 Tests for the API for Video Abstraction Layer
 """
+import json
 
 import mock
 from mock import patch
@@ -1306,8 +1307,10 @@ class CourseVideoImageTest(TestCase):
         video_data = list(video_data_generator)[0]
         self.assertEqual(video_data['courses'][0]['test-course'], self.image_url)
 
+    # attr_class is django magic: https://github.com/django/django/blob/master/django/db/models/fields/files.py#L214
+    @patch('edxval.models.CustomizableImageField.attr_class.save', side_effect = Exception("pretend save doesn't work"))
     @patch('edxval.models.logger')
-    def test_create_or_update_logging(self, mock_logger):
+    def test_create_or_update_logging(self, mock_logger, mock_image_save):
         """
         Tests correct message is logged when save to storge is failed in `create_or_update`.
         """
@@ -1364,6 +1367,7 @@ class CourseVideoImageTest(TestCase):
         # expect a validation error if we try to set a list with more than 3 items
         with self.assertRaises(ValidationError) as set_exception:
             video_image.generated_images = ['a', 'b', 'c', 'd']
+            video_image.save()
 
         self.assertEqual(
             set_exception.exception.message,
@@ -1373,19 +1377,45 @@ class CourseVideoImageTest(TestCase):
         # expect a validation error if we try to a list with non-string items
         with self.assertRaises(ValidationError) as set_exception:
             video_image.generated_images = ['a', 1, 2]
+            video_image.save()
 
         self.assertEqual(set_exception.exception.message, u'list must only contain strings.')
 
         # expect a validation error if we try to set non list data
-        exception_messages = set()
-        for item in ('a string', 555, {'a': 1}, (1,), video_image):
-            with self.assertRaises(ValidationError) as set_exception:
+        for item in ('a string', 555, {'a': 1}, (1,)):
+            with self.assertRaisesRegexp(ValidationError, 'is not a list') as set_exception:
                 video_image.generated_images = item
+                video_image.save()
 
-            exception_messages.add(set_exception.exception.message)
+    def test_video_image_urls_field_to_python_exceptions(self):
+        """
+        Test that `VideoImage.generated_images` field raises ValidationErrors
+        when bad data is somehow in the database for this field.
+        """
+        course_video = CourseVideo.objects.create(video=self.video, course_id='course101')
+        video_image = VideoImage.objects.create(course_video=course_video)
 
-        self.assertEqual(len(exception_messages), 1)
-        self.assertEqual(exception_messages.pop(), u'Must be a valid list of strings.')
+        # Tests that a TypeError is raised and turned into a ValidationError
+        # when the value is not a list (but still JSON serializable).
+        # We patch ListField.get_prep_value() to just do a json dumps without
+        # doing any type checking.
+        video_image.generated_images = {'key': 'image.jpeg'}
+        with patch('edxval.models.ListField.get_prep_value', lambda _, value: json.dumps(value)):
+            video_image.save()
+
+        with self.assertRaisesRegexp(ValidationError, 'Must be a valid list of strings'):
+            video_image.refresh_from_db()
+
+        # Tests that a ValueError is raised and turned into a ValidationError
+        # when the value is not JSON serializable.
+        # We patch ListField.get_prep_value() to let anything
+        # be saved in this field.
+        video_image.generated_images = 'image.jpeg'
+        with patch('edxval.models.ListField.get_prep_value', lambda _, value: value):
+            video_image.save()
+
+        with self.assertRaisesRegexp(ValidationError, 'Must be a valid list of strings'):
+            video_image.refresh_from_db()
 
     def test_video_image_deletion_single(self):
         """

edxval/urls.py

@@ -2,14 +2,12 @@
 Url file for django app edxval.
 """
 
-from django.conf.urls import patterns, url
+from django.conf.urls import url
 
 from edxval import views
 
-urlpatterns = patterns(
-    '',
-    url(
-        r'^videos/$',
+urlpatterns = [
+    url(r'^videos/$',
         views.VideoList.as_view(),
         name="video-list"
     ),
@@ -33,4 +31,4 @@ urlpatterns = patterns(
         views.VideoImagesView.as_view(),
         name='update-video-images'
     ),
-)
+]

requirements.txt

-django>=1.8,<1.9
-git+https://github.com/edx/django-rest-framework.git@1ceda7c086fddffd1c440cc86856441bbf0bd9cb#egg=djangorestframework==3.6.3
-enum34==1.0.4
-lxml==3.3.6
 -e git+https://github.com/edx/django-oauth2-provider.git@0.2.7-fork-edx-6a#egg=django-oauth2-provider==0.2.7-fork-edx-6
 -e git+https://github.com/edx/django-rest-framework-oauth.git@f0b503fda8c254a38f97fef802ded4f5fe367f7a#egg=djangorestframework-oauth
-django-storages==1.5.2
+pillow==4.2.1
 boto==2.46.1
 django-model-utils==2.3.1
-Pillow==3.4
+django-storages==1.5.2
+enum34==1.0.4
+git+https://github.com/edx/django-rest-framework.git@1ceda7c086fddffd1c440cc86856441bbf0bd9cb#egg=djangorestframework==3.6.3
+lxml==3.3.6

setup.py

@@ -39,7 +39,7 @@ def load_requirements(*requirements_paths):
 
 setup(
     name='edxval',
-    version='0.0.19',
+    version='0.1.0',
     author='edX',
     url='http://github.com/edx/edx-val',
     description='edx-val',
@@ -52,6 +52,6 @@ setup(
         'Programming Language :: Python',
     ],
     packages=PACKAGES,
-    install_requires=load_requirements('requirements.txt'),
+    install_requires=load_requirements('requirements.txt', 'django-requirements.txt'),
     tests_require=load_requirements('test-requirements.txt'),
 )

test-requirements.txt

-django-nose==1.4.1
 coverage==3.7.1
+ddt==0.8.0
+django-nose==1.4.4
 mock==1.0.1
 pylint==1.3.0
-ddt==0.8.0
+tox==2.7.0

tox.ini

+[tox]
+envlist = django{18,110,111}
+
+[testenv]
+deps =
+    -r{toxinidir}/requirements.txt
+    -r{toxinidir}/test-requirements.txt
+    django18: Django>=1.8,<1.9
+    django110: Django>=1.10,<1.11
+    django111: Django>=1.11,<2
+commands =
+    python manage.py test {posargs}

urls.py

 from django.conf import settings
-from django.conf.urls import patterns, include, url
+from django.conf.urls import include, url
 from django.contrib import admin
 
 
-urlpatterns = patterns(
-    '',
+urlpatterns = [
     # Django Admin
     url(r'^admin/', include(admin.site.urls)),
 
@@ -13,4 +12,4 @@ urlpatterns = patterns(
 
     # edx-val
     url(r'^edxval/', include('edxval.urls'))
-)
+]