Merge pull request #111 from edx/feature/christina/unify-fields

Move "stringy" functionality into xblock fields

CHANGELOG.rst

@@ -5,6 +5,10 @@ These are notable changes in edx-platform.  This is a rolling list of changes,
 in roughly chronological order, most recent first.  Add your entries at or near
 the top.  Include a label indicating the component affected.
 
+Studio, LMS: Make ModelTypes more strict about their expected content (for
+instance, Boolean, Integer, String), but also allow them to hold either the
+typed value, or a String that can be converted to their typed value. For example,
+an Integer can contain 3 or '3'. This changed an update to the xblock library.
 
 LMS: Some errors handling Non-ASCII data in XML courses have been fixed.
 

cms/djangoapps/contentstore/features/advanced-settings.feature

@@ -28,11 +28,18 @@ Feature: Advanced (manual) course policy
 
   Scenario: Test how multi-line input appears
     Given I am on the Advanced Course Settings page in Studio
-    When I create a JSON object as a value
+    When I create a JSON object as a value for "discussion_topics"
     Then it is displayed as formatted
     And I reload the page
     Then it is displayed as formatted
 
+  Scenario: Test error if value supplied is of the wrong type
+    Given I am on the Advanced Course Settings page in Studio
+    When I create a JSON object as a value for "display_name"
+    Then I get an error on save
+    And I reload the page
+    Then the policy key value is unchanged
+
   Scenario: Test automatic quoting of non-JSON values
     Given I am on the Advanced Course Settings page in Studio
     When I create a non-JSON value not in quotes

cms/djangoapps/contentstore/features/advanced-settings.py

@@ -2,8 +2,7 @@
 #pylint: disable=W0621
 
 from lettuce import world, step
