From 772c1f26281434a0ffaabb843ddcb3f4557d884d Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Fri, 16 Oct 2015 19:36:43 -0700 Subject: [PATCH 1/9] Initial/basic implementation of slider block --- problem_builder/answer.py | 7 - problem_builder/mixins.py | 16 +- .../public/css/problem-builder-edit.css | 4 + .../public/css/problem-builder.css | 26 +++ .../public/js/mentoring_standard_view.js | 10 +- problem_builder/public/js/slider.js | 41 +++++ problem_builder/questionnaire.py | 16 +- problem_builder/slider.py | 161 ++++++++++++++++++ problem_builder/step.py | 3 +- .../templates/html/mentoring_add_buttons.html | 1 + problem_builder/templates/html/slider.html | 17 ++ .../templates/html/slider_edit_footer.html | 5 + setup.py | 1 + 13 files changed, 280 insertions(+), 28 deletions(-) create mode 100644 problem_builder/public/js/slider.js create mode 100644 problem_builder/slider.py create mode 100644 problem_builder/templates/html/slider.html create mode 100644 problem_builder/templates/html/slider_edit_footer.html diff --git a/problem_builder/answer.py b/problem_builder/answer.py index 68ddf973..d2e15d9c 100644 --- a/problem_builder/answer.py +++ b/problem_builder/answer.py @@ -152,13 +152,6 @@ class AnswerBlock(SubmittingXBlockMixin, AnswerMixin, QuestionMixin, StudioEdita default="", multiline_editor=True, ) - weight = Float( - display_name=_("Weight"), - help=_("Defines the maximum total grade of the answer block."), - default=1, - scope=Scope.settings, - enforce_type=True - ) editable_fields = ('question', 'name', 'min_characters', 'weight', 'default_from', 'display_name', 'show_title') diff --git a/problem_builder/mixins.py b/problem_builder/mixins.py index 3513638a..b3aae058 100644 --- a/problem_builder/mixins.py +++ b/problem_builder/mixins.py @@ -1,5 +1,5 @@ from lazy import lazy -from xblock.fields import String, Boolean, Scope +from xblock.fields import String, Boolean, Float, Scope, UNIQUE_ID from xblockutils.helpers import child_isinstance from xblockutils.resources import ResourceLoader @@ -125,12 +125,26 @@ class QuestionMixin(EnumerableChildMixin): has_author_view = True # Fields: + name = String( + # This doesn't need to be a field but is kept for backwards compatibility with v1 student data + display_name=_("Question ID (name)"), + help=_("The ID of this question (required). Should be unique within this mentoring component."), + default=UNIQUE_ID, + scope=Scope.settings, # Must be scope.settings, or the unique ID will change every time this block is edited + ) display_name = String( display_name=_("Question title"), help=_('Leave blank to use the default ("Question 1", "Question 2", etc.)'), default="", # Blank will use 'Question x' - see display_name_with_default scope=Scope.content ) + weight = Float( + display_name=_("Weight"), + help=_("Defines the maximum total grade of this question."), + default=1, + scope=Scope.content, + enforce_type=True + ) @lazy def siblings(self): diff --git a/problem_builder/public/css/problem-builder-edit.css b/problem_builder/public/css/problem-builder-edit.css index 6d6c06a6..c1a0b3a3 100644 --- a/problem_builder/public/css/problem-builder-edit.css +++ b/problem_builder/public/css/problem-builder-edit.css @@ -49,3 +49,7 @@ margin-top: 1em; padding-top: 0.3em; } + +.xblock-author_view-pb-slider .url-name-footer { + margin: 0 -20px -20px -20px; /* Counteract spacing from xblock-render wrapper. */ +} diff --git a/problem_builder/public/css/problem-builder.css b/problem_builder/public/css/problem-builder.css index 41decf07..8417767a 100644 --- a/problem_builder/public/css/problem-builder.css +++ b/problem_builder/public/css/problem-builder.css @@ -64,6 +64,32 @@ margin-bottom: 0; } +.mentoring .xblock-pb-slider p label { + font-size: inherit; +} + +.mentoring .pb-slider-box { + max-width: 400px; +} + +.mentoring .pb-slider-range { + width: 100%; +} + +.mentoring .pb-slider-min-label { + float: left; +} + +.mentoring .pb-slider-max-label { + float: right; +} + +.mentoring .clearfix::after { + clear: both; + display: block; + content: " "; +} + .mentoring .attempts { margin-left: 10px; display: inline-block; diff --git a/problem_builder/public/js/mentoring_standard_view.js b/problem_builder/public/js/mentoring_standard_view.js index 7357d5ab..a484cb63 100644 --- a/problem_builder/public/js/mentoring_standard_view.js +++ b/problem_builder/public/js/mentoring_standard_view.js @@ -7,6 +7,7 @@ function MentoringStandardView(runtime, element, mentoring) { function handleSubmitResults(response, disable_submit) { messagesDOM.empty().hide(); + var all_have_results = response.results.length > 0; $.each(response.results || [], function(index, result_spec) { var input = result_spec[0]; var result = result_spec[1]; @@ -16,6 +17,7 @@ function MentoringStandardView(runtime, element, mentoring) { num_attempts: response.num_attempts }; callIfExists(child, 'handleSubmit', result, options); + all_have_results = all_have_results && !$.isEmptyObject(result); }); $('.attempts', element).data('max_attempts', response.max_attempts); @@ -29,10 +31,10 @@ function MentoringStandardView(runtime, element, mentoring) { messagesDOM.show(); } - // this method is called on successful submission and on page load - // results will be empty only for initial load if no submissions was made - // in such case we must allow submission to support submitting empty read-only long answer recaps - if (disable_submit || response.results.length > 0) { + // 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) { submitDOM.attr('disabled', 'disabled'); } } diff --git a/problem_builder/public/js/slider.js b/problem_builder/public/js/slider.js new file mode 100644 index 00000000..2c22bf2f --- /dev/null +++ b/problem_builder/public/js/slider.js @@ -0,0 +1,41 @@ +function SliderBlock(runtime, element) { + var $slider = $('.pb-slider-range', element); + return { + mode: null, + mentoring: null, + + value: function() { + return parseInt($slider.val()); + }, + + init: function(options) { + this.mentoring = options.mentoring; + this.mode = options.mode; + $slider.on('change', options.onChange); + }, + + submit: function() { + return this.value(); + }, + + handleReview: function(result){ + $slider.val(result.submission); + $slider.prop('disabled', true); + }, + + handleSubmit: function(result) { + // Show a green check if the user has submitted a valid value: + if (typeof result.submission !== "undefined") { + $('.submit-result', element).css('visibility', 'visible'); + } + }, + + clearResult: function() { + $('.submit-result', element).css('visibility', 'hidden'); + }, + + validate: function(){ + return Boolean(this.value() >= 0 && this.value() <= 100); + } + }; +} diff --git a/problem_builder/questionnaire.py b/problem_builder/questionnaire.py index 7e846809..a65b2f4b 100644 --- a/problem_builder/questionnaire.py +++ b/problem_builder/questionnaire.py @@ -24,7 +24,7 @@ from lazy import lazy import uuid from xblock.core import XBlock -from xblock.fields import Scope, String, Float, UNIQUE_ID +from xblock.fields import Scope, String, Float from xblock.fragment import Fragment from xblock.validation import ValidationMessage from xblockutils.helpers import child_isinstance @@ -61,13 +61,6 @@ class QuestionnaireAbstractBlock( values entered by the student, and supports multiple types of multiple-choice set, with preset choices and author-defined values. """ - name = String( - # This doesn't need to be a field but is kept for backwards compatibility with v1 student data - display_name=_("Question ID (name)"), - help=_("The ID of this question (required). Should be unique within this mentoring component."), - default=UNIQUE_ID, - scope=Scope.settings, # Must be scope.settings, or the unique ID will change every time this block is edited - ) question = String( display_name=_("Question"), help=_("Question to ask the student"), @@ -81,13 +74,6 @@ class QuestionnaireAbstractBlock( scope=Scope.content, default="" ) - weight = Float( - display_name=_("Weight"), - help=_("Defines the maximum total grade of this question."), - default=1, - scope=Scope.content, - enforce_type=True - ) editable_fields = ('question', 'message', 'weight', 'display_name', 'show_title') has_children = True answerable = True diff --git a/problem_builder/slider.py b/problem_builder/slider.py new file mode 100644 index 00000000..bbd745ac --- /dev/null +++ b/problem_builder/slider.py @@ -0,0 +1,161 @@ +# -*- coding: utf-8 -*- +# +# Copyright (c) 2014-2015 Harvard, edX & OpenCraft +# +# This software's license gives you freedom; you can copy, convey, +# propagate, redistribute and/or modify this program under the terms of +# the GNU Affero General Public License (AGPL) as published by the Free +# Software Foundation (FSF), either version 3 of the License, or (at your +# option) any later version of the AGPL published by the FSF. +# +# This program is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero +# General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program in a file in the toplevel directory called +# "AGPLv3". If not, see . +# + +# Imports ########################################################### + +import logging +import uuid + +from xblock.core import XBlock +from xblock.fields import Scope, String, Float +from xblock.fragment import Fragment +from xblock.validation import ValidationMessage +from xblockutils.studio_editable import StudioEditableXBlockMixin +from xblockutils.resources import ResourceLoader + +from .mixins import QuestionMixin, XBlockWithTranslationServiceMixin +from .sub_api import sub_api, SubmittingXBlockMixin + + +# Globals ########################################################### + +log = logging.getLogger(__name__) +loader = ResourceLoader(__name__) + + +# Make '_' a no-op so we can scrape strings +def _(text): + return text + +# Classes ########################################################### + + +@XBlock.needs("i18n") +class SliderBlock( + SubmittingXBlockMixin, QuestionMixin, StudioEditableXBlockMixin, XBlockWithTranslationServiceMixin, XBlock, +): + """ + An XBlock used to ask multiple-choice questions + """ + CATEGORY = 'pb-slider' + STUDIO_LABEL = _(u"Ranged Value Slider") + + min_label = String( + display_name=_("Low"), + help=_("Label for low end of the range"), + scope=Scope.content, + default=_("0%"), + ) + max_label = String( + display_name=_("High"), + help=_("Label for high end of the range"), + scope=Scope.content, + default=_("100%"), + ) + + question = String( + display_name=_("Question"), + help=_("Question to ask the student (optional)"), + scope=Scope.content, + default="", + multiline_editor=True, + ) + + student_value = Float( + # The value selected by the student + default=None, + scope=Scope.user_state, + ) + + editable_fields = ('min_label', 'max_label', 'display_name', 'question', 'show_title') + + def mentoring_view(self, context): + """ Main view of this block """ + context = context.copy() if context else {} + context['question'] = self.question + context['slider_id'] = 'pb-slider-{}'.format(uuid.uuid4().hex[:20]) + context['initial_value'] = int(self.student_value*100) if self.student_value is not None else 50 + context['min_label'] = self.min_label + context['max_label'] = self.max_label + context['hide_header'] = context.get('hide_header', False) or not self.show_title + context['instructions_string'] = self._("Select a value from {min_label} to {max_label}").format( + min_label=self.min_label, max_label=self.max_label + ) + html = loader.render_template('templates/html/slider.html', context) + + fragment = Fragment(html) + fragment.add_javascript_url(self.runtime.local_resource_url(self, 'public/js/slider.js')) + fragment.initialize_js('SliderBlock') + return fragment + + def student_view(self, context=None): + """ Normal view of this XBlock, identical to mentoring_view """ + return self.mentoring_view(context) + + def author_view(self, context): + """ + Add some HTML to the author view that allows authors to see the ID of the block, so they + can refer to it in other blocks such as Plot blocks. + """ + fragment = self.student_view(context) + fragment.add_content(loader.render_template('templates/html/slider_edit_footer.html', { + "url_name": self.url_name + })) + return fragment + + def get_last_result(self): + """ Return the current/last result in the required format """ + if not self.student_value: + return {} + return { + 'submission': self.student_value, + 'status': 'correct', + 'tips': [], + 'weight': self.weight, + 'score': 1, + } + + def submit(self, value): + log.debug(u'Received Slider submission: "%s"', value) + value = value / 100.0 + if value < 0 or value > 1: + return {} # Invalid + self.student_value = value + if sub_api: + # Also send to the submissions API: + sub_api.create_submission(self.student_item_key, {'value': value, }) + result = self.get_last_result() + log.debug(u'Slider submission result: %s', result) + return result + + def get_author_edit_view_fragment(self, context): + """ + The options for the 1-5 values of the Likert scale aren't child blocks but we want to + show them in the author edit view, for clarity. + """ + fragment = Fragment(u"

