From 55ace6baa8711faff744de1d8d936ba1e2e15b83 Mon Sep 17 00:00:00 2001 From: Sven Marnach Date: Tue, 4 Aug 2015 12:43:10 +0200 Subject: [PATCH] Make new masquerading code compatible to existing session data. The new masquerading code introduced in PR #8775 adds a new attribute to CourseMasquerade objects stored in the user's session. When users who have active masquerading configuration access instances with the new code, and AttributeError occurs when trying to access the attribute user_name. This fix ensures that CourseMasquerade objects receive all required attributes when they are unpickled from the session. --- lms/djangoapps/courseware/masquerade.py | 13 +++++++++++++ .../courseware/tests/test_masquerade.py | 18 ++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/lms/djangoapps/courseware/masquerade.py b/lms/djangoapps/courseware/masquerade.py index 5c244f18f7..f26fe2d6e6 100644 --- a/lms/djangoapps/courseware/masquerade.py +++ b/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 diff --git a/lms/djangoapps/courseware/tests/test_masquerade.py b/lms/djangoapps/courseware/tests/test_masquerade.py index da52b370b4..6b58f2b857 100644 --- a/lms/djangoapps/courseware/tests/test_masquerade.py +++ b/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)