-from common import *
-from nose.tools import assert_false, assert_equal
+from nose.tools import assert_false, assert_equal, assert_regexp_matches
 
 """
 http://selenium.googlecode.com/svn/trunk/docs/api/py/webdriver/selenium.webdriver.common.keys.html
@@ -52,9 +51,9 @@ def edit_the_value_of_a_policy_key_and_save(step):
     change_display_name_value(step, '"foo"')
 
 
-@step('I create a JSON object as a value$')
-def create_JSON_object(step):
-    change_display_name_value(step, '{"key": "value", "key_2": "value_2"}')
+@step('I create a JSON object as a value for "(.*)"$')
+def create_JSON_object(step, key):
+    change_value(step, key, '{"key": "value", "key_2": "value_2"}')
 
 
 @step('I create a non-JSON value not in quotes$')
@@ -82,7 +81,12 @@ def they_are_alphabetized(step):
 
 @step('it is displayed as formatted$')
 def it_is_formatted(step):
-    assert_policy_entries([DISPLAY_NAME_KEY], ['{\n    "key": "value",\n    "key_2": "value_2"\n}'])
+    assert_policy_entries(['discussion_topics'], ['{\n    "key": "value",\n    "key_2": "value_2"\n}'])
+
+
+@step('I get an error on save$')
+def error_on_save(step):
+    assert_regexp_matches(world.css_text('#notification-error-description'), 'Incorrect setting format')
 
 
 @step('it is displayed as a string')
@@ -124,11 +128,16 @@ def get_display_name_value():
 
 
 def change_display_name_value(step, new_value):
+    change_value(step, DISPLAY_NAME_KEY, new_value)
 
-    world.css_find(".CodeMirror")[get_index_of(DISPLAY_NAME_KEY)].click()
+
+def change_value(step, key, new_value):
+    index = get_index_of(key)
+    world.css_find(".CodeMirror")[index].click()
     g = world.css_find("div.CodeMirror.CodeMirror-focused > div > textarea")
-    display_name = get_display_name_value()
-    for count in range(len(display_name)):
+    current_value = world.css_find(VALUE_CSS)[index].value
+    g._element.send_keys(Keys.CONTROL + Keys.END)
+    for count in range(len(current_value)):
         g._element.send_keys(Keys.END, Keys.BACK_SPACE)
         # Must delete "" before typing the JSON value
     g._element.send_keys(Keys.END, Keys.BACK_SPACE, Keys.BACK_SPACE, new_value)

cms/djangoapps/contentstore/features/problem-editor.py

@@ -41,7 +41,9 @@ def i_see_five_settings_with_values(step):
 
 @step('I can modify the display name')
 def i_can_modify_the_display_name(step):
-    world.get_setting_entry(DISPLAY_NAME).find_by_css('.setting-input')[0].fill('modified')
+    # Verifying that the display name can be a string containing a floating point value
+    # (to confirm that we don't throw an error because it is of the wrong type).
+    world.get_setting_entry(DISPLAY_NAME).find_by_css('.setting-input')[0].fill('3.4')
     verify_modified_display_name()
 
 
@@ -172,7 +174,7 @@ def verify_modified_randomization():
 
 
 def verify_modified_display_name():
-    world.verify_setting_entry(world.get_setting_entry(DISPLAY_NAME), DISPLAY_NAME, 'modified', True)
+    world.verify_setting_entry(world.get_setting_entry(DISPLAY_NAME), DISPLAY_NAME, '3.4', True)
 
 
 def verify_modified_display_name_with_special_chars():

cms/djangoapps/contentstore/tests/test_checklists.py

@@ -19,6 +19,24 @@ class ChecklistTestCase(CourseTestCase):
         modulestore = get_modulestore(self.course.location)
         return modulestore.get_item(self.course.location).checklists
 
+
+    def compare_checklists(self, persisted, request):
+        """
+        Handles url expansion as possible difference and descends into guts
+        :param persisted:
+        :param request:
+        """
+        self.assertEqual(persisted['short_description'], request['short_description'])
+        compare_urls = (persisted.get('action_urls_expanded') == request.get('action_urls_expanded'))
+        for pers, req in zip(persisted['items'], request['items']):
+            self.assertEqual(pers['short_description'], req['short_description'])
+        self.assertEqual(pers['long_description'], req['long_description'])
+        self.assertEqual(pers['is_checked'], req['is_checked'])
+        if compare_urls:
+            self.assertEqual(pers['action_url'], req['action_url'])
+        self.assertEqual(pers['action_text'], req['action_text'])
+        self.assertEqual(pers['action_external'], req['action_external'])
+
     def test_get_checklists(self):
         """ Tests the get checklists method. """
         checklists_url = get_url_reverse('Checklists', self.course)
@@ -31,9 +49,9 @@ class ChecklistTestCase(CourseTestCase):
         self.course.checklists = None
         modulestore = get_modulestore(self.course.location)
         modulestore.update_metadata(self.course.location, own_metadata(self.course))
-        self.assertEquals(self.get_persisted_checklists(), None)
+        self.assertEqual(self.get_persisted_checklists(), None)
         response = self.client.get(checklists_url)
-        self.assertEquals(payload, response.content)
+        self.assertEqual(payload, response.content)
 
     def test_update_checklists_no_index(self):
         """ No checklist index, should return all of them. """
@@ -43,7 +61,8 @@ class ChecklistTestCase(CourseTestCase):
             'name': self.course.location.name})
 
         returned_checklists = json.loads(self.client.get(update_url).content)
-        self.assertListEqual(self.get_persisted_checklists(), returned_checklists)
+        for pay, resp in zip(self.get_persisted_checklists(), returned_checklists):
+            self.compare_checklists(pay, resp)
 
     def test_update_checklists_index_ignored_on_get(self):
         """ Checklist index ignored on get. """
@@ -53,7 +72,8 @@ class ChecklistTestCase(CourseTestCase):
                                                            'checklist_index': 1})
 
         returned_checklists = json.loads(self.client.get(update_url).content)
-        self.assertListEqual(self.get_persisted_checklists(), returned_checklists)
+        for pay, resp in zip(self.get_persisted_checklists(), returned_checklists):
+            self.compare_checklists(pay, resp)
 
     def test_update_checklists_post_no_index(self):
         """ No checklist index, will error on post. """
@@ -78,13 +98,18 @@ class ChecklistTestCase(CourseTestCase):
                                                            'course': self.course.location.course,
                                                            'name': self.course.location.name,
                                                            'checklist_index': 2})
+
+        def get_first_item(checklist):
+            return checklist['items'][0]
+
         payload = self.course.checklists[2]
-        self.assertFalse(payload.get('is_checked'))
-        payload['is_checked'] = True
+        self.assertFalse(get_first_item(payload).get('is_checked'))
+        get_first_item(payload)['is_checked'] = True
 
         returned_checklist = json.loads(self.client.post(update_url, json.dumps(payload), "application/json").content)
-        self.assertTrue(returned_checklist.get('is_checked'))
-        self.assertEqual(self.get_persisted_checklists()[2], returned_checklist)
+        self.assertTrue(get_first_item(returned_checklist).get('is_checked'))
+        pers = self.get_persisted_checklists()
+        self.compare_checklists(pers[2], returned_checklist)
 
     def test_update_checklists_delete_unsupported(self):
         """ Delete operation is not supported. """
@@ -93,4 +118,4 @@ class ChecklistTestCase(CourseTestCase):
                                                            'name': self.course.location.name,
                                                            'checklist_index': 100})
         response = self.client.delete(update_url)
-        self.assertContains(response, 'Unsupported request', status_code=400)
\ No newline at end of file
+        self.assertContains(response, 'Unsupported request', status_code=400)

cms/djangoapps/contentstore/views/course.py

@@ -402,8 +402,11 @@ def course_advanced_updates(request, org, course, name):
                         request_body.update({'tabs': new_tabs})
                         # Indicate that tabs should *not* be filtered out of the metadata
                         filter_tabs = False
-
-        response_json = json.dumps(CourseMetadata.update_from_json(location,
+        try:
+            response_json = json.dumps(CourseMetadata.update_from_json(location,
                                                                    request_body,
                                                                    filter_tabs=filter_tabs))
+        except (TypeError, ValueError), e:
+            return HttpResponseBadRequest("Incorrect setting format. " + str(e), content_type="text/plain")
+
         return HttpResponse(response_json, mimetype="application/json")

cms/xmodule_namespace.py

@@ -5,7 +5,6 @@ Namespace defining common fields used by Studio for all blocks
 import datetime
 
 from xblock.core import Namespace, Scope, ModelType, String
-from xmodule.fields import StringyBoolean
 
 
 class DateTuple(ModelType):
@@ -28,4 +27,3 @@ class CmsNamespace(Namespace):
     """
     published_date = DateTuple(help="Date when the module was published", scope=Scope.settings)
     published_by = String(help="Id of the user who published this module", scope=Scope.settings)
