diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index 1b28e65a4c..01090ff3d1 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -194,16 +194,11 @@ def _prepare_runtime_for_preview(request, block, field_data): # stick the license wrapper in front wrappers.insert(0, partial(wrap_with_license, mako_service=mako_service)) - preview_anonymous_user_id = 'student' + anonymous_user_id = deprecated_anonymous_user_id = 'student' if individualize_anonymous_user_id(course_id): - # There are blocks (capa, html, and video) where we do not want to scope - # the anonymous_user_id to specific courses. These are captured in the - # block attribute 'requires_per_student_anonymous_id'. Please note, - # the course_id field in AnynomousUserID model is blank if value is None. - if getattr(block, 'requires_per_student_anonymous_id', False): - preview_anonymous_user_id = anonymous_id_for_user(request.user, None) - else: - preview_anonymous_user_id = anonymous_id_for_user(request.user, course_id) + anonymous_user_id = anonymous_id_for_user(request.user, course_id) + # See the docstring of `DjangoXBlockUserService`. + deprecated_anonymous_user_id = anonymous_id_for_user(request.user, None) services = { "field-data": field_data, @@ -213,7 +208,8 @@ def _prepare_runtime_for_preview(request, block, field_data): "user": DjangoXBlockUserService( request.user, user_role=get_user_role(request.user, course_id), - anonymous_user_id=preview_anonymous_user_id, + anonymous_user_id=anonymous_user_id, + deprecated_anonymous_user_id=deprecated_anonymous_user_id, ), "partitions": StudioPartitionService(course_id=course_id), "teams_configuration": TeamsConfigurationService(), diff --git a/cms/djangoapps/contentstore/views/tests/test_preview.py b/cms/djangoapps/contentstore/views/tests/test_preview.py index 7e407c966d..1810842d4f 100644 --- a/cms/djangoapps/contentstore/views/tests/test_preview.py +++ b/cms/djangoapps/contentstore/views/tests/test_preview.py @@ -1,12 +1,11 @@ """ Tests for contentstore.views.preview.py """ - - import re from unittest import mock import ddt +from common.djangoapps.xblock_django.constants import ATTR_KEY_ANONYMOUS_USER_ID, ATTR_KEY_DEPRECATED_ANONYMOUS_USER_ID from django.test.client import Client, RequestFactory from django.test.utils import override_settings from edx_toggles.toggles.testutils import override_waffle_flag @@ -309,7 +308,10 @@ class CmsModuleSystemShimTest(ModuleStoreTestCase): block=block, field_data=mock.Mock(), ) - assert block.runtime.anonymous_student_id == '26262401c528d7c4a6bbeabe0455ec46' + deprecated_anonymous_user_id = ( + block.runtime.service(block, 'user').get_current_user().opt_attrs.get(ATTR_KEY_DEPRECATED_ANONYMOUS_USER_ID) + ) + assert deprecated_anonymous_user_id == '26262401c528d7c4a6bbeabe0455ec46' @override_waffle_flag(INDIVIDUALIZE_ANONYMOUS_USER_ID, active=True) def test_anonymous_user_id_individual_per_course(self): @@ -321,4 +323,8 @@ class CmsModuleSystemShimTest(ModuleStoreTestCase): block=block, field_data=mock.Mock(), ) - assert block.runtime.anonymous_student_id == 'ad503f629b55c531fed2e45aa17a3368' + + anonymous_user_id = ( + block.runtime.service(block, 'user').get_current_user().opt_attrs.get(ATTR_KEY_ANONYMOUS_USER_ID) + ) + assert anonymous_user_id == 'ad503f629b55c531fed2e45aa17a3368' diff --git a/common/djangoapps/xblock_django/constants.py b/common/djangoapps/xblock_django/constants.py index cb528f5222..d93aac40a2 100644 --- a/common/djangoapps/xblock_django/constants.py +++ b/common/djangoapps/xblock_django/constants.py @@ -4,6 +4,7 @@ Constants used by DjangoXBlockUserService # Optional attributes stored on the XBlockUser ATTR_KEY_ANONYMOUS_USER_ID = 'edx-platform.anonymous_user_id' +ATTR_KEY_DEPRECATED_ANONYMOUS_USER_ID = 'edx-platform.deprecated_anonymous_user_id' ATTR_KEY_REQUEST_COUNTRY_CODE = 'edx-platform.request_country_code' ATTR_KEY_IS_AUTHENTICATED = 'edx-platform.is_authenticated' ATTR_KEY_USER_ID = 'edx-platform.user_id' diff --git a/common/djangoapps/xblock_django/user_service.py b/common/djangoapps/xblock_django/user_service.py index e9d8e74992..65fa6d9786 100644 --- a/common/djangoapps/xblock_django/user_service.py +++ b/common/djangoapps/xblock_django/user_service.py @@ -13,6 +13,7 @@ from common.djangoapps.student.models import anonymous_id_for_user, get_user_by_ from .constants import ( ATTR_KEY_ANONYMOUS_USER_ID, + ATTR_KEY_DEPRECATED_ANONYMOUS_USER_ID, ATTR_KEY_IS_AUTHENTICATED, ATTR_KEY_REQUEST_COUNTRY_CODE, ATTR_KEY_USER_ID, @@ -38,6 +39,9 @@ class DjangoXBlockUserService(UserService): user_is_staff(bool): optional - whether the user is staff in the course user_role(str): optional -- user's role in the course ('staff', 'instructor', or 'student') anonymous_user_id(str): optional - anonymous_user_id for the user in the course + deprecated_anonymous_user_id(str): optional - There are XBlocks (CAPA and HTML) that use the per-student + (course-agnostic) anonymized ID. To preserve backward compatibility, we will continue to provide it. + Using course-specific anonymous user ID (`anonymous_user_id`) is a preferred approach. request_country_code(str): optional -- country code determined from the user's request IP address. """ super().__init__(**kwargs) @@ -45,6 +49,7 @@ class DjangoXBlockUserService(UserService): self._user_is_staff = kwargs.get('user_is_staff', False) self._user_role = kwargs.get('user_role', 'student') self._anonymous_user_id = kwargs.get('anonymous_user_id', None) + self._deprecated_anonymous_user_id = kwargs.get('deprecated_anonymous_user_id', None) self._request_country_code = kwargs.get('request_country_code', None) def get_current_user(self): @@ -111,6 +116,7 @@ class DjangoXBlockUserService(UserService): xblock_user.full_name = full_name xblock_user.emails = [django_user.email] xblock_user.opt_attrs[ATTR_KEY_ANONYMOUS_USER_ID] = self._anonymous_user_id + xblock_user.opt_attrs[ATTR_KEY_DEPRECATED_ANONYMOUS_USER_ID] = self._deprecated_anonymous_user_id xblock_user.opt_attrs[ATTR_KEY_IS_AUTHENTICATED] = True xblock_user.opt_attrs[ATTR_KEY_REQUEST_COUNTRY_CODE] = self._request_country_code xblock_user.opt_attrs[ATTR_KEY_USER_ID] = django_user.id diff --git a/lms/djangoapps/courseware/block_render.py b/lms/djangoapps/courseware/block_render.py index b66a193f4f..e4ad4c23fa 100644 --- a/lms/djangoapps/courseware/block_render.py +++ b/lms/djangoapps/courseware/block_render.py @@ -493,23 +493,14 @@ def prepare_runtime_for_user( will_recheck_access=will_recheck_access, ) - # These modules store data using the anonymous_student_id as a key. - # To prevent loss of data, we will continue to provide old modules with - # the per-student anonymized id (as we have in the past), - # while giving selected modules a per-course anonymized id. - # As we have the time to manually test more modules, we can add to the list - # of modules that get the per-course anonymized id. - if getattr(block, 'requires_per_student_anonymous_id', False): - anonymous_student_id = anonymous_id_for_user(user, None) - else: - anonymous_student_id = anonymous_id_for_user(user, course_id) - user_is_staff = bool(has_access(user, 'staff', block.location, course_id)) user_service = DjangoXBlockUserService( user, user_is_staff=user_is_staff, user_role=get_user_role(user, course_id), - anonymous_user_id=anonymous_student_id, + anonymous_user_id=anonymous_id_for_user(user, course_id), + # See the docstring of `DjangoXBlockUserService`. + deprecated_anonymous_user_id=anonymous_id_for_user(user, None), request_country_code=user_location, ) diff --git a/lms/djangoapps/courseware/tests/test_block_render.py b/lms/djangoapps/courseware/tests/test_block_render.py index 9c2e5e9766..897b03442d 100644 --- a/lms/djangoapps/courseware/tests/test_block_render.py +++ b/lms/djangoapps/courseware/tests/test_block_render.py @@ -64,7 +64,7 @@ from common.djangoapps.course_modes.models import CourseMode # lint-amnesty, py from common.djangoapps.student.tests.factories import GlobalStaffFactory from common.djangoapps.student.tests.factories import RequestFactoryNoCsrf from common.djangoapps.student.tests.factories import UserFactory -from common.djangoapps.xblock_django.constants import ATTR_KEY_ANONYMOUS_USER_ID +from common.djangoapps.xblock_django.constants import ATTR_KEY_ANONYMOUS_USER_ID, ATTR_KEY_DEPRECATED_ANONYMOUS_USER_ID from lms.djangoapps.badges.tests.factories import BadgeClassFactory from lms.djangoapps.badges.tests.test_models import get_image from lms.djangoapps.courseware import block_render as render @@ -1858,6 +1858,7 @@ class TestStaffDebugInfo(SharedModuleStoreTestCase): PER_COURSE_ANONYMIZED_XBLOCKS = ( LTIBlock, + VideoBlock, ) PER_STUDENT_ANONYMIZED_XBLOCKS = [ AboutBlock, @@ -1865,7 +1866,6 @@ PER_STUDENT_ANONYMIZED_XBLOCKS = [ HtmlBlock, ProblemBlock, StaticTabBlock, - VideoBlock, ] @@ -1886,7 +1886,7 @@ class TestAnonymousStudentId(SharedModuleStoreTestCase, LoginEnrollmentTestCase) self.user = UserFactory() @patch('lms.djangoapps.courseware.block_render.has_access', Mock(return_value=True, autospec=True)) - def _get_anonymous_id(self, course_id, xblock_class): # lint-amnesty, pylint: disable=missing-function-docstring + def _get_anonymous_id(self, course_id, xblock_class, should_get_deprecated_id: bool): # lint-amnesty, pylint: disable=missing-function-docstring location = course_id.make_usage_key('dummy_category', 'dummy_name') block = Mock( spec=xblock_class, @@ -1924,21 +1924,24 @@ class TestAnonymousStudentId(SharedModuleStoreTestCase, LoginEnrollmentTestCase) course=self.course, ) current_user = rendered_block.runtime.service(rendered_block, 'user').get_current_user() + + if should_get_deprecated_id: + return current_user.opt_attrs.get(ATTR_KEY_DEPRECATED_ANONYMOUS_USER_ID) return current_user.opt_attrs.get(ATTR_KEY_ANONYMOUS_USER_ID) @ddt.data(*PER_STUDENT_ANONYMIZED_XBLOCKS) def test_per_student_anonymized_id(self, block_class): for course_id in ('MITx/6.00x/2012_Fall', 'MITx/6.00x/2013_Spring'): assert 'de619ab51c7f4e9c7216b4644c24f3b5' == \ - self._get_anonymous_id(CourseKey.from_string(course_id), block_class) + self._get_anonymous_id(CourseKey.from_string(course_id), block_class, True) @ddt.data(*PER_COURSE_ANONYMIZED_XBLOCKS) def test_per_course_anonymized_id(self, xblock_class): assert '0c706d119cad686d28067412b9178454' == \ - self._get_anonymous_id(CourseKey.from_string('MITx/6.00x/2012_Fall'), xblock_class) + self._get_anonymous_id(CourseKey.from_string('MITx/6.00x/2012_Fall'), xblock_class, False) assert 'e9969c28c12c8efa6e987d6dbeedeb0b' == \ - self._get_anonymous_id(CourseKey.from_string('MITx/6.00x/2013_Spring'), xblock_class) + self._get_anonymous_id(CourseKey.from_string('MITx/6.00x/2013_Spring'), xblock_class, False) @patch('common.djangoapps.track.views.eventtracker', autospec=True) @@ -2720,7 +2723,9 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase): course=self.course, ) # Ensure the problem block returns a per-user anonymous id - assert self.problem_block.runtime.anonymous_student_id == anonymous_id_for_user(self.user, None) + assert self.problem_block.runtime.service(self.problem_block, 'user').get_current_user().opt_attrs.get( + ATTR_KEY_DEPRECATED_ANONYMOUS_USER_ID + ) == anonymous_id_for_user(self.user, None) _ = render.prepare_runtime_for_user( self.user, @@ -2732,10 +2737,14 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase): course=self.course, ) # Ensure the vertical block returns a per-course+user anonymous id - assert self.block.runtime.anonymous_student_id == anonymous_id_for_user(self.user, self.course.id) + assert self.block.runtime.service(self.block, 'user').get_current_user().opt_attrs.get( + ATTR_KEY_ANONYMOUS_USER_ID + ) == anonymous_id_for_user(self.user, self.course.id) # Ensure the problem runtime's anonymous student ID is unchanged after the above call. - assert self.problem_block.runtime.anonymous_student_id == anonymous_id_for_user(self.user, None) + assert self.problem_block.runtime.service(self.problem_block, 'user').get_current_user().opt_attrs.get( + ATTR_KEY_DEPRECATED_ANONYMOUS_USER_ID + ) == anonymous_id_for_user(self.user, None) def test_user_service_with_anonymous_user(self): _ = render.prepare_runtime_for_user( diff --git a/openedx/core/djangoapps/schedules/tests/test_resolvers.py b/openedx/core/djangoapps/schedules/tests/test_resolvers.py index 36bc8beacd..8bc7f9b4e7 100644 --- a/openedx/core/djangoapps/schedules/tests/test_resolvers.py +++ b/openedx/core/djangoapps/schedules/tests/test_resolvers.py @@ -274,7 +274,7 @@ class TestCourseNextSectionUpdateResolver(SchedulesResolverTestMixin, ModuleStor def test_schedule_context(self): resolver = self.create_resolver() # using this to make sure the select_related stays intact - with self.assertNumQueries(38): + with self.assertNumQueries(40): sc = resolver.get_schedules() schedules = list(sc) apple_logo_url = 'http://email-media.s3.amazonaws.com/edX/2021/store_apple_229x78.jpg' diff --git a/openedx/core/djangoapps/xblock/runtime/runtime.py b/openedx/core/djangoapps/xblock/runtime/runtime.py index ae541740c3..8ea9af9dcb 100644 --- a/openedx/core/djangoapps/xblock/runtime/runtime.py +++ b/openedx/core/djangoapps/xblock/runtime/runtime.py @@ -6,6 +6,7 @@ import logging from urllib.parse import urljoin # pylint: disable=import-error import crum +from common.djangoapps.student.models import anonymous_id_for_user from completion.waffle import ENABLE_COMPLETION_TRACKING_SWITCH from completion.models import BlockCompletion from completion.services import CompletionService @@ -235,12 +236,19 @@ class XBlockRuntime(RuntimeShim, Runtime): elif service_name == "completion": return CompletionService(user=self.user, context_key=context_key) elif service_name == "user": + if self.user.is_anonymous: + deprecated_anonymous_student_id = self.user_id + else: + deprecated_anonymous_student_id = anonymous_id_for_user(self.user, course_id=None) + return DjangoXBlockUserService( self.user, # The value should be updated to whether the user is staff in the context when Blockstore runtime adds # support for courses. user_is_staff=self.user.is_staff, anonymous_user_id=self.anonymous_student_id, + # See the docstring of `DjangoXBlockUserService`. + deprecated_anonymous_user_id=deprecated_anonymous_student_id ) elif service_name == "mako": if self.system.student_data_mode == XBlockRuntimeSystem.STUDENT_DATA_EPHEMERAL: diff --git a/openedx/features/course_experience/tests/views/test_course_updates.py b/openedx/features/course_experience/tests/views/test_course_updates.py index 2478f82697..f26e1cd35b 100644 --- a/openedx/features/course_experience/tests/views/test_course_updates.py +++ b/openedx/features/course_experience/tests/views/test_course_updates.py @@ -49,7 +49,7 @@ class TestCourseUpdatesPage(BaseCourseUpdatesTestCase): # Fetch the view and verify that the query counts haven't changed # TODO: decrease query count as part of REVO-28 - with self.assertNumQueries(51, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST): + with self.assertNumQueries(53, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST): with check_mongo_calls(3): url = course_updates_url(self.course) self.client.get(url) diff --git a/xmodule/capa_block.py b/xmodule/capa_block.py index 425e0d1c58..3c02053c0e 100644 --- a/xmodule/capa_block.py +++ b/xmodule/capa_block.py @@ -47,7 +47,7 @@ from xmodule.x_module import ( ) from xmodule.xml_block import XmlMixin from common.djangoapps.xblock_django.constants import ( - ATTR_KEY_ANONYMOUS_USER_ID, + ATTR_KEY_DEPRECATED_ANONYMOUS_USER_ID, ATTR_KEY_USER_IS_STAFF, ATTR_KEY_USER_ID, ) @@ -165,7 +165,6 @@ class ProblemBlock( icon_class = 'problem' uses_xmodule_styles_setup = True - requires_per_student_anonymous_id = True preview_view_js = { 'js': [ @@ -822,7 +821,7 @@ class ProblemBlock( text = self.data user_service = self.runtime.service(self, 'user') - anonymous_student_id = user_service.get_current_user().opt_attrs.get(ATTR_KEY_ANONYMOUS_USER_ID) + anonymous_student_id = user_service.get_current_user().opt_attrs.get(ATTR_KEY_DEPRECATED_ANONYMOUS_USER_ID) seed = user_service.get_current_user().opt_attrs.get(ATTR_KEY_USER_ID) or 0 sandbox_service = self.runtime.service(self, 'sandbox') diff --git a/xmodule/html_block.py b/xmodule/html_block.py index e6b42465d3..ed5d84e395 100644 --- a/xmodule/html_block.py +++ b/xmodule/html_block.py @@ -17,7 +17,8 @@ from path import Path as path from web_fragments.fragment import Fragment from xblock.core import XBlock from xblock.fields import Boolean, List, Scope, String -from common.djangoapps.xblock_django.constants import ATTR_KEY_ANONYMOUS_USER_ID + +from common.djangoapps.xblock_django.constants import ATTR_KEY_DEPRECATED_ANONYMOUS_USER_ID from xmodule.contentstore.content import StaticContent from xmodule.editing_block import EditingMixin from xmodule.edxnotes_utils import edxnotes @@ -119,7 +120,11 @@ class HtmlBlockMixin( # lint-amnesty, pylint: disable=abstract-method """ Returns html required for rendering the block. """ if self.data: data = self.data - user_id = self.runtime.service(self, 'user').get_current_user().opt_attrs.get(ATTR_KEY_ANONYMOUS_USER_ID) + user_id = ( + self.runtime.service(self, 'user') + .get_current_user() + .opt_attrs.get(ATTR_KEY_DEPRECATED_ANONYMOUS_USER_ID) + ) if user_id: data = data.replace("%%USER_ID%%", user_id) data = data.replace("%%COURSE_ID%%", str(self.scope_ids.usage_id.context_key)) @@ -150,7 +155,6 @@ class HtmlBlockMixin( # lint-amnesty, pylint: disable=abstract-method preview_view_css = {'scss': [resource_string(__name__, 'css/html/display.scss')]} uses_xmodule_styles_setup = True - requires_per_student_anonymous_id = True mako_template = "widgets/html-edit.html" resources_dir = None diff --git a/xmodule/tests/__init__.py b/xmodule/tests/__init__.py index 2a903f6f04..a68c8161c2 100644 --- a/xmodule/tests/__init__.py +++ b/xmodule/tests/__init__.py @@ -139,6 +139,7 @@ def get_test_system( user_service = StubUserService( user=user, anonymous_user_id='student', + deprecated_anonymous_user_id='student', user_is_staff=user_is_staff, user_role='student', request_country_code=user_location, @@ -202,6 +203,7 @@ def prepare_block_runtime( user_service = StubUserService( user=user, anonymous_user_id='student', + deprecated_anonymous_user_id='student', user_is_staff=user_is_staff, user_role='student', request_country_code=user_location, diff --git a/xmodule/tests/helpers.py b/xmodule/tests/helpers.py index d69be6bc00..bfe5297c2a 100644 --- a/xmodule/tests/helpers.py +++ b/xmodule/tests/helpers.py @@ -67,12 +67,14 @@ class StubUserService(UserService): user_is_staff=False, user_role=None, anonymous_user_id=None, + deprecated_anonymous_user_id=None, request_country_code=None, **kwargs): self.user = user self.user_is_staff = user_is_staff self.user_role = user_role self.anonymous_user_id = anonymous_user_id + self.deprecated_anonymous_user_id = deprecated_anonymous_user_id self.request_country_code = request_country_code self._django_user = user super().__init__(**kwargs) @@ -84,6 +86,7 @@ class StubUserService(UserService): user = XBlockUser() if self.user and self.user.is_authenticated: user.opt_attrs['edx-platform.anonymous_user_id'] = self.anonymous_user_id + user.opt_attrs['edx-platform.deprecated_anonymous_user_id'] = self.deprecated_anonymous_user_id user.opt_attrs['edx-platform.request_country_code'] = self.request_country_code user.opt_attrs['edx-platform.user_is_staff'] = self.user_is_staff user.opt_attrs['edx-platform.user_id'] = self.user.id diff --git a/xmodule/tests/test_html_block.py b/xmodule/tests/test_html_block.py index 019b32f095..9b769b252a 100644 --- a/xmodule/tests/test_html_block.py +++ b/xmodule/tests/test_html_block.py @@ -138,6 +138,7 @@ class HtmlBlockSubstitutionTestCase(unittest.TestCase): # lint-amnesty, pylint: field_data = DictFieldData({'data': sample_xml}) module_system = get_test_system(user=AnonymousUser()) block = HtmlBlock(module_system, field_data, Mock()) + block.runtime.service(block, 'user')._deprecated_anonymous_user_id = '' # pylint: disable=protected-access assert block.get_html() == sample_xml diff --git a/xmodule/video_block/video_block.py b/xmodule/video_block/video_block.py index 772a2fbcd1..46a0fc40ca 100644 --- a/xmodule/video_block/video_block.py +++ b/xmodule/video_block/video_block.py @@ -152,7 +152,6 @@ class VideoBlock( js_module_name = "TabsEditingDescriptor" uses_xmodule_styles_setup = True - requires_per_student_anonymous_id = True def get_transcripts_for_student(self, transcripts): """Return transcript information necessary for rendering the XModule student view. diff --git a/xmodule/x_module.py b/xmodule/x_module.py index 3434a80b39..b38af074e0 100644 --- a/xmodule/x_module.py +++ b/xmodule/x_module.py @@ -1071,6 +1071,9 @@ class ModuleSystemShim: Returns the anonymous user ID for the current user and course. Deprecated in favor of the user service. + + NOTE: This method returns a course-specific anonymous user ID. If you are looking for the student-specific one, + use `ATTR_KEY_DEPRECATED_ANONYMOUS_USER_ID` from the user service. """ warnings.warn( 'runtime.anonymous_student_id is deprecated. Please use the user service instead.',