update serializer

edxval/api.py

@@ -187,7 +187,7 @@ def update_video_image(edx_video_id, course_id, image_data, file_name):
         error_message = u"CourseVideo not found for edx_video_id: {0}".format(edx_video_id)
         raise ValVideoNotFoundError(error_message)
 
-    video_image, _ = VideoImage.create_or_update(course_video, image_data, file_name)
+    video_image, _ = VideoImage.create_or_update(course_video, file_name, image_data)
     return get_course_video_image_url(video_image=video_image)
 
 
@@ -323,7 +323,7 @@ def get_url_for_profile(edx_video_id, profile):
     return get_urls_for_profiles(edx_video_id, [profile])[profile]
 
 
-def _get_videos_for_filter(video_filter, sort_field=None, sort_dir=SortDirection.asc, context=None):
+def _get_videos_for_filter(video_filter, sort_field=None, sort_dir=SortDirection.asc):
     """
     Returns a generator expression that contains the videos found, sorted by
     the given field and direction, with ties broken by edx_video_id to ensure a
@@ -335,7 +335,7 @@ def _get_videos_for_filter(video_filter, sort_field=None, sort_dir=SortDirection
         videos = videos.order_by(sort_field.value, "edx_video_id")
         if sort_dir == SortDirection.desc:
             videos = videos.reverse()
-    return (VideoSerializer(video, context=context).data for video in videos)
+    return (VideoSerializer(video).data for video in videos)
 
 
 def get_videos_for_course(course_id, sort_field=None, sort_dir=SortDirection.asc):
@@ -356,7 +356,6 @@ def get_videos_for_course(course_id, sort_field=None, sort_dir=SortDirection.asc
         {'courses__course_id': unicode(course_id), 'courses__is_hidden': False},
         sort_field,
         sort_dir,
-        context={'course_id': course_id}
     )
 
 

edxval/models.py

@@ -14,6 +14,7 @@ invalid profile_name will be returned.
 from contextlib import closing
 import logging
 import os
+from uuid import uuid4
 
 from django.db import models
 from django.dispatch import receiver
@@ -41,6 +42,7 @@ class ModelFactoryWithValidation(object):
         ret_val = cls(*args, **kwargs)
         ret_val.full_clean()
         ret_val.save()
+        return ret_val
 
     @classmethod
     def get_or_create_with_validation(cls, *args, **kwargs):
@@ -198,27 +200,38 @@ class VideoImage(TimeStampedModel):
     image = CustomizableImageField()
 
     @classmethod
-    def create_or_update(cls, course_video, image_data, file_name):
+    def create_or_update(cls, course_video, file_name, image_data=None):
         """
         Create a VideoImage object for a CourseVideo.
 
         Arguments:
             course_video (CourseVideo): CourseVideo instance
+            file_name (str): File name of the image
             image_data (InMemoryUploadedFile): Image data to be saved.