-

common/lib/xmodule/xmodule/abtest_module.py

@@ -6,7 +6,7 @@ from xmodule.x_module import XModule
 from xmodule.raw_module import RawDescriptor
 from xmodule.xml_module import XmlDescriptor
 from xmodule.exceptions import InvalidDefinitionError
-from xblock.core import String, Scope, Object
+from xblock.core import String, Scope, Dict
 
 DEFAULT = "_DEFAULT_GROUP"
 
@@ -32,9 +32,9 @@ def group_from_value(groups, v):
 
 
 class ABTestFields(object):
-    group_portions = Object(help="What proportions of students should go in each group", default={DEFAULT: 1}, scope=Scope.content)
-    group_assignments = Object(help="What group this user belongs to", scope=Scope.preferences, default={})
-    group_content = Object(help="What content to display to each group", scope=Scope.content, default={DEFAULT: []})
+    group_portions = Dict(help="What proportions of students should go in each group", default={DEFAULT: 1}, scope=Scope.content)
+    group_assignments = Dict(help="What group this user belongs to", scope=Scope.preferences, default={})
+    group_content = Dict(help="What content to display to each group", scope=Scope.content, default={DEFAULT: []})
     experiment = String(help="Experiment that this A/B test belongs to", scope=Scope.content)
     has_children = True
 

common/lib/xmodule/xmodule/capa_module.py

@@ -18,8 +18,8 @@ from .progress import Progress
 from xmodule.x_module import XModule
 from xmodule.raw_module import RawDescriptor
 from xmodule.exceptions import NotFoundError, ProcessingError
-from xblock.core import Scope, String, Boolean, Object
-from .fields import Timedelta, Date, StringyInteger, StringyFloat
+from xblock.core import Scope, String, Boolean, Dict, Integer, Float
+from .fields import Timedelta, Date
 from django.utils.timezone import UTC
 
 log = logging.getLogger("mitx.courseware")
@@ -65,8 +65,8 @@ class ComplexEncoder(json.JSONEncoder):
 
 
 class CapaFields(object):
-    attempts = StringyInteger(help="Number of attempts taken by the student on this problem", default=0, scope=Scope.user_state)
-    max_attempts = StringyInteger(
+    attempts = Integer(help="Number of attempts taken by the student on this problem", default=0, scope=Scope.user_state)
+    max_attempts = Integer(
         display_name="Maximum Attempts",
         help="Defines the number of times a student can try to answer this problem. If the value is not set, infinite attempts are allowed.",
         values={"min": 0}, scope=Scope.settings
@@ -95,12 +95,12 @@ class CapaFields(object):
                                                         {"display_name": "Per Student", "value": "per_student"}]
     )
     data = String(help="XML data for the problem", scope=Scope.content)
-    correct_map = Object(help="Dictionary with the correctness of current student answers", scope=Scope.user_state, default={})
-    input_state = Object(help="Dictionary for maintaining the state of inputtypes", scope=Scope.user_state)
-    student_answers = Object(help="Dictionary with the current student responses", scope=Scope.user_state)
+    correct_map = Dict(help="Dictionary with the correctness of current student answers", scope=Scope.user_state, default={})
+    input_state = Dict(help="Dictionary for maintaining the state of inputtypes", scope=Scope.user_state)
+    student_answers = Dict(help="Dictionary with the current student responses", scope=Scope.user_state)
     done = Boolean(help="Whether the student has answered the problem", scope=Scope.user_state)
