Move messages to review step.

problem_builder/mentoring.py

@@ -38,7 +38,9 @@ from .message import (
     MentoringMessageBlock, CompletedMentoringMessageShim, IncompleteMentoringMessageShim,
     OnReviewMentoringMessageShim
 )
-from .mixins import _normalize_id, StepParentMixin, QuestionMixin, XBlockWithTranslationServiceMixin
+from .mixins import (
+    _normalize_id, QuestionMixin, MessageParentMixin, StepParentMixin, XBlockWithTranslationServiceMixin
+)
 
 from xblockutils.helpers import child_isinstance
 from xblockutils.resources import ResourceLoader
@@ -80,7 +82,7 @@ PARTIAL = 'partial'
 @XBlock.needs("i18n")
 @XBlock.wants('settings')
 class BaseMentoringBlock(
-        XBlock, XBlockWithTranslationServiceMixin, StudioEditableXBlockMixin
+        XBlock, XBlockWithTranslationServiceMixin, StudioEditableXBlockMixin, MessageParentMixin
 ):
     """
     An XBlock that defines functionality shared by mentoring blocks.
@@ -133,20 +135,6 @@ class BaseMentoringBlock(
     def max_attempts_reached(self):
         return self.max_attempts > 0 and self.num_attempts >= self.max_attempts
 
-    def get_message_content(self, message_type, or_default=False):
-        for child_id in self.children:
-            if child_isinstance(self, child_id, MentoringMessageBlock):
-                child = self.runtime.get_block(child_id)
-                if child.type == message_type:
-                    content = child.content
-                    if hasattr(self.runtime, 'replace_jump_to_id_urls'):
-                        content = self.runtime.replace_jump_to_id_urls(content)
-                    return content
-        if or_default:
-            # Return the default value since no custom message is set.
-            # Note the WYSIWYG editor usually wraps the .content HTML in a <p> tag so we do the same here.
-            return '<p>{}</p>'.format(MentoringMessageBlock.MESSAGE_TYPES[message_type]['default'])
-
     def get_theme(self):
         """
         Gets theme settings from settings service. Falls back to default (LMS) theme
