Merge pull request #78 from edx/mushtaq/fix-coursevideo-bug

Filters out invalid data from course_videos list

edxval/serializers.py

@@ -99,13 +99,14 @@ class CourseSerializer(serializers.RelatedField):
         """
         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_id = data
+        course_video = image = None
+        if data:
+            if isinstance(data, dict):
+                (course_id, image), = data.items()
 
-        course_video = CourseVideo(course_id=course_id)
-        course_video.full_clean(exclude=["video"])
+            course_video = CourseVideo(course_id=course_id)
+            course_video.full_clean(exclude=['video'])
 
         return course_video, image
 
@@ -157,6 +158,10 @@ class VideoSerializer(serializers.ModelSerializer):
         except TypeError:
             raise serializers.ValidationError("profile field needs to be a profile_name (str)")
 
+        # Clean course_video list from any invalid data.
+        course_videos = [(course_video, image) for course_video, image in data.get('courses', []) if course_video]
+        data['courses'] = course_videos
+
         return data
 
     def create(self, validated_data):

edxval/tests/constants.py

@@ -310,6 +310,14 @@ COMPLETE_SET_WITH_COURSE_KEY = dict(
     subtitles=[SUBTITLE_DICT_SRT],
     **VIDEO_DICT_STAR
 )
+COMPLETE_SET_WITH_SOME_INVALID_COURSE_KEY = dict(
+    courses=[None, False, '', 'edX/DemoX/Astonomy'],
+    encoded_videos=[
+        ENCODED_VIDEO_DICT_STAR
+    ],
+    subtitles=[SUBTITLE_DICT_SRT],
+    **VIDEO_DICT_STAR
+)
 COMPLETE_SET_WITH_OTHER_COURSE_KEYS = dict(
     courses=['edX/DemoX/Astonomy', 'edX/DemoX/Zoology'],
     encoded_videos=[

edxval/tests/test_views.py

@@ -2,6 +2,7 @@
 """
 Tests for Video Abstraction Layer views
 """
+import json
 from ddt import ddt, data, unpack
 
 from django.core.urlresolvers import reverse
@@ -457,6 +458,19 @@ class VideoListTest(APIAuthTestCase):
         self.assertEqual(len(video), 1)
         self.assertEqual(len(video[0].get("encoded_videos")), 0)
 
+    def test_post_video_invalid_course_key(self):
+        """
+        Tests POSTing a new Video with course video list containing some invalid course keys.
+        """
+        url = reverse('video-list')
+        response = self.client.post(
+            url, constants.COMPLETE_SET_WITH_SOME_INVALID_COURSE_KEY, format='json'
+        )
+        self.assertEqual(response.status_code, status.HTTP_201_CREATED)
+        response = json.loads(response.content)
+        # Check that invalid course keys have been filtered out.
+        self.assertEqual(response['courses'], [{u'edX/DemoX/Astonomy': None}])
+
     def test_post_non_latin_client_video_id(self):
         """
         Tests POSTing non-latin client_video_id

setup.py

@@ -39,7 +39,7 @@ def load_requirements(*requirements_paths):
 
 setup(
     name='edxval',
-    version='0.0.16',
+    version='0.0.17',
     author='edX',
     url='http://github.com/edx/edx-val',
     description='edx-val',