Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions problem_builder/mentoring.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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
Expand Down Expand Up @@ -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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mtyaka I like this refactoring :)

def get_message(self, completed):
"""
Get the message to display to a student following a submission in normal mode.
Expand Down Expand Up @@ -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:
Expand Down
5 changes: 5 additions & 0 deletions problem_builder/public/js/answer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -72,6 +73,7 @@ function AnswerBlock(runtime, element) {
},

refreshAnswer: function() {
var self = this;
$.ajax({
type: 'POST',
url: runtime.handlerUrl(element, 'answer_value'),
Expand All @@ -86,6 +88,9 @@ function AnswerBlock(runtime, element) {
if (currentAnswer == origAnswer && currentAnswer != newAnswer) {
$textarea.val(newAnswer);
}
if (self.validateXBlock) {
self.validateXBlock();
}
},
});
}
Expand Down
8 changes: 6 additions & 2 deletions problem_builder/public/js/mentoring_standard_view.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,13 @@ function MentoringStandardView(runtime, element, mentoring) {
}
}

// Data may have changed, we have to re-validate.
validateXBlock();

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mtyaka changechanged. Also, is this still necessary now that answer blocks are handling validation themselves?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@itsjeyd I fixed the typo, thanks for noticing. This probably isn't necessary for answer blocks anymore (although it shouldn't do any harm), but I believe it still makes sense for other blocks.

// 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');
}
}
Expand Down Expand Up @@ -110,7 +113,8 @@ function MentoringStandardView(runtime, element, mentoring) {
submitDOM.show();

var options = {
onChange: onChange
onChange: onChange,
validateXBlock: validateXBlock
};

mentoring.initChildren(options);
Expand Down
3 changes: 2 additions & 1 deletion problem_builder/public/js/mentoring_with_steps.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 5 additions & 1 deletion problem_builder/public/js/questionnaire.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion problem_builder/templates/html/mentoring.html
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{% 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>" %}
Expand Down
2 changes: 1 addition & 1 deletion problem_builder/templates/html/mrqblock.html
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<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>
Expand Down
51 changes: 32 additions & 19 deletions problem_builder/tests/integration/test_mentoring.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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())

Expand Down
3 changes: 2 additions & 1 deletion problem_builder/tests/unit/test_problem_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down