From 6b474724acba7e1e156835acf7ac87be4bf886c9 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Thu, 19 Sep 2013 13:11:17 -0400 Subject: [PATCH 1/3] Make XModuleDescriptor and XModule act as a single class By transparently proxying between the XModuleDescriptor and the XModule, and between their runtimes, we can make them act as a single class, so that we can swap in an actual XBlock instead. --- cms/djangoapps/contentstore/views/preview.py | 26 +- common/djangoapps/xmodule_modifiers.py | 8 +- common/lib/xmodule/xmodule/capa_module.py | 32 ++- .../xmodule/combined_open_ended_module.py | 2 +- .../lib/xmodule/xmodule/conditional_module.py | 1 + .../lib/xmodule/xmodule/crowdsource_hinter.py | 8 +- common/lib/xmodule/xmodule/error_module.py | 2 +- .../js/fixtures/crowdsource_hinter.html | 6 +- .../xmodule/js/spec/capa/display_spec.coffee | 4 +- .../xmodule/js/src/capa/display.coffee | 4 +- .../js/src/crowdsource_hinter/display.coffee | 2 +- .../xmodule/xmodule/modulestore/mongo/base.py | 6 +- .../split_mongo/split_mongo_kvs.py | 6 +- common/lib/xmodule/xmodule/modulestore/xml.py | 2 +- .../xmodule/xmodule/peer_grading_module.py | 20 +- common/lib/xmodule/xmodule/seq_module.py | 3 - common/lib/xmodule/xmodule/tests/__init__.py | 20 +- .../xmodule/tests/test_combined_open_ended.py | 40 ++- .../xmodule/xmodule/tests/test_conditional.py | 117 ++++---- .../xmodule/tests/test_crowdsource_hinter.py | 18 +- .../xmodule/tests/test_error_module.py | 16 +- .../xmodule/tests/test_peer_grading.py | 1 + .../xmodule/xmodule/tests/test_progress.py | 2 +- .../xmodule/tests/test_util_open_ended.py | 3 +- .../xmodule/tests/test_xblock_wrappers.py | 58 ++-- common/lib/xmodule/xmodule/video_module.py | 4 - common/lib/xmodule/xmodule/x_module.py | 260 ++++++++++++------ .../vertical/vertical_test.xml | 6 +- lms/djangoapps/courseware/grades.py | 4 +- lms/djangoapps/courseware/model_data.py | 8 +- lms/djangoapps/courseware/module_render.py | 36 +-- lms/djangoapps/courseware/tests/__init__.py | 52 ++-- lms/djangoapps/courseware/tests/test_lti.py | 10 +- .../courseware/tests/test_module_render.py | 2 +- .../courseware/tests/test_timelimit_module.py | 7 +- .../courseware/tests/test_video_mongo.py | 31 +-- .../courseware/tests/test_video_xml.py | 25 +- .../courseware/tests/test_word_cloud.py | 2 +- lms/djangoapps/courseware/views.py | 2 +- .../instructor_task/tasks_helper.py | 1 + .../open_ended_notifications.py | 4 - .../staff_grading_service.py | 3 - lms/djangoapps/open_ended_grading/tests.py | 6 +- lms/djangoapps/open_ended_grading/utils.py | 1 - 44 files changed, 533 insertions(+), 338 deletions(-) diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index b5cfc74a5732..7e55d97899a6 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -95,11 +95,6 @@ def preview_module_system(request, preview_id, descriptor): descriptor: An XModuleDescriptor """ - def preview_field_data(descriptor): - "Helper method to create a DbModel from a descriptor" - student_data = DbModel(SessionKeyValueStore(request)) - return lms_field_data(descriptor._field_data, student_data) - course_id = get_course_for_item(descriptor.location).location.course_id if descriptor.location.category == 'static_tab': @@ -118,7 +113,6 @@ def preview_field_data(descriptor): debug=True, replace_urls=partial(static_replace.replace_static_urls, data_directory=None, course_id=course_id), user=request.user, - xmodule_field_data=preview_field_data, can_execute_unsafe_code=(lambda: can_execute_unsafe_code(course_id)), mixins=settings.XBLOCK_MIXINS, course_id=course_id, @@ -136,7 +130,8 @@ def preview_field_data(descriptor): getattr(descriptor, 'data_dir', descriptor.location.course), course_id=descriptor.location.org + '/' + descriptor.location.course + '/BOGUS_RUN_REPLACE_WHEN_AVAILABLE', ), - ) + ), + error_descriptor_class=ErrorDescriptor, ) @@ -148,17 +143,12 @@ def load_preview_module(request, preview_id, descriptor): preview_id (str): An identifier specifying which preview this module is used for descriptor: An XModuleDescriptor """ - system = preview_module_system(request, preview_id, descriptor) - try: - module = descriptor.xmodule(system) - except: - log.debug("Unable to load preview module", exc_info=True) - module = ErrorDescriptor.from_descriptor( - descriptor, - error_msg=exc_info_to_str(sys.exc_info()) - ).xmodule(system) - - return module + student_data = DbModel(SessionKeyValueStore(request)) + descriptor.bind_for_student( + preview_module_system(request, preview_id, descriptor), + lms_field_data(descriptor._field_data, student_data), # pylint: disable=protected-access + ) + return descriptor def get_preview_html(request, descriptor, idx): diff --git a/common/djangoapps/xmodule_modifiers.py b/common/djangoapps/xmodule_modifiers.py index 7683768e1201..43942f3edfbb 100644 --- a/common/djangoapps/xmodule_modifiers.py +++ b/common/djangoapps/xmodule_modifiers.py @@ -134,7 +134,7 @@ def add_histogram(user, block, view, frag, context): # pylint: disable=unused-a return frag block_id = block.id - if block.descriptor.has_score: + if block.has_score: histogram = grade_histogram(block_id) render_histogram = len(histogram) > 0 else: @@ -142,7 +142,7 @@ def add_histogram(user, block, view, frag, context): # pylint: disable=unused-a render_histogram = False if settings.MITX_FEATURES.get('ENABLE_LMS_MIGRATION'): - [filepath, filename] = getattr(block.descriptor, 'xml_attributes', {}).get('filename', ['', None]) + [filepath, filename] = getattr(block, 'xml_attributes', {}).get('filename', ['', None]) osfs = block.system.filestore if filename is not None and osfs.exists(filename): # if original, unmangled filename exists then use it (github @@ -163,13 +163,13 @@ def add_histogram(user, block, view, frag, context): # pylint: disable=unused-a # TODO (ichuang): use _has_access_descriptor.can_load in lms.courseware.access, instead of now>mstart comparison here now = datetime.datetime.now(UTC()) is_released = "unknown" - mstart = block.descriptor.start + mstart = block.start if mstart is not None: is_released = "Yes!" if (now > mstart) else "Not yet" staff_context = {'fields': [(name, field.read_from(block)) for name, field in block.fields.items()], - 'xml_attributes': getattr(block.descriptor, 'xml_attributes', {}), + 'xml_attributes': getattr(block, 'xml_attributes', {}), 'location': block.location, 'xqa_key': block.xqa_key, 'source_file': source_file, diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index 6ffbf7bd06df..ed0e6bf2477a 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -15,7 +15,7 @@ ResponseError, LoncapaProblemError from capa.util import convert_files_to_filenames from .progress import Progress -from xmodule.x_module import XModule +from xmodule.x_module import XModule, module_attr from xmodule.raw_module import RawDescriptor from xmodule.exceptions import NotFoundError, ProcessingError from xblock.fields import Scope, String, Boolean, Dict, Integer, Float @@ -1193,3 +1193,33 @@ def non_editable_metadata_fields(self): CapaDescriptor.force_save_button, CapaDescriptor.markdown, CapaDescriptor.text_customization]) return non_editable_fields + + # Proxy to CapaModule for access to any of its attributes + answer_available = module_attr('answer_available') + check_button_name = module_attr('check_button_name') + check_problem = module_attr('check_problem') + choose_new_seed = module_attr('choose_new_seed') + closed = module_attr('closed') + get_answer = module_attr('get_answer') + get_problem = module_attr('get_problem') + get_problem_html = module_attr('get_problem_html') + get_state_for_lcp = module_attr('get_state_for_lcp') + handle_input_ajax = module_attr('handle_input_ajax') + handle_problem_html_error = module_attr('handle_problem_html_error') + handle_ungraded_response = module_attr('handle_ungraded_response') + is_attempted = module_attr('is_attempted') + is_correct = module_attr('is_correct') + is_past_due = module_attr('is_past_due') + is_submitted = module_attr('is_submitted') + lcp = module_attr('lcp') + make_dict_of_responses = module_attr('make_dict_of_responses') + new_lcp = module_attr('new_lcp') + publish_grade = module_attr('publish_grade') + rescore_problem = module_attr('rescore_problem') + reset_problem = module_attr('reset_problem') + save_problem = module_attr('save_problem') + set_state_from_lcp = module_attr('set_state_from_lcp') + should_show_check_button = module_attr('should_show_check_button') + should_show_reset_button = module_attr('should_show_reset_button') + should_show_save_button = module_attr('should_show_save_button') + update_score = module_attr('update_score') diff --git a/common/lib/xmodule/xmodule/combined_open_ended_module.py b/common/lib/xmodule/xmodule/combined_open_ended_module.py index 0bc79a4a1c32..6dc2ac045bff 100644 --- a/common/lib/xmodule/xmodule/combined_open_ended_module.py +++ b/common/lib/xmodule/xmodule/combined_open_ended_module.py @@ -496,7 +496,7 @@ class CombinedOpenEndedDescriptor(CombinedOpenEndedFields, RawDescriptor): metadata_translations = { 'is_graded': 'graded', 'attempts': 'max_attempts', - } + } def get_context(self): _context = RawDescriptor.get_context(self) diff --git a/common/lib/xmodule/xmodule/conditional_module.py b/common/lib/xmodule/xmodule/conditional_module.py index a489b8a19f52..03eea40ee37f 100644 --- a/common/lib/xmodule/xmodule/conditional_module.py +++ b/common/lib/xmodule/xmodule/conditional_module.py @@ -18,6 +18,7 @@ class ConditionalFields(object): + has_children = True show_tag_list = List(help="Poll answers", scope=Scope.content) diff --git a/common/lib/xmodule/xmodule/crowdsource_hinter.py b/common/lib/xmodule/xmodule/crowdsource_hinter.py index b302982b0535..8c2ea8a7430e 100644 --- a/common/lib/xmodule/xmodule/crowdsource_hinter.py +++ b/common/lib/xmodule/xmodule/crowdsource_hinter.py @@ -115,15 +115,15 @@ def get_html(self): child = self.get_display_items()[0] out = self.runtime.render_child(child, None, 'student_view').content # The event listener uses the ajax url to find the child. - child_url = child.runtime.ajax_url + child_id = child.id except IndexError: out = u"Error in loading crowdsourced hinter - can't find child problem." - child_url = '' + child_id = '' # Wrap the module in a
. This lets us pass data attributes to the javascript. - out += u'
'.format( + out += u'
'.format( ajax_url=self.runtime.ajax_url, - child_url=child_url + child_id=child_id ) return out diff --git a/common/lib/xmodule/xmodule/error_module.py b/common/lib/xmodule/xmodule/error_module.py index 7fc3747f4460..c6c798a1141a 100644 --- a/common/lib/xmodule/xmodule/error_module.py +++ b/common/lib/xmodule/xmodule/error_module.py @@ -77,7 +77,7 @@ class ErrorDescriptor(ErrorFields, XModuleDescriptor): module_class = ErrorModule def get_html(self): - return '' + return u'' @classmethod def _construct(cls, system, contents, error_msg, location): diff --git a/common/lib/xmodule/xmodule/js/fixtures/crowdsource_hinter.html b/common/lib/xmodule/xmodule/js/fixtures/crowdsource_hinter.html index 02122017bdf2..e8d327a514ac 100644 --- a/common/lib/xmodule/xmodule/js/fixtures/crowdsource_hinter.html +++ b/common/lib/xmodule/xmodule/js/fixtures/crowdsource_hinter.html @@ -1,8 +1,8 @@
  • - +
    - +
    @@ -44,7 +44,7 @@

    - +

    diff --git a/common/lib/xmodule/xmodule/js/spec/capa/display_spec.coffee b/common/lib/xmodule/xmodule/js/spec/capa/display_spec.coffee index d8decf65749b..99344c298380 100644 --- a/common/lib/xmodule/xmodule/js/spec/capa/display_spec.coffee +++ b/common/lib/xmodule/xmodule/js/spec/capa/display_spec.coffee @@ -139,12 +139,12 @@ describe 'Problem', -> it 'log the problem_graded event, after the problem is done grading.', -> spyOn($, 'postWithPrefix').andCallFake (url, answers, callback) -> - response = + response = success: 'correct' contents: 'mock grader response' callback(response) @problem.check() - expect(Logger.log).toHaveBeenCalledWith 'problem_graded', ['foo=1&bar=2', 'mock grader response'], @problem.url + expect(Logger.log).toHaveBeenCalledWith 'problem_graded', ['foo=1&bar=2', 'mock grader response'], @problem.id it 'submit the answer for check', -> spyOn $, 'postWithPrefix' diff --git a/common/lib/xmodule/xmodule/js/src/capa/display.coffee b/common/lib/xmodule/xmodule/js/src/capa/display.coffee index 7786daecf770..7982cbbf654d 100644 --- a/common/lib/xmodule/xmodule/js/src/capa/display.coffee +++ b/common/lib/xmodule/xmodule/js/src/capa/display.coffee @@ -248,7 +248,7 @@ class @Problem @updateProgress response else @gentle_alert response.success - Logger.log 'problem_graded', [@answers, response.contents], @url + Logger.log 'problem_graded', [@answers, response.contents], @id if not abort_submission $.ajaxWithPrefix("#{@url}/problem_check", settings) @@ -271,7 +271,7 @@ class @Problem @el.removeClass 'showed' else @gentle_alert response.success - Logger.log 'problem_graded', [@answers, response.contents], @url + Logger.log 'problem_graded', [@answers, response.contents], @id reset: => Logger.log 'problem_reset', @answers diff --git a/common/lib/xmodule/xmodule/js/src/crowdsource_hinter/display.coffee b/common/lib/xmodule/xmodule/js/src/crowdsource_hinter/display.coffee index f9f3e438262b..8c224770340a 100644 --- a/common/lib/xmodule/xmodule/js/src/crowdsource_hinter/display.coffee +++ b/common/lib/xmodule/xmodule/js/src/crowdsource_hinter/display.coffee @@ -7,7 +7,7 @@ class @Hinter constructor: (element) -> @el = $(element).find('.crowdsource-wrapper') @url = @el.data('url') - Logger.listen('problem_graded', @el.data('child-url'), @capture_problem) + Logger.listen('problem_graded', @el.data('child-id'), @capture_problem) @render() capture_problem: (event_type, data, element) => diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py index 2843287055b9..aec84d1f4c4e 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py @@ -81,7 +81,7 @@ def get(self, key): else: return self._data[key.field_name] else: - raise InvalidScopeError(key.scope) + raise InvalidScopeError(key) def set(self, key, value): if key.scope == Scope.children: @@ -94,7 +94,7 @@ def set(self, key, value): else: self._data[key.field_name] = value else: - raise InvalidScopeError(key.scope) + raise InvalidScopeError(key) def delete(self, key): if key.scope == Scope.children: @@ -108,7 +108,7 @@ def delete(self, key): else: del self._data[key.field_name] else: - raise InvalidScopeError(key.scope) + raise InvalidScopeError(key) def has(self, key): if key.scope in (Scope.children, Scope.parent): diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_mongo_kvs.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_mongo_kvs.py index 02f5aad9f5b5..54007fda73f5 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_mongo_kvs.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_mongo_kvs.py @@ -53,12 +53,12 @@ def get(self, key): raise KeyError() else: - raise InvalidScopeError(key.scope) + raise InvalidScopeError(key) def set(self, key, value): # handle any special cases if key.scope not in [Scope.children, Scope.settings, Scope.content]: - raise InvalidScopeError(key.scope) + raise InvalidScopeError(key) if key.scope == Scope.content: self._load_definition() @@ -75,7 +75,7 @@ def set(self, key, value): def delete(self, key): # handle any special cases if key.scope not in [Scope.children, Scope.settings, Scope.content]: - raise InvalidScopeError(key.scope) + raise InvalidScopeError(key) if key.scope == Scope.content: self._load_definition() diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index a8e9da03d71f..1e7074a8475e 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -204,7 +204,7 @@ def fallback_name(orig_name=None): descriptor.save() return descriptor - render_template = lambda: '' + render_template = lambda template, context: u'' # TODO (vshnayder): we are somewhat architecturally confused in the loading code: # load_item should actually be get_instance, because it expects the course-specific # policy to be loaded. For now, just add the course_id here... diff --git a/common/lib/xmodule/xmodule/peer_grading_module.py b/common/lib/xmodule/xmodule/peer_grading_module.py index c363d9a1f8ba..3bd101afcf96 100644 --- a/common/lib/xmodule/xmodule/peer_grading_module.py +++ b/common/lib/xmodule/xmodule/peer_grading_module.py @@ -6,7 +6,7 @@ from datetime import datetime from pkg_resources import resource_string from .capa_module import ComplexEncoder -from .x_module import XModule +from .x_module import XModule, module_attr from xmodule.raw_module import RawDescriptor from xmodule.modulestore.exceptions import ItemNotFoundError, NoPathToItem from .timeinfo import TimeInfo @@ -106,7 +106,7 @@ def __init__(self, *args, **kwargs): #We need to set the location here so the child modules can use it self.runtime.set('location', self.location) - if (self.system.open_ended_grading_interface): + if (self.runtime.open_ended_grading_interface): self.peer_gs = PeerGradingService(self.system.open_ended_grading_interface, self.system) else: self.peer_gs = MockPeerGradingService() @@ -662,3 +662,19 @@ def get_required_module_descriptors(self): return [self.system.load_item(self.link_to_location)] else: return [] + + # Proxy to PeerGradingModule so that external callers don't have to know if they're working + # with a module or a descriptor + closed = module_attr('closed') + get_instance_state = module_attr('get_instance_state') + get_next_submission = module_attr('get_next_submission') + is_student_calibrated = module_attr('is_student_calibrated') + peer_grading = module_attr('peer_grading') + peer_grading_closed = module_attr('peer_grading_closed') + peer_grading_problem = module_attr('peer_grading_problem') + peer_gs = module_attr('peer_gs') + query_data_for_location = module_attr('query_data_for_location') + save_calibration_essay = module_attr('save_calibration_essay') + save_grade = module_attr('save_grade') + show_calibration_essay = module_attr('show_calibration_essay') + _find_corresponding_module_for_location = module_attr('_find_corresponding_module_for_location') diff --git a/common/lib/xmodule/xmodule/seq_module.py b/common/lib/xmodule/xmodule/seq_module.py index 7890edd586a5..103311dcb500 100644 --- a/common/lib/xmodule/xmodule/seq_module.py +++ b/common/lib/xmodule/xmodule/seq_module.py @@ -45,9 +45,6 @@ def __init__(self, *args, **kwargs): self.rendered = False - def get_instance_state(self): - return json.dumps({'position': self.position}) - def get_html(self): self.render() return self.content diff --git a/common/lib/xmodule/xmodule/tests/__init__.py b/common/lib/xmodule/xmodule/tests/__init__.py index e4ce33845860..2ea9040a1ea1 100644 --- a/common/lib/xmodule/xmodule/tests/__init__.py +++ b/common/lib/xmodule/xmodule/tests/__init__.py @@ -9,6 +9,7 @@ import json import os +import pprint import unittest from mock import Mock @@ -18,6 +19,7 @@ from xmodule.x_module import ModuleSystem, XModuleDescriptor, XModuleMixin from xmodule.modulestore.inheritance import InheritanceMixin from xmodule.mako_module import MakoDescriptorSystem +from xmodule.error_module import ErrorDescriptor # Location of common test DATA directory @@ -54,18 +56,18 @@ def get_test_system(course_id=''): ajax_url='courses/course_id/modx/a_location', track_function=Mock(), get_module=Mock(), - render_template=lambda template, context: repr(context), - replace_urls=lambda html: str(html), + render_template=mock_render_template, + replace_urls=str, user=Mock(is_staff=False), filestore=Mock(), debug=True, hostname="edx.org", xqueue={'interface': None, 'callback_url': '/', 'default_queuename': 'testqueue', 'waittime': 10, 'construct_callback' : Mock(side_effect="/")}, node_path=os.environ.get("NODE_PATH", "/usr/local/lib/node_modules"), - xmodule_field_data=lambda descriptor: descriptor._field_data, anonymous_student_id='student', open_ended_grading_interface=open_ended_grading_interface, course_id=course_id, + error_descriptor_class=ErrorDescriptor, ) @@ -77,11 +79,21 @@ def get_test_descriptor_system(): load_item=Mock(), resources_fs=Mock(), error_tracker=Mock(), - render_template=lambda template, context: repr(context), + render_template=mock_render_template, mixins=(InheritanceMixin, XModuleMixin), ) +def mock_render_template(*args, **kwargs): + """ + Pretty-print the args and kwargs. + + Allows us to not depend on any actual template rendering mechanism, + while still returning a unicode object + """ + return pprint.pformat((args, kwargs)).decode() + + class ModelsTest(unittest.TestCase): def setUp(self): pass diff --git a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py index 0fda51412898..bb7015ecaf28 100644 --- a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py +++ b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py @@ -350,11 +350,11 @@ def test_open_ended_display(self): """ Test storing answer with the open ended module. """ - + # Create a module with no state yet. Important that this start off as a blank slate. test_module = OpenEndedModule(self.test_system, self.location, self.definition, self.descriptor, self.static_data, self.metadata) - + saved_response = "Saved response." submitted_response = "Submitted response." @@ -753,28 +753,36 @@ def test_open_ended_flow_reset(self): #Simulate a student saving an answer html = module.handle_ajax("get_html", {}) + module.save() module.handle_ajax("save_answer", {"student_answer": self.answer}) + module.save() html = module.handle_ajax("get_html", {}) + module.save() #Mock a student submitting an assessment assessment_dict = MockQueryDict() assessment_dict.update({'assessment': sum(assessment), 'score_list[]': assessment}) module.handle_ajax("save_assessment", assessment_dict) + module.save() task_one_json = json.loads(module.task_states[0]) self.assertEqual(json.loads(task_one_json['child_history'][0]['post_assessment']), assessment) rubric = module.handle_ajax("get_combined_rubric", {}) + module.save() #Move to the next step in the problem module.handle_ajax("next_problem", {}) + module.save() self.assertEqual(module.current_task_number, 0) - html = module.get_html() + html = module.runtime.render(module, None, 'student_view').content self.assertIsInstance(html, basestring) rubric = module.handle_ajax("get_combined_rubric", {}) + module.save() self.assertIsInstance(rubric, basestring) self.assertEqual(module.state, "assessing") module.handle_ajax("reset", {}) + module.save() self.assertEqual(module.current_task_number, 0) def test_open_ended_flow_correct(self): @@ -789,31 +797,36 @@ def test_open_ended_flow_correct(self): #Simulate a student saving an answer module.handle_ajax("save_answer", {"student_answer": self.answer}) + module.save() status = module.handle_ajax("get_status", {}) + module.save() self.assertIsInstance(status, basestring) #Mock a student submitting an assessment assessment_dict = MockQueryDict() assessment_dict.update({'assessment': sum(assessment), 'score_list[]': assessment}) module.handle_ajax("save_assessment", assessment_dict) + module.save() task_one_json = json.loads(module.task_states[0]) self.assertEqual(json.loads(task_one_json['child_history'][0]['post_assessment']), assessment) #Move to the next step in the problem try: module.handle_ajax("next_problem", {}) + module.save() except GradingServiceError: #This error is okay. We don't have a grading service to connect to! pass self.assertEqual(module.current_task_number, 1) try: - module.get_html() + module.runtime.render(module, None, 'student_view') except GradingServiceError: #This error is okay. We don't have a grading service to connect to! pass #Try to get the rubric from the module module.handle_ajax("get_combined_rubric", {}) + module.save() #Make a fake reply from the queue queue_reply = { @@ -832,22 +845,27 @@ def test_open_ended_flow_correct(self): } module.handle_ajax("check_for_score", {}) + module.save() #Update the module with the fake queue reply module.handle_ajax("score_update", queue_reply) + module.save() self.assertFalse(module.ready_to_reset) self.assertEqual(module.current_task_number, 1) #Get html and other data client will request - module.get_html() + module.runtime.render(module, None, 'student_view') module.handle_ajax("skip_post_assessment", {}) + module.save() #Get all results module.handle_ajax("get_combined_rubric", {}) + module.save() #reset the problem module.handle_ajax("reset", {}) + module.save() self.assertEqual(module.state, "initial") @@ -876,31 +894,37 @@ def test_reset_fail(self): """ assessment = [0, 1] module = self.get_module_from_location(self.problem_location, COURSE) + module.save() #Simulate a student saving an answer module.handle_ajax("save_answer", {"student_answer": self.answer}) + module.save() #Mock a student submitting an assessment assessment_dict = MockQueryDict() assessment_dict.update({'assessment': sum(assessment), 'score_list[]': assessment}) module.handle_ajax("save_assessment", assessment_dict) + module.save() task_one_json = json.loads(module.task_states[0]) self.assertEqual(json.loads(task_one_json['child_history'][0]['post_assessment']), assessment) #Move to the next step in the problem module.handle_ajax("next_problem", {}) + module.save() self.assertEqual(module.current_task_number, 0) - html = module.get_html() - self.assertTrue(isinstance(html, basestring)) + html = module.runtime.render(module, None, 'student_view').content + self.assertIsInstance(html, basestring) #Module should now be done rubric = module.handle_ajax("get_combined_rubric", {}) - self.assertTrue(isinstance(rubric, basestring)) + module.save() + self.assertIsInstance(rubric, basestring) self.assertEqual(module.state, "done") #Try to reset, should fail because only 1 attempt is allowed reset_data = json.loads(module.handle_ajax("reset", {})) + module.save() self.assertEqual(reset_data['success'], False) class OpenEndedModuleXmlImageUploadTest(unittest.TestCase, DummyModulestore): diff --git a/common/lib/xmodule/xmodule/tests/test_conditional.py b/common/lib/xmodule/xmodule/tests/test_conditional.py index b665ef02f708..742f37c94ffe 100644 --- a/common/lib/xmodule/xmodule/tests/test_conditional.py +++ b/common/lib/xmodule/xmodule/tests/test_conditional.py @@ -1,4 +1,3 @@ - from ast import literal_eval import json import unittest @@ -8,12 +7,11 @@ from xblock.field_data import DictFieldData from xblock.fields import ScopeIds -from xblock.fragment import Fragment from xmodule.error_module import NonStaffErrorDescriptor from xmodule.modulestore import Location from xmodule.modulestore.xml import ImportSystem, XMLModuleStore -from xmodule.conditional_module import ConditionalModule -from xmodule.tests import DATA_DIR, get_test_system +from xmodule.conditional_module import ConditionalDescriptor +from xmodule.tests import DATA_DIR, get_test_system, get_test_descriptor_system ORG = 'test_org' @@ -26,20 +24,15 @@ class DummySystem(ImportSystem): def __init__(self, load_error_modules): xmlstore = XMLModuleStore("data_dir", course_dirs=[], load_error_modules=load_error_modules) - course_id = "/".join([ORG, COURSE, 'test_run']) - course_dir = "test_dir" - policy = {} - error_tracker = Mock() - parent_tracker = Mock() super(DummySystem, self).__init__( - xmlstore, - course_id, - course_dir, - policy, - error_tracker, - parent_tracker, + xmlstore=xmlstore, + course_id='/'.join([ORG, COURSE, 'test_run']), + course_dir='test_dir', + error_tracker=Mock(), + parent_tracker=Mock(), load_error_modules=load_error_modules, + policy={}, ) def render_template(self, template, context): @@ -59,52 +52,55 @@ def create(system, source_is_error_module=False): if the source_is_error_module flag is set, create a real ErrorModule for the source. """ + descriptor_system = get_test_descriptor_system() + # construct source descriptor and module: source_location = Location(["i4x", "edX", "conditional_test", "problem", "SampleProblem"]) if source_is_error_module: # Make an error descriptor and module - source_descriptor = NonStaffErrorDescriptor.from_xml('some random xml data', - system, - org=source_location.org, - course=source_location.course, - error_msg='random error message') - source_module = source_descriptor.xmodule(system) + source_descriptor = NonStaffErrorDescriptor.from_xml( + 'some random xml data', + system, + org=source_location.org, + course=source_location.course, + error_msg='random error message' + ) else: source_descriptor = Mock() source_descriptor.location = source_location - source_module = Mock() + + source_descriptor.runtime = descriptor_system # construct other descriptors: child_descriptor = Mock() - cond_descriptor = Mock() - cond_descriptor.runtime = system - cond_descriptor.get_required_module_descriptors = lambda: [source_descriptor, ] - cond_descriptor.get_children = lambda: [child_descriptor, ] - cond_descriptor.xml_attributes = {"attempted": "true"} - - # create child module: - child_module = Mock() - child_module.runtime = system - child_module.get_html.return_value = u'

    This is a secret

    ' - child_module.student_view.return_value = Fragment(child_module.get_html.return_value) - child_module.displayable_items = lambda: [child_module] - module_map = {source_descriptor: source_module, child_descriptor: child_module} - system.get_module = lambda descriptor: module_map[descriptor] + child_descriptor._xmodule.student_view.return_value.content = u'

    This is a secret

    ' + child_descriptor.displayable_items.return_value = [child_descriptor] + child_descriptor.runtime = descriptor_system + child_descriptor.xmodule_runtime = get_test_system() + + descriptor_system.load_item = {'child': child_descriptor, 'source': source_descriptor}.get # construct conditional module: cond_location = Location(["i4x", "edX", "conditional_test", "conditional", "SampleConditional"]) - field_data = DictFieldData({'data': '', 'location': cond_location}) - cond_module = ConditionalModule( - cond_descriptor, - system, + field_data = DictFieldData({ + 'data': '', + 'xml_attributes': {'attempted': 'true'}, + 'children': ['child'], + }) + + cond_descriptor = ConditionalDescriptor( + descriptor_system, field_data, ScopeIds(None, None, cond_location, cond_location) ) + cond_descriptor.xmodule_runtime = system + system.get_module = lambda desc: desc + cond_descriptor.get_required_module_descriptors = Mock(return_value=[source_descriptor]) # return dict: - return {'cond_module': cond_module, - 'source_module': source_module, - 'child_module': child_module} + return {'cond_module': cond_descriptor, + 'source_module': source_descriptor, + 'child_module': child_descriptor} class ConditionalModuleBasicTest(unittest.TestCase): @@ -129,16 +125,20 @@ def test_get_html(self): modules = ConditionalFactory.create(self.test_system) # because get_test_system returns the repr of the context dict passed to render_template, # we reverse it here - html = modules['cond_module'].get_html() - html_dict = literal_eval(html) - self.assertEqual(html_dict['element_id'], 'i4x-edX-conditional_test-conditional-SampleConditional') - self.assertEqual(html_dict['id'], 'i4x://edX/conditional_test/conditional/SampleConditional') - self.assertEqual(html_dict['depends'], 'i4x-edX-conditional_test-problem-SampleProblem') + html = modules['cond_module'].runtime.render(modules['cond_module'], None, 'student_view').content + expected = modules['cond_module'].xmodule_runtime.render_template('conditional_ajax.html', { + 'ajax_url': modules['cond_module'].xmodule_runtime.ajax_url, + 'element_id': 'i4x-edX-conditional_test-conditional-SampleConditional', + 'id': 'i4x://edX/conditional_test/conditional/SampleConditional', + 'depends': 'i4x-edX-conditional_test-problem-SampleProblem', + }) + self.assertEquals(expected, html) def test_handle_ajax(self): modules = ConditionalFactory.create(self.test_system) modules['source_module'].is_attempted = "false" ajax = json.loads(modules['cond_module'].handle_ajax('', '')) + modules['cond_module'].save() print "ajax: ", ajax html = ajax['html'] self.assertFalse(any(['This is a secret' in item for item in html])) @@ -146,6 +146,7 @@ def test_handle_ajax(self): # now change state of the capa problem to make it completed modules['source_module'].is_attempted = "true" ajax = json.loads(modules['cond_module'].handle_ajax('', '')) + modules['cond_module'].save() print "post-attempt ajax: ", ajax html = ajax['html'] self.assertTrue(any(['This is a secret' in item for item in html])) @@ -157,6 +158,7 @@ def test_error_as_source(self): ''' modules = ConditionalFactory.create(self.test_system, source_is_error_module=True) ajax = json.loads(modules['cond_module'].handle_ajax('', '')) + modules['cond_module'].save() html = ajax['html'] self.assertFalse(any(['This is a secret' in item for item in html])) @@ -196,7 +198,9 @@ def inner_get_module(descriptor): if isinstance(descriptor, Location): location = descriptor descriptor = self.modulestore.get_instance(course.id, location, depth=None) - return descriptor.xmodule(self.test_system) + descriptor.xmodule_runtime = get_test_system() + descriptor.xmodule_runtime.get_module = inner_get_module + return descriptor # edx - HarvardX # cond_test - ER22x @@ -209,20 +213,28 @@ def replace_urls(text, staticfiles_prefix=None, replace_prefix='/static/', cours module = inner_get_module(location) print "module: ", module - print "module.conditions_map: ", module.conditions_map print "module children: ", module.get_children() print "module display items (children): ", module.get_display_items() - html = module.get_html() + html = module.runtime.render(module, None, 'student_view').content print "html type: ", type(html) print "html: ", html - html_expect = "{'ajax_url': 'courses/course_id/modx/a_location', 'element_id': 'i4x-HarvardX-ER22x-conditional-condone', 'id': 'i4x://HarvardX/ER22x/conditional/condone', 'depends': 'i4x-HarvardX-ER22x-problem-choiceprob'}" + html_expect = module.xmodule_runtime.render_template( + 'conditional_ajax.html', + { + 'ajax_url': 'courses/course_id/modx/a_location', + 'element_id': 'i4x-HarvardX-ER22x-conditional-condone', + 'id': 'i4x://HarvardX/ER22x/conditional/condone', + 'depends': 'i4x-HarvardX-ER22x-problem-choiceprob' + } + ) self.assertEqual(html, html_expect) gdi = module.get_display_items() print "gdi=", gdi ajax = json.loads(module.handle_ajax('', '')) + module.save() print "ajax: ", ajax html = ajax['html'] self.assertFalse(any(['This is a secret' in item for item in html])) @@ -234,6 +246,7 @@ def replace_urls(text, staticfiles_prefix=None, replace_prefix='/static/', cours inner_module.save() ajax = json.loads(module.handle_ajax('', '')) + module.save() print "post-attempt ajax: ", ajax html = ajax['html'] self.assertTrue(any(['This is a secret' in item for item in html])) diff --git a/common/lib/xmodule/xmodule/tests/test_crowdsource_hinter.py b/common/lib/xmodule/xmodule/tests/test_crowdsource_hinter.py index 8ddb074d11bb..0757732f3fd1 100644 --- a/common/lib/xmodule/xmodule/tests/test_crowdsource_hinter.py +++ b/common/lib/xmodule/xmodule/tests/test_crowdsource_hinter.py @@ -10,6 +10,7 @@ from xmodule.vertical_module import VerticalModule, VerticalDescriptor from xblock.field_data import DictFieldData from xblock.fragment import Fragment +from xblock.core import XBlock from . import get_test_system @@ -62,7 +63,8 @@ def create(hints=None, """ A factory method for making CHM's """ - field_data = {'data': CHModuleFactory.sample_problem_xml} + # Should have a single child, but it doesn't matter what that child is + field_data = {'data': CHModuleFactory.sample_problem_xml, 'children': [None]} if hints is not None: field_data['hints'] = hints @@ -106,7 +108,8 @@ def create(hints=None, # Make the descriptor have a capa problem child. capa_descriptor = MagicMock() capa_descriptor.name = 'capa' - descriptor.get_children = lambda: [capa_descriptor] + capa_descriptor.displayable_items.return_value = [capa_descriptor] + descriptor.get_children.return_value = [capa_descriptor] # Make a fake capa module. capa_module = MagicMock() @@ -128,7 +131,7 @@ def compare_answer(ans1, ans2): responder.compare_answer = compare_answer capa_module.lcp.responders = {'responder0': responder} - capa_module.displayable_items = lambda: [capa_module] + capa_module.displayable_items.return_value = [capa_module] system = get_test_system() # Make the system have a marginally-functional get_module @@ -137,8 +140,7 @@ def fake_get_module(descriptor): """ A fake module-maker. """ - if descriptor.name == 'capa': - return capa_module + return capa_module system.get_module = fake_get_module module = CrowdsourceHinterModule(descriptor, system, DictFieldData(field_data), Mock()) @@ -205,15 +207,15 @@ def create(): return module -class FakeChild(object): +class FakeChild(XBlock): """ A fake Xmodule. """ def __init__(self): self.runtime = get_test_system() - self.runtime.ajax_url = 'this/is/a/fake/ajax/url' self.student_view = Mock(return_value=Fragment(self.get_html())) self.save = Mock() + self.id = 'i4x://this/is/a/fake/id' def get_html(self): """ @@ -243,7 +245,7 @@ def fake_get_display_items(): mock_module.get_display_items = fake_get_display_items out_html = mock_module.runtime.render(mock_module, None, 'student_view').content self.assertTrue('This is supposed to be test html.' in out_html) - self.assertTrue('this/is/a/fake/ajax/url' in out_html) + self.assertTrue('i4x://this/is/a/fake/id' in out_html) def test_gethtml_nochild(self): """ diff --git a/common/lib/xmodule/xmodule/tests/test_error_module.py b/common/lib/xmodule/xmodule/tests/test_error_module.py index 8a836b4a8a6a..5b898aa6f813 100644 --- a/common/lib/xmodule/xmodule/tests/test_error_module.py +++ b/common/lib/xmodule/xmodule/tests/test_error_module.py @@ -30,8 +30,8 @@ def test_error_module_xml_rendering(self): descriptor = error_module.ErrorDescriptor.from_xml( self.valid_xml, self.system, self.org, self.course, self.error_msg) self.assertIsInstance(descriptor, error_module.ErrorDescriptor) - module = descriptor.xmodule(self.system) - context_repr = module.get_html() + descriptor.xmodule_runtime = self.system + context_repr = self.system.render(descriptor, None, 'student_view').content self.assertIn(self.error_msg, context_repr) self.assertIn(repr(self.valid_xml), context_repr) @@ -44,8 +44,8 @@ def test_error_module_from_descriptor(self): error_descriptor = error_module.ErrorDescriptor.from_descriptor( descriptor, self.error_msg) self.assertIsInstance(error_descriptor, error_module.ErrorDescriptor) - module = error_descriptor.xmodule(self.system) - context_repr = module.get_html() + error_descriptor.xmodule_runtime = self.system + context_repr = self.system.render(error_descriptor, None, 'student_view').content self.assertIn(self.error_msg, context_repr) self.assertIn(repr(descriptor), context_repr) @@ -65,8 +65,8 @@ def test_non_staff_error_module_create(self): def test_from_xml_render(self): descriptor = error_module.NonStaffErrorDescriptor.from_xml( self.valid_xml, self.system, self.org, self.course) - module = descriptor.xmodule(self.system) - context_repr = module.get_html() + descriptor.xmodule_runtime = self.system + context_repr = self.system.render(descriptor, None, 'student_view').content self.assertNotIn(self.error_msg, context_repr) self.assertNotIn(repr(self.valid_xml), context_repr) @@ -79,7 +79,7 @@ def test_error_module_from_descriptor(self): error_descriptor = error_module.NonStaffErrorDescriptor.from_descriptor( descriptor, self.error_msg) self.assertIsInstance(error_descriptor, error_module.ErrorDescriptor) - module = error_descriptor.xmodule(self.system) - context_repr = module.get_html() + error_descriptor.xmodule_runtime = self.system + context_repr = self.system.render(error_descriptor, None, 'student_view').content self.assertNotIn(self.error_msg, context_repr) self.assertNotIn(str(descriptor), context_repr) diff --git a/common/lib/xmodule/xmodule/tests/test_peer_grading.py b/common/lib/xmodule/xmodule/tests/test_peer_grading.py index 0b07a676d301..53333ea17d92 100644 --- a/common/lib/xmodule/xmodule/tests/test_peer_grading.py +++ b/common/lib/xmodule/xmodule/tests/test_peer_grading.py @@ -199,6 +199,7 @@ def test_problem_list(self): html = peer_grading.peer_grading() self.assertIn("Peer-Graded", html) + class PeerGradingModuleLinkedTest(unittest.TestCase, DummyModulestore): """ Test peer grading that is linked to an open ended module. diff --git a/common/lib/xmodule/xmodule/tests/test_progress.py b/common/lib/xmodule/xmodule/tests/test_progress.py index 053768a00763..33d129663e35 100644 --- a/common/lib/xmodule/xmodule/tests/test_progress.py +++ b/common/lib/xmodule/xmodule/tests/test_progress.py @@ -137,6 +137,6 @@ class ModuleProgressTest(unittest.TestCase): ''' def test_xmodule_default(self): '''Make sure default get_progress exists, returns None''' - xm = x_module.XModule(None, get_test_system(), DictFieldData({'location': 'a://b/c/d/e'}), Mock()) + xm = x_module.XModule(Mock(), get_test_system(), DictFieldData({'location': 'a://b/c/d/e'}), Mock()) p = xm.get_progress() self.assertEqual(p, None) diff --git a/common/lib/xmodule/xmodule/tests/test_util_open_ended.py b/common/lib/xmodule/xmodule/tests/test_util_open_ended.py index a7b8a8833791..93d81e392ba1 100644 --- a/common/lib/xmodule/xmodule/tests/test_util_open_ended.py +++ b/common/lib/xmodule/xmodule/tests/test_util_open_ended.py @@ -103,7 +103,8 @@ def get_module_from_location(self, location, course): if not isinstance(location, Location): location = Location(location) descriptor = self.modulestore.get_instance(course.id, location, depth=None) - return descriptor.xmodule(self.test_system) + descriptor.xmodule_runtime = self.test_system + return descriptor # Task state for a module with self assessment then instructor assessment. TEST_STATE_SA_IN = ["{\"child_created\": false, \"child_attempts\": 2, \"version\": 1, \"child_history\": [{\"answer\": \"However venture pursuit he am mr cordial. Forming musical am hearing studied be luckily. Ourselves for determine attending how led gentleman sincerity. Valley afford uneasy joy she thrown though bed set. In me forming general prudent on country carried. Behaved an or suppose justice. Seemed whence how son rather easily and change missed. Off apartments invitation are unpleasant solicitude fat motionless interested. Hardly suffer wisdom wishes valley as an. As friendship advantages resolution it alteration stimulated he or increasing. \\r

    Now led tedious shy lasting females off. Dashwood marianne in of entrance be on wondered possible building. Wondered sociable he carriage in speedily margaret. Up devonshire of he thoroughly insensible alteration. An mr settling occasion insisted distance ladyship so. Not attention say frankness intention out dashwoods now curiosity. Stronger ecstatic as no judgment daughter speedily thoughts. Worse downs nor might she court did nay forth these. \", \"post_assessment\": \"[3, 3, 2, 2, 2]\", \"score\": 12}, {\"answer\": \"Delightful remarkably mr on announcing themselves entreaties favourable. About to in so terms voice at. Equal an would is found seems of. The particular friendship one sufficient terminated frequently themselves. It more shed went up is roof if loud case. Delay music in lived noise an. Beyond genius really enough passed is up. \\r

    John draw real poor on call my from. May she mrs furnished discourse extremely. Ask doubt noisy shade guest did built her him. Ignorant repeated hastened it do. Consider bachelor he yourself expenses no. Her itself active giving for expect vulgar months. Discovery commanded fat mrs remaining son she principle middleton neglected. Be miss he in post sons held. No tried is defer do money scale rooms. \", \"post_assessment\": \"[3, 3, 2, 2, 2]\", \"score\": 12}], \"max_score\": 12, \"child_state\": \"done\"}", "{\"child_created\": false, \"child_attempts\": 0, \"version\": 1, \"child_history\": [{\"answer\": \"However venture pursuit he am mr cordial. Forming musical am hearing studied be luckily. Ourselves for determine attending how led gentleman sincerity. Valley afford uneasy joy she thrown though bed set. In me forming general prudent on country carried. Behaved an or suppose justice. Seemed whence how son rather easily and change missed. Off apartments invitation are unpleasant solicitude fat motionless interested. Hardly suffer wisdom wishes valley as an. As friendship advantages resolution it alteration stimulated he or increasing. \\r

    Now led tedious shy lasting females off. Dashwood marianne in of entrance be on wondered possible building. Wondered sociable he carriage in speedily margaret. Up devonshire of he thoroughly insensible alteration. An mr settling occasion insisted distance ladyship so. Not attention say frankness intention out dashwoods now curiosity. Stronger ecstatic as no judgment daughter speedily thoughts. Worse downs nor might she court did nay forth these. \", \"post_assessment\": \"{\\\"submission_id\\\": 1460, \\\"score\\\": 12, \\\"feedback\\\": \\\"{\\\\\\\"feedback\\\\\\\": \\\\\\\"\\\\\\\"}\\\", \\\"success\\\": true, \\\"grader_id\\\": 5413, \\\"grader_type\\\": \\\"IN\\\", \\\"rubric_scores_complete\\\": true, \\\"rubric_xml\\\": \\\"\\\\nIdeas\\\\n3\\\\nContent\\\\n3\\\\nOrganization\\\\n2\\\\nStyle\\\\n2\\\\nVoice\\\\n2\\\"}\", \"score\": 12}, {\"answer\": \"Delightful remarkably mr on announcing themselves entreaties favourable. About to in so terms voice at. Equal an would is found seems of. The particular friendship one sufficient terminated frequently themselves. It more shed went up is roof if loud case. Delay music in lived noise an. Beyond genius really enough passed is up. \\r

    John draw real poor on call my from. May she mrs furnished discourse extremely. Ask doubt noisy shade guest did built her him. Ignorant repeated hastened it do. Consider bachelor he yourself expenses no. Her itself active giving for expect vulgar months. Discovery commanded fat mrs remaining son she principle middleton neglected. Be miss he in post sons held. No tried is defer do money scale rooms. \", \"post_assessment\": \"{\\\"submission_id\\\": 1462, \\\"score\\\": 12, \\\"feedback\\\": \\\"{\\\\\\\"feedback\\\\\\\": \\\\\\\"\\\\\\\"}\\\", \\\"success\\\": true, \\\"grader_id\\\": 5418, \\\"grader_type\\\": \\\"IN\\\", \\\"rubric_scores_complete\\\": true, \\\"rubric_xml\\\": \\\"\\\\nIdeas\\\\n3\\\\nContent\\\\n3\\\\nOrganization\\\\n2\\\\nStyle\\\\n2\\\\nVoice\\\\n2\\\"}\", \"score\": 12}], \"max_score\": 12, \"child_state\": \"post_assessment\"}"] diff --git a/common/lib/xmodule/xmodule/tests/test_xblock_wrappers.py b/common/lib/xmodule/xmodule/tests/test_xblock_wrappers.py index ea0cea800b8d..e86e75dd92be 100644 --- a/common/lib/xmodule/xmodule/tests/test_xblock_wrappers.py +++ b/common/lib/xmodule/xmodule/tests/test_xblock_wrappers.py @@ -2,6 +2,8 @@ Tests for the wrapping layer that provides the XBlock API using XModule/Descriptor functionality """ +# For tests, ignore access to protected members +# pylint: disable=protected-access from nose.tools import assert_equal # pylint: disable=E0611 from unittest.case import SkipTest @@ -17,6 +19,7 @@ from xmodule.course_module import CourseDescriptor from xmodule.combined_open_ended_module import CombinedOpenEndedDescriptor from xmodule.discussion_module import DiscussionDescriptor +from xmodule.error_module import ErrorDescriptor from xmodule.gst_module import GraphicalSliderToolDescriptor from xmodule.html_module import HtmlDescriptor from xmodule.peer_grading_module import PeerGradingDescriptor @@ -29,7 +32,7 @@ from xmodule.randomize_module import RandomizeDescriptor from xmodule.vertical_module import VerticalDescriptor from xmodule.wrapper_module import WrapperDescriptor -from xmodule.tests import get_test_descriptor_system +from xmodule.tests import get_test_descriptor_system, mock_render_template LEAF_XMODULES = ( AnnotatableDescriptor, @@ -40,21 +43,20 @@ HtmlDescriptor, PeerGradingDescriptor, PollDescriptor, + WordCloudDescriptor, # This is being excluded because it has dependencies on django #VideoDescriptor, - WordCloudDescriptor, ) CONTAINER_XMODULES = ( - CrowdsourceHinterDescriptor, - CourseDescriptor, - SequenceDescriptor, ConditionalDescriptor, + CourseDescriptor, + CrowdsourceHinterDescriptor, RandomizeDescriptor, + SequenceDescriptor, VerticalDescriptor, WrapperDescriptor, - CourseDescriptor, ) # These modules are editable in studio yet @@ -66,26 +68,24 @@ class TestXBlockWrapper(object): - @property def leaf_module_runtime(self): runtime = ModuleSystem( - render_template=lambda *args, **kwargs: u'{!r}, {!r}'.format(args, kwargs), + render_template=mock_render_template, anonymous_student_id='dummy_anonymous_student_id', open_ended_grading_interface={}, static_url='/static', ajax_url='dummy_ajax_url', - xmodule_field_data=lambda d: d._field_data, get_module=Mock(), replace_urls=Mock(), track_function=Mock(), + error_descriptor_class=ErrorDescriptor, ) return runtime def leaf_descriptor(self, descriptor_cls): location = 'i4x://org/course/category/name' runtime = get_test_descriptor_system() - runtime.render_template = lambda *args, **kwargs: u'{!r}, {!r}'.format(args, kwargs) return runtime.construct_xblock_from_class( descriptor_cls, ScopeIds(None, descriptor_cls.__name__, location, location), @@ -93,7 +93,10 @@ def leaf_descriptor(self, descriptor_cls): ) def leaf_module(self, descriptor_cls): - return self.leaf_descriptor(descriptor_cls).xmodule(self.leaf_module_runtime) + """Returns a descriptor that is ready to proxy as an xmodule""" + descriptor = self.leaf_descriptor(descriptor_cls) + descriptor.xmodule_runtime = self.leaf_module_runtime + return descriptor def container_module_runtime(self, depth): runtime = self.leaf_module_runtime @@ -104,10 +107,16 @@ def container_module_runtime(self, depth): runtime.position = 2 return runtime - def container_descriptor(self, descriptor_cls): + def container_descriptor(self, descriptor_cls, depth): + """Return an instance of `descriptor_cls` with `depth` levels of children""" location = 'i4x://org/course/category/name' runtime = get_test_descriptor_system() - runtime.render_template = lambda *args, **kwargs: u'{!r}, {!r}'.format(args, kwargs) + + if depth == 0: + runtime.load_item.side_effect = lambda x: self.leaf_module(HtmlDescriptor) + else: + runtime.load_item.side_effect = lambda x: self.container_module(VerticalDescriptor, depth - 1) + return runtime.construct_xblock_from_class( descriptor_cls, ScopeIds(None, descriptor_cls.__name__, location, location), @@ -117,7 +126,10 @@ def container_descriptor(self, descriptor_cls): ) def container_module(self, descriptor_cls, depth): - return self.container_descriptor(descriptor_cls).xmodule(self.container_module_runtime(depth)) + """Returns a descriptor that is ready to proxy as an xmodule""" + descriptor = self.container_descriptor(descriptor_cls, depth) + descriptor.xmodule_runtime = self.container_module_runtime(depth) + return descriptor class TestStudentView(TestXBlockWrapper): @@ -131,9 +143,11 @@ def test_student_view_leaf_node(self): # Check that when an xmodule is instantiated from descriptor_cls # it generates the same thing from student_view that it does from get_html def check_student_view_leaf_node(self, descriptor_cls): - xmodule = self.leaf_module(descriptor_cls) - assert_equal(xmodule.get_html(), xmodule.runtime.render(xmodule, None, 'student_view').content) - + descriptor = self.leaf_module(descriptor_cls) + assert_equal( + descriptor._xmodule.get_html(), + descriptor.runtime.render(descriptor, None, 'student_view').content + ) # Test that for all container XModule Descriptors, # their corresponding XModule renders the same thing using student_view @@ -147,13 +161,15 @@ def test_student_view_container_node(self): yield self.check_student_view_container_node_mixed, descriptor_cls yield self.check_student_view_container_node_xblocks_only, descriptor_cls - # Check that when an xmodule is generated from descriptor_cls # with only xmodule children, it generates the same html from student_view # as it does using get_html def check_student_view_container_node_xmodules_only(self, descriptor_cls): - xmodule = self.container_module(descriptor_cls, 2) - assert_equal(xmodule.get_html(), xmodule.runtime.render(xmodule, None, 'student_view').content) + descriptor = self.container_module(descriptor_cls, 2) + assert_equal( + descriptor._xmodule.get_html(), + descriptor.runtime.render(descriptor, None, 'student_view').content + ) # Check that when an xmodule is generated from descriptor_cls # with mixed xmodule and xblock children, it generates the same html from student_view @@ -206,7 +222,7 @@ def check_studio_view_container_node_xmodules_only(self, descriptor_cls): if descriptor_cls in NOT_STUDIO_EDITABLE: raise SkipTest(descriptor_cls.__name__ + "is not editable in studio") - descriptor = self.container_descriptor(descriptor_cls) + descriptor = self.container_descriptor(descriptor_cls, 2) assert_equal(descriptor.get_html(), descriptor.runtime.render(descriptor, None, 'studio_view').content) # Check that when a descriptor is generated from descriptor_cls diff --git a/common/lib/xmodule/xmodule/video_module.py b/common/lib/xmodule/xmodule/video_module.py index 96d0a0949800..71860a18641f 100644 --- a/common/lib/xmodule/xmodule/video_module.py +++ b/common/lib/xmodule/xmodule/video_module.py @@ -157,10 +157,6 @@ def handle_ajax(self, dispatch, data): log.debug(u"DISPATCH {0}".format(dispatch)) raise Http404() - def get_instance_state(self): - """Return information about state (position).""" - return json.dumps({'position': self.position}) - def get_html(self): caption_asset_path = "/static/subs/" diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index 34a4b53d2eb2..01e27509760b 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -1,7 +1,9 @@ import logging import yaml import os +import sys +from functools import partial from lxml import etree from collections import namedtuple from pkg_resources import resource_listdir, resource_string, resource_isdir @@ -13,6 +15,7 @@ from xblock.fields import Scope, Integer, Float, List, XBlockMixin, String from xblock.fragment import Fragment from xblock.runtime import Runtime +from xmodule.errortracker import exc_info_to_str from xmodule.modulestore.locator import BlockUsageLocator log = logging.getLogger(__name__) @@ -214,6 +217,114 @@ def get_child_by(self, selector): return child return None + def get_icon_class(self): + """ + Return a css class identifying this module in the context of an icon + """ + return self.icon_class + + # Functions used in the LMS + + def get_score(self): + """ + Score the student received on the problem, or None if there is no + score. + + Returns: + dictionary + {'score': integer, from 0 to get_max_score(), + 'total': get_max_score()} + + NOTE (vshnayder): not sure if this was the intended return value, but + that's what it's doing now. I suspect that we really want it to just + return a number. Would need to change (at least) capa and + modx_dispatch to match if we did that. + """ + return None + + def max_score(self): + """ Maximum score. Two notes: + + * This is generic; in abstract, a problem could be 3/5 points on one + randomization, and 5/7 on another + + * In practice, this is a Very Bad Idea, and (a) will break some code + in place (although that code should get fixed), and (b) break some + analytics we plan to put in place. + """ + return None + + def get_progress(self): + """ Return a progress.Progress object that represents how far the + student has gone in this module. Must be implemented to get correct + progress tracking behavior in nesting modules like sequence and + vertical. + + If this module has no notion of progress, return None. + """ + return None + + def bind_for_student(self, xmodule_runtime, field_data): + """ + Set up this XBlock to act as an XModule instead of an XModuleDescriptor. + + :param xmodule_runtime: the runtime to use when accessing student facing methods + :type xmodule_runtime: :class:`ModuleSystem` + :param field_data: The :class:`FieldData` to use for all subsequent data access + :type field_data: :class:`FieldData` + """ + # pylint: disable=attribute-defined-outside-init + self.xmodule_runtime = xmodule_runtime + self._field_data = field_data + + +class ProxyAttribute(object): + """ + A (python) descriptor that proxies attribute access. + + For example: + + class Foo(object): + def __init__(self, value): + self.foo_attr = value + + class Bar(object): + foo = Foo('x') + foo_attr = ProxyAttribute('foo', 'foo_attr') + + bar = Bar() + + assert bar.foo_attr == 'x' + bar.foo_attr = 'y' + assert bar.foo.foo_attr == 'y' + del bar.foo_attr + assert not hasattr(bar.foo, 'foo_attr') + """ + def __init__(self, source, name): + """ + :param source: The name of the attribute to proxy to + :param name: The name of the attribute to proxy + """ + self._source = source + self._name = name + + def __get__(self, instance, owner): + if instance is None: + return self + + return getattr(getattr(instance, self._source), self._name) + + def __set__(self, instance, value): + setattr(getattr(instance, self._source), self._name, value) + + def __delete__(self, instance): + delattr(getattr(instance, self._source), self._name) + + +module_attr = partial(ProxyAttribute, '_xmodule') # pylint: disable=invalid-name +descriptor_attr = partial(ProxyAttribute, 'descriptor') # pylint: disable=invalid-name +module_runtime_attr = partial(ProxyAttribute, 'xmodule_runtime') # pylint: disable=invalid-name + class XModule(XModuleMixin, HTMLSnippet, XBlock): # pylint: disable=abstract-method """ Implements a generic learning module. @@ -232,6 +343,11 @@ class XModule(XModuleMixin, HTMLSnippet, XBlock): # pylint: disable=abstract-me # in the module icon_class = 'other' + has_score = descriptor_attr('has_score') + _field_data_cache = descriptor_attr('_field_data_cache') + _field_data = descriptor_attr('_field_data') + _dirty_fields = descriptor_attr('_dirty_fields') + def __init__(self, descriptor, *args, **kwargs): """ Construct a new xmodule @@ -243,10 +359,19 @@ def __init__(self, descriptor, *args, **kwargs): field_data: A dictionary-like object that maps field names to values for those fields. """ - super(XModule, self).__init__(*args, **kwargs) - self.system = self.runtime + # Set the descriptor first so that we can proxy to it self.descriptor = descriptor + super(XModule, self).__init__(*args, **kwargs) self._loaded_children = None + self.system = self.runtime + + def __unicode__(self): + return u''.format(self.id) + + def handle_ajax(self, _dispatch, _data): + """ dispatch is last part of the URL. + data is a dictionary-like object with the content of the request""" + return u"" def get_children(self): """ @@ -268,9 +393,6 @@ def get_children(self): return self._loaded_children - def __unicode__(self): - return ''.format(self.id) - def get_child_descriptors(self): """ Returns the descriptors of the child modules @@ -286,58 +408,12 @@ def get_child_descriptors(self): """ return self.descriptor.get_children() - def get_icon_class(self): - """ - Return a css class identifying this module in the context of an icon - """ - return self.icon_class - - # Functions used in the LMS - - def get_score(self): - """ - Score the student received on the problem, or None if there is no - score. - - Returns: - dictionary - {'score': integer, from 0 to get_max_score(), - 'total': get_max_score()} - - NOTE (vshnayder): not sure if this was the intended return value, but - that's what it's doing now. I suspect that we really want it to just - return a number. Would need to change (at least) capa and - modx_dispatch to match if we did that. - """ - return None - - def max_score(self): - """ Maximum score. Two notes: - - * This is generic; in abstract, a problem could be 3/5 points on one - randomization, and 5/7 on another - - * In practice, this is a Very Bad Idea, and (a) will break some code - in place (although that code should get fixed), and (b) break some - analytics we plan to put in place. + def displayable_items(self): """ - return None - - def get_progress(self): - """ Return a progress.Progress object that represents how far the - student has gone in this module. Must be implemented to get correct - progress tracking behavior in nesting modules like sequence and - vertical. - - If this module has no notion of progress, return None. + Returns list of displayable modules contained by this module. If this + module is visible, should return [self]. """ - return None - - def handle_ajax(self, _dispatch, _data): - """ dispatch is last part of the URL. - data is a dictionary-like object with the content of the request""" - return "" - + return [self.descriptor] # ~~~~~~~~~~~~~~~ XBlock API Wrappers ~~~~~~~~~~~~~~~~ def student_view(self, context): @@ -489,24 +565,7 @@ def __init__(self, *args, **kwargs): # by following previous until None # definition_locator is only used by mongostores which separate definitions from blocks self.edited_by = self.edited_on = self.previous_version = self.update_version = self.definition_locator = None - self._child_instances = None - - - def xmodule(self, system): - """ - Returns an XModule. - - system: Module system - """ - # save any field changes - module = system.construct_xblock_from_class( - self.module_class, - descriptor=self, - scope_ids=self.scope_ids, - field_data=system.xmodule_field_data(self), - ) - module.save() - return module + self.xmodule_runtime = None def has_dynamic_children(self): """ @@ -644,6 +703,42 @@ def jsonify_value(field, json_choice): return metadata_fields + # ~~~~~~~~~~~~~~~ XModule Indirection ~~~~~~~~~~~~~~~~ + @property + def _xmodule(self): + """ + Returns the XModule corresponding to this descriptor. Expects that the system + already supports all of the attributes needed by xmodules + """ + assert self.xmodule_runtime is not None + assert self.xmodule_runtime.error_descriptor_class is not None + if self.xmodule_runtime.xmodule_instance is None: + try: + self.xmodule_runtime.xmodule_instance = self.xmodule_runtime.construct_xblock_from_class( + self.module_class, + descriptor=self, + scope_ids=self.scope_ids, + field_data=self._field_data, + ) + self.xmodule_runtime.xmodule_instance.save() + except Exception: # pylint: disable=broad-except + log.exception('Error creating xmodule') + descriptor = self.xmodule_runtime.error_descriptor_class.from_descriptor( + self, + error_msg=exc_info_to_str(sys.exc_info()) + ) + self.xmodule_runtime.xmodule_instance = descriptor._xmodule # pylint: disable=protected-access + return self.xmodule_runtime.xmodule_instance + + displayable_items = module_attr('displayable_items') + get_display_items = module_attr('get_display_items') + get_icon_class = module_attr('get_icon_class') + get_progress = module_attr('get_progress') + get_score = module_attr('get_score') + handle_ajax = module_attr('handle_ajax') + max_score = module_attr('max_score') + student_view = module_attr('student_view') + # ~~~~~~~~~~~~~~~ XBlock API Wrappers ~~~~~~~~~~~~~~~~ def studio_view(self, _context): """ @@ -766,6 +861,13 @@ def get_field_provenance(self, xblock, field): result['default_value'] = field.to_json(field.default) return result + def render(self, block, context, view_name): + if isinstance(block, (XModule, XModuleDescriptor)) and view_name == 'student_view': + assert block.xmodule_runtime is not None + return block.xmodule_runtime.render(block._xmodule, context, view_name) + else: + return super(DescriptorSystem, self).render(block, context, view_name) + class XMLParsingSystem(DescriptorSystem): def __init__(self, process_xml, policy, **kwargs): @@ -795,12 +897,12 @@ class ModuleSystem(ConfigurableFragmentWrapper, Runtime): # pylint: disable=abs """ def __init__( self, static_url, ajax_url, track_function, get_module, render_template, - replace_urls, xmodule_field_data, user=None, filestore=None, + replace_urls, user=None, filestore=None, debug=False, hostname="", xqueue=None, publish=None, node_path="", anonymous_student_id='', course_id=None, open_ended_grading_interface=None, s3_interface=None, cache=None, can_execute_unsafe_code=None, replace_course_urls=None, - replace_jump_to_id_urls=None, **kwargs): + replace_jump_to_id_urls=None, error_descriptor_class=None, **kwargs): """ Create a closure around the system environment. @@ -842,9 +944,6 @@ def __init__( publish(event) - A function that allows XModules to publish events (such as grade changes) - xmodule_field_data - A function that constructs a field_data for an xblock from its - corresponding descriptor - cache - A cache object with two methods: .get(key) returns an object from the cache or None. .set(key, value, timeout_secs=None) stores a value in the cache with a timeout. @@ -852,6 +951,8 @@ def __init__( can_execute_unsafe_code - A function returning a boolean, whether or not to allow the execution of unsafe, unsandboxed code. + error_descriptor_class - The class to use to render XModules with errors + """ # Right now, usage_store is unused, and field_data is always supplanted @@ -873,7 +974,6 @@ def __init__( self.anonymous_student_id = anonymous_student_id self.course_id = course_id self.user_is_staff = user is not None and user.is_staff - self.xmodule_field_data = xmodule_field_data if publish is None: publish = lambda e: None @@ -887,6 +987,8 @@ def __init__( self.can_execute_unsafe_code = can_execute_unsafe_code or (lambda: False) self.replace_course_urls = replace_course_urls self.replace_jump_to_id_urls = replace_jump_to_id_urls + self.error_descriptor_class = error_descriptor_class + self.xmodule_instance = None def get(self, attr): """ provide uniform access to attributes (like etree).""" diff --git a/common/test/data/test_import_course/vertical/vertical_test.xml b/common/test/data/test_import_course/vertical/vertical_test.xml index 68c5745f371d..3dd75a08c82c 100644 --- a/common/test/data/test_import_course/vertical/vertical_test.xml +++ b/common/test/data/test_import_course/vertical/vertical_test.xml @@ -1,7 +1,7 @@ -