Commit a395c2fa by cahrens Committed by Andy Armstrong

Complete Studio support for handling group configuration changes

STUD-1658
parent 73e7ced6
define(["jquery", "underscore", "js/views/xblock", "js/utils/module", "gettext", "js/views/feedback_notification"],
define(["jquery", "underscore", "js/views/xblock", "js/utils/module", "gettext", "js/views/feedback_notification",
"jquery.ui"], // The container view uses sortable, which is provided by jquery.ui.
function ($, _, XBlockView, ModuleUtils, gettext, NotificationView) {
var reorderableClass = '.reorderable-container',
sortableInitializedClass = '.ui-sortable',
......
......@@ -168,10 +168,6 @@ define(["jquery", "underscore", "gettext", "js/views/modals/base_modal",
if (this.runtime) {
this.runtime.notify('modal-hidden');
}
// Completely clear the contents of the modal
this.undelegateEvents();
this.$el.html("");
},
findXBlockInfo: function(xblockWrapperElement, defaultXBlockInfo) {
......
......@@ -28,8 +28,11 @@ define(["jquery", "underscore", "gettext", "js/views/baseview", "js/views/contai
// Hide both blocks until we know which one to show
xblockView.$el.addClass('is-hidden');
// Add actions to any top level buttons, e.g. "Edit" of the container itself
self.addButtonActions(this.$el);
if (!options || !options.refresh) {
// Add actions to any top level buttons, e.g. "Edit" of the container itself.
// Do not add the actions on "refresh" though, as the handlers are already registered.
self.addButtonActions(this.$el);
}
// Render the xblock
xblockView.render({
......@@ -192,7 +195,7 @@ define(["jquery", "underscore", "gettext", "js/views/baseview", "js/views/contai
parentElement = xblockElement.parent(),
rootLocator = this.xblockView.model.id;
if (xblockElement.length === 0 || xblockElement.data('locator') === rootLocator) {
this.render({ });
this.render({refresh: true});
} else if (parentElement.hasClass('reorderable-container')) {
this.refreshChildXBlock(xblockElement);
} else {
......
......@@ -152,47 +152,51 @@
padding: ($baseline/2) ($baseline*.75);
color: $white;
.message-text {
display: inline-block;
width: 93%;
vertical-align: top;
}
[class^="icon-"] {
font-style: normal;
}
&.information {
@extend %t-copy-sub1;
background-color: $gray-l5;
color: $gray-d2;
}
&.warning {
&.validation {
background-color: $gray-d2;
padding: ($baseline/2) $baseline;
color: $white;
.icon-warning-sign {
margin-right: ($baseline/2);
color: $orange;
a {
color: $blue-l2;
}
.message-text {
display: inline-block;
width: 93%;
vertical-align: top;
&.has-warnings {
border-bottom: 3px solid $orange;
.icon-warning-sign {
margin-right: ($baseline/2);
color: $orange;
}
}
}
&.error {
background-color: $gray-d2;
padding: ($baseline/2) $baseline;
color: $white;
&.has-errors {
border-bottom: 3px solid $red-l2;
.icon-exclamation-sign {
margin-right: ($baseline/2);
color: $red-l2;
.icon-exclamation-sign {
margin-right: ($baseline/2);
color: $red-l2;
}
}
}
}
.xblock-message-list {
margin-bottom: 0;
}
.xblock-message-actions {
@extend %actions-header;
padding: ($baseline/2) $baseline;
background-color: $gray-d1;
}
}
......@@ -64,7 +64,7 @@
.no-container-content {
@extend %ui-well;
padding: ($baseline*2);
padding: $baseline;
background-color: $gray-l4;
text-align: center;
color: $gray;
......@@ -78,7 +78,7 @@
@extend %t-action4;
padding: 8px 20px 10px;
text-align: center;
margin-left: $baseline;
margin: ($baseline/2) 0 ($baseline/2) $baseline;
[class^="icon-"] {
margin-right: ($baseline/2);
......@@ -158,6 +158,8 @@ body.view-container .content-primary {
// CASE: page level xblock rendering
&.level-page {
margin: 0;
box-shadow: none;
border: 0;
.xblock-header {
display: none;
......@@ -166,15 +168,36 @@ body.view-container .content-primary {
.xblock-message {
border-radius: 3px 3px 0 0;
&.validation {
padding-top: ($baseline*.75);
}
.xblock-message-list {
margin: ($baseline/5) ($baseline*2.5);
list-style-type: disc;
color: $gray-l3;
}
.xblock-message-item {
padding-bottom: ($baseline/4);
}
&.information {
@extend %t-copy-base;
margin-bottom: $baseline;
border-bottom: 1px solid $gray-l4;
padding: ($baseline/2) ($baseline*.75);
padding: 0 0 ($baseline/2) 0;
background-color: $gray-l5;
color: $gray-d1;
}
}
.no-container-content {
.xblock-message-list {
margin: 0;
list-style-type: none;
color: $gray-d2;
}
}
}
// CASE: nesting level xblock rendering
......@@ -250,6 +273,34 @@ body.view-container .content-primary {
display: none;
}
}
.wrapper-xblock-message {
.xblock-message {
border-radius: 0 0 3px 3px;
.xblock-message-list {
margin: 0;
list-style-type: none;
}
&.information {
@extend %t-copy-sub2;
padding: 0 0 ($baseline/2) $baseline;
color: $gray-l1;
}
&.validation.has-warnings {
border: 0;
border-top: 3px solid $orange;
}
&.validation.has-errors {
border: 0;
border-top: 3px solid $red-l2;
}
}
}
}
}
......@@ -273,9 +324,9 @@ body.view-container .content-primary {
}
&.is-inactive {
margin: $baseline 0 0 0;
margin: ($baseline*1.5) 0 0 0;
border-top: 2px dotted $gray-l2;
padding: ($baseline/2) 0;
padding: ($baseline*.75) 0;
background-color: $gray-l4;
.wrapper-xblock.level-nesting {
......
......@@ -203,6 +203,16 @@ body.course.unit,
padding-top: 0;
color: $gray-l1;
}
&.has-warnings {
border: 0;
border-top: 3px solid $orange;
}
&.has-errors {
border: 0;
border-top: 3px solid $red-l2;
}
}
}
......
......@@ -55,11 +55,13 @@ class ValidationMessage(object):
"""
Represents a single validation message for an xblock.
"""
def __init__(self, xblock, message_text, message_type):
def __init__(self, xblock, message_text, message_type, action_class=None, action_label=None):
assert isinstance(message_text, unicode)
self.xblock = xblock
self.message_text = message_text
self.message_type = message_type
self.action_class = action_class
self.action_label = action_label
def __unicode__(self):
return self.message_text
......@@ -76,12 +78,15 @@ class SplitTestFields(object):
no_partition_selected = {'display_name': _("Not Selected"), 'value': -1}
@staticmethod
def build_partition_values(all_user_partitions):
def build_partition_values(all_user_partitions, selected_user_partition):
"""
This helper method builds up the user_partition values that will
be passed to the Studio editor
"""
SplitTestFields.user_partition_values = [SplitTestFields.no_partition_selected]
SplitTestFields.user_partition_values = []
# Add "No selection" value if there is not a valid selected user partition.
if not selected_user_partition:
SplitTestFields.user_partition_values.append(SplitTestFields.no_partition_selected)
for user_partition in all_user_partitions:
SplitTestFields.user_partition_values.append({"display_name": user_partition.name, "value": user_partition.id})
return SplitTestFields.user_partition_values
......@@ -122,7 +127,6 @@ class SplitTestFields(object):
scope=Scope.content
)
@XBlock.needs('user_tags') # pylint: disable=abstract-method
@XBlock.wants('partitions')
class SplitTestModule(SplitTestFields, XModule, StudioEditableModule):
......@@ -258,15 +262,12 @@ class SplitTestModule(SplitTestFields, XModule, StudioEditableModule):
"""
fragment = Fragment()
root_xblock = context.get('root_xblock')
is_configured = not self.user_partition_id == SplitTestFields.no_partition_selected['value']
is_root = root_xblock and root_xblock.location == self.location
active_groups_preview = None
inactive_groups_preview = None
# We don't show the "add missing groups" button on the unit page-- only when showing the container page.
is_missing_groups = False
if is_root:
user_partition = self.descriptor.get_selected_partition()
[active_children, inactive_children] = self.descriptor.active_and_inactive_children()
is_missing_groups = user_partition and len(active_children) < len(user_partition.groups)
active_groups_preview = self.studio_render_children(
fragment, active_children, context
)
......@@ -277,9 +278,9 @@ class SplitTestModule(SplitTestFields, XModule, StudioEditableModule):
fragment.add_content(self.system.render_template('split_test_author_view.html', {
'split_test': self,
'is_root': is_root,
'is_configured': is_configured,
'active_groups_preview': active_groups_preview,
'inactive_groups_preview': inactive_groups_preview,
'is_missing_groups': is_missing_groups
}))
fragment.add_javascript_url(self.runtime.local_resource_url(self, 'public/js/split_test_author_view.js'))
fragment.initialize_js('SplitTestAuthorView')
......@@ -430,7 +431,7 @@ class SplitTestDescriptor(SplitTestFields, SequenceDescriptor, StudioEditableDes
@property
def editable_metadata_fields(self):
# Update the list of partitions based on the currently available user_partitions.
SplitTestFields.build_partition_values(self.user_partitions)
SplitTestFields.build_partition_values(self.user_partitions, self.get_selected_partition())
editable_fields = super(SplitTestDescriptor, self).editable_metadata_fields
......@@ -508,15 +509,17 @@ class SplitTestDescriptor(SplitTestFields, SequenceDescriptor, StudioEditableDes
if self.user_partition_id < 0:
messages.append(ValidationMessage(
self,
_(u"You must select a group configuration for this content experiment."),
ValidationMessageType.warning
_(u"The experiment is not associated with a group configuration."),
ValidationMessageType.warning,
'edit-button',
_(u"Select a Group Configuration")
))
else:
user_partition = self.get_selected_partition()
if not user_partition:
messages.append(ValidationMessage(
self,
_(u"This content experiment will not be shown to students because it refers to a group configuration that has been deleted. You can delete this experiment or reinstate the group configuration to repair it."), \
_(u"The experiment uses a deleted group configuration. Select a valid group configuration or delete this experiment."),
ValidationMessageType.error
))
else:
......@@ -524,13 +527,15 @@ class SplitTestDescriptor(SplitTestFields, SequenceDescriptor, StudioEditableDes
if len(active_children) < len(user_partition.groups):
messages.append(ValidationMessage(
self,
_(u"This content experiment is missing groups that are defined in the current configuration. You can press the 'Create Missing Groups' button to create them."),
ValidationMessageType.error
_(u"The experiment does not contain all of the groups in the configuration."),
ValidationMessageType.error,
'add-missing-groups-button',
_(u"Add Missing Groups")
))
if len(inactive_children) > 0:
messages.append(ValidationMessage(
self,
_(u"This content experiment has children that are not associated with the selected group configuration. You can move content into an active group or delete it if it is unneeded."),
_(u"The experiment has an inactive group. Move content into active groups, then delete the inactive group."),
ValidationMessageType.warning
))
return messages
......@@ -543,16 +548,17 @@ class SplitTestDescriptor(SplitTestFields, SequenceDescriptor, StudioEditableDes
Called from Studio view.
"""
user_partition = self.get_selected_partition()
changed = False
for group in user_partition.groups:
str_group_id = unicode(group.id)
changed = False
if str_group_id not in self.group_id_to_child:
self._create_vertical_for_group(group)
changed = True
if changed:
# request does not have a user attribute, so pass None for user.
self.system.modulestore.update_item(self, None)
if changed:
# request does not have a user attribute, so pass None for user.
self.system.modulestore.update_item(self, None)
return Response()
def _create_vertical_for_group(self, group):
......
......@@ -180,8 +180,6 @@ class SplitTestModuleStudioTest(SplitTestModuleTest):
html = self.module_system.render(self.split_test_module, AUTHOR_VIEW, context).content
self.assertIn('HTML FOR GROUP 0', html)
self.assertIn('HTML FOR GROUP 1', html)
# Note that the mock xblock system doesn't render the template but the parameters instead
self.assertNotIn('\'is_missing_groups\': True', html)
# When rendering as a child, it shouldn't render either of its groups
context = create_studio_context(self.course_sequence)
......@@ -198,8 +196,6 @@ class SplitTestModuleStudioTest(SplitTestModuleTest):
html = self.module_system.render(self.split_test_module, AUTHOR_VIEW, context).content
self.assertIn('HTML FOR GROUP 0', html)
self.assertIn('HTML FOR GROUP 1', html)
# Note that the mock xblock system doesn't render the template but the parameters instead
self.assertIn('\'is_missing_groups\': True', html)
def test_editable_settings(self):
"""
......@@ -230,6 +226,7 @@ class SplitTestModuleStudioTest(SplitTestModuleTest):
self.assertEqual([], SplitTestDescriptor.user_partition_id.values)
# user_partitions is empty, only the "Not Selected" item will appear.
self.split_test_module.user_partition_id = SplitTestFields.no_partition_selected['value']
self.split_test_module.editable_metadata_fields # pylint: disable=pointless-statement
partitions = SplitTestDescriptor.user_partition_id.values
self.assertEqual(1, len(partitions))
......@@ -246,6 +243,23 @@ class SplitTestModuleStudioTest(SplitTestModuleTest):
self.assertEqual(0, partitions[1]['value'])
self.assertEqual("first_partition", partitions[1]['display_name'])
# Try again with a selected partition and verify that there is no option for "No Selection"
self.split_test_module.user_partition_id = 0
self.split_test_module.editable_metadata_fields # pylint: disable=pointless-statement
partitions = SplitTestDescriptor.user_partition_id.values
self.assertEqual(1, len(partitions))
self.assertEqual(0, partitions[0]['value'])
self.assertEqual("first_partition", partitions[0]['display_name'])
# Finally try again with an invalid selected partition and verify that "No Selection" is an option
self.split_test_module.user_partition_id = 999
self.split_test_module.editable_metadata_fields # pylint: disable=pointless-statement
partitions = SplitTestDescriptor.user_partition_id.values
self.assertEqual(2, len(partitions))
self.assertEqual(SplitTestFields.no_partition_selected['value'], partitions[0]['value'])
self.assertEqual(0, partitions[1]['value'])
self.assertEqual("first_partition", partitions[1]['display_name'])
def test_active_and_inactive_children(self):
"""
Tests the active and inactive children returned for different split test configurations.
......@@ -304,20 +318,27 @@ class SplitTestModuleStudioTest(SplitTestModuleTest):
"""
split_test_module = self.split_test_module
def verify_validation_message(message, expected_message, expected_message_type):
def verify_validation_message(message, expected_message, expected_message_type,
expected_action_class=None, expected_action_label=None):
"""
Verify that the validation message has the expected validation message and type.
"""
self.assertEqual(unicode(message), expected_message)
self.assertEqual(message.message_type, expected_message_type)
self.assertEqual(message.action_class, expected_action_class)
self.assertEqual(message.action_label, expected_action_label)
# Verify the messages for an unconfigured user partition
split_test_module.user_partition_id = -1
messages = split_test_module.validation_messages()
self.assertEqual(len(messages), 1)
verify_validation_message(messages[0],
u"You must select a group configuration for this content experiment.",
ValidationMessageType.warning)
verify_validation_message(
messages[0],
u"The experiment is not associated with a group configuration.",
ValidationMessageType.warning,
'edit-button',
u"Select a Group Configuration",
)
# Verify the messages for a correctly configured split_test
split_test_module.user_partition_id = 0
......@@ -334,11 +355,13 @@ class SplitTestModuleStudioTest(SplitTestModuleTest):
]
messages = split_test_module.validation_messages()
self.assertEqual(len(messages), 1)
verify_validation_message(messages[0],
u"This content experiment is missing groups that are defined in "
u"the current configuration. "
u"You can press the 'Create Missing Groups' button to create them.",
ValidationMessageType.error)
verify_validation_message(
messages[0],
u"The experiment does not contain all of the groups in the configuration.",
ValidationMessageType.error,
'add-missing-groups-button',
u"Add Missing Groups"
)
# Verify the messages for a split test with children that are not associated with any group
split_test_module.user_partitions = [
......@@ -347,11 +370,11 @@ class SplitTestModuleStudioTest(SplitTestModuleTest):
]
messages = split_test_module.validation_messages()
self.assertEqual(len(messages), 1)
verify_validation_message(messages[0],
u"This content experiment has children that are not associated with the "
u"selected group configuration. "
u"You can move content into an active group or delete it if it is unneeded.",
ValidationMessageType.warning)
verify_validation_message(
messages[0],
u"The experiment has an inactive group. Move content into active groups, then delete the inactive group.",
ValidationMessageType.warning
)
# Verify the messages for a split test with both missing and inactive children
split_test_module.user_partitions = [
......@@ -360,23 +383,26 @@ class SplitTestModuleStudioTest(SplitTestModuleTest):
]
messages = split_test_module.validation_messages()
self.assertEqual(len(messages), 2)
verify_validation_message(messages[0],
u"This content experiment is missing groups that are defined in "
u"the current configuration. "
u"You can press the 'Create Missing Groups' button to create them.",
ValidationMessageType.error)
verify_validation_message(messages[1],
u"This content experiment has children that are not associated with the "
u"selected group configuration. "
u"You can move content into an active group or delete it if it is unneeded.",
ValidationMessageType.warning)
verify_validation_message(
messages[0],
u"The experiment does not contain all of the groups in the configuration.",
ValidationMessageType.error,
'add-missing-groups-button',
u"Add Missing Groups"
)
verify_validation_message(
messages[1],
u"The experiment has an inactive group. Move content into active groups, then delete the inactive group.",
ValidationMessageType.warning
)
# Verify the messages for a split test referring to a non-existent user partition
split_test_module.user_partition_id = 2
messages = split_test_module.validation_messages()
self.assertEqual(len(messages), 1)
verify_validation_message(messages[0],
u"This content experiment will not be shown to students because it refers "
u"to a group configuration that has been deleted. "
u"You can delete this experiment or reinstate the group configuration to repair it.",
ValidationMessageType.error)
verify_validation_message(
messages[0],
u"The experiment uses a deleted group configuration. "
u"Select a valid group configuration or delete this experiment.",
ValidationMessageType.error
)
......@@ -35,15 +35,17 @@ XMODULE_METRIC_NAME = 'edxapp.xmodule'
# xblock view names
# This is the view that will be rendered to display the XBlock in the LMS.
# It will also be used to render the block in "preview" mode in Studio, unless
# the XBlock also implements author_view.
STUDENT_VIEW = 'student_view'
# An optional view of the xblock similar to student_view, but with possible inline
# An optional view of the XBlock similar to student_view, but with possible inline
# editing capabilities. This view differs from studio_view in that it should be as similar to student_view
# as possible. When previewing xblocks within Studio, Studio will prefer author_view to student_view.
# as possible. When previewing XBlocks within Studio, Studio will prefer author_view to student_view.
AUTHOR_VIEW = 'author_view'
# The view used to render an editor in Studio. The editor rendering can be completely different
# from the LMS student_view, and it is only shown with the author selects "Edit".
# from the LMS student_view, and it is only shown when the author selects "Edit".
STUDIO_VIEW = 'studio_view'
# Views that present a "preview" view of an xblock (as opposed to an editing view).
......
......@@ -40,8 +40,9 @@ Class Features
These are class attributes or functions that can be provided by an XBlock to customize behaviour
in the LMS.
* student_view (XBlock view): This is the view that will be rendered to display
the XBlock in the LMS.
* student_view (XBlock view): This is the view that will be rendered to display the XBlock
in the LMS. It will also be used to render the block in "preview" mode in Studio, unless
the XBlock also implements author_view.
* has_score (class property): True if this block should appear in the LMS progress page.
* get_progress (method): See documentation in x_module.py:XModuleMixin.get_progress.
* icon_class (class property): This can be one of (``other``, ``video``, or ``problem``), and
......@@ -78,10 +79,10 @@ Class Features
~~~~~~~~~~~~~~
* studio_view (XBlock.view): The view used to render an editor in Studio. The editor rendering can
be completely different from the LMS student_view, and it is only shown with the author selects "Edit".
* author_view (XBlock.view): An optional view of the xblock similar to student_view, but with possible inline
be completely different from the LMS student_view, and it is only shown when the author selects "Edit".
* author_view (XBlock.view): An optional view of the XBlock similar to student_view, but with possible inline
editing capabilities. This view differs from studio_view in that it should be as similar to student_view
as possible. When previewing xblocks within Studio, Studio will prefer author_view to student_view.
as possible. When previewing XBlocks within Studio, Studio will prefer author_view to student_view.
* non_editable_metadata_fields (property): A list of :class:`~xblock.fields.Field` objects that
shouldn't be displayed in the default editing view for Studio.
......
......@@ -7,83 +7,73 @@ user_partition = split_test.descriptor.get_selected_partition()
messages = split_test.descriptor.validation_messages()
%>
% if is_root and not user_partition:
% if is_root and not is_configured:
<div class="no-container-content">
% else:
<div class="wrapper-xblock-message">
% endif
% if not user_partition:
<div class="xblock-message error">
% if user_partition:
<div class="xblock-message information">
<p>
<i class="icon-exclamation-sign"></i>
${_("You must select a group configuration for this content experiment.")}
<a href="#" class="button edit-button action-button">
<i class="icon-pencil"></i> <span class="action-button-text">${_("Select a Group Configuration")}</span>
</a>
<span class="message-text">
${_("This content experiment uses group configuration '{experiment_name}'.").format(experiment_name=user_partition.name)}
</span>
</p>
</div>
% else:
% if is_root or len(messages) == 0:
<div class="xblock-message information">
<p>
<span class="message-text">
${_("This content experiment uses group configuration '{experiment_name}'.").format(experiment_name=user_partition.name)}
</span>
</p>
</div>
% endif
% if len(messages) > 0:
<%
def get_validation_icon(validation_type):
if validation_type == 'error':
return 'icon-exclamation-sign'
elif validation_type == 'warning':
return 'icon-warning-sign'
return None
error_messages = (message for message in messages if message.message_type==ValidationMessageType.error)
has_errors = next(error_messages, False)
aggregate_validation_class = 'has-errors' if has_errors else 'has-warnings'
aggregate_validation_type = 'error' if has_errors else 'warning'
%>
<div class="xblock-message validation ${aggregate_validation_class}">
% if is_configured:
<p class="${aggregate_validation_type}"><i class="${get_validation_icon(aggregate_validation_type)}"></i>
${_("This content experiment has issues that affect content visibility.")}
</p>
% endif
% if is_root:
<ul>
% if is_root or not is_configured:
<ul class="xblock-message-list">
% for message in messages:
<%
message_type = message.message_type
message_type_display_name = ValidationMessageType.display_name(message_type) if message_type else None
%>
<li class="xblock-message ${message_type}">
% if message_type == 'warning':
<i class="icon-warning-sign"></i>
% elif message_type == 'error':
<i class="icon-exclamation-sign"></i>
<li class="xblock-message-item ${message_type}">
% if not is_configured:
<i class="${get_validation_icon(message_type)}"></i>
% endif
<span class="message-text">
% if message_type_display_name:
<span class="sr">${message_type_display_name}:</span>
% endif
${unicode(message)}
% if message.action_class:
<a href="#" class="button action-button ${message.action_class}">
<span class="action-button-text">${message.action_label}</span>
</a>
% endif
</span>
</li>
% endfor
</ul>
% elif len(messages) > 0:
<%
error_messages = (message for message in messages if message.message_type==ValidationMessageType.error)
%>
% if next(error_messages, False):
<div class="xblock-message error">
<i class="icon-exclamation-sign"></i>
${_("This content experiment has errors that should be resolved.")}
</div>
% else:
<div class="xblock-message error">
<i class="icon-warning-sign"></i>
${_("This content experiment has warnings that might need to be investigated.")}
</div>
% endif
% endif
% if is_missing_groups:
<a href="#" class="button add-missing-groups-button action-button">
<span class="action-button-text">${_("Create Missing Groups")}</span>
</a>
% endif
% endif
% if is_root and not user_partition:
</div>
% else:
</div>
% endif
</div>
% if is_root:
<div class="wrapper-groups is-active">
<h3 class="sr">${_("Active Groups")}</h3>
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment