Merge pull request #16536 from edx/enable-videos-upload-by-default

Enable Video Uploads by default

cms/djangoapps/contentstore/views/videos.py

@@ -672,7 +672,6 @@ def videos_post(course, request):
         return JsonResponse({'error': error}, status=400)
 
     bucket = storage_service_bucket()
-    course_video_upload_token = course.video_upload_pipeline['course_video_upload_token']
     req_files = data['files']
     resp_files = []
 
@@ -689,11 +688,16 @@ def videos_post(course, request):
         key = storage_service_key(bucket, file_name=edx_video_id)
 
         metadata_list = [
-            ('course_video_upload_token', course_video_upload_token),
             ('client_video_id', file_name),
             ('course_key', unicode(course.id)),
         ]
 
+        # Only include `course_video_upload_token` if its set, as it won't be required if video uploads
+        # are enabled by default.
+        course_video_upload_token = course.video_upload_pipeline.get('course_video_upload_token')
+        if course_video_upload_token:
+            metadata_list.append(('course_video_upload_token', course_video_upload_token))
+
         is_video_transcript_enabled = VideoTranscriptEnabledFlag.feature_enabled(course.id)
         if is_video_transcript_enabled:
             transcript_preferences = get_transcript_preferences(unicode(course.id))

common/lib/xmodule/xmodule/course_module.py

@@ -12,6 +12,7 @@ from django.conf import settings
 import requests
 from lazy import lazy
 from lxml import etree
+from openedx.core.djangoapps.video_pipeline.models import VideoUploadsEnabledByDefault
 from openedx.core.lib.license import LicenseMixin
 from path import Path as path
 from pytz import utc
@@ -1337,7 +1338,7 @@ class CourseDescriptor(CourseFields, SequenceDescriptor, LicenseMixin):
         """
         Returns whether the video pipeline advanced setting is configured for this course.
         """
