diff --git a/cms/envs/common.py b/cms/envs/common.py
index 11c0842b8d..62a1a60b2a 100644
--- a/cms/envs/common.py
+++ b/cms/envs/common.py
@@ -215,6 +215,9 @@ FEATURES = {
# Show Language selector
'SHOW_LANGUAGE_SELECTOR': False,
+
+ # Set this to False to facilitate cleaning up invalid xml from your modulestore.
+ 'ENABLE_XBLOCK_XML_VALIDATION': True,
}
ENABLE_JASMINE = False
diff --git a/common/lib/xmodule/xmodule/capa_base.py b/common/lib/xmodule/xmodule/capa_base.py
index ce7b6e8f4f..783687fc8e 100644
--- a/common/lib/xmodule/xmodule/capa_base.py
+++ b/common/lib/xmodule/xmodule/capa_base.py
@@ -6,28 +6,27 @@ import hashlib
import json
import logging
import os
-import traceback
+import re
import struct
import sys
-import re
+import traceback
+from django.conf import settings
# We don't want to force a dependency on datadog, so make the import conditional
try:
import dogstats_wrapper as dog_stats_api
except ImportError:
dog_stats_api = None
+from pytz import utc
from capa.capa_problem import LoncapaProblem, LoncapaSystem
-from capa.responsetypes import StudentInputError, \
- ResponseError, LoncapaProblemError
+from capa.responsetypes import StudentInputError, ResponseError, LoncapaProblemError
from capa.util import convert_files_to_filenames, get_inner_html_from_xpath
-from .progress import Progress
-from xmodule.exceptions import NotFoundError
-from xblock.fields import Scope, String, Boolean, Dict, Integer, Float
-from .fields import Timedelta, Date
-from django.utils.timezone import UTC
+from xblock.fields import Boolean, Dict, Float, Integer, Scope, String, XMLString
from xmodule.capa_base_constants import RANDOMIZATION, SHOWANSWER
-from django.conf import settings
+from xmodule.exceptions import NotFoundError
+from .fields import Date, Timedelta
+from .progress import Progress
from openedx.core.djangolib.markup import HTML, Text
@@ -42,6 +41,8 @@ NUM_RANDOMIZATION_BINS = 20
# Never produce more than this many different seeds, no matter what.
MAX_RANDOMIZATION_BINS = 1000
+FEATURES = getattr(settings, 'FEATURES', {})
+
def randomization_bin(seed, problem_id):
"""
@@ -76,7 +77,7 @@ class ComplexEncoder(json.JSONEncoder):
"""
Extend the JSON encoder to correctly handle complex numbers
"""
- def default(self, obj):
+ def default(self, obj): # pylint: disable=method-hidden
"""
Print a nicely formatted complex number, or default to the JSON encoder
"""
@@ -157,7 +158,12 @@ class CapaFields(object):
{"display_name": _("Per Student"), "value": RANDOMIZATION.PER_STUDENT}
]
)
- data = String(help=_("XML data for the problem"), scope=Scope.content, default="")
+ data = XMLString(
+ help=_("XML data for the problem"),
+ scope=Scope.content,
+ enforce_type=FEATURES.get('ENABLE_XBLOCK_XML_VALIDATION', True),
+ default=""
+ )
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)
@@ -257,11 +263,13 @@ class CapaMixin(CapaFields):
)
)
# create a dummy problem with error message instead of failing
- problem_text = (u''
- u'Problem {url} has an error:{msg}'.format(
- url=self.location.to_deprecated_string(),
- msg=msg)
- )
+ problem_text = (
+ u''
+ u'Problem {url} has an error:{msg}'.format(
+ url=self.location.to_deprecated_string(),
+ msg=msg,
+ )
+ )
self.lcp = self.new_lcp(self.get_state_for_lcp(), text=problem_text)
else:
# add extra info and raise
@@ -349,7 +357,7 @@ class CapaMixin(CapaFields):
"""
Set the module's last submission time (when the problem was submitted)
"""
- self.last_submission_time = datetime.datetime.now(UTC())
+ self.last_submission_time = datetime.datetime.now(utc)
def get_score(self):
"""
@@ -803,7 +811,7 @@ class CapaMixin(CapaFields):
Is it now past this problem's due date, including grace period?
"""
return (self.close_date is not None and
- datetime.datetime.now(UTC()) > self.close_date)
+ datetime.datetime.now(utc) > self.close_date)
def closed(self):
"""
@@ -1093,7 +1101,7 @@ class CapaMixin(CapaFields):
metric_name = u'capa.check_problem.{}'.format
# Can override current time
- current_time = datetime.datetime.now(UTC())
+ current_time = datetime.datetime.now(utc)
if override_time is not False:
current_time = override_time
@@ -1128,8 +1136,9 @@ class CapaMixin(CapaFields):
# Wait time between resets: check if is too soon for submission.
if self.last_submission_time is not None and self.submission_wait_seconds != 0:
- if (current_time - self.last_submission_time).total_seconds() < self.submission_wait_seconds:
- remaining_secs = int(self.submission_wait_seconds - (current_time - self.last_submission_time).total_seconds())
+ seconds_since_submission = (current_time - self.last_submission_time).total_seconds()
+ if seconds_since_submission < self.submission_wait_seconds:
+ remaining_secs = int(self.submission_wait_seconds - seconds_since_submission)
msg = _(u'You must wait at least {wait_secs} between submissions. {remaining_secs} remaining.').format(
wait_secs=self.pretty_print_seconds(self.submission_wait_seconds),
remaining_secs=self.pretty_print_seconds(remaining_secs))
@@ -1343,7 +1352,7 @@ class CapaMixin(CapaFields):
log.warning('Input id %s is not mapped to an input type.', input_id)
answer_response = None
- for response, responder in self.lcp.responders.iteritems():
+ for responder in self.lcp.responders.itervalues():
if input_id in responder.answer_ids:
answer_response = responder
@@ -1406,8 +1415,10 @@ class CapaMixin(CapaFields):
if not self.lcp.supports_rescoring():
event_info['failure'] = 'unsupported'
self.track_function_unmask('problem_rescore_fail', event_info)
+ # pylint: disable=line-too-long
# Translators: 'rescoring' refers to the act of re-submitting a student's solution so it can get a new score.
raise NotImplementedError(_("Problem's definition does not support rescoring."))
+ # pylint: enable=line-too-long
if not self.done:
event_info['failure'] = 'unanswered'
@@ -1485,8 +1496,10 @@ class CapaMixin(CapaFields):
self.track_function_unmask('save_problem_fail', event_info)
return {
'success': False,
+ # pylint: disable=line-too-long
# Translators: 'closed' means the problem's due date has passed. You may no longer attempt to solve the problem.
- 'msg': _("Problem is closed.")
+ 'msg': _("Problem is closed."),
+ # pylint: enable=line-too-long
}
# Problem submitted. Student should reset before saving
@@ -1538,8 +1551,10 @@ class CapaMixin(CapaFields):
self.track_function_unmask('reset_problem_fail', event_info)
return {
'success': False,
+ # pylint: disable=line-too-long
# Translators: 'closed' means the problem's due date has passed. You may no longer attempt to solve the problem.
'msg': _("You cannot select Reset for a problem that is closed."),
+ # pylint: enable=line-too-long
}
if not self.is_submitted():
diff --git a/common/lib/xmodule/xmodule/tests/test_capa_module.py b/common/lib/xmodule/xmodule/tests/test_capa_module.py
index 182f1cf549..abeb12b143 100644
--- a/common/lib/xmodule/xmodule/tests/test_capa_module.py
+++ b/common/lib/xmodule/xmodule/tests/test_capa_module.py
@@ -81,14 +81,7 @@ class CapaFactory(object):
)
@classmethod
- def create(cls,
- attempts=None,
- problem_state=None,
- correct=False,
- xml=None,
- override_get_score=True,
- **kwargs
- ):
+ def create(cls, attempts=None, problem_state=None, correct=False, xml=None, override_get_score=True, **kwargs):
"""
All parameters are optional, and are added to the created problem if specified.
@@ -228,7 +221,6 @@ class CapaModuleTest(unittest.TestCase):
useful as unit-code coverage for this current implementation. I don't see a layer where LoncapaProblem
is tested directly
"""
- from capa.correctmap import CorrectMap
student_answers = {'1_2_1': 'abcd'}
correct_map = CorrectMap(answer_id='1_2_1', correctness="correct", npoints=0.9)
module = CapaFactory.create(correct=True, override_get_score=False)
@@ -623,6 +615,7 @@ class CapaModuleTest(unittest.TestCase):
module.submit_problem(get_request_dict)
+ # pylint: disable=line-too-long
# _http_post is called like this:
# _http_post(
# 'http://example.com/xqueue/xqueue/submit/',
@@ -639,9 +632,10 @@ class CapaModuleTest(unittest.TestCase):
# ,
# },
# )
+ # pylint: enable=line-too-long
self.assertEqual(xqueue_interface._http_post.call_count, 1)
- _, kwargs = xqueue_interface._http_post.call_args
+ _, kwargs = xqueue_interface._http_post.call_args # pylint: disable=unpacking-non-sequence
self.assertItemsEqual(fpaths, kwargs['files'].keys())
for fpath, fileobj in kwargs['files'].iteritems():
self.assertEqual(fpath, fileobj.name)
@@ -674,7 +668,7 @@ class CapaModuleTest(unittest.TestCase):
module.handle('xmodule_handler', request, 'problem_check')
self.assertEqual(xqueue_interface._http_post.call_count, 1)
- _, kwargs = xqueue_interface._http_post.call_args
+ _, kwargs = xqueue_interface._http_post.call_args # pylint: disable=unpacking-non-sequence
self.assertItemsEqual(fnames, kwargs['files'].keys())
for fpath, fileobj in kwargs['files'].iteritems():
self.assertEqual(fpath, fileobj.name)
@@ -2487,18 +2481,15 @@ class CapaDescriptorTest(unittest.TestCase):
def test_invalid_xml_handling(self):
"""
- Tests to confirm that invalid XML does not throw a wake-up-ops level error.
- See TNL-5057 for quick fix, TNL-5245 for full resolution.
+ Tests to confirm that invalid XML throws errors during xblock creation,
+ so as not to allow bad data into modulestore.
"""
sample_invalid_xml = textwrap.dedent("""