-            file_name (str): File name.
 
         Returns:
             Returns a tuple of (video_image, created).
         """
         video_image, created = cls.objects.get_or_create(course_video=course_video)
-        with closing(image_data) as image_file:
-            __, file_extension = os.path.splitext(file_name)
-            file_name = '{timestamp}{ext}'.format(timestamp=video_image.modified.strftime("%s"), ext=file_extension)
-            video_image.image.save(file_name, image_file)
-            video_image.save()
+        if image_data:
+            with closing(image_data) as image_file:
+                file_name = '{uuid}{ext}'.format(uuid=uuid4().hex, ext=os.path.splitext(file_name)[1])
+                video_image.image.save(file_name, image_file)
+        else:
+            video_image.image.name = file_name
 
+        video_image.save()
         return video_image, created
 
+    @classmethod
+    def image_url(cls, course_video):
+        """
+        Return image url for a course video image.
+        """
+        storage = get_video_image_storage()
+        if hasattr(course_video, 'video_image'):
+            return storage.url(course_video.video_image.image.name)
+
 
 SUBTITLE_FORMATS = (
     ('srt', 'SubRip'),

edxval/serializers.py

@@ -7,8 +7,7 @@ EncodedVideoSerializer which uses the profile_name as it's profile field.
 from rest_framework import serializers
 from rest_framework.fields import IntegerField, DateTimeField
 
-from edxval.models import Profile, Video, EncodedVideo, Subtitle, CourseVideo
-from edxval.exceptions import ValVideoImageNotFoundError
+from edxval.models import Profile, Video, EncodedVideo, Subtitle, CourseVideo, VideoImage
 
 
 class EncodedVideoSerializer(serializers.ModelSerializer):
@@ -88,14 +87,27 @@ class CourseSerializer(serializers.RelatedField):
     """
     Field for CourseVideo
     """
-    def to_representation(self, value):
-        return value.course_id
+    def to_representation(self, course_video):
+        """
+        Returns a serializable representation of a CourseVideo instance.
+        """
+        return {
+            course_video.course_id: VideoImage.image_url(course_video)
+        }
 
     def to_internal_value(self, data):
-        if data:
-            course_video = CourseVideo(course_id=data)
-            course_video.full_clean(exclude=["video"])
-            return course_video
+        """
+        Convert data into CourseVideo instance and image filename tuple.
+        """
+        if isinstance(data, basestring):
+            course_id, image = data, None
+        elif  isinstance(data, dict):
+            (course_id, image), = data.items()
+
+        course_video = CourseVideo(course_id=course_id)
+        course_video.full_clean(exclude=["video"])
+
+        return course_video, image
 
 
 class VideoSerializer(serializers.ModelSerializer):
@@ -112,7 +124,6 @@ class VideoSerializer(serializers.ModelSerializer):
         required=False,
         queryset=CourseVideo.objects.all()
     )
-    course_video_image_url = serializers.SerializerMethodField()
     url = serializers.SerializerMethodField()
 
     # Django Rest Framework v3 converts datetimes to unicode by default.
@@ -124,21 +135,6 @@ class VideoSerializer(serializers.ModelSerializer):
         lookup_field = "edx_video_id"
         exclude = ('id',)
 
-    def get_course_video_image_url(self, video):
-        """
-        Return image associated with a course video or None if course_id is missing or if there is any error.
-        """
-        # Imported here to avoid circular dependency
-        from edxval.api import get_course_video_image_url
-
-        if self.context is None or 'course_id' not in self.context:
-            return None
-
-        try:
-            return get_course_video_image_url(self.context['course_id'], video.edx_video_id)
-        except ValVideoImageNotFoundError:
-            return None
-
     def get_url(self, obj):
         """
         Return relative url for the object
@@ -185,9 +181,12 @@ class VideoSerializer(serializers.ModelSerializer):
 
         # The CourseSerializer will already have converted the course data
         # to CourseVideo models, so we can just set the video and save.
-        for course_video in courses:
+        # Also create VideoImage objects if an image filename is present
+        for course_video, image in courses:
             course_video.video = video
             course_video.save()
+            if image:
+                VideoImage.create_or_update(course_video, image)
 
         return video
 
@@ -217,8 +216,11 @@ class VideoSerializer(serializers.ModelSerializer):
         # Set courses
         # NOTE: for backwards compatibility with the DRF v2 behavior,
         # we do NOT delete existing course videos during the update.
-        for course_video in validated_data.get("courses", []):
+        # Also update VideoImage objects if an image filename is present
+        for course_video, image in validated_data.get("courses", []):
             course_video.video = instance
             course_video.save()
+            if image:
+                VideoImage.create_or_update(course_video, image)
 
         return instance

edxval/tests/test_api.py

@@ -1241,12 +1241,12 @@ class CourseVideoImageTest(TestCase):
         """
         video_data_generator = api.get_videos_for_course(self.course_id)
         video_data = list(video_data_generator)[0]
-        self.assertEqual(video_data['course_video_image_url'], self.image_url)
+        self.assertEqual(video_data['courses'][0]['test-course'], self.image_url)
 
     def test_get_videos_for_ids(self):
         """
-        Verify that `get_videos_for_ids` api function returns reponse with course_video_image_url set to None.
+        Verify that `get_videos_for_ids` api function returns response with course_video_image_url set to None.
         """
         video_data_generator = api.get_videos_for_ids([self.edx_video_id])
         video_data = list(video_data_generator)[0]
-        self.assertEqual(video_data['course_video_image_url'], None)
+        self.assertEqual(video_data['courses'][0]['test-course'], self.image_url)

edxval/tests/test_views.py

@@ -580,7 +580,7 @@ class VideoListTest(APIAuthTestCase):
         self.assertEqual(response.status_code, status.HTTP_201_CREATED)
         videos = self.client.get("/edxval/videos/").data
         self.assertEqual(len(videos), 1)
-        self.assertEqual(videos[0]['courses'], [course1, course2])
+        self.assertEqual(videos[0]['courses'], [{course1: None}, {course2: None}])
 
         url = reverse('video-list') + '?course=%s' % course1
         videos = self.client.get(url).data