Improve code for video upload feature

Fix i18n for video status strings (broken in commit 4b53f4df) and remove
unnecessary complexity from a test case. This also removes the status
whitelist in the video upload configuration. Because status is included
in the CSV report, it is not necessary to filter the included videos by
status.

cms/djangoapps/contentstore/migrations/0002_auto__del_field_videouploadconfig_status_whitelist.py

+# -*- coding: utf-8 -*-
+import datetime
+from south.db import db
+from south.v2 import SchemaMigration
+from django.db import models
+
+
+class Migration(SchemaMigration):
+
+    def forwards(self, orm):
+        # Deleting field 'VideoUploadConfig.status_whitelist'
+        db.delete_column('contentstore_videouploadconfig', 'status_whitelist')
+
+
+    def backwards(self, orm):
+        # Adding field 'VideoUploadConfig.status_whitelist'
+        db.add_column('contentstore_videouploadconfig', 'status_whitelist',
+                      self.gf('django.db.models.fields.TextField')(default='', blank=True),
+                      keep_default=False)
+
+
+    models = {
+        'auth.group': {
+            'Meta': {'object_name': 'Group'},
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '80'}),
+            'permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'})
+        },
+        'auth.permission': {
+            'Meta': {'ordering': "('content_type__app_label', 'content_type__model', 'codename')", 'unique_together': "(('content_type', 'codename'),)", 'object_name': 'Permission'},
+            'codename': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
+            'content_type': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['contenttypes.ContentType']"}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '50'})
+        },
+        'auth.user': {
+            'Meta': {'object_name': 'User'},
+            'date_joined': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}),
+            'email': ('django.db.models.fields.EmailField', [], {'max_length': '75', 'blank': 'True'}),
+            'first_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}),
+            'groups': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Group']", 'symmetrical': 'False', 'blank': 'True'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'is_active': ('django.db.models.fields.BooleanField', [], {'default': 'True'}),
+            'is_staff': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'is_superuser': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'last_login': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}),
+            'last_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}),
+            'password': ('django.db.models.fields.CharField', [], {'max_length': '128'}),
+            'user_permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}),
+            'username': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '30'})
+        },
+        'contentstore.videouploadconfig': {
+            'Meta': {'object_name': 'VideoUploadConfig'},
+            'change_date': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'blank': 'True'}),
+            'changed_by': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']", 'null': 'True', 'on_delete': 'models.PROTECT'}),
+            'enabled': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'profile_whitelist': ('django.db.models.fields.TextField', [], {'blank': 'True'})
+        },
+        'contenttypes.contenttype': {
+            'Meta': {'ordering': "('name',)", 'unique_together': "(('app_label', 'model'),)", 'object_name': 'ContentType', 'db_table': "'django_content_type'"},
+            'app_label': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'model': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '100'})
+        }
+    }
+
+    complete_apps = ['contentstore']
\ No newline at end of file

cms/djangoapps/contentstore/models.py

@@ -14,23 +14,8 @@ class VideoUploadConfig(ConfigurationModel):
         blank=True,
         help_text="A comma-separated list of names of profiles to include in video encoding downloads."
     )
-    status_whitelist = TextField(
-        blank=True,
-        help_text=(
-            "A comma-separated list of Studio status values;" +
-            " only videos with these status values will be included in video encoding downloads."
-        )
-    )
 
     @classmethod
     def get_profile_whitelist(cls):
         """Get the list of profiles to include in the encoding download"""
         return [profile for profile in cls.current().profile_whitelist.split(",") if profile]
-
-    @classmethod
-    def get_status_whitelist(cls):
-        """
-        Get the list of status values to include files for in the encoding
-        download
-        """
-        return [status for status in cls.current().status_whitelist.split(",") if status]

cms/djangoapps/contentstore/views/tests/test_videos.py

@@ -16,7 +16,7 @@ from mock import Mock, patch
 from edxval.api import create_profile, create_video, get_video_info
 
 from contentstore.models import VideoUploadConfig
-from contentstore.views.videos import KEY_EXPIRATION_IN_SECONDS, VIDEO_ASSET_TYPE, status_display_string
+from contentstore.views.videos import KEY_EXPIRATION_IN_SECONDS, VIDEO_ASSET_TYPE, StatusDisplayStrings
 from contentstore.tests.utils import CourseTestCase
 from contentstore.utils import reverse_course_url
 from xmodule.assetstore import AssetMetadata
@@ -171,7 +171,10 @@ class VideosHandlerTestCase(VideoUploadTestMixin, CourseTestCase):
             dateutil.parser.parse(response_video["created"])
             for field in ["edx_video_id", "client_video_id", "duration"]:
                 self.assertEqual(response_video[field], original_video[field])
-            self.assertEqual(response_video["status"], status_display_string(original_video["status"]))
+            self.assertEqual(
+                response_video["status"],
+                StatusDisplayStrings.get(original_video["status"])
+            )
 
     def test_get_html(self):
         response = self.client.get(self.url)
@@ -313,18 +316,12 @@ class VideoUrlsCsvTestCase(VideoUploadTestMixin, CourseTestCase):
 
     def setUp(self):
         super(VideoUrlsCsvTestCase, self).setUp()
-        VideoUploadConfig(
-            profile_whitelist="profile1",
-            status_whitelist=(
-                status_display_string("file_complete") + "," +
-                status_display_string("transcode_active")
-            )
-        ).save()
+        VideoUploadConfig(profile_whitelist="profile1").save()
 
-    def _check_csv_response(self, expected_video_ids, expected_profiles):
+    def _check_csv_response(self, expected_profiles):
         """
         Check that the response is a valid CSV response containing rows
-        corresponding to expected_video_ids.
+        corresponding to previous_uploads and including the expected profiles.
         """
         response = self.client.get(self.url)
         self.assertEqual(response.status_code, 200)
@@ -346,6 +343,7 @@ class VideoUrlsCsvTestCase(VideoUploadTestMixin, CourseTestCase):
             response_video = {
                 key.decode("utf-8"): value.decode("utf-8") for key, value in row.items()
             }
+            self.assertNotIn(response_video["Video ID"], actual_video_ids)
             actual_video_ids.append(response_video["Video ID"])
             original_video = self._get_previous_upload(response_video["Video ID"])
             self.assertEqual(response_video["Name"], original_video["client_video_id"])
@@ -364,21 +362,14 @@ class VideoUrlsCsvTestCase(VideoUploadTestMixin, CourseTestCase):
                     self.assertEqual(response_profile_url, original_encoded_for_profile["url"])
                 else:
                     self.assertEqual(response_profile_url, "")
-        self.assertEqual(set(actual_video_ids), set(expected_video_ids))
+        self.assertEqual(len(actual_video_ids), len(self.previous_uploads))
 
     def test_basic(self):