-        return (
+        return VideoUploadsEnabledByDefault.feature_enabled(course_id=self.id) or (
             self.video_upload_pipeline is not None and
             'course_video_upload_token' in self.video_upload_pipeline
         )

openedx/core/djangoapps/video_config/admin.py

@@ -7,10 +7,11 @@ from django.contrib import admin
 
 from openedx.core.djangoapps.video_config.forms import (
     CourseHLSPlaybackFlagAdminForm,
+    CourseVideoTranscriptFlagAdminForm,
 )
 from openedx.core.djangoapps.video_config.models import (
     CourseHLSPlaybackEnabledFlag, HLSPlaybackEnabledFlag,
-    CourseVideoTranscriptEnabledFlag, VideoTranscriptEnabledFlag
+    CourseVideoTranscriptEnabledFlag, VideoTranscriptEnabledFlag,
 )
 
 
@@ -45,9 +46,10 @@ class CourseVideoTranscriptEnabledFlagAdmin(CourseSpecificEnabledFlagBaseAdmin):
     Admin of Video Transcript feature on course-by-course basis.
     Allows searching by course id.
     """
-    form = CourseHLSPlaybackFlagAdminForm
+    form = CourseVideoTranscriptFlagAdminForm
 
 admin.site.register(HLSPlaybackEnabledFlag, ConfigurationModelAdmin)
 admin.site.register(CourseHLSPlaybackEnabledFlag, CourseHLSPlaybackEnabledFlagAdmin)
+
 admin.site.register(VideoTranscriptEnabledFlag, ConfigurationModelAdmin)
-admin.site.register(CourseVideoTranscriptEnabledFlag, CourseHLSPlaybackEnabledFlagAdmin)
+admin.site.register(CourseVideoTranscriptEnabledFlag, CourseVideoTranscriptEnabledFlagAdmin)

openedx/core/djangoapps/video_config/forms.py

@@ -7,7 +7,10 @@ from django import forms
 from opaque_keys import InvalidKeyError
 from opaque_keys.edx.locator import CourseLocator
 
-from openedx.core.djangoapps.video_config.models import CourseHLSPlaybackEnabledFlag, CourseVideoTranscriptEnabledFlag
+from openedx.core.djangoapps.video_config.models import (
+    CourseHLSPlaybackEnabledFlag,
+    CourseVideoTranscriptEnabledFlag,
+)
 from xmodule.modulestore.django import modulestore
 
 log = logging.getLogger(__name__)

openedx/core/djangoapps/video_config/tests/test_models.py

@@ -9,7 +9,10 @@ from contextlib import contextmanager
 from django.test import TestCase
 
 from opaque_keys.edx.locator import CourseLocator
-from openedx.core.djangoapps.video_config.models import CourseHLSPlaybackEnabledFlag, HLSPlaybackEnabledFlag
+from openedx.core.djangoapps.video_config.models import (
+    CourseHLSPlaybackEnabledFlag, HLSPlaybackEnabledFlag,
+    CourseVideoTranscriptEnabledFlag, VideoTranscriptEnabledFlag,
+)
 
 
 @contextmanager
@@ -184,8 +187,8 @@ class TestVideoTranscriptFlag(TestCase, FeatureFlagTestMixin):
         with course-specific flags.
         """
         self.verify_feature_flags(
-            all_courses_model_class=HLSPlaybackEnabledFlag,
+            all_courses_model_class=VideoTranscriptEnabledFlag,
-            course_specific_model_class=CourseHLSPlaybackEnabledFlag,
+            course_specific_model_class=CourseVideoTranscriptEnabledFlag,
             global_flag=global_flag,
             enabled_for_all_courses=enabled_for_all_courses,
             enabled_for_course_1=enabled_for_course_1
@@ -196,8 +199,8 @@ class TestVideoTranscriptFlag(TestCase, FeatureFlagTestMixin):
         Ensures that the Video Transcript course specific flag, once enabled for a course, can also be disabled.
         """
         self.verify_enable_disable_course_flag(
-            all_courses_model_class=HLSPlaybackEnabledFlag,
+            all_courses_model_class=VideoTranscriptEnabledFlag,
-            course_specific_model_class=CourseHLSPlaybackEnabledFlag
+            course_specific_model_class=CourseVideoTranscriptEnabledFlag
         )
 
     def test_enable_disable_globally(self):
@@ -205,6 +208,6 @@ class TestVideoTranscriptFlag(TestCase, FeatureFlagTestMixin):
         Ensures that the Video Transcript flag, once enabled globally, can also be disabled.
         """
         self.verify_enable_disable_globally(
-            all_courses_model_class=HLSPlaybackEnabledFlag,
+            all_courses_model_class=VideoTranscriptEnabledFlag,
-            course_specific_model_class=CourseHLSPlaybackEnabledFlag
+            course_specific_model_class=CourseVideoTranscriptEnabledFlag
         )

openedx/core/djangoapps/video_pipeline/admin.py

@@ -4,6 +4,23 @@ Django admin for Video Pipeline models.
 from config_models.admin import ConfigurationModelAdmin
 from django.contrib import admin
 
-from openedx.core.djangoapps.video_pipeline.models import VideoPipelineIntegration
+from openedx.core.djangoapps.video_config.admin import CourseSpecificEnabledFlagBaseAdmin
+from openedx.core.djangoapps.video_pipeline.forms import CourseVideoUploadsEnabledByDefaultAdminForm
+from openedx.core.djangoapps.video_pipeline.models import (
+    VideoPipelineIntegration,
+    VideoUploadsEnabledByDefault,
+    CourseVideoUploadsEnabledByDefault,
+)
+
+
+class CourseVideoUploadsEnabledByDefaultAdmin(CourseSpecificEnabledFlagBaseAdmin):
+    """
+    Admin of video uploads enabled by default feature on course-by-course basis.
+    Allows searching by course id.
+    """
+    form = CourseVideoUploadsEnabledByDefaultAdminForm
 
 admin.site.register(VideoPipelineIntegration, ConfigurationModelAdmin)
+
+admin.site.register(VideoUploadsEnabledByDefault, ConfigurationModelAdmin)
+admin.site.register(CourseVideoUploadsEnabledByDefault, CourseVideoUploadsEnabledByDefaultAdmin)

openedx/core/djangoapps/video_pipeline/forms.py

@@ -0,0 +1,15 @@
+"""
+Defines a form to provide validations for course-specific configuration.
+"""
+from openedx.core.djangoapps.video_config.forms import CourseSpecificFlagAdminBaseForm
+from openedx.core.djangoapps.video_pipeline.models import CourseVideoUploadsEnabledByDefault
+
+
+class CourseVideoUploadsEnabledByDefaultAdminForm(CourseSpecificFlagAdminBaseForm):
+    """
+    Form for course-specific Video Uploads enabled by default configuration.
+    """
+
+    class Meta(object):
+        model = CourseVideoUploadsEnabledByDefault
+        fields = '__all__'

openedx/core/djangoapps/video_pipeline/migrations/0003_coursevideouploadsenabledbydefault_videouploadsenabledbydefault.py

@@ -0,0 +1,46 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+from django.conf import settings
+import django.db.models.deletion
+import openedx.core.djangoapps.xmodule_django.models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        migrations.swappable_dependency(settings.AUTH_USER_MODEL),
+        ('video_pipeline', '0002_auto_20171114_0704'),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name='CourseVideoUploadsEnabledByDefault',
+            fields=[
+                ('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)),
+                ('change_date', models.DateTimeField(auto_now_add=True, verbose_name='Change date')),
+                ('enabled', models.BooleanField(default=False, verbose_name='Enabled')),
+                ('course_id', openedx.core.djangoapps.xmodule_django.models.CourseKeyField(max_length=255, db_index=True)),
+                ('changed_by', models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, editable=False, to=settings.AUTH_USER_MODEL, null=True, verbose_name='Changed by')),
+            ],
+            options={
+                'ordering': ('-change_date',),
+                'abstract': False,
+            },
+        ),
+        migrations.CreateModel(
+            name='VideoUploadsEnabledByDefault',
+            fields=[
+                ('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)),
+                ('change_date', models.DateTimeField(auto_now_add=True, verbose_name='Change date')),
+                ('enabled', models.BooleanField(default=False, verbose_name='Enabled')),
+                ('enabled_for_all_courses', models.BooleanField(default=False)),
+                ('changed_by', models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, editable=False, to=settings.AUTH_USER_MODEL, null=True, verbose_name='Changed by')),
+            ],
+            options={
+                'ordering': ('-change_date',),
+                'abstract': False,
+            },
+        ),
+    ]

