Fix: delay between submitting answer and next step being clickable

problem_builder/mentoring.py

@@ -866,6 +866,27 @@ class MentoringWithExplicitStepsBlock(BaseMentoringBlock, StudioContainerWithNes
         """
         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):
         """
@@ -1007,36 +1028,45 @@ class MentoringWithExplicitStepsBlock(BaseMentoringBlock, StudioContainerWithNes
         ]
 
     @XBlock.json_handler
-    def update_active_step(self, new_value, suffix=''):
+    def submit(self, data, suffix=None):
+        """
+        Called this 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': 1,
+            }
+            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/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/step.js

@@ -44,29 +44,25 @@ function MentoringStepBlock(runtime, element) {
             return is_valid;
         },
 
-        submit: function(resultHandler) {
-            var handler_name = 'submit';
+        getSubmitData: function() {
             var data = {};
             for (var i = 0; i < children.length; i++) {
                 var child = children[i];
                 if (child && child.name !== undefined) {
-                    data[child.name.toString()] = callIfExists(child, handler_name);
+                    data[child.name.toString()] = callIfExists(child, "submit");
                 }
             }
-            var handlerUrl = runtime.handlerUrl(element, handler_name);
-            if (submitXHR) {
-                submitXHR.abort();
-            }
-            submitXHR = $.post(handlerUrl, JSON.stringify(data))
-                .success(function(response) {
-                    resultHandler(response);
-                    if (message.length) {
-                        message.fadeIn();
-                        $(document).click(function() {
-                            message.fadeOut();
-                        });
-                    }
+            return data;
+        },
+
+        showFeedback: function(response) {
+            // Called when user has just submitted an answer or is reviewing their answer durign extended feedback.
+            if (message.length) {
+                message.fadeIn();
+                $(document).click(function() {
+                    message.fadeOut();
                 });
+            }
         },
 
         getResults: function(resultHandler) {

problem_builder/step.py

@@ -160,8 +160,8 @@ class MentoringStepBlock(
     def has_question(self):
         return any(getattr(child, 'answerable', False) for child in self.steps)
 
-    @XBlock.json_handler
-    def submit(self, submissions, suffix=''):
+    def submit(self, submissions):
+        """ Handle a student submission. This is called by the parent XBlock. """
         log.info(u'Received submissions: {}'.format(submissions))
 
         # Submit child blocks (questions) and gather results
@@ -177,6 +177,7 @@ class MentoringStepBlock(
         self.reset()
         for result in submit_results:
             self.student_results.append(result)
+        self.save()
 
         return {
             'message': 'Success!',

problem_builder/templates/html/mentoring_with_steps.html

 {% load i18n %}
-<div class="mentoring themed-xblock" data-active-step="{{ self.active_step }}">
+<div class="mentoring themed-xblock" data-active-step="{{ self.active_step_safe }}">
 
   {% if show_title and title %}
   <div class="title">

problem_builder/tests/unit/test_step_builder.py

-import unittest
-import ddt
-from mock import Mock
-from problem_builder.mentoring import MentoringWithExplicitStepsBlock
-
-from .utils import ScoresTestMixin, instantiate_block
-
-
-@ddt.ddt
-class TestStepBuilder(ScoresTestMixin, unittest.TestCase):
-    """ Unit tests for Step Builder (MentoringWithExplicitStepsBlock) """
-
-    def test_scores(self):
-        """
-        Test that scores are emitted correctly.
-        """
-        # Submit an empty block - score should be 0:
-        block = instantiate_block(MentoringWithExplicitStepsBlock)
-        with self.expect_score_event(block, score=0.0, max_score=1.0):
-            request = Mock(method="POST", body="{}")
-            block.publish_attempt(request, suffix=None)
-
-        # Mock a block to contain an MCQ question, then submit it. Score should be 1:
-        block = instantiate_block(MentoringWithExplicitStepsBlock)
-        block.questions = [Mock(weight=1.0)]
-        block.questions[0].name = 'mcq1'
-        block.steps = [Mock(
-            student_results=[('mcq1', {'score': 1, 'status': 'correct'})]
-        )]
-        block.answer_mapper = lambda _status: None
-        with self.expect_score_event(block, score=1.0, max_score=1.0):
-            request = Mock(method="POST", body="{}")
-            block.publish_attempt(request, suffix=None)

problem_builder/tests/unit/utils.py

 """
 Helper methods for testing Problem Builder / Step Builder blocks
 """
-from contextlib import contextmanager
-from mock import MagicMock, Mock, patch
+from mock import MagicMock, Mock
 from xblock.field_data import DictFieldData
 
 
@@ -20,16 +19,6 @@ class ScoresTestMixin(object):
         self.assertEqual(block.weight, 1.0)  # Default weight should be 1
         self.assertIsInstance(block.max_score(), (int, float))
 
-    @contextmanager
-    def expect_score_event(self, block, score, max_score):
-        """
-        Context manager. Expect that the given block instance will publish the given score.
-        """
-        with patch.object(block.runtime, 'publish') as mocked_publish:
-            yield
-
-            mocked_publish.assert_called_once_with(block, 'grade', {'value': score, 'max_value': max_score})
-
 
 def instantiate_block(cls, fields=None):
     """