-        self._check_csv_response(["test2", "non-ascii"], ["profile1"])
-
-    def test_config(self):
-        VideoUploadConfig(
-            profile_whitelist="profile1,profile2",
-            status_whitelist=(
-                status_display_string("file_complete") + "," +
-                status_display_string("transcode_active") + "," +
-                status_display_string("upload")
-            )
-        ).save()
-        self._check_csv_response(["test1", "test2", "non-ascii"], ["profile1", "profile2"])
+        self._check_csv_response(["profile1"])
+
+    def test_profile_whitelist(self):
+        VideoUploadConfig(profile_whitelist="profile1,profile2").save()
+        self._check_csv_response(["profile1", "profile2"])
 
     def test_non_ascii_course(self):
         course = CourseFactory.create(

cms/djangoapps/contentstore/views/videos.py

@@ -8,7 +8,7 @@ from uuid import uuid4
 from django.conf import settings
 from django.contrib.auth.decorators import login_required
 from django.http import HttpResponse, HttpResponseNotFound
-from django.utils.translation import ugettext as _
+from django.utils.translation import ugettext as _, ugettext_noop
 from django.views.decorators.http import require_GET, require_http_methods
 import rfc6266
 
@@ -37,39 +37,40 @@ KEY_EXPIRATION_IN_SECONDS = 86400
 
 class StatusDisplayStrings(object):
     """
-    Enum of display strings for Video Status presented in Studio (e.g., in UI and in CSV download).
+    A class to map status strings as stored in VAL to display strings for the
+    video upload page
     """
+
     # Translators: This is the status of an active video upload
-    UPLOADING = _("Uploading")
+    _UPLOADING = ugettext_noop("Uploading")
     # Translators: This is the status for a video that the servers are currently processing
-    IN_PROGRESS = _("In Progress")
+    _IN_PROGRESS = ugettext_noop("In Progress")
     # Translators: This is the status for a video that the servers have successfully processed
-    COMPLETE = _("Complete")
+    _COMPLETE = ugettext_noop("Complete")
     # Translators: This is the status for a video that the servers have failed to process
-    FAILED = _("Failed"),
+    _FAILED = ugettext_noop("Failed"),
     # Translators: This is the status for a video for which an invalid
     # processing token was provided in the course settings
-    INVALID_TOKEN = _("Invalid Token"),
+    _INVALID_TOKEN = ugettext_noop("Invalid Token"),
     # Translators: This is the status for a video that is in an unknown state
-    UNKNOWN = _("Unknown")
-
-
-def status_display_string(val_status):
-    """
-    Converts VAL status string to Studio status string.
-    """
-    status_map = {
-        "upload": StatusDisplayStrings.UPLOADING,
-        "ingest": StatusDisplayStrings.IN_PROGRESS,
-        "transcode_queue": StatusDisplayStrings.IN_PROGRESS,
-        "transcode_active": StatusDisplayStrings.IN_PROGRESS,
-        "file_delivered": StatusDisplayStrings.COMPLETE,
-        "file_complete": StatusDisplayStrings.COMPLETE,
-        "file_corrupt": StatusDisplayStrings.FAILED,
-        "pipeline_error": StatusDisplayStrings.FAILED,
-        "invalid_token": StatusDisplayStrings.INVALID_TOKEN
+    _UNKNOWN = ugettext_noop("Unknown")
+
+    _STATUS_MAP = {
+        "upload": _UPLOADING,
+        "ingest": _IN_PROGRESS,
+        "transcode_queue": _IN_PROGRESS,
+        "transcode_active": _IN_PROGRESS,
+        "file_delivered": _COMPLETE,
+        "file_complete": _COMPLETE,
+        "file_corrupt": _FAILED,
+        "pipeline_error": _FAILED,
+        "invalid_token": _INVALID_TOKEN
     }
-    return status_map.get(val_status, StatusDisplayStrings.UNKNOWN)
+
+    @staticmethod
+    def get(val_status):
+        """Map a VAL status string to a localized display string"""
+        return _(StatusDisplayStrings._STATUS_MAP.get(val_status, StatusDisplayStrings._UNKNOWN))
 
 
 @expect_json
@@ -126,7 +127,6 @@ def video_encodings_download(request, course_key_string):
         return _("{profile_name} URL").format(profile_name=profile)
 
     profile_whitelist = VideoUploadConfig.get_profile_whitelist()
-    status_whitelist = VideoUploadConfig.get_status_whitelist()
 
     videos = list(_get_videos(course))
     name_col = _("Name")
@@ -153,7 +153,7 @@ def video_encodings_download(request, course_key_string):
                 (duration_col, duration_val),
                 (added_col, video["created"].isoformat()),
                 (video_id_col, video["edx_video_id"]),
-                (status_col, video["status"]),
+                (status_col, StatusDisplayStrings.get(video["status"])),
             ] +
             [
                 (get_profile_header(encoded_video["profile"]), encoded_video["url"])
@@ -182,8 +182,7 @@ def video_encodings_download(request, course_key_string):
     )
     writer.writeheader()
     for video in videos:
-        if video["status"] in status_whitelist:
-            writer.writerow(make_csv_dict(video))
+        writer.writerow(make_csv_dict(video))
     return response
 
 
@@ -223,7 +222,7 @@ def _get_videos(course):
 
     # convert VAL's status to studio's Video Upload feature status.
     for video in videos:
-        video["status"] = status_display_string(video["status"])
+        video["status"] = StatusDisplayStrings.get(video["status"])
 
     return videos
 

cms/static/js/spec/views/previous_video_upload_spec.js

@@ -65,22 +65,11 @@ define(
                 expect($el.find(".video-id-col").text()).toEqual(testId);
             });
 
-            _.each(
-                [
-                    {status: "Uploading", expected: "Uploading"},
-                    {status: "In Progress", expected: "In Progress"},
-                    {status: "Complete", expected: "Complete"},
-                    {status: "Failed", expected: "Failed"},
-                    {status: "Invalid Token", expected: "Invalid Token"},
-                    {status: "Unknown", expected: "Unknown"}
-                ],
-                function(caseInfo) {
-                    it("should render " + caseInfo.status + " status correctly", function() {
-                        var $el = render({status: caseInfo.status});
-                        expect($el.find(".status-col").text()).toEqual(caseInfo.expected);
-                    });
-                }
-            );
+            it("should render status correctly", function() {
+                var testStatus = "Test Status";
+                var $el = render({status: testStatus});
+                expect($el.find(".status-col").text()).toEqual(testStatus);
+            });
         });
     }
 );