@@ -916,19 +904,6 @@ class MentoringWithExplicitStepsBlock(BaseMentoringBlock, StudioContainerWithNes
         return any(child_isinstance(self, child_id, ReviewStepBlock) for child_id in self.children)
 
     @property
-    def assessment_message(self):
-        """
-        Get the message to display to a student following a submission in assessment mode.
-        """
-        if self.max_attempts_reached:
-            return self.get_message_content('on-review', or_default=True)
-        else:
-            if self.complete:  # All answers correct
-                return self.get_message_content('completed', or_default=True)
-            else:
-                return self.get_message_content('incomplete', or_default=True)
-
-    @property
     def score(self):
         questions = self.questions
         total_child_weight = sum(float(question.weight) for question in questions)
@@ -951,7 +926,7 @@ class MentoringWithExplicitStepsBlock(BaseMentoringBlock, StudioContainerWithNes
 
     @property
     def complete(self):
-        return not any(step.answer_status == 'incorrect' for step in self.steps)
+        return not self.score.incorrect and not self.score.partially_correct
 
     @property
     def review_tips(self):
@@ -1023,9 +998,6 @@ class MentoringWithExplicitStepsBlock(BaseMentoringBlock, StudioContainerWithNes
         return [
             MentoringStepBlock,
             ReviewStepBlock,
-            NestedXBlockSpec(CompletedMentoringMessageShim, boilerplate='completed'),
-            NestedXBlockSpec(IncompleteMentoringMessageShim, boilerplate='incomplete'),
-            NestedXBlockSpec(OnReviewMentoringMessageShim, boilerplate='on-review'),
         ]
 
     @XBlock.json_handler
@@ -1058,7 +1030,8 @@ class MentoringWithExplicitStepsBlock(BaseMentoringBlock, StudioContainerWithNes
             'correct': self.correct_json(stringify=False),
             'incorrect': self.incorrect_json(stringify=False),
             'partial': self.partial_json(stringify=False),
-            'assessment_message': self.assessment_message,
+            'complete': self.complete,
+            'max_attempts_reached': self.max_attempts_reached,
             'assessment_review_tips': self.review_tips,
         }
 

problem_builder/mixins.py

@@ -93,6 +93,27 @@ class StepParentMixin(object):
         return [self.runtime.get_block(child_id) for child_id in self.step_ids]
 
 
+class MessageParentMixin(object):
+    """
+    An XBlock mixin for a parent block containing MentoringMessageBlock children
+    """
+
+    def get_message_content(self, message_type, or_default=False):
+        from problem_builder.message import MentoringMessageBlock  # Import here to avoid circular dependency
+        for child_id in self.children:
+            if child_isinstance(self, child_id, MentoringMessageBlock):
+                child = self.runtime.get_block(child_id)
+                if child.type == message_type:
+                    content = child.content
+                    if hasattr(self.runtime, 'replace_jump_to_id_urls'):
+                        content = self.runtime.replace_jump_to_id_urls(content)
+                    return content
+        if or_default:
+            # Return the default value since no custom message is set.
+            # Note the WYSIWYG editor usually wraps the .content HTML in a <p> tag so we do the same here.
+            return '<p>{}</p>'.format(MentoringMessageBlock.MESSAGE_TYPES[message_type]['default'])
+
+
 class QuestionMixin(EnumerableChildMixin):
     """
     An XBlock mixin for a child block that is a "Step".

problem_builder/public/css/problem-builder-edit.css

@@ -15,6 +15,7 @@
 }
 
 /* Custom appearance for our "Add" buttons */
+.xblock[data-block-type=sb-review-step] .add-xblock-component .new-component .new-component-type .add-xblock-component-button,
 .xblock[data-block-type=sb-step] .add-xblock-component .new-component .new-component-type .add-xblock-component-button,
 .xblock[data-block-type=step-builder] .add-xblock-component .new-component .new-component-type .add-xblock-component-button,
 .xblock[data-block-type=problem-builder] .add-xblock-component .new-component .new-component-type .add-xblock-component-button,
@@ -24,6 +25,8 @@
   line-height: 30px;
 }
 
+.xblock[data-block-type=sb-review-step] .add-xblock-component .new-component .new-component-type .add-xblock-component-button.disabled,
+.xblock[data-block-type=sb-review-step] .add-xblock-component .new-component .new-component-type .add-xblock-component-button.disabled:hover,
 .xblock[data-block-type=sb-step] .add-xblock-component .new-component .new-component-type .add-xblock-component-button.disabled,
 .xblock[data-block-type=sb-step] .add-xblock-component .new-component .new-component-type .add-xblock-component-button.disabled:hover,
 .xblock[data-block-type=step-builder] .add-xblock-component .new-component .new-component-type .add-xblock-component-button.disabled,
@@ -37,6 +40,7 @@
   cursor: default;
 }
 
+.xblock[data-block-type=sb-review-step] .submission-message-help p,
 .xblock[data-block-type=step-builder] .submission-message-help p,
 .xblock[data-block-type=problem-builder] .submission-message-help p {
 	border-top: 1px solid #ddd;

problem_builder/public/js/mentoring_with_steps.js

@@ -23,7 +23,7 @@ function MentoringWithStepsBlock(runtime, element) {
     var reviewTipsTemplate = _.template($('#xblock-review-tips-template').html()); // Tips about specific questions the user got wrong
     var attemptsTemplate = _.template($('#xblock-attempts-template').html());
     var checkmark, submitDOM, nextDOM, reviewDOM, tryAgainDOM,
-        assessmentMessageDOM, gradeDOM, attemptsDOM, reviewTipsDOM, reviewLinkDOM, submitXHR;
+        gradeDOM, attemptsDOM, reviewTipsDOM, reviewLinkDOM, submitXHR;
 
     function isLastStep() {
         return (activeStep === steps.length-1);
@@ -95,12 +95,15 @@ function MentoringWithStepsBlock(runtime, element) {
                 gradeDOM.data('correct', response.correct);
                 gradeDOM.data('incorrect', response.incorrect);
                 gradeDOM.data('partial', response.partial);
-                gradeDOM.data('assessment_message', response.assessment_message);
                 gradeDOM.data('assessment_review_tips', response.assessment_review_tips);
-                updateControls();
+                updateReviewStep(response);
             });
     }
 
+    function updateReviewStep(response) {
+        reviewStep.updateAssessmentMessage(response, updateControls);
+    }
+
     function updateControls() {
         submitDOM.attr('disabled', 'disabled');
 
@@ -159,8 +162,7 @@ function MentoringWithStepsBlock(runtime, element) {
         checkmark.removeClass('checkmark-partially-correct icon-ok fa-check');
         checkmark.removeClass('checkmark-incorrect icon-exclamation fa-exclamation');
         hideAllSteps();
-        assessmentMessageDOM.html('');
-        gradeDOM.html('');
+        hideReviewStep();
         attemptsDOM.html('');
         reviewTipsDOM.empty().hide();
     }
@@ -168,7 +170,6 @@ function MentoringWithStepsBlock(runtime, element) {
     function updateDisplay() {
         cleanAll();
         if (atReviewStep()) {
-            showAssessmentMessage();
             showReviewStep();
             showAttempts();
         } else {
@@ -182,13 +183,9 @@ function MentoringWithStepsBlock(runtime, element) {
         }
     }
 
-    function showAssessmentMessage() {
-        var data = gradeDOM.data();
-        assessmentMessageDOM.html(data.assessment_message);
-    }
-
     function showReviewStep() {
-        var data = gradeDOM.data();
+        // Forward to review step to show assessment message
+        reviewStep.showAssessmentMessage();
 
         // Forward to review step to render grade data
         var showExtendedFeedback = (!someAttemptsLeft() && extendedFeedbackEnabled());
@@ -202,6 +199,7 @@ function MentoringWithStepsBlock(runtime, element) {
             tryAgainDOM.removeAttr('disabled');
 
             // Review tips
+            var data = gradeDOM.data();
             if (data.assessment_review_tips.length > 0) {
                 // on-assessment-review-question messages specific to questions the student got wrong:
                 reviewTipsDOM.html(reviewTipsTemplate({
@@ -217,6 +215,11 @@ function MentoringWithStepsBlock(runtime, element) {
         tryAgainDOM.show();
     }
 
+    function hideReviewStep() {
+        reviewStep.hideAssessmentMessage();
+        gradeDOM.html('');
+    }
+
     function getStepToReview(event) {
         event.preventDefault();
         var stepIndex = parseInt($(event.target).data('step')) - 1;
@@ -314,7 +317,6 @@ function MentoringWithStepsBlock(runtime, element) {
 
     function showGrade() {
         cleanAll();
-        showAssessmentMessage();
         showReviewStep();
         showAttempts();
 
@@ -394,7 +396,6 @@ function MentoringWithStepsBlock(runtime, element) {
         tryAgainDOM = $(element).find('.submit .input-try-again');
         tryAgainDOM.on('click', tryAgain);
 
-        assessmentMessageDOM = $('.assessment-message', element);
         gradeDOM = $('.grade', element);
         attemptsDOM = $('.attempts', element);
         reviewTipsDOM = $('.assessment-review-tips', element);

problem_builder/public/js/mentoring_with_steps_edit.js

@@ -21,17 +21,11 @@ function MentoringWithStepsEdit(runtime, element) {
     var initButtons = function(dataCategory) {
         var $buttons = $('.add-xblock-component-button[data-category='+dataCategory+']', element);
         $buttons.each(function() {
-            if (dataCategory === 'pb-message') {
-                var msg_type = $(this).data('boilerplate');
-                updateButton($(this), blockIsPresent('.submission-message.'+msg_type));
-            } else {
-                updateButton($(this), blockIsPresent('.xblock-header-sb-review-step'));
-            }
+            updateButton($(this), blockIsPresent('.xblock-header-sb-review-step'));
         });
         $buttons.on('click', disableButton);
     };
 
-    initButtons('pb-message');
     initButtons('sb-review-step');
 
     ProblemBuilderUtil.transformClarifications(element);

problem_builder/public/js/review_step.js

@@ -3,8 +3,29 @@ function ReviewStepBlock(runtime, element) {
     var gradeTemplate = _.template($('#xblock-feedback-template').html());
     var reviewStepsTemplate = _.template($('#xblock-step-links-template').html());
 
+    var assessmentMessageDOM = $('.assessment-message', element);
+
     return {
 
+        'showAssessmentMessage': function() {
+            var assessmentMessage = assessmentMessageDOM.data('assessment_message');
+            assessmentMessageDOM.html(assessmentMessage);
+            assessmentMessageDOM.show();
+        },
+
+        'hideAssessmentMessage': function() {
+            assessmentMessageDOM.html('');
+            assessmentMessageDOM.hide();
+        },
+
+        'updateAssessmentMessage': function(grade, callback) {
+            var handlerUrl = runtime.handlerUrl(element, 'get_assessment_message');
+            $.post(handlerUrl, JSON.stringify(grade)).success(function(response) {
+                assessmentMessageDOM.data('assessment_message', response.assessment_message);
+                callback();
+            });
+        },
+
         'renderGrade': function(gradeDOM, showExtendedFeedback) {
 
             var data = gradeDOM.data();

problem_builder/public/js/review_step_edit.js

+function ReviewStepEdit(runtime, element) {
+    "use strict";
+
+    var blockIsPresent = function(klass) {
+        return $('.xblock ' + klass).length > 0;
+    };
+
+    var updateButton = function(button, condition) {
+        button.toggleClass('disabled', condition);
+    };
+
+    var disableButton = function(ev) {
+        if ($(this).is('.disabled')) {
+            ev.preventDefault();
+            ev.stopPropagation();
+        } else {
+            $(this).addClass('disabled');
+        }
+    };
+
+    var initButtons = function(dataCategory) {
+        var $buttons = $('.add-xblock-component-button[data-category='+dataCategory+']', element);
+        $buttons.each(function() {
+            var msg_type = $(this).data('boilerplate');
+            updateButton($(this), blockIsPresent('.submission-message.'+msg_type));
+        });
+        $buttons.on('click', disableButton);
+    };
+
+    initButtons('pb-message');
+
+    ProblemBuilderUtil.transformClarifications(element);
+    StudioEditableXBlockMixin(runtime, element);
+}

problem_builder/step.py

@@ -32,7 +32,10 @@ from xblockutils.studio_editable import (
 
 from problem_builder.answer import AnswerBlock, AnswerRecapBlock
 from problem_builder.mcq import MCQBlock, RatingBlock
-from problem_builder.mixins import EnumerableChildMixin, StepParentMixin
+from .message import (
+    CompletedMentoringMessageShim, IncompleteMentoringMessageShim, OnReviewMentoringMessageShim
+)
+from problem_builder.mixins import EnumerableChildMixin, MessageParentMixin, StepParentMixin
 from problem_builder.mrq import MRQBlock
 from problem_builder.table import MentoringTableBlock
 
@@ -231,7 +234,7 @@ class MentoringStepBlock(
         return fragment
 
 
-class ReviewStepBlock(XBlockWithPreviewMixin, XBlock):
+class ReviewStepBlock(MessageParentMixin, StudioContainerWithNestedXBlocksMixin, XBlockWithPreviewMixin, XBlock):
     """ A dedicated step for reviewing results for a mentoring block """
     CATEGORY = 'sb-review-step'
     STUDIO_LABEL = _("Review Step")
@@ -240,6 +243,45 @@ class ReviewStepBlock(XBlockWithPreviewMixin, XBlock):
         default="Review Step"
     )
 
+    @property
+    def allowed_nested_blocks(self):
+        """
+        Returns a list of allowed nested XBlocks. Each item can be either
+        * An XBlock class
+        * A NestedXBlockSpec
+
+        If XBlock class is used it is assumed that this XBlock is enabled and allows multiple instances.
+        NestedXBlockSpec allows explicitly setting disabled/enabled state,
+        disabled reason (if any) and single/multiple instances.
+        """
+        return [
+            NestedXBlockSpec(CompletedMentoringMessageShim, boilerplate='completed'),
+            NestedXBlockSpec(IncompleteMentoringMessageShim, boilerplate='incomplete'),
+            NestedXBlockSpec(OnReviewMentoringMessageShim, boilerplate='on-review'),
+        ]
+
+    @XBlock.json_handler
+    def get_assessment_message(self, grade, suffix):
+        complete = grade['complete']
+        max_attempts_reached = grade['max_attempts_reached']
+        return {
+            'assessment_message': self.assessment_message(complete, max_attempts_reached)
+        }
+
+    def assessment_message(self, complete=None, max_attempts_reached=None):
+        if complete is None and max_attempts_reached is None:
+            parent = self.get_parent()
+            complete = parent.complete
+            max_attempts_reached = parent.max_attempts_reached
+        if max_attempts_reached:
+            assessment_message = self.get_message_content('on-review', or_default=True)
+        else:
+            if complete:  # All answers correct
+                assessment_message = self.get_message_content('completed', or_default=True)
+            else:
+                assessment_message = self.get_message_content('incomplete', or_default=True)
+        return assessment_message
+
     def mentoring_view(self, context=None):
         """ Mentoring View """
         return self._render_view(context)
@@ -260,3 +302,25 @@ class ReviewStepBlock(XBlockWithPreviewMixin, XBlock):
         fragment.add_javascript_url(self.runtime.local_resource_url(self, 'public/js/review_step.js'))
         fragment.initialize_js('ReviewStepBlock')
         return fragment
+
+    def author_preview_view(self, context):
+        return Fragment(
+            u"<p>{}</p>".format(
+                _(u"This block summarizes a student's performance on the parent Step Builder block.")
+            )
+        )
+
+    def author_edit_view(self, context):
+        """
+        Add some HTML to the author view that allows authors to add child blocks.
+        """
+        context['wrap_children'] = {
+            'head': u'<div class="mentoring">',
+            'tail': u'</div>'
+        }
+        fragment = super(ReviewStepBlock, self).author_edit_view(context)
+        fragment.add_css_url(self.runtime.local_resource_url(self, 'public/css/problem-builder-edit.css'))
+        fragment.add_javascript_url(self.runtime.local_resource_url(self, 'public/js/util.js'))
+        fragment.add_javascript_url(self.runtime.local_resource_url(self, 'public/js/review_step_edit.js'))
+        fragment.initialize_js('ReviewStepEdit')
+        return fragment

problem_builder/templates/html/mentoring_with_steps.html

@@ -9,14 +9,11 @@
 
   <div class="assessment-question-block">
 
-    <div class="assessment-message"></div>
-
     {% for child_content in children_contents %}
       {{ child_content|safe }}
     {% endfor %}
 
     <div class="grade"
-         data-assessment_message="{{ self.assessment_message }}"
          data-score="{{ self.score.percentage }}"
          data-correct_answer="{{ self.score.correct|length }}"
          data-incorrect_answer="{{ self.score.incorrect|length }}"

problem_builder/templates/html/review_step.html

 <div class="sb-review-step">
 
+  <div class="assessment-message" data-assessment_message="{{ self.assessment_message }}"></div>
+
   <script type="text/template" id="xblock-feedback-template">
     <div class="grade-result">
       <h2>

problem_builder/tests/integration/xml_templates/step_builder.xml

@@ -54,18 +54,18 @@
     </pb-mrq>
   </sb-step>
 
-  <sb-review-step></sb-review-step>
+  <sb-review-step>
+    <pb-message type="completed">
+      <html>Block completed message text</html>
+    </pb-message>
 
-  <pb-message type="completed">
-    <html>Block completed message text</html>
-  </pb-message>
+    <pb-message type="incomplete">
+      <html>Block incomplete message text</html>
+    </pb-message>
 
-  <pb-message type="incomplete">
-    <html>Block incomplete message text</html>
-  </pb-message>
-
-  <pb-message type="on-review">
-    <html>On review message text</html>
-  </pb-message>
+    <pb-message type="on-review">
+      <html>On review message text</html>
+    </pb-message>
+  </sb-review-step>
 
 </step-builder>

problem_builder/tests/unit/test_mentoring.py

@@ -154,7 +154,7 @@ class TestMentoringBlockJumpToIds(unittest.TestCase):
         self.block.runtime.replace_jump_to_id_urls = lambda x: x.replace('test', 'replaced-url')
 
     def test_get_message_content(self):
-        with patch('problem_builder.mentoring.child_isinstance') as mock_child_isinstance:
+        with patch('problem_builder.mixins.child_isinstance') as mock_child_isinstance:
             mock_child_isinstance.return_value = True
             self.runtime_mock.get_block = Mock()
             self.runtime_mock.get_block.return_value = self.message_block