Fix various bugs found in QA

problem_builder/mcq.py

@@ -99,7 +99,7 @@ def calculate_results(self, submission):
 
         return {
             'submission': submission,
-            'message': self.message,
+            'message': self.message_formatted,
             'status': 'correct' if correct else 'incorrect',
             'tips': formatted_tips,
             'weight': self.weight,

problem_builder/mentoring.py

@@ -99,6 +99,13 @@ class BaseMentoringBlock(
         scope=Scope.content,
         enforce_type=True
     )
+    weight = Float(
+        display_name=_("Weight"),
+        help=_("Defines the maximum total grade of the block."),
+        default=1,
+        scope=Scope.settings,
+        enforce_type=True
+    )
 
     # User state
     num_attempts = Integer(
@@ -109,6 +116,7 @@ class BaseMentoringBlock(
     )
 
     has_children = True
+    has_score = True  # The Problem/Step Builder XBlocks produce scores. (Their children do not send scores to the LMS.)
 
     icon_class = 'problem'
     block_settings_key = 'mentoring'
@@ -197,8 +205,11 @@ def publish_event(self, data, suffix=''):
         Publish data for analytics purposes
         """
         event_type = data.pop('event_type')
-        self.runtime.publish(self, event_type, data)
+        if (event_type == 'grade'):
+            # This handler can be called from the browser. Don't allow the browser to submit arbitrary grades ;-)
+            raise JsonHandlerError(403, "Posting grade events from the browser is forbidden.")
 
+        self.runtime.publish(self, event_type, data)
         return {'result': 'ok'}
 
     def author_preview_view(self, context):
@@ -214,6 +225,10 @@ def author_preview_view(self, context):
         self.include_theme_files(fragment)
         return fragment
 
+    def max_score(self):
+        """ Maximum score. We scale all scores to a maximum of 1.0 so this is always 1.0 """
+        return 1.0
+
 
 class MentoringBlock(BaseMentoringBlock, StudioContainerXBlockMixin, StepParentMixin):
     """
@@ -262,13 +277,6 @@ class MentoringBlock(BaseMentoringBlock, StudioContainerXBlockMixin, StepParentM
     )
 
     # Settings
-    weight = Float(
-        display_name=_("Weight"),
-        help=_("Defines the maximum total grade of the block."),
-        default=1,
-        scope=Scope.settings,
-        enforce_type=True
-    )
     display_name = String(
         display_name=_("Title (Display name)"),
         help=_("Title to display"),
@@ -323,8 +331,6 @@ class MentoringBlock(BaseMentoringBlock, StudioContainerXBlockMixin, StepParentM
         'display_submit', 'feedback_label', 'weight', 'extended_feedback'
     )
 
-    has_score = True
-
     @property
     def is_assessment(self):
         """ Checks if mentoring XBlock is in assessment mode """
@@ -377,10 +383,6 @@ def score(self):
 
         return Score(score, int(round(score * 100)), correct, incorrect, partially_correct)
 
-    def max_score(self):
-        """ Maximum score. We scale all scores to a maximum of 1.0 so this is always 1.0 """
-        return 1.0
-
     def student_view(self, context):
         # Migrate stored data if necessary
         self.migrate_fields()
@@ -645,7 +647,7 @@ def submit(self, submissions, suffix=''):
             # Save the user's latest score
             self.runtime.publish(self, 'grade', {
                 'value': self.score.raw,
-                'max_value': 1,
+                'max_value': self.max_score(),
             })
 
             # Mark this as having used an attempt:
@@ -712,7 +714,7 @@ def handle_assessment_submit(self, submissions, suffix):
             log.info(u'Last assessment step submitted: {}'.format(submissions))
             self.runtime.publish(self, 'grade', {
                 'value': score.raw,
-                'max_value': 1,
+                'max_value': self.max_score(),
                 'score_type': 'proficiency',
             })
             event_data['final_grade'] = score.raw
@@ -848,7 +850,7 @@ class MentoringWithExplicitStepsBlock(BaseMentoringBlock, StudioContainerWithNes
         enforce_type=True
     )
 
-    editable_fields = ('display_name', 'max_attempts', 'extended_feedback')
+    editable_fields = ('display_name', 'max_attempts', 'extended_feedback', 'weight')
 
     @lazy
     def question_ids(self):
@@ -864,6 +866,27 @@ def questions(self):
         """
         return [self.runtime.get_block(question_id) for question_id in self.question_ids]
 
+    @property
+    def active_step_safe(self):
+        """
+        Get self.active_step and double-check that it is a valid value.
+        The stored value could be invalid if this block has been edited and new steps were
+        added/deleted.
+        """
+        active_step = self.active_step
+        if active_step >= 0 and active_step < len(self.step_ids):
+            return active_step
+        if active_step == -1 and self.has_review_step:
+            return active_step  # -1 indicates the review step
+        return 0
+
+    def get_active_step(self):
+        """ Get the active step as an instantiated XBlock """
+        block = self.runtime.get_block(self.step_ids[self.active_step_safe])
+        if block is None:
+            log.error("Unable to load step builder step child %s", self.step_ids[self.active_step_safe])
+        return block
+
     @lazy
     def step_ids(self):
         """
@@ -956,6 +979,8 @@ def student_view(self, context):
         fragment = Fragment()
         children_contents = []
 
+        context = context or {}
+        context['hide_prev_answer'] = True  # For Step Builder, we don't show the users' old answers when they try again
         for child_id in self.children:
             child = self.runtime.get_block(child_id)
             if child is None:  # child should not be None but it can happen due to bugs or permission issues
@@ -1003,36 +1028,45 @@ def allowed_nested_blocks(self):
         ]
 
     @XBlock.json_handler
-    def update_active_step(self, new_value, suffix=''):
+    def submit(self, data, suffix=None):
+        """
+        Called when the user has submitted the answer[s] for the current step.
+        """
+        # First verify that active_step is correct:
+        if data.get("active_step") != self.active_step_safe:
+            raise JsonHandlerError(400, "Invalid Step. Refresh the page and try again.")
+
+        # The step child will process the data:
+        step_block = self.get_active_step()
+        if not step_block:
+            raise JsonHandlerError(500, "Unable to load the current step block.")
+        response_data = step_block.submit(data)
+
+        # Update the active step:
+        new_value = self.active_step_safe + 1
         if new_value < len(self.step_ids):
             self.active_step = new_value
         elif new_value == len(self.step_ids):
+            # The user just completed the final step.
             if self.has_review_step:
                 self.active_step = -1
-        return {
-            'active_step': self.active_step
-        }
-
-    @XBlock.json_handler
-    def update_num_attempts(self, data, suffix=''):
-        if self.num_attempts < self.max_attempts:
-            self.num_attempts += 1
-        return {
-            'num_attempts': self.num_attempts
-        }
+            # Update the number of attempts, if necessary:
+            if self.num_attempts < self.max_attempts:
+                self.num_attempts += 1
+            response_data['num_attempts'] = self.num_attempts
+            # And publish the score:
+            score = self.score
+            grade_data = {
+                'value': score.raw,
+                'max_value': self.max_score(),
+            }
+            self.runtime.publish(self, 'grade', grade_data)
+            response_data['grade_data'] = self.get_grade()
 
-    @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 {}
+        response_data['active_step'] = self.active_step
+        return response_data
 
-    @XBlock.json_handler
-    def get_grade(self, data, suffix):
+    def get_grade(self, data=None, suffix=None):
         score = self.score
         return {
             'score': score.percentage,

problem_builder/mrq.py

@@ -148,7 +148,7 @@ def calculate_results(self, submissions):
             'submissions': submissions,
             'status': status,
             'choices': results,
-            'message': self.message,
+            'message': self.message_formatted,
             'weight': self.weight,
             'score': (float(score) / len(results)) if results else 0,
         }

problem_builder/public/js/mentoring_with_steps.js

@@ -55,60 +55,22 @@ function MentoringWithStepsBlock(runtime, element) {
         } else {
             checkmark.addClass('checkmark-incorrect icon-exclamation fa-exclamation');
         }
-    }
-
-    function postUpdateStep(response) {
-        activeStep = response.active_step;
-        if (activeStep === -1) {
-            updateNumAttempts();
-        } else {
-            updateControls();
+        var step = steps[activeStep];
+        if (typeof step.showFeedback == 'function') {
+            step.showFeedback(response);
         }
     }
 
-    function handleResults(response) {
-        showFeedback(response);
-
-        // Update active step:
-        // If we end up at the review step, proceed with updating the number of attempts used.
-        // Otherwise, get UI ready for showing next step.
-        var handlerUrl = runtime.handlerUrl(element, 'update_active_step');
-        $.post(handlerUrl, JSON.stringify(activeStep+1))
-            .success(postUpdateStep);
-    }
-
-    function updateNumAttempts() {
-        var handlerUrl = runtime.handlerUrl(element, 'update_num_attempts');
-        $.post(handlerUrl, JSON.stringify({}))
-            .success(function(response) {
-                attemptsDOM.data('num_attempts', response.num_attempts);
-                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();
-            });
-    }
-
-    function updateGrade() {
-        var handlerUrl = runtime.handlerUrl(element, 'get_grade');
-        $.post(handlerUrl, JSON.stringify({}))
-            .success(function(response) {
-                gradeDOM.data('score', response.score);
-                gradeDOM.data('correct_answer', response.correct_answers);
-                gradeDOM.data('incorrect_answer', response.incorrect_answers);
-                gradeDOM.data('partially_correct_answer', response.partially_correct_answers);
-                gradeDOM.data('correct', response.correct);
-                gradeDOM.data('incorrect', response.incorrect);
-                gradeDOM.data('partial', response.partial);
-                gradeDOM.data('assessment_review_tips', response.assessment_review_tips);
-                updateReviewStep(response);
-            });
+    function updateGrade(grade_data) {
+        gradeDOM.data('score', grade_data.score);
+        gradeDOM.data('correct_answer', grade_data.correct_answers);
+        gradeDOM.data('incorrect_answer', grade_data.incorrect_answers);
+        gradeDOM.data('partially_correct_answer', grade_data.partially_correct_answers);
+        gradeDOM.data('correct', grade_data.correct);
+        gradeDOM.data('incorrect', grade_data.incorrect);
+        gradeDOM.data('partial', grade_data.partial);
+        gradeDOM.data('assessment_review_tips', grade_data.assessment_review_tips);
+        updateReviewStep(grade_data);
     }
 
     function updateReviewStep(response) {
@@ -136,16 +98,27 @@ function MentoringWithStepsBlock(runtime, element) {
     }
 
     function submit() {
-        // We do not handle submissions at this level, so just forward to "submit" method of active step
-        var step = steps[activeStep];
-        step.submit(handleResults);
-    }
-
-    function markRead() {
-        var handlerUrl = runtime.handlerUrl(element, 'update_active_step');
-        $.post(handlerUrl, JSON.stringify(activeStep+1)).success(function (response) {
-            postUpdateStep(response);
-            updateDisplay();
+        submitDOM.attr('disabled', 'disabled'); // Disable the button until the results load.
+        var submitUrl = runtime.handlerUrl(element, 'submit');
+
+        var hasQuestion = steps[activeStep].hasQuestion();
+        var data = steps[activeStep].getSubmitData();
+        data["active_step"] = activeStep;
+        $.post(submitUrl, JSON.stringify(data)).success(function(response) {
+            showFeedback(response);
+            activeStep = response.active_step;
+            if (activeStep === -1) {
+                // We are now showing the review step / end
+                // Update the number of attempts.
+                attemptsDOM.data('num_attempts', response.num_attempts);
+                updateGrade(response.grade_data);
+            } else if (!hasQuestion) {
+                // This was a step with no questions, so proceed to the next step / review:
+                updateDisplay();
+            } else {
+                // Enable the Next button so users can proceed.
+                updateControls();
+            }
         });
     }
 
@@ -332,11 +305,11 @@ function MentoringWithStepsBlock(runtime, element) {
         if (isLastStep() && step.hasQuestion()) {
             nextDOM.hide();
         } else if (isLastStep()) {
-            reviewDOM.one('click', markRead);
+            reviewDOM.one('click', submit);
             reviewDOM.removeAttr('disabled');
             nextDOM.hide()
         } else if (!step.hasQuestion()) {
-            nextDOM.one('click', markRead);
+            nextDOM.one('click', submit);
         }
         if (step.hasQuestion()) {
             submitDOM.show();

problem_builder/public/js/questionnaire.js

@@ -122,7 +122,12 @@ function MCQBlock(runtime, element) {
             var mentoring = this.mentoring;
 
             var messageView = MessageView(element, mentoring);
-            messageView.clearResult();
+
+            if (result.message) {
+                var msg = '<div class="message-content">' + result.message + '</div>' +
+                          '<div class="close icon-remove-sign fa-times-circle"></div>';
+                messageView.showMessage(msg);
+            } else { messageView.clearResult(); }
 
             display_message(result.message, messageView, options.checkmark);