Fix max attempts message

.travis.yml

@@ -17,3 +17,5 @@ script:
     - python run_tests.py --with-coverage --cover-package=problem_builder
 notifications:
   email: false
+addons:
+  firefox: "36.0"

problem_builder/mentoring.py

@@ -26,7 +26,7 @@
 from collections import namedtuple
 
 from xblock.core import XBlock
-from xblock.exceptions import NoSuchViewError
+from xblock.exceptions import NoSuchViewError, JsonHandlerError
 from xblock.fields import Boolean, Scope, String, Integer, Float, List
 from xblock.fragment import Fragment
 from xblock.validation import ValidationMessage
@@ -148,6 +148,7 @@ class MentoringBlock(XBlock, StepParentMixin, StudioEditableXBlockMixin, StudioC
         # Has the student attempted this mentoring step?
         default=False,
         scope=Scope.user_state
+        # TODO: Does anything use this 'attempted' field? May want to delete it.
     )
     completed = Boolean(
         # Has the student completed this mentoring step?
@@ -376,15 +377,24 @@ def publish_event(self, data, suffix=''):
         return {'result': 'ok'}
 
     def get_message(self, completed):
-        if self.max_attempts_reached:
-            return self.get_message_html('max_attempts_reached')
-        elif completed:
+        """
+        Get the message to display to a student following a submission in normal mode.
+        """
+        if completed:
+            # Student has achieved a perfect score
             return self.get_message_html('completed')
+        elif self.max_attempts_reached:
+            # Student has not achieved a perfect score and cannot try again
+            return self.get_message_html('max_attempts_reached')
         else:
+            # Student did not achieve a perfect score but can try again:
             return self.get_message_html('incomplete')
 
     @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_html('on-assessment-review')
         else:
@@ -449,7 +459,6 @@ def get_results(self, queries, suffix=''):
         return {
             'results': results,
             'completed': completed,
-            'attempted': self.attempted,
             'message': message,
             'step': step,
             'max_attempts': self.max_attempts,
@@ -459,12 +468,23 @@ def get_results(self, queries, suffix=''):
     @XBlock.json_handler
     def submit(self, submissions, suffix=''):
         log.info(u'Received submissions: {}'.format(submissions))
+        # server-side check that the user is allowed to submit:
+        if self.max_attempts_reached:
+            raise JsonHandlerError(403, "Maximum number of attempts already reached.")
+        elif self.has_missing_dependency:
+            raise JsonHandlerError(
+                403,
+                "You need to complete all previous steps before being able to complete the current one."
+            )
+
+        # This has now been attempted:
         self.attempted = True
 
         if self.is_assessment:
             return self.handle_assessment_submit(submissions, suffix)
 
         submit_results = []
+        previously_completed = self.completed
         completed = True
         for child_id in self.steps:
             child = self.runtime.get_block(child_id)
@@ -475,40 +495,32 @@ def submit(self, submissions, suffix=''):
                 child.save()
                 completed = completed and (child_result['status'] == 'correct')
 
-        message = self.get_message(completed)
-
-        # Once it has been completed once, keep completion even if user changes values
-        if self.completed:
-            completed = True
-
-        # server-side check to not set completion if the max_attempts is reached
-        if self.max_attempts_reached:
-            completed = False
-
-        if self.has_missing_dependency:
-            completed = False
-            message = 'You need to complete all previous steps before being able to complete the current one.'
-        elif completed and self.next_step == self.url_name:
+        if completed and self.next_step == self.url_name:
             self.next_step = self.followed_by
 
-        # Once it was completed, lock score
-        if not self.completed:
-            # save user score and results
+        # Update the score and attempts, unless the user had already achieved a perfect score ("completed"):
+        if not previously_completed:
+            # Update the results
             while self.student_results:
                 self.student_results.pop()
             for result in submit_results:
                 self.student_results.append(result)
 
+            # Save the user's latest score
             self.runtime.publish(self, 'grade', {
                 'value': self.score.raw,
                 'max_value': 1,
             })
 
-        if not self.completed and self.max_attempts > 0:
-            self.num_attempts += 1
+            # Mark this as having used an attempt:
+            if self.max_attempts > 0:
+                self.num_attempts += 1
 
-        self.completed = completed is True
+        # Save the completion status.
+        # Once it has been completed once, keep completion even if user changes values
+        self.completed = bool(completed) or previously_completed
 
+        message = self.get_message(completed)
         raw_score = self.score.raw
 
         self.runtime.publish(self, 'xblock.problem_builder.submitted', {
@@ -520,10 +532,9 @@ def submit(self, submissions, suffix=''):
         return {
             'results': submit_results,
             'completed': self.completed,
-            'attempted': self.attempted,
             'message': message,
             'max_attempts': self.max_attempts,
-            'num_attempts': self.num_attempts
+            'num_attempts': self.num_attempts,
         }
 
     def handle_assessment_submit(self, submissions, suffix):
@@ -561,14 +572,13 @@ def handle_assessment_submit(self, submissions, suffix):
 
         if current_child == steps[-1]:
             log.info(u'Last assessment step submitted: {}'.format(submissions))
-            if not self.max_attempts_reached:
-                self.runtime.publish(self, 'grade', {
-                    'value': score.raw,
-                    'max_value': 1,
-                    'score_type': 'proficiency',
-                })
-                event_data['final_grade'] = score.raw
-                assessment_message = self.assessment_message
+            self.runtime.publish(self, 'grade', {
+                'value': score.raw,
+                'max_value': 1,
+                'score_type': 'proficiency',
+            })
+            event_data['final_grade'] = score.raw
+            assessment_message = self.assessment_message
 
             self.num_attempts += 1
             self.completed = True
@@ -581,7 +591,6 @@ def handle_assessment_submit(self, submissions, suffix):
 
         return {
             'completed': completed,
-            'attempted': self.attempted,
             'max_attempts': self.max_attempts,
             'num_attempts': self.num_attempts,
             'step': self.step,

problem_builder/message.py

@@ -40,6 +40,52 @@ class MentoringMessageBlock(XBlock, StudioEditableXBlockMixin):
     A message which can be conditionally displayed at the mentoring block level,
     for example upon completion of the block
     """
+    MESSAGE_TYPES = {
+        "completed": {
+            "display_name": _(u"Completed"),
+            "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."
+            ),
+        },
+        "incomplete": {
+            "display_name": _(u"Incomplete"),
+            "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."
+            ),
+        },
+        "max_attempts_reached": {
+            "display_name": _(u"Reached max. # of attempts"),
+            "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 "
+                "all of their allowed attempts without achieving a perfect score. "
+                "This message is ignored in assessment mode."
+            ),
+        },
+        "on-assessment-review": {
+            "display_name": _(u"Review with attempts left"),
+            "long_display_name": _(u"Message shown during review when attempts remain"),
+            "default": _(
+                u"You may try this assessment again, and only the latest score will be used."
+            ),
+            "description": _(
+                u"In assessment mode, this message will be shown when the student is reviewing "
+                "their answers to the assessment, if the student is allowed to try again. "
+                "This message is ignored in standard mode and is not shown if the student has "
+                "used up all of their allowed attempts."
+            ),
+        },
+    }
+
     content = String(
         display_name=_("Message"),
         help=_("Message to display upon completion"),
@@ -53,10 +99,10 @@ class MentoringMessageBlock(XBlock, StudioEditableXBlockMixin):
         scope=Scope.content,
         default="completed",
         values=(
-            {"display_name": "Completed", "value": "completed"},
-            {"display_name": "Incompleted", "value": "incomplete"},
-            {"display_name": "Reached max. # of attemps", "value": "max_attempts_reached"},
-            {"display_name": "Review with attempts left", "value": "on-assessment-review"}
+            {"value": "completed", "display_name": MESSAGE_TYPES["completed"]["display_name"]},
+            {"value": "incomplete", "display_name": MESSAGE_TYPES["incomplete"]["display_name"]},
+            {"value": "max_attempts_reached", "display_name": MESSAGE_TYPES["max_attempts_reached"]["display_name"]},
+            {"value": "on-assessment-review", "display_name": MESSAGE_TYPES["on-assessment-review"]["display_name"]},
         ),
     )
     editable_fields = ("content", )
@@ -67,34 +113,44 @@ def _(self, text):
 
     def mentoring_view(self, context=None):
         """ Render this message for use by a mentoring block. """
-        html = u'<div class="message {msg_type}">{content}</div>'.format(msg_type=self.type, content=self.content)
+        html = u'<div class="submission-message {msg_type}">{content}</div>'.format(
+            msg_type=self.type,
+            content=self.content
+        )
         return Fragment(html)
 
     def student_view(self, context=None):
         """ Normal view of this XBlock, identical to mentoring_view """
         return self.mentoring_view(context)
 
+    def author_view(self, context=None):
+        fragment = self.mentoring_view(context)
+        fragment.content += u'<div class="submission-message-help"><p>{}</p></div>'.format(self.help_text)
+        return fragment
+
     @property
     def display_name_with_default(self):
-        if self.type == 'max_attempts_reached':
-            max_attempts = self.get_parent().max_attempts
-            return self._(u"Message when student reaches max. # of attempts ({limit})").format(
-                limit=self._(u"unlimited") if max_attempts == 0 else max_attempts
-            )
-        if self.type == 'completed':
-            return self._(u"Message shown when complete")
-        if self.type == 'incomplete':
-            return self._(u"Message shown when incomplete")
-        if self.type == 'on-assessment-review':
-            return self._(u"Message shown during review when attempts remain")
-        return u"INVALID MESSAGE"
+        try:
+            return self._(self.MESSAGE_TYPES[self.type]["long_display_name"])
+        except KeyError:
+            return u"INVALID MESSAGE"
+
+    @property
+    def help_text(self):
+        try:
+            return self._(self.MESSAGE_TYPES[self.type]["description"])
+        except KeyError:
+            return u"This message is not a valid message type!"
 
     @classmethod
     def get_template(cls, template_id):
         """
         Used to interact with Studio's create_xblock method to instantiate pre-defined templates.
         """
-        return {'data': {'type': template_id, 'content': "Message goes here."}}
+        return {'data': {
+            'type': template_id,
+            'content': cls.MESSAGE_TYPES[template_id]["default"],
+        }}
 
     @classmethod
     def parse_xml(cls, node, runtime, keys, id_generator):

problem_builder/public/css/mentoring_edit.css

@@ -26,3 +26,11 @@
   border-color: #888;
   cursor: default;
 }
+
+.xblock[data-block-type=problem-builder] .submission-message-help p {
+	border-top: 1px solid #ddd;
+	font-size: 0.85em;
+	font-style: italic;
+	margin-top: 1em;
+	padding-top: 0.3em;
+}

problem_builder/public/js/answer.js

@@ -24,7 +24,6 @@ function AnswerBlock(runtime, element) {
         handleSubmit: function(result) {
 
             var checkmark = $('.answer-checkmark', element);
-            $(element).find('.message').text((result || {}).error || '');
 
             this.clearResult();
 

problem_builder/public/js/mentoring_edit.js

@@ -5,7 +5,7 @@ function MentoringEditComponents(runtime, element) {
     var updateButtons = function() {
         $buttons.each(function() {
             var msg_type = $(this).data('boilerplate');
-            $(this).toggleClass('disabled', $('.xblock .message.'+msg_type).length > 0);
+            $(this).toggleClass('disabled', $('.xblock .submission-message.'+msg_type).length > 0);
         });
     };
     updateButtons();

problem_builder/public/js/mentoring_standard_view.js

@@ -32,6 +32,24 @@ function MentoringStandardView(runtime, element, mentoring) {
         submitDOM.attr('disabled', 'disabled');
     }
 
+    function handleSubmitError(jqXHR, textStatus, errorThrown) {
+        if (textStatus == "error") {
+            var errMsg = errorThrown;
+            // Check if there's a more specific JSON error message:
+            if (jqXHR.responseText) {
+                // Is there a more specific error message we can show?
+                try {
+                    errMsg = JSON.parse(jqXHR.responseText).error;
+                } catch (error) { errMsg = jqXHR.responseText.substr(0, 300); }
+            }
+
+            mentoring.setContent(messagesDOM, errMsg);
+            messagesDOM.show();
+
+            submitDOM.attr('disabled', 'disabled');
+        }
+    }
+
     function calculate_results(handler_name) {
         var data = {};
         var children = mentoring.children;
@@ -45,11 +63,7 @@ function MentoringStandardView(runtime, element, mentoring) {
         if (submitXHR) {
             submitXHR.abort();
         }
-        submitXHR = $.post(handlerUrl, JSON.stringify(data)).success(handleSubmitResults);
-    }
-
-    function get_results() {
-        calculate_results('get_results');
+        submitXHR = $.post(handlerUrl, JSON.stringify(data)).success(handleSubmitResults).error(handleSubmitError);
     }
 
     function submit() {