diff --git a/cms/static/js/factories/xblock_validation.js b/cms/static/js/factories/xblock_validation.js
index 14ed40a953..4bcfc3b339 100644
--- a/cms/static/js/factories/xblock_validation.js
+++ b/cms/static/js/factories/xblock_validation.js
@@ -1,12 +1,16 @@
define(['js/views/xblock_validation', 'js/models/xblock_validation'],
function(XBlockValidationView, XBlockValidationModel) {
'use strict';
- return function(validationMessages, hasEditingUrl, isRoot, validationEle) {
+ return function(validationMessages, hasEditingUrl, isRoot, isUnit, validationEle) {
+ var model, response;
+
if (hasEditingUrl && !isRoot) {
validationMessages.showSummaryOnly = true;
}
+ response = validationMessages;
+ response.isUnit = isUnit;
- var model = new XBlockValidationModel(validationMessages, {parse: true});
+ model = new XBlockValidationModel(response, {parse: true});
if (!model.get('empty')) {
new XBlockValidationView({el: validationEle, model: model, root: isRoot}).render();
diff --git a/cms/static/js/models/xblock_validation.js b/cms/static/js/models/xblock_validation.js
index 7f913668d7..bdfe645f78 100644
--- a/cms/static/js/models/xblock_validation.js
+++ b/cms/static/js/models/xblock_validation.js
@@ -19,7 +19,11 @@ define(['backbone', 'gettext', 'underscore'], function(Backbone, gettext, _) {
var summary = 'summary' in response ? response.summary : {};
var messages = 'messages' in response ? response.messages : [];
if (!summary.text) {
- summary.text = gettext('This component has validation issues.');
+ if (response.isUnit) {
+ summary.text = gettext('This unit has validation issues.');
+ } else {
+ summary.text = gettext('This component has validation issues.');
+ }
}
if (!summary.type) {
summary.type = this.WARNING;
diff --git a/cms/static/js/spec/factories/xblock_validation_spec.js b/cms/static/js/spec/factories/xblock_validation_spec.js
index 7f5dc1182f..23cbcbc0e1 100644
--- a/cms/static/js/spec/factories/xblock_validation_spec.js
+++ b/cms/static/js/spec/factories/xblock_validation_spec.js
@@ -1,22 +1,22 @@
define(['jquery', 'js/factories/xblock_validation', 'common/js/spec_helpers/template_helpers'],
function($, XBlockValidationFactory, TemplateHelpers) {
describe('XBlockValidationFactory', function() {
- var messageDiv;
+ var $messageDiv;
beforeEach(function() {
TemplateHelpers.installTemplate('xblock-validation-messages');
appendSetFixtures($(''));
- messageDiv = $('.messages');
+ $messageDiv = $('.messages');
});
it('Does not attach a view if messages is empty', function() {
- XBlockValidationFactory({'empty': true}, false, false, messageDiv);
- expect(messageDiv.children().length).toEqual(0);
+ XBlockValidationFactory({empty: true}, false, false, false, $messageDiv);
+ expect($messageDiv.children().length).toEqual(0);
});
it('Does attach a view if messages are not empty', function() {
- XBlockValidationFactory({'empty': false}, false, false, messageDiv);
- expect(messageDiv.children().length).toEqual(1);
+ XBlockValidationFactory({empty: false}, false, false, false, $messageDiv);
+ expect($messageDiv.children().length).toEqual(1);
});
it('Passes through the root property to the view.', function() {
@@ -29,12 +29,12 @@ define(['jquery', 'js/factories/xblock_validation', 'common/js/spec_helpers/temp
'xblock_id': 'id'
};
// Root is false, will not add noContainerContent.
- XBlockValidationFactory(notConfiguredMessages, true, false, messageDiv);
- expect(messageDiv.find('.validation')).not.toHaveClass(noContainerContent);
+ XBlockValidationFactory(notConfiguredMessages, true, false, false, $messageDiv);
+ expect($messageDiv.find('.validation')).not.toHaveClass(noContainerContent);
// Root is true, will add noContainerContent.
- XBlockValidationFactory(notConfiguredMessages, true, true, messageDiv);
- expect(messageDiv.find('.validation')).toHaveClass(noContainerContent);
+ XBlockValidationFactory(notConfiguredMessages, true, true, false, $messageDiv);
+ expect($messageDiv.find('.validation')).toHaveClass(noContainerContent);
});
describe('Controls display of detailed messages based on url and root property', function() {
@@ -50,25 +50,25 @@ define(['jquery', 'js/factories/xblock_validation', 'common/js/spec_helpers/temp
});
checkDetailedMessages = function(expectedDetailedMessages) {
- expect(messageDiv.children().length).toEqual(1);
- expect(messageDiv.find('.xblock-message-item').length).toBe(expectedDetailedMessages);
+ expect($messageDiv.children().length).toEqual(1);
+ expect($messageDiv.find('.xblock-message-item').length).toBe(expectedDetailedMessages);
};
it('Does not show details if xblock has an editing URL and it is not rendered as root', function() {
- XBlockValidationFactory(messagesWithSummary, true, false, messageDiv);
+ XBlockValidationFactory(messagesWithSummary, true, false, false, $messageDiv);
checkDetailedMessages(0);
});
it('Shows details if xblock does not have its own editing URL, regardless of root value', function() {
- XBlockValidationFactory(messagesWithSummary, false, false, messageDiv);
+ XBlockValidationFactory(messagesWithSummary, false, false, false, $messageDiv);
checkDetailedMessages(2);
- XBlockValidationFactory(messagesWithSummary, false, true, messageDiv);
+ XBlockValidationFactory(messagesWithSummary, false, true, false, $messageDiv);
checkDetailedMessages(2);
});
it('Shows details if xblock has its own editing URL and is rendered as root', function() {
- XBlockValidationFactory(messagesWithSummary, true, true, messageDiv);
+ XBlockValidationFactory(messagesWithSummary, true, true, false, $messageDiv);
checkDetailedMessages(2);
});
});
diff --git a/cms/templates/studio_xblock_wrapper.html b/cms/templates/studio_xblock_wrapper.html
index fea81c1e5f..66afb2aee9 100644
--- a/cms/templates/studio_xblock_wrapper.html
+++ b/cms/templates/studio_xblock_wrapper.html
@@ -3,6 +3,7 @@
from django.utils.translation import ugettext as _
from contentstore.views.helpers import xblock_studio_url
from contentstore.utils import is_visible_to_specific_partition_groups
+from lms.lib.utils import is_unit
from openedx.core.djangolib.js_utils import (
dump_js_escaped_json, js_escaped_string
)
@@ -14,6 +15,7 @@ section_class = "level-nesting" if show_inline else "level-element"
collapsible_class = "is-collapsible" if xblock.has_children else ""
label = xblock.display_name_with_default or xblock.scope_ids.block_type
messages = xblock.validate().to_json()
+block_is_unit = is_unit(xblock)
%>
<%namespace name='static' file='static_content.html'/>
@@ -30,6 +32,7 @@ messages = xblock.validate().to_json()
${messages | n, dump_js_escaped_json},
${bool(xblock_url) | n, dump_js_escaped_json}, // xblock_url will be None or a string
${bool(is_root) | n, dump_js_escaped_json}, // is_root will be None or a boolean
+ ${bool(block_is_unit) | n, dump_js_escaped_json}, // block_is_unit will be None or a boolean
$('div.xblock-validation-messages[data-locator="${xblock.location | n, js_escaped_string}"]')
);
});
diff --git a/cms/templates/visibility_editor.html b/cms/templates/visibility_editor.html
index 98e8efcc68..191049024c 100644
--- a/cms/templates/visibility_editor.html
+++ b/cms/templates/visibility_editor.html
@@ -4,6 +4,7 @@ from django.conf import settings
from django.utils.translation import ugettext as _
from contentstore.utils import ancestor_has_staff_lock, get_visibility_partition_info
from openedx.core.djangolib.markup import HTML, Text
+from lms.lib.utils import is_unit
partition_info = get_visibility_partition_info(xblock)
selectable_partitions = partition_info["selectable_partitions"]
@@ -11,18 +12,30 @@ selected_partition_index = partition_info["selected_partition_index"]
selected_groups_label = partition_info["selected_groups_label"]
is_staff_locked = ancestor_has_staff_lock(xblock)
+block_is_unit = is_unit(xblock)
%>
-
% if len(selectable_partitions) == 0:
${_('Access is not restricted')}
-
${_('Access to this component is not restricted, but visibility might be affected by inherited settings.')}
- % if settings.FEATURES.get('ENABLE_ENROLLMENT_TRACK_USER_PARTITION'):
-
${_('You can restrict access to this component to learners in specific enrollment tracks or content groups.')}
+ % if block_is_unit:
+
${_('Access to this unit is not restricted, but visibility might be affected by inherited settings.')}
% else:
-
${_('You can restrict access to this component to learners in specific content groups.')}
+
${_('Access to this component is not restricted, but visibility might be affected by inherited settings.')}
+ %endif
+ % if settings.FEATURES.get('ENABLE_ENROLLMENT_TRACK_USER_PARTITION'):
+ % if block_is_unit:
+
${_('You can restrict access to this unit to learners in specific enrollment tracks or content groups.')}
+ % else:
+
${_('You can restrict access to this component to learners in specific enrollment tracks or content groups.')}
+ % endif
+ % else:
+ % if block_is_unit:
+
${_('You can restrict access to this unit to learners in specific content groups.')}
+ % else:
+
${_('You can restrict access to this component to learners in specific content groups.')}
+ % endif
% endif
@@ -98,7 +111,9 @@ is_staff_locked = ancestor_has_staff_lock(xblock)
% if group["deleted"]:
% else:
diff --git a/common/test/acceptance/tests/studio/test_studio_container.py b/common/test/acceptance/tests/studio/test_studio_container.py
index 06ae27ab99..409ead8084 100644
--- a/common/test/acceptance/tests/studio/test_studio_container.py
+++ b/common/test/acceptance/tests/studio/test_studio_container.py
@@ -319,7 +319,7 @@ class BaseGroupConfigurationsTest(ContainerBase):
CHOOSE_ONE = "Select a group type"
CONTENT_GROUP_PARTITION = XBlockVisibilityEditorView.CONTENT_GROUP_PARTITION
ENROLLMENT_TRACK_PARTITION = XBlockVisibilityEditorView.ENROLLMENT_TRACK_PARTITION
- MISSING_GROUP_LABEL = 'Deleted Group\nThis group no longer exists. Choose another group or do not restrict access to this component.'
+ MISSING_GROUP_LABEL = 'Deleted Group\nThis group no longer exists. Choose another group or remove the access restriction.'
VALIDATION_ERROR_LABEL = 'This component has validation issues.'
VALIDATION_ERROR_MESSAGE = "Error:\nThis component's access settings refer to deleted or invalid groups."
GROUP_VISIBILITY_MESSAGE = 'Access to some content in this unit is restricted to specific groups of learners.'
diff --git a/lms/djangoapps/lms_xblock/mixin.py b/lms/djangoapps/lms_xblock/mixin.py
index cc1082c56b..aa136cd52f 100644
--- a/lms/djangoapps/lms_xblock/mixin.py
+++ b/lms/djangoapps/lms_xblock/mixin.py
@@ -8,6 +8,7 @@ from xblock.core import XBlock
from xblock.fields import Boolean, Dict, Scope, String, XBlockMixin
from xblock.validation import ValidationMessage
+from lms.lib.utils import is_unit
from xmodule.modulestore.inheritance import UserPartitionList
from xmodule.partitions.partitions import NoSuchUserPartitionError, NoSuchUserPartitionGroupError
@@ -15,8 +16,16 @@ from xmodule.partitions.partitions import NoSuchUserPartitionError, NoSuchUserPa
# more information can be found here: https://openedx.atlassian.net/browse/PLAT-902
_ = lambda text: text
-INVALID_USER_PARTITION_VALIDATION = _(u"This component's access settings refer to deleted or invalid group configurations.")
-INVALID_USER_PARTITION_GROUP_VALIDATION = _(u"This component's access settings refer to deleted or invalid groups.")
+INVALID_USER_PARTITION_VALIDATION_COMPONENT = _(
+ u"This component's access settings refer to deleted or invalid group configurations."
+)
+INVALID_USER_PARTITION_VALIDATION_UNIT = _(
+ u"This unit's access settings refer to deleted or invalid group configurations."
+)
+INVALID_USER_PARTITION_GROUP_VALIDATION_COMPONENT = _(
+ u"This component's access settings refer to deleted or invalid groups."
+)
+INVALID_USER_PARTITION_GROUP_VALIDATION_UNIT = _(u"This unit's access settings refer to deleted or invalid groups.")
NONSENSICAL_ACCESS_RESTRICTION = _(u"This component's access settings contradict its parent's access settings.")
@@ -32,6 +41,7 @@ class GroupAccessDict(Dict):
@XBlock.needs('partitions')
+@XBlock.needs('i18n')
class LmsBlockMixin(XBlockMixin):
"""
Mixin that defines fields common to all blocks used in the LMS
@@ -185,6 +195,7 @@ class LmsBlockMixin(XBlockMixin):
validation = super(LmsBlockMixin, self).validate()
has_invalid_user_partitions = False
has_invalid_groups = False
+ block_is_unit = is_unit(self)
for user_partition_id, group_ids in self.group_access.iteritems():
try:
@@ -204,7 +215,9 @@ class LmsBlockMixin(XBlockMixin):
validation.add(
ValidationMessage(
ValidationMessage.ERROR,
- INVALID_USER_PARTITION_VALIDATION
+ (INVALID_USER_PARTITION_VALIDATION_UNIT
+ if block_is_unit
+ else INVALID_USER_PARTITION_VALIDATION_COMPONENT)
)
)
@@ -212,7 +225,9 @@ class LmsBlockMixin(XBlockMixin):
validation.add(
ValidationMessage(
ValidationMessage.ERROR,
- INVALID_USER_PARTITION_GROUP_VALIDATION
+ (INVALID_USER_PARTITION_GROUP_VALIDATION_UNIT
+ if block_is_unit
+ else INVALID_USER_PARTITION_GROUP_VALIDATION_COMPONENT)
)
)
diff --git a/lms/lib/tests/test_utils.py b/lms/lib/tests/test_utils.py
index 4054ee979e..9215a83f17 100644
--- a/lms/lib/tests/test_utils.py
+++ b/lms/lib/tests/test_utils.py
@@ -57,3 +57,11 @@ class LmsUtilsTest(ModuleStoreTestCase):
self.assertIsNone(utils.get_parent_unit(self.course))
self.assertIsNone(utils.get_parent_unit(self.chapter))
self.assertIsNone(utils.get_parent_unit(self.sequential))
+
+ def test_is_unit(self):
+ """
+ Tests `is_unit` method for the successful result.
+ """
+ self.assertFalse(utils.is_unit(self.html_module_1))
+ self.assertFalse(utils.is_unit(self.child_vertical))
+ self.assertTrue(utils.is_unit(self.vertical))
diff --git a/lms/lib/utils.py b/lms/lib/utils.py
index 3617767561..9328733782 100644
--- a/lms/lib/utils.py
+++ b/lms/lib/utils.py
@@ -28,3 +28,19 @@ def get_parent_unit(xblock):
if parent.category == "vertical" and grandparent.category == "sequential":
return parent
xblock = parent
+
+
+def is_unit(xblock):
+ """
+ Checks whether the xblock is a unit.
+
+ Get_parent_unit() returns None if the current xblock either does
+ not have a parent unit or is itself a unit.
+ To make sure that get_parent_unit() isn't returning None because
+ the xblock is an orphan, we check that the xblock has a parent.
+
+ Returns:
+ True if the xblock is itself a unit, False otherwise.
+ """
+
+ return get_parent_unit(xblock) is None and xblock.get_parent()
diff --git a/lms/lib/xblock/test/test_mixin.py b/lms/lib/xblock/test/test_mixin.py
index a4d8c4a114..c9c533e550 100644
--- a/lms/lib/xblock/test/test_mixin.py
+++ b/lms/lib/xblock/test/test_mixin.py
@@ -5,7 +5,11 @@ import ddt
from nose.plugins.attrib import attr
from lms_xblock.mixin import (
- INVALID_USER_PARTITION_VALIDATION, INVALID_USER_PARTITION_GROUP_VALIDATION, NONSENSICAL_ACCESS_RESTRICTION
+ INVALID_USER_PARTITION_GROUP_VALIDATION_COMPONENT,
+ INVALID_USER_PARTITION_GROUP_VALIDATION_UNIT,
+ INVALID_USER_PARTITION_VALIDATION_COMPONENT,
+ INVALID_USER_PARTITION_VALIDATION_UNIT,
+ NONSENSICAL_ACCESS_RESTRICTION
)
from xblock.validation import ValidationMessage
from xmodule.modulestore import ModuleStoreEnum
@@ -92,14 +96,14 @@ class XBlockValidationTest(LmsXBlockMixinTestCase):
def test_validate_invalid_user_partitions(self):
"""
- Test the validation messages produced for an xblock referring to non-existent user partitions.
+ Test the validation messages produced for a component referring to non-existent user partitions.
"""
self.set_group_access(self.video_location, {999: [self.group1.id]})
validation = self.store.get_item(self.video_location).validate()
self.assertEqual(len(validation.messages), 1)
self.verify_validation_message(
validation.messages[0],
- INVALID_USER_PARTITION_VALIDATION,
+ INVALID_USER_PARTITION_VALIDATION_COMPONENT,
ValidationMessage.ERROR,
)
@@ -111,7 +115,32 @@ class XBlockValidationTest(LmsXBlockMixinTestCase):
self.assertEqual(len(validation.messages), 1)
self.verify_validation_message(
validation.messages[0],
- INVALID_USER_PARTITION_VALIDATION,
+ INVALID_USER_PARTITION_VALIDATION_COMPONENT,
+ ValidationMessage.ERROR,
+ )
+
+ def test_validate_invalid_user_partitions_unit(self):
+ """
+ Test the validation messages produced for a unit referring to non-existent user partitions.
+ """
+ self.set_group_access(self.vertical_location, {999: [self.group1.id]})
+ validation = self.store.get_item(self.vertical_location).validate()
+ self.assertEqual(len(validation.messages), 1)
+ self.verify_validation_message(
+ validation.messages[0],
+ INVALID_USER_PARTITION_VALIDATION_UNIT,
+ ValidationMessage.ERROR,
+ )
+
+ # Now add a second invalid user partition and validate again.
+ # Note that even though there are two invalid configurations,
+ # only a single error message will be returned.
+ self.set_group_access(self.vertical_location, {998: [self.group2.id]})
+ validation = self.store.get_item(self.vertical_location).validate()
+ self.assertEqual(len(validation.messages), 1)
+ self.verify_validation_message(
+ validation.messages[0],
+ INVALID_USER_PARTITION_VALIDATION_UNIT,
ValidationMessage.ERROR,
)
@@ -124,7 +153,7 @@ class XBlockValidationTest(LmsXBlockMixinTestCase):
self.assertEqual(len(validation.messages), 1)
self.verify_validation_message(
validation.messages[0],
- INVALID_USER_PARTITION_GROUP_VALIDATION,
+ INVALID_USER_PARTITION_GROUP_VALIDATION_COMPONENT,
ValidationMessage.ERROR,
)
@@ -134,7 +163,7 @@ class XBlockValidationTest(LmsXBlockMixinTestCase):
self.assertEqual(len(validation.messages), 1)
self.verify_validation_message(
validation.messages[0],
- INVALID_USER_PARTITION_GROUP_VALIDATION,
+ INVALID_USER_PARTITION_GROUP_VALIDATION_COMPONENT,
ValidationMessage.ERROR,
)
@@ -165,6 +194,19 @@ class XBlockValidationTest(LmsXBlockMixinTestCase):
ValidationMessage.ERROR,
)
+ def test_validate_invalid_groups_for_unit(self):
+ """
+ Test the validation messages produced for a unit-level xblock referring to non-existent groups.
+ """
+ self.set_group_access(self.vertical_location, {self.user_partition.id: [self.group1.id, 999]})
+ validation = self.store.get_item(self.vertical_location).validate()
+ self.assertEqual(len(validation.messages), 1)
+ self.verify_validation_message(
+ validation.messages[0],
+ INVALID_USER_PARTITION_GROUP_VALIDATION_UNIT,
+ ValidationMessage.ERROR,
+ )
+
def test_validate_nonsensical_access_restriction(self):
"""
Test the validation messages produced for a component whose
@@ -221,7 +263,7 @@ class XBlockValidationTest(LmsXBlockMixinTestCase):
self.assertEqual(len(validation.messages), 2)
self.verify_validation_message(
validation.messages[0],
- INVALID_USER_PARTITION_GROUP_VALIDATION,
+ INVALID_USER_PARTITION_GROUP_VALIDATION_COMPONENT,
ValidationMessage.ERROR,
)
self.verify_validation_message(