Merge pull request #66 from open-craft/review-step-messages

Step Builder: Add mentoring-level messages to review step and disable mode switching for Problem Builder

doc/Usage.md

@@ -59,10 +59,23 @@ Step Builder will display one step at a time. All questions belonging
 to a step need to be completed before the step can be submitted.
 
 In addition to regular steps, Step Builder also provides a **Review
-Step** block which allows students to review their performance, and to
-jump back to individual steps to review their answers (if **Extended
-feedback** setting is on and maximum number of attempts has been
-reached). Note that only one such block is allowed per instance.
+Step** block which
+
+* allows students to review their performance
+
+* allows students to jump back to individual steps to review their
+  answers (if **Extended feedback** setting is on and maximum number
+  of attempts has been reached)
+
+* supports customizable messages that will be shown when
+
+  * the block is *complete*, i.e., if all answers that the student
+    provided are correct
+  * the block is *incomplete*, i.e., if some answers that the student
+    provided are incorrect or partially correct
+  * the student has used up all attempts
+
+Note that only one such block is allowed per instance.
 
 **Screenshots: Step**
 
@@ -80,10 +93,11 @@ partially correct).
 
 **Screenshots: Review Step**
 
-Unlimited attempts available:
+Unlimited attempts available, all answers correct:
 
 ![Unlimited attempts available](img/review-step-unlimited-attempts-available.png)
-Limited attempts, some attempts remaining:
+
+Limited attempts, some attempts remaining, some answers incorrect:
 
 ![Some attempts remaining](img/review-step-some-attempts-remaining.png)
 

problem_builder/mentoring.py

@@ -34,16 +34,16 @@ from xblock.fields import Boolean, Scope, String, Integer, Float, List
 from xblock.fragment import Fragment
 from xblock.validation import ValidationMessage
 
-from .message import (
-    MentoringMessageBlock, CompletedMentoringMessageShim, IncompleteMentoringMessageShim,
-    MaxAttemptsReachedMentoringMessageShim, OnAssessmentReviewMentoringMessageShim
+from .message import MentoringMessageBlock
+
+from .mixins import (
+    _normalize_id, QuestionMixin, MessageParentMixin, StepParentMixin, XBlockWithTranslationServiceMixin
 )
-from .mixins import _normalize_id, StepParentMixin, QuestionMixin, XBlockWithTranslationServiceMixin
 
 from xblockutils.helpers import child_isinstance
 from xblockutils.resources import ResourceLoader
 from xblockutils.studio_editable import (
-    NestedXBlockSpec, StudioEditableXBlockMixin, StudioContainerXBlockMixin, StudioContainerWithNestedXBlocksMixin
+    StudioEditableXBlockMixin, StudioContainerXBlockMixin, StudioContainerWithNestedXBlocksMixin
 )
 
 
@@ -80,7 +80,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,19 +133,20 @@ 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_content_titles(self):
+        """
+        By default, each Sequential block in a course ("Subsection" in Studio parlance) will
+        display the display_name of each descendant in a tooltip above the content. We don't
+        want that - we only want to display one title for this mentoring block as a whole.
+        Otherwise things like "Choice (yes) (Correct)" will appear in the tooltip.
+
+        If this block has no title set, don't display any title. Then, if this is the only block
+        in the unit, the unit's title will be used. (Why isn't it always just used?)
+        """
+        has_explicitly_set_title = self.fields['display_name'].is_set_on(self)
+        if has_explicitly_set_title:
+            return [self.display_name]
+        return []
 
     def get_theme(self):
         """
@@ -318,7 +319,7 @@ class MentoringBlock(BaseMentoringBlock, StudioContainerXBlockMixin, StepParentM
     )
 
     editable_fields = (
-        'display_name', 'mode', 'followed_by', 'max_attempts', 'enforce_dependency',
+        'display_name', 'followed_by', 'max_attempts', 'enforce_dependency',
         'display_submit', 'feedback_label', 'weight', 'extended_feedback'
     )
 
@@ -807,21 +808,6 @@ class MentoringBlock(BaseMentoringBlock, StudioContainerXBlockMixin, StepParentM
         fragment.initialize_js('MentoringEditComponents')
         return fragment
 
-    def get_content_titles(self):
-        """
-        By default, each Sequential block in a course ("Subsection" in Studio parlance) will
-        display the display_name of each descendant in a tooltip above the content. We don't
-        want that - we only want to display one title for this mentoring block as a whole.
-        Otherwise things like "Choice (yes) (Correct)" will appear in the tooltip.
-
-        If this block has no title set, don't display any title. Then, if this is the only block
-        in the unit, the unit's title will be used. (Why isn't it always just used?)
-        """
-        has_explicitly_set_title = self.fields['display_name'].is_set_on(self)
-        if has_explicitly_set_title:
-            return [self.display_name]
-        return []
-
     @staticmethod
     def workbench_scenarios():
         """
