Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use XBlock 0.3 #839

Merged
merged 1 commit into from
Sep 6, 2013
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions cms/djangoapps/contentstore/module_info_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,11 @@ def set_module_info(store, location, post_data):

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 module._model_data:
del module._model_data[metadata_key]
if module._field_data.has(module, metadata_key):
module._field_data.delete(module, metadata_key)
del posted_metadata[metadata_key]
else:
module._model_data[metadata_key] = value
module._field_data.set(module, metadata_key, value)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Saw this in Don's PR - why does _field_data have a leading underscore when it's being used so much outside of its class? If nothing else, you're probably going to have to deal with a bunch of pylint violations for that...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I mentioned there: It's not intended for public consumption. Generally, external code should just be using the field attributes.


# commit to datastore
# TODO (cpennington): This really shouldn't have to do this much reaching in to get the metadata
Expand Down
51 changes: 26 additions & 25 deletions cms/djangoapps/contentstore/tests/test_contentstore.py
Original file line number Diff line number Diff line change
Expand Up @@ -219,15 +219,15 @@ def test_draft_metadata(self):
'course', '2012_Fall', None]), depth=None)
html_module = draft_store.get_item(['i4x', 'edX', 'simple', 'html', 'test_html', None])

self.assertEqual(html_module.lms.graceperiod, course.lms.graceperiod)
self.assertEqual(html_module.graceperiod, course.graceperiod)
self.assertNotIn('graceperiod', own_metadata(html_module))

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])

self.assertEqual(html_module.lms.graceperiod, course.lms.graceperiod)
self.assertEqual(html_module.graceperiod, course.graceperiod)
self.assertNotIn('graceperiod', own_metadata(html_module))

# publish module
Expand All @@ -236,7 +236,7 @@ def test_draft_metadata(self):
# refetch to check metadata
html_module = draft_store.get_item(['i4x', 'edX', 'simple', 'html', 'test_html', None])

self.assertEqual(html_module.lms.graceperiod, course.lms.graceperiod)
self.assertEqual(html_module.graceperiod, course.graceperiod)
self.assertNotIn('graceperiod', own_metadata(html_module))

# put back in draft and change metadata and see if it's now marked as 'own_metadata'
Expand All @@ -246,20 +246,20 @@ def test_draft_metadata(self):
new_graceperiod = timedelta(hours=1)

self.assertNotIn('graceperiod', own_metadata(html_module))
html_module.lms.graceperiod = new_graceperiod
html_module.graceperiod = new_graceperiod
# Save the data that we've just changed to the underlying
# MongoKeyValueStore before we update the mongo datastore.
html_module.save()
self.assertIn('graceperiod', own_metadata(html_module))
self.assertEqual(html_module.lms.graceperiod, new_graceperiod)
self.assertEqual(html_module.graceperiod, new_graceperiod)

draft_store.update_metadata(html_module.location, own_metadata(html_module))

# read back to make sure it reads as 'own-metadata'
html_module = draft_store.get_item(['i4x', 'edX', 'simple', 'html', 'test_html', None])

self.assertIn('graceperiod', own_metadata(html_module))
self.assertEqual(html_module.lms.graceperiod, new_graceperiod)
self.assertEqual(html_module.graceperiod, new_graceperiod)

# republish
draft_store.publish(html_module.location, 0)
Expand All @@ -269,7 +269,7 @@ def test_draft_metadata(self):
html_module = draft_store.get_item(['i4x', 'edX', 'simple', 'html', 'test_html', None])

self.assertIn('graceperiod', own_metadata(html_module))
self.assertEqual(html_module.lms.graceperiod, new_graceperiod)
self.assertEqual(html_module.graceperiod, new_graceperiod)

def test_get_depth_with_drafts(self):
import_from_xml(modulestore('direct'), 'common/test/data/', ['simple'])
Expand Down Expand Up @@ -696,7 +696,7 @@ def test_clone_course(self):

# we want to assert equality between the objects, but we know the locations
# differ, so just make them equal for testing purposes
source_item.location = new_loc
source_item.scope_ids = source_item.scope_ids._replace(def_id=new_loc, usage_id=new_loc)
if hasattr(source_item, 'data') and hasattr(lookup_item, 'data'):
self.assertEqual(source_item.data, lookup_item.data)

Expand Down Expand Up @@ -877,7 +877,9 @@ def test_export_course(self, mock_get):
depth=1
)
# We had a bug where orphaned draft nodes caused export to fail. This is here to cover that case.
vertical.location = mongo.draft.as_draft(vertical.location.replace(name='no_references'))
draft_loc = mongo.draft.as_draft(vertical.location.replace(name='no_references'))
vertical.scope_ids = vertical.scope_ids._replace(def_id=draft_loc, usage_id=draft_loc)

