Merge pull request #6563 from edx/ned/fix-6013

Fixes for #6013: Implement user service to return currently-logged-in user

cms/djangoapps/contentstore/views/preview.py

@@ -24,6 +24,7 @@
 from xblock.exceptions import NoSuchHandlerError
 from xblock.fragment import Fragment
 from student.auth import has_studio_read_access, has_studio_write_access
+from xblock_django.user_service import DjangoXBlockUserService
 
 from lms.djangoapps.lms_xblock.field_data import LmsFieldData
 from cms.lib.xblock.field_data import CmsFieldData
@@ -112,20 +113,6 @@ def applicable_aside_types(self, block):
         ]
 
 
-class StudioUserService(object):
-    """
-    Provides a Studio implementation of the XBlock user service.
-    """
-
-    def __init__(self, request):
-        super(StudioUserService, self).__init__()
-        self._request = request
-
-    @property
-    def user_id(self):
-        return self._request.user.id
-
-
 class StudioPermissionsService(object):
     """
     Service that can provide information about a user's permissions.
@@ -176,7 +163,6 @@ def _preview_module_system(request, descriptor, field_data):
         _studio_wrap_xblock,
     ]
 
-    descriptor.runtime._services['user'] = StudioUserService(request)  # pylint: disable=protected-access
     descriptor.runtime._services['studio_user_permissions'] = StudioPermissionsService(request)  # pylint: disable=protected-access
 
     return PreviewModuleSystem(
@@ -204,6 +190,7 @@ def _preview_module_system(request, descriptor, field_data):
             "i18n": ModuleI18nService(),
             "field-data": field_data,
             "library_tools": LibraryToolsService(modulestore()),
+            "user": DjangoXBlockUserService(request.user),
         },
     )
 

cms/djangoapps/contentstore/views/tests/test_item.py

@@ -12,7 +12,6 @@
 from django.test.client import RequestFactory
 from django.core.urlresolvers import reverse
 from contentstore.utils import reverse_usage_url, reverse_course_url
-from contentstore.views.preview import StudioUserService
 
 from contentstore.views.component import (
     component_handler, get_component_templates
@@ -30,6 +29,7 @@
 from xmodule.modulestore.tests.factories import ItemFactory, LibraryFactory, check_mongo_calls
 from xmodule.x_module import STUDIO_VIEW, STUDENT_VIEW
 from xblock.exceptions import NoSuchHandlerError
+from xblock_django.user_service import DjangoXBlockUserService
 from opaque_keys.edx.keys import UsageKey, CourseKey
 from opaque_keys.edx.locations import Location
 from xmodule.partitions.partitions import Group, UserPartition
@@ -1170,7 +1170,7 @@ def test_add_groups(self):
         # (CachingDescriptorSystem is used in tests, PreviewModuleSystem in Studio).
         # CachingDescriptorSystem doesn't have user service, that's needed for
         # SplitTestModule. So, in this line of code we add this service manually.
-        split_test.runtime._services['user'] = StudioUserService(self.request)  # pylint: disable=protected-access
+        split_test.runtime._services['user'] = DjangoXBlockUserService(self.user)  # pylint: disable=protected-access
 
         # Call add_missing_groups method to add the missing group.
         split_test.add_missing_groups(self.request)

cms/djangoapps/contentstore/views/tests/test_preview.py

@@ -2,6 +2,9 @@
 Tests for contentstore.views.preview.py
 """
 import re
+import ddt
+from mock import Mock
+from xblock.core import XBlock
 
 from django.test import TestCase
 from django.test.client import RequestFactory
@@ -10,8 +13,9 @@
 from student.tests.factories import UserFactory
 
 from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
+from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 
-from contentstore.views.preview import get_preview_fragment
+from contentstore.views.preview import get_preview_fragment, _preview_module_system
 from xmodule.modulestore import ModuleStoreEnum
 from xmodule.modulestore.tests.test_asides import AsideTestType
 from cms.djangoapps.xblock_config.models import StudioConfig
@@ -101,3 +105,44 @@ def test_preview_no_asides(self):
 
         self.assertNotRegexpMatches(html, r"data-block-type=[\"\']test_aside[\"\']")
         self.assertNotRegexpMatches(html, "Aside rendered")
+
+
+@XBlock.needs("field-data")
+@XBlock.needs("i18n")
+@XBlock.needs("user")
+class PureXBlock(XBlock):
+    """
+    Pure XBlock to use in tests.
+    """
+    pass
+
+
+@ddt.ddt
+class StudioXBlockServiceBindingTest(ModuleStoreTestCase):
+    """
+    Tests that the Studio Module System (XBlock Runtime) provides an expected set of services.
+    """
+    def setUp(self):
+        """
+        Set up the user and request that will be used.
+        """
+        super(StudioXBlockServiceBindingTest, self).setUp()
+        self.user = UserFactory()
+        self.course = CourseFactory.create()
+        self.request = Mock()
+        self.field_data = Mock()
+
+    @XBlock.register_temp_plugin(PureXBlock, identifier='pure')
+    @ddt.data("user", "i18n", "field-data")
+    def test_expected_services_exist(self, expected_service):
+        """
+        Tests that the 'user' and 'i18n' services are provided by the Studio runtime.
+        """
+        descriptor = ItemFactory(category="pure", parent=self.course)
+        runtime = _preview_module_system(
+            self.request,
+            descriptor,
+            self.field_data,
+        )
+        service = runtime.service(descriptor, expected_service)
+        self.assertIsNotNone(service)

