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

refactor: removes calls to deprecated ModuleSystem attributes

Browse Source 
Removes references to these deprecated attributes from the platform code:
* runtime.anonymous_student_id
* runtime.seed
* runtime.user_id
* runtime.user_is_staff

Related changes:

* Ensure that all platform XBlocks which use these attributes "need" the user service.
* ProblemBlock: Removes check for existence of runtime.seed attribute in preparation for removal of this attribute from ModuleSystem.
* edxnotes: Catches NoSuchServiceError just in case some XBlocks using notes don't have the user service.
* UserTagsService refactor: pass user and course_id on creation

(cherry picked from commit 753839276f)
This commit is contained in:
Jillian Vogel
2021-08-31 16:20:53 +09:30
parent cf1064616c
commit bafc0cd91f
9 changed files with 85 additions and 46 deletions
Download Patch File Download Diff File Expand all files Collapse all files

29
common/lib/xmodule/xmodule/capa_module.py
View File

@@ -31,6 +31,11 @@ from capa.capa_problem import LoncapaProblem, LoncapaSystem
from capa.inputtypes import Status
from capa.responsetypes import LoncapaProblemError, ResponseError, StudentInputError
from capa.util import convert_files_to_filenames, get_inner_html_from_xpath
from common.djangoapps.xblock_django.constants import (
ATTR_KEY_ANONYMOUS_USER_ID,
ATTR_KEY_USER_IS_STAFF,
ATTR_KEY_USER_ID,
)
from openedx.core.djangolib.markup import HTML, Text
from xmodule.contentstore.django import contentstore
from xmodule.editing_module import EditingMixin
@@ -113,7 +118,7 @@ class Randomization(String):
to_json = from_json
@XBlock.wants('user')
@XBlock.needs('user')
@XBlock.needs('i18n')
@XBlock.wants('call_to_action')
class ProblemBlock(
@@ -784,9 +789,10 @@ class ProblemBlock(
"""
if self.rerandomize == RANDOMIZATION.NEVER:
self.seed = 1
elif self.rerandomize == RANDOMIZATION.PER_STUDENT and hasattr(self.runtime, 'seed'):
elif self.rerandomize == RANDOMIZATION.PER_STUDENT:
user_id = self.runtime.service(self, 'user').get_current_user().opt_attrs.get(ATTR_KEY_USER_ID) or 0
# see comment on randomization_bin
self.seed = randomization_bin(self.runtime.seed, str(self.location).encode('utf-8'))
self.seed = randomization_bin(user_id, str(self.location).encode('utf-8'))
else:
self.seed = struct.unpack('i', os.urandom(4))[0]
@@ -801,9 +807,13 @@ class ProblemBlock(
if text is None:
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)
seed = user_service.get_current_user().opt_attrs.get(ATTR_KEY_USER_ID) or 0
capa_system = LoncapaSystem(
ajax_url=self.ajax_url,
anonymous_student_id=self.runtime.anonymous_student_id,
anonymous_student_id=anonymous_student_id,
cache=self.runtime.cache,
can_execute_unsafe_code=self.runtime.can_execute_unsafe_code,
get_python_lib_zip=self.runtime.get_python_lib_zip,
@@ -812,7 +822,7 @@ class ProblemBlock(
i18n=self.runtime.service(self, "i18n"),
node_path=self.runtime.node_path,
render_template=self.runtime.render_template,
seed=self.runtime.seed, # Why do we do this if we have self.seed?
seed=seed, # Why do we do this if we have self.seed?
STATIC_URL=self.runtime.STATIC_URL,
xqueue=self.runtime.xqueue,
matlab_api_key=self.matlab_api_key
@@ -1412,6 +1422,7 @@ class ProblemBlock(
"""
Is the user allowed to see an answer?
"""
user_is_staff = self.runtime.service(self, 'user').get_current_user().opt_attrs.get(ATTR_KEY_USER_IS_STAFF)
if not self.correctness_available():
# If correctness is being withheld, then don't show answers either.
return False
@@ -1419,7 +1430,7 @@ class ProblemBlock(
return False
elif self.showanswer == SHOWANSWER.NEVER:
return False
elif self.runtime.user_is_staff:
elif user_is_staff:
# This is after the 'never' check because admins can see the answer
# unless the problem explicitly prevents it
return True
@@ -1459,10 +1470,11 @@ class ProblemBlock(
Limits access to the correct/incorrect flags, messages, and problem score.
"""
user_is_staff = self.runtime.service(self, 'user').get_current_user().opt_attrs.get(ATTR_KEY_USER_IS_STAFF)
return ShowCorrectness.correctness_available(
show_correctness=self.show_correctness,
due_date=self.close_date,
has_staff_access=self.runtime.user_is_staff,
has_staff_access=user_is_staff,
)
def update_score(self, data):
@@ -1777,7 +1789,8 @@ class ProblemBlock(
# If the user is a staff member, include
# the full exception, including traceback,
# in the response
if self.runtime.user_is_staff:
user_is_staff = self.runtime.service(self, 'user').get_current_user().opt_attrs.get(ATTR_KEY_USER_IS_STAFF)
if user_is_staff:
msg = f"Staff debug info: {traceback.format_exc()}"
# Otherwise, display just an error message,

7
common/lib/xmodule/xmodule/html_module.py
View File

@@ -17,6 +17,7 @@ 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 xmodule.contentstore.content import StaticContent
from xmodule.editing_module import EditingMixin
from xmodule.edxnotes_utils import edxnotes
@@ -42,6 +43,7 @@ _ = lambda text: text
@XBlock.needs("i18n")
@XBlock.needs("user")
class HtmlBlockMixin( # lint-amnesty, pylint: disable=abstract-method
XmlMixin, EditingMixin,
XModuleDescriptorToXBlockMixin, XModuleToXBlockMixin, HTMLSnippet, ResourceTemplates, XModuleMixin,
@@ -117,8 +119,9 @@ class HtmlBlockMixin( # lint-amnesty, pylint: disable=abstract-method
""" Returns html required for rendering the block. """
if self.data:
data = self.data
if getattr(self.runtime, 'anonymous_student_id', None):
data = data.replace("%%USER_ID%%", self.runtime.anonymous_student_id)
user_id = self.runtime.service(self, 'user').get_current_user().opt_attrs.get(ATTR_KEY_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))
return data
return self.data

10
common/lib/xmodule/xmodule/lti_module.py
View File

@@ -78,6 +78,7 @@ from xmodule.mako_module import MakoTemplateBlockBase
from openedx.core.djangolib.markup import HTML, Text
from xmodule.editing_module import EditingMixin
from common.djangoapps.xblock_django.constants import ATTR_KEY_ANONYMOUS_USER_ID
from xmodule.lti_2_util import LTI20BlockMixin, LTIError
from xmodule.raw_module import EmptyDataRawMixin
from xmodule.util.xmodule_django import add_webpack_to_fragment
@@ -269,6 +270,7 @@ class LTIFields:
@XBlock.needs("i18n")
@XBlock.needs("user")
class LTIBlock(
LTIFields,
LTI20BlockMixin,
@@ -529,7 +531,10 @@ class LTIBlock(
return Response(template, content_type='text/html')
def get_user_id(self):
user_id = self.runtime.anonymous_student_id
"""
Returns the current user ID, URL-escaped so it is safe to use as a URL component.
"""
user_id = self.runtime.service(self, 'user').get_current_user().opt_attrs.get(ATTR_KEY_ANONYMOUS_USER_ID)
assert user_id is not None
return str(parse.quote(user_id))
@@ -671,7 +676,8 @@ class LTIBlock(
# To test functionality test in LMS
if callable(self.runtime.get_real_user):
real_user_object = self.runtime.get_real_user(self.runtime.anonymous_student_id)
user_id = self.runtime.service(self, 'user').get_current_user().opt_attrs.get(ATTR_KEY_ANONYMOUS_USER_ID)
real_user_object = self.runtime.get_real_user(user_id)
try:
self.user_email = real_user_object.email # lint-amnesty, pylint: disable=attribute-defined-outside-init
except AttributeError:

22
common/lib/xmodule/xmodule/seq_module.py
View File

@@ -33,6 +33,7 @@ from xmodule.x_module import (
XModuleToXBlockMixin,
)
from common.djangoapps.xblock_django.constants import ATTR_KEY_USER_ID, ATTR_KEY_USER_IS_STAFF
from openedx.core.djangoapps.agreements.toggles import is_integrity_signature_enabled
from .exceptions import NotFoundError
@@ -378,7 +379,7 @@ class SequenceBlock(
is_hidden_after_due = False
if self._required_prereq():
if self.runtime.user_is_staff:
if self.runtime.service(self, 'user').get_current_user().opt_attrs.get(ATTR_KEY_USER_IS_STAFF):
banner_text = _(
'This subsection is unlocked for learners when they meet the prerequisite requirements.'
)
@@ -459,7 +460,7 @@ class SequenceBlock(
prereq_met = True
prereq_meta_info = {}
if self._required_prereq():
if self.runtime.user_is_staff:
if self.runtime.service(self, 'user').get_current_user().opt_attrs.get(ATTR_KEY_USER_IS_STAFF):
banner_text = _(
'This subsection is unlocked for learners when they meet the prerequisite requirements.'
)
@@ -553,7 +554,7 @@ class SequenceBlock(
"""
hidden_date = course.end if course.self_paced else self.due
return (
self.runtime.user_is_staff or
self.runtime.service(self, 'user').get_current_user().opt_attrs.get(ATTR_KEY_USER_IS_STAFF) or
self.verify_current_content_visibility(hidden_date, self.hide_after_due)
)
@@ -643,8 +644,9 @@ class SequenceBlock(
"""
gating_service = self.runtime.service(self, 'gating')
if gating_service:
user_id = self.runtime.service(self, 'user').get_current_user().opt_attrs.get(ATTR_KEY_USER_ID)
fulfilled = gating_service.is_gate_fulfilled(
self.course_id, self.location, self.runtime.user_id
self.course_id, self.location, user_id
)
return fulfilled
@@ -692,7 +694,8 @@ class SequenceBlock(
comes to determining whether a student is allowed to access this,
with other checks being done in has_access calls.
"""
if self.runtime.user_is_staff or context.get('specific_masquerade', False):
user_is_staff = self.runtime.service(self, 'user').get_current_user().opt_attrs.get(ATTR_KEY_USER_IS_STAFF)
if user_is_staff or context.get('specific_masquerade', False):
return False
# We're not allowed to see it because of pre-reqs that haven't been
@@ -723,7 +726,8 @@ class SequenceBlock(
"""
gating_service = self.runtime.service(self, 'gating')
if gating_service:
return gating_service.compute_is_prereq_met(self.location, self.runtime.user_id, recalc_on_unmet)
user_id = self.runtime.service(self, 'user').get_current_user().opt_attrs.get(ATTR_KEY_USER_ID)
return gating_service.compute_is_prereq_met(self.location, user_id, recalc_on_unmet)
return True, {}
@@ -915,8 +919,10 @@ class SequenceBlock(
self.is_time_limited
)
if feature_enabled:
user_id = self.runtime.user_id
user_role_in_course = 'staff' if self.runtime.user_is_staff else 'student'
current_user = self.runtime.service(self, 'user').get_current_user()
user_id = current_user.opt_attrs.get(ATTR_KEY_USER_ID)
user_is_staff = current_user.opt_attrs.get(ATTR_KEY_USER_IS_STAFF)
user_role_in_course = 'staff' if user_is_staff else 'student'
course_id = self.runtime.course_id
content_id = self.location

7
lms/djangoapps/courseware/module_render.py
View File

@@ -41,6 +41,7 @@ from xblock.reference.plugins import FSService
from xblock.runtime import KvsFieldData
from common.djangoapps import static_replace
from common.djangoapps.xblock_django.constants import ATTR_KEY_USER_ID
from capa.xqueue_interface import XQueueInterface
from lms.djangoapps.courseware.access import get_user_role, has_access
from lms.djangoapps.courseware.entrance_exams import user_can_skip_entrance_exam, user_has_passed_entrance_exam
@@ -564,8 +565,10 @@ def get_module_system_for_user(
handle_event(block, event)
else:
context = contexts.course_context_from_course_id(course_id)
if block.runtime.user_id:
context['user_id'] = block.runtime.user_id
user_id = user_service.get_current_user().opt_attrs.get(ATTR_KEY_USER_ID)
if user_id:
context['user_id'] = user_id
context['asides'] = {}
for aside in block.runtime.get_asides(block):
if hasattr(aside, 'get_event_context'):

7
lms/djangoapps/courseware/tests/test_module_render.py
View File

@@ -45,6 +45,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 lms.djangoapps.courseware import module_render as render
from lms.djangoapps.courseware.access_response import AccessResponse
from lms.djangoapps.courseware.courses import get_course_info_section, get_course_with_access
@@ -1923,7 +1924,7 @@ class TestAnonymousStudentId(SharedModuleStoreTestCase, LoginEnrollmentTestCase)
if hasattr(xblock_class, 'module_class'):
descriptor.module_class = xblock_class.module_class
return render.get_module_for_descriptor_internal(
module = render.get_module_for_descriptor_internal(
user=self.user,
descriptor=descriptor,
student_data=Mock(spec=FieldData, name='student_data'),
@@ -1932,7 +1933,9 @@ class TestAnonymousStudentId(SharedModuleStoreTestCase, LoginEnrollmentTestCase)
xqueue_callback_url_prefix=Mock(name='xqueue_callback_url_prefix'), # XQueue Callback Url Prefix
request_token='request_token',
course=self.course,
).xmodule_runtime.anonymous_student_id
)
current_user = module.xmodule_runtime.service(module, 'user').get_current_user()
return current_user.opt_attrs.get(ATTR_KEY_ANONYMOUS_USER_ID)
@ddt.data(*PER_STUDENT_ANONYMIZED_DESCRIPTORS)
def test_per_student_anonymized_id(self, descriptor_class):

8
lms/djangoapps/edxnotes/decorators.py
View File

@@ -6,8 +6,10 @@ Decorators related to edXNotes.
import json
from django.conf import settings
from xblock.exceptions import NoSuchServiceError
from common.djangoapps.edxmako.shortcuts import render_to_string
from common.djangoapps.xblock_django.constants import ATTR_KEY_ANONYMOUS_USER_ID
def edxnotes(cls):
@@ -40,7 +42,11 @@ def edxnotes(cls):
# - Harvard Annotation Tool is enabled for the course
# - the feature flag or `edxnotes` setting of the course is set to False
# - the user is not authenticated
user = self.runtime.get_real_user(self.runtime.anonymous_student_id)
try:
user_id = self.runtime.service(self, 'user').get_current_user().opt_attrs.get(ATTR_KEY_ANONYMOUS_USER_ID)
user = self.runtime.get_real_user(user_id)
except NoSuchServiceError:
user = None
if is_studio or not is_feature_enabled(course, user):
return original_get_html(self, *args, **kwargs)

32
lms/djangoapps/lms_xblock/runtime.py
View File

@@ -98,13 +98,9 @@ class UserTagsService:
COURSE_SCOPE = user_course_tag_api.COURSE_SCOPE
def __init__(self, runtime):
self.runtime = runtime
def _get_current_user(self):
"""Returns the real, not anonymized, current user."""
real_user = self.runtime.get_real_user(self.runtime.anonymous_student_id)
return real_user
def __init__(self, user, course_id):
self._user = user
self._course_id = course_id
def get_tag(self, scope, key):
"""
@@ -117,8 +113,8 @@ class UserTagsService:
raise ValueError(f"unexpected scope {scope}")
return user_course_tag_api.get_course_tag(
self._get_current_user(),
self.runtime.course_id, key
self._user,
self._course_id, key
)
def set_tag(self, scope, key, value):
@@ -133,8 +129,8 @@ class UserTagsService:
raise ValueError(f"unexpected scope {scope}")
return user_course_tag_api.set_course_tag(
self._get_current_user(),
self.runtime.course_id, key, value
self._user,
self._course_id, key, value
)
@@ -142,24 +138,28 @@ class LmsModuleSystem(ModuleSystem): # pylint: disable=abstract-method
"""
ModuleSystem specialized to the LMS
"""
def __init__(self, user=None, **kwargs):
def __init__(self, user, **kwargs):
request_cache_dict = DEFAULT_REQUEST_CACHE.data
store = modulestore()
course_id = kwargs.get('course_id')
services = kwargs.setdefault('services', {})
if user and user.is_authenticated:
services['completion'] = CompletionService(user=user, context_key=kwargs.get('course_id'))
services['completion'] = CompletionService(user=user, context_key=course_id)
services['fs'] = xblock.reference.plugins.FSService()
services['i18n'] = ModuleI18nService
services['library_tools'] = LibraryToolsService(store, user_id=user.id if user else None)
services['partitions'] = PartitionService(
course_id=kwargs.get('course_id'),
course_id=course_id,
cache=request_cache_dict
)
services['settings'] = SettingsService()
services['user_tags'] = UserTagsService(self)
services['user_tags'] = UserTagsService(
user=user,
course_id=course_id,
)
if badges_enabled():
services['badging'] = BadgingService(course_id=kwargs.get('course_id'), modulestore=store)
services['badging'] = BadgingService(course_id=course_id, modulestore=store)
self.request_token = kwargs.pop('request_token', None)
services['teams'] = TeamsService()
services['teams_configuration'] = TeamsConfigurationService()

9
lms/djangoapps/lms_xblock/test/test_runtime.py
View File

@@ -66,6 +66,7 @@ class TestHandlerUrl(TestCase):
render_template=Mock(),
replace_urls=str,
course_id=self.course_key,
user=Mock(),
descriptor_runtime=Mock(),
)
@@ -125,18 +126,14 @@ class TestUserServiceAPI(TestCase):
self.course_id = CourseLocator("org", "course", "run")
self.user = UserFactory.create()
def mock_get_real_user(_anon_id):
"""Just returns the test user"""
return self.user
self.runtime = LmsModuleSystem(
static_url='/static',
track_function=Mock(),
get_module=Mock(),
render_template=Mock(),
replace_urls=str,
user=self.user,
course_id=self.course_id,
get_real_user=mock_get_real_user,
descriptor_runtime=Mock(),
)
self.scope = 'course'
@@ -192,6 +189,7 @@ class TestBadgingService(ModuleStoreTestCase):
render_template=Mock(),
replace_urls=str,
course_id=self.course_id,
user=self.user,
get_real_user=mock_get_real_user,
descriptor_runtime=Mock(),
)
@@ -247,6 +245,7 @@ class TestI18nService(ModuleStoreTestCase):
render_template=Mock(),
replace_urls=str,
course_id=self.course.id,
user=Mock(),
descriptor_runtime=Mock(),
)