diff --git a/problem_builder/answer.py b/problem_builder/answer.py index 0e772de5..0acd2b61 100644 --- a/problem_builder/answer.py +++ b/problem_builder/answer.py @@ -189,6 +189,9 @@ def get_results(self, previous_response=None): 'score': 1 if self.status == 'correct' else 0, } + def get_last_result(self): + return self.get_results(None) if self.student_input else {} + def submit(self, submission): """ The parent block is handling a student submission, including a new answer for this diff --git a/problem_builder/mcq.py b/problem_builder/mcq.py index 17e3e367..e658f2ff 100644 --- a/problem_builder/mcq.py +++ b/problem_builder/mcq.py @@ -105,6 +105,9 @@ def calculate_results(self, submission): def get_results(self, previous_result): return self.calculate_results(previous_result['submission']) + def get_last_result(self): + return self.get_results({'submission': self.student_choice}) if self.student_choice else {} + def submit(self, submission): log.debug(u'Received MCQ submission: "%s"', submission) result = self.calculate_results(submission) diff --git a/problem_builder/mentoring.py b/problem_builder/mentoring.py index c0a9f726..4b088505 100644 --- a/problem_builder/mentoring.py +++ b/problem_builder/mentoring.py @@ -435,6 +435,57 @@ def get_results(self, queries, suffix=''): """ Gets detailed results in the case of extended feedback. + Right now there are two ways to get results-- through the template upon loading up + the mentoring block, or after submission of an AJAX request like in + submit or get_results here. + """ + if self.mode == 'standard': + results, completed, show_message = self._get_standard_results() + mentoring_completed = completed + else: + if not self.show_extended_feedback(): + return { + 'results': [], + 'error': 'Extended feedback results cannot be obtained.' + } + + results, completed, show_message = self._get_assessment_results(queries) + mentoring_completed = True + + result = { + 'results': results, + 'completed': completed, + 'step': self.step, + 'max_attempts': self.max_attempts, + 'num_attempts': self.num_attempts, + } + + if show_message: + result['message'] = self.get_message(mentoring_completed) + + return result + + def _get_standard_results(self): + """ + Gets previous submissions results as if submit was called with exactly the same values as last time. + """ + results = [] + completed = True + show_message = bool(self.student_results) + + # In standard mode, all children is visible simultaneously, so need collecting responses from all of them + for child_id in self.steps: + child = self.runtime.get_block(child_id) + child_result = child.get_last_result() + results.append([child.name, child_result]) + completed = completed and (child_result.get('status', None) == 'correct') + + return results, completed, show_message + + def _get_assessment_results(self, queries): + """ + Gets detailed results in the case of extended feedback. + It may be a good idea to eventually have this function get results in the general case instead of loading them in the template in the future, and only using it for extended feedback situations. @@ -444,14 +495,8 @@ def get_results(self, queries, suffix=''): submit or get_results here. """ results = [] - if not self.show_extended_feedback(): - return { - 'results': [], - 'error': 'Extended feedback results cannot be obtained.' - } completed = True choices = dict(self.student_results) - step = self.step # Only one child should ever be of concern with this method. for child_id in self.steps: child = self.runtime.get_block(child_id) @@ -464,17 +509,7 @@ def get_results(self, queries, suffix=''): completed = choices[child.name]['status'] break - # The 'completed' message should always be shown in this case, since no more attempts are available. - message = self.get_message(True) - - return { - 'results': results, - 'completed': completed, - 'message': message, - 'step': step, - 'max_attempts': self.max_attempts, - 'num_attempts': self.num_attempts, - } + return results, completed, True @XBlock.json_handler def submit(self, submissions, suffix=''): diff --git a/problem_builder/mrq.py b/problem_builder/mrq.py index 1d8fad59..f0b88a14 100644 --- a/problem_builder/mrq.py +++ b/problem_builder/mrq.py @@ -81,14 +81,20 @@ def describe_choice_correctness(self, choice_value): return self._(u"Ignored") return self._(u"Not Acceptable") - def get_results(self, previous_result): + def get_results(self, previous_result, only_selected=False): """ Get the results a student has already submitted. """ - result = self.calculate_results(previous_result['submissions']) + result = self.calculate_results(previous_result['submissions'], only_selected) result['completed'] = True return result + def get_last_result(self): + if self.student_choices: + return self.get_results({'submissions': self.student_choices}, only_selected=True) + else: + return {} + def submit(self, submissions): log.debug(u'Received MRQ submissions: "%s"', submissions) @@ -98,13 +104,17 @@ def submit(self, submissions): log.debug(u'MRQ submissions result: %s', result) return result - def calculate_results(self, submissions): + def calculate_results(self, submissions, only_selected=False): score = 0 results = [] + for choice in self.custom_choices: choice_completed = True choice_tips_html = [] choice_selected = choice.value in submissions + if not choice_selected and only_selected: + continue + if choice.value in self.required_choices: if not choice_selected: choice_completed = False diff --git a/problem_builder/public/js/answer.js b/problem_builder/public/js/answer.js index 84d23896..74f0ff4c 100644 --- a/problem_builder/public/js/answer.js +++ b/problem_builder/public/js/answer.js @@ -31,12 +31,13 @@ function AnswerBlock(runtime, element) { // Display of checkmark would be redundant. return } - - if (result.status === "correct") { - checkmark.addClass('checkmark-correct icon-ok fa-check'); - } - else { - checkmark.addClass('checkmark-incorrect icon-exclamation fa-exclamation'); + if (result.status) { + if (result.status === "correct") { + checkmark.addClass('checkmark-correct icon-ok fa-check'); + } + else { + checkmark.addClass('checkmark-incorrect icon-exclamation fa-exclamation'); + } } }, diff --git a/problem_builder/public/js/mentoring_standard_view.js b/problem_builder/public/js/mentoring_standard_view.js index c431a276..b461dfe2 100644 --- a/problem_builder/public/js/mentoring_standard_view.js +++ b/problem_builder/public/js/mentoring_standard_view.js @@ -28,8 +28,6 @@ function MentoringStandardView(runtime, element, mentoring) { messagesDOM.prepend('
' + gettext('Feedback') + '
'); messagesDOM.show(); } - - submitDOM.attr('disabled', 'disabled'); } function handleSubmitError(jqXHR, textStatus, errorThrown) { @@ -45,12 +43,10 @@ function MentoringStandardView(runtime, element, mentoring) { mentoring.setContent(messagesDOM, errMsg); messagesDOM.show(); - - submitDOM.attr('disabled', 'disabled'); } } - function calculate_results(handler_name) { + function calculate_results(handler_name, disable_submit) { var data = {}; var children = mentoring.children; for (var i = 0; i < children.length; i++) { @@ -64,10 +60,19 @@ function MentoringStandardView(runtime, element, mentoring) { submitXHR.abort(); } submitXHR = $.post(handlerUrl, JSON.stringify(data)).success(handleSubmitResults).error(handleSubmitError); + + if (disable_submit) { + var disable_submit_callback = function(){ submitDOM.attr('disabled', 'disabled'); }; + submitXHR.success(disable_submit_callback).error(disable_submit_callback); + } + } + + function get_results(){ + calculate_results('get_results', false); } function submit() { - calculate_results('submit'); + calculate_results('submit', true); } function clearResults() { @@ -97,6 +102,8 @@ function MentoringStandardView(runtime, element, mentoring) { mentoring.initChildren(options); mentoring.renderDependency(); + get_results(); + var submitPossible = submitDOM.length > 0; if (submitPossible) { mentoring.renderAttempts(); diff --git a/problem_builder/tests/integration/base_test.py b/problem_builder/tests/integration/base_test.py index 70616426..059af9c9 100644 --- a/problem_builder/tests/integration/base_test.py +++ b/problem_builder/tests/integration/base_test.py @@ -85,6 +85,13 @@ def click_submit(self, mentoring): submit.click() self.wait_until_disabled(submit) + def click_choice(self, container, choice_text): + """ Click on the choice label with the specified text """ + for label in container.find_elements_by_css_selector('.choice label'): + if choice_text in label.text: + label.click() + break + class MentoringBaseTest(SeleniumBaseTest, PopupCheckMixin): module_name = __name__ diff --git a/problem_builder/tests/integration/test_mentoring.py b/problem_builder/tests/integration/test_mentoring.py index b913abbf..7174db50 100644 --- a/problem_builder/tests/integration/test_mentoring.py +++ b/problem_builder/tests/integration/test_mentoring.py @@ -21,7 +21,7 @@ import mock import ddt from selenium.common.exceptions import NoSuchElementException -from .base_test import MentoringBaseTest, MentoringAssessmentBaseTest, GetChoices +from .base_test import MentoringBaseTest, MentoringAssessmentBaseTest, GetChoices, ProblemBuilderBaseTest class MentoringTest(MentoringBaseTest): @@ -72,3 +72,169 @@ def test_lms_theme_applied(self, theme, expected_color): with mock.patch("problem_builder.MentoringBlock.get_theme") as patched_theme: patched_theme.return_value = _get_mentoring_theme_settings(theme) self.assert_status_icon_color(expected_color) + + +@ddt.ddt +class ProblemBuilderQuestionnaireBlockTest(ProblemBuilderBaseTest): + def _get_xblock(self, mentoring, name): + return mentoring.find_element_by_css_selector(".xblock-v1[data-name='{}']".format(name)) + + def _get_choice(self, questionnaire, choice_index): + return questionnaire.find_elements_by_css_selector(".choices-list .choice")[choice_index] + + def _get_messages_element(self, mentoring): + return mentoring.find_element_by_css_selector('.messages') + + def _get_controls(self, mentoring): + answer = self._get_xblock(mentoring, "feedback_answer_1").find_element_by_css_selector('.answer') + mcq = self._get_xblock(mentoring, "feedback_mcq_2") + mrq = self._get_xblock(mentoring, "feedback_mrq_3") + rating = self._get_xblock(mentoring, "feedback_rating_4") + + return answer, mcq, mrq, rating + + def _assert_checkmark(self, checkmark, shown=True, checkmark_class=None): + choice_result_classes = checkmark.get_attribute('class').split() + if shown: + self.assertTrue(checkmark.is_displayed()) + self.assertIn(checkmark_class, choice_result_classes) + else: + self.assertFalse(checkmark.is_displayed()) + + def _assert_feedback_showed(self, questionnaire, choice_index, expected_text, + click_choice_result=False, success=True): + """ + Asserts that feedback for given element contains particular text + If `click_choice_result` is True - clicks on `choice-result` icon before checking feedback visibility: + MRQ feedbacks are not shown right away + """ + choice = self._get_choice(questionnaire, choice_index) + choice_result = choice.find_element_by_css_selector('.choice-result') + if click_choice_result: + choice_result.click() + + feedback_popup = choice.find_element_by_css_selector(".choice-tips") + checkmark_class = 'checkmark-correct' if success else 'checkmark-incorrect' + self._assert_checkmark(choice_result, shown=True, checkmark_class=checkmark_class) + self.assertTrue(feedback_popup.is_displayed()) + self.assertEqual(feedback_popup.text, expected_text) + + def _assert_feedback_hidden(self, questionnaire, choice_index): + choice = self._get_choice(questionnaire, choice_index) + choice_result = choice.find_element_by_css_selector('.choice-result') + feedback_popup = choice.find_element_by_css_selector(".choice-tips") + choice_result_classes = choice_result.get_attribute('class').split() + + self.assertTrue(choice_result.is_displayed()) + self.assertFalse(feedback_popup.is_displayed()) + self.assertNotIn('checkmark-correct', choice_result_classes) + self.assertNotIn('checkmark-incorrect', choice_result_classes) + + def _standard_filling(self, answer, mcq, mrq, rating): + answer.send_keys('This is the answer') + self.click_choice(mcq, "Yes") + # 1st, 3rd and 4th options, first three are correct, i.e. two mistakes: 2nd and 4th + self.click_choice(mrq, "Its elegance") + self.click_choice(mrq, "Its gracefulness") + self.click_choice(mrq, "Its bugs") + self.click_choice(rating, "4") + + # mcq and rating can't be reset easily, but it's not required; listing them here to keep method signature similar + def _clear_filling(self, answer, mcq, mrq, rating): # pylint: disable=unused-argument + answer.clear() + for checkbox in mrq.find_elements_by_css_selector('.choice input'): + if checkbox.is_selected(): + checkbox.click() + + def _standard_checks(self, answer, mcq, mrq, rating, messages, only_selected=False): + self.assertEqual(answer.get_attribute('value'), 'This is the answer') + self._assert_feedback_showed(mcq, 0, "Great!") + self._assert_feedback_showed( + mrq, 0, "This is something everyone has to like about this MRQ", + click_choice_result=True + ) + if not only_selected: + self._assert_feedback_showed( + mrq, 1, "This is something everyone has to like about beauty", + click_choice_result=True, success=False + ) + else: + self._assert_feedback_hidden(mrq, 1) + self._assert_feedback_showed(mrq, 2, "This MRQ is indeed very graceful", click_choice_result=True) + self._assert_feedback_showed(mrq, 3, "Nah, there aren't any!", click_choice_result=True, success=False) + self._assert_feedback_showed(rating, 3, "I love good grades.", click_choice_result=True) + self.assertTrue(messages.is_displayed()) + self.assertEqual(messages.text, "FEEDBACK\nNot done yet") + + def test_feedbacks_and_messages_is_not_shown_on_first_load(self): + mentoring = self.load_scenario("feedback_persistence.xml") + answer, mcq, mrq, rating = self._get_controls(mentoring) + messages = self._get_messages_element(mentoring) + + answer_checkmark = answer.find_element_by_xpath("parent::*").find_element_by_css_selector(".answer-checkmark") + + self._assert_checkmark(answer_checkmark, shown=False) + for i in range(3): + self._assert_feedback_hidden(mcq, i) + for i in range(4): + self._assert_feedback_hidden(mrq, i) + for i in range(5): + self._assert_feedback_hidden(rating, i) + self.assertFalse(messages.is_displayed()) + + def test_persists_feedback_on_page_reload(self): + mentoring = self.load_scenario("feedback_persistence.xml") + answer, mcq, mrq, rating = self._get_controls(mentoring) + messages = self._get_messages_element(mentoring) + + self._standard_filling(answer, mcq, mrq, rating) + self.click_submit(mentoring) + self._standard_checks(answer, mcq, mrq, rating, messages) + + # now, reload the page and do the same checks again + mentoring = self.go_to_view("student_view") + answer, mcq, mrq, rating = self._get_controls(mentoring) + messages = self._get_messages_element(mentoring) + self._standard_checks(answer, mcq, mrq, rating, messages, only_selected=True) + + def test_given_perfect_score_in_past_loads_current_result(self): + mentoring = self.load_scenario("feedback_persistence.xml") + answer, mcq, mrq, rating = self._get_controls(mentoring) + messages = self._get_messages_element(mentoring) + + answer.send_keys('This is the answer') + self.click_choice(mcq, "Yes") + # 1st, 3rd and 4th options, first three are correct, i.e. two mistakes: 2nd and 4th + self.click_choice(mrq, "Its elegance") + self.click_choice(mrq, "Its gracefulness") + self.click_choice(mrq, "Its beauty") + self.click_choice(rating, "4") + self.click_submit(mentoring) + + # precondition - verifying 100% score achieved + self.assertEqual(answer.get_attribute('value'), 'This is the answer') + self._assert_feedback_showed(mcq, 0, "Great!") + self._assert_feedback_showed( + mrq, 0, "This is something everyone has to like about this MRQ", + click_choice_result=True + ) + self._assert_feedback_showed( + mrq, 1, "This is something everyone has to like about beauty", + click_choice_result=True + ) + self._assert_feedback_showed(mrq, 2, "This MRQ is indeed very graceful", click_choice_result=True) + self._assert_feedback_showed(mrq, 3, "Nah, there aren't any!", click_choice_result=True) + self._assert_feedback_showed(rating, 3, "I love good grades.", click_choice_result=True) + self.assertTrue(messages.is_displayed()) + self.assertEqual(messages.text, "FEEDBACK\nAll Good") + + self._clear_filling(answer, mcq, mrq, rating) + self._standard_filling(answer, mcq, mrq, rating) + self.click_submit(mentoring) + self._standard_checks(answer, mcq, mrq, rating, messages) + + # now, reload the page and make sure LATEST submission is loaded and feedback is shown + mentoring = self.go_to_view("student_view") + answer, mcq, mrq, rating = self._get_controls(mentoring) + messages = self._get_messages_element(mentoring) + self._standard_checks(answer, mcq, mrq, rating, messages, only_selected=True) diff --git a/problem_builder/tests/integration/test_messages.py b/problem_builder/tests/integration/test_messages.py index 4bdfa37c..ebddb079 100644 --- a/problem_builder/tests/integration/test_messages.py +++ b/problem_builder/tests/integration/test_messages.py @@ -50,13 +50,6 @@ def expect_message(self, msg_type, mentoring): message_text = message_text[8:].lstrip() self.assertEqual(MESSAGES[msg_type], message_text) - def click_choice(self, container, choice_text): - """ Click on the choice label with the specified text """ - for label in container.find_elements_by_css_selector('.choices .choice label'): - if choice_text in label.text: - label.click() - break - @ddt.data( ("One", COMPLETED), ("Two", COMPLETED), diff --git a/problem_builder/tests/integration/xml_templates/feedback_persistence.xml b/problem_builder/tests/integration/xml_templates/feedback_persistence.xml new file mode 100644 index 00000000..36352585 --- /dev/null +++ b/problem_builder/tests/integration/xml_templates/feedback_persistence.xml @@ -0,0 +1,38 @@ + + + + + + Yes + Maybe not + I don't understand + + Great! + Ah, damn. +
Really?
+
+ + + Its elegance + Its beauty + Its gracefulness + Its bugs + + This is something everyone has to like about this MRQ + This is something everyone has to like about beauty + This MRQ is indeed very graceful + Nah, there aren't any! + + + + I don't want to rate it + + I love good grades. + Will do better next time... + Your loss! + + + All Good + Not done yet +
+
diff --git a/run_tests.py b/run_tests.py index c4b17c38..919fd2dd 100755 --- a/run_tests.py +++ b/run_tests.py @@ -10,6 +10,14 @@ import os import sys +import logging + +logging_level_overrides = { + 'workbench.views': logging.ERROR, + 'django.request': logging.ERROR, + 'workbench.runtime': logging.ERROR, +} + if __name__ == "__main__": # Use the workbench settings file: os.environ.setdefault("DJANGO_SETTINGS_MODULE", "workbench.settings") @@ -19,6 +27,9 @@ from django.conf import settings settings.INSTALLED_APPS += ("problem_builder", ) + for noisy_logger, log_level in logging_level_overrides.iteritems(): + logging.getLogger(noisy_logger).setLevel(log_level) + from django.core.management import execute_from_command_line args = sys.argv[1:] paths = [arg for arg in args if arg[0] != '-']