diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index bbffe1ff81..4100c2dd47 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -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, diff --git a/cms/djangoapps/contentstore/views/tests/test_preview.py b/cms/djangoapps/contentstore/views/tests/test_preview.py index 66e934911f..a46f35cef5 100644 --- a/cms/djangoapps/contentstore/views/tests/test_preview.py +++ b/cms/djangoapps/contentstore/views/tests/test_preview.py @@ -296,7 +296,7 @@ class CmsModuleSystemShimTest(ModuleStoreTestCase): def test_replace_urls(self): html = '' 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' diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index bd64896cb7..7ff9ad7bfc 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -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, diff --git a/lms/djangoapps/courseware/tests/test_module_render.py b/lms/djangoapps/courseware/tests/test_module_render.py index 15b3384479..7be2e339b7 100644 --- a/lms/djangoapps/courseware/tests/test_module_render.py +++ b/lms/djangoapps/courseware/tests/test_module_render.py @@ -2706,15 +2706,22 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase): def test_replace_urls(self): html = '' 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 = '' 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 = '' - 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 diff --git a/lms/djangoapps/instructor/tasks.py b/lms/djangoapps/instructor/tasks.py index eedfe07fee..b28f5387f8 100644 --- a/lms/djangoapps/instructor/tasks.py +++ b/lms/djangoapps/instructor/tasks.py @@ -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!' diff --git a/lms/djangoapps/lms_xblock/runtime.py b/lms/djangoapps/lms_xblock/runtime.py index 859d01c156..4b5fec0308 100644 --- a/lms/djangoapps/lms_xblock/runtime.py +++ b/lms/djangoapps/lms_xblock/runtime.py @@ -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, diff --git a/lms/djangoapps/lms_xblock/test/test_runtime.py b/lms/djangoapps/lms_xblock/test/test_runtime.py index f90722a746..58b90a8c10 100644 --- a/lms/djangoapps/lms_xblock/test/test_runtime.py +++ b/lms/djangoapps/lms_xblock/test/test_runtime.py @@ -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(), diff --git a/openedx/features/course_duration_limits/access.py b/openedx/features/course_duration_limits/access.py index cee69dd4f3..ff817a3150 100644 --- a/openedx/features/course_duration_limits/access.py +++ b/openedx/features/course_duration_limits/access.py @@ -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 diff --git a/xmodule/discussion_block.py b/xmodule/discussion_block.py index c7eb09d2a2..3e0d4ed254 100644 --- a/xmodule/discussion_block.py +++ b/xmodule/discussion_block.py @@ -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 diff --git a/xmodule/seq_module.py b/xmodule/seq_module.py index ede212722e..a0b11ee793 100644 --- a/xmodule/seq_module.py +++ b/xmodule/seq_module.py @@ -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, diff --git a/xmodule/x_module.py b/xmodule/x_module.py index c617d78613..7859bc8370 100644 --- a/xmodule/x_module.py +++ b/xmodule/x_module.py @@ -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