diff --git a/cms/envs/common.py b/cms/envs/common.py index 9a1e362692..ffc1631134 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -230,9 +230,6 @@ FEATURES = { # Whether or not the dynamic EnrollmentTrackUserPartition should be registered. 'ENABLE_ENROLLMENT_TRACK_USER_PARTITION': False, - - # Fetch `hls` profile from edx-val or not - 'ENABLE_HLS_VIDEO_PROFILE': False, } ENABLE_JASMINE = False @@ -937,6 +934,9 @@ INSTALLED_APPS = ( # Self-paced course configuration 'openedx.core.djangoapps.self_paced', + # Video module configs (This will be moved to Video once it becomes an XBlock) + 'openedx.core.djangoapps.video_config', + # django-oauth2-provider (deprecated) 'provider', 'provider.oauth2', diff --git a/common/lib/xmodule/xmodule/video_module/video_module.py b/common/lib/xmodule/xmodule/video_module/video_module.py index 26b8b92fe8..cc384f3802 100644 --- a/common/lib/xmodule/xmodule/video_module/video_module.py +++ b/common/lib/xmodule/xmodule/video_module/video_module.py @@ -24,6 +24,7 @@ from pkg_resources import resource_string from django.conf import settings from openedx.core.lib.cache_utils import memoize_in_request_cache +from openedx.core.djangoapps.video_config.models import HLSPlaybackEnabledFlag from xblock.core import XBlock from xblock.fields import ScopeIds from xblock.runtime import KvsFieldData @@ -219,7 +220,7 @@ class VideoModule(VideoFields, VideoTranscriptsMixin, VideoStudentViewHandlers, try: val_profiles = ["youtube", "desktop_webm", "desktop_mp4"] - if settings.FEATURES.get('ENABLE_HLS_VIDEO_PROFILE', False): + if HLSPlaybackEnabledFlag.feature_enabled(self.course_id): val_profiles.append('hls') # strip edx_video_id to prevent ValVideoNotFoundError error if unwanted spaces are there. TNL-5769 diff --git a/lms/djangoapps/courseware/tests/test_video_mongo.py b/lms/djangoapps/courseware/tests/test_video_mongo.py index 1d5e857bbe..72bd14ed33 100644 --- a/lms/djangoapps/courseware/tests/test_video_mongo.py +++ b/lms/djangoapps/courseware/tests/test_video_mongo.py @@ -177,6 +177,7 @@ class TestVideoNonYouTube(TestVideo): @attr(shard=1) +@ddt.ddt class TestGetHtmlMethod(BaseTestXmodule): ''' Make sure that `get_html` works correctly. @@ -855,7 +856,33 @@ class TestGetHtmlMethod(BaseTestXmodule): self.item_descriptor.xmodule_runtime.render_template('video.html', expected_context) ) - @patch.dict('django.conf.settings.FEATURES', {'ENABLE_HLS_VIDEO_PROFILE': True}) + @ddt.data( + (True, ['youtube', 'desktop_webm', 'desktop_mp4', 'hls']), + (False, ['youtube', 'desktop_webm', 'desktop_mp4']) + ) + @ddt.unpack + def test_get_html_on_toggling_hls_feature(self, hls_feature_enabled, expected_val_profiles): + """ + Verify val profiles on toggling HLS Playback feature. + """ + with patch('xmodule.video_module.video_module.edxval_api.get_urls_for_profiles') as get_urls_for_profiles: + get_urls_for_profiles.return_value = { + 'desktop_webm': 'https://webm.com/dw.webm', + 'hls': 'https://hls.com/hls.m3u8', + 'youtube': 'https://yt.com/?v=v0TFmdO4ZP0', + 'desktop_mp4': 'https://mp4.com/dm.mp4' + } + with patch('xmodule.video_module.video_module.HLSPlaybackEnabledFlag.feature_enabled') as feature_enabled: + feature_enabled.return_value = hls_feature_enabled + video_xml = '' + self.initialize_module(data=video_xml) + self.item_descriptor.render(STUDENT_VIEW) + get_urls_for_profiles.assert_called_with( + self.item_descriptor.edx_video_id, + expected_val_profiles, + ) + + @patch('xmodule.video_module.video_module.HLSPlaybackEnabledFlag.feature_enabled', Mock(return_value=True)) @patch('xmodule.video_module.video_module.edxval_api.get_urls_for_profiles') def test_get_html_hls(self, get_urls_for_profiles): """ @@ -882,7 +909,6 @@ class TestGetHtmlMethod(BaseTestXmodule): '"sources": ["https://webm.com/dw.webm", "https://mp4.com/dm.mp4", "https://hls.com/hls.m3u8"]', context ) - @patch.dict('django.conf.settings.FEATURES', {'ENABLE_HLS_VIDEO_PROFILE': True}) def test_get_html_hls_no_video_id(self): """ Verify that `download_video_link` is set to None for HLS videos if no video id diff --git a/lms/envs/common.py b/lms/envs/common.py index c3cb23dc5e..6bc25f8283 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -2153,6 +2153,9 @@ INSTALLED_APPS = ( # Verified Track Content Cohorting (Beta feature that will hopefully be removed) 'openedx.core.djangoapps.verified_track_content', + # Video module configs (This will be moved to Video once it becomes an XBlock) + 'openedx.core.djangoapps.video_config', + # Learner's dashboard 'learner_dashboard', diff --git a/openedx/core/djangoapps/video_config/__init__.py b/openedx/core/djangoapps/video_config/__init__.py new file mode 100644 index 0000000000..c1188dde9b --- /dev/null +++ b/openedx/core/djangoapps/video_config/__init__.py @@ -0,0 +1 @@ +# TODO Move this Application to video codebase when Video XModule becomes an XBlock. Reference: TNL-6867. diff --git a/openedx/core/djangoapps/video_config/admin.py b/openedx/core/djangoapps/video_config/admin.py new file mode 100644 index 0000000000..dbc56de70b --- /dev/null +++ b/openedx/core/djangoapps/video_config/admin.py @@ -0,0 +1,27 @@ +""" +Django admin dashboard configuration for Video XModule. +""" + +from django.contrib import admin +from config_models.admin import ConfigurationModelAdmin, KeyedConfigurationModelAdmin + +from openedx.core.djangoapps.video_config.forms import CourseHLSPlaybackFlagAdminForm +from openedx.core.djangoapps.video_config.models import CourseHLSPlaybackEnabledFlag, HLSPlaybackEnabledFlag + + +class CourseHLSPlaybackEnabledFlagAdmin(KeyedConfigurationModelAdmin): + """ + Admin of HLS Playback feature on course-by-course basis. + Allows searching by course id. + """ + form = CourseHLSPlaybackFlagAdminForm + search_fields = ['course_id'] + fieldsets = ( + (None, { + 'fields': ('course_id', 'enabled'), + 'description': 'Enter a valid course id. If it is invalid, an error message will be displayed.' + }), + ) + +admin.site.register(HLSPlaybackEnabledFlag, ConfigurationModelAdmin) +admin.site.register(CourseHLSPlaybackEnabledFlag, CourseHLSPlaybackEnabledFlagAdmin) diff --git a/openedx/core/djangoapps/video_config/forms.py b/openedx/core/djangoapps/video_config/forms.py new file mode 100644 index 0000000000..30ce6cea0b --- /dev/null +++ b/openedx/core/djangoapps/video_config/forms.py @@ -0,0 +1,45 @@ +""" +Defines a form for providing validation of HLS Playback course-specific configuration. +""" +import logging + +from django import forms + +from openedx.core.djangoapps.video_config.models import CourseHLSPlaybackEnabledFlag + +from opaque_keys import InvalidKeyError +from xmodule.modulestore.django import modulestore +from opaque_keys.edx.locator import CourseLocator + +log = logging.getLogger(__name__) + + +class CourseHLSPlaybackFlagAdminForm(forms.ModelForm): + """ + Form for course-specific HLS Playback configuration. + """ + + class Meta(object): + model = CourseHLSPlaybackEnabledFlag + fields = '__all__' + + def clean_course_id(self): + """ + Validate the course id + """ + cleaned_id = self.cleaned_data["course_id"] + try: + course_key = CourseLocator.from_string(cleaned_id) + except InvalidKeyError: + msg = u'Course id invalid. Entered course id was: "{course_id}."'.format( + course_id=cleaned_id + ) + raise forms.ValidationError(msg) + + if not modulestore().has_course(course_key): + msg = u'Course not found. Entered course id was: "{course_key}". '.format( + course_key=unicode(course_key) + ) + raise forms.ValidationError(msg) + + return course_key diff --git a/openedx/core/djangoapps/video_config/migrations/0001_initial.py b/openedx/core/djangoapps/video_config/migrations/0001_initial.py new file mode 100644 index 0000000000..4b90a73207 --- /dev/null +++ b/openedx/core/djangoapps/video_config/migrations/0001_initial.py @@ -0,0 +1,45 @@ +# -*- 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), + ] + + operations = [ + migrations.CreateModel( + name='CourseHLSPlaybackEnabledFlag', + 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='HLSPlaybackEnabledFlag', + 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/migrations/__init__.py b/openedx/core/djangoapps/video_config/migrations/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/openedx/core/djangoapps/video_config/models.py b/openedx/core/djangoapps/video_config/models.py new file mode 100644 index 0000000000..922c7354e7 --- /dev/null +++ b/openedx/core/djangoapps/video_config/models.py @@ -0,0 +1,68 @@ +""" +Configuration models for Video XModule +""" +from config_models.models import ConfigurationModel +from django.db.models import BooleanField + +from openedx.core.djangoapps.xmodule_django.models import CourseKeyField + + +class HLSPlaybackEnabledFlag(ConfigurationModel): + """ + Enables HLS Playback across the platform. + When this feature flag is set to true, individual courses + must also have HLS Playback enabled for this feature to + take effect. + """ + # 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 HLS Playback 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 HLS Playback enabled, the feature is available. + + Arguments: + course_id (CourseKey): course id for whom feature will be checked. + """ + if not HLSPlaybackEnabledFlag.is_enabled(): + return False + elif not HLSPlaybackEnabledFlag.current().enabled_for_all_courses: + feature = (CourseHLSPlaybackEnabledFlag.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 = HLSPlaybackEnabledFlag.current() + return u"HLSPlaybackEnabledFlag: enabled {is_enabled}".format( + is_enabled=current_model.is_enabled() + ) + + +class CourseHLSPlaybackEnabledFlag(ConfigurationModel): + """ + Enables HLS Playback for a specific course. 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}': HLS Playback {not_enabled}Enabled".format( + course_key=unicode(self.course_id), + not_enabled=not_en + ) diff --git a/openedx/core/djangoapps/video_config/tests/__init__.py b/openedx/core/djangoapps/video_config/tests/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/openedx/core/djangoapps/video_config/tests/test_models.py b/openedx/core/djangoapps/video_config/tests/test_models.py new file mode 100644 index 0000000000..8aed33727a --- /dev/null +++ b/openedx/core/djangoapps/video_config/tests/test_models.py @@ -0,0 +1,112 @@ +""" +Tests for the models that configures HLS Playback feature. +""" +import ddt +import itertools + +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 + + +@contextmanager +def hls_playback_feature_flags( + global_flag, enabled_for_all_courses=False, + course_id=None, enabled_for_course=False +): + """ + Yields HLS Playback Configuration records for unit tests + Arguments: + 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) + if course_id: + CourseHLSPlaybackEnabledFlag.objects.create(course_id=course_id, enabled=enabled_for_course) + yield + + +@ddt.ddt +class TestHLSPlaybackFlag(TestCase): + """ + 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( + (True, False), + (True, False), + (True, False), + ) + ) + @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 + with course-specific flags. + """ + with hls_playback_feature_flags( + 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 + ) + + 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)) + + 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))