{}

".format(self.question)) + self.render_children(context, fragment, can_reorder=True, can_add=False) + return fragment + + def validate_field_data(self, validation, data): + """ + Validate this block's field data. + """ + super(SliderBlock, self).validate_field_data(validation, data) diff --git a/problem_builder/step.py b/problem_builder/step.py index 2386b9d0..9372f63b 100644 --- a/problem_builder/step.py +++ b/problem_builder/step.py @@ -38,6 +38,7 @@ from problem_builder.mixins import EnumerableChildMixin, MessageParentMixin, StepParentMixin from problem_builder.mrq import MRQBlock from problem_builder.plot import PlotBlock +from problem_builder.slider import SliderBlock from problem_builder.table import MentoringTableBlock @@ -147,7 +148,7 @@ def allowed_nested_blocks(self): return [ NestedXBlockSpec(AnswerBlock, boilerplate='studio_default'), MCQBlock, RatingBlock, MRQBlock, HtmlBlockShim, - AnswerRecapBlock, MentoringTableBlock, PlotBlock + AnswerRecapBlock, MentoringTableBlock, PlotBlock, SliderBlock ] + additional_blocks @property diff --git a/problem_builder/templates/html/mentoring_add_buttons.html b/problem_builder/templates/html/mentoring_add_buttons.html index da36bf18..d03c7452 100644 --- a/problem_builder/templates/html/mentoring_add_buttons.html +++ b/problem_builder/templates/html/mentoring_add_buttons.html @@ -8,6 +8,7 @@
{% trans "Add New Component" %}
  • {% trans "Multiple Choice Question" %}
  • {% trans "Rating Question" %}
  • {% trans "Multiple Response Question" %}
    • +
  • {% trans "Ranged Value Slider" %}
  • {% trans "HTML" %}
  • {% trans "Long Answer Recap" %}
  • {% trans "Answer Recap Table" %}
    • diff --git a/problem_builder/templates/html/slider.html b/problem_builder/templates/html/slider.html new file mode 100644 index 00000000..453f7ece --- /dev/null +++ b/problem_builder/templates/html/slider.html @@ -0,0 +1,17 @@ +
    + {% if not hide_header %}

    {{ self.display_name_with_default }}

    {% endif %} + {% if question %} +

    + {% endif %} +
    + + + +
    +
    + +
    +
    diff --git a/problem_builder/templates/html/slider_edit_footer.html b/problem_builder/templates/html/slider_edit_footer.html new file mode 100644 index 00000000..1bb013e0 --- /dev/null +++ b/problem_builder/templates/html/slider_edit_footer.html @@ -0,0 +1,5 @@ +{% load i18n %} +
    + {% trans "ID for referencing this slider:" %} + {{ url_name }} +
    diff --git a/setup.py b/setup.py index 61e1534c..80fd6be8 100644 --- a/setup.py +++ b/setup.py @@ -54,6 +54,7 @@ def package_data(pkg, root_list): 'pb-mcq = problem_builder.mcq:MCQBlock', 'pb-rating = problem_builder.mcq:RatingBlock', 'pb-mrq = problem_builder.mrq:MRQBlock', + 'pb-slider = problem_builder.slider:SliderBlock', 'pb-message = problem_builder.message:MentoringMessageBlock', 'pb-tip = problem_builder.tip:TipBlock', 'pb-choice = problem_builder.choice:ChoiceBlock', From b63d2bfdfaab302643e14afaa4669c45233b6034 Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Sat, 17 Oct 2015 00:31:55 -0700 Subject: [PATCH 2/9] Tests for the slider block, plus fix title --- problem_builder/slider.py | 1 + problem_builder/templates/html/slider.html | 2 +- .../tests/integration/test_slider.py | 147 ++++++++++++++++++ .../xml_templates/slider_problem.xml | 12 ++ .../integration/xml_templates/slider_step.xml | 16 ++ 5 files changed, 177 insertions(+), 1 deletion(-) create mode 100644 problem_builder/tests/integration/test_slider.py create mode 100644 problem_builder/tests/integration/xml_templates/slider_problem.xml create mode 100644 problem_builder/tests/integration/xml_templates/slider_step.xml diff --git a/problem_builder/slider.py b/problem_builder/slider.py index bbd745ac..fa8600bd 100644 --- a/problem_builder/slider.py +++ b/problem_builder/slider.py @@ -94,6 +94,7 @@ def mentoring_view(self, context): context['initial_value'] = int(self.student_value*100) if self.student_value is not None else 50 context['min_label'] = self.min_label context['max_label'] = self.max_label + context['title'] = self.display_name_with_default context['hide_header'] = context.get('hide_header', False) or not self.show_title context['instructions_string'] = self._("Select a value from {min_label} to {max_label}").format( min_label=self.min_label, max_label=self.max_label diff --git a/problem_builder/templates/html/slider.html b/problem_builder/templates/html/slider.html index 453f7ece..09bb7c55 100644 --- a/problem_builder/templates/html/slider.html +++ b/problem_builder/templates/html/slider.html @@ -1,5 +1,5 @@
    - {% if not hide_header %}

    {{ self.display_name_with_default }}

    {% endif %} + {% if not hide_header %}

    {{ title }}

    {% endif %} {% if question %}

    {% endif %} diff --git a/problem_builder/tests/integration/test_slider.py b/problem_builder/tests/integration/test_slider.py new file mode 100644 index 00000000..42d00399 --- /dev/null +++ b/problem_builder/tests/integration/test_slider.py @@ -0,0 +1,147 @@ +# -*- coding: utf-8 -*- +# +# Copyright (c) 2014-2015 Harvard, edX & OpenCraft +# +# This software's license gives you freedom; you can copy, convey, +# propagate, redistribute and/or modify this program under the terms of +# the GNU Affero General Public License (AGPL) as published by the Free +# Software Foundation (FSF), either version 3 of the License, or (at your +# option) any later version of the AGPL published by the FSF. +# +# This program is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero +# General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program in a file in the toplevel directory called +# "AGPLv3". If not, see . +# + +# Imports ########################################################### + +from .base_test import ProblemBuilderBaseTest, MentoringAssessmentBaseTest, CORRECT, GetChoices + + +# Classes ########################################################### + + +class SliderBlockTestMixins(object): + """ Mixins for testing slider blocks. Assumes only one slider block is on the page. """ + + def get_slider_value(self): + return int(self.browser.execute_script("return $('.pb-slider-range').val()")) + + def set_slider_value(self, val): + self.browser.execute_script("$('.pb-slider-range').val(arguments[0]).change()", val) + + +class SliderBlockTest(SliderBlockTestMixins, ProblemBuilderBaseTest): + """ + Tests for the SliderBlock inside a normal Problem Builder block. + """ + def test_simple_flow(self): + """ Test a regular Problem Builder block containing one slider """ + pb_wrapper = self.load_scenario("slider_problem.xml", {"include_mcq": False}) + self.wait_for_init() + # The initial value should be 50 and submit should be enabled since 50 is a valid value: + self.assertTrue(self.submit_button.is_enabled()) + self.assertEqual(self.get_slider_value(), 50) + self.expect_checkmark_visible(False) + # Set the value to 75: + self.set_slider_value(75) + self.assertEqual(self.get_slider_value(), 75) + self.click_submit(pb_wrapper) + # Now, we expect submit to be disabled and the checkmark to be visible: + self.expect_checkmark_visible(True) + self.assertFalse(self.submit_button.is_enabled()) + # Now change the value, and the button/checkmark should reset: + self.set_slider_value(45) + self.assertTrue(self.submit_button.is_enabled()) + self.expect_checkmark_visible(False) + # Now reload the page: + self.browser.execute_script("$(document).html(' ');") + pb_wrapper = self.go_to_view("student_view") + self.wait_for_init() + # Now the initial value should be 75 and submit should be disabled (to discourage submitting the same answer): + self.assertEqual(self.get_slider_value(), 75) + self.assertFalse(self.submit_button.is_enabled()) + self.expect_checkmark_visible(True) + + def test_simple_flow_with_peer(self): + """ Test a regular Problem Builder block containing one slider and an MCQ """ + pb_wrapper = self.load_scenario("slider_problem.xml", {"include_mcq": True}) + self.wait_for_init() + # The initial value should be 50 and submit should be disabled until an MCQ choice is selected + self.assertEqual(self.get_slider_value(), 50) + self.assertFalse(self.submit_button.is_enabled()) + self.expect_checkmark_visible(False) + # Set the value to 15: + self.set_slider_value(15) + self.assertEqual(self.get_slider_value(), 15) + self.assertFalse(self.submit_button.is_enabled()) + # Choose a choice: + mcq_choices = pb_wrapper.find_elements_by_css_selector('.choices .choice input') + mcq_choices[0].click() + self.assertTrue(self.submit_button.is_enabled()) + self.click_submit(pb_wrapper) + # Now, we expect submit to be disabled and the checkmark to be visible: + self.expect_checkmark_visible(True) + self.assertFalse(self.submit_button.is_enabled()) + # Now change the value, and the button/checkmark should reset: + self.set_slider_value(20) + self.assertTrue(self.submit_button.is_enabled()) + self.expect_checkmark_visible(False) + + def wait_for_init(self): + """ Wait for the scenario to initialize """ + self.wait_until_hidden(self.browser.find_element_by_css_selector('.messages')) + + @property + def submit_button(self): + return self.browser.find_element_by_css_selector('.submit input.input-main') + + def expect_checkmark_visible(self, visible): + checkmark = self.browser.find_element_by_css_selector('.xblock-pb-slider .submit-result') + self.assertEqual(checkmark.is_displayed(), visible) + + +class SliderStepBlockTest(SliderBlockTestMixins, MentoringAssessmentBaseTest): + """ + Tests for the SliderBlock inside a Step Builder block. + """ + + def test_step_with_slider(self): + """ Test a regular Step Builder block containing one slider and an MCQ """ + step_builder, controls = self.load_assessment_scenario("slider_step.xml") + self.wait_for_init() + self.assertEqual(self.get_slider_value(), 50) + + # Check step 1 (the slider step): + question = self.expect_question_visible(1, step_builder, question_text="Information Reliability") + self.assertIn("How reliable is this information?", question.text) + self.assertIn("Select a value from 0% to 100%", question.text) # Screen reader explanation + self.assertTrue(controls.submit.is_enabled()) + self.assert_hidden(controls.try_again) + + self.set_slider_value(99) + controls.submit.click() + self.do_submit_wait(controls, last=False) + self.wait_until_clickable(controls.next_question) + controls.next_question.click() + + # Submit step 2: + question = self.expect_question_visible(2, step_builder) + GetChoices(question).select("Yes") + controls.submit.click() + self.do_submit_wait(controls, last=True) + self.wait_until_clickable(controls.review) + + controls.review.click() + self.wait_until_visible(controls.try_again) + # You can't get a slider question wrong, but it does count as one correct point by default: + self.assertIn("You answered 2 questions correctly", step_builder.text) + + def wait_for_init(self): + """ Wait for the scenario to initialize """ + self.wait_until_hidden(self.browser.find_element_by_css_selector('.assessment-review-tips')) diff --git a/problem_builder/tests/integration/xml_templates/slider_problem.xml b/problem_builder/tests/integration/xml_templates/slider_problem.xml new file mode 100644 index 00000000..cddf98a2 --- /dev/null +++ b/problem_builder/tests/integration/xml_templates/slider_problem.xml @@ -0,0 +1,12 @@ + + + + {% if include_mcq %} + + Yes + Maybe not + I don't understand + + {% endif %} + + diff --git a/problem_builder/tests/integration/xml_templates/slider_step.xml b/problem_builder/tests/integration/xml_templates/slider_step.xml new file mode 100644 index 00000000..88204c7a --- /dev/null +++ b/problem_builder/tests/integration/xml_templates/slider_step.xml @@ -0,0 +1,16 @@ + + + + + + + + + Yes + No + + + + + + From 6db57129ff129ee393f94cde93f9b3a68b04bea4 Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Mon, 19 Oct 2015 14:48:18 -0700 Subject: [PATCH 3/9] Addressed bugs reported in review --- problem_builder/mixins.py | 1 - problem_builder/slider.py | 16 ++++++---------- problem_builder/tests/integration/test_slider.py | 2 +- 3 files changed, 7 insertions(+), 12 deletions(-) diff --git a/problem_builder/mixins.py b/problem_builder/mixins.py index b3aae058..cdcafd85 100644 --- a/problem_builder/mixins.py +++ b/problem_builder/mixins.py @@ -126,7 +126,6 @@ class QuestionMixin(EnumerableChildMixin): # Fields: name = String( - # This doesn't need to be a field but is kept for backwards compatibility with v1 student data display_name=_("Question ID (name)"), help=_("The ID of this question (required). Should be unique within this mentoring component."), default=UNIQUE_ID, diff --git a/problem_builder/slider.py b/problem_builder/slider.py index fa8600bd..e1d71dd9 100644 --- a/problem_builder/slider.py +++ b/problem_builder/slider.py @@ -115,6 +115,7 @@ def author_view(self, context): Add some HTML to the author view that allows authors to see the ID of the block, so they can refer to it in other blocks such as Plot blocks. """ + context['hide_header'] = True # Header is already shown in the Studio wrapper fragment = self.student_view(context) fragment.add_content(loader.render_template('templates/html/slider_edit_footer.html', { "url_name": self.url_name @@ -123,7 +124,7 @@ def author_view(self, context): def get_last_result(self): """ Return the current/last result in the required format """ - if not self.student_value: + if self.student_value is None: return {} return { 'submission': self.student_value, @@ -133,6 +134,10 @@ def get_last_result(self): 'score': 1, } + def get_results(self, _previous_result_unused=None): + """ Alias for get_last_result() """ + return self.get_last_result() + def submit(self, value): log.debug(u'Received Slider submission: "%s"', value) value = value / 100.0 @@ -146,15 +151,6 @@ def submit(self, value): log.debug(u'Slider submission result: %s', result) return result - def get_author_edit_view_fragment(self, context): - """ - The options for the 1-5 values of the Likert scale aren't child blocks but we want to - show them in the author edit view, for clarity. - """ - fragment = Fragment(u"

    {}

    ".format(self.question)) - self.render_children(context, fragment, can_reorder=True, can_add=False) - return fragment - def validate_field_data(self, validation, data): """ Validate this block's field data. diff --git a/problem_builder/tests/integration/test_slider.py b/problem_builder/tests/integration/test_slider.py index 42d00399..dea1c85b 100644 --- a/problem_builder/tests/integration/test_slider.py +++ b/problem_builder/tests/integration/test_slider.py @@ -124,7 +124,7 @@ def test_step_with_slider(self): self.assertTrue(controls.submit.is_enabled()) self.assert_hidden(controls.try_again) - self.set_slider_value(99) + self.set_slider_value(0) controls.submit.click() self.do_submit_wait(controls, last=False) self.wait_until_clickable(controls.next_question) From b30ab46bbe6988626a5130b6d11848bd0f564376 Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Mon, 19 Oct 2015 14:53:04 -0700 Subject: [PATCH 4/9] Fix: extended review of slider step showed wrong value --- problem_builder/public/js/slider.js | 4 ++-- problem_builder/slider.py | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/problem_builder/public/js/slider.js b/problem_builder/public/js/slider.js index 2c22bf2f..ea1d46fe 100644 --- a/problem_builder/public/js/slider.js +++ b/problem_builder/public/js/slider.js @@ -15,11 +15,11 @@ function SliderBlock(runtime, element) { }, submit: function() { - return this.value(); + return this.value() / 100.0; }, handleReview: function(result){ - $slider.val(result.submission); + $slider.val(result.submission * 100.0); $slider.prop('disabled', true); }, diff --git a/problem_builder/slider.py b/problem_builder/slider.py index e1d71dd9..97a21588 100644 --- a/problem_builder/slider.py +++ b/problem_builder/slider.py @@ -140,7 +140,6 @@ def get_results(self, _previous_result_unused=None): def submit(self, value): log.debug(u'Received Slider submission: "%s"', value) - value = value / 100.0 if value < 0 or value > 1: return {} # Invalid self.student_value = value From c78663b4adf1d61daae6e3c806753d89c9cabea6 Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Tue, 20 Oct 2015 11:44:26 -0700 Subject: [PATCH 5/9] Fix review comments --- problem_builder/questionnaire.py | 2 +- problem_builder/slider.py | 6 +++--- problem_builder/tests/integration/test_slider.py | 5 ++--- .../tests/integration/xml_templates/slider_step.xml | 2 +- 4 files changed, 7 insertions(+), 8 deletions(-) diff --git a/problem_builder/questionnaire.py b/problem_builder/questionnaire.py index a65b2f4b..301da80f 100644 --- a/problem_builder/questionnaire.py +++ b/problem_builder/questionnaire.py @@ -24,7 +24,7 @@ from lazy import lazy import uuid from xblock.core import XBlock -from xblock.fields import Scope, String, Float +from xblock.fields import Scope, String from xblock.fragment import Fragment from xblock.validation import ValidationMessage from xblockutils.helpers import child_isinstance diff --git a/problem_builder/slider.py b/problem_builder/slider.py index 97a21588..0a6c630c 100644 --- a/problem_builder/slider.py +++ b/problem_builder/slider.py @@ -26,7 +26,6 @@ from xblock.core import XBlock from xblock.fields import Scope, String, Float from xblock.fragment import Fragment -from xblock.validation import ValidationMessage from xblockutils.studio_editable import StudioEditableXBlockMixin from xblockutils.resources import ResourceLoader @@ -52,7 +51,8 @@ class SliderBlock( SubmittingXBlockMixin, QuestionMixin, StudioEditableXBlockMixin, XBlockWithTranslationServiceMixin, XBlock, ): """ - An XBlock used to ask multiple-choice questions + An XBlock used by students to indicate a numeric value on a sliding scale. + The student's answer is always considered "correct". """ CATEGORY = 'pb-slider' STUDIO_LABEL = _(u"Ranged Value Slider") @@ -145,7 +145,7 @@ def submit(self, value): self.student_value = value if sub_api: # Also send to the submissions API: - sub_api.create_submission(self.student_item_key, {'value': value, }) + sub_api.create_submission(self.student_item_key, {'value': value}) result = self.get_last_result() log.debug(u'Slider submission result: %s', result) return result diff --git a/problem_builder/tests/integration/test_slider.py b/problem_builder/tests/integration/test_slider.py index dea1c85b..148026d6 100644 --- a/problem_builder/tests/integration/test_slider.py +++ b/problem_builder/tests/integration/test_slider.py @@ -20,7 +20,7 @@ # Imports ########################################################### -from .base_test import ProblemBuilderBaseTest, MentoringAssessmentBaseTest, CORRECT, GetChoices +from .base_test import ProblemBuilderBaseTest, MentoringAssessmentBaseTest, GetChoices # Classes ########################################################### @@ -81,8 +81,7 @@ def test_simple_flow_with_peer(self): self.assertEqual(self.get_slider_value(), 15) self.assertFalse(self.submit_button.is_enabled()) # Choose a choice: - mcq_choices = pb_wrapper.find_elements_by_css_selector('.choices .choice input') - mcq_choices[0].click() + GetChoices(pb_wrapper).select('Yes') self.assertTrue(self.submit_button.is_enabled()) self.click_submit(pb_wrapper) # Now, we expect submit to be disabled and the checkmark to be visible: diff --git a/problem_builder/tests/integration/xml_templates/slider_step.xml b/problem_builder/tests/integration/xml_templates/slider_step.xml index 88204c7a..00c1f4b7 100644 --- a/problem_builder/tests/integration/xml_templates/slider_step.xml +++ b/problem_builder/tests/integration/xml_templates/slider_step.xml @@ -1,4 +1,4 @@ - + From eb164bc15a82f4cda619dc736ef99faac945df39 Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Tue, 20 Oct 2015 11:45:07 -0700 Subject: [PATCH 6/9] Remove headers when sliders appear inside parent block in Studio --- problem_builder/slider.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/problem_builder/slider.py b/problem_builder/slider.py index 0a6c630c..ff952736 100644 --- a/problem_builder/slider.py +++ b/problem_builder/slider.py @@ -106,9 +106,8 @@ def mentoring_view(self, context): fragment.initialize_js('SliderBlock') return fragment - def student_view(self, context=None): - """ Normal view of this XBlock, identical to mentoring_view """ - return self.mentoring_view(context) + student_view = mentoring_view + preview_view = mentoring_view def author_view(self, context): """ From 914d132ce04e649e0d2219217327a7ca828327bb Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Tue, 20 Oct 2015 11:48:52 -0700 Subject: [PATCH 7/9] Fix Travis build --- .travis.yml | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index 6c0587c4..dbb65671 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,12 +5,13 @@ before_install: - "export DISPLAY=:99" - "sh -e /etc/init.d/xvfb start" install: - - "pip install -e git://github.com/edx/xblock-sdk.git#egg=xblock-sdk" + - "pip install -e git://github.com/edx/xblock-sdk.git@22c1b2f173919bef22f2d9d9295ec5396d02dffd#egg=xblock-sdk" - "pip install -r requirements.txt" - - "pip install -r $VIRTUAL_ENV/src/xblock-sdk/requirements.txt" - - "pip install -r $VIRTUAL_ENV/src/xblock-sdk/test-requirements.txt" + - "pip install -r $VIRTUAL_ENV/src/xblock-sdk/requirements/base.txt" + - "pip install -r $VIRTUAL_ENV/src/xblock-sdk/requirements/test.txt" - "pip uninstall -y xblock-problem-builder && python setup.py sdist && pip install dist/xblock-problem-builder-2.0.tar.gz" - "pip install -r test_requirements.txt" + - "mkdir var" script: - pep8 problem_builder --max-line-length=120 - pylint problem_builder --disable=all --enable=function-redefined,undefined-variable,unused-variable From 8aba22ad84ba69f059062517b98ba9bc9e402d49 Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Wed, 21 Oct 2015 00:42:45 -0700 Subject: [PATCH 8/9] Change required post-rebase --- problem_builder/tests/unit/test_step.py | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/problem_builder/tests/unit/test_step.py b/problem_builder/tests/unit/test_step.py index 94e7637f..eaab18b5 100644 --- a/problem_builder/tests/unit/test_step.py +++ b/problem_builder/tests/unit/test_step.py @@ -108,7 +108,17 @@ def test_allowed_nested_blocks(self): block = MentoringStepBlock(Mock(), DictFieldData({}), Mock()) self.assertEqual( self.get_allowed_blocks(block), - ['pb-answer', 'pb-mcq', 'pb-rating', 'pb-mrq', 'html', 'pb-answer-recap', 'pb-table', 'sb-plot'] + [ + 'pb-answer', + 'pb-mcq', + 'pb-rating', + 'pb-mrq', + 'html', + 'pb-answer-recap', + 'pb-table', + 'sb-plot', + 'pb-slider', + ] ) from sys import modules xmodule_mock = Mock() @@ -121,7 +131,16 @@ def test_allowed_nested_blocks(self): with patch.dict(modules, fake_modules): self.assertEqual( self.get_allowed_blocks(block), [ - 'pb-answer', 'pb-mcq', 'pb-rating', 'pb-mrq', 'html', 'pb-answer-recap', - 'pb-table', 'sb-plot', 'video', 'imagemodal' + 'pb-answer', + 'pb-mcq', + 'pb-rating', + 'pb-mrq', + 'html', + 'pb-answer-recap', + 'pb-table', + 'sb-plot', + 'pb-slider', + 'video', + 'imagemodal', ] ) From 2f7e35a1f8d126418f1d55573d1d998754724f52 Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Wed, 21 Oct 2015 10:28:36 -0700 Subject: [PATCH 9/9] Fix to be compatible with PR #76 --- problem_builder/slider.py | 1 + 1 file changed, 1 insertion(+) diff --git a/problem_builder/slider.py b/problem_builder/slider.py index ff952736..4097d768 100644 --- a/problem_builder/slider.py +++ b/problem_builder/slider.py @@ -56,6 +56,7 @@ class SliderBlock( """ CATEGORY = 'pb-slider' STUDIO_LABEL = _(u"Ranged Value Slider") + answerable = True min_label = String( display_name=_("Low"),