Merge pull request #7974 from edx/merge-release-to-master

Merge release to master

lms/djangoapps/courseware/module_render.py

@@ -497,15 +497,16 @@ def get_module_system_for_user(user, field_data_cache,
             user_location=user_location,
             request_token=request_token
         )
-        # rebinds module to a different student.  We'll change system, student_data, and scope_ids
-        authored_data = OverrideFieldData.wrap(
-            real_user, module.descriptor._field_data  # pylint: disable=protected-access
-        )
+
         module.descriptor.bind_for_student(
             inner_system,
-            LmsFieldData(authored_data, inner_student_data),
             real_user.id,
+            [
+                partial(OverrideFieldData.wrap, real_user),
+                partial(LmsFieldData, student_data=inner_student_data),
+            ],
         )
+
         module.descriptor.scope_ids = (
             module.descriptor.scope_ids._replace(user_id=real_user.id)  # pylint: disable=protected-access
         )
@@ -692,8 +693,15 @@ def get_module_for_descriptor_internal(user, descriptor, field_data_cache, cours
         request_token=request_token
     )
 
-    authored_data = OverrideFieldData.wrap(user, descriptor._field_data)  # pylint: disable=protected-access
-    descriptor.bind_for_student(system, LmsFieldData(authored_data, student_data), user.id)
+    descriptor.bind_for_student(
+        system,
+        user.id,
+        [
+            partial(OverrideFieldData.wrap, user),
+            partial(LmsFieldData, student_data=student_data),
+        ],
+    )
+
     descriptor.scope_ids = descriptor.scope_ids._replace(user_id=user.id)  # pylint: disable=protected-access
 
     # Do not check access when it's a noauth request.

lms/djangoapps/courseware/tests/test_module_render.py

@@ -13,6 +13,7 @@ from django.http import Http404, HttpResponse
 from django.core.urlresolvers import reverse
 from django.conf import settings
 from django.test.client import RequestFactory
+from django.test.utils import override_settings
 from django.contrib.auth.models import AnonymousUser
 from mock import MagicMock, patch, Mock
 from opaque_keys.edx.keys import UsageKey, CourseKey
@@ -28,6 +29,7 @@ from xblock.fragment import Fragment
 from capa.tests.response_xml_factory import OptionResponseXMLFactory
 from courseware import module_render as render
 from courseware.courses import get_course_with_access, course_image_url, get_course_info_section
+from courseware.field_overrides import OverrideFieldData
 from courseware.model_data import FieldDataCache
 from courseware.module_render import hash_resource, get_module_for_descriptor
 from courseware.models import StudentModule
@@ -35,6 +37,7 @@ from courseware.tests.factories import StudentModuleFactory, UserFactory, Global
 from courseware.tests.tests import LoginEnrollmentTestCase
 from courseware.tests.test_submitting_problems import TestSubmittingProblems
 from lms.djangoapps.lms_xblock.runtime import quote_slashes
+from lms.djangoapps.lms_xblock.field_data import LmsFieldData
 from student.models import anonymous_id_for_user
 from xmodule.modulestore.tests.django_utils import (
     TEST_DATA_MIXED_TOY_MODULESTORE,
@@ -101,10 +104,15 @@ class ModuleRenderTestCase(ModuleStoreTestCase, LoginEnrollmentTestCase):
         self.dispatch = 'score_update'
 
         # Construct a 'standard' xqueue_callback url
-        self.callback_url = reverse('xqueue_callback', kwargs=dict(course_id=self.course_key.to_deprecated_string(),
-                                                                   userid=str(self.mock_user.id),
-                                                                   mod_id=self.mock_module.id,
-                                                                   dispatch=self.dispatch))
+        self.callback_url = reverse(
+            'xqueue_callback',
+            kwargs=dict(
+                course_id=self.course_key.to_deprecated_string(),
+                userid=str(self.mock_user.id),
+                mod_id=self.mock_module.id,
+                dispatch=self.dispatch
+            )
+        )
 
     def test_get_module(self):
         self.assertEqual(
@@ -251,6 +259,63 @@ class ModuleRenderTestCase(ModuleStoreTestCase, LoginEnrollmentTestCase):
         render.get_module_for_descriptor(self.mock_user, request, descriptor, field_data_cache, self.toy_course.id)
         render.get_module_for_descriptor(self.mock_user, request, descriptor, field_data_cache, self.toy_course.id)
 
+    @override_settings(FIELD_OVERRIDE_PROVIDERS=(
+        'ccx.overrides.CustomCoursesForEdxOverrideProvider',))
+    def test_rebind_different_users_ccx(self):
+        """
+        This tests the rebinding a descriptor to a student does not result
+        in overly nested _field_data when CCX is enabled.
+        """
+        request = self.request_factory.get('')
+        request.user = self.mock_user
+        course = CourseFactory()
+
+        descriptor = ItemFactory(category='html', parent=course)
+        field_data_cache = FieldDataCache(
+            [self.toy_course, descriptor], self.toy_course.id, self.mock_user
+        )
+
+        # grab what _field_data was originally set to
+        original_field_data = descriptor._field_data  # pylint: disable=protected-access, no-member
+
+        render.get_module_for_descriptor(
+            self.mock_user, request, descriptor, field_data_cache, self.toy_course.id
+        )
+
+        # check that _unwrapped_field_data is the same as the original
+        # _field_data, but now _field_data as been reset.
+        # pylint: disable=protected-access, no-member
+        self.assertIs(descriptor._unwrapped_field_data, original_field_data)
+        self.assertIsNot(descriptor._unwrapped_field_data, descriptor._field_data)
+
+        # now bind this module to a few other students
+        for user in [UserFactory(), UserFactory(), UserFactory()]:
+            render.get_module_for_descriptor(
+                user,
+                request,
+                descriptor,
+                field_data_cache,
+                self.toy_course.id
+            )
+
+        # _field_data should now be wrapped by LmsFieldData
+        # pylint: disable=protected-access, no-member
+        self.assertIsInstance(descriptor._field_data, LmsFieldData)
+
+        # the LmsFieldData should now wrap OverrideFieldData
+        self.assertIsInstance(
+            # pylint: disable=protected-access, no-member
+            descriptor._field_data._authored_data._source,
+            OverrideFieldData
+        )
+
+        # the OverrideFieldData should point to the original unwrapped field_data
+        self.assertIs(
+            # pylint: disable=protected-access, no-member
+            descriptor._field_data._authored_data._source.fallback,
+            descriptor._unwrapped_field_data
+        )
+
     def test_hash_resource(self):
         """
         Ensure that the resource hasher works and does not fail on unicode,