lookup subtitles by language rather than id

edxval/models.py

@@ -129,13 +129,13 @@ class Subtitle(models.Model):
     video = models.ForeignKey(Video, related_name="subtitles")
     fmt = models.CharField(max_length=20, db_index=True, choices=SUBTITLE_FORMATS)
     language = models.CharField(max_length=8, db_index=True)
-    content = models.TextField()
+    content = models.TextField(default='')
 
     def __str__(self):
         return '%s Subtitle for %s' % (self.language, self.video)
 
     def get_absolute_url(self):
-        return reverse('subtitle-content', args=[str(self.id)])
+        return reverse('subtitle-content', args=[self.video.edx_video_id, self.language])
 
     @property
     def content_type(self):

edxval/serializers.py

@@ -51,7 +51,7 @@ class EncodedVideoSerializer(serializers.ModelSerializer):
         return data.get('profile', None)
 
 
-class SubtitleSerializer(serializers.HyperlinkedModelSerializer):
+class SubtitleSerializer(serializers.ModelSerializer):
     """
     Serializer for Subtitle objects
     """
@@ -79,10 +79,8 @@ class SubtitleSerializer(serializers.HyperlinkedModelSerializer):
         fields = (
             "fmt",
             "language",
-            "id",
             "content_url",
-            "url",
-            "content"
+            "content",
         )
 
 
@@ -93,7 +91,7 @@ class VideoSerializer(serializers.HyperlinkedModelSerializer):
     encoded_videos takes a list of dicts EncodedVideo data.
     """
     encoded_videos = EncodedVideoSerializer(many=True, allow_add_remove=True)
-    subtitles = SubtitleSerializer(many=True, allow_add_remove=True)
+    subtitles = SubtitleSerializer(many=True, allow_add_remove=True, required=False)
 
     class Meta:  # pylint: disable=C0111
         model = Video

edxval/tests/constants.py

@@ -200,7 +200,7 @@ COMPLETE_SET_DIFFERENT_ID_UPDATE_FISH = dict(
         ENCODED_VIDEO_DICT_UPDATE_FISH_MOBILE,
         ENCODED_VIDEO_DICT_UPDATE_FISH_DESKTOP
     ],
-    subtitles=[SUBTITLE_DICT_SRT],
+    subtitles=[SUBTITLE_DICT_SRT, SUBTITLE_DICT_SJSON],
     **VIDEO_DICT_DIFFERENT_ID_FISH
 )
 COMPLETE_SET_FIRST_HALF_UPDATE_FISH = dict(
@@ -208,7 +208,7 @@ COMPLETE_SET_FIRST_HALF_UPDATE_FISH = dict(
         ENCODED_VIDEO_DICT_UPDATE_FISH_MOBILE,
         ENCODED_VIDEO_DICT_FISH_DESKTOP
     ],
-    subtitles=[SUBTITLE_DICT_SRT],
+    subtitles=[SUBTITLE_DICT_SRT, SUBTITLE_DICT_SJSON],
     **VIDEO_DICT_FISH
 )
 COMPLETE_SET_UPDATE_ONLY_DESKTOP_FISH = dict(

edxval/tests/test_api.py

@@ -225,7 +225,7 @@ class GetVideoInfoTestWithHttpCalls(APITestCase):
         """
         Tests number of queries for a Video/EncodedVideo(1) pair
         """
-        with self.assertNumQueries(5):
+        with self.assertNumQueries(7):
             api.get_video_info(constants.COMPLETE_SET_FISH.get("edx_video_id"))
 
     def test_get_info_queries_for_one_encoded_video(self):
@@ -237,7 +237,7 @@ class GetVideoInfoTestWithHttpCalls(APITestCase):
             url, constants.COMPLETE_SET_STAR, format='json'
         )
         self.assertEqual(response.status_code, status.HTTP_201_CREATED)
-        with self.assertNumQueries(4):
+        with self.assertNumQueries(5):
             api.get_video_info(constants.COMPLETE_SET_STAR.get("edx_video_id"))
 
     def test_get_info_queries_for_only_video(self):

edxval/tests/test_views.py

@@ -140,6 +140,7 @@ class VideoDetail(APITestCase):
         Tests PUTting one of two EncodedVideo(s) and then a single EncodedVideo PUT back.
         """
         url = reverse('video-list')