-    seed = StringyInteger(help="Random seed for this student", scope=Scope.user_state)
-    weight = StringyFloat(
+    seed = Integer(help="Random seed for this student", scope=Scope.user_state)
+    weight = Float(
         display_name="Problem Weight",
         help="Defines the number of points each problem is worth. If the value is not set, each response field in the problem is worth one point.",
         values={"min": 0, "step": .1},
@@ -315,7 +315,7 @@ class CapaModule(CapaFields, XModule):
         # If the user has forced the save button to display,
         # then show it as long as the problem is not closed
         # (past due / too many attempts)
-        if self.force_save_button == "true":
+        if self.force_save_button:
             return not self.closed()
         else:
             is_survey_question = (self.max_attempts == 0)

common/lib/xmodule/xmodule/combined_open_ended_module.py

@@ -5,10 +5,10 @@ from pkg_resources import resource_string
 
 from xmodule.raw_module import RawDescriptor
 from .x_module import XModule
-from xblock.core import Integer, Scope, String, List
+from xblock.core import Integer, Scope, String, List, Float, Boolean
 from xmodule.open_ended_grading_classes.combined_open_ended_modulev1 import CombinedOpenEndedV1Module, CombinedOpenEndedV1Descriptor
 from collections import namedtuple
-from .fields import Date, StringyFloat, StringyInteger, StringyBoolean
+from .fields import Date
 
 log = logging.getLogger("mitx.courseware")
 
@@ -53,27 +53,27 @@ class CombinedOpenEndedFields(object):
         help="This name appears in the horizontal navigation at the top of the page.",
         default="Open Ended Grading", scope=Scope.settings
     )
-    current_task_number = StringyInteger(help="Current task that the student is on.", default=0, scope=Scope.user_state)
+    current_task_number = Integer(help="Current task that the student is on.", default=0, scope=Scope.user_state)
     task_states = List(help="List of state dictionaries of each task within this module.", scope=Scope.user_state)
     state = String(help="Which step within the current task that the student is on.", default="initial",
                    scope=Scope.user_state)
-    student_attempts = StringyInteger(help="Number of attempts taken by the student on this problem", default=0,
+    student_attempts = Integer(help="Number of attempts taken by the student on this problem", default=0,
                                       scope=Scope.user_state)
-    ready_to_reset = StringyBoolean(
+    ready_to_reset = Boolean(
         help="If the problem is ready to be reset or not.", default=False,
         scope=Scope.user_state
     )
-    attempts = StringyInteger(
+    attempts = Integer(
         display_name="Maximum Attempts",
         help="The number of times the student can try to answer this problem.", default=1,
         scope=Scope.settings, values = {"min" : 1 }
     )
-    is_graded = StringyBoolean(display_name="Graded", help="Whether or not the problem is graded.", default=False, scope=Scope.settings)
-    accept_file_upload = StringyBoolean(
+    is_graded = Boolean(display_name="Graded", help="Whether or not the problem is graded.", default=False, scope=Scope.settings)
+    accept_file_upload = Boolean(
         display_name="Allow File Uploads",
         help="Whether or not the student can submit files as a response.", default=False, scope=Scope.settings
     )
-    skip_spelling_checks = StringyBoolean(
+    skip_spelling_checks = Boolean(
         display_name="Disable Quality Filter",
         help="If False, the Quality Filter is enabled and submissions with poor spelling, short length, or poor grammar will not be peer reviewed.",
         default=False, scope=Scope.settings
@@ -86,7 +86,7 @@ class CombinedOpenEndedFields(object):
     )
     version = VersionInteger(help="Current version number", default=DEFAULT_VERSION, scope=Scope.settings)
     data = String(help="XML data for the problem", scope=Scope.content)
-    weight = StringyFloat(
+    weight = Float(
         display_name="Problem Weight",
         help="Defines the number of points each problem is worth. If the value is not set, each problem is worth one point.",
         scope=Scope.settings, values = {"min" : 0 , "step": ".1"}

common/lib/xmodule/xmodule/course_module.py

@@ -15,7 +15,7 @@ from xmodule.util.decorators import lazyproperty
 from xmodule.graders import grader_from_conf
 import json
 
-from xblock.core import Scope, List, String, Object, Boolean
+from xblock.core import Scope, List, String, Dict, Boolean
 from .fields import Date
 from django.utils.timezone import UTC
 from xmodule.util import date_utils
@@ -154,25 +154,25 @@ class CourseFields(object):
     start = Date(help="Start time when this module is visible", scope=Scope.settings)
     end = Date(help="Date that this class ends", scope=Scope.settings)
     advertised_start = String(help="Date that this course is advertised to start", scope=Scope.settings)
-    grading_policy = Object(help="Grading policy definition for this class", scope=Scope.content)
+    grading_policy = Dict(help="Grading policy definition for this class", scope=Scope.content)
     show_calculator = Boolean(help="Whether to show the calculator in this course", default=False, scope=Scope.settings)
     display_name = String(help="Display name for this module", scope=Scope.settings)
     tabs = List(help="List of tabs to enable in this course", scope=Scope.settings)
     end_of_course_survey_url = String(help="Url for the end-of-course survey", scope=Scope.settings)
     discussion_blackouts = List(help="List of pairs of start/end dates for discussion blackouts", scope=Scope.settings)
-    discussion_topics = Object(
+    discussion_topics = Dict(
         help="Map of topics names to ids",
         scope=Scope.settings
         )
-    testcenter_info = Object(help="Dictionary of Test Center info", scope=Scope.settings)
+    testcenter_info = Dict(help="Dictionary of Test Center info", scope=Scope.settings)
     announcement = Date(help="Date this course is announced", scope=Scope.settings)
-    cohort_config = Object(help="Dictionary defining cohort configuration", scope=Scope.settings)
+    cohort_config = Dict(help="Dictionary defining cohort configuration", scope=Scope.settings)
     is_new = Boolean(help="Whether this course should be flagged as new", scope=Scope.settings)
     no_grade = Boolean(help="True if this course isn't graded", default=False, scope=Scope.settings)
     disable_progress_graph = Boolean(help="True if this course shouldn't display the progress graph", default=False, scope=Scope.settings)
     pdf_textbooks = List(help="List of dictionaries containing pdf_textbook configuration", scope=Scope.settings)
     html_textbooks = List(help="List of dictionaries containing html_textbook configuration", scope=Scope.settings)
-    remote_gradebook = Object(scope=Scope.settings)
+    remote_gradebook = Dict(scope=Scope.settings)
     allow_anonymous = Boolean(scope=Scope.settings, default=True)
     allow_anonymous_to_peers = Boolean(scope=Scope.settings, default=False)
     advanced_modules = List(help="Beta modules used in your course", scope=Scope.settings)

common/lib/xmodule/xmodule/fields.py

@@ -6,7 +6,6 @@ from xblock.core import ModelType
 import datetime
 import dateutil.parser
 
-from xblock.core import Integer, Float, Boolean
 from django.utils.timezone import UTC
 
 log = logging.getLogger(__name__)
@@ -93,42 +92,3 @@ class Timedelta(ModelType):
             if cur_value > 0:
                 values.append("%d %s" % (cur_value, attr))
         return ' '.join(values)
-
-
-class StringyInteger(Integer):
-    """
-    A model type that converts from strings to integers when reading from json.
-    If value does not parse as an int, returns None.
-    """
-    def from_json(self, value):
-        try:
-            return int(value)
-        except Exception:
-            return None
-
-
-class StringyFloat(Float):
-    """
-    A model type that converts from string to floats when reading from json.
-    If value does not parse as a float, returns None.
-    """
-    def from_json(self, value):
-        try:
-            return float(value)
-        except:
-            return None
-
-
-class StringyBoolean(Boolean):
-    """
-    Reads strings from JSON as booleans.
-
-    If the string is 'true' (case insensitive), then return True,
-    otherwise False.
-
-    JSON values that aren't strings are returned as-is.
-    """
-    def from_json(self, value):
-        if isinstance(value, basestring):
-            return value.lower() == 'true'
-        return value

common/lib/xmodule/xmodule/peer_grading_module.py

@@ -10,8 +10,8 @@ from .x_module import XModule
 from xmodule.raw_module import RawDescriptor
 from xmodule.modulestore.django import modulestore
 from .timeinfo import TimeInfo
-from xblock.core import Object, String, Scope
-from xmodule.fields import Date, StringyFloat, StringyInteger, StringyBoolean
+from xblock.core import Dict, String, Scope, Boolean, Integer, Float
+from xmodule.fields import Date
 
 from xmodule.open_ended_grading_classes.peer_grading_service import PeerGradingService, GradingServiceError, MockPeerGradingService
 from open_ended_grading_classes import combined_open_ended_rubric
@@ -21,7 +21,6 @@ log = logging.getLogger(__name__)
 
 USE_FOR_SINGLE_LOCATION = False
 LINK_TO_LOCATION = ""
-TRUE_DICT = [True, "True", "true", "TRUE"]
 MAX_SCORE = 1
 IS_GRADED = False
 
@@ -29,7 +28,7 @@ EXTERNAL_GRADER_NO_CONTACT_ERROR = "Failed to contact external graders.  Please
 
 
 class PeerGradingFields(object):
-    use_for_single_location = StringyBoolean(
+    use_for_single_location = Boolean(
         display_name="Show Single Problem",
         help='When True, only the single problem specified by "Link to Problem Location" is shown. '
              'When False, a panel is displayed with all problems available for peer grading.',
@@ -40,22 +39,22 @@ class PeerGradingFields(object):
         help='The location of the problem being graded. Only used when "Show Single Problem" is True.',
         default=LINK_TO_LOCATION, scope=Scope.settings
     )
-    is_graded = StringyBoolean(
+    is_graded = Boolean(
         display_name="Graded",
         help='Defines whether the student gets credit for grading this problem. Only used when "Show Single Problem" is True.',
         default=IS_GRADED, scope=Scope.settings
     )
     due_date = Date(help="Due date that should be displayed.", default=None, scope=Scope.settings)
     grace_period_string = String(help="Amount of grace to give on the due date.", default=None, scope=Scope.settings)
-    max_grade = StringyInteger(
+    max_grade = Integer(
         help="The maximum grade that a student can receive for this problem.", default=MAX_SCORE,
         scope=Scope.settings, values={"min": 0}
     )
-    student_data_for_location = Object(
+    student_data_for_location = Dict(
         help="Student data for a given peer grading problem.",
         scope=Scope.user_state
     )
-    weight = StringyFloat(
+    weight = Float(
         display_name="Problem Weight",
         help="Defines the number of points each problem is worth. If the value is not set, each problem is worth one point.",
         scope=Scope.settings, values={"min": 0, "step": ".1"}
@@ -85,7 +84,7 @@ class PeerGradingModule(PeerGradingFields, XModule):
         else:
             self.peer_gs = MockPeerGradingService()
 
-        if self.use_for_single_location in TRUE_DICT:
+        if self.use_for_single_location:
             try:
                 self.linked_problem = modulestore().get_instance(self.system.course_id, self.link_to_location)
             except:
@@ -113,7 +112,7 @@ class PeerGradingModule(PeerGradingFields, XModule):
         if not self.ajax_url.endswith("/"):
             self.ajax_url = self.ajax_url + "/"
 
-        # StringyInteger could return None, so keep this check.
+        # Integer could return None, so keep this check.
         if not isinstance(self.max_grade, int):
             raise TypeError("max_grade needs to be an integer.")
 
@@ -147,7 +146,7 @@ class PeerGradingModule(PeerGradingFields, XModule):
         """
         if self.closed():
             return self.peer_grading_closed()
-        if self.use_for_single_location not in TRUE_DICT:
+        if not self.use_for_single_location:
             return self.peer_grading()
         else:
             return self.peer_grading_problem({'location': self.link_to_location})['html']
@@ -204,7 +203,7 @@ class PeerGradingModule(PeerGradingFields, XModule):
             'score': score,
             'total': max_score,
         }
-        if self.use_for_single_location not in TRUE_DICT or self.is_graded not in TRUE_DICT:
+        if not self.use_for_single_location or not self.is_graded:
             return score_dict
 
         try:
@@ -239,7 +238,7 @@ class PeerGradingModule(PeerGradingFields, XModule):
               randomization, and 5/7 on another
         '''
         max_grade = None
-        if self.use_for_single_location in TRUE_DICT and self.is_graded in TRUE_DICT:
+        if self.use_for_single_location and self.is_graded:
             max_grade = self.max_grade
         return max_grade
 
@@ -557,7 +556,7 @@ class PeerGradingModule(PeerGradingFields, XModule):
         Show individual problem interface
         '''
         if get is None or get.get('location') is None:
-            if self.use_for_single_location not in TRUE_DICT:
+            if not self.use_for_single_location:
                 # This is an error case, because it must be set to use a single location to be called without get parameters
                 # This is a dev_facing_error
                 log.error(

common/lib/xmodule/xmodule/poll_module.py

@@ -19,7 +19,7 @@ from xmodule.x_module import XModule
 from xmodule.stringify import stringify_children
 from xmodule.mako_module import MakoModuleDescriptor
 from xmodule.xml_module import XmlDescriptor
-from xblock.core import Scope, String, Object, Boolean, List
+from xblock.core import Scope, String, Dict, Boolean, List
 
 log = logging.getLogger(__name__)
 
@@ -30,7 +30,7 @@ class PollFields(object):
 
     voted = Boolean(help="Whether this student has voted on the poll", scope=Scope.user_state, default=False)
     poll_answer = String(help="Student answer", scope=Scope.user_state, default='')
-    poll_answers = Object(help="All possible answers for the poll fro other students", scope=Scope.content)
+    poll_answers = Dict(help="All possible answers for the poll fro other students", scope=Scope.content)
 
     answers = List(help="Poll answers from xml", scope=Scope.content, default=[])
     question = String(help="Poll question", scope=Scope.content, default='')

common/lib/xmodule/xmodule/tests/test_fields.py

 """Tests for classes defined in fields.py."""
 import datetime
 import unittest
-from xmodule.fields import Date, StringyFloat, StringyInteger, StringyBoolean
 from django.utils.timezone import UTC
+from xmodule.fields import Date, Timedelta
+
 
 class DateTest(unittest.TestCase):
     date = Date()
@@ -70,54 +71,22 @@ class DateTest(unittest.TestCase):
             "2012-12-31T23:00:01-01:00")
 
 
-class StringyIntegerTest(unittest.TestCase):
-    def assertEquals(self, expected, arg):
-        self.assertEqual(expected, StringyInteger().from_json(arg))
-
-    def test_integer(self):
-        self.assertEquals(5, '5')
-        self.assertEquals(0, '0')
-        self.assertEquals(-1023, '-1023')
-
-    def test_none(self):
-        self.assertEquals(None, None)
-        self.assertEquals(None, 'abc')
-        self.assertEquals(None, '[1]')
-        self.assertEquals(None, '1.023')
-
-
-class StringyFloatTest(unittest.TestCase):
-
-    def assertEquals(self, expected, arg):
-        self.assertEqual(expected, StringyFloat().from_json(arg))
-
-    def test_float(self):
-        self.assertEquals(.23, '.23')
-        self.assertEquals(5, '5')
-        self.assertEquals(0, '0.0')
-        self.assertEquals(-1023.22, '-1023.22')
-
-    def test_none(self):
-        self.assertEquals(None, None)
-        self.assertEquals(None, 'abc')
-        self.assertEquals(None, '[1]')
-
+class TimedeltaTest(unittest.TestCase):
+    delta = Timedelta()
 
-class StringyBooleanTest(unittest.TestCase):
-
-    def assertEquals(self, expected, arg):
-        self.assertEqual(expected, StringyBoolean().from_json(arg))
-
-    def test_false(self):
-        self.assertEquals(False, "false")
-        self.assertEquals(False, "False")
-        self.assertEquals(False, "")
-        self.assertEquals(False, "hahahahah")
-
-    def test_true(self):
-        self.assertEquals(True, "true")
-        self.assertEquals(True, "TruE")
+    def test_from_json(self):
+        self.assertEqual(
+            TimedeltaTest.delta.from_json('1 day 12 hours 59 minutes 59 seconds'),
+            datetime.timedelta(days=1, hours=12, minutes=59, seconds=59)
+        )
 
-    def test_pass_through(self):
-        self.assertEquals(123, 123)
+        self.assertEqual(
+            TimedeltaTest.delta.from_json('1 day 46799 seconds'),
+            datetime.timedelta(days=1, seconds=46799)
+        )
 
+    def test_to_json(self):
+        self.assertEqual(
+            '1 days 46799 seconds',
+            TimedeltaTest.delta.to_json(datetime.timedelta(days=1, hours=12, minutes=59, seconds=59))
+        )

common/lib/xmodule/xmodule/word_cloud_module.py

@@ -14,8 +14,7 @@ from xmodule.raw_module import RawDescriptor
 from xmodule.editing_module import MetadataOnlyEditingDescriptor
 from xmodule.x_module import XModule
 
-from xblock.core import Scope, Object, Boolean, List
-from fields import StringyBoolean, StringyInteger
+from xblock.core import Scope, Dict, Boolean, List, Integer
 
 log = logging.getLogger(__name__)
 
@@ -32,21 +31,21 @@ def pretty_bool(value):
 
 class WordCloudFields(object):
     """XFields for word cloud."""
-    num_inputs = StringyInteger(
+    num_inputs = Integer(
         display_name="Inputs",
         help="Number of text boxes available for students to input words/sentences.",
         scope=Scope.settings,
         default=5,
         values={"min": 1}
     )
-    num_top_words = StringyInteger(
+    num_top_words = Integer(
         display_name="Maximum Words",
         help="Maximum number of words to be displayed in generated word cloud.",
         scope=Scope.settings,
         default=250,
         values={"min": 1}
     )
-    display_student_percents = StringyBoolean(
+    display_student_percents = Boolean(
         display_name="Show Percents",
         help="Statistics are shown for entered words near that word.",
         scope=Scope.settings,
@@ -64,11 +63,11 @@ class WordCloudFields(object):
         scope=Scope.user_state,
         default=[]
     )
-    all_words = Object(
+    all_words = Dict(
         help="All possible words from all students.",
         scope=Scope.content
     )
-    top_words = Object(
+    top_words = Dict(
         help="Top num_top_words words for word cloud.",
         scope=Scope.content
     )

common/lib/xmodule/xmodule/xml_module.py

@@ -6,7 +6,7 @@ import sys
 from collections import namedtuple
 from lxml import etree
 
-from xblock.core import Object, Scope
+from xblock.core import Dict, Scope
 from xmodule.x_module import (XModuleDescriptor, policy_key)
 from xmodule.modulestore import Location
 from xmodule.modulestore.inheritance import own_metadata
@@ -79,12 +79,53 @@ class AttrMap(_AttrMapBase):
         return _AttrMapBase.__new__(_cls, from_xml, to_xml)
 
 
+def serialize_field(value):
+    """
+        Return a string version of the value (where value is the JSON-formatted, internally stored value).
+
+        By default, this is the result of calling json.dumps on the input value.
+        """
+    return json.dumps(value)
+
+
+def deserialize_field(field, value):
+    """
+    Deserialize the string version to the value stored internally.
+
+    Note that this is not the same as the value returned by from_json, as model types typically store
+    their value internally as JSON. By default, this method will return the result of calling json.loads
+    on the supplied value, unless json.loads throws a TypeError, or the type of the value returned by json.loads
+    is not supported for this class (from_json throws an Error). In either of those cases, this method returns
+    the input value.
+    """
+    try:
+        deserialized = json.loads(value)
+        if deserialized is None:
+            return deserialized
+        try:
+            field.from_json(deserialized)
+            return deserialized
+        except (ValueError, TypeError):
+            # Support older serialized version, which was just a string, not result of json.dumps.
+            # If the deserialized version cannot be converted to the type (via from_json),
+            # just return the original value. For example, if a string value of '3.4' was
+            # stored for a String field (before we started storing the result of json.dumps),
+            # then it would be deserialized as 3.4, but 3.4 is not supported for a String
+            # field. Therefore field.from_json(3.4) will throw an Error, and we should
+            # actually return the original value of '3.4'.
+            return value
+
+    except (ValueError, TypeError):
+        # Support older serialized version.
+        return value
+
+
 class XmlDescriptor(XModuleDescriptor):
     """
     Mixin class for standardized parsing of from xml
     """
 
-    xml_attributes = Object(help="Map of unhandled xml attributes, used only for storage between import and export",
+    xml_attributes = Dict(help="Map of unhandled xml attributes, used only for storage between import and export",
         default={}, scope=Scope.settings)
 
     # Extension to append to filename paths
@@ -120,25 +161,15 @@ class XmlDescriptor(XModuleDescriptor):
 
     metadata_to_export_to_policy = ('discussion_topics')
 
-    # A dictionary mapping xml attribute names AttrMaps that describe how
-    # to import and export them
-    # Allow json to specify either the string "true", or the bool True.  The string is preferred.
-    to_bool = lambda val: val == 'true' or val == True
-    from_bool = lambda val: str(val).lower()
-    bool_map = AttrMap(to_bool, from_bool)
-
-    to_int = lambda val: int(val)
-    from_int = lambda val: str(val)
-    int_map = AttrMap(to_int, from_int)
-    xml_attribute_map = {
-        # type conversion: want True/False in python, "true"/"false" in xml
-        'graded': bool_map,
-        'hide_progress_tab': bool_map,
-        'allow_anonymous': bool_map,
-        'allow_anonymous_to_peers': bool_map,
-        'show_timezone': bool_map,
-    }
+    @classmethod
+    def get_map_for_field(cls, attr):
+        for field in set(cls.fields + cls.lms.fields):
+            if field.name == attr:
+                from_xml = lambda val: deserialize_field(field, val)
+                to_xml = lambda val : serialize_field(val)
+                return AttrMap(from_xml, to_xml)
 
+        return AttrMap()
 
     @classmethod
     def definition_from_xml(cls, xml_object, system):
@@ -188,7 +219,6 @@ class XmlDescriptor(XModuleDescriptor):
                 filepath, location.url(), str(err))
             raise Exception, msg, sys.exc_info()[2]
 
-
     @classmethod
     def load_definition(cls, xml_object, system, location):
         '''Load a descriptor definition from the specified xml_object.
@@ -246,7 +276,7 @@ class XmlDescriptor(XModuleDescriptor):
                     # don't load these
                     continue
 
-                attr_map = cls.xml_attribute_map.get(attr, AttrMap())
+                attr_map = cls.get_map_for_field(attr)
                 metadata[attr] = attr_map.from_xml(val)
         return metadata
 
@@ -258,7 +288,7 @@ class XmlDescriptor(XModuleDescriptor):
         through the attrmap.  Updates the metadata dict in place.
         """
         for attr in policy:
-            attr_map = cls.xml_attribute_map.get(attr, AttrMap())
+            attr_map = cls.get_map_for_field(attr)
             metadata[cls._translate(attr)] = attr_map.from_xml(policy[attr])
 
     @classmethod
@@ -347,7 +377,7 @@ class XmlDescriptor(XModuleDescriptor):
 
     def export_to_xml(self, resource_fs):
         """
-        Returns an xml string representign this module, and all modules
+        Returns an xml string representing this module, and all modules
         underneath it.  May also write required resources out to resource_fs
 
         Assumes that modules have single parentage (that no module appears twice
@@ -372,7 +402,7 @@ class XmlDescriptor(XModuleDescriptor):
             """Get the value for this attribute that we want to store.
             (Possible format conversion through an AttrMap).
              """
-            attr_map = self.xml_attribute_map.get(attr, AttrMap())
+            attr_map = self.get_map_for_field(attr)
             return attr_map.to_xml(self._model_data[attr])
 
         # Add the non-inherited metadata

common/test/data/full/course.xml

-<course filename="6.002_Spring_2012" slug="6.002_Spring_2012" graceperiod="1 day 12 hours 59 minutes 59 seconds" showanswer="attempted" rerandomize="never" name="6.002 Spring 2012" start="2015-07-17T12:00" course="full" org="edX" show_timezone="true"/>
+<course filename="6.002_Spring_2012" slug="6.002_Spring_2012" graceperiod="1 day 12 hours 59 minutes 59 seconds" showanswer="attempted" rerandomize="never" name="6.002 Spring 2012" start="2015-07-17T12:00" course="full" org="edX" show_timezone="true" advanced_modules="[&quot;videoalpha&quot;]"/>

lms/xmodule_namespace.py

 """
 Namespace that defines fields common to all blocks used in the LMS
 """
-from xblock.core import Namespace, Boolean, Scope, String
-from xmodule.fields import Date, Timedelta, StringyFloat, StringyBoolean
+from xblock.core import Namespace, Boolean, Scope, String, Float
+from xmodule.fields import Date, Timedelta
 
 
 class LmsNamespace(Namespace):
     """
     Namespace that defines fields common to all blocks used in the LMS
     """
-    hide_from_toc = StringyBoolean(
+    hide_from_toc = Boolean(
         help="Whether to display this module in the table of contents",
         default=False,
         scope=Scope.settings
@@ -37,7 +37,7 @@ class LmsNamespace(Namespace):
     )
     showanswer = String(help="When to show the problem answer to the student", scope=Scope.settings, default="closed")
     rerandomize = String(help="When to rerandomize the problem", default="always", scope=Scope.settings)
-    days_early_for_beta = StringyFloat(
+    days_early_for_beta = Float(
         help="Number of days early to show content to beta users",
         default=None,
         scope=Scope.settings

requirements/edx/github.txt

@@ -8,6 +8,6 @@
 -e git://github.com/eventbrite/zendesk.git@d53fe0e81b623f084e91776bcf6369f8b7b63879#egg=zendesk
 
 # Our libraries:
--e git+https://github.com/edx/XBlock.git@2144a25d#egg=XBlock
+-e git+https://github.com/edx/XBlock.git@4d8735e883#egg=XBlock
 -e git+https://github.com/edx/codejail.git@0a1b468#egg=codejail
 -e git+https://github.com/edx/diff-cover.git@v0.1.1#egg=diff_cover