@@ -916,17 +902,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 not self.max_attempts_reached:
-            return self.get_message_content('on-assessment-review', or_default=True)
-        else:
-            assessment_message = _("Note: you have used all attempts. Continue to the next unit.")
-            return '<p>{}</p>'.format(assessment_message)
-
-    @property
     def score(self):
         questions = self.questions
         total_child_weight = sum(float(question.weight) for question in questions)
@@ -948,6 +923,10 @@ class MentoringWithExplicitStepsBlock(BaseMentoringBlock, StudioContainerWithNes
         return Score(score, int(round(score * 100)), correct, incorrect, partially_correct)
 
     @property
+    def complete(self):
+        return not self.score.incorrect and not self.score.partially_correct
+
+    @property
     def review_tips(self):
         """ Get review tips, shown for wrong answers. """
         review_tips = []
@@ -1017,7 +996,6 @@ class MentoringWithExplicitStepsBlock(BaseMentoringBlock, StudioContainerWithNes
         return [
             MentoringStepBlock,
             ReviewStepBlock,
-            NestedXBlockSpec(OnAssessmentReviewMentoringMessageShim, boilerplate='on-assessment-review'),
         ]
 
     @XBlock.json_handler
@@ -1040,6 +1018,16 @@ class MentoringWithExplicitStepsBlock(BaseMentoringBlock, StudioContainerWithNes
         }
 
     @XBlock.json_handler
+    def publish_attempt(self, data, suffix):
+        score = self.score
+        grade_data = {
+            'value': score.raw,
+            'max_value': 1,
+        }
+        self.runtime.publish(self, 'grade', grade_data)
+        return {}
+
+    @XBlock.json_handler
     def get_grade(self, data, suffix):
         score = self.score
         return {
@@ -1050,7 +1038,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/message.py

@@ -48,9 +48,8 @@ class MentoringMessageBlock(XBlock, StudioEditableXBlockMixin, XBlockWithTransla
             "long_display_name": _(u"Message shown when complete"),
             "default": _(u"Great job!"),
             "description": _(
-                u"In standard mode, this message will be shown when the student achieves a "
-                "perfect score. "
-                "This message is ignored in assessment mode."
+                u"This message will be shown when the student achieves a perfect score. "
+                "Note that it is ignored in Problem Builder blocks using the legacy assessment mode."
             ),
         },
         "incomplete": {
@@ -58,9 +57,9 @@ class MentoringMessageBlock(XBlock, StudioEditableXBlockMixin, XBlockWithTransla
             "long_display_name": _(u"Message shown when incomplete"),
             "default": _(u"Not quite! You can try again, though."),
             "description": _(
-                u"In standard mode, this message will be shown when the student gets at least "
-                "one question wrong, but is allowed to try again. "
-                "This message is ignored in assessment mode."
+                u"This message will be shown when the student gets at least one question wrong, "
+                "but is allowed to try again. "
+                "Note that it is ignored in Problem Builder blocks using the legacy assessment mode."
             ),
         },
         "max_attempts_reached": {
@@ -68,9 +67,9 @@ class MentoringMessageBlock(XBlock, StudioEditableXBlockMixin, XBlockWithTransla
             "long_display_name": _(u"Message shown when student reaches max. # of attempts"),
             "default": _(u"Sorry, you have used up all of your allowed submissions."),
             "description": _(
-                u"In standard mode, this message will be shown when the student has used up "
+                u"This message will be shown when the student has used up "
                 "all of their allowed attempts without achieving a perfect score. "
-                "This message is ignored in assessment mode."
+                "Note that it is ignored in Problem Builder blocks using the legacy assessment mode."
             ),
         },
         "on-assessment-review": {
@@ -101,6 +100,18 @@ class MentoringMessageBlock(XBlock, StudioEditableXBlockMixin, XBlockWithTransla
                 "used up all of their allowed attempts."
             ),
         },
+        "on-review": {
+            "display_name": _(u"Message shown when no attempts left"),
+            "long_display_name": _(u"Message shown during review when no attempts remain"),
+            "default": _(
+                u"Note: you have used all attempts. Continue to the next unit."
+            ),
+            "description": _(
+                u"This message will be shown when the student is reviewing their answers to the assessment, "
+                "if the student has used up all of their allowed attempts. "
+                "It is not shown if the student is allowed to try again."
+            ),
+        },
     }
 
     content = String(
@@ -194,11 +205,6 @@ class IncompleteMentoringMessageShim(object):
     STUDIO_LABEL = _("Message (Incomplete)")
 
 
-class MaxAttemptsReachedMentoringMessageShim(object):
-    CATEGORY = 'pb-message'
-    STUDIO_LABEL = _("Message (Max # Attempts)")
-
-
-class OnAssessmentReviewMentoringMessageShim(object):
+class OnReviewMentoringMessageShim(object):
     CATEGORY = 'pb-message'
-    STUDIO_LABEL = _("Message (Assessment Review)")
+    STUDIO_LABEL = _("Message (Review)")

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);
@@ -79,7 +79,15 @@ function MentoringWithStepsBlock(runtime, element) {
         $.post(handlerUrl, JSON.stringify({}))
             .success(function(response) {
                 attemptsDOM.data('num_attempts', response.num_attempts);
-                // Now that relevant info is up-to-date, get the latest grade
+                publishAttempt();
+            });
+    }
+
+    function publishAttempt() {
+        var handlerUrl = runtime.handlerUrl(element, 'publish_attempt');
+        $.post(handlerUrl, JSON.stringify({}))
+            .success(function(response) {
+                // Now that relevant info is up-to-date and attempt has been published, get the latest grade
                 updateGrade();
             });
     }
@@ -95,12 +103,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 +170,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 +178,6 @@ function MentoringWithStepsBlock(runtime, element) {
     function updateDisplay() {
         cleanAll();
         if (atReviewStep()) {
-            showAssessmentMessage();
             showReviewStep();
             showAttempts();
         } else {
@@ -182,13 +191,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 +207,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 +223,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 +325,6 @@ function MentoringWithStepsBlock(runtime, element) {
 
     function showGrade() {
         cleanAll();
-        showAssessmentMessage();
         showReviewStep();
         showAttempts();
 
@@ -394,7 +404,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,46 @@ 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):
+        # Data passed as "grade" comes from "get_grade" handler of Step Builder (MentoringWithExplicitStepsBlock)
+        complete = grade.get('complete')
+        max_attempts_reached = grade.get('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 +303,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/test_step_builder.py

-from .base_test import CORRECT, INCORRECT, PARTIAL, MentoringAssessmentBaseTest, GetChoices
-
+from mock import patch
 from ddt import ddt, data
 
+from workbench.runtime import WorkbenchRuntime
+from .base_test import CORRECT, INCORRECT, PARTIAL, MentoringAssessmentBaseTest, GetChoices
+
 
 @ddt
 class StepBuilderTest(MentoringAssessmentBaseTest):
@@ -108,7 +110,7 @@ class StepBuilderTest(MentoringAssessmentBaseTest):
 
         # Check grade breakdown
         if expected["correct"] == 1:
-            self.assertIn("You answered 1 questions correctly.".format(**expected), step_builder.text)
+            self.assertIn("You answered 1 question correctly.".format(**expected), step_builder.text)
         else:
             self.assertIn("You answered {correct} questions correctly.".format(**expected), step_builder.text)
 
@@ -238,7 +240,23 @@ class StepBuilderTest(MentoringAssessmentBaseTest):
 
         # Last step
         # Submit MRQ, go to review
-        self.multiple_response_question(None, step_builder, controls, ("Its beauty",), PARTIAL, last=True)
+        with patch.object(WorkbenchRuntime, 'publish') as patched_method:
+            self.multiple_response_question(None, step_builder, controls, ("Its beauty",), PARTIAL, last=True)
+
+            # Check if "grade" event was published
+            # Note that we can't use patched_method.assert_called_once_with here
+            # because there is no way to obtain a reference to the block instance generated from the XML scenario
+            self.assertTrue(patched_method.called)
+            self.assertEquals(len(patched_method.call_args_list), 1)
+
+            block_object = self.load_root_xblock()
+
+            positional_args = patched_method.call_args[0]
+            block, event, data = positional_args
+
+            self.assertEquals(block.scope_ids.usage_id, block_object.scope_ids.usage_id)
+            self.assertEquals(event, 'grade')
+            self.assertEquals(data, {'value': 0.625, 'max_value': 1})
 
         # Review step
         expected_results = {
@@ -248,11 +266,11 @@ class StepBuilderTest(MentoringAssessmentBaseTest):
         self.peek_at_review(step_builder, controls, expected_results, extended_feedback=extended_feedback)
 
         if max_attempts == 1:
-            self.assert_message_text(step_builder, "Note: you have used all attempts. Continue to the next unit.")
+            self.assert_message_text(step_builder, "On review message text")
             self.assert_disabled(controls.try_again)
             return
 
-        self.assert_message_text(step_builder, "Assessment additional feedback message text")
+        self.assert_message_text(step_builder, "Block incomplete message text")
         self.assert_clickable(controls.try_again)
 
         # Try again
@@ -261,15 +279,24 @@ class StepBuilderTest(MentoringAssessmentBaseTest):
         self.wait_until_hidden(controls.try_again)
         self.assert_no_message_text(step_builder)
 
+        # Step 1
+        # Submit free-form answer, go to next step
         self.freeform_answer(
             None, step_builder, controls, 'This is a different answer', CORRECT, saved_value='This is the answer'
         )
+        # Step 2
+        # Submit MCQ, go to next step
         self.single_choice_question(None, step_builder, controls, 'Yes', CORRECT)
+        # Step 3
+        # Submit rating, go to next step
         self.rating_question(None, step_builder, controls, "1 - Not good at all", INCORRECT)
 
+        # Last step
+        # Submit MRQ, go to review
         user_selection = ("Its elegance", "Its beauty", "Its gracefulness")
         self.multiple_response_question(None, step_builder, controls, user_selection, CORRECT, last=True)
 
+        # Review step
         expected_results = {
             "correct": 3, "partial": 0, "incorrect": 1, "percentage": 75,
             "num_attempts": 2, "max_attempts": max_attempts
@@ -282,9 +309,9 @@ class StepBuilderTest(MentoringAssessmentBaseTest):
             self.assert_clickable(controls.try_again)
 
         if 1 <= max_attempts <= 2:
-            self.assert_message_text(step_builder, "Note: you have used all attempts. Continue to the next unit.")
+            self.assert_message_text(step_builder, "On review message text")
         else:
-            self.assert_message_text(step_builder, "Assessment additional feedback message text")
+            self.assert_message_text(step_builder, "Block incomplete message text")
 
         if extended_feedback:
             self.extended_feedback_checks(step_builder, controls, expected_results)
@@ -311,8 +338,8 @@ class StepBuilderTest(MentoringAssessmentBaseTest):
         self.assertIn('Lesson 1', review_tips.text)
         self.assertNotIn('Lesson 2', review_tips.text)  # This MCQ was correct
         self.assertIn('Lesson 3', review_tips.text)
-        # The on-assessment-review message is also shown if attempts remain:
-        self.assert_message_text(step_builder, "Assessment additional feedback message text")
+        # If attempts remain and student got some answers wrong, show "incomplete" message
+        self.assert_message_text(step_builder, "Block incomplete message text")
 
         # Try again
         self.assert_clickable(controls.try_again)
@@ -327,7 +354,8 @@ class StepBuilderTest(MentoringAssessmentBaseTest):
         user_selection = ("Its elegance", "Its beauty", "Its gracefulness")
         self.multiple_response_question(None, step_builder, controls, user_selection, CORRECT, last=True)
 
-        self.assert_message_text(step_builder, "Assessment additional feedback message text")
+        # If attempts remain and student got all answers right, show "complete" message
+        self.assert_message_text(step_builder, "Block completed message text")
         self.assertFalse(review_tips.is_displayed())
 
         # Try again
@@ -344,3 +372,87 @@ class StepBuilderTest(MentoringAssessmentBaseTest):
 
         # The review tips will not be shown because no attempts remain:
         self.assertFalse(review_tips.is_displayed())
+
+    def test_default_messages(self):
+        max_attempts = 3
+        extended_feedback = False
+        params = {
+            "max_attempts": max_attempts,
+            "extended_feedback": extended_feedback,
+        }
+        step_builder, controls = self.load_assessment_scenario("step_builder_default_messages.xml", params)
+
+        # First attempt: incomplete (second question wrong)
+
+        # Step 1
+        # Submit free-form answer, go to next step
+        self.freeform_answer(None, step_builder, controls, 'This is the answer', CORRECT)
+
+        # Step 2
+        # Submit MCQ, go to next step
+        self.single_choice_question(None, step_builder, controls, 'Maybe not', INCORRECT, last=True)
+
+        # Review step
+        expected_results = {
+            "correct": 1, "partial": 0, "incorrect": 1, "percentage": 50,
+            "num_attempts": 1, "max_attempts": max_attempts
+        }
+        self.peek_at_review(step_builder, controls, expected_results, extended_feedback=extended_feedback)
+
+        # Should show default message for incomplete submission
+        self.assert_message_text(step_builder, "Not quite! You can try again, though.")
+
+        # Try again
+        controls.try_again.click()
+
+        self.wait_until_hidden(controls.try_again)
+        self.assert_no_message_text(step_builder)
+
+        # Second attempt: complete (both questions correct)
+
+        # Step 1
+        # Submit free-form answer, go to next step
+        self.freeform_answer(
+            None, step_builder, controls, 'This is a different answer', CORRECT, saved_value='This is the answer'
+        )
+        # Step 2
+        # Submit MCQ, go to next step
+        self.single_choice_question(None, step_builder, controls, 'Yes', CORRECT, last=True)
+
+        # Review step
+        expected_results = {
+            "correct": 2, "partial": 0, "incorrect": 0, "percentage": 100,
+            "num_attempts": 2, "max_attempts": max_attempts
+        }
+        self.peek_at_review(step_builder, controls, expected_results, extended_feedback=extended_feedback)
+
+        # Should show default message for complete submission
+        self.assert_message_text(step_builder, "Great job!")
+
+        # Try again
+        controls.try_again.click()
+
+        self.wait_until_hidden(controls.try_again)
+        self.assert_no_message_text(step_builder)
+
+        # Last attempt: complete (both questions correct)
+
+        # Step 1
+        # Submit free-form answer, go to next step
+        self.freeform_answer(
+            None, step_builder, controls, 'This is yet another answer', CORRECT,
+            saved_value='This is a different answer'
+        )
+        # Step 2
+        # Submit MCQ, go to next step
+        self.single_choice_question(None, step_builder, controls, 'Yes', CORRECT, last=True)
+
+        # Review step
+        expected_results = {
+            "correct": 2, "partial": 0, "incorrect": 0, "percentage": 100,
+            "num_attempts": 3, "max_attempts": max_attempts
+        }
+        self.peek_at_review(step_builder, controls, expected_results, extended_feedback=extended_feedback)
+
+        # Should show default message for review
+        self.assert_message_text(step_builder, "Note: you have used all attempts. Continue to the next unit.")

problem_builder/tests/integration/xml_templates/step_builder.xml

@@ -54,10 +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="on-assessment-review">
-    <html>Assessment additional feedback 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>
+  </sb-review-step>
 
 </step-builder>

problem_builder/tests/integration/xml_templates/step_builder_default_messages.xml

+<step-builder url_name="step-builder" display_name="Step Builder"
+              max_attempts="{{max_attempts}}" extended_feedback="{{extended_feedback}}">
+
+  <sb-step display_name="First step">
+    <pb-answer name="goal" question="What is your goal?" />
+  </sb-step>
+
+  <sb-step display_name="Second step">
+    <pb-mcq name="mcq_1_1" question="Do you like this MCQ?" correct_choices='["yes"]'>
+        <pb-choice value="yes">Yes</pb-choice>
+        <pb-choice value="maybenot">Maybe not</pb-choice>
+        <pb-choice value="understand">I don't understand</pb-choice>
+    </pb-mcq>
+  </sb-step>
+
+  <sb-review-step></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