Merge pull request #117 from edx/mrehan/transcript-foreign-key

Make Video an explicit foreign key in VideoTranscript model

edxval/admin.py

@@ -69,7 +69,7 @@ class CourseVideoAdmin(admin.ModelAdmin):
 
 
 class VideoTranscriptAdmin(admin.ModelAdmin):
-    list_display = ('video_id', 'language_code', 'provider', 'file_format')
+    list_display = ('video', 'language_code', 'provider', 'file_format')
 
     model = VideoTranscript
 

edxval/api.py

@@ -192,7 +192,7 @@ def is_transcript_available(video_id, language_code=None):
         video_id: it can be an edx_video_id or an external_id extracted from external sources in a video component.
         language_code: it will the language code of the requested transcript.
     """
-    filter_attrs = {'video_id': video_id}
+    filter_attrs = {'video__edx_video_id': video_id}
     if language_code:
         filter_attrs['language_code'] = language_code
 
@@ -200,22 +200,6 @@ def is_transcript_available(video_id, language_code=None):
     return transcript_set.exists()
 
 
-def get_video_transcripts(video_id):
-    """
-    Get a video's transcripts
-
-    Arguments:
-        video_id: it can be an edx_video_id or an external_id extracted from external sources in a video component.
-    """
-    transcripts_set = VideoTranscript.objects.filter(video_id=video_id)
-
-    transcripts = []
-    if transcripts_set.exists():
-        transcripts = TranscriptSerializer(transcripts_set, many=True).data
-
-    return transcripts
-
-
 def get_video_transcript(video_id, language_code):
     """
     Get video transcript info
@@ -245,7 +229,7 @@ def get_video_transcript_data(video_ids, language_code):
     transcript_data = None
     for video_id in video_ids:
         try:
