From fe54b085c6cc1651bd35f829de4e186d918dd4e3 Mon Sep 17 00:00:00 2001 From: Usman Khalid <2200617@gmail.com> Date: Wed, 1 Jul 2015 20:55:59 +0500 Subject: [PATCH] Added ability to disable xblock types in LMS. TNL-2305 --- cms/envs/common.py | 2 + common/djangoapps/xblock_django/admin.py | 9 +++ .../xblock_django/migrations/0001_initial.py | 74 +++++++++++++++++++ .../xblock_django/migrations/__init__.py | 0 common/djangoapps/xblock_django/models.py | 39 ++++++++++ common/lib/xmodule/xmodule/hidden_module.py | 3 + .../xmodule/xmodule/modulestore/__init__.py | 3 +- .../lib/xmodule/xmodule/modulestore/django.py | 11 +++ .../xmodule/xmodule/modulestore/mongo/base.py | 1 + .../xmodule/modulestore/split_mongo/split.py | 1 + common/lib/xmodule/xmodule/x_module.py | 14 +++- .../courseware/tests/test_module_render.py | 23 ++++++ lms/envs/common.py | 5 ++ .../sass/course/courseware/_courseware.scss | 18 +++-- openedx/core/lib/xblock_utils.py | 5 +- 15 files changed, 199 insertions(+), 9 deletions(-) create mode 100644 common/djangoapps/xblock_django/admin.py create mode 100644 common/djangoapps/xblock_django/migrations/0001_initial.py create mode 100644 common/djangoapps/xblock_django/migrations/__init__.py create mode 100644 common/djangoapps/xblock_django/models.py diff --git a/cms/envs/common.py b/cms/envs/common.py index d9b7725859..c4d0026368 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -795,6 +795,8 @@ INSTALLED_APPS = ( # Import/Export API 'rest_framework', 'openedx.core.djangoapps.import_export', + + 'xblock_django', ) diff --git a/common/djangoapps/xblock_django/admin.py b/common/djangoapps/xblock_django/admin.py new file mode 100644 index 0000000000..8114460671 --- /dev/null +++ b/common/djangoapps/xblock_django/admin.py @@ -0,0 +1,9 @@ +""" +Django admin dashboard configuration. +""" + +from django.contrib import admin +from config_models.admin import ConfigurationModelAdmin +from xblock_django.models import XBlockDisableConfig + +admin.site.register(XBlockDisableConfig, ConfigurationModelAdmin) diff --git a/common/djangoapps/xblock_django/migrations/0001_initial.py b/common/djangoapps/xblock_django/migrations/0001_initial.py new file mode 100644 index 0000000000..68485f0196 --- /dev/null +++ b/common/djangoapps/xblock_django/migrations/0001_initial.py @@ -0,0 +1,74 @@ +# -*- coding: utf-8 -*- +from south.utils import datetime_utils as datetime +from south.db import db +from south.v2 import SchemaMigration +from django.db import models + + +class Migration(SchemaMigration): + + def forwards(self, orm): + # Adding model 'XBlockDisableConfig' + db.create_table('xblock_django_xblockdisableconfig', ( + ('id', self.gf('django.db.models.fields.AutoField')(primary_key=True)), + ('change_date', self.gf('django.db.models.fields.DateTimeField')(auto_now_add=True, blank=True)), + ('changed_by', self.gf('django.db.models.fields.related.ForeignKey')(to=orm['auth.User'], null=True, on_delete=models.PROTECT)), + ('enabled', self.gf('django.db.models.fields.BooleanField')(default=False)), + ('disabled_blocks', self.gf('django.db.models.fields.TextField')(default='', blank=True)), + )) + db.send_create_signal('xblock_django', ['XBlockDisableConfig']) + + + def backwards(self, orm): + # Deleting model 'XBlockDisableConfig' + db.delete_table('xblock_django_xblockdisableconfig') + + + 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'}) + }, + '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'}) + }, + 'xblock_django.xblockdisableconfig': { + 'Meta': {'ordering': "('-change_date',)", 'object_name': 'XBlockDisableConfig'}, + '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'}), + 'disabled_blocks': ('django.db.models.fields.TextField', [], {'default': "''", 'blank': 'True'}), + 'enabled': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}) + } + } + + complete_apps = ['xblock_django'] \ No newline at end of file diff --git a/common/djangoapps/xblock_django/migrations/__init__.py b/common/djangoapps/xblock_django/migrations/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/common/djangoapps/xblock_django/models.py b/common/djangoapps/xblock_django/models.py new file mode 100644 index 0000000000..863be4a3a7 --- /dev/null +++ b/common/djangoapps/xblock_django/models.py @@ -0,0 +1,39 @@ +""" +Models. +""" +from django.utils.translation import ugettext_lazy as _ + +from django.db.models import TextField + +from config_models.models import ConfigurationModel + + +class XBlockDisableConfig(ConfigurationModel): + """ + Configuration for disabling XBlocks. + """ + + disabled_blocks = TextField( + default='', blank=True, + help_text=_('Space-separated list of XBlocks which should not render.') + ) + + @classmethod + def is_block_type_disabled(cls, block_type): + """ Return True if block_type is disabled. """ + + config = cls.current() + if not config.enabled: + return False + + return block_type in config.disabled_blocks.split() # pylint: disable=no-member + + @classmethod + def disabled_block_types(cls): + """ Return list of disabled xblock types. """ + + config = cls.current() + if not config.enabled: + return () + + return config.disabled_blocks.split() # pylint: disable=no-member diff --git a/common/lib/xmodule/xmodule/hidden_module.py b/common/lib/xmodule/xmodule/hidden_module.py index 50a72af786..ebce62ee49 100644 --- a/common/lib/xmodule/xmodule/hidden_module.py +++ b/common/lib/xmodule/xmodule/hidden_module.py @@ -3,6 +3,9 @@ from xmodule.raw_module import RawDescriptor class HiddenModule(XModule): + + HIDDEN = True + def get_html(self): if self.system.user_is_staff: return u"ERROR: This module is unknown--students will not see it at all" diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index 9d997cc2a5..c511aa9b32 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -1122,7 +1122,7 @@ class ModuleStoreReadBase(BulkOperationsMixin, ModuleStoreRead): contentstore=None, doc_store_config=None, # ignore if passed up metadata_inheritance_cache_subsystem=None, request_cache=None, - xblock_mixins=(), xblock_select=None, + xblock_mixins=(), xblock_select=None, disabled_xblock_types=(), # pylint: disable=bad-continuation # temporary parms to enable backward compatibility. remove once all envs migrated db=None, collection=None, host=None, port=None, tz_aware=True, user=None, password=None, # allow lower level init args to pass harmlessly @@ -1139,6 +1139,7 @@ class ModuleStoreReadBase(BulkOperationsMixin, ModuleStoreRead): self.request_cache = request_cache self.xblock_mixins = xblock_mixins self.xblock_select = xblock_select + self.disabled_xblock_types = disabled_xblock_types self.contentstore = contentstore def get_course_errors(self, course_key): diff --git a/common/lib/xmodule/xmodule/modulestore/django.py b/common/lib/xmodule/xmodule/modulestore/django.py index e851f43c71..8c631a75ae 100644 --- a/common/lib/xmodule/xmodule/modulestore/django.py +++ b/common/lib/xmodule/xmodule/modulestore/django.py @@ -46,6 +46,11 @@ try: except ImportError: HAS_USER_SERVICE = False +try: + from xblock_django.models import XBlockDisableConfig +except ImportError: + XBlockDisableConfig = None + log = logging.getLogger(__name__) ASSET_IGNORE_REGEX = getattr(settings, "ASSET_IGNORE_REGEX", r"(^\._.*$)|(^\.DS_Store$)|(^.*~$)") @@ -161,12 +166,18 @@ def create_modulestore_instance( if 'read_preference' in doc_store_config: doc_store_config['read_preference'] = getattr(ReadPreference, doc_store_config['read_preference']) + if XBlockDisableConfig and settings.FEATURES.get('ENABLE_DISABLING_XBLOCK_TYPES', False): + disabled_xblock_types = XBlockDisableConfig.disabled_block_types() + else: + disabled_xblock_types = () + return class_( contentstore=content_store, metadata_inheritance_cache_subsystem=metadata_inheritance_cache, request_cache=request_cache, xblock_mixins=getattr(settings, 'XBLOCK_MIXINS', ()), xblock_select=getattr(settings, 'XBLOCK_SELECT_FUNCTION', None), + disabled_xblock_types=disabled_xblock_types, doc_store_config=doc_store_config, i18n_service=i18n_service or ModuleI18nService(), fs_service=fs_service or xblock.reference.plugins.FSService(), diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py index d7ef5d542b..898db379c0 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py @@ -937,6 +937,7 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo cached_metadata=cached_metadata, mixins=self.xblock_mixins, select=self.xblock_select, + disabled_xblock_types=self.disabled_xblock_types, services=services, ) else: diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py index b8e425eb27..64ccfb824a 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py @@ -3099,6 +3099,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): render_template=self.render_template, mixins=self.xblock_mixins, select=self.xblock_select, + disabled_xblock_types=self.disabled_xblock_types, services=self.services, ) diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index 4cb4ccfd40..0280655965 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -1298,9 +1298,9 @@ class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, Runtime): # p """ Base class for :class:`Runtime`s to be used with :class:`XModuleDescriptor`s """ - + # pylint: disable=bad-continuation def __init__( - self, load_item, resources_fs, error_tracker, get_policy=None, **kwargs + self, load_item, resources_fs, error_tracker, get_policy=None, disabled_xblock_types=(), **kwargs ): """ load_item: Takes a Location and returns an XModuleDescriptor @@ -1358,10 +1358,20 @@ class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, Runtime): # p else: self.get_policy = lambda u: {} + self.disabled_xblock_types = disabled_xblock_types + def get_block(self, usage_id, for_parent=None): """See documentation for `xblock.runtime:Runtime.get_block`""" return self.load_item(usage_id, for_parent=for_parent) + def load_block_type(self, block_type): + """ + Returns a subclass of :class:`.XBlock` that corresponds to the specified `block_type`. + """ + if block_type in self.disabled_xblock_types: + return self.default_class + return super(DescriptorSystem, self).load_block_type(block_type) + def get_field_provenance(self, xblock, field): """ For the given xblock, return a dict for the field's current state: diff --git a/lms/djangoapps/courseware/tests/test_module_render.py b/lms/djangoapps/courseware/tests/test_module_render.py index 3459f4eb46..ba9331fa6b 100644 --- a/lms/djangoapps/courseware/tests/test_module_render.py +++ b/lms/djangoapps/courseware/tests/test_module_render.py @@ -1687,3 +1687,26 @@ class TestFilteredChildren(ModuleStoreTestCase): Used to assert that sets of children are equivalent. """ self.assertEquals(set(child_usage_ids), set(child.scope_ids.usage_id for child in block.get_children())) + + +@attr('shard_1') +@ddt.ddt +class TestDisabledXBlockTypes(ModuleStoreTestCase): + """ + Tests that verify disabled XBlock types are not loaded. + """ + # pylint: disable=attribute-defined-outside-init, no-member + def setUp(self): + super(TestDisabledXBlockTypes, self).setUp() + + for store in self.store.modulestores: + store.disabled_xblock_types = ('combinedopenended', 'peergrading', 'video') + + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) + def test_get_item(self, default_ms): + with self.store.default_store(default_ms): + course = CourseFactory() + for block_type in ('peergrading', 'combinedopenended', 'video'): + item = ItemFactory(category=block_type, parent=course) + item = self.store.get_item(item.scope_ids.usage_id) + self.assertEqual(item.__class__.__name__, 'RawDescriptorWithMixins') diff --git a/lms/envs/common.py b/lms/envs/common.py index a26aa80ba0..38f3e1e5c6 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -413,6 +413,9 @@ FEATURES = { # Full Course/Library Import/Export API 'ENABLE_IMPORT_EXPORT_LMS': False, + + # The block types to disable need to be specified in "x block disable config" in django admin. + 'ENABLE_DISABLING_XBLOCK_TYPES': True, } # Ignore static asset files on import which match this pattern @@ -1929,6 +1932,8 @@ INSTALLED_APPS = ( # Import/Export API 'openedx.core.djangoapps.import_export', + + 'xblock_django', ) ######################### CSRF ######################################### diff --git a/lms/static/sass/course/courseware/_courseware.scss b/lms/static/sass/course/courseware/_courseware.scss index 9548715201..d43a0597c1 100644 --- a/lms/static/sass/course/courseware/_courseware.scss +++ b/lms/static/sass/course/courseware/_courseware.scss @@ -142,11 +142,6 @@ div.course-wrapper { list-style: none; > div { - @extend .clearfix; - border-bottom: 1px solid #ddd; - margin-bottom: ($baseline*0.75); - padding: 0 0 15px; - .collapsible { header { margin-bottom: 0; @@ -212,6 +207,19 @@ div.course-wrapper { } } } + + .vert > .xblock-student_view { + @extend .clearfix; + border-bottom: 1px solid #ddd; + margin-bottom: ($baseline*0.75); + padding: 0 0 15px; + } + + .vert > .xblock-student_view.is-hidden { + display: none; + border-bottom: 0px; + margin-bottom: 0px; + } } section.xblock-student_view-wrapper div.vert-mod > div { diff --git a/openedx/core/lib/xblock_utils.py b/openedx/core/lib/xblock_utils.py index bc105af8c4..3431a8e15d 100644 --- a/openedx/core/lib/xblock_utils.py +++ b/openedx/core/lib/xblock_utils.py @@ -106,6 +106,9 @@ def wrap_xblock( # The block is acting as an XModuleDescriptor css_classes.append('xmodule_edit') + if getattr(block, 'HIDDEN', False): + css_classes.append('is-hidden') + css_classes.append('xmodule_' + markupsafe.escape(class_name)) data['type'] = block.js_module_name shim_xmodule_js(block, frag) @@ -240,7 +243,7 @@ def add_staff_markup(user, has_instructor_access, disable_staff_debug_info, bloc else: return frag - if isinstance(block, SequenceModule): + if isinstance(block, SequenceModule) or getattr(block, 'HIDDEN', False): return frag block_id = block.location