Have you changed your mind?
Yes
No
diff --git a/lms/djangoapps/courseware/grades.py b/lms/djangoapps/courseware/grades.py
index e7a488ec75..4640cd3f82 100644
--- a/lms/djangoapps/courseware/grades.py
+++ b/lms/djangoapps/courseware/grades.py
@@ -305,9 +305,9 @@ def progress_summary(student, request, course, field_data_cache):
graded = section_module.graded
scores = []
- module_creator = section_module.system.get_module
+ module_creator = section_module.xmodule_runtime.get_module
- for module_descriptor in yield_dynamic_descriptor_descendents(section_module.descriptor, module_creator):
+ for module_descriptor in yield_dynamic_descriptor_descendents(section_module, module_creator):
course_id = course.id
(correct, total) = get_score(course_id, student, module_descriptor, module_creator, field_data_cache)
diff --git a/lms/djangoapps/courseware/model_data.py b/lms/djangoapps/courseware/model_data.py
index f480d0ef44..da5d639bf0 100644
--- a/lms/djangoapps/courseware/model_data.py
+++ b/lms/djangoapps/courseware/model_data.py
@@ -289,7 +289,7 @@ class DjangoKeyValueStore(KeyValueStore):
def get(self, key):
if key.scope not in self._allowed_scopes:
- raise InvalidScopeError(key.scope)
+ raise InvalidScopeError(key)
field_object = self._field_data_cache.find(key)
if field_object is None:
@@ -320,7 +320,7 @@ class DjangoKeyValueStore(KeyValueStore):
for field in kv_dict:
# Check field for validity
if field.scope not in self._allowed_scopes:
- raise InvalidScopeError(field.scope)
+ raise InvalidScopeError(field)
# If the field is valid and isn't already in the dictionary, add it.
field_object = self._field_data_cache.find_or_create(field)
@@ -352,7 +352,7 @@ class DjangoKeyValueStore(KeyValueStore):
def delete(self, key):
if key.scope not in self._allowed_scopes:
- raise InvalidScopeError(key.scope)
+ raise InvalidScopeError(key)
field_object = self._field_data_cache.find(key)
if field_object is None:
@@ -368,7 +368,7 @@ class DjangoKeyValueStore(KeyValueStore):
def has(self, key):
if key.scope not in self._allowed_scopes:
- raise InvalidScopeError(key.scope)
+ raise InvalidScopeError(key)
field_object = self._field_data_cache.find(key)
if field_object is None:
diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py
index ab656a0923..aa8f1f99d8 100644
--- a/lms/djangoapps/courseware/module_render.py
+++ b/lms/djangoapps/courseware/module_render.py
@@ -219,6 +219,9 @@ def get_module_for_descriptor_internal(user, descriptor, field_data_cache, cours
if not has_access(user, descriptor, 'load', course_id):
return None
+ student_data = DbModel(DjangoKeyValueStore(field_data_cache))
+ descriptor._field_data = lms_field_data(descriptor._field_data, student_data)
+
# Setup system context for module instance
ajax_url = reverse(
'modx_dispatch',
@@ -294,10 +297,6 @@ def get_module_for_descriptor_internal(user, descriptor, field_data_cache, cours
position, wrap_xmodule_display, grade_bucket_type,
static_asset_path)
- def xmodule_field_data(descriptor):
- student_data = DbModel(DjangoKeyValueStore(field_data_cache))
- return lms_field_data(descriptor._field_data, student_data)
-
def publish(event):
"""A function that allows XModules to publish events. This only supports grade changes right now."""
if event.get('event_name') != 'grade':
@@ -405,7 +404,6 @@ def get_module_for_descriptor_internal(user, descriptor, field_data_cache, cours
jump_to_id_base_url=reverse('jump_to_id', kwargs={'course_id': course_id, 'module_id': ''})
),
node_path=settings.NODE_PATH,
- xmodule_field_data=xmodule_field_data,
publish=publish,
anonymous_student_id=unique_id_for_user(user),
course_id=course_id,
@@ -426,27 +424,17 @@ def get_module_for_descriptor_internal(user, descriptor, field_data_cache, cours
make_psychometrics_data_update_handler(course_id, user, descriptor.location.url())
)
- try:
- module = descriptor.xmodule(system)
- except:
- log.exception("Error creating module from descriptor {0}".format(descriptor))
-
- # make an ErrorDescriptor -- assuming that the descriptor's system is ok
- if has_access(user, descriptor.location, 'staff', course_id):
- err_descriptor_class = ErrorDescriptor
- else:
- err_descriptor_class = NonStaffErrorDescriptor
-
- err_descriptor = err_descriptor_class.from_descriptor(
- descriptor,
- error_msg=exc_info_to_str(sys.exc_info())
- )
-
- # Make an error module
- return err_descriptor.xmodule(system)
-
system.set('user_is_staff', has_access(user, descriptor.location, 'staff', course_id))
- return module
+
+ # make an ErrorDescriptor -- assuming that the descriptor's system is ok
+ if has_access(user, descriptor.location, 'staff', course_id):
+ system.error_descriptor_class = ErrorDescriptor
+ else:
+ system.error_descriptor_class = NonStaffErrorDescriptor
+
+ descriptor.xmodule_runtime = system
+ descriptor.scope_ids = descriptor.scope_ids._replace(user_id=user.id)
+ return descriptor
def find_target_student_module(request, user_id, course_id, mod_id):
diff --git a/lms/djangoapps/courseware/tests/__init__.py b/lms/djangoapps/courseware/tests/__init__.py
index afd5fae2cc..91be45516d 100644
--- a/lms/djangoapps/courseware/tests/__init__.py
+++ b/lms/djangoapps/courseware/tests/__init__.py
@@ -16,7 +16,7 @@ from student.tests.factories import UserFactory, CourseEnrollmentFactory
from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE
from xblock.field_data import DictFieldData
from xblock.fields import Scope
-from xmodule.tests import get_test_system
+from xmodule.tests import get_test_system, get_test_descriptor_system
from xmodule.modulestore import Location
from xmodule.modulestore.django import modulestore
from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
@@ -49,11 +49,26 @@ class BaseTestXmodule(ModuleStoreTestCase):
DATA = ''
MODEL_DATA = {'data': ''}
- def xmodule_field_data(self, descriptor):
- field_data = {}
- field_data.update(self.MODEL_DATA)
- student_data = DictFieldData(field_data)
- return lms_field_data(descriptor._field_data, student_data)
+ def new_module_runtime(self):
+ """
+ Generate a new ModuleSystem that is minimally set up for testing
+ """
+ runtime = get_test_system(course_id=self.course.id)
+
+ # When asked for a module out of a descriptor, just create a new xmodule runtime,
+ # and inject it into the descriptor
+ def get_module(descr):
+ descr.xmodule_runtime = self.new_module_runtime()
+ return descr
+
+ runtime.get_module = get_module
+
+ return runtime
+
+ def new_descriptor_runtime(self):
+ runtime = get_test_descriptor_system()
+ runtime.get_block = modulestore().get_item
+ return runtime
def setUp(self):
@@ -87,16 +102,15 @@ class BaseTestXmodule(ModuleStoreTestCase):
data=self.DATA
)
- self.runtime = get_test_system(course_id=self.course.id)
- # Allow us to assert that the template was called in the same way from
- # different code paths while maintaining the type returned by render_template
- self.runtime.render_template = lambda template, context: u'{!r}, {!r}'.format(template, sorted(context.items()))
+ self.runtime = self.new_descriptor_runtime()
- self.runtime.xmodule_field_data = self.xmodule_field_data
+ field_data = {}
+ field_data.update(self.MODEL_DATA)
+ student_data = DictFieldData(field_data)
+ self.item_descriptor._field_data = lms_field_data(self.item_descriptor._field_data, student_data)
- self.runtime.get_module = lambda descr: descr.xmodule(self.runtime)
-
- self.item_module = self.item_descriptor.xmodule(self.runtime)
+ self.item_descriptor.xmodule_runtime = self.new_module_runtime()
+ self.item_module = self.item_descriptor
self.item_url = Location(self.item_module.location).url()
@@ -119,7 +133,11 @@ class BaseTestXmodule(ModuleStoreTestCase):
class XModuleRenderingTestBase(BaseTestXmodule):
- def setUp(self):
- super(XModuleRenderingTestBase, self).setUp()
- self.runtime.render_template = render_to_string
+ def new_module_runtime(self):
+ """
+ Create a runtime that actually does html rendering
+ """
+ runtime = super(XModuleRenderingTestBase, self).new_module_runtime()
+ runtime.render_template = render_to_string
+ return runtime
diff --git a/lms/djangoapps/courseware/tests/test_lti.py b/lms/djangoapps/courseware/tests/test_lti.py
index 596ec2dd9a..9c3ae2bd58 100644
--- a/lms/djangoapps/courseware/tests/test_lti.py
+++ b/lms/djangoapps/courseware/tests/test_lti.py
@@ -39,7 +39,7 @@ class TestLTI(BaseTestXmodule):
u'oauth_consumer_key': u'',
u'oauth_signature_method': u'HMAC-SHA1',
u'oauth_version': u'1.0',
- u'user_id': self.runtime.anonymous_student_id,
+ u'user_id': self.item_descriptor.xmodule_runtime.anonymous_student_id,
u'role': u'student',
u'oauth_signature': mocked_decoded_signature
}
@@ -69,12 +69,14 @@ class TestLTI(BaseTestXmodule):
"""
Makes sure that all parameters extracted.
"""
- self.runtime.render_template = lambda template, context: context
- generated_context = self.item_module.get_html()
+ generated_context = self.item_module.runtime.render(self.item_module, None, 'student_view').content
expected_context = {
'input_fields': self.correct_headers,
'element_class': self.item_module.location.category,
'element_id': self.item_module.location.html_id(),
'launch_url': 'http://www.example.com', # default value
}
- self.assertDictEqual(generated_context, expected_context)
+ self.assertEqual(
+ generated_context,
+ self.runtime.render_template('lti.html', expected_context),
+ )
diff --git a/lms/djangoapps/courseware/tests/test_module_render.py b/lms/djangoapps/courseware/tests/test_module_render.py
index f287c9acc4..9d720109e7 100644
--- a/lms/djangoapps/courseware/tests/test_module_render.py
+++ b/lms/djangoapps/courseware/tests/test_module_render.py
@@ -74,7 +74,7 @@ class ModuleRenderTestCase(ModuleStoreTestCase, LoginEnrollmentTestCase):
)
# get the rendered HTML output which should have the rewritten link
- html = module.system.render(module, None, 'student_view').content
+ html = module.runtime.render(module, None, 'student_view').content
# See if the url got rewritten to the target link
# note if the URL mapping changes then this assertion will break
diff --git a/lms/djangoapps/courseware/tests/test_timelimit_module.py b/lms/djangoapps/courseware/tests/test_timelimit_module.py
index 41b2a00304..532dd23675 100644
--- a/lms/djangoapps/courseware/tests/test_timelimit_module.py
+++ b/lms/djangoapps/courseware/tests/test_timelimit_module.py
@@ -17,10 +17,13 @@ class TestTimeLimitModuleRendering(XModuleRenderingTestBase):
"""
def test_with_children(self):
block = ItemFactory.create(category='timelimit')
+ block.xmodule_runtime = self.new_module_runtime()
ItemFactory.create(category='html', data='This is just text', parent=block)
- assert_student_view(block, self.runtime.render(block.xmodule(self.runtime), None, 'student_view'))
+ assert_student_view(block, self.runtime.render(block, None, 'student_view'))
def test_without_children(self):
block = ItemFactory.create(category='timelimit')
- assert_student_view(block, self.runtime.render(block.xmodule(self.runtime), None, 'student_view'))
+ block.xmodule_runtime = self.new_module_runtime()
+
+ assert_student_view(block, self.runtime.render(block, None, 'student_view'))
diff --git a/lms/djangoapps/courseware/tests/test_video_mongo.py b/lms/djangoapps/courseware/tests/test_video_mongo.py
index 75cf55c1cc..2b94b561d0 100644
--- a/lms/djangoapps/courseware/tests/test_video_mongo.py
+++ b/lms/djangoapps/courseware/tests/test_video_mongo.py
@@ -13,14 +13,6 @@ class TestVideo(BaseTestXmodule):
CATEGORY = "video"
DATA = SOURCE_XML
- def setUp(self):
- # Since the VideoDescriptor changes `self._field_data`,
- # we need to instantiate `self.item_module` through
- # `self.item_descriptor` rather than directly constructing it
- super(TestVideo, self).setUp()
- self.item_module = self.item_descriptor.xmodule(self.runtime)
- self.item_module.runtime.render_template = lambda template, context: context
-
def test_handle_ajax_dispatch(self):
responses = {
user.username: self.clients[user.username].post(
@@ -40,7 +32,7 @@ class TestVideo(BaseTestXmodule):
def test_video_constructor(self):
"""Make sure that all parameters extracted correclty from xml"""
- context = self.item_module.get_html()
+ context = self.item_module.runtime.render(self.item_module, None, 'student_view').content
sources = {
'main': u'example.mp4',
@@ -53,7 +45,7 @@ class TestVideo(BaseTestXmodule):
'data_dir': getattr(self, 'data_dir', None),
'caption_asset_path': '/static/subs/',
'show_captions': 'true',
- 'display_name': 'A Name',
+ 'display_name': u'A Name',
'end': 3610.0,
'id': self.item_module.location.html_id(),
'sources': sources,
@@ -66,8 +58,10 @@ class TestVideo(BaseTestXmodule):
'yt_test_url': 'https://gdata.youtube.com/feeds/api/videos/'
}
- self.maxDiff = None
- self.assertEqual(context, expected_context)
+ self.assertEqual(
+ context,
+ self.item_module.xmodule_runtime.render_template('video.html', expected_context)
+ )
class TestVideoNonYouTube(TestVideo):
@@ -93,24 +87,24 @@ class TestVideoNonYouTube(TestVideo):
the template generates an empty string for the YouTube streams.
"""
sources = {
- u'main': u'example.mp4',
+ 'main': u'example.mp4',
u'mp4': u'example.mp4',
u'webm': u'example.webm',
u'ogv': u'example.ogv'
}
- context = self.item_module.get_html()
+ context = self.item_module.runtime.render(self.item_module, None, 'student_view').content
expected_context = {
'data_dir': getattr(self, 'data_dir', None),
'caption_asset_path': '/static/subs/',
'show_captions': 'true',
- 'display_name': 'A Name',
+ 'display_name': u'A Name',
'end': 3610.0,
'id': self.item_module.location.html_id(),
'sources': sources,
'start': 3603.0,
- 'sub': 'a_sub_file.srt.sjson',
+ 'sub': u'a_sub_file.srt.sjson',
'track': '',
'youtube_streams': '1.00:OEoXaMPEzfM',
'autoplay': settings.MITX_FEATURES.get('AUTOPLAY_VIDEOS', True),
@@ -118,4 +112,7 @@ class TestVideoNonYouTube(TestVideo):
'yt_test_url': 'https://gdata.youtube.com/feeds/api/videos/'
}
- self.assertEqual(context, expected_context)
+ self.assertEqual(
+ context,
+ self.item_module.xmodule_runtime.render_template('video.html', expected_context)
+ )
diff --git a/lms/djangoapps/courseware/tests/test_video_xml.py b/lms/djangoapps/courseware/tests/test_video_xml.py
index 28cee28e7c..3402855a67 100644
--- a/lms/djangoapps/courseware/tests/test_video_xml.py
+++ b/lms/djangoapps/courseware/tests/test_video_xml.py
@@ -22,7 +22,7 @@ from django.conf import settings
from xmodule.video_module import VideoDescriptor, _create_youtube_string
from xmodule.modulestore import Location
-from xmodule.tests import get_test_system, LogicTest
+from xmodule.tests import get_test_system, LogicTest, get_test_descriptor_system
from xblock.field_data import DictFieldData
from xblock.fields import ScopeIds
@@ -57,23 +57,18 @@ class VideoFactory(object):
field_data = {'data': VideoFactory.sample_problem_xml_youtube,
'location': location}
- system = get_test_system()
- system.render_template = lambda template, context: context
+ system = get_test_descriptor_system()
descriptor = VideoDescriptor(system, DictFieldData(field_data), ScopeIds(None, None, None, None))
-
- module = descriptor.xmodule(system)
-
- return module
+ descriptor.xmodule_runtime = get_test_system()
+ return descriptor
class VideoModuleUnitTest(unittest.TestCase):
"""Unit tests for Video Xmodule."""
-
def test_video_get_html(self):
"""Make sure that all parameters extracted correclty from xml"""
module = VideoFactory.create()
- module.runtime.render_template = lambda template, context: context
sources = {
'main': 'example.mp4',
@@ -99,14 +94,10 @@ class VideoModuleUnitTest(unittest.TestCase):
'yt_test_url': 'https://gdata.youtube.com/feeds/api/videos/'
}
- self.assertEqual(module.get_html(), expected_context)
-
- def test_video_instance_state(self):
- module = VideoFactory.create()
-
- self.assertDictEqual(
- json.loads(module.get_instance_state()),
- {'position': 0})
+ self.assertEqual(
+ module.runtime.render(module, None, 'student_view').content,
+ module.runtime.render_template('video.html', expected_context)
+ )
class VideoModuleLogicTest(LogicTest):
diff --git a/lms/djangoapps/courseware/tests/test_word_cloud.py b/lms/djangoapps/courseware/tests/test_word_cloud.py
index 6b01ae94f4..9967ea4915 100644
--- a/lms/djangoapps/courseware/tests/test_word_cloud.py
+++ b/lms/djangoapps/courseware/tests/test_word_cloud.py
@@ -245,7 +245,7 @@ class TestWordCloud(BaseTestXmodule):
fragment = self.runtime.render(self.item_module, None, 'student_view')
expected_context = {
- 'ajax_url': self.item_module.system.ajax_url,
+ 'ajax_url': self.item_module.xmodule_runtime.ajax_url,
'element_class': self.item_module.location.category,
'element_id': self.item_module.location.html_id(),
'num_inputs': 5, # default value
diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py
index c5489d3b30..672dc4d747 100644
--- a/lms/djangoapps/courseware/views.py
+++ b/lms/djangoapps/courseware/views.py
@@ -142,7 +142,7 @@ def redirect_to_course_position(course_module):
the first child.
"""
- urlargs = {'course_id': course_module.descriptor.id}
+ urlargs = {'course_id': course_module.id}
chapter = get_current_child(course_module)
if chapter is None:
# oops. Something bad has happened.
diff --git a/lms/djangoapps/instructor_task/tasks_helper.py b/lms/djangoapps/instructor_task/tasks_helper.py
index 4f2d37a212..de5ac1e53b 100644
--- a/lms/djangoapps/instructor_task/tasks_helper.py
+++ b/lms/djangoapps/instructor_task/tasks_helper.py
@@ -310,6 +310,7 @@ def rescore_problem_module_state(module_descriptor, student_module, xmodule_inst
raise UpdateProblemModuleStateError(msg)
result = instance.rescore_problem()
+ instance.save()
if 'success' not in result:
# don't consider these fatal, but false means that the individual call didn't complete:
TASK_LOG.warning(u"error processing rescore call for course {course}, problem {loc} and student {student}: "
diff --git a/lms/djangoapps/open_ended_grading/open_ended_notifications.py b/lms/djangoapps/open_ended_grading/open_ended_notifications.py
index 00b9f7e866..3074e12540 100644
--- a/lms/djangoapps/open_ended_grading/open_ended_notifications.py
+++ b/lms/djangoapps/open_ended_grading/open_ended_notifications.py
@@ -13,8 +13,6 @@ from xmodule.x_module import ModuleSystem
from mitxmako.shortcuts import render_to_string
import datetime
-from xblock.field_data import DictFieldData
-
log = logging.getLogger(__name__)
NOTIFICATION_CACHE_TIME = 300
@@ -70,7 +68,6 @@ def peer_grading_notifications(course, user):
get_module = None,
render_template=render_to_string,
replace_urls=None,
- xmodule_field_data=DictFieldData({}),
)
peer_gs = peer_grading_service.PeerGradingService(settings.OPEN_ENDED_GRADING_INTERFACE, system)
pending_grading = False
@@ -132,7 +129,6 @@ def combined_notifications(course, user):
get_module = None,
render_template=render_to_string,
replace_urls=None,
- xmodule_field_data=DictFieldData({})
)
#Initialize controller query service using our mock system
controller_qs = ControllerQueryService(settings.OPEN_ENDED_GRADING_INTERFACE, system)
diff --git a/lms/djangoapps/open_ended_grading/staff_grading_service.py b/lms/djangoapps/open_ended_grading/staff_grading_service.py
index dacf52b39e..55ff2b1371 100644
--- a/lms/djangoapps/open_ended_grading/staff_grading_service.py
+++ b/lms/djangoapps/open_ended_grading/staff_grading_service.py
@@ -9,8 +9,6 @@ from xmodule.open_ended_grading_classes.grading_service_module import GradingSer
from django.conf import settings
from django.http import HttpResponse, Http404
-from xblock.field_data import DictFieldData
-
from courseware.access import has_access
from util.json_request import expect_json
from xmodule.course_module import CourseDescriptor
@@ -76,7 +74,6 @@ class StaffGradingService(GradingService):
get_module = None,
render_template=render_to_string,
replace_urls=None,
- xmodule_field_data=DictFieldData({})
)
super(StaffGradingService, self).__init__(config)
self.url = config['url'] + config['staff_grading']
diff --git a/lms/djangoapps/open_ended_grading/tests.py b/lms/djangoapps/open_ended_grading/tests.py
index 65a1ce68cc..7b804d8342 100644
--- a/lms/djangoapps/open_ended_grading/tests.py
+++ b/lms/djangoapps/open_ended_grading/tests.py
@@ -16,6 +16,7 @@ from xmodule.open_ended_grading_classes import peer_grading_service, controller_
from xmodule import peer_grading_module
from xmodule.modulestore.django import modulestore
from xmodule.x_module import ModuleSystem
+from xmodule.error_module import ErrorDescriptor
from xblock.fields import ScopeIds
from open_ended_grading import staff_grading_service, views, utils
@@ -251,13 +252,14 @@ class TestPeerGradingService(ModuleStoreTestCase, LoginEnrollmentTestCase):
get_module=None,
render_template=render_to_string,
replace_urls=None,
- xmodule_field_data=lambda d: d._field_data,
s3_interface=test_util_open_ended.S3_INTERFACE,
open_ended_grading_interface=test_util_open_ended.OPEN_ENDED_GRADING_INTERFACE,
mixins=settings.XBLOCK_MIXINS,
+ error_descriptor_class=ErrorDescriptor,
)
self.descriptor = peer_grading_module.PeerGradingDescriptor(self.system, field_data, ScopeIds(None, None, None, None))
- self.peer_module = self.descriptor.xmodule(self.system)
+ self.descriptor.xmodule_runtime = self.system
+ self.peer_module = self.descriptor
self.peer_module.peer_gs = self.mock_service
self.logout()
diff --git a/lms/djangoapps/open_ended_grading/utils.py b/lms/djangoapps/open_ended_grading/utils.py
index 3ad85544f2..786b328fcf 100644
--- a/lms/djangoapps/open_ended_grading/utils.py
+++ b/lms/djangoapps/open_ended_grading/utils.py
@@ -36,7 +36,6 @@ system = ModuleSystem(
get_module=None,
render_template=render_to_string,
replace_urls=None,
- xmodule_field_data=DictFieldData({}),
)