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

refactor: reuse services and wrappers between XBlocks

Browse Source
This commit is contained in:
Agrendalath
2023-06-22 12:11:29 +02:00
committed by Piotr Surowiec
parent 80374ed1ce
commit 36cc415fc2
19 changed files with 253 additions and 247 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -220,7 +220,7 @@ def _prepare_runtime_for_preview(request, block):
# Set up functions to modify the fragment produced by student_view
block.runtime.wrappers = wrappers
block.runtime.wrappers_asides = wrappers_asides
block.runtime._runtime_services.update(services) # lint-amnesty, pylint: disable=protected-access
block.runtime._services.update(services) # pylint: disable=protected-access
# xmodules can check for this attribute during rendering to determine if
# they are being rendered for preview (i.e. in Studio)

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

@@ -2,7 +2,7 @@
Block rendering
"""
from __future__ import annotations
import json
import logging
import textwrap
@@ -12,7 +12,7 @@ from functools import partial
from completion.services import CompletionService
from django.conf import settings
from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user
from django.contrib.auth.models import AnonymousUser, User # lint-amnesty, pylint: disable=imported-auth-user
from django.core.cache import cache
from django.db import transaction
from django.http import Http404, HttpResponse, HttpResponseForbidden
@@ -33,6 +33,7 @@ from opaque_keys import InvalidKeyError
from opaque_keys.edx.keys import CourseKey, UsageKey
from rest_framework.decorators import api_view
from rest_framework.exceptions import APIException
from typing import Callable, TYPE_CHECKING
from web_fragments.fragment import Fragment
from xblock.django.request import django_to_webob_request, webob_to_django_response
from xblock.exceptions import NoSuchHandlerError, NoSuchViewError
@@ -97,6 +98,13 @@ from common.djangoapps.edxmako.services import MakoService
from common.djangoapps.xblock_django.user_service import DjangoXBlockUserService
from openedx.core.lib.cache_utils import CacheService
if TYPE_CHECKING:
from rest_framework.request import Request
from xblock.core import XBlock
from xblock.runtime import Runtime
from xmodule.course_block import CourseBlock
log = logging.getLogger(__name__)
# TODO: course_id and course_key are used interchangeably in this file, which is wrong.
@@ -289,8 +297,9 @@ def get_block(user, request, usage_key, field_data_cache, position=None, log_if_
and such works based on user.
- usage_key : A UsageKey object identifying the module to load
- field_data_cache : a FieldDataCache
- position : extra information from URL for user-specified
position within module
- position : Extra information from URL for user-specified position within module.
It is used to determine the active tab within the `SequenceBlock`/subsection.
Once the legacy course experience is removed, it should be safe to remove this, too.
- log_if_not_found : If this is True, we log a debug message if we cannot find the requested xmodule.
- wrap_xblock_display : If this is True, wrap the output display in a single div to allow for the
XModule javascript to be bound correctly
@@ -364,10 +373,24 @@ def display_access_messages(user, block, view, frag, context): # pylint: disabl
# pylint: disable=too-many-statements
def get_block_for_descriptor(user, request, block, field_data_cache, course_key,
position=None, wrap_xblock_display=True, grade_bucket_type=None,
static_asset_path='', disable_staff_debug_info=False,
course=None, will_recheck_access=False):
def get_block_for_descriptor(
user: User | AnonymousUser,
request: Request | None,
block: XBlock,
field_data_cache: FieldDataCache | None,
course_key: CourseKey,
position: int | None = None,
wrap_xblock_display: bool = True,
grade_bucket_type: str | None = None,
static_asset_path: str = '',
disable_staff_debug_info: bool = False,
course: CourseBlock | None = None,
will_recheck_access: bool = False,
track_function: Callable[[str, dict], None] | None = None,
student_data: KvsFieldData | None = None,
request_token: str | None = None,
user_location: str | None = None,
) -> XBlock | None:
"""
Implements get_block, extracting out the request-specific functionality.
@@ -375,49 +398,107 @@ def get_block_for_descriptor(user, request, block, field_data_cache, course_key,
See get_block() docstring for further details.
"""
track_function = make_track_function(request)
if request:
track_function = track_function or make_track_function(request)
user_location = user_location or getattr(request, 'session', {}).get('country_code')
request_token = request_token or xblock_request_token(request)
user_location = getattr(request, 'session', {}).get('country_code')
if not student_data:
student_kvs = DjangoKeyValueStore(field_data_cache)
if is_masquerading_as_specific_student(user, course_key):
student_kvs = MasqueradingKeyValueStore(student_kvs, request.session)
student_data = KvsFieldData(student_kvs)
student_kvs = DjangoKeyValueStore(field_data_cache)
if is_masquerading_as_specific_student(user, course_key):
student_kvs = MasqueradingKeyValueStore(student_kvs, request.session)
student_data = KvsFieldData(student_kvs)
# The runtime is already shared between XBlocks. If there are no wrappers, it is the first initialization.
should_recreate_runtime = not block.runtime.wrappers
return get_block_for_descriptor_internal(
user=user,
block=block,
student_data=student_data,
course_id=course_key,
track_function=track_function,
position=position,
wrap_xblock_display=wrap_xblock_display,
grade_bucket_type=grade_bucket_type,
static_asset_path=static_asset_path,
user_location=user_location,
request_token=xblock_request_token(request),
disable_staff_debug_info=disable_staff_debug_info,
course=course,
will_recheck_access=will_recheck_access,
# If the runtime was prepared for another user, it should be recreated.
# This part can be removed if we remove all user-specific handling from the runtime services and pull this
# information directly from XBlocks during the service initialization.
if not should_recreate_runtime:
# If the user service is absent (which should never happen), the runtime should be reinitialized.
# We retrieve this service directly to bypass service declaration checks.
if user_service := block.runtime._services.get('user'): # pylint: disable=protected-access
# Check the user ID bound to the runtime. This operation can run often for complex course structures, so we
# are accessing the protected attribute of the user service to reduce the number of queries.
should_recreate_runtime = user.id != user_service._django_user.id # pylint: disable=protected-access
else:
should_recreate_runtime = True
if should_recreate_runtime:
prepare_runtime_for_user(
user=user,
student_data=student_data, # These have implicit user bindings, the rest of args are considered not to
runtime=block.runtime,
course_id=course_key,
track_function=track_function,
wrap_xblock_display=wrap_xblock_display,
grade_bucket_type=grade_bucket_type,
static_asset_path=static_asset_path,
user_location=user_location,
request_token=request_token,
disable_staff_debug_info=disable_staff_debug_info,
course=course,
will_recheck_access=will_recheck_access,
)
# Pass position specified in URL to runtime.
if position is not None:
try:
position = int(position)
except (ValueError, TypeError):
log.exception('Non-integer %r passed as position.', position)
position = None
block.runtime.set('position', position)
block.bind_for_student(
user.id,
[
partial(DateLookupFieldData, course_id=course_key, user=user),
partial(OverrideFieldData.wrap, user, course),
partial(LmsFieldData, student_data=student_data),
],
)
# Do not check access when it's a noauth request.
# Not that the access check needs to happen after the block is bound
# for the student, since there may be field override data for the student
# that affects xblock visibility.
user_needs_access_check = getattr(user, 'known', True) and not isinstance(user, SystemUser)
if user_needs_access_check:
access = has_access(user, 'load', block, course_key)
# A block should only be returned if either the user has access, or the user doesn't have access, but
# the failed access has a message for the user and the caller of this function specifies it will check access
# again. This allows blocks to show specific error message or upsells when access is denied.
caller_will_handle_access_error = (
not access
and will_recheck_access
and (access.user_message or access.user_fragment)
)
if access or caller_will_handle_access_error:
block.has_access_error = bool(caller_will_handle_access_error)
return block
return None
return block
def prepare_runtime_for_user(
user,
student_data, # TODO
user: User | AnonymousUser,
student_data: KvsFieldData,
# Arguments preceding this comment have user binding, those following don't
block,
course_id,
track_function,
request_token,
position=None,
wrap_xblock_display=True,
grade_bucket_type=None,
static_asset_path='',
user_location=None,
disable_staff_debug_info=False,
course=None,
will_recheck_access=False,
runtime: Runtime,
course_id: CourseKey,
track_function: Callable[[str, dict], None],
request_token: str,
position: int | None = None,
wrap_xblock_display: bool = True,
grade_bucket_type: str | None = None,
static_asset_path: str = '',
user_location: str | None = None,
disable_staff_debug_info: bool = False,
course: CourseBlock | None = None,
will_recheck_access: bool = False,
):
"""
Helper function that binds the given xblock to a user and student_data to a user and the block.
@@ -427,25 +508,26 @@ def prepare_runtime_for_user(
The arguments fall into two categories: those that have explicit or implicit user binding, which are user
and student_data, and those don't and are used to instantiate the service required in LMS, which
are all the other arguments. Ultimately, this isn't too different than how get_block_for_descriptor_internal
was before refactoring.
are all the other arguments.
Arguments:
see arguments for get_block()
request_token (str): A token unique to the request use by xblock initialization
"""
def inner_get_block(block):
def inner_get_block(block: XBlock) -> XBlock | None:
"""
Delegate to get_block_for_descriptor_internal() with all values except `block` set.
Delegate to get_block_for_descriptor() with all values except `block` set.
Because it does an access check, it may return None.
"""
return get_block_for_descriptor_internal(
return get_block_for_descriptor(
user=user,
request=None,
field_data_cache=None,
block=block,
student_data=student_data,
course_id=course_id,
course_key=course_id,
track_function=track_function,
request_token=request_token,
position=position,
@@ -479,16 +561,6 @@ def prepare_runtime_for_user(
request_token=request_token,
))
# HACK: The following test fails when we do not access this attribute.
# lms/djangoapps/courseware/tests/test_views.py::TestRenderXBlock::test_success_enrolled_staff
# This happens because accessing `field_data` caches the XBlock's parents through the inheritance mixin.
# If we do this operation before assigning `inner_get_block` to the `get_block_for_descriptor` attribute, these
# parents will be initialized without binding the user data (that's how the `get_block` works in `x_module.py`).
# If we retrieve the parents after this operation (e.g., in the `enclosing_sequence_for_gating_checks` function),
# they will have their runtimes initialized, which can lead to an altered behavior of the XBlock-specific
# parts of other runtimes. We will keep this workaround until we remove all XBlock-specific code from here.
block.static_asset_path # pylint: disable=pointless-statement
replace_url_service = partial(
ReplaceURLService,
static_asset_path=static_asset_path,
@@ -553,7 +625,6 @@ def prepare_runtime_for_user(
'rebind_user': RebindUserService(
user,
course_id,
prepare_runtime_for_user,
track_function=track_function,
position=position,
wrap_xblock_display=wrap_xblock_display,
@@ -576,86 +647,13 @@ def prepare_runtime_for_user(
'publish': EventPublishingService(user, course_id, track_function),
}
block.runtime.get_block_for_descriptor = inner_get_block
runtime.get_block_for_descriptor = inner_get_block
block.runtime.wrappers = block_wrappers
block.runtime._runtime_services.update(services) # lint-amnesty, pylint: disable=protected-access
block.runtime.request_token = request_token
block.runtime.wrap_asides_override = lms_wrappers_aside
block.runtime.applicable_aside_types_override = lms_applicable_aside_types
# pass position specified in URL to runtime
if position is not None:
try:
position = int(position)
except (ValueError, TypeError):
log.exception('Non-integer %r passed as position.', position)
position = None
block.runtime.set('position', position)
# TODO: Find all the places that this method is called and figure out how to
# get a loaded course passed into it
def get_block_for_descriptor_internal(user, block, student_data, course_id, track_function, request_token,
position=None, wrap_xblock_display=True, grade_bucket_type=None,
static_asset_path='', user_location=None, disable_staff_debug_info=False,
course=None, will_recheck_access=False):
"""
Actually implement get_block, without requiring a request.
See get_block() docstring for further details.
Arguments:
request_token (str): A unique token for this request, used to isolate xblock rendering
"""
prepare_runtime_for_user(
user=user,
student_data=student_data, # These have implicit user bindings, the rest of args are considered not to
block=block,
course_id=course_id,
track_function=track_function,
position=position,
wrap_xblock_display=wrap_xblock_display,
grade_bucket_type=grade_bucket_type,
static_asset_path=static_asset_path,
user_location=user_location,
request_token=request_token,
disable_staff_debug_info=disable_staff_debug_info,
course=course,
will_recheck_access=will_recheck_access,
)
block.bind_for_student(
user.id,
[
partial(DateLookupFieldData, course_id=course_id, user=user),
partial(OverrideFieldData.wrap, user, course),
partial(LmsFieldData, student_data=student_data),
],
)
# Do not check access when it's a noauth request.
# Not that the access check needs to happen after the block is bound
# for the student, since there may be field override data for the student
# that affects xblock visibility.
user_needs_access_check = getattr(user, 'known', True) and not isinstance(user, SystemUser)
if user_needs_access_check:
access = has_access(user, 'load', block, course_id)
# A block should only be returned if either the user has access, or the user doesn't have access, but
# the failed access has a message for the user and the caller of this function specifies it will check access
# again. This allows blocks to show specific error message or upsells when access is denied.
caller_will_handle_access_error = (
not access
and will_recheck_access
and (access.user_message or access.user_fragment)
)
if access or caller_will_handle_access_error:
block.has_access_error = bool(caller_will_handle_access_error)
return block
return None
return block
runtime.wrappers = block_wrappers
runtime._services.update(services) # lint-amnesty, pylint: disable=protected-access
runtime.request_token = request_token
runtime.wrap_asides_override = lms_wrappers_aside
runtime.applicable_aside_types_override = lms_applicable_aside_types
def load_single_xblock(request, user_id, course_id, usage_key_string, course=None, will_recheck_access=False):

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

@@ -949,7 +949,7 @@ class TestHandleXBlockCallback(SharedModuleStoreTestCase, LoginEnrollmentTestCas
request.user = self.mock_user
render.handle_xblock_callback(request, str(course.id), usage_id, handler)
assert mock_get_block.call_count == 1
assert mock_get_block.call_count == 2
assert mock_get_block.call_args[1]['will_recheck_access'] == will_recheck_access
@@ -1922,14 +1922,16 @@ class TestAnonymousStudentId(SharedModuleStoreTestCase, LoginEnrollmentTestCase)
if hasattr(xblock_class, 'module_class'):
block.module_class = xblock_class.module_class
rendered_block = render.get_block_for_descriptor_internal(
rendered_block = render.get_block_for_descriptor(
user=self.user,
block=block,
student_data=Mock(spec=FieldData, name='student_data'),
course_id=course_id,
course_key=course_id,
track_function=Mock(name='track_function'), # Track Function
request_token='request_token',
course=self.course,
request=None,
field_data_cache=None,
)
current_user = rendered_block.runtime.service(rendered_block, 'user').get_current_user()
@@ -2285,7 +2287,7 @@ class LMSXBlockServiceMixin(SharedModuleStoreTestCase):
render.prepare_runtime_for_user(
self.user,
self.student_data,
self.block,
self.block.runtime,
self.course.id,
self.track_function,
self.request_token,
@@ -2439,7 +2441,6 @@ class TestI18nService(LMSXBlockServiceMixin):
Test: NoSuchServiceError should be raised if i18n service is none.
"""
i18nService = self.block.runtime._services['i18n'] # pylint: disable=protected-access
self.block.runtime._runtime_services['i18n'] = None # pylint: disable=protected-access
self.block.runtime._services['i18n'] = None # pylint: disable=protected-access
with pytest.raises(NoSuchServiceError):
self.block.runtime.service(self.block, 'i18n')
@@ -2656,7 +2657,7 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase):
render.prepare_runtime_for_user(
self.user,
self.student_data,
self.block,
self.block.runtime,
self.course.id,
self.track_function,
self.request_token,
@@ -2684,7 +2685,7 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase):
render.prepare_runtime_for_user(
self.user,
self.student_data,
self.block,
self.block.runtime,
self.course.id,
self.track_function,
self.request_token,
@@ -2706,7 +2707,7 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase):
render.prepare_runtime_for_user(
self.user,
self.student_data,
self.block,
self.block.runtime,
self.course.id,
self.track_function,
self.request_token,
@@ -2726,7 +2727,7 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase):
render.prepare_runtime_for_user(
self.user,
self.student_data,
self.block,
self.block.runtime,
self.course.id,
self.track_function,
self.request_token,
@@ -2747,7 +2748,7 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase):
render.prepare_runtime_for_user(
self.user,
self.student_data,
self.block,
self.block.runtime,
self.course.id,
self.track_function,
self.request_token,
@@ -2776,7 +2777,7 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase):
render.prepare_runtime_for_user(
self.user,
self.student_data,
self.problem_block,
self.problem_block.runtime,
self.course.id,
self.track_function,
self.request_token,
@@ -2790,7 +2791,7 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase):
render.prepare_runtime_for_user(
self.user,
self.student_data,
self.block,
self.block.runtime,
self.course.id,
self.track_function,
self.request_token,
@@ -2810,7 +2811,7 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase):
render.prepare_runtime_for_user(
AnonymousUser(),
self.student_data,
self.block,
self.block.runtime,
self.course.id,
self.track_function,
self.request_token,
@@ -2839,7 +2840,7 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase):
render.prepare_runtime_for_user(
self.user,
self.student_data,
self.block,
self.block.runtime,
self.course.id,
self.track_function,
self.request_token,

26
lms/djangoapps/courseware/tests/test_discussion_xblock.py
View File

@@ -21,7 +21,7 @@ from xmodule.modulestore.tests.django_utils import TEST_DATA_SPLIT_MODULESTORE,
from xmodule.modulestore.tests.factories import BlockFactory, ToyCourseFactory
from lms.djangoapps.course_api.blocks.tests.helpers import deserialize_usage_key
from lms.djangoapps.courseware.block_render import get_block_for_descriptor_internal
from lms.djangoapps.courseware.block_render import get_block_for_descriptor
from lms.djangoapps.courseware.tests.helpers import XModuleRenderingTestBase
from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration, Provider
from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory
@@ -293,13 +293,15 @@ class TestXBlockInCourse(SharedModuleStoreTestCase):
"""
Test rendered DiscussionXBlock permissions.
"""
discussion_xblock = get_block_for_descriptor_internal(
discussion_xblock = get_block_for_descriptor(
user=self.user,
block=self.discussion,
student_data=mock.Mock(name='student_data'),
course_id=self.course.id,
course_key=self.course.id,
track_function=mock.Mock(name='track_function'),
request_token='request_token',
request=None,
field_data_cache=None,
)
fragment = discussion_xblock.render('student_view')
@@ -336,13 +338,15 @@ class TestXBlockInCourse(SharedModuleStoreTestCase):
assert orphan_sequential.location.block_id == root.location.block_id
# Get xblock bound to a user and a block.
discussion_xblock = get_block_for_descriptor_internal(
discussion_xblock = get_block_for_descriptor(
user=self.user,
block=discussion,
student_data=mock.Mock(name='student_data'),
course_id=self.course.id,
course_key=self.course.id,
track_function=mock.Mock(name='track_function'),
request_token='request_token',
request=None,
field_data_cache=None,
)
fragment = discussion_xblock.render('student_view')
@@ -386,13 +390,15 @@ class TestXBlockInCourse(SharedModuleStoreTestCase):
provider_type=Provider.OPEN_EDX,
)
discussion_xblock = get_block_for_descriptor_internal(
discussion_xblock = get_block_for_descriptor(
user=self.user,
block=self.discussion,
student_data=mock.Mock(name='student_data'),
course_id=self.course.id,
course_key=self.course.id,
track_function=mock.Mock(name='track_function'),
request_token='request_token',
request=None,
field_data_cache=None,
)
fragment = discussion_xblock.render('student_view')
@@ -436,13 +442,15 @@ class TestXBlockQueryLoad(SharedModuleStoreTestCase):
num_queries = 6
for discussion in discussions:
discussion_xblock = get_block_for_descriptor_internal(
discussion_xblock = get_block_for_descriptor(
user=user,
block=discussion,
student_data=mock.Mock(name='student_data'),
course_id=course.id,
course_key=course.id,
track_function=mock.Mock(name='track_function'),
request_token='request_token',
request=None,
field_data_cache=None,
)
with self.assertNumQueries(num_queries):
fragment = discussion_xblock.render('student_view')

18
lms/djangoapps/courseware/testutils.py
View File

@@ -44,9 +44,8 @@ class RenderXBlockTestMixin(MasqueradeMixin, metaclass=ABCMeta):
'<div class="container"',
]
# DOM elements that appear in an xBlock,
# but are excluded from the xBlock-only rendering.
XBLOCK_REMOVED_HTML_ELEMENTS = [
# DOM elements that should only be present when viewing the XBlock as staff.
XBLOCK_STAFF_DEBUG_INFO = [
'<div class="wrap-instructor-info"',
]
@@ -143,14 +142,14 @@ class RenderXBlockTestMixin(MasqueradeMixin, metaclass=ABCMeta):
if login:
self.login()
def verify_response(self, expected_response_code=200, url_params=None):
def verify_response(self, expected_response_code=200, url_params=None, is_staff=False):
"""
Helper method that calls the endpoint, verifies the expected response code, and returns the response.
Arguments:
expected_response_code: The expected response code.
url_params: URL parameters that will be encoded and passed to the request.
is_staff: Whether the user has staff permissions in the course.
"""
if url_params:
url_params = urlencode(url_params)
@@ -158,8 +157,9 @@ class RenderXBlockTestMixin(MasqueradeMixin, metaclass=ABCMeta):
response = self.get_response(self.block_to_be_tested.location, url_params)
if expected_response_code == 200:
self.assertContains(response, self.html_block.data, status_code=expected_response_code)
unexpected_elements = self.block_specific_chrome_html_elements
unexpected_elements += self.COURSEWARE_CHROME_HTML_ELEMENTS + self.XBLOCK_REMOVED_HTML_ELEMENTS
unexpected_elements = self.block_specific_chrome_html_elements + self.COURSEWARE_CHROME_HTML_ELEMENTS
if not is_staff:
unexpected_elements += self.XBLOCK_STAFF_DEBUG_INFO
for chrome_element in unexpected_elements:
self.assertNotContains(response, chrome_element)
else:
@@ -202,12 +202,12 @@ class RenderXBlockTestMixin(MasqueradeMixin, metaclass=ABCMeta):
# (3) definition - HTML block
# (4) definition - edx_notes decorator (original_get_html)
with check_mongo_calls(4):
self.verify_response()
self.verify_response(is_staff=True)
def test_success_unenrolled_staff(self):
self.setup_course()
self.setup_user(admin=True, enroll=False, login=True)
self.verify_response()
self.verify_response(is_staff=True)
def test_success_unenrolled_staff_masquerading_as_student(self):
self.setup_course()

10
lms/djangoapps/courseware/views/views.py
View File

@@ -1521,7 +1521,7 @@ def _check_sequence_exam_access(request, location):
@xframe_options_exempt
@transaction.non_atomic_requests
@ensure_csrf_cookie
def render_xblock(request, usage_key_string, check_if_enrolled=True):
def render_xblock(request, usage_key_string, check_if_enrolled=True, disable_staff_debug_info=False):
"""
Returns an HttpResponse with HTML content for the xBlock with the given usage_key.
The returned HTML is a chromeless rendering of the xBlock (excluding content of the containing courseware).
@@ -1571,8 +1571,12 @@ def render_xblock(request, usage_key_string, check_if_enrolled=True):
# get the block, which verifies whether the user has access to the block.
recheck_access = request.GET.get('recheck_access') == '1'
block, _ = get_block_by_usage_id(
request, str(course_key), str(usage_key), disable_staff_debug_info=True, course=course,
will_recheck_access=recheck_access
request,
str(course_key),
str(usage_key),
disable_staff_debug_info=disable_staff_debug_info,
course=course,
will_recheck_access=recheck_access,
)
student_view_context = request.GET.dict()

24
lms/djangoapps/instructor_task/tasks_helper/module_state.py
View File

@@ -9,7 +9,6 @@ from time import time
from django.utils.translation import gettext_noop
from opaque_keys.edx.keys import UsageKey
from xblock.runtime import KvsFieldData
from xblock.scorable import Score
from xmodule.capa.responsetypes import LoncapaProblemError, ResponseError, StudentInputError
@@ -18,9 +17,9 @@ from common.djangoapps.track.event_transaction_utils import create_new_event_tra
from common.djangoapps.track.views import task_track
from common.djangoapps.util.db import outer_atomic
from lms.djangoapps.courseware.courses import get_problems_in_section
from lms.djangoapps.courseware.model_data import DjangoKeyValueStore, FieldDataCache
from lms.djangoapps.courseware.model_data import FieldDataCache
from lms.djangoapps.courseware.models import StudentModule
from lms.djangoapps.courseware.block_render import get_block_for_descriptor_internal
from lms.djangoapps.courseware.block_render import get_block_for_descriptor
from lms.djangoapps.grades.api import events as grades_events
from openedx.core.lib.courses import get_course_by_id
from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order
@@ -334,16 +333,10 @@ def _get_module_instance_for_task(course_id, student, block, xblock_instance_arg
"""
Fetches a StudentModule instance for a given `course_id`, `student` object, and `block`.
`xblock_instance_args` is used to provide information for creating a track function and an XQueue callback.
These are passed, along with `grade_bucket_type`, to get_block_for_descriptor_internal, which sidesteps
the need for a Request object when instantiating an xblock instance.
`xblock_instance_args` is used to provide information for creating a track function.
It is passed, along with `grade_bucket_type`, to get_block_for_descriptor.
"""
# reconstitute the problem's corresponding XBlock:
field_data_cache = FieldDataCache.cache_for_block_descendents(course_id, student, block)
student_data = KvsFieldData(DjangoKeyValueStore(field_data_cache))
# get request-related tracking information from args passthrough, and supplement with task-specific
# information:
# get request-related tracking information from args passthrough, and supplement with task-specific information:
request_info = xblock_instance_args.get('request_info', {}) if xblock_instance_args is not None else {}
task_info = {"student": student.username, "task_id": _get_task_id_from_xblock_args(xblock_instance_args)}
@@ -357,11 +350,12 @@ def _get_module_instance_for_task(course_id, student, block, xblock_instance_arg
'''
return lambda event_type, event: task_track(request_info, task_info, event_type, event, page='x_module_task')
return get_block_for_descriptor_internal(
return get_block_for_descriptor(
user=student,
request=None,
block=block,
student_data=student_data,
course_id=course_id,
field_data_cache=FieldDataCache.cache_for_block_descendents(course_id, student, block),
course_key=course_id,
track_function=make_track_function(),
grade_bucket_type=grade_bucket_type,
# This module isn't being used for front-end rendering

12
lms/djangoapps/instructor_task/tests/test_tasks.py
View File

@@ -292,7 +292,7 @@ class TestOverrideScoreInstructorTask(TestInstructorTasks):
mock_instance = MagicMock()
del mock_instance.set_score
with patch(
'lms.djangoapps.instructor_task.tasks_helper.module_state.get_block_for_descriptor_internal'
'lms.djangoapps.instructor_task.tasks_helper.module_state.get_block_for_descriptor'
) as mock_get_block:
mock_get_block.return_value = mock_instance
with pytest.raises(UpdateProblemModuleStateError):
@@ -313,7 +313,7 @@ class TestOverrideScoreInstructorTask(TestInstructorTasks):
num_students = 1
self._create_students_with_state(num_students, input_state)
task_entry = self._create_input_entry(score=0)
with patch('lms.djangoapps.instructor_task.tasks_helper.module_state.get_block_for_descriptor_internal',
with patch('lms.djangoapps.instructor_task.tasks_helper.module_state.get_block_for_descriptor',
return_value=None):
self._run_task_with_mock_celery(override_problem_score, task_entry.id, task_entry.task_id)
@@ -338,7 +338,7 @@ class TestOverrideScoreInstructorTask(TestInstructorTasks):
self._create_students_with_state(num_students)
task_entry = self._create_input_entry(score=0)
with patch(
'lms.djangoapps.instructor_task.tasks_helper.module_state.get_block_for_descriptor_internal'
'lms.djangoapps.instructor_task.tasks_helper.module_state.get_block_for_descriptor'
) as mock_get_block:
mock_get_block.return_value = mock_instance
mock_instance.max_score = MagicMock(return_value=99999.0)
@@ -425,7 +425,7 @@ class TestRescoreInstructorTask(TestInstructorTasks):
mock_instance = MagicMock()
del mock_instance.rescore_problem
del mock_instance.rescore
with patch('lms.djangoapps.instructor_task.tasks_helper.module_state.get_block_for_descriptor_internal') as mock_get_block: # lint-amnesty, pylint: disable=line-too-long
with patch('lms.djangoapps.instructor_task.tasks_helper.module_state.get_block_for_descriptor') as mock_get_block: # lint-amnesty, pylint: disable=line-too-long
mock_get_block.return_value = mock_instance
with pytest.raises(UpdateProblemModuleStateError):
self._run_task_with_mock_celery(rescore_problem, task_entry.id, task_entry.task_id)
@@ -448,7 +448,7 @@ class TestRescoreInstructorTask(TestInstructorTasks):
num_students = 1
self._create_students_with_state(num_students, input_state)
task_entry = self._create_input_entry()
with patch('lms.djangoapps.instructor_task.tasks_helper.module_state.get_block_for_descriptor_internal', return_value=None): # lint-amnesty, pylint: disable=line-too-long
with patch('lms.djangoapps.instructor_task.tasks_helper.module_state.get_block_for_descriptor', return_value=None): # lint-amnesty, pylint: disable=line-too-long
self._run_task_with_mock_celery(rescore_problem, task_entry.id, task_entry.task_id)
self.assert_task_output(
@@ -474,7 +474,7 @@ class TestRescoreInstructorTask(TestInstructorTasks):
self._create_students_with_state(num_students)
task_entry = self._create_input_entry()
with patch(
'lms.djangoapps.instructor_task.tasks_helper.module_state.get_block_for_descriptor_internal'
'lms.djangoapps.instructor_task.tasks_helper.module_state.get_block_for_descriptor'
) as mock_get_block:
mock_get_block.return_value = mock_instance
self._run_task_with_mock_celery(rescore_problem, task_entry.id, task_entry.task_id)

12
lms/djangoapps/lti_provider/tests/test_views.py
View File

@@ -112,6 +112,18 @@ class LtiLaunchTest(LtiTestMixin, TestCase):
self.consumer
)
@patch('lms.djangoapps.courseware.views.views.render_xblock')
@patch('lms.djangoapps.lti_provider.views.authenticate_lti_user')
def test_render_xblock_params(self, _authenticate, render):
"""
Verifies that the LTI renders an XBlock without:
1. Checking the enrollment.
2. Displaying staff debug info.
"""
request = build_launch_request()
views.lti_launch(request, str(COURSE_KEY), str(USAGE_KEY))
render.assert_called_with(request, str(USAGE_KEY), check_if_enrolled=False, disable_staff_debug_info=True)
@patch('lms.djangoapps.lti_provider.views.render_courseware')
@patch('lms.djangoapps.lti_provider.views.store_outcome_parameters')
@patch('lms.djangoapps.lti_provider.views.authenticate_lti_user')

2
lms/djangoapps/lti_provider/views.py
View File

@@ -145,7 +145,7 @@ def render_courseware(request, usage_key):
"""
# return an HttpResponse object that contains the template and necessary context to render the courseware.
from lms.djangoapps.courseware.views.views import render_xblock
return render_xblock(request, str(usage_key), check_if_enrolled=False)
return render_xblock(request, str(usage_key), check_if_enrolled=False, disable_staff_debug_info=True)
def parse_course_and_usage_keys(course_id, usage_id):

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

@@ -32,7 +32,6 @@ from common.djangoapps.track import contexts as track_contexts
from common.djangoapps.track import views as track_views
from common.djangoapps.xblock_django.user_service import DjangoXBlockUserService
from lms.djangoapps.courseware.model_data import DjangoKeyValueStore, FieldDataCache
from lms.djangoapps.courseware import block_render
from lms.djangoapps.grades.api import signals as grades_signals
from openedx.core.djangoapps.xblock.apps import get_xblock_app_config
from openedx.core.djangoapps.xblock.runtime.blockstore_field_data import BlockstoreChildrenData, BlockstoreFieldData
@@ -270,7 +269,6 @@ class XBlockRuntime(RuntimeShim, Runtime):
return RebindUserService(
self.user,
context_key,
block_render.prepare_runtime_for_user,
track_function=make_track_function(),
request_token=request_token(crum.get_current_request()),
)

13
xmodule/services.py
View File

@@ -157,19 +157,13 @@ class RebindUserService(Service):
user (User) - A Django User object
course_id (str) - Course ID
course (Course) - Course Object
prepare_runtime_for_user (function) - The helper function that will be called to create a module system
for a specfic user. This is the parent function from which this service was reactored out.
`lms.djangoapps.courseware.block_render.prepare_runtime_for_user`
kwargs (dict) - all the keyword arguments that need to be passed to the `prepare_runtime_for_user`
function when it is called during rebinding
"""
def __init__(self, user, course_id, prepare_runtime_for_user, **kwargs):
def __init__(self, user, course_id, **kwargs):
super().__init__(**kwargs)
self.user = user
self.course_id = course_id
self._ref = {
"prepare_runtime_for_user": prepare_runtime_for_user
}
self._kwargs = kwargs
def rebind_noauth_module_to_user(self, block, real_user):
@@ -201,10 +195,11 @@ class RebindUserService(Service):
with modulestore().bulk_operations(self.course_id):
course = modulestore().get_course(course_key=self.course_id)
self._ref["prepare_runtime_for_user"](
from lms.djangoapps.courseware.block_render import prepare_runtime_for_user
prepare_runtime_for_user(
user=real_user,
student_data=student_data_real_user, # These have implicit user bindings, rest of args considered not to
block=block,
runtime=block.runtime,
course_id=self.course_id,
course=course,
**self._kwargs

2
xmodule/tests/test_library_content.py
View File

@@ -59,7 +59,7 @@ class LibraryContentTest(MixedSplitTestCase):
Bind a block (part of self.course) so we can access student-specific data.
"""
prepare_block_runtime(block.runtime, course_id=block.location.course_key)
block.runtime._runtime_services.update({'library_tools': self.tools}) # lint-amnesty, pylint: disable=protected-access
block.runtime._services.update({'library_tools': self.tools}) # lint-amnesty, pylint: disable=protected-access
def get_block(descriptor):
"""Mocks module_system get_block function"""

2
xmodule/tests/test_lti20_unit.py
View File

@@ -370,7 +370,7 @@ class LTI20RESTResultServiceTest(unittest.TestCase):
Test that we get a 404 when the supplied user does not exist
"""
self.setup_system_xblock_mocks_for_lti20_request_test()
self.runtime._runtime_services['user'] = StubUserService(user=None) # pylint: disable=protected-access
self.runtime._services['user'] = StubUserService(user=None) # pylint: disable=protected-access
mock_request = self.get_signed_lti20_mock_request(self.GOOD_JSON_PUT)
response = self.xblock.lti_2_0_result_rest_handler(mock_request, "user/abcd")
assert response.status_code == 404

12
xmodule/tests/test_lti_unit.py
View File

@@ -63,16 +63,16 @@ class LTIBlockTest(TestCase):
</imsx_POXEnvelopeRequest>
""")
self.course_id = CourseKey.from_string('org/course/run')
self.system = get_test_system(self.course_id)
self.system.publish = Mock()
self.system._runtime_services['rebind_user'] = Mock() # pylint: disable=protected-access
self.runtime = get_test_system(self.course_id)
self.runtime.publish = Mock()
self.runtime._services['rebind_user'] = Mock() # pylint: disable=protected-access
self.xblock = LTIBlock(
self.system,
self.runtime,
DictFieldData({}),
ScopeIds(None, None, None, BlockUsageLocator(self.course_id, 'lti', 'name'))
)
current_user = self.system.service(self.xblock, 'user').get_current_user()
current_user = self.runtime.service(self.xblock, 'user').get_current_user()
self.user_id = current_user.opt_attrs.get(ATTR_KEY_ANONYMOUS_USER_ID)
self.lti_id = self.xblock.lti_id
@@ -178,7 +178,7 @@ class LTIBlockTest(TestCase):
"""
If we have no real user, we should send back failure response.
"""
self.system._runtime_services['user'] = StubUserService(user=None) # pylint: disable=protected-access
self.runtime._services['user'] = StubUserService(user=None) # pylint: disable=protected-access
self.xblock.verify_oauth_body_sign = Mock()
self.xblock.has_score = True
request = Request(self.environ)

12
xmodule/tests/test_sequence.py
View File

@@ -94,8 +94,8 @@ class SequenceBlockTestCase(XModuleXmlImportTest):
self._set_up_module_system(block)
block.runtime._runtime_services['bookmarks'] = Mock() # pylint: disable=protected-access
block.runtime._runtime_services['user'] = StubUserService(user=Mock()) # pylint: disable=protected-access
block.runtime._services['bookmarks'] = Mock() # pylint: disable=protected-access
block.runtime._services['user'] = StubUserService(user=Mock()) # pylint: disable=protected-access
block.parent = parent.location
return block
@@ -368,7 +368,7 @@ class SequenceBlockTestCase(XModuleXmlImportTest):
def test_xblock_handler_get_completion_success(self):
"""Test that the completion data is returned successfully on targeted vertical through ajax call"""
self.sequence_3_1.runtime._runtime_services['completion'] = Mock( # pylint: disable=protected-access
self.sequence_3_1.runtime._services['completion'] = Mock( # pylint: disable=protected-access
return_value=Mock(vertical_is_complete=Mock(return_value=True))
)
for child in self.sequence_3_1.get_children():
@@ -380,7 +380,7 @@ class SequenceBlockTestCase(XModuleXmlImportTest):
)
completion_return = self.sequence_3_1.handle('get_completion', request)
assert completion_return.json == {'complete': True}
self.sequence_3_1.runtime._runtime_services['completion'] = None # pylint: disable=protected-access
self.sequence_3_1.runtime._services['completion'] = None # pylint: disable=protected-access
def test_xblock_handler_get_completion_bad_key(self):
"""Test that the completion data is returned as False when usage key is None through ajax call"""
@@ -394,14 +394,14 @@ class SequenceBlockTestCase(XModuleXmlImportTest):
def test_handle_ajax_get_completion_success(self):
"""Test that the old-style ajax handler for completion still works"""
self.sequence_3_1.runtime._runtime_services['completion'] = Mock( # pylint: disable=protected-access
self.sequence_3_1.runtime._services['completion'] = Mock( # pylint: disable=protected-access
return_value=Mock(vertical_is_complete=Mock(return_value=True))
)
for child in self.sequence_3_1.get_children():
usage_key = str(child.location)
completion_return = self.sequence_3_1.handle_ajax('get_completion', {'usage_key': usage_key})
assert json.loads(completion_return) == {'complete': True}
self.sequence_3_1.runtime._runtime_services['completion'] = None # pylint: disable=protected-access
self.sequence_3_1.runtime._services['completion'] = None # pylint: disable=protected-access
def test_xblock_handler_goto_position_success(self):
"""Test that we can set position through ajax call"""

2
xmodule/tests/test_split_test_block.py
View File

@@ -106,7 +106,7 @@ class SplitTestBlockTest(XModuleXmlImportTest, PartitionTestCase):
self.course,
course_id=self.course.id,
)
self.course.runtime._runtime_services['partitions'] = partitions_service # pylint: disable=protected-access
self.course.runtime._services['partitions'] = partitions_service # pylint: disable=protected-access
# Mock user_service user
user_service = Mock()

8
xmodule/tests/test_vertical.py
View File

@@ -212,17 +212,17 @@ class VerticalBlockTestCase(BaseVerticalBlockTest):
"""
Test the rendering of the student and public view.
"""
self.course.runtime._runtime_services['bookmarks'] = Mock()
self.course.runtime._services['bookmarks'] = Mock()
now = datetime.now(pytz.UTC)
self.vertical.due = now + timedelta(days=days)
if view == STUDENT_VIEW:
self.course.runtime._runtime_services['user'] = StubUserService(user=Mock(username=self.username))
self.course.runtime._runtime_services['completion'] = StubCompletionService(
self.course.runtime._services['user'] = StubUserService(user=Mock(username=self.username))
self.course.runtime._services['completion'] = StubCompletionService(
enabled=True,
completion_value=completion_value
)
elif view == PUBLIC_VIEW:
self.course.runtime._runtime_services['user'] = StubUserService(user=AnonymousUser())
self.course.runtime._services['user'] = StubUserService(user=AnonymousUser())
html = self.course.runtime.render(
self.vertical, view, self.default_context if context is None else context

36
xmodule/x_module.py
View File

@@ -1077,7 +1077,7 @@ class ModuleSystemShim:
'runtime.anonymous_student_id is deprecated. Please use the user service instead.',
DeprecationWarning, stacklevel=3,
)
user_service = self._runtime_services.get('user') or self._services.get('user')
user_service = self._services.get('user')
if user_service:
return user_service.get_current_user().opt_attrs.get(ATTR_KEY_ANONYMOUS_USER_ID)
return None
@@ -1107,7 +1107,7 @@ class ModuleSystemShim:
'runtime.user_id is deprecated. Use block.scope_ids.user_id or the user service instead.',
DeprecationWarning, stacklevel=2,
)
user_service = self._runtime_services.get('user') or self._services.get('user')
user_service = self._services.get('user')
if user_service:
return user_service.get_current_user().opt_attrs.get(ATTR_KEY_USER_ID)
return None
@@ -1123,7 +1123,7 @@ class ModuleSystemShim:
'runtime.user_is_staff is deprecated. Please use the user service instead.',
DeprecationWarning, stacklevel=2,
)
user_service = self._runtime_services.get('user') or self._services.get('user')
user_service = self._services.get('user')
if user_service:
return user_service.get_current_user().opt_attrs.get(ATTR_KEY_USER_IS_STAFF)
return None
@@ -1139,7 +1139,7 @@ class ModuleSystemShim:
'runtime.user_location is deprecated. Please use the user service instead.',
DeprecationWarning, stacklevel=2,
)
user_service = self._runtime_services.get('user') or self._services.get('user')
user_service = self._services.get('user')
if user_service:
return user_service.get_current_user().opt_attrs.get(ATTR_KEY_REQUEST_COUNTRY_CODE)
return None
@@ -1159,7 +1159,7 @@ class ModuleSystemShim:
'runtime.get_real_user is deprecated. Please use the user service instead.',
DeprecationWarning, stacklevel=2,
)
user_service = self._runtime_services.get('user') or self._services.get('user')
user_service = self._services.get('user')
if user_service:
return user_service.get_user_by_anonymous_id
return None
@@ -1177,7 +1177,7 @@ class ModuleSystemShim:
'runtime.get_user_role is deprecated. Please use the user service instead.',
DeprecationWarning, stacklevel=2,
)
user_service = self._runtime_services.get('user') or self._services.get('user')
user_service = self._services.get('user')
if user_service:
return partial(user_service.get_current_user().opt_attrs.get, ATTR_KEY_USER_ROLE)
@@ -1192,7 +1192,7 @@ class ModuleSystemShim:
'runtime.user_is_beta_tester is deprecated. Please use the user service instead.',
DeprecationWarning, stacklevel=2,
)
user_service = self._runtime_services.get('user') or self._services.get('user')
user_service = self._services.get('user')
if user_service:
return user_service.get_current_user().opt_attrs.get(ATTR_KEY_USER_IS_BETA_TESTER)
@@ -1207,7 +1207,7 @@ class ModuleSystemShim:
'runtime.user_is_admin is deprecated. Please use the user service instead.',
DeprecationWarning, stacklevel=2,
)
user_service = self._runtime_services.get('user') or self._services.get('user')
user_service = self._services.get('user')
if user_service:
return user_service.get_current_user().opt_attrs.get(ATTR_KEY_USER_IS_GLOBAL_STAFF)
@@ -1225,7 +1225,7 @@ class ModuleSystemShim:
)
if hasattr(self, '_deprecated_render_template'):
return self._deprecated_render_template
render_service = self._runtime_services.get('mako') or self._services.get('mako')
render_service = self._services.get('mako')
if render_service:
return render_service.render_template
return None
@@ -1251,7 +1251,7 @@ class ModuleSystemShim:
'runtime.can_execute_unsafe_code is deprecated. Please use the sandbox service instead.',
DeprecationWarning, stacklevel=2,
)
sandbox_service = self._runtime_services.get('sandbox') or self._services.get('sandbox')
sandbox_service = self._services.get('sandbox')
if sandbox_service:
return sandbox_service.can_execute_unsafe_code
# Default to saying "no unsafe code".
@@ -1271,7 +1271,7 @@ class ModuleSystemShim:
'runtime.get_python_lib_zip is deprecated. Please use the sandbox service instead.',
DeprecationWarning, stacklevel=2,
)
sandbox_service = self._runtime_services.get('sandbox') or self._services.get('sandbox')
sandbox_service = self._services.get('sandbox')
if sandbox_service:
return sandbox_service.get_python_lib_zip
# Default to saying "no lib data"
@@ -1290,7 +1290,7 @@ class ModuleSystemShim:
'runtime.cache is deprecated. Please use the cache service instead.',
DeprecationWarning, stacklevel=2,
)
return self._runtime_services.get('cache') or self._services.get('cache') or DoNothingCache()
return self._services.get('cache') or DoNothingCache()
@property
def filestore(self):
@@ -1341,7 +1341,7 @@ class ModuleSystemShim:
"rebind_noauth_module_to_user is deprecated. Please use the 'rebind_user' service instead.",
DeprecationWarning, stacklevel=2,
)
rebind_user_service = self._runtime_services.get('rebind_user') or self._services.get('rebind_user')
rebind_user_service = self._services.get('rebind_user')
if rebind_user_service:
return partial(rebind_user_service.rebind_noauth_module_to_user)
@@ -1421,7 +1421,6 @@ class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemSh
self.get_policy = lambda u: {}
self.disabled_xblock_types = disabled_xblock_types
self._runtime_services = {}
def get(self, attr):
""" provide uniform access to attributes (like etree)."""
@@ -1519,7 +1518,7 @@ class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemSh
Publish events through the `EventPublishingService`.
This ensures that the correct track method is used for Instructor tasks.
"""
if publish_service := self._runtime_services.get('publish') or self._services.get('publish'):
if publish_service := self._services.get('publish'):
publish_service.publish(block, event_type, event)
def service(self, block, service_name):
@@ -1536,11 +1535,8 @@ class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemSh
Returns:
An object implementing the requested service, or None.
"""
declaration = block.service_declaration(service_name)
service = self._runtime_services.get(service_name)
if declaration is None or service is None:
# getting the service from parent module. making sure of block service declarations.
service = super().service(block=block, service_name=service_name)
# Getting the service from parent module. making sure of block service declarations.
service = super().service(block=block, service_name=service_name)
# Passing the block to service if it is callable e.g. XBlockI18nService. It is the responsibility of calling
# service to handle the passing argument.
if callable(service):