common/djangoapps/xblock_django/tests/test_user_service.py

@@ -0,0 +1,57 @@
+"""
+Tests for the DjangoXBlockUserService.
+"""
+from django.test import TestCase
+from xblock_django.user_service import (
+    DjangoXBlockUserService,
+    ATTR_KEY_IS_AUTHENTICATED,
+    ATTR_KEY_USER_ID,
+    ATTR_KEY_USERNAME,
+)
+from student.tests.factories import UserFactory, AnonymousUserFactory
+
+
+class UserServiceTestCase(TestCase):
+    """
+    Tests for the DjangoXBlockUserService.
+    """
+    def setUp(self):
+        self.user = UserFactory(username="tester", email="test@tester.com")
+        self.user.profile.name = "Test Tester"
+        self.anon_user = AnonymousUserFactory()
+
+    def assert_is_anon_xb_user(self, xb_user):
+        """
+        A set of assertions for an anonymous XBlockUser.
+        """
+        self.assertFalse(xb_user.opt_attrs[ATTR_KEY_IS_AUTHENTICATED])
+        self.assertIsNone(xb_user.full_name)
+        self.assertListEqual(xb_user.emails, [])
+
+    def assert_xblock_user_matches_django(self, xb_user, dj_user):
+        """
+        A set of assertions for comparing a XBlockUser to a django User
+        """
+        self.assertTrue(xb_user.opt_attrs[ATTR_KEY_IS_AUTHENTICATED])
+        self.assertEqual(xb_user.emails[0], dj_user.email)
+        self.assertEqual(xb_user.full_name, dj_user.profile.name)
+        self.assertEqual(xb_user.opt_attrs[ATTR_KEY_USERNAME], dj_user.username)
+        self.assertEqual(xb_user.opt_attrs[ATTR_KEY_USER_ID], dj_user.id)
+
+    def test_convert_anon_user(self):
+        """
+        Tests for convert_django_user_to_xblock_user behavior when django user is AnonymousUser.
+        """
+        django_user_service = DjangoXBlockUserService(self.anon_user)
+        xb_user = django_user_service.get_current_user()
+        self.assertTrue(xb_user.is_current_user)
+        self.assert_is_anon_xb_user(xb_user)
+
+    def test_convert_authenticate_user(self):
+        """
+        Tests for convert_django_user_to_xblock_user behavior when django user is User.
+        """
+        django_user_service = DjangoXBlockUserService(self.user)
+        xb_user = django_user_service.get_current_user()
+        self.assertTrue(xb_user.is_current_user)
+        self.assert_xblock_user_matches_django(xb_user, self.user)

common/djangoapps/xblock_django/user_service.py

@@ -0,0 +1,42 @@
+"""
+Support for converting a django user to an XBlock user
+"""
+from xblock.reference.user_service import XBlockUser, UserService
+
+ATTR_KEY_IS_AUTHENTICATED = 'edx-platform.is_authenticated'
+ATTR_KEY_USER_ID = 'edx-platform.user_id'
+ATTR_KEY_USERNAME = 'edx-platform.username'
+
+
+class DjangoXBlockUserService(UserService):
+    """
+    A user service that converts Django users to XBlockUser
+    """
+    def __init__(self, django_user, **kwargs):
+        super(DjangoXBlockUserService, self).__init__(**kwargs)
+        self._django_user = django_user
+
+    def get_current_user(self):
+        """
+        Returns the currently-logged in user, as an instance of XBlockUser
+        """
+        return self._convert_django_user_to_xblock_user(self._django_user)
+
+    def _convert_django_user_to_xblock_user(self, django_user):
+        """
+        A function that returns an XBlockUser from the current Django request.user
+        """
+        xblock_user = XBlockUser(is_current_user=True)
+
+        if django_user is not None and django_user.is_authenticated():
+            # This full_name is dependent on edx-platform's profile implementation
+            full_name = getattr(django_user.profile, 'name') if hasattr(django_user, 'profile') else None
+            xblock_user.full_name = full_name
+            xblock_user.emails = [django_user.email]
+            xblock_user.opt_attrs[ATTR_KEY_IS_AUTHENTICATED] = True
+            xblock_user.opt_attrs[ATTR_KEY_USER_ID] = django_user.id
+            xblock_user.opt_attrs[ATTR_KEY_USERNAME] = django_user.username
+        else:
+            xblock_user.opt_attrs[ATTR_KEY_IS_AUTHENTICATED] = False
+
+        return xblock_user

common/lib/xmodule/xmodule/library_content_module.py

