address feedback

edxval/api.py

@@ -148,7 +148,9 @@ def get_course_video_image_url(course_id, edx_video_id):
     Returns course video image url or None if no image found
     """
     try:
-        video_image = CourseVideo.objects.get(course_id=course_id, video__edx_video_id=edx_video_id).video_image
+        video_image = CourseVideo.objects.select_related('video_image').get(
+            course_id=course_id, video__edx_video_id=edx_video_id
+        ).video_image
         return video_image.image_url()
     except ObjectDoesNotExist:
         return None
@@ -168,7 +170,9 @@ def update_video_image(edx_video_id, course_id, image_data, file_name):
         Raises ValVideoNotFoundError if the CourseVideo cannot be retrieved.
     """
     try:
-        course_video = CourseVideo.objects.get(course_id=course_id, video__edx_video_id=edx_video_id)
+        course_video = CourseVideo.objects.select_related('video').get(
+            course_id=course_id, video__edx_video_id=edx_video_id
+        )
     except ObjectDoesNotExist:
         error_message = u'CourseVideo not found for edx_video_id: {0}'.format(edx_video_id)
         raise ValVideoNotFoundError(error_message)
@@ -482,13 +486,13 @@ def copy_course_videos(source_course_id, destination_course_id):
     )
 
     for course_video in course_videos:
-        dest_course_video, __ = CourseVideo.objects.get_or_create(
+        destination_course_video, __ = CourseVideo.objects.get_or_create(
             video=course_video.video,
             course_id=destination_course_id
         )
         try:
             VideoImage.create_or_update(
-                course_video=dest_course_video,
+                course_video=destination_course_video,
                 file_name=course_video.video_image.image.name
             )
         except VideoImage.DoesNotExist:
@@ -509,12 +513,12 @@ def export_to_xml(edx_video_id, course_id=None):
     Raises:
         ValVideoNotFoundError: if the video does not exist
     """
-    image = ''
+    video_image_name = ''
     video = _get_video(edx_video_id)
 
     try:
         course_video = CourseVideo.objects.select_related('video_image').get(course_id=course_id, video=video)
-        image = course_video.video_image.image.name
+        video_image_name = course_video.video_image.image.name
     except ObjectDoesNotExist:
         pass
 
@@ -523,7 +527,7 @@ def export_to_xml(edx_video_id, course_id=None):
         attrib={
             'client_video_id': video.client_video_id,
             'duration': unicode(video.duration),
-            'image': image
+            'image': video_image_name
         }
     )
     for encoded_video in video.encoded_videos.all():

edxval/tests/test_api.py

@@ -808,7 +808,9 @@ class TestCopyCourse(TestCase):
         CourseVideo.objects.create(video=self.video3, course_id=self.course_id2)
 
     def test_successful_copy(self):
-        """Tests a successful copy course"""
+        """
+        Tests a successful copy course
+        """
         destination_course_id = 'course-copy1'
         api.copy_course_videos(self.course_id, destination_course_id)
         original_videos = Video.objects.filter(courses__course_id=self.course_id)
@@ -957,7 +959,7 @@ class ImportTest(TestCase):
         CourseVideo.objects.create(video=video, course_id='existing_course_id')
 
     def make_import_xml(self, video_dict, encoded_video_dicts=None, image=None):
-        ret = etree.Element(
+        import_xml = etree.Element(
             "video_asset",
             attrib={
                 key: unicode(video_dict[key])
@@ -966,18 +968,18 @@ class ImportTest(TestCase):
         )
 
         if image:
-            ret.attrib['image'] = image
+            import_xml.attrib['image'] = image
 
         for encoding_dict in (encoded_video_dicts or []):
             etree.SubElement(
-                ret,
+                import_xml,
                 "encoded_video",
                 attrib={
                     key: unicode(val)
                     for key, val in encoding_dict.items()
                 }
             )
-        return ret
+        return import_xml
 
     def assert_obj_matches_dict_for_keys(self, obj, dict_, keys):
         for key in keys:
@@ -1271,6 +1273,22 @@ class CourseVideoImageTest(TestCase):
         image_url = api.get_course_video_image_url(self.course_id, self.edx_video_id)
         self.assertIsNone(image_url)
 
+    def test_num_queries_update_video_image(self):
+        """
+        Test number of queries executed to upload a course video image.
+        """
+        with self.assertNumQueries(4):
+            api.update_video_image(
+                self.edx_video_id, self.course_id, ImageFile(open(self.image_path1)), 'image.jpg'
+            )
+
+    def test_num_queries_get_course_video_image_url(self):
+        """
+        Test number of queries executed to get a course video image url.
+        """
+        with self.assertNumQueries(1):
+            api.get_course_video_image_url(self.course_id, self.edx_video_id)
+
     def test_get_videos_for_course(self):
         """
         Verify that `get_videos_for_course` api function has correct course_video_image_url.