draft_store.save_xmodule(vertical)
orphan_vertical = draft_store.get_item(vertical.location)
self.assertEqual(orphan_vertical.location.name, 'no_references')
Expand All @@ -894,7 +896,8 @@ def test_export_course(self, mock_get):
root_dir = path(mkdtemp_clean())

# 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_loc = mongo.draft.as_draft(Location(['i4x', 'edX', 'toy', 'vertical', 'a_private_vertical', None]))
vertical.scope_ids = vertical.scope_ids._replace(def_id=draft_loc, usage_id=draft_loc)
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
Expand Down Expand Up @@ -965,7 +968,7 @@ def test_export_course(self, mock_get):
self.assertTrue(getattr(vertical, 'is_draft', False))
self.assertNotIn('index_in_children_list', child.xml_attributes)
self.assertNotIn('parent_sequential_url', vertical.xml_attributes)

for child in vertical.get_children():
self.assertTrue(getattr(child, 'is_draft', False))
self.assertNotIn('index_in_children_list', child.xml_attributes)
Expand Down Expand Up @@ -1628,8 +1631,8 @@ 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.assertEqual(course.xqa_key, vertical.xqa_key)
self.assertEqual(course.start, vertical.start)

self.assertGreater(len(verticals), 0)

Expand All @@ -1645,32 +1648,32 @@ def test_metadata_inheritance(self):
new_module = module_store.get_item(new_component_location)

# 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(parent.graceperiod, new_module.graceperiod)
self.assertEqual(parent.start, new_module.start)
self.assertEqual(course.start, new_module.start)

self.assertEqual(course.lms.xqa_key, new_module.lms.xqa_key)
self.assertEqual(course.xqa_key, new_module.xqa_key)

#
# now let's define an override at the leaf node level
#
new_module.lms.graceperiod = timedelta(1)
new_module.graceperiod = timedelta(1)
new_module.save()
module_store.update_metadata(new_module.location, own_metadata(new_module))

# flush the cache and refetch
module_store.refresh_cached_metadata_inheritance_tree(new_component_location)
new_module = module_store.get_item(new_component_location)

self.assertEqual(timedelta(1), new_module.lms.graceperiod)
self.assertEqual(timedelta(1), new_module.graceperiod)

def test_default_metadata_inheritance(self):
course = CourseFactory.create()
vertical = ItemFactory.create(parent_location=course.location)
course.children.append(vertical)
# in memory
self.assertIsNotNone(course.start)
self.assertEqual(course.start, vertical.lms.start)
self.assertEqual(course.start, vertical.start)
self.assertEqual(course.textbooks, [])
self.assertIn('GRADER', course.grading_policy)
self.assertIn('GRADE_CUTOFFS', course.grading_policy)
Expand All @@ -1682,7 +1685,7 @@ def test_default_metadata_inheritance(self):
fetched_item = module_store.get_item(vertical.location)
self.assertIsNotNone(fetched_course.start)
self.assertEqual(course.start, fetched_course.start)
self.assertEqual(fetched_course.start, fetched_item.lms.start)
self.assertEqual(fetched_course.start, fetched_item.start)
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)
Expand Down Expand Up @@ -1755,12 +1758,10 @@ def test_metadata_not_persistence(self):
'track'
}

fields = self.video_descriptor.fields
location = self.video_descriptor.location

for field in fields:
if field.name in attrs_to_strip:
field.delete_from(self.video_descriptor)
for field_name in attrs_to_strip:
delattr(self.video_descriptor, field_name)

self.assertNotIn('html5_sources', own_metadata(self.video_descriptor))
get_modulestore(location).update_metadata(
Expand Down
12 changes: 6 additions & 6 deletions cms/djangoapps/contentstore/tests/test_course_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -343,26 +343,26 @@ def test_update_section_grader_type(self):
section_grader_type = CourseGradingModel.get_section_grader_type(self.course.location)

self.assertEqual('Not Graded', section_grader_type['graderType'])
self.assertEqual(None, descriptor.lms.format)
self.assertEqual(False, descriptor.lms.graded)
self.assertEqual(None, descriptor.format)
self.assertEqual(False, descriptor.graded)

# Change the default grader type to Homework, which should also mark the section as graded
CourseGradingModel.update_section_grader_type(self.course.location, {'graderType': 'Homework'})
descriptor = get_modulestore(self.course.location).get_item(self.course.location)
section_grader_type = CourseGradingModel.get_section_grader_type(self.course.location)

self.assertEqual('Homework', section_grader_type['graderType'])
self.assertEqual('Homework', descriptor.lms.format)
self.assertEqual(True, descriptor.lms.graded)
self.assertEqual('Homework', descriptor.format)
self.assertEqual(True, descriptor.graded)

# Change the grader type back to Not Graded, which should also unmark the section as graded
CourseGradingModel.update_section_grader_type(self.course.location, {'graderType': 'Not Graded'})
descriptor = get_modulestore(self.course.location).get_item(self.course.location)
section_grader_type = CourseGradingModel.get_section_grader_type(self.course.location)

self.assertEqual('Not Graded', section_grader_type['graderType'])
self.assertEqual(None, descriptor.lms.format)
self.assertEqual(False, descriptor.lms.graded)
self.assertEqual(None, descriptor.format)
self.assertEqual(False, descriptor.graded)


class CourseMetadataEditingTest(CourseTestCase):
Expand Down
14 changes: 5 additions & 9 deletions cms/djangoapps/contentstore/tests/test_crud.py
Original file line number Diff line number Diff line change
Expand Up @@ -220,18 +220,14 @@ def load_from_json(json_data, system, default_class=None, parent_xblock=None):
if not '_inherited_settings' in json_data and parent_xblock is not None:
json_data['_inherited_settings'] = parent_xblock.xblock_kvs.get_inherited_settings().copy()
json_fields = json_data.get('fields', {})
for field in inheritance.INHERITABLE_METADATA:
if field in json_fields:
json_data['_inherited_settings'][field] = json_fields[field]
for field_name in inheritance.InheritanceMixin.fields:
if field_name in json_fields:
json_data['_inherited_settings'][field_name] = json_fields[field_name]

new_block = system.xblock_from_json(class_, usage_id, json_data)
if parent_xblock is not None:
children = parent_xblock.children
children.append(new_block)
# trigger setter method by using top level field access
parent_xblock.children = children
# decache pending children field settings (Note, truly persisting at this point would break b/c
# persistence assumes children is a list of ids not actual xblocks)
parent_xblock.children.append(new_block.scope_ids.usage_id)
# decache pending children field settings
parent_xblock.save()
return new_block

6 changes: 3 additions & 3 deletions cms/djangoapps/contentstore/tests/test_import_nostatic.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,9 @@ def test_static_import(self):

self.assertIsNotNone(content)

# make sure course.lms.static_asset_path is correct
print "static_asset_path = {0}".format(course.lms.static_asset_path)
self.assertEqual(course.lms.static_asset_path, 'test_import_course')
# make sure course.static_asset_path is correct
print "static_asset_path = {0}".format(course.static_asset_path)
self.assertEqual(course.static_asset_path, 'test_import_course')

def test_asset_import_nostatic(self):
'''
Expand Down
12 changes: 6 additions & 6 deletions cms/djangoapps/contentstore/tests/test_item.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from contentstore.tests.test_course_settings import CourseTestCase
from contentstore.tests.utils import CourseTestCase
from xmodule.modulestore.tests.factories import CourseFactory
from django.core.urlresolvers import reverse
from xmodule.capa_module import CapaDescriptor
Expand Down Expand Up @@ -69,7 +69,7 @@ def test_create_nicely(self):
# 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.scope_ids.block_type, 'chapter')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to figure out these new scope_ids. I'll hunt around xblock code for it.

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)
Expand Down Expand Up @@ -226,7 +226,7 @@ def test_date_fields(self):
Test setting due & start dates on sequential
"""
sequential = modulestore().get_item(self.seq_location)
self.assertIsNone(sequential.lms.due)
self.assertIsNone(sequential.due)
self.client.post(
reverse('save_item'),
json.dumps({
Expand All @@ -236,7 +236,7 @@ def test_date_fields(self):
content_type="application/json"
)
sequential = modulestore().get_item(self.seq_location)
self.assertEqual(sequential.lms.due, datetime.datetime(2010, 11, 22, 4, 0, tzinfo=UTC))
self.assertEqual(sequential.due, datetime.datetime(2010, 11, 22, 4, 0, tzinfo=UTC))
self.client.post(
reverse('save_item'),
json.dumps({
Expand All @@ -246,5 +246,5 @@ def test_date_fields(self):
content_type="application/json"
)
sequential = modulestore().get_item(self.seq_location)
self.assertEqual(sequential.lms.due, datetime.datetime(2010, 11, 22, 4, 0, tzinfo=UTC))
self.assertEqual(sequential.lms.start, datetime.datetime(2010, 9, 12, 14, 0, tzinfo=UTC))
self.assertEqual(sequential.due, datetime.datetime(2010, 11, 22, 4, 0, tzinfo=UTC))
self.assertEqual(sequential.start, datetime.datetime(2010, 9, 12, 14, 0, tzinfo=UTC))
Loading