diff --git a/problem_builder/mentoring.py b/problem_builder/mentoring.py index db4955f2..5f4420bf 100644 --- a/problem_builder/mentoring.py +++ b/problem_builder/mentoring.py @@ -374,7 +374,7 @@ def score(self): 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 @@ def student_view(self, context): 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 @@ def next_step_url(self): """ 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 @@ def _get_standard_results(self): """ 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: diff --git a/problem_builder/public/js/answer.js b/problem_builder/public/js/answer.js index f46f6f87..544c4110 100644 --- a/problem_builder/public/js/answer.js +++ b/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(); + } }, }); } diff --git a/problem_builder/public/js/mentoring_standard_view.js b/problem_builder/public/js/mentoring_standard_view.js index 149a25a6..93a374cd 100644 --- a/problem_builder/public/js/mentoring_standard_view.js +++ b/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); diff --git a/problem_builder/public/js/mentoring_with_steps.js b/problem_builder/public/js/mentoring_with_steps.js index 532c4f9e..aa323b7c 100644 --- a/problem_builder/public/js/mentoring_with_steps.js +++ b/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); diff --git a/problem_builder/public/js/questionnaire.js b/problem_builder/public/js/questionnaire.js index 827d946f..7263636d 100644 --- a/problem_builder/public/js/questionnaire.js +++ b/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); diff --git a/problem_builder/templates/html/mentoring.html b/problem_builder/templates/html/mentoring.html index 25ba2f56..d01cfd1e 100644 --- a/problem_builder/templates/html/mentoring.html +++ b/problem_builder/templates/html/mentoring.html @@ -1,5 +1,5 @@ {% load i18n %} -
+
{% with url=missing_dependency_url|safe %} {% blocktrans with link_start="" link_end="" %} diff --git a/problem_builder/templates/html/mrqblock.html b/problem_builder/templates/html/mrqblock.html index b7760367..468df273 100644 --- a/problem_builder/templates/html/mrqblock.html +++ b/problem_builder/templates/html/mrqblock.html @@ -1,4 +1,4 @@ -
+
{% if not hide_header %}

{{ self.display_name_with_default }}

{% endif %}

{{ self.question|safe }}

diff --git a/problem_builder/tests/integration/test_mentoring.py b/problem_builder/tests/integration/test_mentoring.py index 0d7cc657..807cb732 100644 --- a/problem_builder/tests/integration/test_mentoring.py +++ b/problem_builder/tests/integration/test_mentoring.py @@ -164,17 +164,22 @@ def _assert_not_checked(self, questionnaire, choice_index): 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 @@ def _mcq_hide_previous_answer_checks(self, answer, mcq, mrq, rating, messages): 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 @@ def _mcq_hide_previous_answer_hide_feedback_checks(self, answer, mcq, mrq, ratin 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 @@ def _test_persistence(self, options, after_reload_checks): # ... 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()) diff --git a/problem_builder/tests/unit/test_problem_builder.py b/problem_builder/tests/unit/test_problem_builder.py index e6f526d5..e2490b4f 100644 --- a/problem_builder/tests/unit/test_problem_builder.py +++ b/problem_builder/tests/unit/test_problem_builder.py @@ -227,7 +227,8 @@ def test_student_view_calls_get_option(self): 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: