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

refactor: deprecate course_id from ModuleSystem

Browse Source 
This attribute is already deprecated for XBlocks in favour of directly
retrieving it like `block.scope_ids.usage_id.context_key`.

This commit also removes some redundant logging code which was omitted in the
Datadog removal in #19420.
This commit is contained in:
Agrendalath
2022-07-07 22:07:04 +02:00
parent c2ac3d83c3
commit dd97c74fde
11 changed files with 44 additions and 64 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -211,7 +211,6 @@ def _preview_module_system(request, descriptor, field_data):
track_function=lambda event_type, event: None,
get_module=partial(_load_preview_module, request),
mixins=settings.XBLOCK_MIXINS,
course_id=course_id,
# Set up functions to modify the fragment produced by student_view
wrappers=wrappers,

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

@@ -296,7 +296,7 @@ class CmsModuleSystemShimTest(ModuleStoreTestCase):
def test_replace_urls(self):
html = '<a href="/static/id">'
assert self.runtime.replace_urls(html) == \
static_replace.replace_static_urls(html, course_id=self.runtime.course_id)
static_replace.replace_static_urls(html, course_id=self.course.id)
def test_anonymous_user_id_preview(self):
assert self.runtime.anonymous_student_id == 'student'

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

@@ -683,7 +683,6 @@ def get_module_system_for_user(
get_module=inner_get_module,
user=user,
publish=publish,
course_id=course_id,
# TODO: When we merge the descriptor and module systems, we can stop reaching into the mixologist (cpennington)
mixins=descriptor.runtime.mixologist._mixins, # pylint: disable=protected-access
wrappers=block_wrappers,

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

@@ -2706,15 +2706,22 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase):
def test_replace_urls(self):
html = '<a href="/static/id">'
assert self.runtime.replace_urls(html) == \
static_replace.replace_static_urls(html, course_id=self.runtime.course_id)
static_replace.replace_static_urls(html, course_id=self.course.id)
def test_replace_course_urls(self):
html = '<a href="/course/id">'
assert self.runtime.replace_course_urls(html) == \
static_replace.replace_course_urls(html, course_key=self.runtime.course_id)
static_replace.replace_course_urls(html, course_key=self.course.id)
def test_replace_jump_to_id_urls(self):
html = '<a href="/jump_to_id/id">'
jump_to_id_base_url = reverse('jump_to_id', kwargs={'course_id': str(self.runtime.course_id), 'module_id': ''})
jump_to_id_base_url = reverse('jump_to_id', kwargs={'course_id': str(self.course.id), 'module_id': ''})
assert self.runtime.replace_jump_to_id_urls(html) == \
static_replace.replace_jump_to_id_urls(html, self.runtime.course_id, jump_to_id_base_url)
static_replace.replace_jump_to_id_urls(html, self.course.id, jump_to_id_base_url)
@XBlock.register_temp_plugin(PureXBlockWithChildren, identifier='xblock')
def test_course_id(self):
descriptor = ItemFactory(category="pure", parent=self.course)
block = render.get_module(self.user, Mock(), descriptor.location, None)
assert str(block.runtime.course_id) == self.COURSE_ID

4
lms/djangoapps/instructor/tasks.py
View File

@@ -69,10 +69,10 @@ def update_exam_completion_task(user_identifier: str, content_id: str, completio
# Now evil modulestore magic to inflate our descriptor with user state and
# permissions checks.
field_data_cache = FieldDataCache.cache_for_descriptor_descendents(
root_descriptor.course_id, user, root_descriptor, read_only=True,
root_descriptor.scope_ids.usage_id.context_key, user, root_descriptor, read_only=True,
)
root_module = get_module_for_descriptor(
user, request, root_descriptor, field_data_cache, root_descriptor.course_id,
user, request, root_descriptor, field_data_cache, root_descriptor.scope_ids.usage_id.context_key,
)
if not root_module:
err_msg = err_msg_prefix + 'Module unable to be created from descriptor!'

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

@@ -51,7 +51,7 @@ def handler_url(block, handler_name, suffix='', query='', thirdparty=False):
view_name = 'xblock_handler_noauth'
url = reverse(view_name, kwargs={
'course_id': str(block.location.course_key),
'course_id': str(block.scope_ids.usage_id.context_key),
'usage_id': quote_slashes(str(block.scope_ids.usage_id)),
'handler': handler_name,
'suffix': suffix,

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

@@ -26,6 +26,14 @@ from xmodule.modulestore.tests.factories import CourseFactory # lint-amnesty, p
class BlockMock(Mock):
"""Mock class that we fill with our "handler" methods."""
scope_ids = ScopeIds(
None,
None,
None,
BlockUsageLocator(
CourseLocator(org="mockx", course="100", run="2015"), block_type='mock_type', block_id="mock_id"
),
)
def handler(self, _context):
"""
@@ -45,20 +53,13 @@ class BlockMock(Mock):
"""
pass # lint-amnesty, pylint: disable=unnecessary-pass
@property
def location(self):
"""Create a functional BlockUsageLocator for testing URL generation."""
course_key = CourseLocator(org="mockx", course="100", run="2015")
return BlockUsageLocator(course_key, block_type='mock_type', block_id="mock_id")
class TestHandlerUrl(TestCase):
"""Test the LMS handler_url"""
def setUp(self):
super().setUp()
self.block = BlockMock(name='block', scope_ids=ScopeIds(None, None, None, 'dummy'))
self.course_key = CourseLocator("org", "course", "run")
self.block = BlockMock(name='block')
self.runtime = LmsModuleSystem(
track_function=Mock(),
get_module=Mock(),

2
openedx/features/course_duration_limits/access.py
View File

@@ -249,7 +249,7 @@ def course_expiration_wrapper(user, block, view, frag, context): # pylint: disa
return frag
course_expiration_fragment = generate_course_expired_fragment_from_key(
user, block.course_id
user, block.scope_ids.usage_id.context_key
)
if not course_expiration_fragment:
return frag

7
xmodule/discussion_block.py
View File

@@ -73,13 +73,6 @@ class DiscussionXBlock(XBlock, StudioEditableXBlockMixin, XmlParserMixin): # li
@property
def course_key(self):
"""
:return: int course id
NB: The goal is to move this XBlock out of edx-platform, and so we use
scope_ids.usage_id instead of runtime.course_id so that the code will
continue to work with workbench-based testing.
"""
return getattr(self.scope_ids.usage_id, 'course_key', None)
@property

12
xmodule/seq_module.py
View File

@@ -213,7 +213,7 @@ class ProctoringFields:
"""
Return course by course id.
"""
return self.runtime.modulestore.get_course(self.course_id) # pylint: disable=no-member
return self.runtime.modulestore.get_course(self.scope_ids.usage_id.context_key) # pylint: disable=no-member
@property
def is_timed_exam(self):
@@ -451,7 +451,7 @@ class SequenceBlock(
content_type_gating_service = self.runtime.service(self, 'content_type_gating')
if content_type_gating_service:
self.gated_sequence_paywall = content_type_gating_service.check_children_for_content_type_gating_paywall(
self, self.course_id
self, self.scope_ids.usage_id.context_key
)
def student_view(self, context):
@@ -614,7 +614,7 @@ class SequenceBlock(
if SHOW_PROGRESS_BAR.is_enabled() and getattr(settings, 'COMPLETION_AGGREGATOR_URL', ''):
parent_block_id = self.get_parent().scope_ids.usage_id.block_id
params['chapter_completion_aggregator_url'] = '/'.join(
[settings.COMPLETION_AGGREGATOR_URL, str(self.course_id), parent_block_id]) + '/'
[settings.COMPLETION_AGGREGATOR_URL, str(self.scope_ids.usage_id.context_key), parent_block_id]) + '/'
fragment.add_content(self.runtime.service(self, 'mako').render_template("seq_module.html", params))
self._capture_full_seq_item_metrics(display_items)
@@ -655,7 +655,7 @@ class SequenceBlock(
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, user_id
self.scope_ids.usage_id.context_key, self.location, user_id
)
return fulfilled
@@ -671,7 +671,7 @@ class SequenceBlock(
gating_service = self.runtime.service(self, 'gating')
if gating_service:
milestone = gating_service.required_prereq(
self.course_id, self.location, 'requires'
self.scope_ids.usage_id.context_key, self.location, 'requires'
)
return milestone
@@ -810,7 +810,7 @@ class SequenceBlock(
contains_content_type_gated_content = False
if content_type_gating_service:
contains_content_type_gated_content = content_type_gating_service.check_children_for_content_type_gating_paywall( # pylint:disable=line-too-long
item, self.course_id
item, self.scope_ids.usage_id.context_key
) is not None
iteminfo = {
'content': content,

45
xmodule/x_module.py
View File

@@ -1073,25 +1073,11 @@ class MetricsMixin:
def render(self, block, view_name, context=None): # lint-amnesty, pylint: disable=missing-function-docstring
start_time = time.time()
status = "success"
try:
return super().render(block, view_name, context=context)
except:
status = "failure"
raise
finally:
end_time = time.time()
duration = end_time - start_time
course_id = getattr(self, 'course_id', '')
tags = [ # lint-amnesty, pylint: disable=unused-variable
f'view_name:{view_name}',
'action:render',
f'action_status:{status}',
f'course_id:{course_id}',
f'block_type:{block.scope_ids.block_type}',
f'block_family:{block.entry_point}',
]
log.debug(
"%.3fs - render %s.%s (%s)",
duration,
@@ -1102,25 +1088,11 @@ class MetricsMixin:
def handle(self, block, handler_name, request, suffix=''): # lint-amnesty, pylint: disable=missing-function-docstring
start_time = time.time()
status = "success"
try:
return super().handle(block, handler_name, request, suffix=suffix)
except:
status = "failure"
raise
finally:
end_time = time.time()
duration = end_time - start_time
course_id = getattr(self, 'course_id', '')
tags = [ # lint-amnesty, pylint: disable=unused-variable
f'handler_name:{handler_name}',
'action:handle',
f'action_status:{status}',
f'course_id:{course_id}',
f'block_type:{block.scope_ids.block_type}',
f'block_family:{block.entry_point}',
]
log.debug(
"%.3fs - handle %s.%s (%s)",
duration,
@@ -1718,6 +1690,19 @@ class ModuleSystemShim:
)
return settings.STATIC_URL
@property
def course_id(self):
"""
Old API to get the course ID.
Deprecated in favor of `runtime.scope_ids.usage_id.context_key`.
"""
warnings.warn(
"`runtime.course_id` is deprecated. Use `context_key` instead: `runtime.scope_ids.usage_id.context_key`.",
DeprecationWarning, stacklevel=3,
)
return self.descriptor_runtime.course_id.for_branch(None)
class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemShim, Runtime):
"""
@@ -1738,7 +1723,6 @@ class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemShim,
get_module,
descriptor_runtime,
publish=None,
course_id=None,
**kwargs,
):
"""
@@ -1755,8 +1739,6 @@ class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemShim,
descriptor_runtime - A `DescriptorSystem` to use for loading xblocks by id
course_id - the course_id containing this module
publish(event) - A function that allows XModules to publish events (such as grade changes)
"""
@@ -1766,7 +1748,6 @@ class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemShim,
self.track_function = track_function
self.get_module = get_module
self.course_id = course_id
if publish:
self.publish = publish