diff --git a/problem_builder/mentoring.py b/problem_builder/mentoring.py index 3c11826b..8330c6b8 100644 --- a/problem_builder/mentoring.py +++ b/problem_builder/mentoring.py @@ -42,6 +42,7 @@ from xblockutils.helpers import child_isinstance from xblockutils.resources import ResourceLoader +from xblockutils.settings import XBlockWithSettingsMixin, ThemableXBlockMixin from xblockutils.studio_editable import ( NestedXBlockSpec, StudioEditableXBlockMixin, StudioContainerXBlockMixin, StudioContainerWithNestedXBlocksMixin, ) @@ -63,6 +64,10 @@ 'locations': ['public/themes/lms.css'] } +_default_options_config = { + 'pb_mcq_hide_previous_answer': False +} + # Make '_' a no-op so we can scrape strings def _(text): @@ -80,7 +85,8 @@ def _(text): @XBlock.needs("i18n") @XBlock.wants('settings') class BaseMentoringBlock( - XBlock, XBlockWithTranslationServiceMixin, StudioEditableXBlockMixin, MessageParentMixin + XBlock, XBlockWithTranslationServiceMixin, XBlockWithSettingsMixin, + StudioEditableXBlockMixin, ThemableXBlockMixin, MessageParentMixin, ): """ An XBlock that defines functionality shared by mentoring blocks. @@ -121,6 +127,7 @@ class BaseMentoringBlock( icon_class = 'problem' block_settings_key = 'mentoring' theme_key = 'theme' + options_key = 'options' @property def url_name(self): @@ -156,24 +163,23 @@ def get_content_titles(self): return [self.display_name] return [] - def get_theme(self): + def get_options(self): + """ + Get options settings for this block from settings service. + + Fall back on default options if xblock settings have not been customized at all + or no customizations for options available. """ - Gets theme settings from settings service. Falls back to default (LMS) theme - if settings service is not available, xblock theme settings are not set or does - contain mentoring theme settings. + xblock_settings = self.get_xblock_settings(_default_options_config) + if xblock_settings and self.options_key in xblock_settings: + return xblock_settings[self.options_key] + return _default_options_config + + def get_option(self, option): + """ + Get value of a specific instance-wide `option`. """ - settings_service = self.runtime.service(self, "settings") - if settings_service: - xblock_settings = settings_service.get_settings_bucket(self) - if xblock_settings and self.theme_key in xblock_settings: - return xblock_settings[self.theme_key] - return _default_theme_config - - def include_theme_files(self, fragment): - theme = self.get_theme() - theme_package, theme_files = theme['package'], theme['locations'] - for theme_file in theme_files: - fragment.add_css(ResourceLoader(theme_package).load_unicode(theme_file)) + return self.get_options()[option] @XBlock.json_handler def view(self, data, suffix=''): @@ -368,6 +374,8 @@ 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 + # Migrate stored data if necessary self.migrate_fields() @@ -379,6 +387,8 @@ def student_view(self, context): fragment = Fragment() child_content = u"" + mcq_hide_previous_answer = self.get_option('pb_mcq_hide_previous_answer') + for child_id in self.children: child = self.runtime.get_block(child_id) if child is None: # child should not be None but it can happen due to bugs or permission issues @@ -388,6 +398,10 @@ 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): + context['hide_prev_answer'] = True + else: + context['hide_prev_answer'] = False child_fragment = child.render('mentoring_view', context) except NoSuchViewError: if child.scope_ids.block_type == 'html' and getattr(self.runtime, 'is_author_mode', False): diff --git a/problem_builder/public/js/mentoring.js b/problem_builder/public/js/mentoring.js index 04840e0c..3eb47f66 100644 --- a/problem_builder/public/js/mentoring.js +++ b/problem_builder/public/js/mentoring.js @@ -8,7 +8,9 @@ function MentoringBlock(runtime, element) { var attemptsTemplate = _.template($('#xblock-attempts-template').html()); var data = $('.mentoring', element).data(); var children = runtime.children(element); - var steps = runtime.children(element).filter(function(c) { return c.element.className.indexOf('assessment_step_view') > -1; }); + var steps = runtime.children(element).filter(function(c) { + return $(c.element).attr("class").indexOf('assessment_step_view') > -1; + }); var step = data.step; var mentoring = { diff --git a/problem_builder/public/js/questionnaire.js b/problem_builder/public/js/questionnaire.js index 4a4a8b27..e0cb4827 100644 --- a/problem_builder/public/js/questionnaire.js +++ b/problem_builder/public/js/questionnaire.js @@ -122,7 +122,7 @@ function MCQBlock(runtime, element) { var mentoring = this.mentoring; var messageView = MessageView(element, mentoring); - + if (result.message) { var msg = '
' + result.message + '
' + '
'; @@ -138,24 +138,27 @@ function MCQBlock(runtime, element) { var choiceResultDOM = $('.choice-result', choiceDOM); var choiceTipsDOM = $('.choice-tips', choiceDOM); - if (result.status === "correct" && choiceInputDOM.val() === result.submission) { - choiceDOM.addClass('correct'); - choiceResultDOM.addClass('checkmark-correct icon-ok fa-check'); - } - else if (choiceInputDOM.val() === result.submission || _.isNull(result.submission)) { - choiceDOM.addClass('incorrect'); - choiceResultDOM.addClass('checkmark-incorrect icon-exclamation fa-exclamation'); - } - - if (result.tips && choiceInputDOM.val() === result.submission) { - mentoring.setContent(choiceTipsDOM, result.tips); - } + if (choiceInputDOM.prop('checked')) { // We're showing previous answers, + // so go ahead and display results as well + if (result.status === "correct" && choiceInputDOM.val() === result.submission) { + choiceDOM.addClass('correct'); + choiceResultDOM.addClass('checkmark-correct icon-ok fa-check'); + } + else if (choiceInputDOM.val() === result.submission || _.isNull(result.submission)) { + choiceDOM.addClass('incorrect'); + choiceResultDOM.addClass('checkmark-incorrect icon-exclamation fa-exclamation'); + } - choiceResultDOM.off('click').on('click', function() { - if (choiceTipsDOM.html() !== '') { - messageView.showMessage(choiceTipsDOM); + if (result.tips && choiceInputDOM.val() === result.submission) { + mentoring.setContent(choiceTipsDOM, result.tips); } - }); + + choiceResultDOM.off('click').on('click', function() { + if (choiceTipsDOM.html() !== '') { + messageView.showMessage(choiceTipsDOM); + } + }); + } }); if (_.isNull(result.submission)) { diff --git a/problem_builder/tests/integration/test_mentoring.py b/problem_builder/tests/integration/test_mentoring.py index de71e82a..1ec843a1 100644 --- a/problem_builder/tests/integration/test_mentoring.py +++ b/problem_builder/tests/integration/test_mentoring.py @@ -131,6 +131,11 @@ def _assert_feedback_hidden(self, questionnaire, choice_index): self.assertNotIn('checkmark-correct', choice_result_classes) self.assertNotIn('checkmark-incorrect', choice_result_classes) + def _assert_not_checked(self, questionnaire, choice_index): + choice = self._get_choice(questionnaire, choice_index) + choice_input = choice.find_element_by_css_selector('input') + self.assertFalse(choice_input.is_selected()) + def _standard_filling(self, answer, mcq, mrq, rating): answer.send_keys('This is the answer') self.click_choice(mcq, "Yes") @@ -164,6 +169,32 @@ def _standard_checks(self, answer, mcq, mrq, rating, messages): self.assertTrue(messages.is_displayed()) self.assertEqual(messages.text, "FEEDBACK\nNot done yet") + def _feedback_customized_checks(self, answer, mcq, mrq, rating, messages): + # Long answer: Previous answer and feedback visible + self.assertEqual(answer.get_attribute('value'), 'This is the answer') + # MCQ: Previous answer and feedback hidden + for i in range(3): + self._assert_feedback_hidden(mcq, i) + self._assert_not_checked(mcq, i) + # MRQ: Previous answer and feedback visible + 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, success=False + ) + 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) + # Rating: Previous answer and feedback hidden + for i in range(5): + self._assert_feedback_hidden(rating, i) + self._assert_not_checked(rating, i) + # Messages + self.assertTrue(messages.is_displayed()) + self.assertEqual(messages.text, "FEEDBACK\nNot done yet") + def reload_student_view(self): # Load another page (the home page), then go back to the page we want. This is the only reliable way to reload. self.browser.get(self.live_server_url + '/') @@ -218,6 +249,32 @@ def test_persists_feedback_on_page_reload(self): self.click_choice(mrq, "Its elegance") self.assertTrue(submit.is_enabled()) + def test_does_not_persist_mcq_feedback_on_page_reload_if_disabled(self): + with mock.patch("problem_builder.mentoring.MentoringBlock.get_options") as patched_options: + patched_options.return_value = {'pb_mcq_hide_previous_answer': True} + 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 see if previous answers and results for MCQs are hidden + mentoring = self.reload_student_view() + answer, mcq, mrq, rating = self._get_controls(mentoring) + messages = self._get_messages_element(mentoring) + submit = mentoring.find_element_by_css_selector('.submit input.input-main') + + self._feedback_customized_checks(answer, mcq, mrq, rating, messages) + # after reloading submit is disabled... + self.assertFalse(submit.is_enabled()) + + # ... until student answers MCQs again + self.click_choice(mcq, "Maybe not") + self.click_choice(rating, "2") + self.assertTrue(submit.is_enabled()) + 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) diff --git a/problem_builder/tests/integration/test_titles.py b/problem_builder/tests/integration/test_titles.py index 0d3fe297..34fbae1b 100644 --- a/problem_builder/tests/integration/test_titles.py +++ b/problem_builder/tests/integration/test_titles.py @@ -74,7 +74,7 @@ class StepTitlesTest(SeleniumXBlockTest): mcq_template = """ Gaius Baltar @@ -90,7 +90,7 @@ class StepTitlesTest(SeleniumXBlockTest): mrq_template = """ Lots of choices @@ -103,7 +103,7 @@ class StepTitlesTest(SeleniumXBlockTest): rating_template = """ More than 5 stars diff --git a/problem_builder/tests/unit/test_problem_builder.py b/problem_builder/tests/unit/test_problem_builder.py index 84d8d167..663a7309 100644 --- a/problem_builder/tests/unit/test_problem_builder.py +++ b/problem_builder/tests/unit/test_problem_builder.py @@ -1,9 +1,15 @@ -import unittest import ddt +import unittest + from mock import MagicMock, Mock, patch +from random import random + from xblock.field_data import DictFieldData + from problem_builder.mcq import MCQBlock -from problem_builder.mentoring import MentoringBlock, MentoringMessageBlock, _default_theme_config +from problem_builder.mentoring import ( + MentoringBlock, MentoringMessageBlock, _default_theme_config, _default_options_config +) @ddt.ddt @@ -63,7 +69,6 @@ def test_does_not_crash_when_get_child_is_broken(self): self.assertIn('Unable to load child component', fragment.content) -@ddt.ddt class TestMentoringBlockTheming(unittest.TestCase): def setUp(self): self.service_mock = Mock() @@ -71,76 +76,65 @@ def setUp(self): self.runtime_mock.service = Mock(return_value=self.service_mock) self.block = MentoringBlock(self.runtime_mock, DictFieldData({}), Mock()) - def test_theme_uses_default_theme_if_settings_service_is_not_available(self): - self.runtime_mock.service = Mock(return_value=None) - self.assertEqual(self.block.get_theme(), _default_theme_config) - - def test_theme_uses_default_theme_if_no_theme_is_set(self): - self.service_mock.get_settings_bucket = Mock(return_value=None) - self.assertEqual(self.block.get_theme(), _default_theme_config) - self.service_mock.get_settings_bucket.assert_called_once_with(self.block) - - @ddt.data(123, object()) - def test_theme_raises_if_theme_object_is_not_iterable(self, theme_config): - self.service_mock.get_settings_bucket = Mock(return_value=theme_config) - with self.assertRaises(TypeError): - self.block.get_theme() - self.service_mock.get_settings_bucket.assert_called_once_with(self.block) - - @ddt.data( - {}, {'mass': 123}, {'spin': {}}, {'parity': "1"} - ) - def test_theme_uses_default_theme_if_no_mentoring_theme_is_set_up(self, theme_config): - self.service_mock.get_settings_bucket = Mock(return_value=theme_config) - self.assertEqual(self.block.get_theme(), _default_theme_config) - self.service_mock.get_settings_bucket.assert_called_once_with(self.block) - - @ddt.data( - {MentoringBlock.theme_key: 123}, - {MentoringBlock.theme_key: [1, 2, 3]}, - {MentoringBlock.theme_key: {'package': 'qwerty', 'locations': ['something_else.css']}}, - ) - def test_theme_correctly_returns_configured_theme(self, theme_config): - self.service_mock.get_settings_bucket = Mock(return_value=theme_config) - self.assertEqual(self.block.get_theme(), theme_config[MentoringBlock.theme_key]) - - def test_theme_files_are_loaded_from_correct_package(self): - fragment = MagicMock() - package_name = 'some_package' - theme_config = {MentoringBlock.theme_key: {'package': package_name, 'locations': ['lms.css']}} - self.service_mock.get_settings_bucket = Mock(return_value=theme_config) - with patch("problem_builder.mentoring.ResourceLoader") as patched_resource_loader: - self.block.include_theme_files(fragment) - patched_resource_loader.assert_called_with(package_name) - - @ddt.data( - ('problem_builder', ['public/themes/lms.css']), - ('problem_builder', ['public/themes/lms.css', 'public/themes/lms.part2.css']), - ('my_app.my_rules', ['typography.css', 'icons.css']), - ) - @ddt.unpack - def test_theme_files_are_added_to_fragment(self, package_name, locations): - fragment = MagicMock() - theme_config = {MentoringBlock.theme_key: {'package': package_name, 'locations': locations}} - self.service_mock.get_settings_bucket = Mock(return_value=theme_config) - with patch("problem_builder.mentoring.ResourceLoader.load_unicode") as patched_load_unicode: - self.block.include_theme_files(fragment) - for location in locations: - patched_load_unicode.assert_any_call(location) - - self.assertEqual(patched_load_unicode.call_count, len(locations)) - def test_student_view_calls_include_theme_files(self): + self.service_mock.get_settings_bucket = Mock(return_value={}) with patch.object(self.block, 'include_theme_files') as patched_include_theme_files: fragment = self.block.student_view({}) patched_include_theme_files.assert_called_with(fragment) def test_author_preview_view_calls_include_theme_files(self): + self.service_mock.get_settings_bucket = Mock(return_value={}) with patch.object(self.block, 'include_theme_files') as patched_include_theme_files: fragment = self.block.author_preview_view({}) patched_include_theme_files.assert_called_with(fragment) +@ddt.ddt +class TestMentoringBlockOptions(unittest.TestCase): + def setUp(self): + self.service_mock = Mock() + self.runtime_mock = Mock() + self.runtime_mock.service = Mock(return_value=self.service_mock) + self.block = MentoringBlock(self.runtime_mock, DictFieldData({}), Mock()) + + def test_get_options_returns_default_if_xblock_settings_not_customized(self): + self.block.get_xblock_settings = Mock(return_value=None) + self.assertEqual(self.block.get_options(), _default_options_config) + self.block.get_xblock_settings.assert_called_once_with(_default_options_config) + + @ddt.data( + {}, {'mass': 123}, {'spin': {}}, {'parity': "1"} + ) + def test_get_options_returns_default_if_options_not_customized(self, xblock_settings): + self.block.get_xblock_settings = Mock(return_value=xblock_settings) + self.assertEqual(self.block.get_options(), _default_options_config) + self.block.get_xblock_settings.assert_called_once_with(_default_options_config) + + @ddt.data( + {MentoringBlock.options_key: 123}, + {MentoringBlock.options_key: [1, 2, 3]}, + {MentoringBlock.options_key: {'pb_mcq_hide_previous_answer': False}}, + ) + def test_get_options_correctly_returns_customized_options(self, xblock_settings): + self.block.get_xblock_settings = Mock(return_value=xblock_settings) + self.assertEqual(self.block.get_options(), xblock_settings[MentoringBlock.options_key]) + self.block.get_xblock_settings.assert_called_once_with(_default_options_config) + + def test_get_option(self): + random_key, random_value = random(), random() + with patch.object(self.block, 'get_options') as patched_get_options: + patched_get_options.return_value = {random_key: random_value} + option = self.block.get_option(random_key) + patched_get_options.assert_called_once_with() + self.assertEqual(option, random_value) + + def test_student_view_calls_get_option(self): + self.service_mock.get_settings_bucket = 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') + + class TestMentoringBlockJumpToIds(unittest.TestCase): def setUp(self): self.service_mock = Mock()