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 @@

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. -
    3. Another item
    4. -
    5. Yet another item
    6. -
    -

    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 = '