+
         response = self.client.post(url, constants.COMPLETE_SET_FISH, format='json')
         self.assertEqual(response.status_code, status.HTTP_201_CREATED)
         url = reverse(
@@ -442,7 +443,7 @@ class VideoListTest(APITestCase):
         Tests number of queries for a Video/EncodedVideo(2) pair
         """
         url = reverse('video-list')
-        with self.assertNumQueries(14):
+        with self.assertNumQueries(16):
             self.client.post(url, constants.COMPLETE_SET_FISH, format='json')
 
     def test_queries_for_single_encoded_videos(self):
@@ -450,7 +451,7 @@ class VideoListTest(APITestCase):
         Tests number of queries for a Video/EncodedVideo(1) pair
                 """
         url = reverse('video-list')
-        with self.assertNumQueries(9):
+        with self.assertNumQueries(10):
             self.client.post(url, constants.COMPLETE_SET_STAR, format='json')
 
 
@@ -490,11 +491,11 @@ class VideoDetailTest(APITestCase):
             self.client.get("/edxval/video/").data
         response = self.client.post(url, constants.COMPLETE_SET_FISH, format='json')
         self.assertEqual(response.status_code, status.HTTP_201_CREATED)
-        with self.assertNumQueries(7):
+        with self.assertNumQueries(9):
             self.client.get("/edxval/video/").data
         response = self.client.post(url, constants.COMPLETE_SET_STAR, format='json')
         self.assertEqual(response.status_code, status.HTTP_201_CREATED)
-        with self.assertNumQueries(9):
+        with self.assertNumQueries(12):
             self.client.get("/edxval/video/").data
 
 
@@ -540,7 +541,7 @@ class SubtitleDetailTest(APITestCase):
         self.assertEqual(response.status_code, status.HTTP_201_CREATED)
         video = response.data
         st = video['subtitles'][0]
-        url = reverse('subtitle-detail', kwargs={'id': st['id']})
+        url = reverse('subtitle-detail', kwargs={'video__edx_video_id': video['edx_video_id'], 'language': st['language']})
 
         st['content'] = 'testing 123'
         response = self.client.put(
@@ -561,7 +562,7 @@ class SubtitleDetailTest(APITestCase):
         self.assertEqual(response.status_code, status.HTTP_201_CREATED)
         video = response.data
         st = video['subtitles'][1]
-        url = reverse('subtitle-detail', kwargs={'id': st['id']})
+        url = reverse('subtitle-detail', kwargs={'video__edx_video_id': video['edx_video_id'], 'language': st['language']})
 
         st['content'] = 'testing 123'
         response = self.client.put(

edxval/urls.py

@@ -14,16 +14,17 @@ urlpatterns = patterns(
         name="video-list"
     ),
     url(
-        r'^edxval/video/(?P<edx_video_id>[-\w]+)',
+        r'^edxval/video/(?P<edx_video_id>[-\w]+)$',
         views.VideoDetail.as_view(),
         name="video-detail"
     ),
     url(
-    	r'^edxval/subtitle/(?P<id>[\d]+)$',
+    	r'^edxval/video/(?P<video__edx_video_id>[-\w]+)/(?P<language>[-_\w]+)$',
         views.SubtitleDetail.as_view(),
         name="subtitle-detail"
     ),
-    url(r'^edxval/subtitle/(?P<subtitle_id>[\d]+)/content$',
+    url(
+    	r'^edxval/video/(?P<edx_video_id>[-\w]+)/(?P<language>[-_\w]+)/subtitle$',
         views.get_subtitle,
         name="subtitle-content"
     ),

edxval/views.py

@@ -4,6 +4,7 @@ Views file for django app edxval.
 
 from rest_framework import generics
 from django.http import HttpResponse
+from django.shortcuts import get_object_or_404
 from django.views.decorators.http import last_modified
 
 from edxval.models import Video, Profile, Subtitle
@@ -13,6 +14,19 @@ from edxval.serializers import (
     SubtitleSerializer
 )
 
+class MultipleFieldLookupMixin(object):
+    """
+    Apply this mixin to any view or viewset to get multiple field filtering
+    based on a `lookup_fields` attribute, instead of the default single field filtering.
+    """
+    def get_object(self):
+        queryset = self.get_queryset()             # Get the base queryset
+        queryset = self.filter_queryset(queryset)  # Apply any filter backends
+        filter = {}
+        for field in self.lookup_fields:
+            filter[field] = self.kwargs[field]
+        return get_object_or_404(queryset, **filter)  # Lookup the object
+
 
 class VideoList(generics.ListCreateAPIView):
     """
@@ -41,20 +55,23 @@ class VideoDetail(generics.RetrieveUpdateDestroyAPIView):
     serializer_class = VideoSerializer
 
 
-class SubtitleDetail(generics.RetrieveUpdateDestroyAPIView):
+class SubtitleDetail(MultipleFieldLookupMixin, generics.RetrieveUpdateDestroyAPIView):
     """
     Gets a subtitle instance given its id
     """
-    lookup_field = "id"
+    lookup_fields = ("video__edx_video_id", "language")
     queryset = Subtitle.objects.all()
     serializer_class = SubtitleSerializer
 
 
-@last_modified(last_modified_func=lambda request, subtitle_id: Subtitle.objects.get(pk=subtitle_id).modified)
-def get_subtitle(request, subtitle_id):
+def _last_modified_subtitle(request, edx_video_id, language):
+    return Subtitle.objects.get(video__edx_video_id=edx_video_id, language=language).modified
+
+@last_modified(last_modified_func=_last_modified_subtitle)
+def get_subtitle(request, edx_video_id, language):
     """
     Return content of subtitle by id
     """
-    sub = Subtitle.objects.get(pk=subtitle_id)
+    sub = Subtitle.objects.get(video__edx_video_id=edx_video_id, language=language)
     response = HttpResponse(sub.content, content_type=sub.content_type)
     return response