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
48 changes: 31 additions & 17 deletions problem_builder/mentoring.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand All @@ -63,6 +64,10 @@
'locations': ['public/themes/lms.css']
Copy link
Member

Choose a reason for hiding this comment

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

@itsjeyd Is this _default_theme_config being used? Looks to me like with ThemableXBlockMixin you need to define default_theme_config as an attribute on the class instead.

}

_default_options_config = {
'pb_mcq_hide_previous_answer': False
}


# Make '_' a no-op so we can scrape strings
def _(text):
Expand All @@ -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.
Expand Down Expand Up @@ -121,6 +127,7 @@ class BaseMentoringBlock(
icon_class = 'problem'
block_settings_key = 'mentoring'
theme_key = 'theme'
options_key = 'options'

@property
def url_name(self):
Expand Down Expand Up @@ -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=''):
Expand Down Expand Up @@ -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()

Expand All @@ -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
Expand All @@ -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):
Expand Down
4 changes: 3 additions & 1 deletion problem_builder/public/js/mentoring.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
});
Copy link
Member

Choose a reason for hiding this comment

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

In case you're wondering why you had to make this change: https://openedx.atlassian.net/browse/TNL-4161

I believe the earlier version is "better" but it doesn't work with the latest edx-platform master. The new version you have here should always work, so we may as well keep this change.

var step = data.step;

var mentoring = {
Expand Down
37 changes: 20 additions & 17 deletions problem_builder/public/js/questionnaire.js
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ function MCQBlock(runtime, element) {
var mentoring = this.mentoring;

var messageView = MessageView(element, mentoring);

if (result.message) {
var msg = '<div class="message-content">' + result.message + '</div>' +
'<div class="close icon-remove-sign fa-times-circle"></div>';
Expand All @@ -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)) {
Expand Down
57 changes: 57 additions & 0 deletions problem_builder/tests/integration/test_mentoring.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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 + '/')
Expand Down Expand Up @@ -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)
Expand Down
6 changes: 3 additions & 3 deletions problem_builder/tests/integration/test_titles.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ class StepTitlesTest(SeleniumXBlockTest):
mcq_template = """
<problem-builder mode="{{mode}}">
<pb-mcq name="mcq_1_1" question="Who was your favorite character?"
correct_choices="gaius,adama,starbuck,roslin,six,lee"
correct_choices="[gaius,adama,starbuck,roslin,six,lee]"
{display_name_attr} {show_title_attr}
>
<pb-choice value="gaius">Gaius Baltar</pb-choice>
Expand All @@ -90,7 +90,7 @@ class StepTitlesTest(SeleniumXBlockTest):
mrq_template = """
<problem-builder mode="{{mode}}">
<pb-mrq name="mrq_1_1" question="What makes a great MRQ?"
ignored_choices="1,2,3"
ignored_choices="[1,2,3]"
{display_name_attr} {show_title_attr}
>
<pb-choice value="1">Lots of choices</pb-choice>
Expand All @@ -103,7 +103,7 @@ class StepTitlesTest(SeleniumXBlockTest):
rating_template = """
<problem-builder mode="{{mode}}">
<pb-rating name="rating_1_1" question="How do you rate Battlestar Galactica?"
correct_choices="5,6"
correct_choices="[5,6]"
{display_name_attr} {show_title_attr}
>
<pb-choice value="6">More than 5 stars</pb-choice>
Expand Down
Loading