From 119ffd4677ffc66b51cf9cc2a9a8e021bfce2701 Mon Sep 17 00:00:00 2001
From: Sarina Canelake <sarina@edx.org>
Date: Tue, 11 Mar 2014 10:03:33 -0400
Subject: [PATCH] Bugfix for split test on sequential save

Bug would occur when comparing Groups or UserPartitions - because these types
lacked comparator methods, different objects would come up as different when
their values were equal.
---
 .../xmodule/xmodule/partitions/partitions.py  | 23 +++++----
 .../courseware/tests/test_split_module.py     | 49 +++++++++++++++++++
 2 files changed, 60 insertions(+), 12 deletions(-)

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