@@ -381,7 +381,11 @@ def refresh_children(self, request=None, suffix=None):  # pylint: disable=unused
         lib_tools = self.runtime.service(self, 'library_tools')
         user_service = self.runtime.service(self, 'user')
         user_perms = self.runtime.service(self, 'studio_user_permissions')
-        user_id = user_service.user_id if user_service else None  # May be None when creating bok choy test fixtures
+        if user_service:
+            # May be None when creating bok choy test fixtures
+            user_id = user_service.get_current_user().opt_attrs.get('edx-platform.user_id', None)
+        else:
+            user_id = None
         lib_tools.update_children(self, user_id, user_perms)
         return Response()
 

common/lib/xmodule/xmodule/modulestore/django.py

@@ -14,7 +14,6 @@
 import django.utils
 
 import re
-import threading
 
 from xmodule.util.django import get_current_request_hostname
 import xmodule.modulestore  # pylint: disable=unused-import
@@ -30,6 +29,14 @@
 except ImportError:
     HAS_REQUEST_CACHE = False
 
+# We also may not always have the current request user (crum) module available
+try:
+    from xblock_django.user_service import DjangoXBlockUserService
+    from crum import get_current_user
+    HAS_USER_SERVICE = True
+except ImportError:
+    HAS_USER_SERVICE = False
+
 ASSET_IGNORE_REGEX = getattr(settings, "ASSET_IGNORE_REGEX", r"(^\._.*$)|(^\.DS_Store$)|(^.*~$)")
 
 
@@ -44,7 +51,15 @@ def load_function(path):
     return getattr(import_module(module_path), name)
 
 
-def create_modulestore_instance(engine, content_store, doc_store_config, options, i18n_service=None, fs_service=None):
+def create_modulestore_instance(
+        engine,
+        content_store,
+        doc_store_config,
+        options,
+        i18n_service=None,
+        fs_service=None,
+        user_service=None,
+):
     """
     This will return a new instance of a modulestore given an engine and options
     """
@@ -74,6 +89,11 @@ def create_modulestore_instance(engine, content_store, doc_store_config, options
     if issubclass(class_, BranchSettingMixin):
         _options['branch_setting_func'] = _get_modulestore_branch_setting
 
+    if HAS_USER_SERVICE and not user_service:
+        xb_user_service = DjangoXBlockUserService(get_current_user())
+    else:
+        xb_user_service = None
+
     return class_(
         contentstore=content_store,
         metadata_inheritance_cache_subsystem=metadata_inheritance_cache,
@@ -83,6 +103,7 @@ def create_modulestore_instance(engine, content_store, doc_store_config, options
         doc_store_config=doc_store_config,
         i18n_service=i18n_service or ModuleI18nService(),
         fs_service=fs_service or xblock.reference.plugins.FSService(),
+        user_service=user_service or xb_user_service,
         **_options
     )
 

common/lib/xmodule/xmodule/modulestore/mixed.py

@@ -99,7 +99,17 @@ class MixedModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase):
     """
     ModuleStore knows how to route requests to the right persistence ms
     """
-    def __init__(self, contentstore, mappings, stores, i18n_service=None, fs_service=None, create_modulestore_instance=None, **kwargs):
+    def __init__(
+            self,
+            contentstore,
+            mappings,
+            stores,
+            i18n_service=None,
+            fs_service=None,
+            user_service=None,
+            create_modulestore_instance=None,
+            **kwargs
+    ):
         """
         Initialize a MixedModuleStore. Here we look into our passed in kwargs which should be a
         collection of other modulestore configuration information
@@ -139,6 +149,7 @@ def __init__(self, contentstore, mappings, stores, i18n_service=None, fs_service
                 store_settings.get('OPTIONS', {}),
                 i18n_service=i18n_service,
                 fs_service=fs_service,
+                user_service=user_service,
             )
             # replace all named pointers to the store into actual pointers
             for course_key, store_name in self.mappings.iteritems():
@@ -303,7 +314,7 @@ def get_course(self, course_key, depth=0, **kwargs):
 
         :param course_key: must be a CourseKey
         """
-        assert(isinstance(course_key, CourseKey))
+        assert isinstance(course_key, CourseKey)
         store = self._get_modulestore_for_courseid(course_key)
         try:
             return store.get_course(course_key, depth=depth, **kwargs)
@@ -340,15 +351,15 @@ def has_course(self, course_id, ignore_case=False, **kwargs):
         * ignore_case (bool): If True, do a case insensitive search. If
             False, do a case sensitive search
         """
-        assert(isinstance(course_id, CourseKey))
+        assert isinstance(course_id, CourseKey)
         store = self._get_modulestore_for_courseid(course_id)
         return store.has_course(course_id, ignore_case, **kwargs)
 
     def delete_course(self, course_key, user_id):
         """
         See xmodule.modulestore.__init__.ModuleStoreWrite.delete_course
         """
-        assert(isinstance(course_key, CourseKey))
+        assert isinstance(course_key, CourseKey)
         store = self._get_modulestore_for_courseid(course_key)
         return store.delete_course(course_key, user_id)