From a830073bb281b87536210ba933910cefa278f9b2 Mon Sep 17 00:00:00 2001 From: Eric Fischer Date: Thu, 21 Sep 2017 16:31:46 -0400 Subject: [PATCH 1/2] Add waffle flag for NewAssetsPage --- cms/djangoapps/contentstore/admin.py | 22 ++++- .../contentstore/config/__init__.py | 0 cms/djangoapps/contentstore/config/forms.py | 37 ++++++++ cms/djangoapps/contentstore/config/models.py | 77 ++++++++++++++++ .../contentstore/config/tests/__init__.py | 0 .../contentstore/config/tests/test_models.py | 92 +++++++++++++++++++ .../contentstore/config/tests/utils.py | 27 ++++++ .../migrations/0002_add_assets_page_flag.py | 38 ++++++++ cms/djangoapps/contentstore/views/assets.py | 2 + 9 files changed, 294 insertions(+), 1 deletion(-) create mode 100644 cms/djangoapps/contentstore/config/__init__.py create mode 100644 cms/djangoapps/contentstore/config/forms.py create mode 100644 cms/djangoapps/contentstore/config/models.py create mode 100644 cms/djangoapps/contentstore/config/tests/__init__.py create mode 100644 cms/djangoapps/contentstore/config/tests/test_models.py create mode 100644 cms/djangoapps/contentstore/config/tests/utils.py create mode 100644 cms/djangoapps/contentstore/migrations/0002_add_assets_page_flag.py diff --git a/cms/djangoapps/contentstore/admin.py b/cms/djangoapps/contentstore/admin.py index c28a5355e8..0807e6843e 100644 --- a/cms/djangoapps/contentstore/admin.py +++ b/cms/djangoapps/contentstore/admin.py @@ -2,10 +2,30 @@ Admin site bindings for contentstore """ -from config_models.admin import ConfigurationModelAdmin +from config_models.admin import ConfigurationModelAdmin, KeyedConfigurationModelAdmin from django.contrib import admin +from contentstore.config.forms import CourseNewAssetsPageAdminForm +from contentstore.config.models import NewAssetsPageFlag, CourseNewAssetsPageFlag from contentstore.models import PushNotificationConfig, VideoUploadConfig + +class CourseNewAssetsPageAdmin(KeyedConfigurationModelAdmin): + """ + Admin for enabling new asset page on a course-by-course basis. + Allows searching by course id. + """ + form = CourseNewAssetsPageAdminForm + search_fields = ['course_id'] + fieldsets = ( + (None, { + 'fields': ('course_id', 'enabled'), + 'description': 'Enter a valid course id. If it is invalid, an error message will display.' + }), + ) + +admin.site.register(NewAssetsPageFlag, ConfigurationModelAdmin) +admin.site.register(CourseNewAssetsPageFlag, CourseNewAssetsPageAdmin) + admin.site.register(VideoUploadConfig, ConfigurationModelAdmin) admin.site.register(PushNotificationConfig, ConfigurationModelAdmin) diff --git a/cms/djangoapps/contentstore/config/__init__.py b/cms/djangoapps/contentstore/config/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/cms/djangoapps/contentstore/config/forms.py b/cms/djangoapps/contentstore/config/forms.py new file mode 100644 index 0000000000..a9e77a1cf7 --- /dev/null +++ b/cms/djangoapps/contentstore/config/forms.py @@ -0,0 +1,37 @@ +""" +Defines a form for providing validation. +""" +import logging + +from django import forms + +from contentstore.config.models import CourseNewAssetsPageFlag + +from opaque_keys import InvalidKeyError +from xmodule.modulestore.django import modulestore +from opaque_keys.edx.locator import CourseLocator + +log = logging.getLogger(__name__) + + +class CourseNewAssetsPageAdminForm(forms.ModelForm): + """Input form for new asset page enablment, allowing us to verify user input.""" + + class Meta(object): + model = CourseNewAssetsPageFlag + 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: "{0}."'.format(cleaned_id) + raise forms.ValidationError(msg) + + if not modulestore().has_course(course_key): + msg = u'Course not found. Entered course id was: "{0}". '.format(course_key.to_deprecated_string()) + raise forms.ValidationError(msg) + + return course_key diff --git a/cms/djangoapps/contentstore/config/models.py b/cms/djangoapps/contentstore/config/models.py new file mode 100644 index 0000000000..a75bf17bd2 --- /dev/null +++ b/cms/djangoapps/contentstore/config/models.py @@ -0,0 +1,77 @@ +""" +Models for configuration of the feature flags +controlling the new assets page. +""" +from config_models.models import ConfigurationModel +from django.db.models import BooleanField +from openedx.core.djangoapps.xmodule_django.models import CourseKeyField + + +class NewAssetsPageFlag(ConfigurationModel): + """ + Enables the in-development new assets page from studio-frontend. + + Defaults to False platform-wide, but can be overriden via a course-specific + flag. The idea is that we can use this to do a gradual rollout, and remove + the flag entirely once generally released to everyone. + """ + # this field overrides course-specific settings to enable the feature for all courses + enabled_for_all_courses = BooleanField(default=False) + + @classmethod + def feature_enabled(cls, course_id=None): + """ + Looks at the currently active configuration model to determine whether + the new assets page feature is available. + + There are 2 booleans to be concerned with - enabled_for_all_courses, + and the implicit is_enabled(). They interact in the following ways: + - is_enabled: False, enabled_for_all_courses: True or False + - no one can use the feature. + - is_enabled: True, enabled_for_all_courses: False + - check for a CourseNewAssetsPageFlag, use that value (default False) + - if no course_id provided, return False + - is_enabled: True, enabled_for_all_courses: True + - everyone can use the feature + """ + if not NewAssetsPageFlag.is_enabled(): + return False + elif not NewAssetsPageFlag.current().enabled_for_all_courses: + if course_id: + effective = CourseNewAssetsPageFlag.objects.filter(course_id=course_id).order_by('-change_date').first() + return effective.enabled if effective is not None else False + else: + return False + else: + return True + + class Meta(object): + app_label = "contentstore" + + def __unicode__(self): + current_model = NewAssetsPageFlag.current() + return u"NewAssetsPageFlag: enabled {}".format( + current_model.is_enabled() + ) + + +class CourseNewAssetsPageFlag(ConfigurationModel): + """ + Enables new assets page for a specific + course. Only has an effect if the general + flag above is set to True. + """ + KEY_FIELDS = ('course_id',) + + class Meta(object): + app_label = "contentstore" + + # The course that these features are attached to. + course_id = CourseKeyField(max_length=255, db_index=True) + + def __unicode__(self): + not_en = "Not " + if self.enabled: + not_en = "" + # pylint: disable=no-member + return u"Course '{}': Persistent Grades {}Enabled".format(self.course_id.to_deprecated_string(), not_en) diff --git a/cms/djangoapps/contentstore/config/tests/__init__.py b/cms/djangoapps/contentstore/config/tests/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/cms/djangoapps/contentstore/config/tests/test_models.py b/cms/djangoapps/contentstore/config/tests/test_models.py new file mode 100644 index 0000000000..743d38c147 --- /dev/null +++ b/cms/djangoapps/contentstore/config/tests/test_models.py @@ -0,0 +1,92 @@ +""" +Tests for the models that control the +persistent grading feature. +""" +import itertools + +import ddt +from django.conf import settings +from django.test import TestCase +from mock import patch +from opaque_keys.edx.locator import CourseLocator + +from contentstore.config.models import NewAssetsPageFlag +from contentstore.config.tests.utils import new_assets_page_feature_flags + + +@ddt.ddt +class NewAssetsPageFlagTests(TestCase): + """ + Tests the behavior of the feature flags for the new assets page. + These are set via Django admin settings. + """ + def setUp(self): + super(NewAssetsPageFlagTests, 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_new_assets_page_feature_flags(self, global_flag, enabled_for_all_courses, enabled_for_course_1): + with new_assets_page_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(NewAssetsPageFlag.feature_enabled(), global_flag and enabled_for_all_courses) + self.assertEqual( + NewAssetsPageFlag.feature_enabled(self.course_id_1), + global_flag and (enabled_for_all_courses or enabled_for_course_1) + ) + self.assertEqual( + NewAssetsPageFlag.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 new_assets_page_feature_flags( + global_flag=True, + enabled_for_all_courses=False, + course_id=self.course_id_1, + enabled_for_course=True + ): + self.assertTrue(NewAssetsPageFlag.feature_enabled(self.course_id_1)) + with new_assets_page_feature_flags( + global_flag=True, + enabled_for_all_courses=False, + course_id=self.course_id_1, + enabled_for_course=False + ): + self.assertFalse(NewAssetsPageFlag.feature_enabled(self.course_id_1)) + + def test_enable_disable_globally(self): + """ + Ensures that the flag, once enabled globally, can also be disabled. + """ + with new_assets_page_feature_flags( + global_flag=True, + enabled_for_all_courses=True, + ): + self.assertTrue(NewAssetsPageFlag.feature_enabled()) + self.assertTrue(NewAssetsPageFlag.feature_enabled(self.course_id_1)) + with new_assets_page_feature_flags( + global_flag=True, + enabled_for_all_courses=False, + course_id=self.course_id_1, + enabled_for_course=True + ): + self.assertFalse(NewAssetsPageFlag.feature_enabled()) + self.assertTrue(NewAssetsPageFlag.feature_enabled(self.course_id_1)) + with new_assets_page_feature_flags( + global_flag=False, + ): + self.assertFalse(NewAssetsPageFlag.feature_enabled()) + self.assertFalse(NewAssetsPageFlag.feature_enabled(self.course_id_1)) diff --git a/cms/djangoapps/contentstore/config/tests/utils.py b/cms/djangoapps/contentstore/config/tests/utils.py new file mode 100644 index 0000000000..69430ccb1d --- /dev/null +++ b/cms/djangoapps/contentstore/config/tests/utils.py @@ -0,0 +1,27 @@ +""" +Provides helper functions for tests that want +to configure flags related to persistent grading. +""" +from contextlib import contextmanager + +from contentstore.config.models import NewAssetsPageFlag, CourseNewAssetsPageFlag +from request_cache.middleware import RequestCache + + +@contextmanager +def new_assets_page_feature_flags( + global_flag, + enabled_for_all_courses=False, + course_id=None, + enabled_for_course=False +): + """ + Most test cases will use a single call to this manager, + as they need to set the global setting and the course-specific + setting for a single course. + """ + RequestCache.clear_request_cache() + NewAssetsPageFlag.objects.create(enabled=global_flag, enabled_for_all_courses=enabled_for_all_courses) + if course_id: + CourseNewAssetsPageFlag.objects.create(course_id=course_id, enabled=enabled_for_course) + yield diff --git a/cms/djangoapps/contentstore/migrations/0002_add_assets_page_flag.py b/cms/djangoapps/contentstore/migrations/0002_add_assets_page_flag.py new file mode 100644 index 0000000000..8d4469ac39 --- /dev/null +++ b/cms/djangoapps/contentstore/migrations/0002_add_assets_page_flag.py @@ -0,0 +1,38 @@ +# -*- 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), + ('contentstore', '0001_initial'), + ] + + operations = [ + migrations.CreateModel( + name='CourseNewAssetsPageFlag', + 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')), + ], + ), + migrations.CreateModel( + name='NewAssetsPageFlag', + 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')), + ], + ), + ] diff --git a/cms/djangoapps/contentstore/views/assets.py b/cms/djangoapps/contentstore/views/assets.py index cd782a598c..86b7ecc20a 100644 --- a/cms/djangoapps/contentstore/views/assets.py +++ b/cms/djangoapps/contentstore/views/assets.py @@ -18,6 +18,7 @@ from xmodule.exceptions import NotFoundError from xmodule.modulestore.django import modulestore from xmodule.modulestore.exceptions import ItemNotFoundError +from contentstore.config.models import NewAssetsPageFlag from contentstore.utils import reverse_course_url from contentstore.views.exception import AssetNotFoundException, AssetSizeTooLargeException from edxmako.shortcuts import render_to_response @@ -95,6 +96,7 @@ def _asset_index(course_key): course_module = modulestore().get_course(course_key) return render_to_response('asset_index.html', { + 'waffle_flag_enabled': NewAssetsPageFlag.feature_enabled(course_key), 'context_course': course_module, 'max_file_size_in_mbs': settings.MAX_ASSET_UPLOAD_FILE_SIZE_IN_MB, 'chunk_size_in_mbs': settings.UPLOAD_CHUNK_SIZE_IN_MB, From ac801833433c024cbea6c472eb69835814e3cdac Mon Sep 17 00:00:00 2001 From: Eric Fischer Date: Thu, 21 Sep 2017 16:32:06 -0400 Subject: [PATCH 2/2] Add studio-frontend to studio, if waffle flag set --- .gitignore | 1 + cms/templates/asset_index.html | 23 +++++++++++++++++------ package.json | 1 + pavelib/assets.py | 18 +++++++++++++++--- 4 files changed, 34 insertions(+), 9 deletions(-) diff --git a/.gitignore b/.gitignore index 637b00b853..13e6d07f4f 100644 --- a/.gitignore +++ b/.gitignore @@ -95,6 +95,7 @@ lms/static/css/ lms/static/certificates/css/ cms/static/css/ common/static/common/js/vendor/ +common/static/common/css/vendor/ common/static/bundles webpack-stats.json diff --git a/cms/templates/asset_index.html b/cms/templates/asset_index.html index aa9e4e65cc..9634501eaa 100644 --- a/cms/templates/asset_index.html +++ b/cms/templates/asset_index.html @@ -13,13 +13,19 @@ <%namespace name='static' file='static_content.html'/> <%block name="header_extras"> -% for template_name in ["asset"]: - -% endfor +% if waffle_flag_enabled: + + +% else: + % for template_name in ["asset"]: + + % endfor +% endif +% if not waffle_flag_enabled: <%block name="requirejs"> require(["js/factories/asset_index"], function (AssetIndexFactory) { AssetIndexFactory({ @@ -30,6 +36,7 @@ }); }); +% endif <%block name="content"> @@ -53,7 +60,11 @@
-
+ % if waffle_flag_enabled: +
+ % else: +
+ % endif

${_("Loading")}

diff --git a/package.json b/package.json index fa0be9b955..cd3cb8a4ba 100644 --- a/package.json +++ b/package.json @@ -10,6 +10,7 @@ "backbone.paginator": "~2.0.3", "coffee-loader": "^0.7.3", "coffee-script": "1.6.1", + "@edx/studio-frontend": "0.1.0", "edx-bootstrap": "^0.2.1", "edx-pattern-library": "0.18.1", "edx-ui-toolkit": "1.5.2", diff --git a/pavelib/assets.py b/pavelib/assets.py index 2d4261c219..282372d9f8 100644 --- a/pavelib/assets.py +++ b/pavelib/assets.py @@ -64,6 +64,10 @@ NPM_INSTALLED_LIBRARIES = [ 'requirejs/require.js', 'underscore.string/dist/underscore.string.js', 'underscore/underscore.js', + '@edx/studio-frontend/dist/assets.min.js', + '@edx/studio-frontend/dist/assets.min.js.map', + '@edx/studio-frontend/dist/studio-frontend.min.css', + '@edx/studio-frontend/dist/studio-frontend.min.css.map' ] # A list of NPM installed developer libraries that should be copied into the common @@ -74,7 +78,9 @@ NPM_INSTALLED_DEVELOPER_LIBRARIES = [ ] # Directory to install static vendor files -NPM_VENDOR_DIRECTORY = path('common/static/common/js/vendor') +NPM_JS_VENDOR_DIRECTORY = path('common/static/common/js/vendor') +NPM_CSS_VENDOR_DIRECTORY = path("common/static/common/css/vendor") +NPM_CSS_DIRECTORY = path("common/static/common/css") # system specific lookup path additions, add sass dirs if one system depends on the sass files for other systems SASS_LOOKUP_DEPENDENCIES = { @@ -604,10 +610,14 @@ def process_npm_assets(): Copies a vendor library to the shared vendor directory. """ library_path = 'node_modules/{library}'.format(library=library) + if library.endswith('.css') or library.endswith('.css.map'): + vendor_dir = NPM_CSS_VENDOR_DIRECTORY + else: + vendor_dir = NPM_JS_VENDOR_DIRECTORY if os.path.exists(library_path): sh('/bin/cp -rf {library_path} {vendor_dir}'.format( library_path=library_path, - vendor_dir=NPM_VENDOR_DIRECTORY, + vendor_dir=vendor_dir, )) elif not skip_if_missing: raise Exception('Missing vendor file {library_path}'.format(library_path=library_path)) @@ -618,7 +628,9 @@ def process_npm_assets(): return # Ensure that the vendor directory exists - NPM_VENDOR_DIRECTORY.mkdir_p() + NPM_JS_VENDOR_DIRECTORY.mkdir_p() + NPM_CSS_DIRECTORY.mkdir_p() + NPM_CSS_VENDOR_DIRECTORY.mkdir_p() # Copy each file to the vendor directory, overwriting any existing file. print("Copying vendor files into static directory")