From f13d337a73f080c321733b348f86ae061aa5ccef Mon Sep 17 00:00:00 2001 From: "E. Kolpakov" Date: Thu, 7 May 2015 15:12:41 +0300 Subject: [PATCH 1/6] Suppressed noisy loggers --- run_tests.py | 11 +++++++++++ 1 file changed, 11 insertions(+) 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] != '-'] From 395c40348c1b6c7e5831195a0ea7d87546d5c86a Mon Sep 17 00:00:00 2001 From: "E. Kolpakov" Date: Wed, 6 May 2015 18:00:33 +0300 Subject: [PATCH 2/6] Preserving feedback messages and tips on page reload for standard mode --- problem_builder/mentoring.py | 73 ++++++++++++++----- .../public/js/mentoring_standard_view.js | 6 ++ 2 files changed, 62 insertions(+), 17 deletions(-) diff --git a/problem_builder/mentoring.py b/problem_builder/mentoring.py index c0a9f726..5c335ced 100644 --- a/problem_builder/mentoring.py +++ b/problem_builder/mentoring.py @@ -435,6 +435,61 @@ 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, show_message = True, False + choices = dict(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) + if child.name and child.name in choices: + show_message = True + child_result = child.get_results(choices[child.name]) + results.append([child.name, child_result]) + completed = completed and (child_result['status'] == 'correct') + else: + completed = False + + 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 +499,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 +513,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/public/js/mentoring_standard_view.js b/problem_builder/public/js/mentoring_standard_view.js index c431a276..91137c1e 100644 --- a/problem_builder/public/js/mentoring_standard_view.js +++ b/problem_builder/public/js/mentoring_standard_view.js @@ -66,6 +66,10 @@ function MentoringStandardView(runtime, element, mentoring) { submitXHR = $.post(handlerUrl, JSON.stringify(data)).success(handleSubmitResults).error(handleSubmitError); } + function get_results(){ + calculate_results('get_results'); + } + function submit() { calculate_results('submit'); } @@ -97,6 +101,8 @@ function MentoringStandardView(runtime, element, mentoring) { mentoring.initChildren(options); mentoring.renderDependency(); + get_results(); + var submitPossible = submitDOM.length > 0; if (submitPossible) { mentoring.renderAttempts(); From 40ff34ff3d59d19b133683c018113633c3ad0a13 Mon Sep 17 00:00:00 2001 From: "E. Kolpakov" Date: Thu, 7 May 2015 15:12:28 +0300 Subject: [PATCH 3/6] Restoring feedback on reload for stadnard-mode mentoring + tests --- .../public/js/mentoring_standard_view.js | 15 ++++---- .../tests/integration/base_test.py | 7 ++++ .../tests/integration/test_messages.py | 7 ---- .../tests/integration/test_questionnaire.py | 34 ++++++++++++++++++- 4 files changed, 48 insertions(+), 15 deletions(-) diff --git a/problem_builder/public/js/mentoring_standard_view.js b/problem_builder/public/js/mentoring_standard_view.js index 91137c1e..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,14 +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'); + calculate_results('get_results', false); } function submit() { - calculate_results('submit'); + calculate_results('submit', true); } function clearResults() { diff --git a/problem_builder/tests/integration/base_test.py b/problem_builder/tests/integration/base_test.py index 70616426..9307ff9a 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('.choices .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_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/test_questionnaire.py b/problem_builder/tests/integration/test_questionnaire.py index aabdba6b..0cda112f 100644 --- a/problem_builder/tests/integration/test_questionnaire.py +++ b/problem_builder/tests/integration/test_questionnaire.py @@ -24,7 +24,7 @@ from mock import patch, Mock from problem_builder import MentoringBlock -from .base_test import MentoringBaseTest +from .base_test import MentoringBaseTest, ProblemBuilderBaseTest # Classes ########################################################### @@ -292,3 +292,35 @@ class QuestionnaireBlockAprosThemeTest(QuestionnaireBlockTest): Test MRQ/MCQ questions without the LMS theme which is on by default. """ pass + + +@ddt.ddt +class ProblemBuilderQuestionnaireBlockTest(ProblemBuilderBaseTest): + def _get_choice_feedback_popup(self, mentoring, choice_index): + choices = mentoring.find_elements_by_css_selector(".choices-list .choice") + target_choice = choices[choice_index] + + return target_choice.find_element_by_css_selector(".choice-tips") + + def _get_messages_element(self, mentoring): + return mentoring.find_element_by_css_selector('.messages') + + @ddt.data(("One", 0), ("Two", 1)) + @ddt.unpack + def test_persists_feedback_on_page_reload(self, choice_value, choice_index): + mentoring = self.load_scenario("messages.xml", {"max_attempts": 1}) + + self.click_choice(mentoring, choice_value) + self.click_submit(mentoring) + + feedback_popup = self._get_choice_feedback_popup(mentoring, choice_index) + messages = self._get_messages_element(mentoring) + self.assertTrue(feedback_popup.is_displayed()) + self.assertTrue(messages.is_displayed()) + + # now, reload the page + mentoring = self.go_to_view("student_view") + feedback_popup = self._get_choice_feedback_popup(mentoring, choice_index) + messages = self._get_messages_element(mentoring) + self.assertTrue(feedback_popup.is_displayed()) + self.assertTrue(messages.is_displayed()) From bfd829e97652015168f608717a8af29af5e47724 Mon Sep 17 00:00:00 2001 From: "E. Kolpakov" Date: Fri, 8 May 2015 15:04:18 +0300 Subject: [PATCH 4/6] Added tests for initial state (no feedback should be shown) and for perfect-score-in-the-past scenario --- .../tests/integration/base_test.py | 7 + .../tests/integration/test_mentoring.py | 156 +++++++++++++++++- .../tests/integration/test_questionnaire.py | 34 +--- .../xml_templates/feedback_persistence.xml | 38 +++++ 4 files changed, 201 insertions(+), 34 deletions(-) create mode 100644 problem_builder/tests/integration/xml_templates/feedback_persistence.xml diff --git a/problem_builder/tests/integration/base_test.py b/problem_builder/tests/integration/base_test.py index 9307ff9a..2aa370f4 100644 --- a/problem_builder/tests/integration/base_test.py +++ b/problem_builder/tests/integration/base_test.py @@ -92,6 +92,13 @@ def click_choice(self, container, choice_text): label.click() break + 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..a280e3f8 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,157 @@ 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_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 _assert_feedback_visible(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") + self.assertTrue(choice_result.is_displayed()) + self.assertTrue(feedback_popup.is_displayed()) + self.assertEqual(feedback_popup.text, expected_text) + choice_result_classes = choice_result.get_attribute('class').split() + self.assertIn('checkmark-correct' if success else '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): + self.assertEqual(answer.get_attribute('value'), 'This is the answer') + self._assert_feedback_visible(mcq, 0, "Great!") + self._assert_feedback_visible( + mrq, 0, "This is something everyone has to like about this MRQ", + click_choice_result=True + ) + self._assert_feedback_visible( + mrq, 1, "This is something everyone has to like about beauty", + click_choice_result=True, success=False + ) + self._assert_feedback_visible(mrq, 2, "This MRQ is indeed very graceful", click_choice_result=True) + self._assert_feedback_visible(mrq, 3, "Nah, there aren't any!", click_choice_result=True, success=False) + self._assert_feedback_visible(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) + + 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) + + 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_visible(mcq, 0, "Great!") + self._assert_feedback_visible( + mrq, 0, "This is something everyone has to like about this MRQ", + click_choice_result=True + ) + self._assert_feedback_visible( + mrq, 1, "This is something everyone has to like about beauty", + click_choice_result=True + ) + self._assert_feedback_visible(mrq, 2, "This MRQ is indeed very graceful", click_choice_result=True) + self._assert_feedback_visible(mrq, 3, "Nah, there aren't any!", click_choice_result=True) + self._assert_feedback_visible(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) \ No newline at end of file diff --git a/problem_builder/tests/integration/test_questionnaire.py b/problem_builder/tests/integration/test_questionnaire.py index 0cda112f..aabdba6b 100644 --- a/problem_builder/tests/integration/test_questionnaire.py +++ b/problem_builder/tests/integration/test_questionnaire.py @@ -24,7 +24,7 @@ from mock import patch, Mock from problem_builder import MentoringBlock -from .base_test import MentoringBaseTest, ProblemBuilderBaseTest +from .base_test import MentoringBaseTest # Classes ########################################################### @@ -292,35 +292,3 @@ class QuestionnaireBlockAprosThemeTest(QuestionnaireBlockTest): Test MRQ/MCQ questions without the LMS theme which is on by default. """ pass - - -@ddt.ddt -class ProblemBuilderQuestionnaireBlockTest(ProblemBuilderBaseTest): - def _get_choice_feedback_popup(self, mentoring, choice_index): - choices = mentoring.find_elements_by_css_selector(".choices-list .choice") - target_choice = choices[choice_index] - - return target_choice.find_element_by_css_selector(".choice-tips") - - def _get_messages_element(self, mentoring): - return mentoring.find_element_by_css_selector('.messages') - - @ddt.data(("One", 0), ("Two", 1)) - @ddt.unpack - def test_persists_feedback_on_page_reload(self, choice_value, choice_index): - mentoring = self.load_scenario("messages.xml", {"max_attempts": 1}) - - self.click_choice(mentoring, choice_value) - self.click_submit(mentoring) - - feedback_popup = self._get_choice_feedback_popup(mentoring, choice_index) - messages = self._get_messages_element(mentoring) - self.assertTrue(feedback_popup.is_displayed()) - self.assertTrue(messages.is_displayed()) - - # now, reload the page - mentoring = self.go_to_view("student_view") - feedback_popup = self._get_choice_feedback_popup(mentoring, choice_index) - messages = self._get_messages_element(mentoring) - self.assertTrue(feedback_popup.is_displayed()) - self.assertTrue(messages.is_displayed()) 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 + + From 742c22389df3812ac5e3be9bb2435e2d82b4a8d5 Mon Sep 17 00:00:00 2001 From: "E. Kolpakov" Date: Fri, 8 May 2015 16:22:55 +0300 Subject: [PATCH 5/6] Showing feedback for last submission --- problem_builder/answer.py | 3 +++ problem_builder/mcq.py | 3 +++ problem_builder/mentoring.py | 14 +++++--------- problem_builder/mrq.py | 3 +++ problem_builder/tests/integration/base_test.py | 7 ------- .../tests/integration/test_mentoring.py | 4 ++-- 6 files changed, 16 insertions(+), 18 deletions(-) diff --git a/problem_builder/answer.py b/problem_builder/answer.py index 0e772de5..b8a1e6af 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) + 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 5c335ced..4b088505 100644 --- a/problem_builder/mentoring.py +++ b/problem_builder/mentoring.py @@ -470,19 +470,15 @@ def _get_standard_results(self): Gets previous submissions results as if submit was called with exactly the same values as last time. """ results = [] - completed, show_message = True, False - choices = dict(self.student_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) - if child.name and child.name in choices: - show_message = True - child_result = child.get_results(choices[child.name]) - results.append([child.name, child_result]) - completed = completed and (child_result['status'] == 'correct') - else: - completed = False + 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 diff --git a/problem_builder/mrq.py b/problem_builder/mrq.py index 1d8fad59..8e9175d2 100644 --- a/problem_builder/mrq.py +++ b/problem_builder/mrq.py @@ -89,6 +89,9 @@ def get_results(self, previous_result): result['completed'] = True return result + def get_last_result(self): + return self.get_results({'submissions': self.student_choices}) if self.student_choices else {} + def submit(self, submissions): log.debug(u'Received MRQ submissions: "%s"', submissions) diff --git a/problem_builder/tests/integration/base_test.py b/problem_builder/tests/integration/base_test.py index 2aa370f4..059af9c9 100644 --- a/problem_builder/tests/integration/base_test.py +++ b/problem_builder/tests/integration/base_test.py @@ -85,13 +85,6 @@ 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('.choices .choice label'): - if choice_text in label.text: - label.click() - break - 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'): diff --git a/problem_builder/tests/integration/test_mentoring.py b/problem_builder/tests/integration/test_mentoring.py index a280e3f8..f2740679 100644 --- a/problem_builder/tests/integration/test_mentoring.py +++ b/problem_builder/tests/integration/test_mentoring.py @@ -159,7 +159,7 @@ def _standard_checks(self, answer, mcq, mrq, rating, messages): 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) + _, mcq, mrq, rating = self._get_controls(mentoring) messages = self._get_messages_element(mentoring) for i in range(3): @@ -225,4 +225,4 @@ def test_given_perfect_score_in_past_loads_current_result(self): 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) \ No newline at end of file + self._standard_checks(answer, mcq, mrq, rating, messages) From 43ba649b381d1bd41a6e2e7fb7ca7a1fb7724107 Mon Sep 17 00:00:00 2001 From: "E. Kolpakov" Date: Mon, 11 May 2015 15:48:08 +0300 Subject: [PATCH 6/6] Fixed showing feedback for all MRQ options on page reload and marking answer blocks as incomplete on first load (+tests) --- problem_builder/answer.py | 2 +- problem_builder/mrq.py | 15 +++- problem_builder/public/js/answer.js | 13 +-- .../tests/integration/test_mentoring.py | 82 +++++++++++-------- 4 files changed, 66 insertions(+), 46 deletions(-) diff --git a/problem_builder/answer.py b/problem_builder/answer.py index b8a1e6af..0acd2b61 100644 --- a/problem_builder/answer.py +++ b/problem_builder/answer.py @@ -190,7 +190,7 @@ def get_results(self, previous_response=None): } def get_last_result(self): - return self.get_results(None) + return self.get_results(None) if self.student_input else {} def submit(self, submission): """ diff --git a/problem_builder/mrq.py b/problem_builder/mrq.py index 8e9175d2..f0b88a14 100644 --- a/problem_builder/mrq.py +++ b/problem_builder/mrq.py @@ -81,16 +81,19 @@ 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): - return self.get_results({'submissions': self.student_choices}) if self.student_choices else {} + 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) @@ -101,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/tests/integration/test_mentoring.py b/problem_builder/tests/integration/test_mentoring.py index f2740679..7174db50 100644 --- a/problem_builder/tests/integration/test_mentoring.py +++ b/problem_builder/tests/integration/test_mentoring.py @@ -93,20 +93,16 @@ def _get_controls(self, mentoring): return answer, mcq, mrq, rating - 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 _assert_feedback_visible(self, questionnaire, choice_index, expected_text, - click_choice_result=False, success=True): + 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: @@ -118,11 +114,21 @@ def _assert_feedback_visible(self, questionnaire, choice_index, expected_text, choice_result.click() feedback_popup = choice.find_element_by_css_selector(".choice-tips") - self.assertTrue(choice_result.is_displayed()) + 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.assertIn('checkmark-correct' if success else 'checkmark-incorrect', choice_result_classes) + + 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') @@ -140,28 +146,34 @@ def _clear_filling(self, answer, mcq, mrq, rating): # pylint: disable=unuse if checkbox.is_selected(): checkbox.click() - def _standard_checks(self, answer, mcq, mrq, rating, messages): + 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_visible(mcq, 0, "Great!") - self._assert_feedback_visible( + 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_visible( - mrq, 1, "This is something everyone has to like about beauty", - click_choice_result=True, success=False - ) - self._assert_feedback_visible(mrq, 2, "This MRQ is indeed very graceful", click_choice_result=True) - self._assert_feedback_visible(mrq, 3, "Nah, there aren't any!", click_choice_result=True, success=False) - self._assert_feedback_visible(rating, 3, "I love good grades.", 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") - _, mcq, mrq, rating = self._get_controls(mentoring) + 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): @@ -183,7 +195,7 @@ def test_persists_feedback_on_page_reload(self): 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) + 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") @@ -201,18 +213,18 @@ def test_given_perfect_score_in_past_loads_current_result(self): # precondition - verifying 100% score achieved self.assertEqual(answer.get_attribute('value'), 'This is the answer') - self._assert_feedback_visible(mcq, 0, "Great!") - self._assert_feedback_visible( + 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_visible( + self._assert_feedback_showed( mrq, 1, "This is something everyone has to like about beauty", click_choice_result=True ) - self._assert_feedback_visible(mrq, 2, "This MRQ is indeed very graceful", click_choice_result=True) - self._assert_feedback_visible(mrq, 3, "Nah, there aren't any!", click_choice_result=True) - self._assert_feedback_visible(rating, 3, "I love good grades.", 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") @@ -225,4 +237,4 @@ def test_given_perfect_score_in_past_loads_current_result(self): 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) + self._standard_checks(answer, mcq, mrq, rating, messages, only_selected=True)