Skip to content

P
problem-builder
Commit ba65ce0a by Braden MacDonald
Options

Merge Upgrade Script Fixes - PR #2

parents f6afc98a a4751871
Showing with 104 additions and 70 deletions
problem_builder/questionnaire.py
...@@ -21,6 +21,7 @@ ...@@ -21,6 +21,7 @@
# Imports ########################################################### # Imports ###########################################################
from django.utils.safestring import mark_safe from django.utils.safestring import mark_safe
import logging
from lxml import etree from lxml import etree
from xblock.core import XBlock from xblock.core import XBlock
from xblock.fields import Scope, String, Float, List, UNIQUE_ID from xblock.fields import Scope, String, Float, List, UNIQUE_ID
...@@ -99,6 +100,9 @@ class QuestionnaireAbstractBlock(StudioEditableXBlockMixin, StudioContainerXBloc ...@@ -99,6 +100,9 @@ class QuestionnaireAbstractBlock(StudioEditableXBlockMixin, StudioContainerXBloc
# Load XBlock properties from the XML attributes: # Load XBlock properties from the XML attributes:
for name, value in node.items(): for name, value in node.items():
if name not in block.fields:
logging.warn("XBlock %s does not contain field %s", type(block), name)
continue
field = block.fields[name] field = block.fields[name]
if isinstance(field, List) and not value.startswith('['): if isinstance(field, List) and not value.startswith('['):
# This list attribute is just a string of comma separated strings: # This list attribute is just a string of comma separated strings:
......
problem_builder/v1/tests/xml/v1_upgrade_b_new.xml
...@@ -8,7 +8,7 @@ ...@@ -8,7 +8,7 @@
<pb-choice value="yes">Yes</pb-choice> <pb-choice value="yes">Yes</pb-choice>
<pb-choice value="maybenot">Maybe not</pb-choice> <pb-choice value="maybenot">Maybe not</pb-choice>
<pb-choice value="understand">I don't understand</pb-choice> <pb-choice value="understand">I don't understand</pb-choice>
<pb-tip values="yes">Great. Your frog should be happy for you.</pb-tip> <pb-tip width="350" height="100" values="yes">Great. Your frog should be happy for you.</pb-tip>
<pb-tip values="maybenot">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.</pb-tip> <pb-tip values="maybenot">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.</pb-tip>
<pb-tip values="understand"> <pb-tip values="understand">
<p>If a frog is <span class="italic">happy for you</span>, 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?</p> <p>If a frog is <span class="italic">happy for you</span>, 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?</p>
......
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) ...@@ -16,7 +16,7 @@ This contains a typical problem taken from a live course (content changed)
<choice value="yes">Yes</choice> <choice value="yes">Yes</choice>
<choice value="maybenot">Maybe not</choice> <choice value="maybenot">Maybe not</choice>
<choice value="understand">I don't understand</choice> <choice value="understand">I don't understand</choice>
<tip display="yes">Great. Your frog should be happy for you.</tip> <tip width="350" height="100" display="yes">Great. Your frog should be happy for you.</tip>
<tip display="maybenot">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.</tip> <tip display="maybenot">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.</tip>
<tip display="understand"> <tip display="understand">
<html> <html>
......
problem_builder/v1/upgrade.py
...@@ -24,15 +24,16 @@ This file contains a script to help migrate mentoring blocks to the new format w ...@@ -24,15 +24,16 @@ This file contains a script to help migrate mentoring blocks to the new format w
optimized for editing in Studio. optimized for editing in Studio.
To run the script on devstack: 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 import logging
from lxml import etree from lxml import etree
from mentoring import MentoringBlock from mentoring import MentoringBlock
from problem_builder import MentoringBlock as NewMentoringBlock
from StringIO import StringIO from StringIO import StringIO
import sys import sys
import warnings
from courseware.models import StudentModule from courseware.models import StudentModule
from xmodule.modulestore.exceptions import DuplicateItemError
from .studio_xml_utils import studio_update_from_node from .studio_xml_utils import studio_update_from_node
from .xml_changes import convert_xml_v1_to_v2 from .xml_changes import convert_xml_v1_to_v2
...@@ -42,14 +43,18 @@ def upgrade_block(block): ...@@ -42,14 +43,18 @@ def upgrade_block(block):
Given a MentoringBlock "block" with old-style (v1) data in its "xml_content" field, parse 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. 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 assert bool(block.xml_content) # If it's a v1 block it will have xml_content
store = block.runtime.modulestore store = block.runtime.modulestore
xml_content_str = block.xml_content xml_content_str = block.xml_content
parser = etree.XMLParser(remove_blank_text=True) parser = etree.XMLParser(remove_blank_text=True)
root = etree.parse(StringIO(xml_content_str), parser=parser).getroot() root = etree.parse(StringIO(xml_content_str), parser=parser).getroot()
assert root.tag == "mentoring" 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: # We need some special-case handling to deal with HTML being an XModule and not a pure XBlock:
try: try:
...@@ -71,53 +76,46 @@ def upgrade_block(block): ...@@ -71,53 +76,46 @@ def upgrade_block(block):
root.attrib["xml_content"] = xml_content_str root.attrib["xml_content"] = xml_content_str
# Was block already published? # 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) 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: old_usage_id = block.location
delete_on_success = None if old_usage_id.block_type != "problem-builder":
if "url_name" in root.attrib: # We need to change the block type from "mentoring" to "problem-builder", which requires
url_name = root.attrib.pop("url_name") # deleting then re-creating the block:
if block.url_name != url_name: store.delete_item(old_usage_id, user_id=None)
print(" ➔ This block has two conflicting url_name values set. Attempting to fix...") parent_children = parent.children
# Fix the url_name by replacing the block with a blank block with the correct url_name index = parent_children.index(old_usage_id)
parent_children = parent.children
old_usage_id = block.location url_name = unicode(old_usage_id.block_id)
index = parent_children.index(old_usage_id) if "url_name" in root.attrib:
try: url_name_xml = root.attrib.pop("url_name")
new_block = store.create_item( if url_name != url_name_xml:
user_id=None, print(u" ➔ Two conflicting url_name values! Using the 'real' one : {}".format(url_name))
course_key=block.location.course_key, print(u" ➔ References to the old url_name ({}) need to be updated manually.".format(url_name_xml))
block_type="mentoring", block = store.create_item(
block_id=url_name, user_id=None,
fields={"xml_content": xml_content_str}, course_key=old_usage_id.course_key,
) block_type="problem-builder",
delete_on_success = block block_id=url_name,
parent_children[index] = new_block.location fields={"xml_content": xml_content_str},
parent.save() )
store.update_item(parent, user_id=None) parent_children[index] = block.location
block = new_block parent.save()
print(" ➔ url_name changed to {}".format(url_name)) store.update_item(parent, user_id=None)
# Now we've fixed the block's url_name but in doing so we've disrupted the student data. print(u" ➔ problem-builder created: {}".format(url_name))
# Migrate it now:
student_data = StudentModule.objects.filter(module_state_key=old_usage_id) # Now we've changed the block's block_type but in doing so we've disrupted the student data.
num_entries = student_data.count() # Migrate it now:
if num_entries > 0: student_data = StudentModule.objects.filter(module_state_key=old_usage_id)
print(" ➔ Migrating {} student records to new url_name".format(num_entries)) num_entries = student_data.count()
student_data.update(module_state_key=new_block.location) if num_entries > 0:
except DuplicateItemError: print(u" ➔ Migrating {} student records to new block".format(num_entries))
print( student_data.update(module_state_key=block.location)
"\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
)
)
# Replace block with the new version and the new children: # Replace block with the new version and the new children:
studio_update_from_node(block, root) studio_update_from_node(block, root)
if delete_on_success:
store.delete_item(delete_on_success.location, user_id=None)
if parent_was_published: if parent_was_published:
store.publish(parent.location, user_id=None) store.publish(parent.location, user_id=None)
...@@ -130,9 +128,9 @@ if __name__ == '__main__': ...@@ -130,9 +128,9 @@ if __name__ == '__main__':
from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.keys import CourseKey
from xmodule.modulestore.django import modulestore from xmodule.modulestore.django import modulestore
print("┏━━━━━━━━━━━━━━━━━━━━━━━━━━┓") print(u"┏━━━━━━━━━━━━━━━━━━━━━━━━━━┓")
print("┃ Mentoring Upgrade Script ┃") print(u"┃ Mentoring Upgrade Script ┃")
print("┗━━━━━━━━━━━━━━━━━━━━━━━━━━┛") print(u"┗━━━━━━━━━━━━━━━━━━━━━━━━━━┛")
try: try:
course_id = CourseKey.from_string(sys.argv[1]) course_id = CourseKey.from_string(sys.argv[1])
...@@ -143,26 +141,58 @@ if __name__ == '__main__': ...@@ -143,26 +141,58 @@ if __name__ == '__main__':
course = store.get_course(course_id) course = store.get_course(course_id)
if course is None: if course is None:
sys.exit(u"Course '{}' not found.".format(unicode(course_id))) sys.exit(u"Course '{}' not found.".format(unicode(course_id)))
print(" ➔ Found course: {}".format(course.display_name)) print(u" ➔ Found course: {}".format(course.display_name))
print(" ➔ Searching for mentoring blocks") print(u" ➔ Searching for mentoring blocks")
blocks_found = [] blocks_found = []
def find_mentoring_blocks(block): 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) blocks_found.append(block.scope_ids.usage_id)
elif block.has_children: elif block.has_children:
for child_id in block.children: for child_id in block.children:
find_mentoring_blocks(block.runtime.get_block(child_id)) find_mentoring_blocks(block.runtime.get_block(child_id))
find_mentoring_blocks(course) find_mentoring_blocks(course)
total = len(blocks_found) 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): with store.bulk_operations(course.location.course_key):
count = 1 count = 1
for block_id in blocks_found: for block_id in blocks_found:
block = course.runtime.get_block(block_id) 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 count += 1
upgrade_block(block) upgrade_block(block)
print(" ➔ Complete.") print(u" ➔ Complete.")
problem_builder/v1/xml_changes.py
...@@ -184,7 +184,6 @@ class ReadOnlyAnswerToRecap(Change): ...@@ -184,7 +184,6 @@ class ReadOnlyAnswerToRecap(Change):
def apply(self): def apply(self):
self.node.tag = "pb-answer-recap" self.node.tag = "pb-answer-recap"
self.node.attrib
self.node.attrib.pop("read_only") self.node.attrib.pop("read_only")
for name in self.node.attrib: for name in self.node.attrib:
if name != "name": if name != "name":
...@@ -258,28 +257,29 @@ class TipChanges(Change): ...@@ -258,28 +257,29 @@ class TipChanges(Change):
else: else:
p.attrib[list_name] = value p.attrib[list_name] = value
if len(self.node.attrib) > 1:
warnings.warn("Invalid <tip> element found.")
return
mode = self.node.attrib.keys()[0]
value = self.node.attrib[mode]
if p.tag == "pb-mrq": 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) 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) add_to_list("required_choices", value)
elif mode != "reject": elif self.node.attrib.get("reject"):
warnings.warn("Invalid <tip> element: has {}={}".format(mode, value)) value = self.node.attrib.pop("reject")
else:
warnings.warn("Invalid <tip> element found.")
return return
else: else:
# This is an MCQ or Rating question: # 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) add_to_list("correct_choices", value)
elif mode != "reject": elif self.node.attrib.get("reject"):
warnings.warn("Invalid <tip> element: has {}={}".format(mode, value)) value = self.node.attrib.pop("reject")
else:
warnings.warn("Invalid <tip> element found.")
return return
self.node.attrib["values"] = value self.node.attrib["values"] = value
self.node.attrib.pop(mode)
class SharedHeaderToHTML(Change): class SharedHeaderToHTML(Change):
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment