From 3722685e1a185ff4a0a085bb380ed793cf6d1e59 Mon Sep 17 00:00:00 2001 From: Don Mitchell Date: Mon, 24 Jun 2013 15:44:49 -0400 Subject: [PATCH] No longer persist XModule templates Instead, we use XModule field default values when creating an empty XModule. Driven by this use case, we also allow for XModules to be created in memory without being persisted to the database at all. This necessitates a change to the Modulestore api, replacing clone_item with create_draft and save_xmodule. --- CHANGELOG.rst | 7 + README.md | 1 - .../contentstore/course_info_model.py | 4 +- .../contentstore/features/common.py | 2 +- .../component_settings_editor_helpers.py | 16 +- .../features/discussion-editor.py | 4 +- .../contentstore/features/html-editor.py | 2 +- .../contentstore/features/problem-editor.py | 2 +- .../features/studio-overview-togglesection.py | 8 +- cms/djangoapps/contentstore/features/video.py | 2 +- .../management/commands/update_templates.py | 10 - .../contentstore/module_info_model.py | 24 +-- .../contentstore/tests/test_contentstore.py | 110 ++++------ .../tests/test_course_settings.py | 1 + .../contentstore/tests/test_i18n.py | 1 - .../contentstore/tests/test_item.py | 196 +++++++++++++++++- cms/djangoapps/contentstore/tests/utils.py | 1 - cms/djangoapps/contentstore/utils.py | 8 +- .../contentstore/views/checklist.py | 6 +- .../contentstore/views/component.py | 65 ++++-- cms/djangoapps/contentstore/views/course.py | 34 ++- cms/djangoapps/contentstore/views/item.py | 79 ++++--- cms/djangoapps/contentstore/views/user.py | 2 +- .../models/settings/course_grading.py | 10 +- .../coffee/src/views/module_edit.coffee | 17 +- cms/static/coffee/src/views/tabs.coffee | 4 +- cms/static/coffee/src/views/unit.coffee | 4 +- cms/static/js/base.js | 22 +- cms/templates/index.html | 4 +- cms/templates/overview.html | 12 +- cms/templates/unit.html | 99 +++++---- cms/templates/widgets/units.html | 2 +- cms/urls.py | 2 +- common/djangoapps/terrain/course_helpers.py | 2 - common/djangoapps/tests.py | 6 +- common/lib/xmodule/xmodule/abtest_module.py | 2 - .../lib/xmodule/xmodule/annotatable_module.py | 1 - .../xmodule/combined_open_ended_module.py | 1 - common/lib/xmodule/xmodule/course_module.py | 2 - common/lib/xmodule/xmodule/error_module.py | 7 +- common/lib/xmodule/xmodule/foldit_module.py | 1 - common/lib/xmodule/xmodule/gst_module.py | 1 - common/lib/xmodule/xmodule/html_module.py | 4 +- .../spec/combinedopenended/edit_spec.coffee | 4 +- .../xmodule/js/spec/problem/edit_spec.coffee | 4 +- .../js/src/combinedopenended/edit.coffee | 3 +- .../xmodule/js/src/problem/edit.coffee | 5 +- .../xmodule/xmodule/modulestore/__init__.py | 9 +- .../xmodule/xmodule/modulestore/mongo/base.py | 181 +++++++++++----- .../xmodule/modulestore/mongo/draft.py | 56 +++-- .../xmodule/modulestore/tests/django_utils.py | 20 -- .../xmodule/modulestore/tests/factories.py | 120 +++-------- .../xmodule/modulestore/tests/test_mongo.py | 4 +- common/lib/xmodule/xmodule/modulestore/xml.py | 5 +- .../combined_open_ended_modulev1.py | 1 - .../open_ended_module.py | 1 - .../self_assessment_module.py | 1 - .../xmodule/xmodule/peer_grading_module.py | 1 - common/lib/xmodule/xmodule/poll_module.py | 1 - common/lib/xmodule/xmodule/templates.py | 96 +-------- .../xmodule/templates/about/empty.yaml | 1 - .../templates/annotatable/default.yaml | 1 - .../templates/combinedopenended/default.yaml | 1 - .../xmodule/templates/course/empty.yaml | 1 - .../xmodule/templates/courseinfo/empty.yaml | 1 - .../xmodule/templates/default/empty.yaml | 1 - .../xmodule/templates/discussion/default.yaml | 1 - .../xmodule/templates/html/announcement.yaml | 1 - .../xmodule/xmodule/templates/html/empty.yaml | 1 - .../xmodule/templates/html/everything.yaml | 33 --- .../xmodule/templates/html/latex_html.yaml | 1 - .../templates/peer_grading/default.yaml | 1 - .../templates/problem/circuitschematic.yaml | 2 +- .../templates/problem/customgrader.yaml | 89 ++++---- .../xmodule/templates/problem/empty.yaml | 1 - .../templates/problem/emptyadvanced.yaml | 10 - .../templates/problem/forumularesponse.yaml | 3 +- .../templates/problem/imageresponse.yaml | 4 +- .../templates/problem/latex_problem.yaml | 2 +- .../templates/problem/multiplechoice.yaml | 13 +- .../templates/problem/numericalresponse.yaml | 22 +- .../templates/problem/optionresponse.yaml | 11 +- .../templates/problem/problem_with_hint.yaml | 3 +- .../templates/problem/string_response.yaml | 11 +- .../templates/sequence/with_video.yaml | 7 - .../xmodule/templates/statictab/empty.yaml | 1 - .../xmodule/templates/video/default.yaml | 1 - .../xmodule/templates/videoalpha/default.yaml | 1 - .../xmodule/templates/word_cloud/default.yaml | 1 - .../lib/xmodule/xmodule/tests/test_logic.py | 3 +- .../xmodule/xmodule/tests/test_xml_module.py | 1 + common/lib/xmodule/xmodule/video_module.py | 1 - .../lib/xmodule/xmodule/videoalpha_module.py | 1 - common/lib/xmodule/xmodule/xml_module.py | 1 + lms/djangoapps/courseware/features/common.py | 5 +- .../courseware/features/navigation.py | 11 +- .../courseware/features/problems.py | 2 +- .../courseware/features/problems_setup.py | 6 +- lms/djangoapps/courseware/features/video.py | 7 +- lms/djangoapps/courseware/tests/__init__.py | 12 +- .../tests/test_submitting_problems.py | 6 +- .../courseware/tests/test_video_mongo.py | 2 +- .../courseware/tests/test_videoalpha_mongo.py | 2 +- lms/djangoapps/courseware/tests/tests.py | 9 +- .../instructor/tests/test_gradebook.py | 10 +- .../instructor_task/tests/test_base.py | 4 +- .../instructor_task/tests/test_integration.py | 4 +- lms/djangoapps/open_ended_grading/tests.py | 2 +- rakelib/django.rake | 5 - 109 files changed, 819 insertions(+), 801 deletions(-) delete mode 100644 cms/djangoapps/contentstore/management/commands/update_templates.py delete mode 100644 common/lib/xmodule/xmodule/templates/about/empty.yaml delete mode 100644 common/lib/xmodule/xmodule/templates/annotatable/default.yaml delete mode 100644 common/lib/xmodule/xmodule/templates/combinedopenended/default.yaml delete mode 100644 common/lib/xmodule/xmodule/templates/course/empty.yaml delete mode 100644 common/lib/xmodule/xmodule/templates/courseinfo/empty.yaml delete mode 100644 common/lib/xmodule/xmodule/templates/default/empty.yaml delete mode 100644 common/lib/xmodule/xmodule/templates/discussion/default.yaml delete mode 100644 common/lib/xmodule/xmodule/templates/html/empty.yaml delete mode 100644 common/lib/xmodule/xmodule/templates/html/everything.yaml delete mode 100644 common/lib/xmodule/xmodule/templates/peer_grading/default.yaml delete mode 100644 common/lib/xmodule/xmodule/templates/problem/empty.yaml delete mode 100644 common/lib/xmodule/xmodule/templates/problem/emptyadvanced.yaml delete mode 100644 common/lib/xmodule/xmodule/templates/sequence/with_video.yaml delete mode 100644 common/lib/xmodule/xmodule/templates/statictab/empty.yaml delete mode 100644 common/lib/xmodule/xmodule/templates/video/default.yaml delete mode 100644 common/lib/xmodule/xmodule/templates/videoalpha/default.yaml delete mode 100644 common/lib/xmodule/xmodule/templates/word_cloud/default.yaml diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 4d117a9c73c..04c8a5baaee 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -43,6 +43,13 @@ history of background tasks for a given problem and student. Blades: Small UX fix on capa multiple-choice problems. Make labels only as wide as the text to reduce accidental choice selections. +Studio: +- use xblock field defaults to initialize all new instances' fields and +only use templates as override samples. +- create new instances via in memory create_xmodule and related methods rather +than cloning a db record. +- have an explicit method for making a draft copy as distinct from making a new module. + Studio: Remove XML from the video component editor. All settings are moved to be edited as metadata. diff --git a/README.md b/README.md index e533459c8b7..2208fe1cada 100644 --- a/README.md +++ b/README.md @@ -239,7 +239,6 @@ CMS templates. Fortunately, `rake` will do all of this for you! Just run: $ rake django-admin[syncdb] $ rake django-admin[migrate] - $ rake cms:update_templates If you are running these commands using the [`zsh`](http://www.zsh.org/) shell, zsh will assume that you are doing diff --git a/cms/djangoapps/contentstore/course_info_model.py b/cms/djangoapps/contentstore/course_info_model.py index ada38739920..7e1e6470ff3 100644 --- a/cms/djangoapps/contentstore/course_info_model.py +++ b/cms/djangoapps/contentstore/course_info_model.py @@ -20,8 +20,8 @@ def get_course_updates(location): try: course_updates = modulestore('direct').get_item(location) except ItemNotFoundError: - template = Location(['i4x', 'edx', "templates", 'course_info', "Empty"]) - course_updates = modulestore('direct').clone_item(template, Location(location)) + modulestore('direct').create_and_save_xmodule(location) + course_updates = modulestore('direct').get_item(location) # current db rep: {"_id" : locationjson, "definition" : { "data" : "
    [

  1. date

    content
    2. ]
"} "metadata" : ignored} location_base = course_updates.location.url() diff --git a/cms/djangoapps/contentstore/features/common.py b/cms/djangoapps/contentstore/features/common.py index cb24af47e0d..756adad7c40 100644 --- a/cms/djangoapps/contentstore/features/common.py +++ b/cms/djangoapps/contentstore/features/common.py @@ -208,7 +208,7 @@ def set_date_and_time(date_css, desired_date, time_css, desired_time): def i_created_a_video_component(step): world.create_component_instance( step, '.large-video-icon', - 'i4x://edx/templates/video/default', + 'video', '.xmodule_VideoModule' ) diff --git a/cms/djangoapps/contentstore/features/component_settings_editor_helpers.py b/cms/djangoapps/contentstore/features/component_settings_editor_helpers.py index 43164f62be4..2f1788c6a48 100644 --- a/cms/djangoapps/contentstore/features/component_settings_editor_helpers.py +++ b/cms/djangoapps/contentstore/features/component_settings_editor_helpers.py @@ -7,9 +7,9 @@ @world.absorb -def create_component_instance(step, component_button_css, instance_id, expected_css): +def create_component_instance(step, component_button_css, category, expected_css, boilerplate=None): click_new_component_button(step, component_button_css) - click_component_from_menu(instance_id, expected_css) + click_component_from_menu(category, boilerplate, expected_css) @world.absorb @@ -19,7 +19,7 @@ def click_new_component_button(step, component_button_css): @world.absorb -def click_component_from_menu(instance_id, expected_css): +def click_component_from_menu(category, boilerplate, expected_css): """ Creates a component from `instance_id`. For components with more than one template, clicks on `elem_css` to create the new @@ -27,11 +27,13 @@ def click_component_from_menu(instance_id, expected_css): as the user clicks the appropriate button, so we assert that the expected component is present. """ - elem_css = "a[data-location='%s']" % instance_id + if boilerplate: + elem_css = "a[data-category='{}'][data-boilerplate='{}']".format(category, boilerplate) + else: + elem_css = "a[data-category='{}']:not([data-boilerplate])".format(category) elements = world.css_find(elem_css) - assert(len(elements) == 1) - if elements[0]['id'] == instance_id: # If this is a component with multiple templates - world.css_click(elem_css) + assert_equal(len(elements), 1) + world.css_click(elem_css) assert_equal(1, len(world.css_find(expected_css))) diff --git a/cms/djangoapps/contentstore/features/discussion-editor.py b/cms/djangoapps/contentstore/features/discussion-editor.py index a4a4b716687..8e4becb62e3 100644 --- a/cms/djangoapps/contentstore/features/discussion-editor.py +++ b/cms/djangoapps/contentstore/features/discussion-editor.py @@ -8,7 +8,7 @@ def i_created_discussion_tag(step): world.create_component_instance( step, '.large-discussion-icon', - 'i4x://edx/templates/discussion/Discussion_Tag', + 'discussion', '.xmodule_DiscussionModule' ) @@ -26,5 +26,5 @@ def i_see_only_the_settings_and_values(step): @step('creating a discussion takes a single click') def discussion_takes_a_single_click(step): assert(not world.is_css_present('.xmodule_DiscussionModule')) - world.css_click("a[data-location='i4x://edx/templates/discussion/Discussion_Tag']") + world.css_click("a[data-category='discussion']") assert(world.is_css_present('.xmodule_DiscussionModule')) diff --git a/cms/djangoapps/contentstore/features/html-editor.py b/cms/djangoapps/contentstore/features/html-editor.py index 53462ba0947..b03388c89ac 100644 --- a/cms/djangoapps/contentstore/features/html-editor.py +++ b/cms/djangoapps/contentstore/features/html-editor.py @@ -7,7 +7,7 @@ @step('I have created a Blank HTML Page$') def i_created_blank_html_page(step): world.create_component_instance( - step, '.large-html-icon', 'i4x://edx/templates/html/Blank_HTML_Page', + step, '.large-html-icon', 'html', '.xmodule_HtmlModule' ) diff --git a/cms/djangoapps/contentstore/features/problem-editor.py b/cms/djangoapps/contentstore/features/problem-editor.py index 15f5da95e9e..99b693225d2 100644 --- a/cms/djangoapps/contentstore/features/problem-editor.py +++ b/cms/djangoapps/contentstore/features/problem-editor.py @@ -158,7 +158,7 @@ def create_latex_problem(step): world.click_new_component_button(step, '.large-problem-icon') # Go to advanced tab. world.css_click('#ui-id-2') - world.click_component_from_menu("i4x://edx/templates/problem/Problem_Written_in_LaTeX", '.xmodule_CapaModule') + world.click_component_from_menu("problem", "latex_problem.yaml", '.xmodule_CapaModule') @step('I edit and compile the High Level Source') diff --git a/cms/djangoapps/contentstore/features/studio-overview-togglesection.py b/cms/djangoapps/contentstore/features/studio-overview-togglesection.py index 9ab17fbdacc..41e39513ea9 100644 --- a/cms/djangoapps/contentstore/features/studio-overview-togglesection.py +++ b/cms/djangoapps/contentstore/features/studio-overview-togglesection.py @@ -22,7 +22,7 @@ def have_a_course_with_1_section(step): section = world.ItemFactory.create(parent_location=course.location) subsection1 = world.ItemFactory.create( parent_location=section.location, - template='i4x://edx/templates/sequential/Empty', + category='sequential', display_name='Subsection One',) @@ -33,18 +33,18 @@ def have_a_course_with_two_sections(step): section = world.ItemFactory.create(parent_location=course.location) subsection1 = world.ItemFactory.create( parent_location=section.location, - template='i4x://edx/templates/sequential/Empty', + category='sequential', display_name='Subsection One',) section2 = world.ItemFactory.create( parent_location=course.location, display_name='Section Two',) subsection2 = world.ItemFactory.create( parent_location=section2.location, - template='i4x://edx/templates/sequential/Empty', + category='sequential', display_name='Subsection Alpha',) subsection3 = world.ItemFactory.create( parent_location=section2.location, - template='i4x://edx/templates/sequential/Empty', + category='sequential', display_name='Subsection Beta',) diff --git a/cms/djangoapps/contentstore/features/video.py b/cms/djangoapps/contentstore/features/video.py index cb59193f176..a6a362befc7 100644 --- a/cms/djangoapps/contentstore/features/video.py +++ b/cms/djangoapps/contentstore/features/video.py @@ -14,7 +14,7 @@ def does_not_autoplay(_step): @step('creating a video takes a single click') def video_takes_a_single_click(_step): assert(not world.is_css_present('.xmodule_VideoModule')) - world.css_click("a[data-location='i4x://edx/templates/video/default']") + world.css_click("a[data-category='video']") assert(world.is_css_present('.xmodule_VideoModule')) diff --git a/cms/djangoapps/contentstore/management/commands/update_templates.py b/cms/djangoapps/contentstore/management/commands/update_templates.py deleted file mode 100644 index 36348314b9d..00000000000 --- a/cms/djangoapps/contentstore/management/commands/update_templates.py +++ /dev/null @@ -1,10 +0,0 @@ -from xmodule.templates import update_templates -from xmodule.modulestore.django import modulestore -from django.core.management.base import BaseCommand - - -class Command(BaseCommand): - help = 'Imports and updates the Studio component templates from the code pack and put in the DB' - - def handle(self, *args, **options): - update_templates(modulestore('direct')) diff --git a/cms/djangoapps/contentstore/module_info_model.py b/cms/djangoapps/contentstore/module_info_model.py index 726d4bb0ceb..bce4b0326cf 100644 --- a/cms/djangoapps/contentstore/module_info_model.py +++ b/cms/djangoapps/contentstore/module_info_model.py @@ -3,13 +3,13 @@ from xmodule.modulestore import Location -def get_module_info(store, location, parent_location=None, rewrite_static_links=False): +def get_module_info(store, location, rewrite_static_links=False): try: module = store.get_item(location) except ItemNotFoundError: # create a new one - template_location = Location(['i4x', 'edx', 'templates', location.category, 'Empty']) - module = store.clone_item(template_location, location) + store.create_and_save_xmodule(location) + module = store.get_item(location) data = module.data if rewrite_static_links: @@ -29,7 +29,8 @@ def get_module_info(store, location, parent_location=None, rewrite_static_links= 'id': module.location.url(), 'data': data, # TODO (cpennington): This really shouldn't have to do this much reaching in to get the metadata - 'metadata': module._model_data._kvs._metadata + # what's the intent here? all metadata incl inherited & namespaced? + 'metadata': module.xblock_kvs._metadata } @@ -37,14 +38,11 @@ def set_module_info(store, location, post_data): module = None try: module = store.get_item(location) - except: - pass - - if module is None: - # new module at this location - # presume that we have an 'Empty' template - template_location = Location(['i4x', 'edx', 'templates', location.category, 'Empty']) - module = store.clone_item(template_location, location) + except ItemNotFoundError: + # new module at this location: almost always used for the course about pages; thus, no parent. (there + # are quite a handful of about page types available for a course and only the overview is pre-created) + store.create_and_save_xmodule(location) + module = store.get_item(location) if post_data.get('data') is not None: data = post_data['data'] @@ -79,4 +77,4 @@ def set_module_info(store, location, post_data): # commit to datastore # TODO (cpennington): This really shouldn't have to do this much reaching in to get the metadata - store.update_metadata(location, module._model_data._kvs._metadata) + store.update_metadata(location, module.xblock_kvs._metadata) diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 6099b60eb19..fa7c45cb1df 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -24,12 +24,11 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory -from xmodule.modulestore import Location +from xmodule.modulestore import Location, mongo from xmodule.modulestore.store_utilities import clone_course from xmodule.modulestore.store_utilities import delete_course from xmodule.modulestore.django import modulestore from xmodule.contentstore.django import contentstore, _CONTENTSTORE -from xmodule.templates import update_templates from xmodule.modulestore.xml_exporter import export_to_xml from xmodule.modulestore.xml_importer import import_from_xml, perform_xlint from xmodule.modulestore.inheritance import own_metadata @@ -183,7 +182,7 @@ def test_get_items(self): html_module = draft_store.get_item(['i4x', 'edX', 'simple', 'html', 'test_html', None]) - draft_store.clone_item(html_module.location, html_module.location) + draft_store.convert_to_draft(html_module.location) # now query get_items() to get this location with revision=None, this should just # return back a single item (not 2) @@ -215,7 +214,7 @@ def test_draft_metadata(self): self.assertEqual(html_module.lms.graceperiod, course.lms.graceperiod) self.assertNotIn('graceperiod', own_metadata(html_module)) - draft_store.clone_item(html_module.location, html_module.location) + draft_store.convert_to_draft(html_module.location) # refetch to check metadata html_module = draft_store.get_item(['i4x', 'edX', 'simple', 'html', 'test_html', None]) @@ -233,7 +232,7 @@ def test_draft_metadata(self): self.assertNotIn('graceperiod', own_metadata(html_module)) # put back in draft and change metadata and see if it's now marked as 'own_metadata' - draft_store.clone_item(html_module.location, html_module.location) + draft_store.convert_to_draft(html_module.location) html_module = draft_store.get_item(['i4x', 'edX', 'simple', 'html', 'test_html', None]) new_graceperiod = timedelta(hours=1) @@ -255,7 +254,7 @@ def test_draft_metadata(self): draft_store.publish(html_module.location, 0) # and re-read and verify 'own-metadata' - draft_store.clone_item(html_module.location, html_module.location) + draft_store.convert_to_draft(html_module.location) html_module = draft_store.get_item(['i4x', 'edX', 'simple', 'html', 'test_html', None]) self.assertIn('graceperiod', own_metadata(html_module)) @@ -278,7 +277,7 @@ def test_get_depth_with_drafts(self): ) # put into draft - modulestore('draft').clone_item(problem.location, problem.location) + modulestore('draft').convert_to_draft(problem.location) # make sure we can query that item and verify that it is a draft draft_problem = modulestore('draft').get_item( @@ -574,7 +573,6 @@ def test_empty_trashcan(self): def test_clone_course(self): course_data = { - 'template': 'i4x://edx/templates/course/Empty', 'org': 'MITx', 'number': '999', 'display_name': 'Robot Super Course', @@ -614,10 +612,10 @@ def test_illegal_draft_crud_ops(self): CourseFactory.create(org='MITx', course='999', display_name='Robot Super Course') location = Location('i4x://MITx/999/chapter/neuvo') - self.assertRaises(InvalidVersionError, draft_store.clone_item, 'i4x://edx/templates/chapter/Empty', - location) - direct_store.clone_item('i4x://edx/templates/chapter/Empty', location) - self.assertRaises(InvalidVersionError, draft_store.clone_item, location, location) + # Ensure draft mongo store does not allow us to create chapters either directly or via convert to draft + self.assertRaises(InvalidVersionError, draft_store.create_and_save_xmodule, location) + direct_store.create_and_save_xmodule(location) + self.assertRaises(InvalidVersionError, draft_store.convert_to_draft, location) self.assertRaises(InvalidVersionError, draft_store.update_item, location, 'chapter data') @@ -687,26 +685,35 @@ def test_export_course(self): import_from_xml(module_store, 'common/test/data/', ['toy']) location = CourseDescriptor.id_to_location('edX/toy/2012_Fall') - # get a vertical (and components in it) to put into 'draft' - vertical = module_store.get_item(Location(['i4x', 'edX', 'toy', - 'vertical', 'vertical_test', None]), depth=1) - - draft_store.clone_item(vertical.location, vertical.location) - + # get a vertical (and components in it) to copy into an orphan sub dag + vertical = module_store.get_item( + Location(['i4x', 'edX', 'toy', 'vertical', 'vertical_test', None]), + depth=1 + ) # We had a bug where orphaned draft nodes caused export to fail. This is here to cover that case. - draft_store.clone_item(vertical.location, Location(['i4x', 'edX', 'toy', - 'vertical', 'no_references', 'draft'])) - + vertical.location = mongo.draft.as_draft(vertical.location.replace(name='no_references')) + draft_store.save_xmodule(vertical) + orphan_vertical = draft_store.get_item(vertical.location) + self.assertEqual(orphan_vertical.location.name, 'no_references') + + # get the original vertical (and components in it) to put into 'draft' + vertical = module_store.get_item( + Location(['i4x', 'edX', 'toy', 'vertical', 'vertical_test', None]), + depth=1) + self.assertEqual(len(orphan_vertical.children), len(vertical.children)) + draft_store.convert_to_draft(vertical.location) for child in vertical.get_children(): - draft_store.clone_item(child.location, child.location) + draft_store.convert_to_draft(child.location) root_dir = path(mkdtemp_clean()) - # now create a private vertical - private_vertical = draft_store.clone_item(vertical.location, - Location(['i4x', 'edX', 'toy', 'vertical', 'a_private_vertical', None])) + # now create a new/different private (draft only) vertical + vertical.location = mongo.draft.as_draft(Location(['i4x', 'edX', 'toy', 'vertical', 'a_private_vertical', None])) + draft_store.save_xmodule(vertical) + private_vertical = draft_store.get_item(vertical.location) + vertical = None # blank out b/c i destructively manipulated its location 2 lines above - # add private to list of children + # add the new private to list of children sequential = module_store.get_item(Location(['i4x', 'edX', 'toy', 'sequential', 'vertical_sequential', None])) private_location_no_draft = private_vertical.location.replace(revision=None) @@ -885,7 +892,6 @@ def setUp(self): self.client.login(username=uname, password=password) self.course_data = { - 'template': 'i4x://edx/templates/course/Empty', 'org': 'MITx', 'number': '999', 'display_name': 'Robot Super Course', @@ -1029,17 +1035,17 @@ def test_course_overview_view_with_course(self): html=True ) - def test_clone_item(self): + def test_create_item(self): """Test cloning an item. E.g. creating a new section""" CourseFactory.create(org='MITx', course='999', display_name='Robot Super Course') section_data = { 'parent_location': 'i4x://MITx/999/course/Robot_Super_Course', - 'template': 'i4x://edx/templates/chapter/Empty', + 'category': 'chapter', 'display_name': 'Section One', } - resp = self.client.post(reverse('clone_item'), section_data) + resp = self.client.post(reverse('create_item'), section_data) self.assertEqual(resp.status_code, 200) data = parse_json(resp) @@ -1054,14 +1060,14 @@ def test_capa_module(self): problem_data = { 'parent_location': 'i4x://MITx/999/course/Robot_Super_Course', - 'template': 'i4x://edx/templates/problem/Blank_Common_Problem' + 'category': 'problem' } - resp = self.client.post(reverse('clone_item'), problem_data) + resp = self.client.post(reverse('create_item'), problem_data) self.assertEqual(resp.status_code, 200) payload = parse_json(resp) - problem_loc = payload['id'] + problem_loc = Location(payload['id']) problem = get_modulestore(problem_loc).get_item(problem_loc) # should be a CapaDescriptor self.assertIsInstance(problem, CapaDescriptor, "New problem is not a CapaDescriptor") @@ -1194,10 +1200,9 @@ def test_forum_id_generation(self): CourseFactory.create(org='edX', course='999', display_name='Robot Super Course') new_component_location = Location('i4x', 'edX', '999', 'discussion', 'new_component') - source_template_location = Location('i4x', 'edx', 'templates', 'discussion', 'Discussion_Tag') # crate a new module and add it as a child to a vertical - module_store.clone_item(source_template_location, new_component_location) + module_store.create_and_save_xmodule(new_component_location) new_discussion_item = module_store.get_item(new_component_location) @@ -1218,10 +1223,9 @@ def _signal_hander(modulestore=None, course_id=None, location=None, **kwargs): module_store.modulestore_update_signal.connect(_signal_hander) new_component_location = Location('i4x', 'edX', '999', 'html', 'new_component') - source_template_location = Location('i4x', 'edx', 'templates', 'html', 'Blank_HTML_Page') # crate a new module - module_store.clone_item(source_template_location, new_component_location) + module_store.create_and_save_xmodule(new_component_location) finally: module_store.modulestore_update_signal = None @@ -1239,14 +1243,14 @@ def test_metadata_inheritance(self): # let's assert on the metadata_inheritance on an existing vertical for vertical in verticals: self.assertEqual(course.lms.xqa_key, vertical.lms.xqa_key) + self.assertEqual(course.start, vertical.lms.start) self.assertGreater(len(verticals), 0) new_component_location = Location('i4x', 'edX', 'toy', 'html', 'new_component') - source_template_location = Location('i4x', 'edx', 'templates', 'html', 'Blank_HTML_Page') # crate a new module and add it as a child to a vertical - module_store.clone_item(source_template_location, new_component_location) + module_store.create_and_save_xmodule(new_component_location) parent = verticals[0] module_store.update_children(parent.location, parent.children + [new_component_location.url()]) @@ -1256,6 +1260,8 @@ def test_metadata_inheritance(self): # check for grace period definition which should be defined at the course level self.assertEqual(parent.lms.graceperiod, new_module.lms.graceperiod) + self.assertEqual(parent.lms.start, new_module.lms.start) + self.assertEqual(course.start, new_module.lms.start) self.assertEqual(course.lms.xqa_key, new_module.lms.xqa_key) @@ -1293,29 +1299,3 @@ def test_default_metadata_inheritance(self): self.assertEqual(course.textbooks, fetched_course.textbooks) # is this test too strict? i.e., it requires the dicts to be == self.assertEqual(course.checklists, fetched_course.checklists) - -class TemplateTestCase(ModuleStoreTestCase): - - def test_template_cleanup(self): - module_store = modulestore('direct') - - # insert a bogus template in the store - bogus_template_location = Location('i4x', 'edx', 'templates', 'html', 'bogus') - source_template_location = Location('i4x', 'edx', 'templates', 'html', 'Blank_HTML_Page') - - module_store.clone_item(source_template_location, bogus_template_location) - - verify_create = module_store.get_item(bogus_template_location) - self.assertIsNotNone(verify_create) - - # now run cleanup - update_templates(modulestore('direct')) - - # now try to find dangling template, it should not be in DB any longer - asserted = False - try: - verify_create = module_store.get_item(bogus_template_location) - except ItemNotFoundError: - asserted = True - - self.assertTrue(asserted) diff --git a/cms/djangoapps/contentstore/tests/test_course_settings.py b/cms/djangoapps/contentstore/tests/test_course_settings.py index 6c23e682403..fc04ad0a587 100644 --- a/cms/djangoapps/contentstore/tests/test_course_settings.py +++ b/cms/djangoapps/contentstore/tests/test_course_settings.py @@ -19,6 +19,7 @@ from models.settings.course_metadata import CourseMetadata from xmodule.modulestore.xml_importer import import_from_xml +from xmodule.modulestore.django import modulestore from xmodule.fields import Date from .utils import CourseTestCase diff --git a/cms/djangoapps/contentstore/tests/test_i18n.py b/cms/djangoapps/contentstore/tests/test_i18n.py index a292b7316e0..88df19ec2db 100644 --- a/cms/djangoapps/contentstore/tests/test_i18n.py +++ b/cms/djangoapps/contentstore/tests/test_i18n.py @@ -35,7 +35,6 @@ def setUp(self): self.user.save() self.course_data = { - 'template': 'i4x://edx/templates/course/Empty', 'org': 'MITx', 'number': '999', 'display_name': 'Robot Super Course', diff --git a/cms/djangoapps/contentstore/tests/test_item.py b/cms/djangoapps/contentstore/tests/test_item.py index 4e6c951d9b9..2b514c07262 100644 --- a/cms/djangoapps/contentstore/tests/test_item.py +++ b/cms/djangoapps/contentstore/tests/test_item.py @@ -1,6 +1,9 @@ from contentstore.tests.test_course_settings import CourseTestCase from xmodule.modulestore.tests.factories import CourseFactory from django.core.urlresolvers import reverse +from xmodule.capa_module import CapaDescriptor +import json +from xmodule.modulestore.django import modulestore class DeleteItem(CourseTestCase): @@ -11,14 +14,199 @@ def setUp(self): def testDeleteStaticPage(self): # Add static tab - data = { + data = json.dumps({ 'parent_location': 'i4x://mitX/333/course/Dummy_Course', - 'template': 'i4x://edx/templates/static_tab/Empty' - } + 'category': 'static_tab' + }) - resp = self.client.post(reverse('clone_item'), data) + resp = self.client.post(reverse('create_item'), data, + content_type="application/json") self.assertEqual(resp.status_code, 200) # Now delete it. There was a bug that the delete was failing (static tabs do not exist in draft modulestore). resp = self.client.post(reverse('delete_item'), resp.content, "application/json") self.assertEqual(resp.status_code, 200) + + +class TestCreateItem(CourseTestCase): + """ + Test the create_item handler thoroughly + """ + def response_id(self, response): + """ + Get the id from the response payload + :param response: + """ + parsed = json.loads(response.content) + return parsed['id'] + + def test_create_nicely(self): + """ + Try the straightforward use cases + """ + # create a chapter + display_name = 'Nicely created' + resp = self.client.post( + reverse('create_item'), + json.dumps({ + 'parent_location': self.course_location.url(), + 'display_name': display_name, + 'category': 'chapter' + }), + content_type="application/json" + ) + self.assertEqual(resp.status_code, 200) + + # get the new item and check its category and display_name + chap_location = self.response_id(resp) + new_obj = modulestore().get_item(chap_location) + self.assertEqual(new_obj.category, 'chapter') + self.assertEqual(new_obj.display_name, display_name) + self.assertEqual(new_obj.location.org, self.course_location.org) + self.assertEqual(new_obj.location.course, self.course_location.course) + + # get the course and ensure it now points to this one + course = modulestore().get_item(self.course_location) + self.assertIn(chap_location, course.children) + + # use default display name + resp = self.client.post( + reverse('create_item'), + json.dumps({ + 'parent_location': chap_location, + 'category': 'vertical' + }), + content_type="application/json" + ) + self.assertEqual(resp.status_code, 200) + + vert_location = self.response_id(resp) + + # create problem w/ boilerplate + template_id = 'multiplechoice.yaml' + resp = self.client.post( + reverse('create_item'), + json.dumps({ + 'parent_location': vert_location, + 'category': 'problem', + 'boilerplate': template_id + }), + content_type="application/json" + ) + self.assertEqual(resp.status_code, 200) + prob_location = self.response_id(resp) + problem = modulestore('draft').get_item(prob_location) + # ensure it's draft + self.assertTrue(problem.is_draft) + # check against the template + template = CapaDescriptor.get_template(template_id) + self.assertEqual(problem.data, template['data']) + self.assertEqual(problem.display_name, template['metadata']['display_name']) + self.assertEqual(problem.markdown, template['metadata']['markdown']) + + def test_create_item_negative(self): + """ + Negative tests for create_item + """ + # non-existent boilerplate: creates a default + resp = self.client.post( + reverse('create_item'), + json.dumps( + {'parent_location': self.course_location.url(), + 'category': 'problem', + 'boilerplate': 'nosuchboilerplate.yaml' + }), + content_type="application/json" + ) + self.assertEqual(resp.status_code, 200) + +class TestEditItem(CourseTestCase): + """ + Test contentstore.views.item.save_item + """ + def response_id(self, response): + """ + Get the id from the response payload + :param response: + """ + parsed = json.loads(response.content) + return parsed['id'] + + def setUp(self): + """ Creates the test course structure and a couple problems to 'edit'. """ + super(TestEditItem, self).setUp() + # create a chapter + display_name = 'chapter created' + resp = self.client.post( + reverse('create_item'), + json.dumps( + {'parent_location': self.course_location.url(), + 'display_name': display_name, + 'category': 'chapter' + }), + content_type="application/json" + ) + chap_location = self.response_id(resp) + resp = self.client.post( + reverse('create_item'), + json.dumps( + {'parent_location': chap_location, + 'category': 'vertical' + }), + content_type="application/json" + ) + vert_location = self.response_id(resp) + # create problem w/ boilerplate + template_id = 'multiplechoice.yaml' + resp = self.client.post( + reverse('create_item'), + json.dumps({'parent_location': vert_location, + 'category': 'problem', + 'boilerplate': template_id + }), + content_type="application/json" + ) + self.problems = [self.response_id(resp)] + + def test_delete_field(self): + """ + Sending null in for a field 'deletes' it + """ + self.client.post( + reverse('save_item'), + json.dumps({ + 'id': self.problems[0], + 'metadata': {'rerandomize': 'onreset'} + }), + content_type="application/json" + ) + problem = modulestore('draft').get_item(self.problems[0]) + self.assertEqual(problem.rerandomize, 'onreset') + self.client.post( + reverse('save_item'), + json.dumps({ + 'id': self.problems[0], + 'metadata': {'rerandomize': None} + }), + content_type="application/json" + ) + problem = modulestore('draft').get_item(self.problems[0]) + self.assertEqual(problem.rerandomize, 'never') + + + def test_null_field(self): + """ + Sending null in for a field 'deletes' it + """ + problem = modulestore('draft').get_item(self.problems[0]) + self.assertIsNotNone(problem.markdown) + self.client.post( + reverse('save_item'), + json.dumps({ + 'id': self.problems[0], + 'nullout': ['markdown'] + }), + content_type="application/json" + ) + problem = modulestore('draft').get_item(self.problems[0]) + self.assertIsNo/ne(problem.markdown) diff --git a/cms/djangoapps/contentstore/tests/utils.py b/cms/djangoapps/contentstore/tests/utils.py index bc9e9e8bae8..a3f211a703d 100644 --- a/cms/djangoapps/contentstore/tests/utils.py +++ b/cms/djangoapps/contentstore/tests/utils.py @@ -54,7 +54,6 @@ def setUp(self): self.client.login(username=uname, password=password) self.course = CourseFactory.create( - template='i4x://edx/templates/course/Empty', org='MITx', number='999', display_name='Robot Super Course', diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 5fa0d949b08..4973bddaca0 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -19,14 +19,14 @@ EXTRA_TAB_PANELS = dict([(p['type'], p) for p in [OPEN_ENDED_PANEL, NOTES_PANEL]]) -def get_modulestore(location): +def get_modulestore(category_or_location): """ Returns the correct modulestore to use for modifying the specified location """ - if not isinstance(location, Location): - location = Location(location) + if isinstance(category_or_location, Location): + category_or_location = category_or_location.category - if location.category in DIRECT_ONLY_CATEGORIES: + if category_or_location in DIRECT_ONLY_CATEGORIES: return modulestore('direct') else: return modulestore() diff --git a/cms/djangoapps/contentstore/views/checklist.py b/cms/djangoapps/contentstore/views/checklist.py index fa0a7b7b62f..fdb5857ba76 100644 --- a/cms/djangoapps/contentstore/views/checklist.py +++ b/cms/djangoapps/contentstore/views/checklist.py @@ -7,11 +7,11 @@ from django_future.csrf import ensure_csrf_cookie from mitxmako.shortcuts import render_to_response -from xmodule.modulestore import Location from xmodule.modulestore.inheritance import own_metadata from ..utils import get_modulestore, get_url_reverse from .access import get_location_and_verify_access +from xmodule.course_module import CourseDescriptor __all__ = ['get_checklists', 'update_checklist'] @@ -28,13 +28,11 @@ def get_checklists(request, org, course, name): modulestore = get_modulestore(location) course_module = modulestore.get_item(location) - new_course_template = Location('i4x', 'edx', 'templates', 'course', 'Empty') - template_module = modulestore.get_item(new_course_template) # If course was created before checklists were introduced, copy them over from the template. copied = False if not course_module.checklists: - course_module.checklists = template_module.checklists + course_module.checklists = CourseDescriptor.checklists.default copied = True checklists, modified = expand_checklist_action_urls(course_module) diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py index 30958d58663..13eca522dda 100644 --- a/cms/djangoapps/contentstore/views/component.py +++ b/cms/djangoapps/contentstore/views/component.py @@ -26,6 +26,8 @@ from .requests import _xmodule_recurse from .access import has_access +from xmodule.x_module import XModuleDescriptor +from xblock.plugin import PluginMissingError __all__ = ['OPEN_ENDED_COMPONENT_TYPES', 'ADVANCED_COMPONENT_POLICY_KEY', @@ -101,7 +103,7 @@ def edit_subsection(request, location): return render_to_response('edit_subsection.html', {'subsection': item, 'context_course': course, - 'create_new_unit_template': Location('i4x', 'edx', 'templates', 'vertical', 'Empty'), + 'new_unit_category': 'vertical', 'lms_link': lms_link, 'preview_link': preview_link, 'course_graders': json.dumps(CourseGradingModel.fetch(course.location).graders), @@ -134,10 +136,27 @@ def edit_unit(request, location): item = modulestore().get_item(location, depth=1) except ItemNotFoundError: return HttpResponseBadRequest() - lms_link = get_lms_link_for_item(item.location, course_id=course.location.course_id) component_templates = defaultdict(list) + for category in COMPONENT_TYPES: + component_class = XModuleDescriptor.load_class(category) + # add the default template + has_markdown = hasattr(component_class, 'markdown') and component_class.markdown is not None + component_templates[category].append(( + component_class.display_name.default or 'Blank', + category, + has_markdown, + None # no boilerplate for overrides + )) + # add boilerplates + for template in component_class.templates(): + component_templates[category].append(( + template['metadata'].get('display_name'), + category, + template['metadata'].get('markdown') is not None, + template.get('template_id') + )) # Check if there are any advanced modules specified in the course policy. These modules # should be specified as a list of strings, where the strings are the names of the modules @@ -145,29 +164,29 @@ def edit_unit(request, location): course_advanced_keys = course.advanced_modules # Set component types according to course policy file - component_types = list(COMPONENT_TYPES) if isinstance(course_advanced_keys, list): - course_advanced_keys = [c for c in course_advanced_keys if c in ADVANCED_COMPONENT_TYPES] - if len(course_advanced_keys) > 0: - component_types.append(ADVANCED_COMPONENT_CATEGORY) + for category in course_advanced_keys: + if category in ADVANCED_COMPONENT_TYPES: + # Do I need to allow for boilerplates or just defaults on the class? i.e., can an advanced + # have more than one entry in the menu? one for default and others for prefilled boilerplates? + try: + component_class = XModuleDescriptor.load_class(category) + + component_templates['advanced'].append(( + component_class.display_name.default or category, + category, + hasattr(component_class, 'markdown') and component_class.markdown is not None, + None # don't override default data + )) + except PluginMissingError: + # dhm: I got this once but it can happen any time the course author configures + # an advanced component which does not exist on the server. This code here merely + # prevents any authors from trying to instantiate the non-existent component type + # by not showing it in the menu + pass else: log.error("Improper format for course advanced keys! {0}".format(course_advanced_keys)) - templates = modulestore().get_items(Location('i4x', 'edx', 'templates')) - for template in templates: - category = template.location.category - - if category in course_advanced_keys: - category = ADVANCED_COMPONENT_CATEGORY - - if category in component_types: - # This is a hack to create categories for different xmodules - component_templates[category].append(( - template.display_name_with_default, - template.location.url(), - hasattr(template, 'markdown') and template.markdown is not None - )) - components = [ component.location.url() for component @@ -219,7 +238,7 @@ def edit_unit(request, location): 'subsection': containing_subsection, 'release_date': get_default_time_display(containing_subsection.lms.start) if containing_subsection.lms.start is not None else None, 'section': containing_section, - 'create_new_unit_template': Location('i4x', 'edx', 'templates', 'vertical', 'Empty'), + 'new_unit_category': 'vertical', 'unit_state': unit_state, 'published_date': get_default_time_display(item.cms.published_date) if item.cms.published_date is not None else None }) @@ -253,7 +272,7 @@ def create_draft(request): # This clones the existing item location to a draft location (the draft is implicit, # because modulestore is a Draft modulestore) - modulestore().clone_item(location, location) + modulestore().convert_to_draft(location) return HttpResponse() diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index f8de053d958..4c95d6e06e9 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -82,10 +82,11 @@ def course_index(request, org, course, name): 'sections': sections, 'course_graders': json.dumps(CourseGradingModel.fetch(course.location).graders), 'parent_location': course.location, - 'new_section_template': Location('i4x', 'edx', 'templates', 'chapter', 'Empty'), - 'new_subsection_template': Location('i4x', 'edx', 'templates', 'sequential', 'Empty'), # for now they are the same, but the could be different at some point... + 'new_section_category': 'chapter', + 'new_subsection_category': 'sequential', 'upload_asset_callback_url': upload_asset_callback_url, - 'create_new_unit_template': Location('i4x', 'edx', 'templates', 'vertical', 'Empty') + 'new_unit_category': 'vertical', + 'category': 'vertical' }) @@ -98,12 +99,6 @@ def create_new_course(request): if not is_user_in_creator_group(request.user): raise PermissionDenied() - # This logic is repeated in xmodule/modulestore/tests/factories.py - # so if you change anything here, you need to also change it there. - # TODO: write a test that creates two courses, one with the factory and - # the other with this method, then compare them to make sure they are - # equivalent. - template = Location(request.POST['template']) org = request.POST.get('org') number = request.POST.get('number') display_name = request.POST.get('display_name') @@ -121,29 +116,26 @@ def create_new_course(request): existing_course = modulestore('direct').get_item(dest_location) except ItemNotFoundError: pass - if existing_course is not None: return JsonResponse({'ErrMsg': 'There is already a course defined with this name.'}) course_search_location = ['i4x', dest_location.org, dest_location.course, 'course', None] courses = modulestore().get_items(course_search_location) - if len(courses) > 0: return JsonResponse({'ErrMsg': 'There is already a course defined with the same organization and course number.'}) - new_course = modulestore('direct').clone_item(template, dest_location) + # instantiate the CourseDescriptor and then persist it + # note: no system to pass + if display_name is None: + metadata = {} + else: + metadata = {'display_name': display_name} + modulestore('direct').create_and_save_xmodule(dest_location, metadata=metadata) + new_course = modulestore('direct').get_item(dest_location) # clone a default 'about' module as well - - about_template_location = Location(['i4x', 'edx', 'templates', 'about', 'overview']) dest_about_location = dest_location._replace(category='about', name='overview') - modulestore('direct').clone_item(about_template_location, dest_about_location) - - if display_name is not None: - new_course.display_name = display_name - - # set a default start date to now - new_course.start = datetime.datetime.now(UTC()) + modulestore('direct').create_and_save_xmodule(dest_about_location, system=new_course.system) initialize_course_tabs(new_course) diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index abc5f48564e..90dae10c238 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -13,16 +13,26 @@ from ..utils import get_modulestore from .access import has_access from .requests import _xmodule_recurse +from xmodule.x_module import XModuleDescriptor -__all__ = ['save_item', 'clone_item', 'delete_item'] +__all__ = ['save_item', 'create_item', 'delete_item'] # cdodge: these are categories which should not be parented, they are detached from the hierarchy DETACHED_CATEGORIES = ['about', 'static_tab', 'course_info'] - @login_required @expect_json def save_item(request): + """ + Will carry a json payload with these possible fields + :id (required): the id + :data (optional): the new value for the data + :metadata (optional): new values for the metadata fields. + Any whose values are None will be deleted not set to None! Absent ones will be left alone + :nullout (optional): which metadata fields to set to None + """ + # The nullout is a bit of a temporary copout until we can make module_edit.coffee and the metadata editors a + # little smarter and able to pass something more akin to {unset: [field, field]} item_location = request.POST['id'] # check permissions for this user within this course @@ -42,30 +52,25 @@ def save_item(request): children = request.POST['children'] store.update_children(item_location, children) - # cdodge: also commit any metadata which might have been passed along in the - # POST from the client, if it is there - # NOTE, that the postback is not the complete metadata, as there's system metadata which is - # not presented to the end-user for editing. So let's fetch the original and - # 'apply' the submitted metadata, so we don't end up deleting system metadata - if request.POST.get('metadata') is not None: - posted_metadata = request.POST['metadata'] - # fetch original + # cdodge: also commit any metadata which might have been passed along + if request.POST.get('nullout') is not None or request.POST.get('metadata') is not None: + # the postback is not the complete metadata, as there's system metadata which is + # not presented to the end-user for editing. So let's fetch the original and + # 'apply' the submitted metadata, so we don't end up deleting system metadata existing_item = modulestore().get_item(item_location) + for metadata_key in request.POST.get('nullout', []): + setattr(existing_item, metadata_key, None) # update existing metadata with submitted metadata (which can be partial) - # IMPORTANT NOTE: if the client passed pack 'null' (None) for a piece of metadata that means 'remove it' - for metadata_key, value in posted_metadata.items(): - - if posted_metadata[metadata_key] is None: - # remove both from passed in collection as well as the collection read in from the modulestore - if metadata_key in existing_item._model_data: - del existing_item._model_data[metadata_key] - del posted_metadata[metadata_key] - else: - existing_item._model_data[metadata_key] = value + # IMPORTANT NOTE: if the client passed 'null' (None) for a piece of metadata that means 'remove it'. If + # the intent is to make it None, use the nullout field + for metadata_key, value in request.POST.get('metadata', {}).items(): + if value is None: + delattr(existing_item, metadata_key) + else: + setattr(existing_item, metadata_key, value) # commit to datastore - # TODO (cpennington): This really shouldn't have to do this much reaching in to get the metadata store.update_metadata(item_location, own_metadata(existing_item)) return HttpResponse() @@ -73,28 +78,38 @@ def save_item(request): @login_required @expect_json -def clone_item(request): +def create_item(request): parent_location = Location(request.POST['parent_location']) - template = Location(request.POST['template']) + category = request.POST['category'] display_name = request.POST.get('display_name') if not has_access(request.user, parent_location): raise PermissionDenied() - parent = get_modulestore(template).get_item(parent_location) - dest_location = parent_location._replace(category=template.category, name=uuid4().hex) - - new_item = get_modulestore(template).clone_item(template, dest_location) + parent = get_modulestore(category).get_item(parent_location) + dest_location = parent_location.replace(category=category, name=uuid4().hex) + + # get the metadata, display_name, and definition from the request + metadata = {} + data = None + template_id = request.POST.get('boilerplate') + if template_id is not None: + clz = XModuleDescriptor.load_class(category) + if clz is not None: + template = clz.get_template(template_id) + if template is not None: + metadata = template.get('metadata', {}) + data = template.get('data') - # replace the display name with an optional parameter passed in from the caller if display_name is not None: - new_item.display_name = display_name + metadata['display_name'] = display_name - get_modulestore(template).update_metadata(new_item.location.url(), own_metadata(new_item)) + get_modulestore(category).create_and_save_xmodule(dest_location, definition_data=data, + metadata=metadata, system=parent.system) - if new_item.location.category not in DETACHED_CATEGORIES: - get_modulestore(parent.location).update_children(parent_location, parent.children + [new_item.location.url()]) + if category not in DETACHED_CATEGORIES: + get_modulestore(parent.location).update_children(parent_location, parent.children + [dest_location.url()]) return HttpResponse(json.dumps({'id': dest_location.url()})) diff --git a/cms/djangoapps/contentstore/views/user.py b/cms/djangoapps/contentstore/views/user.py index 948ed614d2c..ee6b0bf84d2 100644 --- a/cms/djangoapps/contentstore/views/user.py +++ b/cms/djangoapps/contentstore/views/user.py @@ -27,6 +27,7 @@ def index(request): # filter out courses that we don't have access too def course_filter(course): return (has_access(request.user, course.location) + # TODO remove this condition when templates purged from db and course.location.course != 'templates' and course.location.org != '' and course.location.course != '' @@ -34,7 +35,6 @@ def course_filter(course): courses = filter(course_filter, courses) return render_to_response('index.html', { - 'new_course_template': Location('i4x', 'edx', 'templates', 'course', 'Empty'), 'courses': [(course.display_name, get_url_reverse('CourseOutline', course), get_lms_link_for_item(course.location, course_id=course.location.course_id)) diff --git a/cms/djangoapps/models/settings/course_grading.py b/cms/djangoapps/models/settings/course_grading.py index 4ea9f2f5db2..e529a284c6b 100644 --- a/cms/djangoapps/models/settings/course_grading.py +++ b/cms/djangoapps/models/settings/course_grading.py @@ -9,7 +9,7 @@ class CourseGradingModel(object): """ def __init__(self, course_descriptor): self.course_location = course_descriptor.location - self.graders = [CourseGradingModel.jsonize_grader(i, grader) for i, grader in enumerate(course_descriptor.raw_grader)] # weights transformed to ints [0..100] + self.graders = [CourseGradingModel.jsonize_grader(i, grader) for i, grader in enumerate(course_descriptor.raw_grader)] # weights transformed to ints [0..100] self.grade_cutoffs = course_descriptor.grade_cutoffs self.grace_period = CourseGradingModel.convert_set_grace_period(course_descriptor) @@ -81,7 +81,7 @@ def update_from_json(jsondict): Decode the json into CourseGradingModel and save any changes. Returns the modified model. Probably not the usual path for updates as it's too coarse grained. """ - course_location = jsondict['course_location'] + course_location = Location(jsondict['course_location']) descriptor = get_modulestore(course_location).get_item(course_location) graders_parsed = [CourseGradingModel.parse_grader(jsonele) for jsonele in jsondict['graders']] @@ -89,7 +89,7 @@ def update_from_json(jsondict): descriptor.raw_grader = graders_parsed descriptor.grade_cutoffs = jsondict['grade_cutoffs'] - get_modulestore(course_location).update_item(course_location, descriptor._model_data._kvs._data) + get_modulestore(course_location).update_item(course_location, descriptor.xblock_kvs._data) CourseGradingModel.update_grace_period_from_json(course_location, jsondict['grace_period']) return CourseGradingModel.fetch(course_location) @@ -209,7 +209,7 @@ def get_section_grader_type(location): descriptor = get_modulestore(location).get_item(location) return {"graderType": descriptor.lms.format if descriptor.lms.format is not None else 'Not Graded', "location": location, - "id": 99 # just an arbitrary value to + "id": 99 # just an arbitrary value to } @staticmethod @@ -232,7 +232,7 @@ def convert_set_grace_period(descriptor): # 5 hours 59 minutes 59 seconds => converted to iso format rawgrace = descriptor.lms.graceperiod if rawgrace: - hours_from_days = rawgrace.days*24 + hours_from_days = rawgrace.days * 24 seconds = rawgrace.seconds hours_from_seconds = int(seconds / 3600) hours = hours_from_days + hours_from_seconds diff --git a/cms/static/coffee/src/views/module_edit.coffee b/cms/static/coffee/src/views/module_edit.coffee index 5154591d6fd..62083fa26d6 100644 --- a/cms/static/coffee/src/views/module_edit.coffee +++ b/cms/static/coffee/src/views/module_edit.coffee @@ -56,14 +56,15 @@ class CMS.Views.ModuleEdit extends Backbone.View changedMetadata: -> return _.extend(@metadataEditor.getModifiedMetadataValues(), @customMetadata()) - cloneTemplate: (parent, template) -> - $.post("/clone_item", { - parent_location: parent - template: template - }, (data) => - @model.set(id: data.id) - @$el.data('id', data.id) - @render() + createItem: (parent, payload) -> + payload.parent_location = parent + $.post( + "/create_item" + payload + (data) => + @model.set(id: data.id) + @$el.data('id', data.id) + @render() ) render: -> diff --git a/cms/static/coffee/src/views/tabs.coffee b/cms/static/coffee/src/views/tabs.coffee index 1034fc988e6..58f52f27a3a 100644 --- a/cms/static/coffee/src/views/tabs.coffee +++ b/cms/static/coffee/src/views/tabs.coffee @@ -55,9 +55,9 @@ class CMS.Views.TabsEdit extends Backbone.View editor.$el.removeClass('new') , 500) - editor.cloneTemplate( + editor.createItem( @model.get('id'), - 'i4x://edx/templates/static_tab/Empty' + {category: 'static_tab'} ) analytics.track "Added Static Page", diff --git a/cms/static/coffee/src/views/unit.coffee b/cms/static/coffee/src/views/unit.coffee index 058bcf0ce1d..774ef04f6d0 100644 --- a/cms/static/coffee/src/views/unit.coffee +++ b/cms/static/coffee/src/views/unit.coffee @@ -89,9 +89,9 @@ class CMS.Views.UnitEdit extends Backbone.View @$newComponentItem.before(editor.$el) - editor.cloneTemplate( + editor.createItem( @$el.data('id'), - $(event.currentTarget).data('location') + $(event.currentTarget).data() ) analytics.track "Added a Component", diff --git a/cms/static/js/base.js b/cms/static/js/base.js index b53d116808b..3d8cd7684e7 100644 --- a/cms/static/js/base.js +++ b/cms/static/js/base.js @@ -338,7 +338,7 @@ function createNewUnit(e) { e.preventDefault(); var parent = $(this).data('parent'); - var template = $(this).data('template'); + var category = $(this).data('category'); analytics.track('Created a Unit', { 'course': course_location_analytics, @@ -346,9 +346,9 @@ function createNewUnit(e) { }); - $.post('/clone_item', { + $.post('/create_item', { 'parent_location': parent, - 'template': template, + 'category': category, 'display_name': 'New Unit' }, @@ -551,7 +551,7 @@ function saveNewSection(e) { var $saveButton = $(this).find('.new-section-name-save'); var parent = $saveButton.data('parent'); - var template = $saveButton.data('template'); + var category = $saveButton.data('category'); var display_name = $(this).find('.new-section-name').val(); analytics.track('Created a Section', { @@ -559,9 +559,9 @@ function saveNewSection(e) { 'display_name': display_name }); - $.post('/clone_item', { + $.post('/create_item', { 'parent_location': parent, - 'template': template, + 'category': category, 'display_name': display_name, }, @@ -595,7 +595,6 @@ function saveNewCourse(e) { e.preventDefault(); var $newCourse = $(this).closest('.new-course'); - var template = $(this).find('.new-course-save').data('template'); var org = $newCourse.find('.new-course-org').val(); var number = $newCourse.find('.new-course-number').val(); var display_name = $newCourse.find('.new-course-name').val(); @@ -612,7 +611,6 @@ function saveNewCourse(e) { }); $.post('/create_new_course', { - 'template': template, 'org': org, 'number': number, 'display_name': display_name @@ -646,7 +644,7 @@ function addNewSubsection(e) { var parent = $(this).parents("section.branch").data("id"); $saveButton.data('parent', parent); - $saveButton.data('template', $(this).data('template')); + $saveButton.data('category', $(this).data('category')); $newSubsection.find('.new-subsection-form').bind('submit', saveNewSubsection); $cancelButton.bind('click', cancelNewSubsection); @@ -659,7 +657,7 @@ function saveNewSubsection(e) { e.preventDefault(); var parent = $(this).find('.new-subsection-name-save').data('parent'); - var template = $(this).find('.new-subsection-name-save').data('template'); + var category = $(this).find('.new-subsection-name-save').data('category'); var display_name = $(this).find('.new-subsection-name-input').val(); analytics.track('Created a Subsection', { @@ -668,9 +666,9 @@ function saveNewSubsection(e) { }); - $.post('/clone_item', { + $.post('/create_item', { 'parent_location': parent, - 'template': template, + 'category': category, 'display_name': display_name }, diff --git a/cms/templates/index.html b/cms/templates/index.html index 57921641ce5..f0baef4f095 100644 --- a/cms/templates/index.html +++ b/cms/templates/index.html @@ -25,8 +25,8 @@
- - + +
diff --git a/cms/templates/overview.html b/cms/templates/overview.html index ab7788c64a3..56836b00ad3 100644 --- a/cms/templates/overview.html +++ b/cms/templates/overview.html @@ -1,4 +1,4 @@ -<%! from django.utils.translation import ugettext as _ %> +/<%! from django.utils.translation import ugettext as _ %> <%inherit file="base.html" /> <%! import logging @@ -66,7 +66,8 @@

- +

@@ -83,8 +84,9 @@

Click here to set the section name
- -

+ +
@@ -181,7 +183,7 @@

- + ${_("New Subsection")}
diff --git a/cms/templates/unit.html b/cms/templates/unit.html index ce4f239ab11..300d6314213 100644 --- a/cms/templates/unit.html +++ b/cms/templates/unit.html @@ -59,8 +59,8 @@
${_("Add New Component")}
% if type == 'advanced' or len(templates) > 1: % else: - % for __, location, __ in templates: - + % for __, category, __, __ in templates: + % endfor % endif @@ -74,49 +74,60 @@
${_("Add New Component")}
% if len(templates) > 1 or type == 'advanced':
% if type == "problem": -
- - % endif -
-
- % endif - ${_("Cancel")} -
- % endif + % endif +
+
    + % for name, category, has_markdown, boilerplate_name in sorted(templates): + % if has_markdown or type != "problem": + % if boilerplate_name is None: +
  • + + ${name} + +
    • + + % else: +
  • + + ${name} + +
    • + % endif + % endif + + %endfor +
+
+ % if type == "problem": +
+
    + % for name, category, has_markdown, boilerplate_name in sorted(templates): + % if not has_markdown: +
  • + + ${name} + +
    • + % endif + % endfor +
+
+
+ % endif + Cancel +
+ % endif % endfor diff --git a/cms/templates/widgets/units.html b/cms/templates/widgets/units.html index 5ac05e79eb6..62c1fb62d7c 100644 --- a/cms/templates/widgets/units.html +++ b/cms/templates/widgets/units.html @@ -34,7 +34,7 @@ % endfor
  • - + New Unit
    • diff --git a/cms/urls.py b/cms/urls.py index 6db07dc2edc..711742d89f3 100644 --- a/cms/urls.py +++ b/cms/urls.py @@ -17,7 +17,7 @@ url(r'^preview_component/(?P.*?)$', 'contentstore.views.preview_component', name='preview_component'), url(r'^save_item$', 'contentstore.views.save_item', name='save_item'), url(r'^delete_item$', 'contentstore.views.delete_item', name='delete_item'), - url(r'^clone_item$', 'contentstore.views.clone_item', name='clone_item'), + url(r'^create_item$', 'contentstore.views.create_item', name='create_item'), url(r'^create_draft$', 'contentstore.views.create_draft', name='create_draft'), url(r'^publish_draft$', 'contentstore.views.publish_draft', name='publish_draft'), url(r'^unpublish_unit$', 'contentstore.views.unpublish_unit', name='unpublish_unit'), diff --git a/common/djangoapps/terrain/course_helpers.py b/common/djangoapps/terrain/course_helpers.py index 27bf95099d9..c62b1a1e79a 100644 --- a/common/djangoapps/terrain/course_helpers.py +++ b/common/djangoapps/terrain/course_helpers.py @@ -12,7 +12,6 @@ from student.models import CourseEnrollment from xmodule.modulestore.django import modulestore from xmodule.contentstore.django import contentstore -from xmodule.templates import update_templates from urllib import quote_plus @@ -84,5 +83,4 @@ def clear_courses(): # from the bash shell to drop it: # $ mongo test_xmodule --eval "db.dropDatabase()" modulestore().collection.drop() - update_templates(modulestore('direct')) contentstore().fs_files.drop() diff --git a/common/djangoapps/tests.py b/common/djangoapps/tests.py index 8e78ee7f377..4a61a106d44 100644 --- a/common/djangoapps/tests.py +++ b/common/djangoapps/tests.py @@ -23,15 +23,15 @@ def _test_add_histogram(self): number='313', display_name='histogram test') section = ItemFactory.create( parent_location=course.location, display_name='chapter hist', - template='i4x://edx/templates/chapter/Empty') + category='chapter') problem = ItemFactory.create( parent_location=section.location, display_name='problem hist 1', - template='i4x://edx/templates/problem/Blank_Common_Problem') + category='problem') problem.has_score = False # don't trip trying to retrieve db data late_problem = ItemFactory.create( parent_location=section.location, display_name='problem hist 2', - template='i4x://edx/templates/problem/Blank_Common_Problem') + category='problem') late_problem.lms.start = datetime.datetime.now(UTC) + datetime.timedelta(days=32) late_problem.has_score = False diff --git a/common/lib/xmodule/xmodule/abtest_module.py b/common/lib/xmodule/xmodule/abtest_module.py index 2e61076e941..53f080eb3a8 100644 --- a/common/lib/xmodule/xmodule/abtest_module.py +++ b/common/lib/xmodule/xmodule/abtest_module.py @@ -80,8 +80,6 @@ def displayable_items(self): class ABTestDescriptor(ABTestFields, RawDescriptor, XmlDescriptor): module_class = ABTestModule - template_dir_name = "abtest" - @classmethod def definition_from_xml(cls, xml_object, system): """ diff --git a/common/lib/xmodule/xmodule/annotatable_module.py b/common/lib/xmodule/xmodule/annotatable_module.py index 8b1bbc71d3e..f80e3e488e6 100644 --- a/common/lib/xmodule/xmodule/annotatable_module.py +++ b/common/lib/xmodule/xmodule/annotatable_module.py @@ -150,5 +150,4 @@ def get_html(self): class AnnotatableDescriptor(AnnotatableFields, RawDescriptor): module_class = AnnotatableModule - template_dir_name = "annotatable" mako_template = "widgets/raw-edit.html" diff --git a/common/lib/xmodule/xmodule/combined_open_ended_module.py b/common/lib/xmodule/xmodule/combined_open_ended_module.py index 13f00bb77ef..60103a21267 100644 --- a/common/lib/xmodule/xmodule/combined_open_ended_module.py +++ b/common/lib/xmodule/xmodule/combined_open_ended_module.py @@ -290,7 +290,6 @@ class CombinedOpenEndedDescriptor(CombinedOpenEndedFields, RawDescriptor): has_score = True always_recalculate_grades = True - template_dir_name = "combinedopenended" #Specify whether or not to pass in S3 interface needs_s3_interface = True diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index ceadee1c155..d4aac5b0ae6 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -366,8 +366,6 @@ class CourseFields(object): class CourseDescriptor(CourseFields, SequenceDescriptor): module_class = SequenceModule - template_dir_name = 'course' - def __init__(self, *args, **kwargs): """ Expects the same arguments as XModuleDescriptor.__init__ diff --git a/common/lib/xmodule/xmodule/error_module.py b/common/lib/xmodule/xmodule/error_module.py index a37081c4479..db1f9981879 100644 --- a/common/lib/xmodule/xmodule/error_module.py +++ b/common/lib/xmodule/xmodule/error_module.py @@ -96,6 +96,7 @@ def _construct(cls, system, contents, error_msg, location): 'contents': contents, 'display_name': 'Error: ' + location.name, 'location': location, + 'category': 'error' } return cls( system, @@ -109,12 +110,12 @@ def get_context(self): } @classmethod - def from_json(cls, json_data, system, error_msg='Error not available'): + def from_json(cls, json_data, system, location, error_msg='Error not available'): return cls._construct( system, - json.dumps(json_data, indent=4), + json.dumps(json_data, skipkeys=False, indent=4), error_msg, - location=Location(json_data['location']), + location=location ) @classmethod diff --git a/common/lib/xmodule/xmodule/foldit_module.py b/common/lib/xmodule/xmodule/foldit_module.py index fdab14b58d7..c4e9e2d35c9 100644 --- a/common/lib/xmodule/xmodule/foldit_module.py +++ b/common/lib/xmodule/xmodule/foldit_module.py @@ -184,7 +184,6 @@ class FolditDescriptor(FolditFields, XmlDescriptor, EditingDescriptor): filename_extension = "xml" has_score = True - template_dir_name = "foldit" js = {'coffee': [resource_string(__name__, 'js/src/html/edit.coffee')]} js_module_name = "HTMLEditingDescriptor" diff --git a/common/lib/xmodule/xmodule/gst_module.py b/common/lib/xmodule/xmodule/gst_module.py index e101d90b4cd..5a4930ff959 100644 --- a/common/lib/xmodule/xmodule/gst_module.py +++ b/common/lib/xmodule/xmodule/gst_module.py @@ -141,7 +141,6 @@ def build_configuration_json(self): class GraphicalSliderToolDescriptor(GraphicalSliderToolFields, MakoModuleDescriptor, XmlDescriptor): module_class = GraphicalSliderToolModule - template_dir_name = 'graphical_slider_tool' @classmethod def definition_from_xml(cls, xml_object, system): diff --git a/common/lib/xmodule/xmodule/html_module.py b/common/lib/xmodule/xmodule/html_module.py index 9ff2e4d9a87..2c7c9e0b012 100644 --- a/common/lib/xmodule/xmodule/html_module.py +++ b/common/lib/xmodule/xmodule/html_module.py @@ -234,7 +234,7 @@ class StaticTabDescriptor(StaticTabFields, HtmlDescriptor): These pieces of course content are treated as HtmlModules but we need to overload where the templates are located in order to be able to create new ones """ - template_dir_name = "statictab" + template_dir_name = None module_class = StaticTabModule @@ -261,5 +261,5 @@ class CourseInfoDescriptor(CourseInfoFields, HtmlDescriptor): These pieces of course content are treated as HtmlModules but we need to overload where the templates are located in order to be able to create new ones """ - template_dir_name = "courseinfo" + template_dir_name = None module_class = CourseInfoModule diff --git a/common/lib/xmodule/xmodule/js/spec/combinedopenended/edit_spec.coffee b/common/lib/xmodule/xmodule/js/spec/combinedopenended/edit_spec.coffee index aa077da4508..d859a59ddae 100644 --- a/common/lib/xmodule/xmodule/js/spec/combinedopenended/edit_spec.coffee +++ b/common/lib/xmodule/xmodule/js/spec/combinedopenended/edit_spec.coffee @@ -11,13 +11,13 @@ describe 'OpenEndedMarkdownEditingDescriptor', -> @descriptor = new OpenEndedMarkdownEditingDescriptor($('.combinedopenended-editor')) @descriptor.createXMLEditor('replace with markdown') saveResult = @descriptor.save() - expect(saveResult.metadata.markdown).toEqual(null) + expect(saveResult.nullout).toEqual(['markdown']) expect(saveResult.data).toEqual('replace with markdown') it 'saves xml from the xml editor', -> loadFixtures 'combinedopenended-without-markdown.html' @descriptor = new OpenEndedMarkdownEditingDescriptor($('.combinedopenended-editor')) saveResult = @descriptor.save() - expect(saveResult.metadata.markdown).toEqual(null) + expect(saveResult.nullout).toEqual(['markdown']) expect(saveResult.data).toEqual('xml only') describe 'insertPrompt', -> diff --git a/common/lib/xmodule/xmodule/js/spec/problem/edit_spec.coffee b/common/lib/xmodule/xmodule/js/spec/problem/edit_spec.coffee index 5161e658e74..1df9587037f 100644 --- a/common/lib/xmodule/xmodule/js/spec/problem/edit_spec.coffee +++ b/common/lib/xmodule/xmodule/js/spec/problem/edit_spec.coffee @@ -11,13 +11,13 @@ describe 'MarkdownEditingDescriptor', -> @descriptor = new MarkdownEditingDescriptor($('.problem-editor')) @descriptor.createXMLEditor('replace with markdown') saveResult = @descriptor.save() - expect(saveResult.metadata.markdown).toEqual(null) + expect(saveResult.nullout).toEqual(['markdown']) expect(saveResult.data).toEqual('replace with markdown') it 'saves xml from the xml editor', -> loadFixtures 'problem-without-markdown.html' @descriptor = new MarkdownEditingDescriptor($('.problem-editor')) saveResult = @descriptor.save() - expect(saveResult.metadata.markdown).toEqual(null) + expect(saveResult.nullout).toEqual(['markdown']) expect(saveResult.data).toEqual('xml only') describe 'insertMultipleChoice', -> diff --git a/common/lib/xmodule/xmodule/js/src/combinedopenended/edit.coffee b/common/lib/xmodule/xmodule/js/src/combinedopenended/edit.coffee index 1b7f9bb4fbf..4cdd571c652 100644 --- a/common/lib/xmodule/xmodule/js/src/combinedopenended/edit.coffee +++ b/common/lib/xmodule/xmodule/js/src/combinedopenended/edit.coffee @@ -153,8 +153,7 @@ Write a persuasive essay to a newspaper reflecting your vies on censorship in li else { data: @xml_editor.getValue() - metadata: - markdown: null + nullout: ['markdown'] } @insertRubric: (selectedText) -> diff --git a/common/lib/xmodule/xmodule/js/src/problem/edit.coffee b/common/lib/xmodule/xmodule/js/src/problem/edit.coffee index b723f230e99..bd2871eb61f 100644 --- a/common/lib/xmodule/xmodule/js/src/problem/edit.coffee +++ b/common/lib/xmodule/xmodule/js/src/problem/edit.coffee @@ -123,9 +123,8 @@ class @MarkdownEditingDescriptor extends XModule.Descriptor } else { - data: @xml_editor.getValue() - metadata: - markdown: null + data: @xml_editor.getValue() + nullout: ['markdown'] } @insertMultipleChoice: (selectedText) -> diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index 2fa12e2e902..eb721dfc995 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -310,14 +310,7 @@ def get_items(self, location, course_id=None, depth=0): """ raise NotImplementedError - def clone_item(self, source, location): - """ - Clone a new item that is a copy of the item at the location `source` - and writes it to `location` - """ - raise NotImplementedError - - def update_item(self, location, data): + def update_item(self, location, data, allow_not_found=False): """ Set the data in the item specified by the location to data diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py index f56393d75e9..0b1c601a2ff 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py @@ -33,7 +33,7 @@ from xblock.core import Scope from xmodule.modulestore import ModuleStoreBase, Location, namedtuple_to_son -from xmodule.modulestore.exceptions import ItemNotFoundError, DuplicateItemError +from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.modulestore.inheritance import own_metadata, INHERITABLE_METADATA, inherit_metadata log = logging.getLogger(__name__) @@ -62,11 +62,12 @@ class MongoKeyValueStore(KeyValueStore): A KeyValueStore that maps keyed data access to one of the 3 data areas known to the MongoModuleStore (data, children, and metadata) """ - def __init__(self, data, children, metadata, location): + def __init__(self, data, children, metadata, location, category): self._data = data self._children = children self._metadata = metadata self._location = location + self._category = category def get(self, key): if key.scope == Scope.children: @@ -78,6 +79,8 @@ def get(self, key): elif key.scope == Scope.content: if key.field_name == 'location': return self._location + elif key.field_name == 'category': + return self._category elif key.field_name == 'data' and not isinstance(self._data, dict): return self._data else: @@ -93,6 +96,8 @@ def set(self, key, value): elif key.scope == Scope.content: if key.field_name == 'location': self._location = value + elif key.field_name == 'category': + self._category = value elif key.field_name == 'data' and not isinstance(self._data, dict): self._data = value else: @@ -109,6 +114,8 @@ def delete(self, key): elif key.scope == Scope.content: if key.field_name == 'location': self._location = Location(None) + elif key.field_name == 'category': + self._category = None elif key.field_name == 'data' and not isinstance(self._data, dict): self._data = None else: @@ -123,7 +130,10 @@ def has(self, key): return key.field_name in self._metadata elif key.scope == Scope.content: if key.field_name == 'location': + # WHY TRUE? if it's been deleted should it be False? return True + elif key.field_name == 'category': + return self._category is not None elif key.field_name == 'data' and not isinstance(self._data, dict): return True else: @@ -185,8 +195,9 @@ def load_item(self, location): else: # load the module and apply the inherited metadata try: + category = json_data['location']['category'] class_ = XModuleDescriptor.load_class( - json_data['location']['category'], + category, self.default_class ) definition = json_data.get('definition', {}) @@ -201,9 +212,12 @@ def load_item(self, location): definition.get('children', []), metadata, location, + category ) model_data = DbModel(kvs, class_, None, MongoUsage(self.course_id, location)) + model_data['category'] = category + model_data['location'] = location module = class_(self, model_data) if self.cached_metadata is not None: # parent container pointers don't differentiate between draft and non-draft @@ -217,6 +231,7 @@ def load_item(self, location): return ErrorDescriptor.from_json( json_data, self, + json_data['location'], error_msg=exc_info_to_str(sys.exc_info()) ) @@ -582,51 +597,97 @@ def get_items(self, location, course_id=None, depth=0): modules = self._load_items(list(items), depth) return modules - def clone_item(self, source, location): - """ - Clone a new item that is a copy of the item at the location `source` - and writes it to `location` - """ - item = None - try: - source_item = self.collection.find_one(location_to_query(source)) - - # allow for some programmatically generated substitutions in metadata, e.g. Discussion_id's should be auto-generated - for key in source_item['metadata'].keys(): - if source_item['metadata'][key] == '$$GUID$$': - source_item['metadata'][key] = uuid4().hex - - source_item['_id'] = Location(location).dict() - self.collection.insert( - source_item, - # Must include this to avoid the django debug toolbar (which defines the deprecated "safe=False") - # from overriding our default value set in the init method. - safe=self.collection.safe + def create_xmodule(self, location, definition_data=None, metadata=None, system=None): + """ + Create the new xmodule but don't save it. Returns the new module. + + :param location: a Location--must have a category + :param definition_data: can be empty. The initial definition_data for the kvs + :param metadata: can be empty, the initial metadata for the kvs + :param system: if you already have an xmodule from the course, the xmodule.system value + """ + if not isinstance(location, Location): + location = Location(location) + # differs from split mongo in that I believe most of this logic should be above the persistence + # layer but added it here to enable quick conversion. I'll need to reconcile these. + if metadata is None: + metadata = {} + if system is None: + system = CachingDescriptorSystem( + self, + {}, + self.default_class, + None, + self.error_tracker, + self.render_template, + {} ) - item = self._load_items([source_item])[0] - - # VS[compat] cdodge: This is a hack because static_tabs also have references from the course module, so - # if we add one then we need to also add it to the policy information (i.e. metadata) - # we should remove this once we can break this reference from the course to static tabs - if location.category == 'static_tab': - course = self.get_course_for_item(item.location) - existing_tabs = course.tabs or [] - existing_tabs.append({ - 'type': 'static_tab', - 'name': item.display_name, - 'url_slug': item.location.name - }) - course.tabs = existing_tabs - self.update_metadata(course.location, course._model_data._kvs._metadata) - - except pymongo.errors.DuplicateKeyError: - raise DuplicateItemError(location) - + xblock_class = XModuleDescriptor.load_class(location.category, self.default_class) + if definition_data is None: + if hasattr(xblock_class, 'data') and getattr(xblock_class, 'data').default is not None: + definition_data = getattr(xblock_class, 'data').default + else: + definition_data = {} + dbmodel = self._create_new_model_data(location.category, location, definition_data, metadata) + xmodule = xblock_class(system, dbmodel) + # force inherited fields w/ defaults to take the defaults so the children can inherit + for attr in INHERITABLE_METADATA: + if hasattr(xmodule, attr): + xmodule._model_data[attr] = getattr(xmodule, attr) + return xmodule + + def save_xmodule(self, xmodule): + """ + Save the given xmodule (will either create or update based on whether id already exists). + Pulls out the data definition v metadata v children locally but saves it all. + + :param xmodule: + """ + # split mongo's persist_dag is more general and useful. + self.collection.save({ + '_id': xmodule.location.dict(), + 'metadata': own_metadata(xmodule), + 'definition': { + 'data': xmodule.xblock_kvs._data, + 'children': xmodule.children if xmodule.has_children else [] + } + }) # recompute (and update) the metadata inheritance tree which is cached - self.refresh_cached_metadata_inheritance_tree(Location(location)) - self.fire_updated_modulestore_signal(get_course_id_no_run(Location(location)), Location(location)) + self.refresh_cached_metadata_inheritance_tree(xmodule.location) + self.fire_updated_modulestore_signal(get_course_id_no_run(xmodule.location), xmodule.location) - return item + def create_and_save_xmodule(self, location, definition_data=None, metadata=None, system=None): + """ + Create the new xmodule and save it. Does not return the new module because if the caller + will insert it as a child, it's inherited metadata will completely change. The difference + between this and just doing create_xmodule and save_xmodule is this ensures static_tabs get + pointed to by the course. + + :param location: a Location--must have a category + :param definition_data: can be empty. The initial definition_data for the kvs + :param metadata: can be empty, the initial metadata for the kvs + :param system: if you already have an xmodule from the course, the xmodule.system value + """ + # differs from split mongo in that I believe most of this logic should be above the persistence + # layer but added it here to enable quick conversion. I'll need to reconcile these. + new_object = self.create_xmodule(location, definition_data, metadata, system) + location = new_object.location + self.save_xmodule(new_object) + + # VS[compat] cdodge: This is a hack because static_tabs also have references from the course module, so + # if we add one then we need to also add it to the policy information (i.e. metadata) + # we should remove this once we can break this reference from the course to static tabs + # TODO move this special casing to app tier (similar to attaching new element to parent) + if location.category == 'static_tab': + course = self.get_course_for_item(location) + existing_tabs = course.tabs or [] + existing_tabs.append({ + 'type': 'static_tab', + 'name': new_object.display_name, + 'url_slug': new_object.location.name + }) + course.tabs = existing_tabs + self.update_metadata(course.location, course.xblock_kvs._metadata) def fire_updated_modulestore_signal(self, course_id, location): """ @@ -683,7 +744,7 @@ def _update_single_item(self, location, update): if result['n'] == 0: raise ItemNotFoundError(location) - def update_item(self, location, data): + def update_item(self, location, data, allow_not_found=False): """ Set the data in the item specified by the location to data @@ -691,8 +752,11 @@ def update_item(self, location, data): location: Something that can be passed to Location data: A nested dictionary of problem data """ - - self._update_single_item(location, {'definition.data': data}) + try: + self._update_single_item(location, {'definition.data': data}) + except ItemNotFoundError: + if not allow_not_found: + raise def update_children(self, location, children): """ @@ -775,3 +839,24 @@ def get_errored_courses(self): are loaded on demand, rather than up front """ return {} + + def _create_new_model_data(self, category, location, definition_data, metadata): + """ + To instantiate a new xmodule which will be saved latter, set up the dbModel and kvs + """ + kvs = MongoKeyValueStore( + definition_data, + [], + metadata, + location, + category + ) + + class_ = XModuleDescriptor.load_class( + category, + self.default_class + ) + model_data = DbModel(kvs, class_, None, MongoUsage(None, location)) + model_data['category'] = category + model_data['location'] = location + return model_data diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py index f34c3a53f98..d289e037399 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py @@ -8,11 +8,12 @@ from datetime import datetime +from xmodule.exceptions import InvalidVersionError from xmodule.modulestore import Location, namedtuple_to_son -from xmodule.modulestore.exceptions import ItemNotFoundError +from xmodule.modulestore.exceptions import ItemNotFoundError, DuplicateItemError from xmodule.modulestore.inheritance import own_metadata -from xmodule.exceptions import InvalidVersionError -from xmodule.modulestore.mongo.base import MongoModuleStore +from xmodule.modulestore.mongo.base import location_to_query, get_course_id_no_run, MongoModuleStore +import pymongo from pytz import UTC DRAFT = 'draft' @@ -92,6 +93,21 @@ def get_instance(self, course_id, location, depth=0): except ItemNotFoundError: return wrap_draft(super(DraftModuleStore, self).get_instance(course_id, location, depth=depth)) + def create_xmodule(self, location, definition_data=None, metadata=None, system=None): + """ + Create the new xmodule but don't save it. Returns the new module with a draft locator + + :param location: a Location--must have a category + :param definition_data: can be empty. The initial definition_data for the kvs + :param metadata: can be empty, the initial metadata for the kvs + :param system: if you already have an xmodule from the course, the xmodule.system value + """ + draft_loc = as_draft(location) + if draft_loc.category in DIRECT_ONLY_CATEGORIES: + raise InvalidVersionError(location) + return super(DraftModuleStore, self).create_xmodule(draft_loc, definition_data, metadata, system) + + def get_items(self, location, course_id=None, depth=0): """ Returns a list of XModuleDescriptor instances for the items @@ -119,14 +135,26 @@ def get_items(self, location, course_id=None, depth=0): ] return [wrap_draft(item) for item in draft_items + non_draft_items] - def clone_item(self, source, location): + def convert_to_draft(self, source_location): """ - Clone a new item that is a copy of the item at the location `source` - and writes it to `location` + Create a copy of the source and mark its revision as draft. + + :param source: the location of the source (its revision must be None) """ - if Location(location).category in DIRECT_ONLY_CATEGORIES: - raise InvalidVersionError(location) - return wrap_draft(super(DraftModuleStore, self).clone_item(source, as_draft(location))) + original = self.collection.find_one(location_to_query(source_location)) + draft_location = as_draft(source_location) + if draft_location.category in DIRECT_ONLY_CATEGORIES: + raise InvalidVersionError(source_location) + original['_id'] = draft_location.dict() + try: + self.collection.insert(original) + except pymongo.errors.DuplicateKeyError: + raise DuplicateItemError(original['_id']) + + self.refresh_cached_metadata_inheritance_tree(draft_location) + self.fire_updated_modulestore_signal(get_course_id_no_run(draft_location), draft_location) + + return self._load_items([original])[0] def update_item(self, location, data, allow_not_found=False): """ @@ -140,7 +168,7 @@ def update_item(self, location, data, allow_not_found=False): try: draft_item = self.get_item(location) if not getattr(draft_item, 'is_draft', False): - self.clone_item(location, draft_loc) + self.convert_to_draft(location) except ItemNotFoundError, e: if not allow_not_found: raise e @@ -158,7 +186,7 @@ def update_children(self, location, children): draft_loc = as_draft(location) draft_item = self.get_item(location) if not getattr(draft_item, 'is_draft', False): - self.clone_item(location, draft_loc) + self.convert_to_draft(location) return super(DraftModuleStore, self).update_children(draft_loc, children) @@ -174,7 +202,7 @@ def update_metadata(self, location, metadata): draft_item = self.get_item(location) if not getattr(draft_item, 'is_draft', False): - self.clone_item(location, draft_loc) + self.convert_to_draft(location) if 'is_draft' in metadata: del metadata['is_draft'] @@ -218,9 +246,7 @@ def unpublish(self, location): """ Turn the published version into a draft, removing the published version """ - if Location(location).category in DIRECT_ONLY_CATEGORIES: - raise InvalidVersionError(location) - super(DraftModuleStore, self).clone_item(location, as_draft(location)) + self.convert_to_draft(location) super(DraftModuleStore, self).delete_item(location) def _query_children_for_cache_children(self, items): diff --git a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py b/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py index c32d0bca4c4..564aac141da 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py @@ -5,7 +5,6 @@ from django.conf import settings import xmodule.modulestore.django -from xmodule.templates import update_templates from unittest.util import safe_repr @@ -110,22 +109,6 @@ def flush_mongo_except_templates(): modulestore.collection.remove(query) modulestore.collection.drop() - @staticmethod - def load_templates_if_necessary(): - """ - Load templates into the direct modulestore only if they do not already exist. - We need the templates, because they are copied to create - XModules such as sections and problems. - """ - modulestore = xmodule.modulestore.django.modulestore('direct') - - # Count the number of templates - query = {"_id.course": "templates"} - num_templates = modulestore.collection.find(query).count() - - if num_templates < 1: - update_templates(modulestore) - @classmethod def setUpClass(cls): """ @@ -169,9 +152,6 @@ def _pre_setup(self): # Flush anything that is not a template ModuleStoreTestCase.flush_mongo_except_templates() - # Check that we have templates loaded; if not, load them - ModuleStoreTestCase.load_templates_if_necessary() - # Call superclass implementation super(ModuleStoreTestCase, self)._pre_setup() diff --git a/common/lib/xmodule/xmodule/modulestore/tests/factories.py b/common/lib/xmodule/xmodule/modulestore/tests/factories.py index 457a88482a0..9a0c87ff971 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/factories.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/factories.py @@ -1,15 +1,13 @@ -from factory import Factory, lazy_attribute_sequence, lazy_attribute -from uuid import uuid4 import datetime -from xmodule.modulestore import Location -from xmodule.modulestore.django import modulestore -from xmodule.modulestore.inheritance import own_metadata -from xmodule.x_module import ModuleSystem -from mitxmako.shortcuts import render_to_string -from xblock.runtime import InvalidScopeError +from factory import Factory, LazyAttributeSequence +from uuid import uuid4 from pytz import UTC +from xmodule.modulestore import Location +from xmodule.modulestore.django import modulestore +from xmodule.course_module import CourseDescriptor +from xblock.core import Scope class XModuleCourseFactory(Factory): """ @@ -21,9 +19,8 @@ class XModuleCourseFactory(Factory): @classmethod def _create(cls, target_class, **kwargs): - template = Location('i4x', 'edx', 'templates', 'course', 'Empty') org = kwargs.pop('org', None) - number = kwargs.pop('number', None) + number = kwargs.pop('number', kwargs.pop('course', None)) display_name = kwargs.pop('display_name', None) location = Location('i4x', org, number, 'course', Location.clean(display_name)) @@ -33,7 +30,7 @@ def _create(cls, target_class, **kwargs): store = modulestore() # Write the data to the mongo datastore - new_course = store.clone_item(template, location) + new_course = store.create_xmodule(location) # This metadata code was copied from cms/djangoapps/contentstore/views.py if display_name is not None: @@ -56,13 +53,7 @@ def _create(cls, target_class, **kwargs): setattr(new_course, k, v) # Update the data in the mongo datastore - store.update_metadata(new_course.location, own_metadata(new_course)) - store.update_item(new_course.location, new_course._model_data._kvs._data) - - # update_item updates the the course as it exists in the modulestore, but doesn't - # update the instance we are working with, so have to refetch the course after updating it. - new_course = store.get_instance(new_course.id, new_course.location) - + store.save_xmodule(new_course) return new_course @@ -73,7 +64,6 @@ class Course: class CourseFactory(XModuleCourseFactory): FACTORY_FOR = Course - template = 'i4x://edx/templates/course/Empty' org = 'MITx' number = '999' display_name = 'Robot Super Course' @@ -86,18 +76,14 @@ class XModuleItemFactory(Factory): ABSTRACT_FACTORY = True - display_name = None - - @lazy_attribute - def category(attr): - template = Location(attr.template) - return template.category + parent_location = 'i4x://MITx/999/course/Robot_Super_Course' + category = 'problem' + display_name = LazyAttributeSequence(lambda o, n: "{} {}".format(o.category, n)) - @lazy_attribute - def location(attr): - parent = Location(attr.parent_location) - dest_name = attr.display_name.replace(" ", "_") if attr.display_name is not None else uuid4().hex - return parent._replace(category=attr.category, name=dest_name) + @staticmethod + def location(parent, category, display_name): + dest_name = display_name.replace(" ", "_") if display_name is not None else uuid4().hex + return Location(parent).replace(category=category, name=dest_name) @classmethod def _create(cls, target_class, **kwargs): @@ -107,8 +93,7 @@ def _create(cls, target_class, **kwargs): *parent_location* (required): the location of the parent module (e.g. the parent course or section) - *template* (required): the template to create the item from - (e.g. i4x://templates/section/Empty) + category: the category of the resulting item. *data* (optional): the data for the item (e.g. XML problem definition for a problem item) @@ -121,41 +106,32 @@ def _create(cls, target_class, **kwargs): """ DETACHED_CATEGORIES = ['about', 'static_tab', 'course_info'] - + # catch any old style users before they get into trouble + assert not 'template' in kwargs parent_location = Location(kwargs.get('parent_location')) - template = Location(kwargs.get('template')) data = kwargs.get('data') + category = kwargs.get('category') display_name = kwargs.get('display_name') metadata = kwargs.get('metadata', {}) + location = kwargs.get('location', XModuleItemFactory.location(parent_location, category, display_name)) + assert location != parent_location store = modulestore('direct') # This code was based off that in cms/djangoapps/contentstore/views.py parent = store.get_item(parent_location) - new_item = store.clone_item(template, kwargs.get('location')) - # replace the display name with an optional parameter passed in from the caller if display_name is not None: - new_item.display_name = display_name + metadata['display_name'] = display_name + # note that location comes from above lazy_attribute + store.create_and_save_xmodule(location, metadata=metadata, definition_data=data) - # Add additional metadata or override current metadata - item_metadata = own_metadata(new_item) - item_metadata.update(metadata) - store.update_metadata(new_item.location.url(), item_metadata) + if location.category not in DETACHED_CATEGORIES: + parent.children.append(location.url()) + store.update_children(parent_location, parent.children) - # replace the data with the optional *data* parameter - if data is not None: - store.update_item(new_item.location, data) - - if new_item.location.category not in DETACHED_CATEGORIES: - store.update_children(parent_location, parent.children + [new_item.location.url()]) - - # update_children updates the the item as it exists in the modulestore, but doesn't - # update the instance we are working with, so have to refetch the item after updating it. - new_item = store.get_item(new_item.location) - - return new_item + return store.get_item(location) class Item: @@ -164,40 +140,4 @@ class Item: class ItemFactory(XModuleItemFactory): FACTORY_FOR = Item - - parent_location = 'i4x://MITx/999/course/Robot_Super_Course' - template = 'i4x://edx/templates/chapter/Empty' - - @lazy_attribute_sequence - def display_name(attr, n): - return "{} {}".format(attr.category.title(), n) - - -def get_test_xmodule_for_descriptor(descriptor): - """ - Attempts to create an xmodule which responds usually correctly from the descriptor. Not guaranteed. - - :param descriptor: - """ - module_sys = ModuleSystem( - ajax_url='', - track_function=None, - get_module=None, - render_template=render_to_string, - replace_urls=None, - xblock_model_data=_test_xblock_model_data_accessor(descriptor) - ) - return descriptor.xmodule(module_sys) - - -def _test_xblock_model_data_accessor(descriptor): - simple_map = {} - for field in descriptor.fields: - try: - simple_map[field.name] = getattr(descriptor, field.name) - except InvalidScopeError: - simple_map[field.name] = field.default - for field in descriptor.module_class.fields: - if field.name not in simple_map: - simple_map[field.name] = field.default - return lambda o: simple_map + category = 'chapter' diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py index 44e69fb0eda..c149724cc7e 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py @@ -9,7 +9,6 @@ from xmodule.modulestore import Location from xmodule.modulestore.mongo import MongoModuleStore, MongoKeyValueStore from xmodule.modulestore.xml_importer import import_from_xml -from xmodule.templates import update_templates from .test_modulestore import check_path_to_location from . import DATA_DIR @@ -51,7 +50,6 @@ def initdb(): # Explicitly list the courses to load (don't want the big one) courses = ['toy', 'simple'] import_from_xml(store, DATA_DIR, courses) - update_templates(store) return store @staticmethod @@ -126,7 +124,7 @@ def setUp(self): self.location = Location('i4x://org/course/category/name@version') self.children = ['i4x://org/course/child/a', 'i4x://org/course/child/b'] self.metadata = {'meta': 'meta_val'} - self.kvs = MongoKeyValueStore(self.data, self.children, self.metadata, self.location) + self.kvs = MongoKeyValueStore(self.data, self.children, self.metadata, self.location, 'category') def _check_read(self, key, expected_value): assert_equals(expected_value, self.kvs.get(key)) diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index 26c8b9bfcac..012740ff9a2 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -463,7 +463,10 @@ def _load_extra_content(self, system, course_descriptor, category, path, course_ # tabs are referenced in policy.json through a 'slug' which is just the filename without the .html suffix slug = os.path.splitext(os.path.basename(filepath))[0] loc = Location('i4x', course_descriptor.location.org, course_descriptor.location.course, category, slug) - module = HtmlDescriptor(system, {'data': html, 'location': loc}) + module = HtmlDescriptor( + system, + {'data': html, 'location': loc, 'category': category} + ) # VS[compat]: # Hack because we need to pull in the 'display_name' for static tabs (because we need to edit them) # from the course policy diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py index 1fe62035e65..60eb9dc749f 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py @@ -810,7 +810,6 @@ class CombinedOpenEndedV1Descriptor(): filename_extension = "xml" has_score = True - template_dir_name = "combinedopenended" def __init__(self, system): self.system = system diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py index 0f0851fbf75..8d8a85f7883 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py @@ -730,7 +730,6 @@ class OpenEndedDescriptor(): filename_extension = "xml" has_score = True - template_dir_name = "openended" def __init__(self, system): self.system = system diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/self_assessment_module.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/self_assessment_module.py index a5498289e28..1262e1f68fd 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/self_assessment_module.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/self_assessment_module.py @@ -287,7 +287,6 @@ class SelfAssessmentDescriptor(): filename_extension = "xml" has_score = True - template_dir_name = "selfassessment" def __init__(self, system): self.system = system diff --git a/common/lib/xmodule/xmodule/peer_grading_module.py b/common/lib/xmodule/xmodule/peer_grading_module.py index c88a2e1b388..03003ed1e5b 100644 --- a/common/lib/xmodule/xmodule/peer_grading_module.py +++ b/common/lib/xmodule/xmodule/peer_grading_module.py @@ -613,7 +613,6 @@ class PeerGradingDescriptor(PeerGradingFields, RawDescriptor): has_score = True always_recalculate_grades = True - template_dir_name = "peer_grading" #Specify whether or not to pass in open ended interface needs_open_ended_interface = True diff --git a/common/lib/xmodule/xmodule/poll_module.py b/common/lib/xmodule/xmodule/poll_module.py index ca12f239abc..8e7407752a0 100644 --- a/common/lib/xmodule/xmodule/poll_module.py +++ b/common/lib/xmodule/xmodule/poll_module.py @@ -140,7 +140,6 @@ class PollDescriptor(PollFields, MakoModuleDescriptor, XmlDescriptor): _child_tag_name = 'answer' module_class = PollModule - template_dir_name = 'poll' @classmethod def definition_from_xml(cls, xml_object, system): diff --git a/common/lib/xmodule/xmodule/templates.py b/common/lib/xmodule/xmodule/templates.py index 6479b3df246..8e350bb6187 100644 --- a/common/lib/xmodule/xmodule/templates.py +++ b/common/lib/xmodule/xmodule/templates.py @@ -1,34 +1,18 @@ """ -This module handles loading xmodule templates from disk into the modulestore. -These templates are used by the CMS to provide baseline content that -can be cloned when adding new modules to a course. +This module handles loading xmodule templates +These templates are used by the CMS to provide content that overrides xmodule defaults for +samples. -`Template`s are defined in x_module. They contain 3 attributes: - metadata: A dictionary with the template metadata. This should contain - any values for fields - * with scope Scope.settings - * that have values different than the field defaults - * and that are to be editable in Studio - data: A JSON value that defines the template content. This should be a dictionary - containing values for fields - * with scope Scope.content - * that have values different than the field defaults - * and that are to be editable in Studio - or, if the module uses a single Scope.content String field named `data`, this - should be a string containing the contents of that field - children: A list of Location urls that define the template children - -Templates are defined on XModuleDescriptor types, in the template attribute. +``Template``s are defined in x_module. They contain 2 attributes: + :metadata: A dictionary with the template metadata + :data: A JSON value that defines the template content """ - +# should this move to cms since it's really only for module crud? import logging -from fs.memoryfs import MemoryFS from collections import defaultdict from .x_module import XModuleDescriptor -from .mako_module import MakoDescriptorSystem -from .modulestore import Location log = logging.getLogger(__name__) @@ -37,73 +21,9 @@ def all_templates(): """ Returns all templates for enabled modules, grouped by descriptor type """ - + # TODO use memcache to memoize w/ expiration templates = defaultdict(list) for category, descriptor in XModuleDescriptor.load_classes(): templates[category] = descriptor.templates() return templates - - -class TemplateTestSystem(MakoDescriptorSystem): - """ - This system exists to help verify that XModuleDescriptors can be instantiated - from their defined templates before we load the templates into the modulestore. - """ - def __init__(self): - super(TemplateTestSystem, self).__init__( - lambda *a, **k: None, - MemoryFS(), - lambda msg: None, - render_template=lambda *a, **k: None, - ) - - -def update_templates(modulestore): - """ - Updates the set of templates in the modulestore with all templates currently - available from the installed plugins - """ - - # cdodge: build up a list of all existing templates. This will be used to determine which - # templates have been removed from disk - and thus we need to remove from the DB - templates_to_delete = modulestore.get_items(['i4x', 'edx', 'templates', None, None, None]) - - for category, templates in all_templates().items(): - for template in templates: - if 'display_name' not in template.metadata: - log.warning('No display_name specified in template {0}, skipping'.format(template)) - continue - - template_location = Location('i4x', 'edx', 'templates', category, Location.clean_for_url_name(template.metadata['display_name'])) - - try: - json_data = { - 'definition': { - 'data': template.data, - 'children': template.children - }, - 'metadata': template.metadata - } - json_data['location'] = template_location.dict() - - XModuleDescriptor.load_from_json(json_data, TemplateTestSystem()) - except: - log.warning('Unable to instantiate {cat} from template {template}, skipping'.format( - cat=category, - template=template - ), exc_info=True) - continue - - modulestore.update_item(template_location, template.data) - modulestore.update_children(template_location, template.children) - modulestore.update_metadata(template_location, template.metadata) - - # remove template from list of templates to delete - templates_to_delete = [t for t in templates_to_delete if t.location != template_location] - - # now remove all templates which appear to have removed from disk - if len(templates_to_delete) > 0: - logging.debug('deleting dangling templates = {0}'.format(templates_to_delete)) - for template in templates_to_delete: - modulestore.delete_item(template.location) diff --git a/common/lib/xmodule/xmodule/templates/about/empty.yaml b/common/lib/xmodule/xmodule/templates/about/empty.yaml deleted file mode 100644 index 0967ef424bc..00000000000 --- a/common/lib/xmodule/xmodule/templates/about/empty.yaml +++ /dev/null @@ -1 +0,0 @@ -{} diff --git a/common/lib/xmodule/xmodule/templates/annotatable/default.yaml b/common/lib/xmodule/xmodule/templates/annotatable/default.yaml deleted file mode 100644 index 0967ef424bc..00000000000 --- a/common/lib/xmodule/xmodule/templates/annotatable/default.yaml +++ /dev/null @@ -1 +0,0 @@ -{} diff --git a/common/lib/xmodule/xmodule/templates/combinedopenended/default.yaml b/common/lib/xmodule/xmodule/templates/combinedopenended/default.yaml deleted file mode 100644 index 0967ef424bc..00000000000 --- a/common/lib/xmodule/xmodule/templates/combinedopenended/default.yaml +++ /dev/null @@ -1 +0,0 @@ -{} diff --git a/common/lib/xmodule/xmodule/templates/course/empty.yaml b/common/lib/xmodule/xmodule/templates/course/empty.yaml deleted file mode 100644 index 0967ef424bc..00000000000 --- a/common/lib/xmodule/xmodule/templates/course/empty.yaml +++ /dev/null @@ -1 +0,0 @@ -{} diff --git a/common/lib/xmodule/xmodule/templates/courseinfo/empty.yaml b/common/lib/xmodule/xmodule/templates/courseinfo/empty.yaml deleted file mode 100644 index 0967ef424bc..00000000000 --- a/common/lib/xmodule/xmodule/templates/courseinfo/empty.yaml +++ /dev/null @@ -1 +0,0 @@ -{} diff --git a/common/lib/xmodule/xmodule/templates/default/empty.yaml b/common/lib/xmodule/xmodule/templates/default/empty.yaml deleted file mode 100644 index 0967ef424bc..00000000000 --- a/common/lib/xmodule/xmodule/templates/default/empty.yaml +++ /dev/null @@ -1 +0,0 @@ -{} diff --git a/common/lib/xmodule/xmodule/templates/discussion/default.yaml b/common/lib/xmodule/xmodule/templates/discussion/default.yaml deleted file mode 100644 index 0967ef424bc..00000000000 --- a/common/lib/xmodule/xmodule/templates/discussion/default.yaml +++ /dev/null @@ -1 +0,0 @@ -{} diff --git a/common/lib/xmodule/xmodule/templates/html/announcement.yaml b/common/lib/xmodule/xmodule/templates/html/announcement.yaml index 30a8ccb41e2..c0ecc615249 100644 --- a/common/lib/xmodule/xmodule/templates/html/announcement.yaml +++ b/common/lib/xmodule/xmodule/templates/html/announcement.yaml @@ -21,4 +21,3 @@ data: | -children: [] diff --git a/common/lib/xmodule/xmodule/templates/html/empty.yaml b/common/lib/xmodule/xmodule/templates/html/empty.yaml deleted file mode 100644 index 0967ef424bc..00000000000 --- a/common/lib/xmodule/xmodule/templates/html/empty.yaml +++ /dev/null @@ -1 +0,0 @@ -{} diff --git a/common/lib/xmodule/xmodule/templates/html/everything.yaml b/common/lib/xmodule/xmodule/templates/html/everything.yaml deleted file mode 100644 index 348ce64fa1f..00000000000 --- a/common/lib/xmodule/xmodule/templates/html/everything.yaml +++ /dev/null @@ -1,33 +0,0 @@ ---- -metadata: - display_name: Announcement - -data: | -

    Heading of document

    -

    First subheading

    -

    This is a paragraph. It will take care of line breaks for you.

    HTML only parses the location - - of tags for inserting line breaks into your doc, not - line - breaks - you - add - yourself. -

    -

    Links

    -

    You can refer to other parts of the internet with a link, to other parts of your course by prepending your link with /course/

    -

    Now a list:

    -
      -
    • An item
      • -
    • Another item
      • -
    • And yet another
      • -
    -

    This list has an ordering

    -
      -
    1. An item
      2. -
    2. Another item
      3. -
    3. Yet another item
      4. -
    -

    Note, we have a lot of standard edX styles, so please try to avoid any custom styling, and make sure that you make a note of any custom styling that you do yourself so that we can incorporate it into - tools that other people can use.

    -children: [] diff --git a/common/lib/xmodule/xmodule/templates/html/latex_html.yaml b/common/lib/xmodule/xmodule/templates/html/latex_html.yaml index ba5c4b5c063..2db7e98c659 100644 --- a/common/lib/xmodule/xmodule/templates/html/latex_html.yaml +++ b/common/lib/xmodule/xmodule/templates/html/latex_html.yaml @@ -19,4 +19,3 @@ data: | It is very convenient to write complex equations in LaTeX.

    -children: [] diff --git a/common/lib/xmodule/xmodule/templates/peer_grading/default.yaml b/common/lib/xmodule/xmodule/templates/peer_grading/default.yaml deleted file mode 100644 index 0967ef424bc..00000000000 --- a/common/lib/xmodule/xmodule/templates/peer_grading/default.yaml +++ /dev/null @@ -1 +0,0 @@ -{} diff --git a/common/lib/xmodule/xmodule/templates/problem/circuitschematic.yaml b/common/lib/xmodule/xmodule/templates/problem/circuitschematic.yaml index 3b051f2ba83..1717bb91ad3 100644 --- a/common/lib/xmodule/xmodule/templates/problem/circuitschematic.yaml +++ b/common/lib/xmodule/xmodule/templates/problem/circuitschematic.yaml @@ -1,9 +1,9 @@ - --- metadata: display_name: Circuit Schematic Builder rerandomize: never showanswer: finished + markdown: !!null data: | Please make a voltage divider that splits the provided voltage evenly. diff --git a/common/lib/xmodule/xmodule/templates/problem/customgrader.yaml b/common/lib/xmodule/xmodule/templates/problem/customgrader.yaml index 48feef481bd..05de74f28c4 100644 --- a/common/lib/xmodule/xmodule/templates/problem/customgrader.yaml +++ b/common/lib/xmodule/xmodule/templates/problem/customgrader.yaml @@ -1,50 +1,47 @@ --- metadata: display_name: Custom Python-Evaluated Input - rerandomize: never - showanswer: finished + markdown: !!null data: | - -

    - A custom python-evaluated input problem accepts one or more lines of text input from the - student, and evaluates the inputs for correctness based on evaluation using a - python script embedded within the problem. -

    - - - -

    Enter two integers which sum to 10:

    - -
    - -     - -

    Enter two integers which sum to 20:

    - -
    - -     - - -
    -

    Explanation

    -

    Any set of integers on the line \(y = 10 - x\) and \(y = 20 - x\) satisfy these constraints.

    - -
    -     -     - -children: [] + +

    + A custom python-evaluated input problem accepts one or more lines of text input from the + student, and evaluates the inputs for correctness based on evaluation using a + python script embedded within the problem. +

    + + + +

    Enter two integers which sum to 10:

    + +
    + +     + +

    Enter two integers which sum to 20:

    + +
    + +     + + +
    +

    Explanation

    +

    Any set of integers on the line \(y = 10 - x\) and \(y = 20 - x\) satisfy these constraints.

    + +
    +     +     diff --git a/common/lib/xmodule/xmodule/templates/problem/empty.yaml b/common/lib/xmodule/xmodule/templates/problem/empty.yaml deleted file mode 100644 index 0967ef424bc..00000000000 --- a/common/lib/xmodule/xmodule/templates/problem/empty.yaml +++ /dev/null @@ -1 +0,0 @@ -{} diff --git a/common/lib/xmodule/xmodule/templates/problem/emptyadvanced.yaml b/common/lib/xmodule/xmodule/templates/problem/emptyadvanced.yaml deleted file mode 100644 index 3d696ec2fd2..00000000000 --- a/common/lib/xmodule/xmodule/templates/problem/emptyadvanced.yaml +++ /dev/null @@ -1,10 +0,0 @@ ---- -metadata: - display_name: Blank Advanced Problem - rerandomize: never - showanswer: finished -data: | - - - -children: [] diff --git a/common/lib/xmodule/xmodule/templates/problem/forumularesponse.yaml b/common/lib/xmodule/xmodule/templates/problem/forumularesponse.yaml index 0401a01c316..4cf877bd1fe 100644 --- a/common/lib/xmodule/xmodule/templates/problem/forumularesponse.yaml +++ b/common/lib/xmodule/xmodule/templates/problem/forumularesponse.yaml @@ -3,6 +3,7 @@ metadata: display_name: Math Expression Input rerandomize: never showanswer: finished + markdown: !!null data: |

    @@ -43,5 +44,3 @@ data: |

    - -children: [] diff --git a/common/lib/xmodule/xmodule/templates/problem/imageresponse.yaml b/common/lib/xmodule/xmodule/templates/problem/imageresponse.yaml index ab1f22e3b23..566997671dd 100644 --- a/common/lib/xmodule/xmodule/templates/problem/imageresponse.yaml +++ b/common/lib/xmodule/xmodule/templates/problem/imageresponse.yaml @@ -3,6 +3,7 @@ metadata: display_name: Image Mapped Input rerandomize: never showanswer: finished + markdown: !!null data: |

    @@ -21,6 +22,3 @@ data: | - - -children: [] diff --git a/common/lib/xmodule/xmodule/templates/problem/latex_problem.yaml b/common/lib/xmodule/xmodule/templates/problem/latex_problem.yaml index 82d7e8c1ae5..097055cfe37 100644 --- a/common/lib/xmodule/xmodule/templates/problem/latex_problem.yaml +++ b/common/lib/xmodule/xmodule/templates/problem/latex_problem.yaml @@ -85,6 +85,7 @@ metadata: can contain equations: $\alpha = \frac{2}{\sqrt{1+\gamma}}$ } This is some text after the showhide example. + markdown: !!null data: | @@ -214,4 +215,3 @@ data: |

    -children: [] diff --git a/common/lib/xmodule/xmodule/templates/problem/multiplechoice.yaml b/common/lib/xmodule/xmodule/templates/problem/multiplechoice.yaml index 10d51de280f..202fc03b44a 100644 --- a/common/lib/xmodule/xmodule/templates/problem/multiplechoice.yaml +++ b/common/lib/xmodule/xmodule/templates/problem/multiplechoice.yaml @@ -3,33 +3,25 @@ metadata: display_name: Multiple Choice rerandomize: never showanswer: finished - markdown: - "A multiple choice problem presents radio buttons for student input. Students can only select a single + markdown: | + A multiple choice problem presents radio buttons for student input. Students can only select a single option presented. Multiple Choice questions have been the subject of many areas of research due to the early invention and adoption of bubble sheets. - One of the main elements that goes into a good multiple choice question is the existence of good distractors. That is, each of the alternate responses presented to the student should be the result of a plausible mistake that a student might make. - What Apple device competed with the portable CD player? - ( ) The iPad - ( ) Napster - (x) The iPod - ( ) The vegetable peeler - [explanation] The release of the iPod allowed consumers to carry their entire music library with them in a format that did not rely on fragile and energy-intensive spinning disks. [explanation] - " data: |

    @@ -54,4 +46,3 @@ data: | -children: [] diff --git a/common/lib/xmodule/xmodule/templates/problem/numericalresponse.yaml b/common/lib/xmodule/xmodule/templates/problem/numericalresponse.yaml index 548fd94fab1..9b2ddec2a7a 100644 --- a/common/lib/xmodule/xmodule/templates/problem/numericalresponse.yaml +++ b/common/lib/xmodule/xmodule/templates/problem/numericalresponse.yaml @@ -3,43 +3,33 @@ metadata: display_name: Numerical Input rerandomize: never showanswer: finished - markdown: - "A numerical input problem accepts a line of text input from the + markdown: | + A numerical input problem accepts a line of text input from the student, and evaluates the input for correctness based on its numerical value. - The answer is correct if it is within a specified numerical tolerance of the expected answer. - Enter the numerical value of Pi: - = 3.14159 +- .02 - Enter the approximate value of 502*9: - = 4518 +- 15% - - + Enter the number of fingers on a human hand: - = 5 - [explanation] Pi, or the the ratio between a circle's circumference to its diameter, is an irrational number known to extreme precision. It is value is approximately equal to 3.14. - + Although you can get an exact value by typing 502*9 into a calculator, the result will be close to 500*10, or 5,000. The grader accepts any response within 15% of the true value, 4518, so that you can use any estimation technique that you like. - + If you look at your hand, you can count that you have five fingers. [explanation] - " - data: |

    @@ -83,5 +73,3 @@ data: | - -children: [] diff --git a/common/lib/xmodule/xmodule/templates/problem/optionresponse.yaml b/common/lib/xmodule/xmodule/templates/problem/optionresponse.yaml index c2edfb1cbc1..8e59f8ae4d2 100644 --- a/common/lib/xmodule/xmodule/templates/problem/optionresponse.yaml +++ b/common/lib/xmodule/xmodule/templates/problem/optionresponse.yaml @@ -3,19 +3,16 @@ metadata: display_name: Dropdown rerandomize: never showanswer: finished - markdown: - "Dropdown problems give a limited set of options for students to respond with, and present those options + markdown: | + Dropdown problems give a limited set of options for students to respond with, and present those options in a format that encourages them to search for a specific answer rather than being immediately presented with options from which to recognize the correct answer. - The answer options and the identification of the correct answer is defined in the optioninput tag. - Translation between Dropdown and __________ is extremely straightforward: - - [[(Multiple Choice), Text Input, Numerical Input, External Response, Image Response]] + [[(Multiple Choice), Text Input, Numerical Input, External Response, Image Response]] [explanation] Multiple Choice also allows students to select from a variety of pre-written responses, although the @@ -23,7 +20,6 @@ metadata: slightly because students are more likely to think of an answer and then search for it rather than relying purely on recognition to answer the question. [explanation] - " data: |

    Dropdown problems give a limited set of options for students to respond with, and present those options @@ -45,4 +41,3 @@ data: | -children: [] diff --git a/common/lib/xmodule/xmodule/templates/problem/problem_with_hint.yaml b/common/lib/xmodule/xmodule/templates/problem/problem_with_hint.yaml index 73a94ed9417..0d93cd3c5e8 100644 --- a/common/lib/xmodule/xmodule/templates/problem/problem_with_hint.yaml +++ b/common/lib/xmodule/xmodule/templates/problem/problem_with_hint.yaml @@ -46,7 +46,7 @@ metadata: enter your answer in upper or lower case, with or without quotes. \edXabox{type="custom" cfn='test_str' expect='python' hintfn='hint_fn'} - + markdown: !!null data: | @@ -92,4 +92,3 @@ data: |

    -children: [] diff --git a/common/lib/xmodule/xmodule/templates/problem/string_response.yaml b/common/lib/xmodule/xmodule/templates/problem/string_response.yaml index 64e3dc062f3..9c59ae3bc24 100644 --- a/common/lib/xmodule/xmodule/templates/problem/string_response.yaml +++ b/common/lib/xmodule/xmodule/templates/problem/string_response.yaml @@ -3,16 +3,13 @@ metadata: display_name: Text Input rerandomize: never showanswer: finished - # Note, the extra newlines are needed to make the yaml parser add blank lines instead of folding - markdown: - "A text input problem accepts a line of text from the + markdown: | + A text input problem accepts a line of text from the student, and evaluates the input for correctness based on an expected answer. - The answer is correct if it matches every character of the expected answer. This can be a problem with international spelling, dates, or anything where the format of the answer is not clear. - Which US state has Lansing as its capital? @@ -23,9 +20,8 @@ metadata: Lansing is the capital of Michigan, although it is not Michgan's largest city, or even the seat of the county in which it resides. [explanation] - " data: | - +

    A text input problem accepts a line of text from the @@ -46,4 +42,3 @@ data: | -children: [] diff --git a/common/lib/xmodule/xmodule/templates/sequence/with_video.yaml b/common/lib/xmodule/xmodule/templates/sequence/with_video.yaml deleted file mode 100644 index a56d44ebff0..00000000000 --- a/common/lib/xmodule/xmodule/templates/sequence/with_video.yaml +++ /dev/null @@ -1,7 +0,0 @@ ---- -metadata: - display_name: Sequence with Video - data_dir: a_made_up_name -data: '' -children: - - 'i4x://edx/templates/video/default' diff --git a/common/lib/xmodule/xmodule/templates/statictab/empty.yaml b/common/lib/xmodule/xmodule/templates/statictab/empty.yaml deleted file mode 100644 index 9e26dfeeb6e..00000000000 --- a/common/lib/xmodule/xmodule/templates/statictab/empty.yaml +++ /dev/null @@ -1 +0,0 @@ -{} \ No newline at end of file diff --git a/common/lib/xmodule/xmodule/templates/video/default.yaml b/common/lib/xmodule/xmodule/templates/video/default.yaml deleted file mode 100644 index 0967ef424bc..00000000000 --- a/common/lib/xmodule/xmodule/templates/video/default.yaml +++ /dev/null @@ -1 +0,0 @@ -{} diff --git a/common/lib/xmodule/xmodule/templates/videoalpha/default.yaml b/common/lib/xmodule/xmodule/templates/videoalpha/default.yaml deleted file mode 100644 index 0967ef424bc..00000000000 --- a/common/lib/xmodule/xmodule/templates/videoalpha/default.yaml +++ /dev/null @@ -1 +0,0 @@ -{} diff --git a/common/lib/xmodule/xmodule/templates/word_cloud/default.yaml b/common/lib/xmodule/xmodule/templates/word_cloud/default.yaml deleted file mode 100644 index 0967ef424bc..00000000000 --- a/common/lib/xmodule/xmodule/templates/word_cloud/default.yaml +++ /dev/null @@ -1 +0,0 @@ -{} diff --git a/common/lib/xmodule/xmodule/tests/test_logic.py b/common/lib/xmodule/xmodule/tests/test_logic.py index 9be533885c2..5fe7aa28326 100644 --- a/common/lib/xmodule/xmodule/tests/test_logic.py +++ b/common/lib/xmodule/xmodule/tests/test_logic.py @@ -28,7 +28,8 @@ class LogicTest(unittest.TestCase): def setUp(self): class EmptyClass: """Empty object.""" - pass + url_name = '' + category = 'test' self.system = get_test_system() self.descriptor = EmptyClass() diff --git a/common/lib/xmodule/xmodule/tests/test_xml_module.py b/common/lib/xmodule/xmodule/tests/test_xml_module.py index 7ccc71dd965..a277ff2900d 100644 --- a/common/lib/xmodule/xmodule/tests/test_xml_module.py +++ b/common/lib/xmodule/xmodule/tests/test_xml_module.py @@ -141,6 +141,7 @@ def test_type_and_options(self): def get_xml_editable_fields(self, model_data): system = get_test_system() system.render_template = Mock(return_value="

    Test Template HTML
    ") + model_data['category'] = 'test' return XmlDescriptor(runtime=system, model_data=model_data).editable_metadata_fields def get_descriptor(self, model_data): diff --git a/common/lib/xmodule/xmodule/video_module.py b/common/lib/xmodule/xmodule/video_module.py index ebff888f344..381cc9a622d 100644 --- a/common/lib/xmodule/xmodule/video_module.py +++ b/common/lib/xmodule/xmodule/video_module.py @@ -97,7 +97,6 @@ class VideoDescriptor(VideoFields, MetadataOnlyEditingDescriptor, RawDescriptor): module_class = VideoModule - template_dir_name = "video" def __init__(self, *args, **kwargs): super(VideoDescriptor, self).__init__(*args, **kwargs) diff --git a/common/lib/xmodule/xmodule/videoalpha_module.py b/common/lib/xmodule/xmodule/videoalpha_module.py index 33945c33fc9..d8ed8949f1d 100644 --- a/common/lib/xmodule/xmodule/videoalpha_module.py +++ b/common/lib/xmodule/xmodule/videoalpha_module.py @@ -179,4 +179,3 @@ def get_html(self): class VideoAlphaDescriptor(VideoAlphaFields, RawDescriptor): """Descriptor for `VideoAlphaModule`.""" module_class = VideoAlphaModule - template_dir_name = "videoalpha" diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py index 0528bbfb6cc..882e308c77d 100644 --- a/common/lib/xmodule/xmodule/xml_module.py +++ b/common/lib/xmodule/xmodule/xml_module.py @@ -356,6 +356,7 @@ def from_xml(cls, xml_data, system, org=None, course=None): if key not in set(f.name for f in cls.fields + cls.lms.fields): model_data['xml_attributes'][key] = value model_data['location'] = location + model_data['category'] = xml_object.tag return cls( system, diff --git a/lms/djangoapps/courseware/features/common.py b/lms/djangoapps/courseware/features/common.py index 0aa079ebacc..55e82e0e909 100644 --- a/lms/djangoapps/courseware/features/common.py +++ b/lms/djangoapps/courseware/features/common.py @@ -10,7 +10,6 @@ from student.models import CourseEnrollment from xmodule.modulestore import Location from xmodule.modulestore.django import _MODULESTORES, modulestore -from xmodule.templates import update_templates from xmodule.course_module import CourseDescriptor from courseware.courses import get_course_by_id from xmodule import seq_module, vertical_module @@ -39,7 +38,7 @@ def create_course(step, course): display_name='Test Section') problem_section = world.ItemFactory.create(parent_location=world.scenario_dict['SECTION'].location, - template='i4x://edx/templates/sequential/Empty', + category='sequential' display_name='Test Section') @@ -62,7 +61,7 @@ def i_am_registered_for_the_course(step, course): @step(u'The course "([^"]*)" has extra tab "([^"]*)"$') def add_tab_to_course(step, course, extra_tab_name): section_item = world.ItemFactory.create(parent_location=course_location(course), - template="i4x://edx/templates/static_tab/Empty", + category="static_tab", display_name=str(extra_tab_name)) diff --git a/lms/djangoapps/courseware/features/navigation.py b/lms/djangoapps/courseware/features/navigation.py index 7c2474ae1a1..c87e6122a47 100644 --- a/lms/djangoapps/courseware/features/navigation.py +++ b/lms/djangoapps/courseware/features/navigation.py @@ -24,11 +24,11 @@ def view_course_multiple_sections(step): display_name=section_name(2)) place1 = world.ItemFactory.create(parent_location=section1.location, - template='i4x://edx/templates/sequential/Empty', + category='sequential', display_name=subsection_name(1)) place2 = world.ItemFactory.create(parent_location=section2.location, - template='i4x://edx/templates/sequential/Empty', + category='sequential', display_name=subsection_name(2)) add_problem_to_course_section('model_course', 'multiple choice', place1.location) @@ -46,7 +46,7 @@ def view_course_multiple_subsections(step): display_name=section_name(1)) place1 = world.ItemFactory.create(parent_location=section1.location, - template='i4x://edx/templates/sequential/Empty', + category='sequential', display_name=subsection_name(1)) place2 = world.ItemFactory.create(parent_location=section1.location, @@ -66,7 +66,7 @@ def view_course_multiple_sequences(step): display_name=section_name(1)) place1 = world.ItemFactory.create(parent_location=section1.location, - template='i4x://edx/templates/sequential/Empty', + category='sequential', display_name=subsection_name(1)) add_problem_to_course_section('model_course', 'multiple choice', place1.location) @@ -177,9 +177,8 @@ def add_problem_to_course_section(course, problem_type, parent_location, extraMe # Create a problem item using our generated XML # We set rerandomize=always in the metadata so that the "Reset" button # will appear. - template_name = "i4x://edx/templates/problem/Blank_Common_Problem" world.ItemFactory.create(parent_location=parent_location, - template=template_name, + category='problem', display_name=str(problem_type), data=problem_xml, metadata=metadata) diff --git a/lms/djangoapps/courseware/features/problems.py b/lms/djangoapps/courseware/features/problems.py index 82bb4959a80..e0c3c004da9 100644 --- a/lms/djangoapps/courseware/features/problems.py +++ b/lms/djangoapps/courseware/features/problems.py @@ -17,7 +17,7 @@ def view_problem_with_attempts(step, problem_type, attempts): i_am_registered_for_the_course(step, 'model_course') # Ensure that the course has this problem type - add_problem_to_course(world.scenario_dict['COURSE'].number, problem_type, {'attempts': attempts}) + add_problem_to_course(world.scenario_dict['COURSE'].number, problem_type, {'max_attempts': attempts}) # Go to the one section in the factory-created course # which should be loaded with the correct problem diff --git a/lms/djangoapps/courseware/features/problems_setup.py b/lms/djangoapps/courseware/features/problems_setup.py index 1805da55d0b..6086d7fa5e3 100644 --- a/lms/djangoapps/courseware/features/problems_setup.py +++ b/lms/djangoapps/courseware/features/problems_setup.py @@ -273,9 +273,9 @@ def add_problem_to_course(course, problem_type, extraMeta=None): # Create a problem item using our generated XML # We set rerandomize=always in the metadata so that the "Reset" button # will appear. - template_name = "i4x://edx/templates/problem/Blank_Common_Problem" - world.ItemFactory.create(parent_location=section_location(course), - template=template_name, + category_name = "problem" + return world.ItemFactory.create(parent_location=section_location(course), + category=category_name, display_name=str(problem_type), data=problem_xml, metadata=metadata) diff --git a/lms/djangoapps/courseware/features/video.py b/lms/djangoapps/courseware/features/video.py index 6b05af51b5e..f95ffd9917d 100644 --- a/lms/djangoapps/courseware/features/video.py +++ b/lms/djangoapps/courseware/features/video.py @@ -43,14 +43,13 @@ def view_videoalpha(step): def add_video_to_course(course): - template_name = 'i4x://edx/templates/video/default' world.ItemFactory.create(parent_location=section_location(course), - template=template_name, + category='video', display_name='Video') def add_videoalpha_to_course(course): - template_name = 'i4x://edx/templates/videoalpha/Video_Alpha' + category = 'videoalpha' world.ItemFactory.create(parent_location=section_location(course), - template=template_name, + category=category, display_name='Video Alpha') diff --git a/lms/djangoapps/courseware/tests/__init__.py b/lms/djangoapps/courseware/tests/__init__.py index 0abbaa02cf1..31fe376d694 100644 --- a/lms/djangoapps/courseware/tests/__init__.py +++ b/lms/djangoapps/courseware/tests/__init__.py @@ -29,17 +29,17 @@ class BaseTestXmodule(ModuleStoreTestCase): 2. create, enrol and login users for this course; Any xmodule should overwrite only next parameters for test: - 1. TEMPLATE_NAME + 1. CATEGORY 2. DATA 3. MODEL_DATA - This class should not contain any tests, because TEMPLATE_NAME + This class should not contain any tests, because CATEGORY should be defined in child class. """ USER_COUNT = 2 # Data from YAML common/lib/xmodule/xmodule/templates/NAME/default.yaml - TEMPLATE_NAME = "" + CATEGORY = "" DATA = '' MODEL_DATA = {'data': ''} @@ -53,11 +53,11 @@ def setUp(self): chapter = ItemFactory.create( parent_location=self.course.location, - template="i4x://edx/templates/sequential/Empty", + category="sequential", ) section = ItemFactory.create( parent_location=chapter.location, - template="i4x://edx/templates/sequential/Empty" + category="sequential" ) # username = robot{0}, password = 'test' @@ -71,7 +71,7 @@ def setUp(self): self.item_descriptor = ItemFactory.create( parent_location=section.location, - template=self.TEMPLATE_NAME, + category=self.CATEGORY, data=self.DATA ) diff --git a/lms/djangoapps/courseware/tests/test_submitting_problems.py b/lms/djangoapps/courseware/tests/test_submitting_problems.py index 83ae7dc73e7..7e9b55a4fba 100644 --- a/lms/djangoapps/courseware/tests/test_submitting_problems.py +++ b/lms/djangoapps/courseware/tests/test_submitting_problems.py @@ -130,7 +130,7 @@ def add_dropdown_to_section(self, section_location, name, num_inputs=2): problem = ItemFactory.create( parent_location=section_location, - template=problem_template, + category='problem', data=prob_xml, metadata={'randomize': 'always'}, display_name=name @@ -149,13 +149,13 @@ def add_graded_section_to_course(self, name, section_format='Homework'): if not(hasattr(self, 'chapter')): self.chapter = ItemFactory.create( parent_location=self.course.location, - template="i4x://edx/templates/chapter/Empty", + category='chapter' ) section = ItemFactory.create( parent_location=self.chapter.location, display_name=name, - template="i4x://edx/templates/sequential/Empty", + category='sequential', metadata={'graded': True, 'format': section_format} ) diff --git a/lms/djangoapps/courseware/tests/test_video_mongo.py b/lms/djangoapps/courseware/tests/test_video_mongo.py index a0fdecc77a4..829308423c2 100644 --- a/lms/djangoapps/courseware/tests/test_video_mongo.py +++ b/lms/djangoapps/courseware/tests/test_video_mongo.py @@ -7,7 +7,7 @@ class TestVideo(BaseTestXmodule): """Integration tests: web client + mongo.""" - TEMPLATE_NAME = "i4x://edx/templates/video/default" + TEMPLATE_NAME = "video" DATA = '