-            video_transcript = VideoTranscript.objects.get(video_id=video_id, language_code=language_code)
+            video_transcript = VideoTranscript.objects.get(video__edx_video_id=video_id, language_code=language_code)
             transcript_data = dict(
                 file_name=video_transcript.filename,
                 content=video_transcript.transcript.file.read()
@@ -276,7 +260,7 @@ def get_available_transcript_languages(video_ids):
         A list containing unique transcript language codes for the video ids.
     """
     available_languages = VideoTranscript.objects.filter(
-        video_id__in=video_ids
+        video__edx_video_id__in=video_ids
     ).values_list(
         'language_code', flat=True
     )
@@ -324,7 +308,12 @@ def create_or_update_video_transcript(video_id, language_code, metadata, file_da
     if provider and provider not in dict(TranscriptProviderType.CHOICES).keys():
         raise InvalidTranscriptProvider('{} transcript provider is not supported'.format(provider))
 
-    video_transcript, __ = VideoTranscript.create_or_update(video_id, language_code, metadata, file_data)
+    try:
+        # Video should be present in edxval in order to attach transcripts to it.
+        video = Video.objects.get(edx_video_id=video_id)
+        video_transcript, __ = VideoTranscript.create_or_update(video, language_code, metadata, file_data)
+    except Video.DoesNotExist:
+        return None
 
     return video_transcript.url()
 
@@ -338,7 +327,7 @@ def delete_video_transcript(video_id, language_code):
         language_code: language code of a video transcript
     """
     try:
-        video_transcript = VideoTranscript.objects.get(video_id=video_id, language_code=language_code)
+        video_transcript = VideoTranscript.objects.get(video__edx_video_id=video_id, language_code=language_code)
         # delete the actual transcript file from storage
         video_transcript.transcript.delete()
         # delete the record from db
@@ -775,10 +764,9 @@ def export_to_xml(video_ids, course_id=None, external=False):
     Raises:
         ValVideoNotFoundError: if the video does not exist
     """
-    # val does not store external videos, so construct transcripts information only.
+    # TODO: This will be removed as a part of EDUCATOR-1789
     if external:
-        video_el = Element('video_asset')
-        return create_transcripts_xml(video_ids, video_el)
+        return Element('video_asset')
 
     # for an internal video, first video id must be edx_video_id
     video_id = video_ids[0]
@@ -824,7 +812,7 @@ def create_transcripts_xml(video_ids, video_el):
     Returns:
         lxml Element object with transcripts information
     """
-    video_transcripts = VideoTranscript.objects.filter(video_id__in=video_ids)
+    video_transcripts = VideoTranscript.objects.filter(video__edx_video_id__in=video_ids).order_by('language_code')
     # create transcripts node only when we have transcripts for a video
     if video_transcripts.exists():
         transcripts_el = SubElement(video_el, 'transcripts')
@@ -836,7 +824,7 @@ def create_transcripts_xml(video_ids, video_el):
                 transcripts_el,
                 'transcript',
                 {
-                    'video_id': video_transcript.video_id,
+                    'video_id': video_transcript.video.edx_video_id,
                     'file_name': video_transcript.transcript.name,
                     'language_code': video_transcript.language_code,
                     'file_format': video_transcript.file_format,
@@ -866,9 +854,9 @@ def import_from_xml(xml, edx_video_id, course_id=None):
     if xml.tag != 'video_asset':
         raise ValCannotCreateError('Invalid XML')
 
-    # if edx_video_id does not exist then create video transcripts only
+    # TODO this will be moved as a part of EDUCATOR-2173
     if not edx_video_id:
-        return create_transcript_objects(xml)
+        return
 
     # If video with edx_video_id already exists, associate it with the given course_id.
     try:
@@ -885,9 +873,6 @@ def import_from_xml(xml, edx_video_id, course_id=None):
             if image_file_name:
                 VideoImage.create_or_update(course_video, image_file_name)
 
-        # import transcripts
-        create_transcript_objects(xml)
-
         return
     except ValidationError as err:
         logger.exception(err.message)
@@ -934,7 +919,7 @@ def create_transcript_objects(xml):
     """
     for transcript in xml.findall('.//transcripts/transcript'):
         try:
-            VideoTranscript.create_or_update(
+            create_or_update_video_transcript(
                 transcript.attrib['video_id'],
                 transcript.attrib['language_code'],
                 metadata=dict(

edxval/migrations/0010_add_video_as_foreign_key.py

+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('edxval', '0009_auto_20171127_0406'),
+    ]
+
+    operations = [
+        migrations.AlterUniqueTogether(
+            name='videotranscript',
+            unique_together=set([('language_code',)]),
+        ),
+        migrations.RemoveField(
+            model_name='videotranscript',
+            name='video_id',
+        ),
+        migrations.AddField(
+            model_name='videotranscript',
+            name='video',
+            field=models.ForeignKey(related_name='video_transcripts', to='edxval.Video', null=True),
+        ),
+        migrations.AlterUniqueTogether(
+            name='videotranscript',
+            unique_together=set([('video', 'language_code')]),
+        ),
+    ]

edxval/models.py

@@ -404,7 +404,7 @@ class VideoTranscript(TimeStampedModel):
     """
     Transcript for a video
     """
-    video_id = models.CharField(max_length=255, help_text='It can be an edx_video_id or an external video id')
+    video = models.ForeignKey(Video, related_name='video_transcripts', null=True)
     transcript = CustomizableFileField()
     language_code = models.CharField(max_length=50, db_index=True)
     provider = models.CharField(
@@ -415,27 +415,19 @@ class VideoTranscript(TimeStampedModel):
     file_format = models.CharField(max_length=20, db_index=True, choices=TranscriptFormat.CHOICES)
 
     class Meta:
-        unique_together = ('video_id', 'language_code')
+        unique_together = ('video', 'language_code')
 
     @property
     def filename(self):
         """
         Returns readable filename for a transcript
         """
-        try:
-            video = Video.objects.get(edx_video_id=self.video_id)
-            client_id, __ = os.path.splitext(video.client_video_id)
-            file_name = u'{name}-{language}.{format}'.format(
-                name=client_id,
-                language=self.language_code,
-                format=self.file_format
-            )
-        except Video.DoesNotExist:
-            file_name = u'{name}-{language}.{format}'.format(
-                name=self.video_id,
-                language=self.language_code,
-                format=self.file_format
-            )
+        client_id, __ = os.path.splitext(self.video.client_video_id)
+        file_name = u'{name}-{language}.{format}'.format(
+            name=client_id,
+            language=self.language_code,
+            format=self.file_format
+        )
 
         return file_name
 
@@ -449,19 +441,19 @@ class VideoTranscript(TimeStampedModel):
             language_code(unicode): language of the requested transcript
         """
         try:
-            transcript = cls.objects.get(video_id=video_id, language_code=language_code)
+            transcript = cls.objects.get(video__edx_video_id=video_id, language_code=language_code)
         except cls.DoesNotExist:
             transcript = None
 
         return transcript
 
     @classmethod
-    def create_or_update(cls, video_id, language_code, metadata, file_data=None):
+    def create_or_update(cls, video, language_code, metadata, file_data=None):
         """
         Create or update Transcript object.
 
         Arguments:
-            video_id (str): unique id for a video
+            video (Video): Video for which transcript is going to be saved.
             language_code (str): language code for (to be created/updated) transcript
             metadata (dict): A dict containing (to be overwritten) properties
             file_data (InMemoryUploadedFile): File data to be saved
@@ -469,7 +461,7 @@ class VideoTranscript(TimeStampedModel):
         Returns:
             Returns a tuple of (video_transcript, created).
         """
-        video_transcript, created = cls.objects.get_or_create(video_id=video_id, language_code=language_code)
+        video_transcript, created = cls.objects.get_or_create(video=video, language_code=language_code)
 
         for prop, value in metadata.iteritems():
             if prop in ['language_code', 'file_format', 'provider']:
@@ -489,7 +481,7 @@ class VideoTranscript(TimeStampedModel):
                 try:
                     video_transcript.transcript.save(file_name, transcript_file_data)
                 except Exception:
-                    logger.exception('VAL: Transcript save failed to storage for video_id [%s]', video_id)
+                    logger.exception('VAL: Transcript save failed to storage for video_id [%s]', video.edx_video_id)
                     raise
 
         video_transcript.save()
@@ -503,7 +495,7 @@ class VideoTranscript(TimeStampedModel):
         return storage.url(self.transcript.name)
 
     def __unicode__(self):
-        return u'{lang} Transcript for {video}'.format(lang=self.language_code, video=self.video_id)
+        return u'{lang} Transcript for {video}'.format(lang=self.language_code, video=self.video.edx_video_id)
 
 
 class Cielo24Turnaround(object):

edxval/serializers.py

@@ -57,11 +57,17 @@ class TranscriptSerializer(serializers.ModelSerializer):
     """
     class Meta:  # pylint: disable=C1001, C0111
         model = VideoTranscript
-        lookup_field = 'video_id'
         fields = ('video_id', 'url', 'language_code', 'provider', 'file_format')
 
+    video_id = serializers.SerializerMethodField()
     url = serializers.SerializerMethodField()
 
+    def get_video_id(self, transcript):
+        """
+        Returns an edx video ID for the related video.
+        """
+        return transcript.video.edx_video_id
+
     def get_url(self, transcript):
         """
         Retrieves the transcript url.

edxval/tests/constants.py

@@ -378,14 +378,6 @@ VIDEO_TRANSCRIPT_3PLAY = dict(
     file_format=TranscriptFormat.SJSON,
 )
 
-VIDEO_TRANSCRIPT_CUSTOM = dict(
-    video_id='external_video_id',
-    language_code='de',
-    transcript='wow.srt',
-    provider=TranscriptProviderType.CUSTOM,
-    file_format=TranscriptFormat.SRT,
-)
-
 TRANSCRIPT_PREFERENCES_CIELO24 = dict(
     course_id='edX/DemoX/Demo_Course',
     provider=TranscriptProviderType.CIELO24,

edxval/tests/test_views.py

@@ -3,7 +3,6 @@
 Tests for Video Abstraction Layer views
 """
 import json
-import unittest
 
 from ddt import data, ddt, unpack
 from django.core.urlresolvers import reverse
@@ -821,13 +820,18 @@ class VideoTranscriptViewTest(APIAuthTestCase):
 
         serialized_data = TranscriptSerializer(VideoTranscript.objects.first()).data
         post_transcript_data['url'] = post_transcript_data.pop('name')
-        self.assertEqual(serialized_data, post_transcript_data)
+        self.assertDictEqual(serialized_data, post_transcript_data)
 
     def test_update_existing_transcript(self):
         """
         Tests updating existing transcript works as expected.
         """
-        VideoTranscript.objects.create(**self.transcript_data)
+        VideoTranscript.objects.create(
+            video=self.video,
+            language_code=self.transcript_data['language_code'],
+            file_format=self.transcript_data['file_format'],
+            provider=self.transcript_data['provider'],
+        )
 
         post_transcript_data = dict(self.transcript_data)
         post_transcript_data['name'] = post_transcript_data.pop('transcript')