diff --git a/cms/djangoapps/contentstore/views/tests/test_videos.py b/cms/djangoapps/contentstore/views/tests/test_videos.py index c3e3d3b2ca..273217397c 100644 --- a/cms/djangoapps/contentstore/views/tests/test_videos.py +++ b/cms/djangoapps/contentstore/views/tests/test_videos.py @@ -554,6 +554,22 @@ class VideosHandlerTestCase(VideoUploadTestMixin, CourseTestCase): self.assert_video_status(url, edx_video_id, 'Failed') + @ddt.data(True, False) + @patch('openedx.core.djangoapps.video_config.models.VideoTranscriptEnabledFlag.feature_enabled') + def test_video_index_transcript_feature_enablement(self, is_video_transcript_enabled, video_transcript_feature): + """ + Test that when video transcript is enabled/disabled, correct response is rendered. + """ + video_transcript_feature.return_value = is_video_transcript_enabled + response = self.client.get(self.url) + self.assertEqual(response.status_code, 200) + + # Verify that course video button is present in the response if videos transcript feature is enabled. + self.assertEqual( + ' + <%if (dateModified) { %> - <%- gettext('Last updated')%> <%- dateModified %> + <%- gettext('Last updated')%> <%- dateModified %> <% } %> + diff --git a/cms/templates/videos_index.html b/cms/templates/videos_index.html index 3413e530eb..996dae1bf9 100644 --- a/cms/templates/videos_index.html +++ b/cms/templates/videos_index.html @@ -40,6 +40,7 @@ ${video_upload_max_file_size | n, dump_js_escaped_json}, ${active_transcript_preferences | n, dump_js_escaped_json}, ${video_transcript_settings | n, dump_js_escaped_json}, + ${is_video_transcript_enabled | n, dump_js_escaped_json}, ${video_image_settings | n, dump_js_escaped_json} ); }); @@ -55,12 +56,14 @@ > ${_("Video Uploads")} + % if is_video_transcript_enabled : + % endif diff --git a/openedx/core/djangoapps/video_config/admin.py b/openedx/core/djangoapps/video_config/admin.py index 521524fcf5..5c072b6aa3 100644 --- a/openedx/core/djangoapps/video_config/admin.py +++ b/openedx/core/djangoapps/video_config/admin.py @@ -5,16 +5,24 @@ Django admin dashboard configuration for Video XModule. from config_models.admin import ConfigurationModelAdmin, KeyedConfigurationModelAdmin from django.contrib import admin -from openedx.core.djangoapps.video_config.forms import CourseHLSPlaybackFlagAdminForm -from openedx.core.djangoapps.video_config.models import CourseHLSPlaybackEnabledFlag, HLSPlaybackEnabledFlag +from openedx.core.djangoapps.video_config.forms import ( + CourseHLSPlaybackFlagAdminForm, CourseVideoTranscriptFlagAdminForm +) +from openedx.core.djangoapps.video_config.models import ( + CourseHLSPlaybackEnabledFlag, HLSPlaybackEnabledFlag, + CourseVideoTranscriptEnabledFlag, VideoTranscriptEnabledFlag +) -class CourseHLSPlaybackEnabledFlagAdmin(KeyedConfigurationModelAdmin): +class CourseSpecificEnabledFlagBaseAdmin(KeyedConfigurationModelAdmin): """ - Admin of HLS Playback feature on course-by-course basis. + Admin of course specific feature on course-by-course basis. Allows searching by course id. """ - form = CourseHLSPlaybackFlagAdminForm + # Make abstract base class + class Meta(object): + abstract = True + search_fields = ['course_id'] fieldsets = ( (None, { @@ -23,5 +31,23 @@ class CourseHLSPlaybackEnabledFlagAdmin(KeyedConfigurationModelAdmin): }), ) + +class CourseHLSPlaybackEnabledFlagAdmin(CourseSpecificEnabledFlagBaseAdmin): + """ + Admin of HLS Playback feature on course-by-course basis. + Allows searching by course id. + """ + form = CourseHLSPlaybackFlagAdminForm + + +class CourseVideoTranscriptEnabledFlagAdmin(CourseSpecificEnabledFlagBaseAdmin): + """ + Admin of Video Transcript feature on course-by-course basis. + Allows searching by course id. + """ + form = CourseHLSPlaybackFlagAdminForm + admin.site.register(HLSPlaybackEnabledFlag, ConfigurationModelAdmin) admin.site.register(CourseHLSPlaybackEnabledFlag, CourseHLSPlaybackEnabledFlagAdmin) +admin.site.register(VideoTranscriptEnabledFlag, ConfigurationModelAdmin) +admin.site.register(CourseVideoTranscriptEnabledFlag, CourseHLSPlaybackEnabledFlagAdmin) diff --git a/openedx/core/djangoapps/video_config/forms.py b/openedx/core/djangoapps/video_config/forms.py index 93aab18a87..e37477d3d5 100644 --- a/openedx/core/djangoapps/video_config/forms.py +++ b/openedx/core/djangoapps/video_config/forms.py @@ -7,20 +7,20 @@ 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 +from openedx.core.djangoapps.video_config.models import CourseHLSPlaybackEnabledFlag, CourseVideoTranscriptEnabledFlag from xmodule.modulestore.django import modulestore log = logging.getLogger(__name__) -class CourseHLSPlaybackFlagAdminForm(forms.ModelForm): +class CourseSpecificFlagAdminBaseForm(forms.ModelForm): """ - Form for course-specific HLS Playback configuration. + Form for course-specific feature configuration. """ + # Make abstract base class class Meta(object): - model = CourseHLSPlaybackEnabledFlag - fields = '__all__' + abstract = True def clean_course_id(self): """ @@ -42,3 +42,23 @@ class CourseHLSPlaybackFlagAdminForm(forms.ModelForm): raise forms.ValidationError(msg) return course_key + + +class CourseHLSPlaybackFlagAdminForm(CourseSpecificFlagAdminBaseForm): + """ + Form for course-specific HLS Playback configuration. + """ + + class Meta(object): + model = CourseHLSPlaybackEnabledFlag + fields = '__all__' + + +class CourseVideoTranscriptFlagAdminForm(CourseSpecificFlagAdminBaseForm): + """ + Form for course-specific Video Transcript configuration. + """ + + class Meta(object): + model = CourseVideoTranscriptEnabledFlag + fields = '__all__' diff --git a/openedx/core/djangoapps/video_config/migrations/0002_coursevideotranscriptenabledflag_videotranscriptenabledflag.py b/openedx/core/djangoapps/video_config/migrations/0002_coursevideotranscriptenabledflag_videotranscriptenabledflag.py new file mode 100644 index 0000000000..2a2c6fb08e --- /dev/null +++ b/openedx/core/djangoapps/video_config/migrations/0002_coursevideotranscriptenabledflag_videotranscriptenabledflag.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_config', '0001_initial'), + ] + + operations = [ + migrations.CreateModel( + name='CourseVideoTranscriptEnabledFlag', + 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='VideoTranscriptEnabledFlag', + 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, + }, + ), + ] diff --git a/openedx/core/djangoapps/video_config/models.py b/openedx/core/djangoapps/video_config/models.py index 922c7354e7..cdd63066c2 100644 --- a/openedx/core/djangoapps/video_config/models.py +++ b/openedx/core/djangoapps/video_config/models.py @@ -66,3 +66,68 @@ class CourseHLSPlaybackEnabledFlag(ConfigurationModel): course_key=unicode(self.course_id), not_enabled=not_en ) + + +class VideoTranscriptEnabledFlag(ConfigurationModel): + """ + Enables Video Transcript across the platform. + When this feature flag is set to true, individual courses + must also have Video Transcript enabled for this feature to + take effect. + When this feature is enabled, 3rd party transcript integration functionality would be available accross all + courses or some specific courses and S3 video transcript would be served (currently as a fallback). + """ + # this field overrides course-specific settings + enabled_for_all_courses = BooleanField(default=False) + + @classmethod + def feature_enabled(cls, course_id): + """ + Looks at the currently active configuration model to determine whether + the Video Transcript 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 an course + with Video Transcript enabled, the feature is available. + + Arguments: + course_id (CourseKey): course id for whom feature will be checked. + """ + if not VideoTranscriptEnabledFlag.is_enabled(): + return False + elif not VideoTranscriptEnabledFlag.current().enabled_for_all_courses: + feature = (CourseVideoTranscriptEnabledFlag.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 = VideoTranscriptEnabledFlag.current() + return u"VideoTranscriptEnabledFlag: enabled {is_enabled}".format( + is_enabled=current_model.is_enabled() + ) + + +class CourseVideoTranscriptEnabledFlag(ConfigurationModel): + """ + Enables Video Transcript for a specific course. Global feature must be + enabled for this to take effect. + When this feature is enabled, 3rd party transcript integration functionality would be available for the + specific course and S3 video transcript would be served (currently as a fallback). + """ + 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 Transcript {not_enabled}Enabled".format( + course_key=unicode(self.course_id), + not_enabled=not_en + ) diff --git a/openedx/core/djangoapps/video_config/tests/test_models.py b/openedx/core/djangoapps/video_config/tests/test_models.py index 8aed33727a..aeb3622131 100644 --- a/openedx/core/djangoapps/video_config/tests/test_models.py +++ b/openedx/core/djangoapps/video_config/tests/test_models.py @@ -13,34 +13,115 @@ from openedx.core.djangoapps.video_config.models import CourseHLSPlaybackEnabled @contextmanager -def hls_playback_feature_flags( +def video_feature_flags( + all_courses_model_class, course_specific_model_class, global_flag, enabled_for_all_courses=False, course_id=None, enabled_for_course=False ): """ - Yields HLS Playback Configuration records for unit tests + Yields video feature configuration records for unit tests Arguments: + all_courses_model_class: Model class to enable feature for all courses + course_specific_model_class: Model class to nable feature for course specific global_flag (bool): Specifies whether feature is enabled globally enabled_for_all_courses (bool): Specifies whether feature is enabled for all courses course_id (CourseLocator): Course locator for course specific configurations enabled_for_course (bool): Specifies whether feature should be available for a course """ - HLSPlaybackEnabledFlag.objects.create(enabled=global_flag, enabled_for_all_courses=enabled_for_all_courses) + all_courses_model_class.objects.create(enabled=global_flag, enabled_for_all_courses=enabled_for_all_courses) if course_id: - CourseHLSPlaybackEnabledFlag.objects.create(course_id=course_id, enabled=enabled_for_course) + course_specific_model_class.objects.create(course_id=course_id, enabled=enabled_for_course) yield +class FeatureFlagTestMixin(object): + """ + Adds util methods to test the behavior of the flags for video feature. + """ + course_id_1 = CourseLocator(org="edx", course="course", run="run") + course_id_2 = CourseLocator(org="edx", course="course2", run="run") + + def verify_feature_flags(self, all_courses_model_class, course_specific_model_class, + global_flag, enabled_for_all_courses, enabled_for_course_1): + """ + Verifies that the feature flags works correctly on tweaking global flags in combination + with course-specific flags. + """ + with video_feature_flags( + all_courses_model_class=all_courses_model_class, + course_specific_model_class=course_specific_model_class, + global_flag=global_flag, + enabled_for_all_courses=enabled_for_all_courses, + course_id=self.course_id_1, + enabled_for_course=enabled_for_course_1 + ): + self.assertEqual( + all_courses_model_class.feature_enabled(self.course_id_1), + global_flag and (enabled_for_all_courses or enabled_for_course_1) + ) + self.assertEqual( + all_courses_model_class.feature_enabled(self.course_id_2), + global_flag and enabled_for_all_courses + ) + + def verify_enable_disable_course_flag(self, all_courses_model_class, course_specific_model_class): + """ + Verifies that the course specific flag, once enabled for a course, can also be disabled. + """ + with video_feature_flags( + all_courses_model_class=all_courses_model_class, + course_specific_model_class=course_specific_model_class, + global_flag=True, + enabled_for_all_courses=False, + course_id=self.course_id_1, + enabled_for_course=True + ): + self.assertTrue(all_courses_model_class.feature_enabled(self.course_id_1)) + with video_feature_flags( + all_courses_model_class=all_courses_model_class, + course_specific_model_class=course_specific_model_class, + global_flag=True, + enabled_for_all_courses=False, + course_id=self.course_id_1, + enabled_for_course=False + ): + self.assertFalse(all_courses_model_class.feature_enabled(self.course_id_1)) + + def verify_enable_disable_globally(self, all_courses_model_class, course_specific_model_class): + """ + Verifies that global flag, once enabled globally, can also be disabled. + """ + with video_feature_flags( + all_courses_model_class=all_courses_model_class, + course_specific_model_class=course_specific_model_class, + global_flag=True, + enabled_for_all_courses=True, + ): + self.assertTrue(all_courses_model_class.feature_enabled(self.course_id_1)) + self.assertTrue(all_courses_model_class.feature_enabled(self.course_id_2)) + with video_feature_flags( + all_courses_model_class=all_courses_model_class, + course_specific_model_class=course_specific_model_class, + global_flag=True, + enabled_for_all_courses=False, + ): + self.assertFalse(all_courses_model_class.feature_enabled(self.course_id_1)) + self.assertFalse(all_courses_model_class.feature_enabled(self.course_id_2)) + with video_feature_flags( + all_courses_model_class=all_courses_model_class, + course_specific_model_class=course_specific_model_class, + global_flag=False, + ): + self.assertFalse(all_courses_model_class.feature_enabled(self.course_id_1)) + self.assertFalse(all_courses_model_class.feature_enabled(self.course_id_2)) + + @ddt.ddt -class TestHLSPlaybackFlag(TestCase): +class TestHLSPlaybackFlag(TestCase, FeatureFlagTestMixin): """ Tests the behavior of the flags for HLS Playback feature. These are set via Django admin settings. """ - def setUp(self): - super(TestHLSPlaybackFlag, self).setUp() - self.course_id_1 = CourseLocator(org="edx", course="course", run="run") - self.course_id_2 = CourseLocator(org="edx", course="course2", run="run") @ddt.data( *itertools.product( @@ -52,61 +133,78 @@ class TestHLSPlaybackFlag(TestCase): @ddt.unpack def test_hls_playback_feature_flags(self, global_flag, enabled_for_all_courses, enabled_for_course_1): """ - Tests that the feature flags works correctly on tweaking global flags in combination + Tests that the HLS Playback feature flags works correctly on tweaking global flags in combination with course-specific flags. """ - with hls_playback_feature_flags( + self.verify_feature_flags( + all_courses_model_class=HLSPlaybackEnabledFlag, + course_specific_model_class=CourseHLSPlaybackEnabledFlag, global_flag=global_flag, enabled_for_all_courses=enabled_for_all_courses, - course_id=self.course_id_1, - enabled_for_course=enabled_for_course_1 - ): - self.assertEqual( - HLSPlaybackEnabledFlag.feature_enabled(self.course_id_1), - global_flag and (enabled_for_all_courses or enabled_for_course_1) - ) - self.assertEqual( - HLSPlaybackEnabledFlag.feature_enabled(self.course_id_2), - global_flag and enabled_for_all_courses - ) + enabled_for_course_1=enabled_for_course_1 + ) def test_enable_disable_course_flag(self): """ Ensures that the flag, once enabled for a course, can also be disabled. """ - with hls_playback_feature_flags( - global_flag=True, - enabled_for_all_courses=False, - course_id=self.course_id_1, - enabled_for_course=True - ): - self.assertTrue(HLSPlaybackEnabledFlag.feature_enabled(self.course_id_1)) - with hls_playback_feature_flags( - global_flag=True, - enabled_for_all_courses=False, - course_id=self.course_id_1, - enabled_for_course=False - ): - self.assertFalse(HLSPlaybackEnabledFlag.feature_enabled(self.course_id_1)) + self.verify_enable_disable_course_flag( + all_courses_model_class=HLSPlaybackEnabledFlag, + course_specific_model_class=CourseHLSPlaybackEnabledFlag + ) def test_enable_disable_globally(self): """ Ensures that the flag, once enabled globally, can also be disabled. """ - with hls_playback_feature_flags( - global_flag=True, - enabled_for_all_courses=True, - ): - self.assertTrue(HLSPlaybackEnabledFlag.feature_enabled(self.course_id_1)) - self.assertTrue(HLSPlaybackEnabledFlag.feature_enabled(self.course_id_2)) - with hls_playback_feature_flags( - global_flag=True, - enabled_for_all_courses=False, - ): - self.assertFalse(HLSPlaybackEnabledFlag.feature_enabled(self.course_id_1)) - self.assertFalse(HLSPlaybackEnabledFlag.feature_enabled(self.course_id_2)) - with hls_playback_feature_flags( - global_flag=False, - ): - self.assertFalse(HLSPlaybackEnabledFlag.feature_enabled(self.course_id_1)) - self.assertFalse(HLSPlaybackEnabledFlag.feature_enabled(self.course_id_2)) + self.verify_enable_disable_globally( + all_courses_model_class=HLSPlaybackEnabledFlag, + course_specific_model_class=CourseHLSPlaybackEnabledFlag + ) + + +@ddt.ddt +class TestVideoTranscriptFlag(TestCase, FeatureFlagTestMixin): + """ + Tests the behavior of the flags for Video Transcript feature. + These are set via Django admin settings. + """ + + @ddt.data( + *itertools.product( + (True, False), + (True, False), + (True, False), + ) + ) + @ddt.unpack + def test_video_transcript_feature_flags(self, global_flag, enabled_for_all_courses, enabled_for_course_1): + """ + Tests that Video Transcript feature flags works correctly on tweaking global flags in combination + with course-specific flags. + """ + self.verify_feature_flags( + all_courses_model_class=HLSPlaybackEnabledFlag, + course_specific_model_class=CourseHLSPlaybackEnabledFlag, + 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 Transcript course specific flag, once enabled for a course, can also be disabled. + """ + self.verify_enable_disable_course_flag( + all_courses_model_class=HLSPlaybackEnabledFlag, + course_specific_model_class=CourseHLSPlaybackEnabledFlag + ) + + def test_enable_disable_globally(self): + """ + Ensures that the Video Transcript flag, once enabled globally, can also be disabled. + """ + self.verify_enable_disable_globally( + all_courses_model_class=HLSPlaybackEnabledFlag, + course_specific_model_class=CourseHLSPlaybackEnabledFlag + )