Merge pull request #110 from open-craft/freetext-submit

Enable freeform answer submit when feedback is hidden

problem_builder/mentoring.py

@@ -374,7 +374,7 @@ class MentoringBlock(BaseMentoringBlock, StudioContainerXBlockMixin, StepParentM
         return Score(score, int(round(score * 100)), correct, incorrect, partially_correct)
 
     def student_view(self, context):
-        from .mcq import MCQBlock  # Import here to avoid circular dependency
+        from .questionnaire import QuestionnaireAbstractBlock  # Import here to avoid circular dependency
 
         # Migrate stored data if necessary
         self.migrate_fields()
@@ -398,7 +398,7 @@ class MentoringBlock(BaseMentoringBlock, StudioContainerXBlockMixin, StepParentM
                     if self.is_assessment and isinstance(child, QuestionMixin):
                         child_fragment = child.render('assessment_step_view', context)
                     else:
-                        if mcq_hide_previous_answer and isinstance(child, MCQBlock):
+                        if mcq_hide_previous_answer and isinstance(child, QuestionnaireAbstractBlock):
                             context['hide_prev_answer'] = True
                         else:
                             context['hide_prev_answer'] = False
@@ -478,6 +478,10 @@ class MentoringBlock(BaseMentoringBlock, StudioContainerXBlockMixin, StepParentM
         """
         return '/jump_to_id/{}'.format(self.next_step)
 
+    @property
+    def hide_feedback(self):
+        return self.get_option("pb_hide_feedback_if_attempts_remain") and not self.max_attempts_reached
+
     def get_message(self, completed):
         """
         Get the message to display to a student following a submission in normal mode.
@@ -564,8 +568,7 @@ class MentoringBlock(BaseMentoringBlock, StudioContainerXBlockMixin, StepParentM
         """
         results = []
         completed = True
-        hide_feedback = self.get_option("pb_hide_feedback_if_attempts_remain") and not self.max_attempts_reached
-        show_message = (not hide_feedback) and bool(self.student_results)
+        show_message = (not self.hide_feedback) and bool(self.student_results)
 
         # In standard mode, all children are visible simultaneously, so need to collect results for all of them
         for child in self.steps:

problem_builder/public/js/answer.js

@@ -6,6 +6,7 @@ function AnswerBlock(runtime, element) {
             $(':input', element).on('keyup', options.onChange);
 
             this.mode = options.mode;
+            this.validateXBlock = options.validateXBlock;
 
             // In the LMS, the HTML of multiple units can be loaded at once,
             // and the user can flip among them. If that happens, the answer in
@@ -72,6 +73,7 @@ function AnswerBlock(runtime, element) {
         },
 
         refreshAnswer: function() {
+            var self = this;
             $.ajax({
                 type: 'POST',
                 url: runtime.handlerUrl(element, 'answer_value'),
@@ -86,6 +88,9 @@ function AnswerBlock(runtime, element) {
                     if (currentAnswer == origAnswer && currentAnswer != newAnswer) {
                         $textarea.val(newAnswer);
                     }
+                    if (self.validateXBlock) {
+                      self.validateXBlock();
+                    }
                 },
             });
         }

problem_builder/public/js/mentoring_standard_view.js

@@ -35,10 +35,13 @@ function MentoringStandardView(runtime, element, mentoring) {
             }
         }
 
+        // Data may have changed, we have to re-validate.
+        validateXBlock();
+
         // Disable the submit button if we have just submitted new answers,
         // or if we have just [re]loaded the page and are showing a complete set
         // of old answers.
-        if (disable_submit || all_have_results) {
+        if (disable_submit || (all_have_results && mentoring.data.hide_feedback !== 'True')) {
             submitDOM.attr('disabled', 'disabled');
         }
     }
@@ -110,7 +113,8 @@ function MentoringStandardView(runtime, element, mentoring) {
         submitDOM.show();
 
         var options = {
-            onChange: onChange
+            onChange: onChange,
+            validateXBlock: validateXBlock
         };
 
         mentoring.initChildren(options);

problem_builder/public/js/mentoring_with_steps.js

@@ -302,7 +302,8 @@ function MentoringWithStepsBlock(runtime, element) {
             var step = steps[i];
             var mentoring = {
                 setContent: setContent,
-                publish_event: publishEvent
+                publish_event: publishEvent,
+                is_step_builder: true
             };
             options.mentoring = mentoring;
             step.initChildren(options);

problem_builder/public/js/questionnaire.js

@@ -210,7 +210,11 @@ function MRQBlock(runtime, element) {
 
             var questionnaireDOM = $('fieldset.questionnaire', element);
             var data = questionnaireDOM.data();
-            var hide_results = (data.hide_results === 'True');
+            var hide_results = (data.hide_results === 'True' ||
+                                (data.hide_prev_answer === 'True' && !mentoring.is_step_builder));
+            // hide_prev_answer should only take effect when we initially render (previous) results,
+            // so set hide_prev_answer to False after initial render.
+            questionnaireDOM.data('hide_prev_answer', 'False');
 
             $.each(result.choices, function(index, choice) {
                 var choiceInputDOM = $('.choice input[value='+choice.value+']', element);

problem_builder/templates/html/mentoring.html

 {% load i18n %}
-<div class="mentoring themed-xblock" data-mode="{{ self.mode }}" data-step="{{ self.step }}" data-feedback_label="{{ self.feedback_label}}">
+<div class="mentoring themed-xblock" data-mode="{{ self.mode }}" data-step="{{ self.step }}" data-feedback_label="{{ self.feedback_label }}" data-hide_feedback="{{ self.hide_feedback }}">
   <div class="missing-dependency warning" data-missing="{{ self.has_missing_dependency }}">
     {% with url=missing_dependency_url|safe %}
     {% blocktrans with link_start="<a href='"|add:url|add:"'>" link_end="</a>" %}

problem_builder/templates/html/mrqblock.html

-<fieldset class="choices questionnaire" data-hide_results="{{self.hide_results}}">
+<fieldset class="choices questionnaire" data-hide_results="{{self.hide_results}}" data-hide_prev_answer="{{hide_prev_answer}}">
   <legend class="question">
 	  {% if not hide_header %}<h3 class="question-title">{{ self.display_name_with_default }}</h3>{% endif %}
     <p>{{ self.question|safe }}</p>

problem_builder/tests/integration/test_mentoring.py

@@ -164,17 +164,22 @@ class ProblemBuilderQuestionnaireBlockTest(ProblemBuilderBaseTest):
         choice_input = choice.find_element_by_css_selector('input')
         self.assertFalse(choice_input.is_selected())
 
-    def _assert_mrq(self, mrq):
-        self._assert_feedback_shown(
-            mrq, 0, "This is something everyone has to like about this MRQ",
-            click_choice_result=True
-        )
-        self._assert_feedback_shown(
-            mrq, 1, "This is something everyone has to like about beauty",
-            click_choice_result=True, success=False
-        )
-        self._assert_feedback_shown(mrq, 2, "This MRQ is indeed very graceful", click_choice_result=True)
-        self._assert_feedback_shown(mrq, 3, "Nah, there aren't any!", click_choice_result=True, success=False)
+    def _assert_mrq(self, mrq, previous_answer_shown=True):
+        if previous_answer_shown:
+            self._assert_feedback_shown(
+                mrq, 0, "This is something everyone has to like about this MRQ",
+                click_choice_result=True
+            )
+            self._assert_feedback_shown(
+                mrq, 1, "This is something everyone has to like about beauty",
+                click_choice_result=True, success=False
+            )
+            self._assert_feedback_shown(mrq, 2, "This MRQ is indeed very graceful", click_choice_result=True)
+            self._assert_feedback_shown(mrq, 3, "Nah, there aren't any!", click_choice_result=True, success=False)
+        else:
+            for i in range(3):
+                self._assert_feedback_hidden(mrq, i)
+                self._assert_not_checked(mrq, i)
 
     def _assert_messages(self, messages, shown=True):
         if shown:
@@ -227,8 +232,8 @@ class ProblemBuilderQuestionnaireBlockTest(ProblemBuilderBaseTest):
         self._assert_answer(answer)
         # MCQ: Previous answer and results hidden
         self._assert_mcq(mcq, previous_answer_shown=False)
-        # MRQ: Previous answer and results visible
-        self._assert_mrq(mrq)
+        # MRQ: Previous answer and results hidden
+        self._assert_mrq(mrq, previous_answer_shown=False)
         # Rating: Previous answer and results hidden
         self._assert_rating(rating, previous_answer_shown=False)
         # Messages visible
@@ -251,8 +256,8 @@ class ProblemBuilderQuestionnaireBlockTest(ProblemBuilderBaseTest):
         self._assert_answer(answer, results_shown=False)
         # MCQ: Previous answer and results hidden
         self._assert_mcq(mcq, previous_answer_shown=False)
-        # MRQ: Previous answer and results visible
-        self._assert_mrq(mrq)
+        # MRQ: Previous answer and results hidden
+        self._assert_mrq(mrq, previous_answer_shown=False)
         # Rating: Previous answer and feedback hidden
         self._assert_rating(rating, previous_answer_shown=False)
         # Messages hidden
@@ -338,11 +343,19 @@ class ProblemBuilderQuestionnaireBlockTest(ProblemBuilderBaseTest):
             # ... and see if previous answers, results, feedback are shown/hidden correctly
             getattr(self, after_reload_checks)(answer, mcq, mrq, rating, messages)
 
-            # After reloading, submit is disabled...
-            self.assertFalse(submit.is_enabled())
-
-            # ... until student makes changes
+            # After reloading, submit is enabled only when:
+            #  - feedback is hidden; and
+            #  - previous MCQ/MRQ answers are visible.
+            # When feedback is visible there's no need to resubmit the same answer;
+            # and when previous MCQ/MRQ answers are hidden, submit is disabled until you select some options.
+            if options['pb_hide_feedback_if_attempts_remain'] and not options['pb_mcq_hide_previous_answer']:
+                self.assertTrue(submit.is_enabled())
+            else:
+                self.assertFalse(submit.is_enabled())
+
+            # When student makes changes, submit is enabled again.
             self.click_choice(mcq, "Maybe not")
+            self.click_choice(mrq, "Its elegance")
             self.click_choice(rating, "2")
             self.assertTrue(submit.is_enabled())
 

problem_builder/tests/unit/test_problem_builder.py

@@ -227,7 +227,8 @@ class TestMentoringBlockOptions(unittest.TestCase):
         self.block.get_xblock_settings = Mock(return_value={})
         with patch.object(self.block, 'get_option') as patched_get_option:
             self.block.student_view({})
-            patched_get_option.assert_called_with('pb_mcq_hide_previous_answer')
+            patched_get_option.assert_any_call('pb_mcq_hide_previous_answer')
+            patched_get_option.assert_any_call('pb_hide_feedback_if_attempts_remain')
 
     def test_get_standard_results_calls_get_option(self):
         with patch.object(self.block, 'get_option') as patched_get_option: