diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 8c321f03dd..d52330f5ff 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -1763,6 +1763,26 @@ class ContentStoreTest(ModuleStoreTestCase): def test_import_into_new_course_id_wiki_slug_renamespacing(self): module_store = modulestore('direct') + + # If reimporting into the same course do not change the wiki_slug. + target_location = Location('i4x', 'edX', 'toy', 'course', '2012_Fall') + course_data = { + 'org': target_location.org, + 'number': target_location.course, + 'display_name': 'Robot Super Course', + 'run': target_location.name + } + _create_course(self, course_data) + course_module = module_store.get_instance(target_location.course_id, target_location) + course_module.wiki_slug = 'toy' + course_module.save() + + # Import a course with wiki_slug == location.course + import_from_xml(module_store, 'common/test/data/', ['toy'], target_location_namespace=target_location) + course_module = module_store.get_instance(target_location.course_id, target_location) + self.assertEquals(course_module.wiki_slug, 'toy') + + # But change the wiki_slug if it is a different course. target_location = Location('i4x', 'MITx', '999', 'course', '2013_Spring') course_data = { 'org': target_location.org, @@ -1770,7 +1790,6 @@ class ContentStoreTest(ModuleStoreTestCase): 'display_name': 'Robot Super Course', 'run': target_location.name } - target_course_id = '{0}/{1}/{2}'.format(target_location.org, target_location.course, target_location.name) _create_course(self, course_data) # Import a course with wiki_slug == location.course @@ -1778,9 +1797,9 @@ class ContentStoreTest(ModuleStoreTestCase): course_module = module_store.get_instance(target_location.course_id, target_location) self.assertEquals(course_module.wiki_slug, 'MITx.999.2013_Spring') - # Now try importing a course with wiki_slug == '{0}{1}{2}'.format(location.org, location.course, location.name) + # Now try importing a course with wiki_slug == '{0}.{1}.{2}'.format(location.org, location.course, location.name) import_from_xml(module_store, 'common/test/data/', ['two_toys'], target_location_namespace=target_location) - course_module = module_store.get_instance(target_course_id, target_location) + course_module = module_store.get_instance(target_location.course_id, target_location) self.assertEquals(course_module.wiki_slug, 'MITx.999.2013_Spring') def test_import_metadata_with_attempts_empty_string(self): diff --git a/common/lib/xmodule/xmodule/modulestore/tests/factories.py b/common/lib/xmodule/xmodule/modulestore/tests/factories.py index 6fe138e05c..8f4cf1ffa8 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/factories.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/factories.py @@ -56,6 +56,8 @@ class CourseFactory(XModuleFactory): for k, v in kwargs.iteritems(): setattr(new_course, k, v) + # Save the attributes we just set + new_course.save() # Update the data in the mongo datastore store.update_item(new_course) return new_course @@ -156,6 +158,7 @@ class ItemFactory(XModuleFactory): for attr, val in kwargs.items(): setattr(module, attr, val) + # Save the attributes we just set module.save() store.update_item(module) diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index cd82f97aec..d398f5d931 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -515,19 +515,20 @@ def remap_namespace(module, target_location_namespace): ) # Original wiki_slugs had value location.course. To make them unique this was changed to 'org.course.name'. - # If the wiki_slug is equal to either of these default values then remap that so that the wiki does not point - # to the old wiki. - original_unique_wiki_slug = '{0}.{1}.{2}'.format( - original_location.org, - original_location.course, - original_location.name - ) - if module.wiki_slug == original_unique_wiki_slug or module.wiki_slug == original_location.course: - module.wiki_slug = '{0}.{1}.{2}'.format( - target_location_namespace.org, - target_location_namespace.course, - target_location_namespace.name, + # If we are importing into a course with a different course_id and wiki_slug is equal to either of these default + # values then remap it so that the wiki does not point to the old wiki. + if original_location.course_id != target_location_namespace.course_id: + original_unique_wiki_slug = '{0}.{1}.{2}'.format( + original_location.org, + original_location.course, + original_location.name ) + if module.wiki_slug == original_unique_wiki_slug or module.wiki_slug == original_location.course: + module.wiki_slug = '{0}.{1}.{2}'.format( + target_location_namespace.org, + target_location_namespace.course, + target_location_namespace.name, + ) module.save() diff --git a/common/lib/xmodule/xmodule/partitions/partitions.py b/common/lib/xmodule/xmodule/partitions/partitions.py index e5d78b79e8..cd6b7cbcfa 100644 --- a/common/lib/xmodule/xmodule/partitions/partitions.py +++ b/common/lib/xmodule/xmodule/partitions/partitions.py @@ -1,11 +1,11 @@ """Defines ``Group`` and ``UserPartition`` models for partitioning""" - +from collections import namedtuple # We use ``id`` in this file as the IDs of our Groups and UserPartitions, # which Pylint disapproves of. # pylint: disable=invalid-name, redefined-builtin -class Group(object): +class Group(namedtuple("Group", "id name")): """ An id and name for a group of students. The id should be unique within the UserPartition this group appears in. @@ -14,9 +14,9 @@ class Group(object): # for deserializing old versions. (This will be serialized in courses) VERSION = 1 - def __init__(self, id, name): - self.id = int(id) - self.name = name + def __new__(cls, id, name): + # pylint: disable=super-on-old-class + return super(Group, cls).__new__(cls, int(id), name) def to_json(self): """ @@ -25,6 +25,7 @@ class Group(object): Returns: a dictionary with keys for the properties of the group. """ + # pylint: disable=no-member return { "id": self.id, "name": self.name, @@ -53,7 +54,7 @@ class Group(object): return Group(value["id"], value["name"]) -class UserPartition(object): +class UserPartition(namedtuple("UserPartition", "id name description groups")): """ A named way to partition users into groups, primarily intended for running experiments. It is expected that each user will be in at most one group in a @@ -65,12 +66,9 @@ class UserPartition(object): """ VERSION = 1 - def __init__(self, id, name, description, groups): - - self.id = int(id) - self.name = name - self.description = description - self.groups = groups + def __new__(cls, id, name, description, groups): + # pylint: disable=super-on-old-class + return super(UserPartition, cls).__new__(cls, int(id), name, description, groups) def to_json(self): """ @@ -79,6 +77,7 @@ class UserPartition(object): Returns: a dictionary with keys for the properties of the partition. """ + # pylint: disable=no-member return { "id": self.id, "name": self.name, diff --git a/lms/djangoapps/courseware/tests/test_split_module.py b/lms/djangoapps/courseware/tests/test_split_module.py index 958712476e..6c7731fa23 100644 --- a/lms/djangoapps/courseware/tests/test_split_module.py +++ b/lms/djangoapps/courseware/tests/test_split_module.py @@ -3,9 +3,12 @@ Test for split test XModule """ from django.core.urlresolvers import reverse from django.test.utils import override_settings +from mock import MagicMock from student.tests.factories import UserFactory, CourseEnrollmentFactory from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE +from courseware.module_render import get_module_for_descriptor +from courseware.model_data import FieldDataCache from xmodule.modulestore.tests.factories import ItemFactory, CourseFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase @@ -267,3 +270,49 @@ class TestSplitTestVert(SplitTestBase): ) video1 = self._video(cond1vert, 1) html1 = self._html(cond1vert, 1) + + +@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) +class SplitTestPosition(ModuleStoreTestCase): + """ + Check that we can change positions in a course with partitions defined + """ + def setUp(self): + self.partition = UserPartition( + 0, + 'first_partition', + 'First Partition', + [ + Group(0, 'alpha'), + Group(1, 'beta') + ] + ) + + self.course = CourseFactory.create( + user_partitions=[self.partition] + ) + + self.chapter = ItemFactory.create( + parent_location=self.course.location, + category="chapter", + display_name="test chapter", + ) + + self.student = UserFactory.create() + CourseEnrollmentFactory.create(user=self.student, course_id=self.course.id) + self.client.login(username=self.student.username, password='test') + + def test_changing_position_works(self): + # Make a mock FieldDataCache for this course, so we can get the course module + mock_field_data_cache = FieldDataCache([self.course], self.course.id, self.student) + course = get_module_for_descriptor( + self.student, + MagicMock(name='request'), + self.course, + mock_field_data_cache, + self.course.id + ) + + # Now that we have the course, change the position and save, nothing should explode! + course.position = 2 + course.save()