openedx/core/djangoapps/video_pipeline/models.py

@@ -6,6 +6,8 @@ from django.contrib.auth import get_user_model
 from django.db import models
 from django.utils.translation import ugettext_lazy as _
 
+from openedx.core.djangoapps.xmodule_django.models import CourseKeyField
+
 
 class VideoPipelineIntegration(ConfigurationModel):
     """
@@ -37,3 +39,62 @@ class VideoPipelineIntegration(ConfigurationModel):
         # in lms/startup.py.
         User = get_user_model()  # pylint: disable=invalid-name
         return User.objects.get(username=self.service_username)
+
+
+class VideoUploadsEnabledByDefault(ConfigurationModel):
+    """
+    Enables video uploads enabled By default feature across the platform.
+    """
+    # this field overrides course-specific settings
+    enabled_for_all_courses = models.BooleanField(default=False)
+
+    @classmethod
+    def feature_enabled(cls, course_id):
+        """
+        Looks at the currently active configuration model to determine whether
+        the VideoUploadsEnabledByDefault feature is available.
+
+        If the feature flag is not enabled, the feature is not available.
+        If the flag is enabled for all the courses, feature is available.
+        If the flag is enabled and the provided course_id is for a course
+            with CourseVideoUploadsEnabledByDefault enabled, then the
+            feature is available.
+
+        Arguments:
+            course_id (CourseKey): course id for whom feature will be checked.
+        """
+        if not cls.is_enabled():
+            return False
+        elif not cls.current().enabled_for_all_courses:
+            feature = (CourseVideoUploadsEnabledByDefault.objects
+                       .filter(course_id=course_id)
+                       .order_by('-change_date')
+                       .first())
+            return feature.enabled if feature else False
+        return True
+
+    def __unicode__(self):
+        current_model = VideoUploadsEnabledByDefault.current()
+        return u"VideoUploadsEnabledByDefault: enabled {is_enabled}".format(
+            is_enabled=current_model.is_enabled()
+        )
+
+
+class CourseVideoUploadsEnabledByDefault(ConfigurationModel):
+    """
+    Enables video uploads enabled by default feature for a specific course. Its global feature must be
+    enabled for this to take effect.
+    """
+    KEY_FIELDS = ('course_id',)
+
+    course_id = CourseKeyField(max_length=255, db_index=True)
+
+    def __unicode__(self):
+        not_en = "Not "
+        if self.enabled:
+            not_en = ""
+
+        return u"Course '{course_key}': Video Uploads {not_enabled}Enabled by default.".format(
+            course_key=unicode(self.course_id),
+            not_enabled=not_en
+        )

openedx/core/djangoapps/video_pipeline/tests/test_models.py

@@ -0,0 +1,59 @@
+"""
+Tests for the models that configures 'VideoUploadsEnabledByDefault' feature.
+"""
+import ddt
+import itertools
+
+from django.test import TestCase
+from openedx.core.djangoapps.video_config.tests.test_models import FeatureFlagTestMixin
+from openedx.core.djangoapps.video_pipeline.models import (
+    CourseVideoUploadsEnabledByDefault, VideoUploadsEnabledByDefault,
+)
+
+
+@ddt.ddt
+class TestVideoUploadsEnabledByDefault(TestCase, FeatureFlagTestMixin):
+    """
+    Tests the behavior of the flags for video uploads enabled by default feature.
+    These are set via Django admin settings.
+    """
+    @ddt.data(
+        *itertools.product(
+            (True, False),
+            (True, False),
+            (True, False),
+        )
+    )
+    @ddt.unpack
+    def test_video_upload_enabled_by_default_feature_flags(self, global_flag,
+                                                           enabled_for_all_courses, enabled_for_course_1):
+        """
+        Tests that video uploads enabled by default feature flags works correctly on tweaking global flags
+        in combination with course-specific flags.
+        """
+        self.verify_feature_flags(
+            all_courses_model_class=VideoUploadsEnabledByDefault,
+            course_specific_model_class=CourseVideoUploadsEnabledByDefault,
+            global_flag=global_flag,
+            enabled_for_all_courses=enabled_for_all_courses,
+            enabled_for_course_1=enabled_for_course_1
+        )
+
+    def test_enable_disable_course_flag(self):
+        """
+        Ensures that the video uploads enabled by default course specific flag, once enabled for a course,
+        can also be disabled.
+        """
+        self.verify_enable_disable_course_flag(
+            all_courses_model_class=VideoUploadsEnabledByDefault,
+            course_specific_model_class=CourseVideoUploadsEnabledByDefault
+        )
+
+    def test_enable_disable_globally(self):
+        """
+        Ensures that the video uploads enabled by default flag, once enabled globally, can also be disabled.
+        """
+        self.verify_enable_disable_globally(
+            all_courses_model_class=VideoUploadsEnabledByDefault,
+            course_specific_model_class=CourseVideoUploadsEnabledByDefault
+        )