diff --git a/problem_builder/tests/integration/test_mcq.py b/problem_builder/tests/integration/test_mcq.py
index 9cfc1680..34d7d3d3 100644
--- a/problem_builder/tests/integration/test_mcq.py
+++ b/problem_builder/tests/integration/test_mcq.py
@@ -45,6 +45,9 @@ def _selenium_bug_workaround_scroll_to(self, mcq_legend):
"""
mcq_legend.click()
+ def _get_choice_label_text(self, choice):
+ return choice.find_element_by_css_selector('label').text
+
def _get_inputs(self, choices):
return [choice.find_element_by_css_selector('input') for choice in choices]
@@ -71,20 +74,17 @@ def test_mcq_choices_rating(self):
self.assertEqual(mcq1_legend.text, 'Question 1\nDo you like this MCQ?')
self.assertEqual(mcq2_legend.text, 'Question 2\nHow do you rate this MCQ?')
- mcq1_choices = mcq1.find_elements_by_css_selector('.choices .choice label')
- mcq2_choices = mcq2.find_elements_by_css_selector('.rating .choice label')
-
- self.assertEqual(len(mcq1_choices), 3)
- self.assertEqual(len(mcq2_choices), 6)
- self.assertEqual(mcq1_choices[0].text, 'Yes')
- self.assertEqual(mcq1_choices[1].text, 'Maybe not')
- self.assertEqual(mcq1_choices[2].text, "I don't understand")
- self.assertEqual(mcq2_choices[0].text, '1 - Not good at all')
- self.assertEqual(mcq2_choices[1].text, '2')
- self.assertEqual(mcq2_choices[2].text, '3')
- self.assertEqual(mcq2_choices[3].text, '4')
- self.assertEqual(mcq2_choices[4].text, '5 - Extremely good')
- self.assertEqual(mcq2_choices[5].text, "I don't want to rate it")
+ mcq1_choices = mcq1.find_elements_by_css_selector('.choices .choice')
+ mcq2_choices = mcq2.find_elements_by_css_selector('.rating .choice')
+
+ self.assertListEqual(
+ [self._get_choice_label_text(choice) for choice in mcq1_choices],
+ ["Yes", "Maybe not", "I don't understand"]
+ )
+ self.assertListEqual(
+ [self._get_choice_label_text(choice) for choice in mcq2_choices],
+ ['1 - Not good at all', '2', '3', '4', '5 - Extremely good', "I don't want to rate it"]
+ )
mcq1_choices_input = self._get_inputs(mcq1_choices)
mcq2_choices_input = self._get_inputs(mcq2_choices)
@@ -172,13 +172,13 @@ def test_mcq_with_comments(self):
mcq_legend = mcq.find_element_by_css_selector('legend')
self.assertEqual(mcq_legend.text, 'Question\nWhat do you like in this MRQ?')
- mcq_choices = mcq.find_elements_by_css_selector('.choices .choice label')
+ mcq_choices = mcq.find_elements_by_css_selector('.choices .choice')
self.assertEqual(len(mcq_choices), 4)
- self.assertEqual(mcq_choices[0].text, 'Its elegance')
- self.assertEqual(mcq_choices[1].text, 'Its beauty')
- self.assertEqual(mcq_choices[2].text, "Its gracefulness")
- self.assertEqual(mcq_choices[3].text, "Its bugs")
+ self.assertListEqual(
+ [self._get_choice_label_text(choice) for choice in mcq_choices],
+ ['Its elegance', 'Its beauty', "Its gracefulness", "Its bugs"]
+ )
mcq_choices_input = self._get_inputs(mcq_choices)
self.assertEqual(mcq_choices_input[0].get_attribute('value'), 'elegance')
@@ -200,7 +200,7 @@ def test_mcq_feedback_popups(self):
for index, expected_feedback in enumerate(item_feedbacks):
choice_wrapper = choices_list.find_elements_by_css_selector(".choice")[index]
- choice_wrapper.find_element_by_css_selector(".choice-selector").click() # clicking on actual radio button
+ choice_wrapper.find_element_by_css_selector(".choice-selector input").click() # click actual radio button
submit.click()
self.wait_until_disabled(submit)
item_feedback_icon = choice_wrapper.find_element_by_css_selector(".choice-result")
@@ -220,8 +220,8 @@ def _get_questionnaire_options(self, questionnaire):
result = []
# this could be a list comprehension, but a bit complicated one - hence explicit loop
for choice_wrapper in questionnaire.find_elements_by_css_selector(".choice"):
- choice_label = choice_wrapper.find_element_by_css_selector("label .choice-text")
- result.append(choice_label.get_attribute('innerHTML'))
+ choice_label = choice_wrapper.find_element_by_css_selector("label")
+ result.append(choice_label.get_attribute('innerHTML').strip())
return result
@@ -249,7 +249,7 @@ def test_questionnaire_html_choices(self, page):
submit = mentoring.find_element_by_css_selector('.submit input.input-main')
self.assertFalse(submit.is_enabled())
- inputs = choices_list.find_elements_by_css_selector('input.choice-selector')
+ inputs = choices_list.find_elements_by_css_selector('.choice-selector input')
self._selenium_bug_workaround_scroll_to(choices_list)
inputs[0].click()
inputs[1].click()
@@ -261,6 +261,41 @@ def test_questionnaire_html_choices(self, page):
self.assertIn('Congratulations!', messages.text)
+ def _get_inner_height(self, elem):
+ return elem.size['height'] - \
+ int(elem.value_of_css_property("padding-top").replace(u'px', u'')) - \
+ int(elem.value_of_css_property("padding-bottom").replace(u'px', u''))
+
+ @ddt.unpack
+ @ddt.data(
+ ('yes', 40),
+ ('maybenot', 60),
+ ('understand', 600)
+ )
+ def test_tip_height(self, choice_value, expected_height):
+ mentoring = self.go_to_page("Mcq With Fixed Height Tips")
+ choices_list = mentoring.find_element_by_css_selector(".choices-list")
+ submit = mentoring.find_element_by_css_selector('.submit input.input-main')
+
+ choice_input_css_selector = ".choice input[value={}]".format(choice_value)
+ choice_input = choices_list.find_element_by_css_selector(choice_input_css_selector)
+ choice_wrapper = choice_input.find_element_by_xpath("./ancestor::div[@class='choice']")
+
+ choice_input.click()
+ self.wait_until_clickable(submit)
+ submit.click()
+ self.wait_until_disabled(submit)
+
+ item_feedback_popup = choice_wrapper.find_element_by_css_selector(".choice-tips")
+ self.assertTrue(item_feedback_popup.is_displayed())
+ feedback_height = self._get_inner_height(item_feedback_popup)
+ self.assertEqual(feedback_height, expected_height)
+
+ choice_wrapper.find_element_by_css_selector(".choice-result").click()
+ item_feedback_popup = choice_wrapper.find_element_by_css_selector(".choice-tips")
+ item_feedback_height = self._get_inner_height(item_feedback_popup)
+ self.assertEqual(item_feedback_height, expected_height)
+
@patch.object(MentoringBlock, 'get_theme', Mock(return_value={'package': 'problem_builder',
'locations': ['public/themes/lms.css']}))
diff --git a/problem_builder/tests/integration/test_titles.py b/problem_builder/tests/integration/test_titles.py
new file mode 100644
index 00000000..498d6a73
--- /dev/null
+++ b/problem_builder/tests/integration/test_titles.py
@@ -0,0 +1,126 @@
+# -*- 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
.
+#
+"""
+Test that the various title/display_name options for Answer and MCQ/MRQ/Ratings work.
+"""
+
+# Imports ###########################################################
+from mock import patch
+from xblockutils.base_test import SeleniumXBlockTest
+
+
+# Classes ###########################################################
+
+
+class StepTitlesTest(SeleniumXBlockTest):
+ """
+ Test that the various title/display_name options for Answer and MCQ/MRQ/Ratings work.
+ """
+
+ test_parameters = (
+ # display_name, show_title?, expected_title: (None means default value)
+ ("Custom Title", None, "Custom Title",),
+ ("Custom Title", True, "Custom Title",),
+ ("Custom Title", False, None),
+ ("", None, "Question"),
+ ("", True, "Question"),
+ ("", False, None),
+ )
+
+ mcq_template = """
+
+
+ Gaius Baltar
+ Admiral William Adama
+ Starbuck
+ Laura Roslin
+ Number Six
+ Lee Adama
+
+
+ """
+
+ mrq_template = """
+
+
+ Lots of choices
+ Funny choices
+ Not sure
+
+
+ """
+
+ rating_template = """
+
+
+ More than 5 stars
+
+
+ """
+
+ long_answer_template = """
+
+
+
+ """
+
+ def setUp(self):
+ super(StepTitlesTest, self).setUp()
+ # Disable asides for this test since the acid aside seems to cause Database errors
+ # When we test multiple scenarios in one test method.
+ patcher = patch(
+ 'workbench.runtime.WorkbenchRuntime.applicable_aside_types',
+ lambda self, block: [], create=True
+ )
+ patcher.start()
+ self.addCleanup(patcher.stop)
+
+ def test_all_the_things(self):
+ """ Test various permutations of our problem-builder components and title options. """
+ # We use a loop within the test rather than DDT, because this is WAY faster
+ # since we can bypass the Selenium set-up and teardown
+ for display_name, show_title, expected_title in self.test_parameters:
+ for mode in ("standard", "assessment"):
+ for qtype in ("mcq", "mrq", "rating", "long_answer"):
+ template = getattr(self, qtype + "_template")
+ xml = template.format(
+ mode=mode,
+ display_name_attr='display_name="{}"'.format(display_name) if display_name is not None else "",
+ show_title_attr='show_title="{}"'.format(show_title) if show_title is not None else "",
+ )
+ self.set_scenario_xml(xml)
+ pb_element = self.go_to_view()
+ if expected_title:
+ h3 = pb_element.find_element_by_css_selector('h3')
+ self.assertEqual(h3.text, expected_title)
+ else:
+ # No
element should be present:
+ all_h3s = pb_element.find_elements_by_css_selector('h3')
+ self.assertEqual(len(all_h3s), 0)
diff --git a/problem_builder/tests/integration/xml/mcq_with_fixed_height_tips.xml b/problem_builder/tests/integration/xml/mcq_with_fixed_height_tips.xml
new file mode 100644
index 00000000..4a4dee69
--- /dev/null
+++ b/problem_builder/tests/integration/xml/mcq_with_fixed_height_tips.xml
@@ -0,0 +1,13 @@
+
+
+
+ Yes
+ Maybe not
+ I don't understand
+
+ Great!
+ Ah, damn.
+ Really?
+
+
+
diff --git a/problem_builder/v1/tests/xml/v1_upgrade_b_new.xml b/problem_builder/v1/tests/xml/v1_upgrade_b_new.xml
index 1c571895..726d1e7c 100644
--- a/problem_builder/v1/tests/xml/v1_upgrade_b_new.xml
+++ b/problem_builder/v1/tests/xml/v1_upgrade_b_new.xml
@@ -8,7 +8,7 @@
Yes
Maybe not
I don't understand
- Great. Your frog should be happy for you.
+ Great. Your frog should be happy for you.
In the end, all the feedback you have gotten from others should not lead you to choose a frog that does not also feel happy and important to you.
If a frog is happy for you, that means it is a frog that you genuinely feel in your own heart to be something that you want to improve. What is in your heart?
diff --git a/problem_builder/v1/tests/xml/v1_upgrade_b_old.xml b/problem_builder/v1/tests/xml/v1_upgrade_b_old.xml
index 5a088699..0487c291 100644
--- a/problem_builder/v1/tests/xml/v1_upgrade_b_old.xml
+++ b/problem_builder/v1/tests/xml/v1_upgrade_b_old.xml
@@ -16,7 +16,7 @@ This contains a typical problem taken from a live course (content changed)
Yes
Maybe not
I don't understand
- Great. Your frog should be happy for you.
+ Great. Your frog should be happy for you.
In the end, all the feedback you have gotten from others should not lead you to choose a frog that does not also feel happy and important to you.
diff --git a/problem_builder/v1/upgrade.py b/problem_builder/v1/upgrade.py
index a790a5d2..a6bf8dc0 100644
--- a/problem_builder/v1/upgrade.py
+++ b/problem_builder/v1/upgrade.py
@@ -24,15 +24,16 @@
optimized for editing in Studio.
To run the script on devstack:
-SERVICE_VARIANT=cms DJANGO_SETTINGS_MODULE="cms.envs.devstack" python -m mentoring.v1.upgrade
+SERVICE_VARIANT=cms DJANGO_SETTINGS_MODULE="cms.envs.devstack" python -m problem_builder.v1.upgrade [course id here]
"""
import logging
from lxml import etree
from mentoring import MentoringBlock
+from problem_builder import MentoringBlock as NewMentoringBlock
from StringIO import StringIO
import sys
+import warnings
from courseware.models import StudentModule
-from xmodule.modulestore.exceptions import DuplicateItemError
from .studio_xml_utils import studio_update_from_node
from .xml_changes import convert_xml_v1_to_v2
@@ -42,14 +43,18 @@ def upgrade_block(block):
Given a MentoringBlock "block" with old-style (v1) data in its "xml_content" field, parse
the XML and re-create the block with new-style (v2) children and settings.
"""
- assert isinstance(block, MentoringBlock)
+ assert isinstance(block, (MentoringBlock, NewMentoringBlock))
assert bool(block.xml_content) # If it's a v1 block it will have xml_content
store = block.runtime.modulestore
xml_content_str = block.xml_content
parser = etree.XMLParser(remove_blank_text=True)
root = etree.parse(StringIO(xml_content_str), parser=parser).getroot()
assert root.tag == "mentoring"
- convert_xml_v1_to_v2(root)
+ with warnings.catch_warnings(record=True) as warnings_caught:
+ warnings.simplefilter("always")
+ convert_xml_v1_to_v2(root)
+ for warning in warnings_caught:
+ print(u" ➔ {}".format(str(warning.message)))
# We need some special-case handling to deal with HTML being an XModule and not a pure XBlock:
try:
@@ -71,53 +76,46 @@ def parse_xml_for_HtmlDescriptor(cls, node, runtime, keys, id_generator):
root.attrib["xml_content"] = xml_content_str
# Was block already published?
- parent = block.get_parent()
+ parent = block.runtime.get_block(block.parent) # Don't use get_parent() as it may be an outdated cached version
parent_was_published = not store.has_changes(parent)
- # If the block has a url_name attribute that doesn't match Studio's url_name, fix that:
- delete_on_success = None
- if "url_name" in root.attrib:
- url_name = root.attrib.pop("url_name")
- if block.url_name != url_name:
- print(" ➔ This block has two conflicting url_name values set. Attempting to fix...")
- # Fix the url_name by replacing the block with a blank block with the correct url_name
- parent_children = parent.children
- old_usage_id = block.location
- index = parent_children.index(old_usage_id)
- try:
- new_block = store.create_item(
- user_id=None,
- course_key=block.location.course_key,
- block_type="mentoring",
- block_id=url_name,
- fields={"xml_content": xml_content_str},
- )
- delete_on_success = block
- parent_children[index] = new_block.location
- parent.save()
- store.update_item(parent, user_id=None)
- block = new_block
- print(" ➔ url_name changed to {}".format(url_name))
- # Now we've fixed the block's url_name but in doing so we've disrupted the student data.
- # Migrate it now:
- student_data = StudentModule.objects.filter(module_state_key=old_usage_id)
- num_entries = student_data.count()
- if num_entries > 0:
- print(" ➔ Migrating {} student records to new url_name".format(num_entries))
- student_data.update(module_state_key=new_block.location)
- except DuplicateItemError:
- print(
- "\n WARNING: The block with url_name '{}' doesn't match "
- "the real url_name '{}' and auto repair failed.\n".format(
- url_name, block.url_name
- )
- )
+ old_usage_id = block.location
+ if old_usage_id.block_type != "problem-builder":
+ # We need to change the block type from "mentoring" to "problem-builder", which requires
+ # deleting then re-creating the block:
+ store.delete_item(old_usage_id, user_id=None)
+ parent_children = parent.children
+ index = parent_children.index(old_usage_id)
+
+ url_name = unicode(old_usage_id.block_id)
+ if "url_name" in root.attrib:
+ url_name_xml = root.attrib.pop("url_name")
+ if url_name != url_name_xml:
+ print(u" ➔ Two conflicting url_name values! Using the 'real' one : {}".format(url_name))
+ print(u" ➔ References to the old url_name ({}) need to be updated manually.".format(url_name_xml))
+ block = store.create_item(
+ user_id=None,
+ course_key=old_usage_id.course_key,
+ block_type="problem-builder",
+ block_id=url_name,
+ fields={"xml_content": xml_content_str},
+ )
+ parent_children[index] = block.location
+ parent.save()
+ store.update_item(parent, user_id=None)
+ print(u" ➔ problem-builder created: {}".format(url_name))
+
+ # Now we've changed the block's block_type but in doing so we've disrupted the student data.
+ # Migrate it now:
+ student_data = StudentModule.objects.filter(module_state_key=old_usage_id)
+ num_entries = student_data.count()
+ if num_entries > 0:
+ print(u" ➔ Migrating {} student records to new block".format(num_entries))
+ student_data.update(module_state_key=block.location)
# Replace block with the new version and the new children:
studio_update_from_node(block, root)
- if delete_on_success:
- store.delete_item(delete_on_success.location, user_id=None)
if parent_was_published:
store.publish(parent.location, user_id=None)
@@ -130,9 +128,9 @@ def parse_xml_for_HtmlDescriptor(cls, node, runtime, keys, id_generator):
from opaque_keys.edx.keys import CourseKey
from xmodule.modulestore.django import modulestore
- print("┏━━━━━━━━━━━━━━━━━━━━━━━━━━┓")
- print("┃ Mentoring Upgrade Script ┃")
- print("┗━━━━━━━━━━━━━━━━━━━━━━━━━━┛")
+ print(u"┏━━━━━━━━━━━━━━━━━━━━━━━━━━┓")
+ print(u"┃ Mentoring Upgrade Script ┃")
+ print(u"┗━━━━━━━━━━━━━━━━━━━━━━━━━━┛")
try:
course_id = CourseKey.from_string(sys.argv[1])
@@ -143,26 +141,58 @@ def parse_xml_for_HtmlDescriptor(cls, node, runtime, keys, id_generator):
course = store.get_course(course_id)
if course is None:
sys.exit(u"Course '{}' not found.".format(unicode(course_id)))
- print(" ➔ Found course: {}".format(course.display_name))
- print(" ➔ Searching for mentoring blocks")
+ print(u" ➔ Found course: {}".format(course.display_name))
+ print(u" ➔ Searching for mentoring blocks")
blocks_found = []
def find_mentoring_blocks(block):
- if isinstance(block, MentoringBlock) and block.xml_content: # If it's a v1 block it will have xml_content
+ """
+ Find mentoring and recently-upgraded blocks. We check the recently upgraded ones
+ in case an error happened and we're re-running the upgrade.
+ """
+ # If it's a v1 block or a recently upgraded block it will have xml_content:
+ if isinstance(block, (MentoringBlock, NewMentoringBlock)) and block.xml_content:
blocks_found.append(block.scope_ids.usage_id)
elif block.has_children:
for child_id in block.children:
find_mentoring_blocks(block.runtime.get_block(child_id))
find_mentoring_blocks(course)
+
total = len(blocks_found)
- print(" ➔ Found {} mentoring blocks".format(total))
+ print(u" ➔ Found {} mentoring blocks".format(total))
+
+ print(u" ➔ Doing a quick sanity check of the url_names")
+ url_names = set()
+ stop = False
+ for block_id in blocks_found:
+ url_name = block_id.block_id
+ block = course.runtime.get_block(block_id)
+ if url_name in url_names:
+ print(u" ➔ Mentoring block {} appears in the course in multiple places!".format(url_name))
+ print(u' (display_name: "{}", parent {}: "{}")'.format(
+ block.display_name, block.parent, block.get_parent().display_name
+ ))
+ print(u' To fix, you must delete the extra occurences.')
+ stop = True
+ continue
+ if block.url_name and block.url_name != unicode(block_id.block_id):
+ print(u" ➔ Warning: Mentoring block {} has a different url_name set in the XML.".format(url_name))
+ print(u" If other blocks reference this block using the XML url_name '{}',".format(block.url_name))
+ print(u" those blocks will need to be updated.")
+ if "--force" not in sys.argv:
+ print(u" In order to force this upgrade to continue, add --force to the end of the command.")
+ stop = True
+ url_names.add(url_name)
+
+ if stop:
+ sys.exit(u" ➔ Exiting due to errors preventing the upgrade.")
with store.bulk_operations(course.location.course_key):
count = 1
for block_id in blocks_found:
block = course.runtime.get_block(block_id)
- print(" ➔ Upgrading block {} of {} - \"{}\"".format(count, total, block.url_name))
+ print(u" ➔ Upgrading block {} of {} - \"{}\"".format(count, total, block.url_name))
count += 1
upgrade_block(block)
- print(" ➔ Complete.")
+ print(u" ➔ Complete.")
diff --git a/problem_builder/v1/xml_changes.py b/problem_builder/v1/xml_changes.py
index 43c6e5e1..a4c2d94d 100644
--- a/problem_builder/v1/xml_changes.py
+++ b/problem_builder/v1/xml_changes.py
@@ -184,7 +184,6 @@ def applies_to(node):
def apply(self):
self.node.tag = "pb-answer-recap"
- self.node.attrib
self.node.attrib.pop("read_only")
for name in self.node.attrib:
if name != "name":
@@ -258,28 +257,29 @@ def add_to_list(list_name, value):
else:
p.attrib[list_name] = value
- if len(self.node.attrib) > 1:
- warnings.warn("Invalid element found.")
- return
- mode = self.node.attrib.keys()[0]
- value = self.node.attrib[mode]
if p.tag == "pb-mrq":
- if mode == "display":
+ if self.node.attrib.get("display"):
+ value = self.node.attrib.pop("display")
add_to_list("ignored_choices", value)
- elif mode == "require":
+ elif self.node.attrib.get("require"):
+ value = self.node.attrib.pop("require")
add_to_list("required_choices", value)
- elif mode != "reject":
- warnings.warn("Invalid element: has {}={}".format(mode, value))
+ elif self.node.attrib.get("reject"):
+ value = self.node.attrib.pop("reject")
+ else:
+ warnings.warn("Invalid element found.")
return
else:
# This is an MCQ or Rating question:
- if mode == "display":
+ if self.node.attrib.get("display"):
+ value = self.node.attrib.pop("display")
add_to_list("correct_choices", value)
- elif mode != "reject":
- warnings.warn("Invalid element: has {}={}".format(mode, value))
+ elif self.node.attrib.get("reject"):
+ value = self.node.attrib.pop("reject")
+ else:
+ warnings.warn("Invalid element found.")
return
self.node.attrib["values"] = value
- self.node.attrib.pop(mode)
class SharedHeaderToHTML(Change):