Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #32165 from open-craft/agrendalath/bd-13-fix_anonymous_student_ids

Browse Source 
fix: anonymous student IDs [BD-13]
This commit is contained in:
Jeremy Ristau
2023-05-02 12:32:47 -04:00
committed by GitHub
parent 3be9297abf aa85257b19
commit bdd2a0d212
16 changed files with 72 additions and 44 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
cms/djangoapps/contentstore/views/preview.py
View File

@@ -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(),

14
cms/djangoapps/contentstore/views/tests/test_preview.py
View File

@@ -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'

1
common/djangoapps/xblock_django/constants.py
View File

@@ -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'

6
common/djangoapps/xblock_django/user_service.py
View File

@@ -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

15
lms/djangoapps/courseware/block_render.py
View File

@@ -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,
)

27
lms/djangoapps/courseware/tests/test_block_render.py
View File

@@ -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(

2
openedx/core/djangoapps/schedules/tests/test_resolvers.py
View File

@@ -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'

8
openedx/core/djangoapps/xblock/runtime/runtime.py
View File

@@ -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:

2
openedx/features/course_experience/tests/views/test_course_updates.py
View File

@@ -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)

5
xmodule/capa_block.py
View File

@@ -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')

10
xmodule/html_block.py
View File

@@ -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

2
xmodule/tests/__init__.py
View File

@@ -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,

3
xmodule/tests/helpers.py
View File

@@ -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

1
xmodule/tests/test_html_block.py
View File

@@ -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

1
xmodule/video_block/video_block.py
View File

@@ -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.

3
xmodule/x_module.py
View File

@@ -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.',