Merge pull request #9208 from open-craft/smarnach/course-masquerade-session-fix

Make new masquerading code compatible to existing session data.

lms/djangoapps/courseware/masquerade.py

@@ -34,12 +34,25 @@ class CourseMasquerade(object):
     Masquerade settings for a particular course.
     """
     def __init__(self, course_key, role='student', user_partition_id=None, group_id=None, user_name=None):
+        # All parameters to this function must be named identically to the corresponding attribute.
+        # If you remove or rename an attribute, also update the __setstate__() method to migrate
+        # old data from users' sessions.
         self.course_key = course_key
         self.role = role
         self.user_partition_id = user_partition_id
         self.group_id = group_id
         self.user_name = user_name
 
+    def __setstate__(self, state):
+        """
+        Ensure that all attributes are initialised when unpickling CourseMasquerade objects.
+
+        Users might still have CourseMasquerade objects from older versions of the code in their
+        session.  These old objects might not have all attributes set, possibly resulting in
+        AttributeErrors.
+        """
+        self.__init__(**state)
+
 
 @require_POST
 @login_required

lms/djangoapps/courseware/tests/test_masquerade.py

@@ -2,6 +2,7 @@
 Unit tests for masquerade.
 """
 import json
+import pickle
 from mock import patch
 from nose.plugins.attrib import attr
 from datetime import datetime
@@ -12,6 +13,7 @@ from django.utils.timezone import UTC
 
 from capa.tests.response_xml_factory import OptionResponseXMLFactory
 from courseware.masquerade import (
+    CourseMasquerade,
     MasqueradingKeyValueStore,
     handle_ajax,
     setup_masquerade,
@@ -394,3 +396,19 @@ class MasqueradingKeyValueStoreTest(TestCase):
                 self.kvs.get(key)
 
         self.assertEqual(self.kvs.get('c'), 'OpenCraft')
+
+
+class CourseMasqueradeTest(TestCase):
+    """
+    Unit tests for the CourseMasquerade class.
+    """
+    def test_unpickling_sets_all_attributes(self):
+        """
+        Make sure that old CourseMasquerade objects receive missing attributes when unpickled from
+        the session.
+        """
+        cmasq = CourseMasquerade(7)
+        del cmasq.user_name
+        pickled_cmasq = pickle.dumps(cmasq)
+        unpickled_cmasq = pickle.loads(pickled_cmasq)
+        self.assertEqual(unpickled_cmasq.user_name, None)