From 4d729100605bcaac658db5ff6765f208dda9f23c Mon Sep 17 00:00:00 2001 From: muzaffaryousaf Date: Mon, 1 Dec 2014 19:31:34 +0500 Subject: [PATCH 1/6] Adding get_anonymous_user_id in Xblock 'user' service. TNL-836 --- .../xblock_django/tests/test_user_service.py | 38 +++++++++++++++++++ .../djangoapps/xblock_django/user_service.py | 28 ++++++++++++++ lms/djangoapps/courseware/module_render.py | 6 ++- .../courseware/tests/test_module_render.py | 20 ++++++++++ 4 files changed, 90 insertions(+), 2 deletions(-) diff --git a/common/djangoapps/xblock_django/tests/test_user_service.py b/common/djangoapps/xblock_django/tests/test_user_service.py index 536a2af1eb..2f910de3c4 100644 --- a/common/djangoapps/xblock_django/tests/test_user_service.py +++ b/common/djangoapps/xblock_django/tests/test_user_service.py @@ -8,7 +8,9 @@ from xblock_django.user_service import ( ATTR_KEY_USER_ID, ATTR_KEY_USERNAME, ) +from student.models import anonymous_id_for_user from student.tests.factories import UserFactory, AnonymousUserFactory +from opaque_keys.edx.locations import SlashSeparatedCourseKey class UserServiceTestCase(TestCase): @@ -55,3 +57,39 @@ class UserServiceTestCase(TestCase): 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) + + def test_get_anonymous_user_id_returns_none_for_non_staff_users(self): + """ + Tests for anonymous_user_id method to return None if user is Non-Staff. + """ + django_user_service = DjangoXBlockUserService(self.user, user_is_staff=False) + + anonymous_user_id = django_user_service.get_anonymous_user_id(username=self.user.username, course_id='edx/toy/2012_Fall') + self.assertIsNone(anonymous_user_id) + + def test_get_anonymous_user_id_returns_none_for_non_existing_users(self): + """ + Tests for anonymous_user_id method to return None username does not exist in system. + """ + django_user_service = DjangoXBlockUserService(self.user, user_is_staff=True) + + anonymous_user_id = django_user_service.get_anonymous_user_id(username="No User", course_id='edx/toy/2012_Fall') + self.assertIsNone(anonymous_user_id) + + def test_get_anonymous_user_id_returns_id_for_existing_users(self): + """ + Tests for anonymous_user_id method returns anonymous user id for a user. + """ + anon_user_id = anonymous_id_for_user( + user=self.user, + course_id=SlashSeparatedCourseKey('edX', 'toy', '2012_Fall'), + save=True + ) + + django_user_service = DjangoXBlockUserService(self.user, user_is_staff=True) + anonymous_user_id = django_user_service.get_anonymous_user_id( + username=self.user.username, + course_id='edX/toy/2012_Fall' + ) + + self.assertEqual(anonymous_user_id, anon_user_id) diff --git a/common/djangoapps/xblock_django/user_service.py b/common/djangoapps/xblock_django/user_service.py index 6867db9f38..7b9e290492 100644 --- a/common/djangoapps/xblock_django/user_service.py +++ b/common/djangoapps/xblock_django/user_service.py @@ -1,7 +1,11 @@ """ Support for converting a django user to an XBlock user """ +from django.contrib.auth.models import User + from xblock.reference.user_service import XBlockUser, UserService +from student.models import anonymous_id_for_user, get_user_by_username_or_email +from opaque_keys.edx.locations import SlashSeparatedCourseKey ATTR_KEY_IS_AUTHENTICATED = 'edx-platform.is_authenticated' ATTR_KEY_USER_ID = 'edx-platform.user_id' @@ -15,6 +19,7 @@ class DjangoXBlockUserService(UserService): def __init__(self, django_user, **kwargs): super(DjangoXBlockUserService, self).__init__(**kwargs) self._django_user = django_user + self._user_is_staff = kwargs.get('user_is_staff', False) def get_current_user(self): """ @@ -22,6 +27,29 @@ class DjangoXBlockUserService(UserService): """ return self._convert_django_user_to_xblock_user(self._django_user) + def get_anonymous_user_id(self, username, course_id): + """ + Get the anonymous user id for a user. + + Args: + username(str): username of a user. + course_id(str): course id of particular course. + + Returns: + A unique anonymous_user_id for (user, course) pair. + None for Non-staff users. + """ + if not self._user_is_staff: + return None + + try: + user = get_user_by_username_or_email(username_or_email=username) + except User.DoesNotExist: + return None + + course_id = SlashSeparatedCourseKey.from_deprecated_string(course_id) + return anonymous_id_for_user(user=user, course_id=course_id, save=False) + def _convert_django_user_to_xblock_user(self, django_user): """ A function that returns an XBlockUser from the current Django request.user diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 4a0ae1e8a4..854cb6efe0 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -598,6 +598,8 @@ def get_module_system_for_user(user, field_data_cache, field_data = LmsFieldData(descriptor._field_data, student_data) # pylint: disable=protected-access + user_is_staff = has_access(user, u'staff', descriptor.location, course_id) + system = LmsModuleSystem( track_function=track_function, render_template=render_to_string, @@ -644,7 +646,7 @@ def get_module_system_for_user(user, field_data_cache, 'i18n': ModuleI18nService(), 'fs': xblock.reference.plugins.FSService(), 'field-data': field_data, - 'user': DjangoXBlockUserService(user), + 'user': DjangoXBlockUserService(user, user_is_staff=user_is_staff), }, get_user_role=lambda: get_user_role(user, course_id), descriptor_runtime=descriptor.runtime, @@ -668,7 +670,7 @@ def get_module_system_for_user(user, field_data_cache, make_psychometrics_data_update_handler(course_id, user, descriptor.location) ) - system.set(u'user_is_staff', has_access(user, u'staff', descriptor.location, course_id)) + system.set(u'user_is_staff', user_is_staff) system.set(u'user_is_admin', has_access(user, u'staff', 'global')) # make an ErrorDescriptor -- assuming that the descriptor's system is ok diff --git a/lms/djangoapps/courseware/tests/test_module_render.py b/lms/djangoapps/courseware/tests/test_module_render.py index b4156ccc35..c0dc82e97c 100644 --- a/lms/djangoapps/courseware/tests/test_module_render.py +++ b/lms/djangoapps/courseware/tests/test_module_render.py @@ -1218,3 +1218,23 @@ class LMSXBlockServiceBindingTest(ModuleStoreTestCase): ) service = runtime.service(descriptor, expected_service) self.assertIsNotNone(service) + + @XBlock.register_temp_plugin(PureXBlock, identifier='pure') + @ddt.data("user") + def test_expected_user_service_exists_with_staff_info(self, expected_service): + """ + Tests that the LMS runtime contains the 'user' service with appropriate staff info. + """ + descriptor = ItemFactory(category="pure", parent=self.course) + runtime, _ = render.get_module_system_for_user( + self.user, + self.field_data_cache, + descriptor, + self.course.id, + self.track_function, + self.xqueue_callback_url_prefix, + self.request_token + ) + service = runtime.service(descriptor, expected_service) + self.assertIsNotNone(service) + self.assertTrue(hasattr(service, '_user_is_staff')) From 2189ef7c50bd22d11f1027cf203e8615c8f5b41c Mon Sep 17 00:00:00 2001 From: muzaffaryousaf Date: Wed, 28 Jan 2015 15:01:40 +0500 Subject: [PATCH 2/6] Fixing the library tests. TNL-836 --- common/djangoapps/xblock_django/user_service.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/common/djangoapps/xblock_django/user_service.py b/common/djangoapps/xblock_django/user_service.py index 7b9e290492..3c54eb61e4 100644 --- a/common/djangoapps/xblock_django/user_service.py +++ b/common/djangoapps/xblock_django/user_service.py @@ -2,10 +2,8 @@ Support for converting a django user to an XBlock user """ from django.contrib.auth.models import User - -from xblock.reference.user_service import XBlockUser, UserService -from student.models import anonymous_id_for_user, get_user_by_username_or_email from opaque_keys.edx.locations import SlashSeparatedCourseKey +from xblock.reference.user_service import XBlockUser, UserService ATTR_KEY_IS_AUTHENTICATED = 'edx-platform.is_authenticated' ATTR_KEY_USER_ID = 'edx-platform.user_id' @@ -42,6 +40,9 @@ class DjangoXBlockUserService(UserService): if not self._user_is_staff: return None + # If we import these in start, it causes the contentstore library tests failed. + from student.models import anonymous_id_for_user, get_user_by_username_or_email + try: user = get_user_by_username_or_email(username_or_email=username) except User.DoesNotExist: From 7b10d14f5d83d3248712746e22511307ec31cf24 Mon Sep 17 00:00:00 2001 From: muzaffaryousaf Date: Wed, 28 Jan 2015 21:37:25 +0500 Subject: [PATCH 3/6] Adding 'user_is_staff' filed to opt_attrs in xblock user. TNL-1185 --- common/djangoapps/xblock_django/user_service.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/common/djangoapps/xblock_django/user_service.py b/common/djangoapps/xblock_django/user_service.py index 3c54eb61e4..ee77921d90 100644 --- a/common/djangoapps/xblock_django/user_service.py +++ b/common/djangoapps/xblock_django/user_service.py @@ -2,12 +2,13 @@ Support for converting a django user to an XBlock user """ from django.contrib.auth.models import User -from opaque_keys.edx.locations import SlashSeparatedCourseKey +from opaque_keys.edx.keys import CourseKey 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' +ATTR_KEY_USER_IS_STAFF = 'edx-platform.user_is_staff' class DjangoXBlockUserService(UserService): @@ -17,7 +18,8 @@ class DjangoXBlockUserService(UserService): def __init__(self, django_user, **kwargs): super(DjangoXBlockUserService, self).__init__(**kwargs) self._django_user = django_user - self._user_is_staff = kwargs.get('user_is_staff', False) + if self._django_user: + self._django_user.user_is_staff = kwargs.get('user_is_staff', False) def get_current_user(self): """ @@ -37,7 +39,7 @@ class DjangoXBlockUserService(UserService): A unique anonymous_user_id for (user, course) pair. None for Non-staff users. """ - if not self._user_is_staff: + if not self.get_current_user().opt_attrs.get(ATTR_KEY_USER_IS_STAFF): return None # If we import these in start, it causes the contentstore library tests failed. @@ -48,7 +50,7 @@ class DjangoXBlockUserService(UserService): except User.DoesNotExist: return None - course_id = SlashSeparatedCourseKey.from_deprecated_string(course_id) + course_id = CourseKey.from_string(course_id) return anonymous_id_for_user(user=user, course_id=course_id, save=False) def _convert_django_user_to_xblock_user(self, django_user): @@ -65,6 +67,7 @@ class DjangoXBlockUserService(UserService): 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 + xblock_user.opt_attrs[ATTR_KEY_USER_IS_STAFF] = django_user.user_is_staff else: xblock_user.opt_attrs[ATTR_KEY_IS_AUTHENTICATED] = False From 3f4ee28e272364d686ad2cb1e8a6929bb2df8768 Mon Sep 17 00:00:00 2001 From: muzaffaryousaf Date: Thu, 29 Jan 2015 12:51:30 +0500 Subject: [PATCH 4/6] Refactoring the user service test. TNL-1185 --- common/djangoapps/xblock_django/tests/test_user_service.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/common/djangoapps/xblock_django/tests/test_user_service.py b/common/djangoapps/xblock_django/tests/test_user_service.py index 2f910de3c4..4c88590e4e 100644 --- a/common/djangoapps/xblock_django/tests/test_user_service.py +++ b/common/djangoapps/xblock_django/tests/test_user_service.py @@ -10,7 +10,7 @@ from xblock_django.user_service import ( ) from student.models import anonymous_id_for_user from student.tests.factories import UserFactory, AnonymousUserFactory -from opaque_keys.edx.locations import SlashSeparatedCourseKey +from opaque_keys.edx.keys import CourseKey class UserServiceTestCase(TestCase): @@ -80,9 +80,10 @@ class UserServiceTestCase(TestCase): """ Tests for anonymous_user_id method returns anonymous user id for a user. """ + course_key = CourseKey.from_string('edX/toy/2012_Fall') anon_user_id = anonymous_id_for_user( user=self.user, - course_id=SlashSeparatedCourseKey('edX', 'toy', '2012_Fall'), + course_id=course_key, save=True ) From c461541d3554944354a1e83d4c797feb8f9227b3 Mon Sep 17 00:00:00 2001 From: muzaffaryousaf Date: Thu, 29 Jan 2015 13:33:28 +0500 Subject: [PATCH 5/6] Refactoring tests. TNL-1185 --- .../xblock_django/tests/test_user_service.py | 2 ++ .../djangoapps/xblock_django/user_service.py | 4 +--- .../courseware/tests/test_module_render.py | 20 ------------------- 3 files changed, 3 insertions(+), 23 deletions(-) diff --git a/common/djangoapps/xblock_django/tests/test_user_service.py b/common/djangoapps/xblock_django/tests/test_user_service.py index 4c88590e4e..e3bf764a53 100644 --- a/common/djangoapps/xblock_django/tests/test_user_service.py +++ b/common/djangoapps/xblock_django/tests/test_user_service.py @@ -7,6 +7,7 @@ from xblock_django.user_service import ( ATTR_KEY_IS_AUTHENTICATED, ATTR_KEY_USER_ID, ATTR_KEY_USERNAME, + ATTR_KEY_USER_IS_STAFF, ) from student.models import anonymous_id_for_user from student.tests.factories import UserFactory, AnonymousUserFactory @@ -39,6 +40,7 @@ class UserServiceTestCase(TestCase): 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) + self.assertFalse(xb_user.opt_attrs[ATTR_KEY_USER_IS_STAFF]) def test_convert_anon_user(self): """ diff --git a/common/djangoapps/xblock_django/user_service.py b/common/djangoapps/xblock_django/user_service.py index ee77921d90..c09e518578 100644 --- a/common/djangoapps/xblock_django/user_service.py +++ b/common/djangoapps/xblock_django/user_service.py @@ -4,6 +4,7 @@ Support for converting a django user to an XBlock user from django.contrib.auth.models import User from opaque_keys.edx.keys import CourseKey from xblock.reference.user_service import XBlockUser, UserService +from student.models import anonymous_id_for_user, get_user_by_username_or_email ATTR_KEY_IS_AUTHENTICATED = 'edx-platform.is_authenticated' ATTR_KEY_USER_ID = 'edx-platform.user_id' @@ -42,9 +43,6 @@ class DjangoXBlockUserService(UserService): if not self.get_current_user().opt_attrs.get(ATTR_KEY_USER_IS_STAFF): return None - # If we import these in start, it causes the contentstore library tests failed. - from student.models import anonymous_id_for_user, get_user_by_username_or_email - try: user = get_user_by_username_or_email(username_or_email=username) except User.DoesNotExist: diff --git a/lms/djangoapps/courseware/tests/test_module_render.py b/lms/djangoapps/courseware/tests/test_module_render.py index c0dc82e97c..b4156ccc35 100644 --- a/lms/djangoapps/courseware/tests/test_module_render.py +++ b/lms/djangoapps/courseware/tests/test_module_render.py @@ -1218,23 +1218,3 @@ class LMSXBlockServiceBindingTest(ModuleStoreTestCase): ) service = runtime.service(descriptor, expected_service) self.assertIsNotNone(service) - - @XBlock.register_temp_plugin(PureXBlock, identifier='pure') - @ddt.data("user") - def test_expected_user_service_exists_with_staff_info(self, expected_service): - """ - Tests that the LMS runtime contains the 'user' service with appropriate staff info. - """ - descriptor = ItemFactory(category="pure", parent=self.course) - runtime, _ = render.get_module_system_for_user( - self.user, - self.field_data_cache, - descriptor, - self.course.id, - self.track_function, - self.xqueue_callback_url_prefix, - self.request_token - ) - service = runtime.service(descriptor, expected_service) - self.assertIsNotNone(service) - self.assertTrue(hasattr(service, '_user_is_staff')) From 3a09af07e5e895da8e1ff5b647d06bca33799fff Mon Sep 17 00:00:00 2001 From: muzaffaryousaf Date: Thu, 29 Jan 2015 18:37:22 +0500 Subject: [PATCH 6/6] Fixing the broken tests. TNL-1185 --- cms/djangoapps/contentstore/tests/test_libraries.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cms/djangoapps/contentstore/tests/test_libraries.py b/cms/djangoapps/contentstore/tests/test_libraries.py index dc6d8b976d..1faed0c74a 100644 --- a/cms/djangoapps/contentstore/tests/test_libraries.py +++ b/cms/djangoapps/contentstore/tests/test_libraries.py @@ -80,7 +80,9 @@ class LibraryTestCase(ModuleStoreTestCase): of a LibraryContent block """ if 'user' not in lib_content_block.runtime._services: # pylint: disable=protected-access - lib_content_block.runtime._services['user'] = Mock(user_id=self.user.id) # pylint: disable=protected-access + mocked_user_service = Mock(user_id=self.user.id).get_current_user.return_value = {} + lib_content_block.runtime._services['user'] = mocked_user_service # pylint: disable=protected-access + handler_url = reverse_usage_url( 'component_handler', lib_content_block.location,