From 3a0ab4b6199ddd92adb30f2234af13a78985b691 Mon Sep 17 00:00:00 2001 From: Serhii Nanai Date: Mon, 4 Aug 2025 19:54:02 +0300 Subject: [PATCH 01/20] feat: store and reuse transformed course blocks --- lms/djangoapps/course_api/blocks/api.py | 5 ++++- lms/djangoapps/grades/course_data.py | 6 ++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/lms/djangoapps/course_api/blocks/api.py b/lms/djangoapps/course_api/blocks/api.py index a79e9759e191..af1ef016d868 100644 --- a/lms/djangoapps/course_api/blocks/api.py +++ b/lms/djangoapps/course_api/blocks/api.py @@ -1,7 +1,7 @@ """ API function for retrieving course blocks data """ - +from crum import get_current_request import lms.djangoapps.course_blocks.api as course_blocks_api from lms.djangoapps.course_blocks.transformers.access_denied_filter import AccessDeniedMessageFilterTransformer @@ -138,6 +138,9 @@ def get_blocks( for block_key in block_keys_to_remove: blocks.remove_block(block_key, keep_descendants=True) + # store transformed blocks in the current request to be reused where possible for optimization + get_current_request()._reusable_transformed_blocks = blocks + # serialize serializer_context = { 'request': request, diff --git a/lms/djangoapps/grades/course_data.py b/lms/djangoapps/grades/course_data.py index 5464c4f88105..41cfee9cb529 100644 --- a/lms/djangoapps/grades/course_data.py +++ b/lms/djangoapps/grades/course_data.py @@ -1,7 +1,7 @@ """ Code used to get and cache the requested course-data """ - +from crum import get_current_request from lms.djangoapps.course_blocks.api import get_course_blocks from openedx.core.djangoapps.content.block_structure.api import get_block_structure_manager @@ -56,7 +56,9 @@ def location(self): # lint-amnesty, pylint: disable=missing-function-docstring @property def structure(self): # lint-amnesty, pylint: disable=missing-function-docstring if self._structure is None: - self._structure = get_course_blocks( + # reuse transformed blocks from request if available + _reusable_transformed_blocks = getattr(get_current_request(), "_reusable_transformed_blocks", None) + self._structure = _reusable_transformed_blocks or get_course_blocks( self.user, self.location, collected_block_structure=self._collected_block_structure, From 24e7bfe6ec4023dae1b4ec6537518b4dc1a44bf1 Mon Sep 17 00:00:00 2001 From: Serhii Nanai Date: Mon, 4 Aug 2025 20:16:41 +0300 Subject: [PATCH 02/20] test: add test_response_keys to TestBlocksInfoInCourseView --- .../tests/test_course_info_views.py | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/lms/djangoapps/mobile_api/tests/test_course_info_views.py b/lms/djangoapps/mobile_api/tests/test_course_info_views.py index efb3f7d9fdbb..0d16cd99e534 100644 --- a/lms/djangoapps/mobile_api/tests/test_course_info_views.py +++ b/lms/djangoapps/mobile_api/tests/test_course_info_views.py @@ -432,6 +432,68 @@ def test_extend_sequential_info_with_assignment_progress_for_other_types(self, b for block_info in response.data['blocks'].values(): self.assertNotEqual('assignment_progress', block_info) + def test_response_keys(self): + response = self.verify_response(url=self.url) + data = response.data + + expected_top_level_keys = { + 'blocks', + 'certificate', + 'course_about', + 'course_access_details', + 'course_handouts', + 'course_modes', + 'course_progress', + 'course_sharing_utm_parameters', + 'course_updates', + 'deprecate_youtube', + 'discussion_url', + 'end', + 'enrollment_details', + 'id', + 'is_self_paced', + 'media', + 'name', + 'number', + 'org', + 'org_logo', + 'root', + 'start', + 'start_display', + 'start_type' + } + expected_course_access_keys = { + "has_unmet_prerequisites", + "is_too_early", + "is_staff", + "audit_access_expires", + "courseware_access" + } + expected_courseware_access_keys = { + "has_access", + "error_code", + "developer_message", + "user_message", + "additional_context_user_message", + "user_fragment" + } + expected_enrollment_details_keys = {"created", "mode", "is_active", "upgrade_deadline"} + expected_media_keys = {"image"} + expected_image_keys = {"raw", "small", "large"} + expected_course_sharing_keys = {"facebook", "twitter"} + expected_course_modes_keys = {"slug", "sku", "android_sku", "ios_sku", "min_price"} + expected_course_progress_keys = {"total_assignments_count", "assignments_completed"} + + self.assertSetEqual(set(data), expected_top_level_keys) + self.assertSetEqual(set(data["course_access_details"]), expected_course_access_keys) + self.assertSetEqual(set(data["course_access_details"]["courseware_access"]), expected_courseware_access_keys) + self.assertSetEqual(set(data["enrollment_details"]), expected_enrollment_details_keys) + self.assertSetEqual(set(data["media"]), expected_media_keys) + self.assertSetEqual(set(data["media"]["image"]), expected_image_keys) + self.assertSetEqual(set(data["course_sharing_utm_parameters"]), expected_course_sharing_keys) + self.assertSetEqual(set(data["course_modes"][0]), expected_course_modes_keys) + self.assertSetEqual(set(data["course_progress"]), expected_course_progress_keys) + class TestCourseEnrollmentDetailsView(MobileAPITestCase, MilestonesTestCaseMixin): # lint-amnesty, pylint: disable=test-inherits-tests """ From 92d50b9be814f43067d1df1b5e00550dcb5c1dc3 Mon Sep 17 00:00:00 2001 From: Serhii Nanai Date: Tue, 5 Aug 2025 10:51:18 +0300 Subject: [PATCH 03/20] fix: make reusable_transformed_blocks public --- lms/djangoapps/course_api/blocks/api.py | 2 +- lms/djangoapps/grades/course_data.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lms/djangoapps/course_api/blocks/api.py b/lms/djangoapps/course_api/blocks/api.py index af1ef016d868..3e565de3d533 100644 --- a/lms/djangoapps/course_api/blocks/api.py +++ b/lms/djangoapps/course_api/blocks/api.py @@ -139,7 +139,7 @@ def get_blocks( blocks.remove_block(block_key, keep_descendants=True) # store transformed blocks in the current request to be reused where possible for optimization - get_current_request()._reusable_transformed_blocks = blocks + get_current_request().reusable_transformed_blocks = blocks # serialize serializer_context = { diff --git a/lms/djangoapps/grades/course_data.py b/lms/djangoapps/grades/course_data.py index 41cfee9cb529..668f814f613a 100644 --- a/lms/djangoapps/grades/course_data.py +++ b/lms/djangoapps/grades/course_data.py @@ -57,8 +57,8 @@ def location(self): # lint-amnesty, pylint: disable=missing-function-docstring def structure(self): # lint-amnesty, pylint: disable=missing-function-docstring if self._structure is None: # reuse transformed blocks from request if available - _reusable_transformed_blocks = getattr(get_current_request(), "_reusable_transformed_blocks", None) - self._structure = _reusable_transformed_blocks or get_course_blocks( + reusable_transformed_blocks = getattr(get_current_request(), "reusable_transformed_blocks", None) + self._structure = reusable_transformed_blocks or get_course_blocks( self.user, self.location, collected_block_structure=self._collected_block_structure, From 86c42f3a1afd0d6b945a10672f221fc9059d5825 Mon Sep 17 00:00:00 2001 From: Serhii Nanai Date: Tue, 5 Aug 2025 11:22:00 +0300 Subject: [PATCH 04/20] fix: check request is not None before setting attribute --- lms/djangoapps/course_api/blocks/api.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lms/djangoapps/course_api/blocks/api.py b/lms/djangoapps/course_api/blocks/api.py index 3e565de3d533..84e1e8a8a083 100644 --- a/lms/djangoapps/course_api/blocks/api.py +++ b/lms/djangoapps/course_api/blocks/api.py @@ -139,7 +139,8 @@ def get_blocks( blocks.remove_block(block_key, keep_descendants=True) # store transformed blocks in the current request to be reused where possible for optimization - get_current_request().reusable_transformed_blocks = blocks + if current_request := get_current_request(): + setattr(current_request, "reusable_transformed_blocks", blocks) # serialize serializer_context = { From 1845775601a8d1e8cf37ba6fc0113f9ea8d7b9ec Mon Sep 17 00:00:00 2001 From: Serhii Nanai Date: Tue, 5 Aug 2025 11:36:50 +0300 Subject: [PATCH 05/20] fix: disable pylint warning --- lms/djangoapps/course_api/blocks/api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/djangoapps/course_api/blocks/api.py b/lms/djangoapps/course_api/blocks/api.py index 84e1e8a8a083..7bc8c1fc53bb 100644 --- a/lms/djangoapps/course_api/blocks/api.py +++ b/lms/djangoapps/course_api/blocks/api.py @@ -140,7 +140,7 @@ def get_blocks( # store transformed blocks in the current request to be reused where possible for optimization if current_request := get_current_request(): - setattr(current_request, "reusable_transformed_blocks", blocks) + setattr(current_request, "reusable_transformed_blocks", blocks) # pylint: disable=literal-used-as-attribute # serialize serializer_context = { From 2fc81d45ba33a7d06b8608d27ec42f94f824ec01 Mon Sep 17 00:00:00 2001 From: Serhii Nanai Date: Wed, 13 Aug 2025 14:51:20 +0300 Subject: [PATCH 06/20] fix: use RequestCache to store course blocks --- lms/djangoapps/course_api/blocks/api.py | 8 ++++---- lms/djangoapps/grades/course_data.py | 12 +++++++++--- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/lms/djangoapps/course_api/blocks/api.py b/lms/djangoapps/course_api/blocks/api.py index 7bc8c1fc53bb..ea37f2fae4a4 100644 --- a/lms/djangoapps/course_api/blocks/api.py +++ b/lms/djangoapps/course_api/blocks/api.py @@ -1,7 +1,7 @@ """ API function for retrieving course blocks data """ -from crum import get_current_request +from edx_django_utils.cache import RequestCache import lms.djangoapps.course_blocks.api as course_blocks_api from lms.djangoapps.course_blocks.transformers.access_denied_filter import AccessDeniedMessageFilterTransformer @@ -138,9 +138,9 @@ def get_blocks( for block_key in block_keys_to_remove: blocks.remove_block(block_key, keep_descendants=True) - # store transformed blocks in the current request to be reused where possible for optimization - if current_request := get_current_request(): - setattr(current_request, "reusable_transformed_blocks", blocks) # pylint: disable=literal-used-as-attribute + # store transformed blocks in RequestCache to be reused where possible for optimization + request_cache = RequestCache("course_blocks") + request_cache.set("reusable_transformed_blocks", blocks) # serialize serializer_context = { diff --git a/lms/djangoapps/grades/course_data.py b/lms/djangoapps/grades/course_data.py index 668f814f613a..c57ff5635b3f 100644 --- a/lms/djangoapps/grades/course_data.py +++ b/lms/djangoapps/grades/course_data.py @@ -1,7 +1,7 @@ """ Code used to get and cache the requested course-data """ -from crum import get_current_request +from edx_django_utils.cache import RequestCache from lms.djangoapps.course_blocks.api import get_course_blocks from openedx.core.djangoapps.content.block_structure.api import get_block_structure_manager @@ -56,8 +56,14 @@ def location(self): # lint-amnesty, pylint: disable=missing-function-docstring @property def structure(self): # lint-amnesty, pylint: disable=missing-function-docstring if self._structure is None: - # reuse transformed blocks from request if available - reusable_transformed_blocks = getattr(get_current_request(), "reusable_transformed_blocks", None) + """ + The get_course_blocks function proved to be a major time sink during a request at "blocks/". + This caching logic helps improve the response time by getting the already transformed course blocks + from RequestCache and thus reducing the number of times that the get_course_blocks function is called. + """ + request_cache = RequestCache("course_blocks") + cached_response = request_cache.get_cached_response("reusable_transformed_blocks") + reusable_transformed_blocks = cached_response.value if cached_response.is_found else None self._structure = reusable_transformed_blocks or get_course_blocks( self.user, self.location, From 5bc490e073378d53b37ed8ebf442843b7af24d24 Mon Sep 17 00:00:00 2001 From: Serhii Nanai Date: Wed, 13 Aug 2025 17:10:59 +0300 Subject: [PATCH 07/20] docs: use comment instead of docstring --- lms/djangoapps/grades/course_data.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/lms/djangoapps/grades/course_data.py b/lms/djangoapps/grades/course_data.py index c57ff5635b3f..0e4395c5e652 100644 --- a/lms/djangoapps/grades/course_data.py +++ b/lms/djangoapps/grades/course_data.py @@ -56,11 +56,10 @@ def location(self): # lint-amnesty, pylint: disable=missing-function-docstring @property def structure(self): # lint-amnesty, pylint: disable=missing-function-docstring if self._structure is None: - """ - The get_course_blocks function proved to be a major time sink during a request at "blocks/". - This caching logic helps improve the response time by getting the already transformed course blocks - from RequestCache and thus reducing the number of times that the get_course_blocks function is called. - """ + # The get_course_blocks function proved to be a major time sink during a request at "blocks/". + # This caching logic helps improve the response time by getting the already transformed course blocks + # from RequestCache and thus reducing the number of times that the get_course_blocks function is called. + request_cache = RequestCache("course_blocks") cached_response = request_cache.get_cached_response("reusable_transformed_blocks") reusable_transformed_blocks = cached_response.value if cached_response.is_found else None From 7ef8fd11ed0a549173878985ba6bc9d1bee062d2 Mon Sep 17 00:00:00 2001 From: Serhii Nanai Date: Mon, 18 Aug 2025 11:36:05 +0300 Subject: [PATCH 08/20] fix: copy blocks before caching, rename cache key --- lms/djangoapps/course_api/blocks/api.py | 9 +++++---- lms/djangoapps/grades/course_data.py | 7 ++++--- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/lms/djangoapps/course_api/blocks/api.py b/lms/djangoapps/course_api/blocks/api.py index ea37f2fae4a4..31610c607629 100644 --- a/lms/djangoapps/course_api/blocks/api.py +++ b/lms/djangoapps/course_api/blocks/api.py @@ -128,6 +128,11 @@ def get_blocks( include_has_scheduled_content=include_has_scheduled_content ) + # store a copy of the transformed, but still unfiltered, course blocks in RequestCache to be reused + # wherever possible for optimization + request_cache = RequestCache("unfiltered_course_structure") + request_cache.set("reusable_transformed_blocks", blocks.copy()) + # filter blocks by types if block_types_filter: block_keys_to_remove = [] @@ -138,10 +143,6 @@ def get_blocks( for block_key in block_keys_to_remove: blocks.remove_block(block_key, keep_descendants=True) - # store transformed blocks in RequestCache to be reused where possible for optimization - request_cache = RequestCache("course_blocks") - request_cache.set("reusable_transformed_blocks", blocks) - # serialize serializer_context = { 'request': request, diff --git a/lms/djangoapps/grades/course_data.py b/lms/djangoapps/grades/course_data.py index 0e4395c5e652..60c39d5afdbc 100644 --- a/lms/djangoapps/grades/course_data.py +++ b/lms/djangoapps/grades/course_data.py @@ -57,10 +57,11 @@ def location(self): # lint-amnesty, pylint: disable=missing-function-docstring def structure(self): # lint-amnesty, pylint: disable=missing-function-docstring if self._structure is None: # The get_course_blocks function proved to be a major time sink during a request at "blocks/". - # This caching logic helps improve the response time by getting the already transformed course blocks - # from RequestCache and thus reducing the number of times that the get_course_blocks function is called. + # This caching logic helps improve the response time by getting a copy of the already transformed, but still + # unfiltered, course blocks from RequestCache and thus reducing the number of times that + # the get_course_blocks function is called. - request_cache = RequestCache("course_blocks") + request_cache = RequestCache("unfiltered_course_structure") cached_response = request_cache.get_cached_response("reusable_transformed_blocks") reusable_transformed_blocks = cached_response.value if cached_response.is_found else None self._structure = reusable_transformed_blocks or get_course_blocks( From b40d755af0e05c5110fb23ebe26befaabe6526ab Mon Sep 17 00:00:00 2001 From: Serhii Nanai Date: Mon, 18 Aug 2025 16:45:19 +0300 Subject: [PATCH 09/20] test: add test for depth in request params --- .../mobile_api/tests/test_course_info_views.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/lms/djangoapps/mobile_api/tests/test_course_info_views.py b/lms/djangoapps/mobile_api/tests/test_course_info_views.py index 0d16cd99e534..b7cdc6b03961 100644 --- a/lms/djangoapps/mobile_api/tests/test_course_info_views.py +++ b/lms/djangoapps/mobile_api/tests/test_course_info_views.py @@ -494,6 +494,16 @@ def test_response_keys(self): self.assertSetEqual(set(data["course_modes"][0]), expected_course_modes_keys) self.assertSetEqual(set(data["course_progress"]), expected_course_progress_keys) + def test_block_count_depends_on_depth_in_request_params(self): + response_depth_zero = self.verify_response(url=self.url, params={'depth': 0}) + response_depth_one = self.verify_response(url=self.url, params={'depth': 1}) + blocks_depth_zero = [block for block in self.store.get_items(self.course_key) if block.category == "course"] + blocks_depth_one = [ + block for block in self.store.get_items(self.course_key) if block.category in ("course", "chapter") + ] + self.assertEqual(len(response_depth_zero.data["blocks"]), len(blocks_depth_zero)) + self.assertEqual(len(response_depth_one.data["blocks"]), len(blocks_depth_one)) + class TestCourseEnrollmentDetailsView(MobileAPITestCase, MilestonesTestCaseMixin): # lint-amnesty, pylint: disable=test-inherits-tests """ From 76b9cb3cc77b459047dc77ae10088101077a5f30 Mon Sep 17 00:00:00 2001 From: Serhii Nanai Date: Mon, 18 Aug 2025 16:51:07 +0300 Subject: [PATCH 10/20] docs: expand comment on copying before caching --- lms/djangoapps/course_api/blocks/api.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/course_api/blocks/api.py b/lms/djangoapps/course_api/blocks/api.py index 31610c607629..b51658439b50 100644 --- a/lms/djangoapps/course_api/blocks/api.py +++ b/lms/djangoapps/course_api/blocks/api.py @@ -128,8 +128,9 @@ def get_blocks( include_has_scheduled_content=include_has_scheduled_content ) - # store a copy of the transformed, but still unfiltered, course blocks in RequestCache to be reused - # wherever possible for optimization + # Store a copy of the transformed, but still unfiltered, course blocks in RequestCache to be reused + # wherever possible for optimization. Copying is required to make sure the cached structure is not mutated + # by the filtering below. request_cache = RequestCache("unfiltered_course_structure") request_cache.set("reusable_transformed_blocks", blocks.copy()) From a25a1234e5b19b783817d63669ea8c868f87046f Mon Sep 17 00:00:00 2001 From: Serhii Nanai Date: Fri, 3 Oct 2025 22:37:25 +0300 Subject: [PATCH 11/20] feat: add remove_future_blocks function, reuse blocks in getting assignments --- lms/djangoapps/course_api/blocks/api.py | 10 ++++- lms/djangoapps/course_api/blocks/views.py | 47 +++++++++++++++++++++++ lms/djangoapps/courseware/courses.py | 9 ++++- 3 files changed, 63 insertions(+), 3 deletions(-) diff --git a/lms/djangoapps/course_api/blocks/api.py b/lms/djangoapps/course_api/blocks/api.py index b51658439b50..cf52acd07f04 100644 --- a/lms/djangoapps/course_api/blocks/api.py +++ b/lms/djangoapps/course_api/blocks/api.py @@ -123,7 +123,7 @@ def get_blocks( user, usage_key, transformers, - allow_start_dates_in_future=allow_start_dates_in_future, + allow_start_dates_in_future=True, include_completion=include_completion, include_has_scheduled_content=include_has_scheduled_content ) @@ -145,10 +145,16 @@ def get_blocks( blocks.remove_block(block_key, keep_descendants=True) # serialize + + # Include start field to be able to use it in filtering. + requested_fields = requested_fields or set() + if 'start' not in requested_fields: + requested_fields.add('start') + serializer_context = { 'request': request, 'block_structure': blocks, - 'requested_fields': requested_fields or [], + 'requested_fields': requested_fields, } if return_type == 'dict': diff --git a/lms/djangoapps/course_api/blocks/views.py b/lms/djangoapps/course_api/blocks/views.py index 96679a562957..b24c3e8ad423 100644 --- a/lms/djangoapps/course_api/blocks/views.py +++ b/lms/djangoapps/course_api/blocks/views.py @@ -339,10 +339,57 @@ def list(self, request, hide_access_denials=False): # pylint: disable=arguments if not root: raise ValidationError(f"Unable to find course block in '{course_key_string}'") + include_start = "start" in request.query_params['requested_fields'] + self.remove_future_blocks(course_blocks, include_start) + recurse_mark_complete(root, course_blocks) return response + @staticmethod + def remove_future_blocks(course_blocks, include_start: bool): + """ + Mutates course_blocks in place: + - removes blocks whose 'start' is in the future + - also removes references to them from parents' 'children' lists + - removes 'start' key from all blocks if it wasn't requested + """ + from datetime import datetime, timezone + + # blocks = response_data.get("blocks", {}) + if not course_blocks: + return course_blocks + + now = datetime.now(timezone.utc) + + # 1. Collect IDs of blocks to remove + to_remove = set() + for block_id, block in course_blocks.items(): + start = block.get("start") + if start and start > now: + to_remove.add(block_id) + + if not to_remove: + return course_blocks + + # 2. Remove the blocks themselves + for block_id in to_remove: + course_blocks.pop(block_id, None) + + # 3. Clean up children lists + for block in course_blocks.values(): + children = block.get("children") + if children: + block["children"] = [cid for cid in children if cid not in to_remove] + + # 4. Optionally remove 'start' key from visible blocks + if not include_start: + for block in course_blocks.values(): + block.pop("start", None) + + return course_blocks + + @method_decorator(transaction.non_atomic_requests, name='dispatch') @view_auth_classes(is_authenticated=False) class BlockMetadataView(DeveloperErrorViewMixin, ListAPIView): diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 2c46248456f2..34ee59f33e8d 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -17,6 +17,7 @@ from django.http import Http404, QueryDict from django.urls import reverse from django.utils.translation import gettext as _ +from edx_django_utils.cache import RequestCache from edx_django_utils.monitoring import function_trace, set_custom_attribute from fs.errors import ResourceNotFound from opaque_keys.edx.keys import UsageKey @@ -632,7 +633,13 @@ def get_course_assignments(course_key, user, include_access=False, include_witho store = modulestore() course_usage_key = store.make_course_usage_key(course_key) - block_data = get_course_blocks(user, course_usage_key, allow_start_dates_in_future=True, include_completion=True) + + request_cache = RequestCache("unfiltered_course_structure") + cached_response = request_cache.get_cached_response("reusable_transformed_blocks") + reusable_transformed_blocks = cached_response.value if cached_response.is_found else None + block_data = reusable_transformed_blocks or get_course_blocks( + user, course_usage_key, allow_start_dates_in_future=True, include_completion=True + ) now = datetime.now(pytz.UTC) assignments = [] From 9e0d7368717ce9ae2d2b151b09df95170320ca78 Mon Sep 17 00:00:00 2001 From: Serhii Nanai Date: Thu, 9 Oct 2025 11:00:08 +0300 Subject: [PATCH 12/20] docs: add comments --- lms/djangoapps/course_api/blocks/api.py | 9 +++++++-- lms/djangoapps/course_api/blocks/views.py | 2 ++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/course_api/blocks/api.py b/lms/djangoapps/course_api/blocks/api.py index cf52acd07f04..4a74e9f6dcea 100644 --- a/lms/djangoapps/course_api/blocks/api.py +++ b/lms/djangoapps/course_api/blocks/api.py @@ -118,12 +118,15 @@ def get_blocks( ), ] + # Include future dates such that get_course_assignments can reuse the block structure from RequestCache + allow_start_dates_in_future = True + # transform blocks = course_blocks_api.get_course_blocks( user, usage_key, transformers, - allow_start_dates_in_future=True, + allow_start_dates_in_future=allow_start_dates_in_future, include_completion=include_completion, include_has_scheduled_content=include_has_scheduled_content ) @@ -146,7 +149,9 @@ def get_blocks( # serialize - # Include start field to be able to use it in filtering. + # Since we included blocks with future start dates in our block structure, + # we need to include the 'start' field to filter out such blocks before returning the response. + # If 'start' field is not requested, it will be removed from the response. requested_fields = requested_fields or set() if 'start' not in requested_fields: requested_fields.add('start') diff --git a/lms/djangoapps/course_api/blocks/views.py b/lms/djangoapps/course_api/blocks/views.py index b24c3e8ad423..d57c56062a28 100644 --- a/lms/djangoapps/course_api/blocks/views.py +++ b/lms/djangoapps/course_api/blocks/views.py @@ -339,6 +339,8 @@ def list(self, request, hide_access_denials=False): # pylint: disable=arguments if not root: raise ValidationError(f"Unable to find course block in '{course_key_string}'") + # Earlier we included blocks with future start dates in the collected/cached block structure. + # Now we need to emulate allow_start_dates_in_future=False by removing any such blocks. include_start = "start" in request.query_params['requested_fields'] self.remove_future_blocks(course_blocks, include_start) From e2ad6ece6f6eb8c927f8e2889cf96868a5dfda9c Mon Sep 17 00:00:00 2001 From: Serhii Nanai Date: Thu, 9 Oct 2025 11:10:37 +0300 Subject: [PATCH 13/20] fix: fix lint issues --- lms/djangoapps/course_api/blocks/views.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lms/djangoapps/course_api/blocks/views.py b/lms/djangoapps/course_api/blocks/views.py index d57c56062a28..eaf1bca90d6f 100644 --- a/lms/djangoapps/course_api/blocks/views.py +++ b/lms/djangoapps/course_api/blocks/views.py @@ -341,13 +341,12 @@ def list(self, request, hide_access_denials=False): # pylint: disable=arguments # Earlier we included blocks with future start dates in the collected/cached block structure. # Now we need to emulate allow_start_dates_in_future=False by removing any such blocks. - include_start = "start" in request.query_params['requested_fields'] + include_start = "start" in request.query_params['requested_fields'] self.remove_future_blocks(course_blocks, include_start) recurse_mark_complete(root, course_blocks) return response - @staticmethod def remove_future_blocks(course_blocks, include_start: bool): """ From e6d8fc11eed4c15cf5df6030285d14a0da086953 Mon Sep 17 00:00:00 2001 From: Serhii Nanai Date: Thu, 9 Oct 2025 12:00:47 +0300 Subject: [PATCH 14/20] fix: wrap requested_fields in set() --- lms/djangoapps/course_api/blocks/api.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/lms/djangoapps/course_api/blocks/api.py b/lms/djangoapps/course_api/blocks/api.py index 4a74e9f6dcea..35b098959a44 100644 --- a/lms/djangoapps/course_api/blocks/api.py +++ b/lms/djangoapps/course_api/blocks/api.py @@ -137,6 +137,12 @@ def get_blocks( request_cache = RequestCache("unfiltered_course_structure") request_cache.set("reusable_transformed_blocks", blocks.copy()) + # Since we included blocks with future start dates in our block structure, + # we need to include the 'start' field to filter out such blocks before returning the response. + # If 'start' field is not requested, it will be removed from the response. + requested_fields = set(requested_fields) + requested_fields.add('start') + # filter blocks by types if block_types_filter: block_keys_to_remove = [] @@ -148,14 +154,6 @@ def get_blocks( blocks.remove_block(block_key, keep_descendants=True) # serialize - - # Since we included blocks with future start dates in our block structure, - # we need to include the 'start' field to filter out such blocks before returning the response. - # If 'start' field is not requested, it will be removed from the response. - requested_fields = requested_fields or set() - if 'start' not in requested_fields: - requested_fields.add('start') - serializer_context = { 'request': request, 'block_structure': blocks, From 3ec750c670c809a0d96815c99025f65d844fbc56 Mon Sep 17 00:00:00 2001 From: Serhii Nanai Date: Thu, 9 Oct 2025 12:08:00 +0300 Subject: [PATCH 15/20] fix: handle start key in the main loop --- lms/djangoapps/course_api/blocks/views.py | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/lms/djangoapps/course_api/blocks/views.py b/lms/djangoapps/course_api/blocks/views.py index eaf1bca90d6f..973731c7a0cd 100644 --- a/lms/djangoapps/course_api/blocks/views.py +++ b/lms/djangoapps/course_api/blocks/views.py @@ -357,7 +357,6 @@ def remove_future_blocks(course_blocks, include_start: bool): """ from datetime import datetime, timezone - # blocks = response_data.get("blocks", {}) if not course_blocks: return course_blocks @@ -366,7 +365,8 @@ def remove_future_blocks(course_blocks, include_start: bool): # 1. Collect IDs of blocks to remove to_remove = set() for block_id, block in course_blocks.items(): - start = block.get("start") + get_field = block.get if include_start else block.pop + start = get_field("start") if start and start > now: to_remove.add(block_id) @@ -383,11 +383,6 @@ def remove_future_blocks(course_blocks, include_start: bool): if children: block["children"] = [cid for cid in children if cid not in to_remove] - # 4. Optionally remove 'start' key from visible blocks - if not include_start: - for block in course_blocks.values(): - block.pop("start", None) - return course_blocks From 2a0b398e59f483abaa5e36863e94fdad148a650c Mon Sep 17 00:00:00 2001 From: Serhii Nanai Date: Thu, 9 Oct 2025 13:34:19 +0300 Subject: [PATCH 16/20] fix: pass for_blocks_view param to get_blocks --- lms/djangoapps/course_api/blocks/api.py | 30 +++++++++++++---------- lms/djangoapps/course_api/blocks/views.py | 1 + 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/lms/djangoapps/course_api/blocks/api.py b/lms/djangoapps/course_api/blocks/api.py index 35b098959a44..6c9a8bdf1b52 100644 --- a/lms/djangoapps/course_api/blocks/api.py +++ b/lms/djangoapps/course_api/blocks/api.py @@ -29,6 +29,7 @@ def get_blocks( block_types_filter=None, hide_access_denials=False, allow_start_dates_in_future=False, + for_blocks_view=False, ): """ Return a serialized representation of the course blocks. @@ -61,6 +62,7 @@ def get_blocks( allow_start_dates_in_future (bool): When True, will allow blocks to be returned that can bypass the StartDateTransformer's filter to show blocks with start dates in the future. + for_blocks_view (bool): When True, will use the block caching logic using RequestCache """ if HIDE_ACCESS_DENIALS_FLAG.is_enabled(): @@ -118,8 +120,9 @@ def get_blocks( ), ] - # Include future dates such that get_course_assignments can reuse the block structure from RequestCache - allow_start_dates_in_future = True + if for_blocks_view: + # Include future dates such that get_course_assignments can reuse the block structure from RequestCache + allow_start_dates_in_future = True # transform blocks = course_blocks_api.get_course_blocks( @@ -131,17 +134,18 @@ def get_blocks( include_has_scheduled_content=include_has_scheduled_content ) - # Store a copy of the transformed, but still unfiltered, course blocks in RequestCache to be reused - # wherever possible for optimization. Copying is required to make sure the cached structure is not mutated - # by the filtering below. - request_cache = RequestCache("unfiltered_course_structure") - request_cache.set("reusable_transformed_blocks", blocks.copy()) - - # Since we included blocks with future start dates in our block structure, - # we need to include the 'start' field to filter out such blocks before returning the response. - # If 'start' field is not requested, it will be removed from the response. - requested_fields = set(requested_fields) - requested_fields.add('start') + if for_blocks_view: + # Store a copy of the transformed, but still unfiltered, course blocks in RequestCache to be reused + # wherever possible for optimization. Copying is required to make sure the cached structure is not mutated + # by the filtering below. + request_cache = RequestCache("unfiltered_course_structure") + request_cache.set("reusable_transformed_blocks", blocks.copy()) + + # Since we included blocks with future start dates in our block structure, + # we need to include the 'start' field to filter out such blocks before returning the response. + # If 'start' field is not requested, it will be removed from the response. + requested_fields = set(requested_fields) + requested_fields.add('start') # filter blocks by types if block_types_filter: diff --git a/lms/djangoapps/course_api/blocks/views.py b/lms/djangoapps/course_api/blocks/views.py index 973731c7a0cd..6338971b4da4 100644 --- a/lms/djangoapps/course_api/blocks/views.py +++ b/lms/djangoapps/course_api/blocks/views.py @@ -237,6 +237,7 @@ def list(self, request, usage_key_string, hide_access_denials=False): # pylint: params.cleaned_data['return_type'], params.cleaned_data.get('block_types_filter', None), hide_access_denials=hide_access_denials, + for_blocks_view=True ) ) # If the username is an empty string, and not None, then we are requesting From f7bc539a269f1cb0dead740e13350fdc88af6ba9 Mon Sep 17 00:00:00 2001 From: Serhii Nanai Date: Fri, 17 Oct 2025 18:37:36 +0300 Subject: [PATCH 17/20] refactor: abstract caching logic, move to utils, add key constants --- lms/djangoapps/course_api/blocks/api.py | 13 +++++++------ lms/djangoapps/course_api/blocks/utils.py | 17 +++++++++++++++++ lms/djangoapps/course_api/blocks/views.py | 2 +- lms/djangoapps/courseware/courses.py | 7 ++----- lms/djangoapps/grades/course_data.py | 8 ++------ 5 files changed, 29 insertions(+), 18 deletions(-) diff --git a/lms/djangoapps/course_api/blocks/api.py b/lms/djangoapps/course_api/blocks/api.py index 6c9a8bdf1b52..28dc0531cbf7 100644 --- a/lms/djangoapps/course_api/blocks/api.py +++ b/lms/djangoapps/course_api/blocks/api.py @@ -14,6 +14,7 @@ from .toggles import HIDE_ACCESS_DENIALS_FLAG from .transformers.blocks_api import BlocksAPITransformer from .transformers.milestones import MilestonesAndSpecialExamsTransformer +from .utils import UNFILTERED_STRUCTURE_CACHE_KEY, REUSABLE_BLOCKS_CACHE_KEY def get_blocks( @@ -29,7 +30,7 @@ def get_blocks( block_types_filter=None, hide_access_denials=False, allow_start_dates_in_future=False, - for_blocks_view=False, + cache_with_future_dates=False, ): """ Return a serialized representation of the course blocks. @@ -62,7 +63,7 @@ def get_blocks( allow_start_dates_in_future (bool): When True, will allow blocks to be returned that can bypass the StartDateTransformer's filter to show blocks with start dates in the future. - for_blocks_view (bool): When True, will use the block caching logic using RequestCache + cache_with_future_dates (bool): When True, will use the block caching logic using RequestCache """ if HIDE_ACCESS_DENIALS_FLAG.is_enabled(): @@ -120,7 +121,7 @@ def get_blocks( ), ] - if for_blocks_view: + if cache_with_future_dates: # Include future dates such that get_course_assignments can reuse the block structure from RequestCache allow_start_dates_in_future = True @@ -134,12 +135,12 @@ def get_blocks( include_has_scheduled_content=include_has_scheduled_content ) - if for_blocks_view: + if cache_with_future_dates: # Store a copy of the transformed, but still unfiltered, course blocks in RequestCache to be reused # wherever possible for optimization. Copying is required to make sure the cached structure is not mutated # by the filtering below. - request_cache = RequestCache("unfiltered_course_structure") - request_cache.set("reusable_transformed_blocks", blocks.copy()) + request_cache = RequestCache(UNFILTERED_STRUCTURE_CACHE_KEY) + request_cache.set(REUSABLE_BLOCKS_CACHE_KEY, blocks.copy()) # Since we included blocks with future start dates in our block structure, # we need to include the 'start' field to filter out such blocks before returning the response. diff --git a/lms/djangoapps/course_api/blocks/utils.py b/lms/djangoapps/course_api/blocks/utils.py index 6f371624b7df..71cf0fd39405 100644 --- a/lms/djangoapps/course_api/blocks/utils.py +++ b/lms/djangoapps/course_api/blocks/utils.py @@ -1,6 +1,7 @@ """ Utils for Blocks """ +from edx_django_utils.cache import RequestCache from rest_framework.utils.serializer_helpers import ReturnList from openedx.core.djangoapps.discussions.models import ( @@ -9,6 +10,10 @@ ) +UNFILTERED_STRUCTURE_CACHE_KEY = "unfiltered_course_structure" +REUSABLE_BLOCKS_CACHE_KEY = "reusable_transformed_blocks" + + def filter_discussion_xblocks_from_response(response, course_key): """ Removes discussion xblocks if discussion provider is openedx. @@ -63,3 +68,15 @@ def filter_discussion_xblocks_from_response(response, course_key): response.data['blocks'] = filtered_blocks return response + + +def get_cached_transformed_blocks(): + """ + Helper function to get an unfiltered course structure from RequestCache, + including blocks with start dates in the future. + """ + request_cache = RequestCache(UNFILTERED_STRUCTURE_CACHE_KEY) + cached_response = request_cache.get_cached_response(REUSABLE_BLOCKS_CACHE_KEY) + reusable_transformed_blocks = cached_response.value if cached_response.is_found else None + + return reusable_transformed_blocks diff --git a/lms/djangoapps/course_api/blocks/views.py b/lms/djangoapps/course_api/blocks/views.py index 6338971b4da4..9b6d578f1fb6 100644 --- a/lms/djangoapps/course_api/blocks/views.py +++ b/lms/djangoapps/course_api/blocks/views.py @@ -237,7 +237,7 @@ def list(self, request, usage_key_string, hide_access_denials=False): # pylint: params.cleaned_data['return_type'], params.cleaned_data.get('block_types_filter', None), hide_access_denials=hide_access_denials, - for_blocks_view=True + cache_with_future_dates=True ) ) # If the username is an empty string, and not None, then we are requesting diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 34ee59f33e8d..c001c8c22b27 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -17,7 +17,6 @@ from django.http import Http404, QueryDict from django.urls import reverse from django.utils.translation import gettext as _ -from edx_django_utils.cache import RequestCache from edx_django_utils.monitoring import function_trace, set_custom_attribute from fs.errors import ResourceNotFound from opaque_keys.edx.keys import UsageKey @@ -27,6 +26,7 @@ from common.djangoapps.static_replace import replace_static_urls from common.djangoapps.util.date_utils import strftime_localized from lms.djangoapps import branding +from lms.djangoapps.course_api.blocks.utils import get_cached_transformed_blocks from lms.djangoapps.course_blocks.api import get_course_blocks from lms.djangoapps.courseware.access import has_access from lms.djangoapps.courseware.access_response import ( @@ -634,10 +634,7 @@ def get_course_assignments(course_key, user, include_access=False, include_witho store = modulestore() course_usage_key = store.make_course_usage_key(course_key) - request_cache = RequestCache("unfiltered_course_structure") - cached_response = request_cache.get_cached_response("reusable_transformed_blocks") - reusable_transformed_blocks = cached_response.value if cached_response.is_found else None - block_data = reusable_transformed_blocks or get_course_blocks( + block_data = get_cached_transformed_blocks() or get_course_blocks( user, course_usage_key, allow_start_dates_in_future=True, include_completion=True ) diff --git a/lms/djangoapps/grades/course_data.py b/lms/djangoapps/grades/course_data.py index 60c39d5afdbc..523d6e6df38d 100644 --- a/lms/djangoapps/grades/course_data.py +++ b/lms/djangoapps/grades/course_data.py @@ -1,13 +1,13 @@ """ Code used to get and cache the requested course-data """ -from edx_django_utils.cache import RequestCache from lms.djangoapps.course_blocks.api import get_course_blocks from openedx.core.djangoapps.content.block_structure.api import get_block_structure_manager from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order from .transformer import GradesTransformer +from ..course_api.blocks.utils import get_cached_transformed_blocks class CourseData: @@ -60,11 +60,7 @@ def structure(self): # lint-amnesty, pylint: disable=missing-function-docstring # This caching logic helps improve the response time by getting a copy of the already transformed, but still # unfiltered, course blocks from RequestCache and thus reducing the number of times that # the get_course_blocks function is called. - - request_cache = RequestCache("unfiltered_course_structure") - cached_response = request_cache.get_cached_response("reusable_transformed_blocks") - reusable_transformed_blocks = cached_response.value if cached_response.is_found else None - self._structure = reusable_transformed_blocks or get_course_blocks( + self._structure = get_cached_transformed_blocks() or get_course_blocks( self.user, self.location, collected_block_structure=self._collected_block_structure, From 74bb27f28e57f702798efd1f7cc224899ea53e58 Mon Sep 17 00:00:00 2001 From: Serhii Nanai Date: Wed, 29 Oct 2025 11:51:06 +0200 Subject: [PATCH 18/20] refactor: move datetime import to module-level --- lms/djangoapps/course_api/blocks/views.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lms/djangoapps/course_api/blocks/views.py b/lms/djangoapps/course_api/blocks/views.py index 9b6d578f1fb6..7f81861b9b7b 100644 --- a/lms/djangoapps/course_api/blocks/views.py +++ b/lms/djangoapps/course_api/blocks/views.py @@ -2,6 +2,7 @@ CourseBlocks API views """ +from datetime import datetime, timezone from django.core.exceptions import ValidationError from django.db import transaction @@ -356,8 +357,6 @@ def remove_future_blocks(course_blocks, include_start: bool): - also removes references to them from parents' 'children' lists - removes 'start' key from all blocks if it wasn't requested """ - from datetime import datetime, timezone - if not course_blocks: return course_blocks From 37e7a9e9cb4f5eb3d6bb200291073372e332759e Mon Sep 17 00:00:00 2001 From: Serhii Nanai Date: Wed, 29 Oct 2025 12:51:14 +0200 Subject: [PATCH 19/20] refactor: change request cache namespace variable and value --- lms/djangoapps/course_api/blocks/api.py | 4 ++-- lms/djangoapps/course_api/blocks/utils.py | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lms/djangoapps/course_api/blocks/api.py b/lms/djangoapps/course_api/blocks/api.py index 28dc0531cbf7..80c2bb7fc5ff 100644 --- a/lms/djangoapps/course_api/blocks/api.py +++ b/lms/djangoapps/course_api/blocks/api.py @@ -14,7 +14,7 @@ from .toggles import HIDE_ACCESS_DENIALS_FLAG from .transformers.blocks_api import BlocksAPITransformer from .transformers.milestones import MilestonesAndSpecialExamsTransformer -from .utils import UNFILTERED_STRUCTURE_CACHE_KEY, REUSABLE_BLOCKS_CACHE_KEY +from .utils import COURSE_API_REQUEST_CACHE_NAMESPACE, REUSABLE_BLOCKS_CACHE_KEY def get_blocks( @@ -139,7 +139,7 @@ def get_blocks( # Store a copy of the transformed, but still unfiltered, course blocks in RequestCache to be reused # wherever possible for optimization. Copying is required to make sure the cached structure is not mutated # by the filtering below. - request_cache = RequestCache(UNFILTERED_STRUCTURE_CACHE_KEY) + request_cache = RequestCache(COURSE_API_REQUEST_CACHE_NAMESPACE) request_cache.set(REUSABLE_BLOCKS_CACHE_KEY, blocks.copy()) # Since we included blocks with future start dates in our block structure, diff --git a/lms/djangoapps/course_api/blocks/utils.py b/lms/djangoapps/course_api/blocks/utils.py index 71cf0fd39405..c14530e37555 100644 --- a/lms/djangoapps/course_api/blocks/utils.py +++ b/lms/djangoapps/course_api/blocks/utils.py @@ -10,7 +10,7 @@ ) -UNFILTERED_STRUCTURE_CACHE_KEY = "unfiltered_course_structure" +COURSE_API_REQUEST_CACHE_NAMESPACE = "course_api" REUSABLE_BLOCKS_CACHE_KEY = "reusable_transformed_blocks" @@ -75,8 +75,8 @@ def get_cached_transformed_blocks(): Helper function to get an unfiltered course structure from RequestCache, including blocks with start dates in the future. """ - request_cache = RequestCache(UNFILTERED_STRUCTURE_CACHE_KEY) + request_cache = RequestCache(COURSE_API_REQUEST_CACHE_NAMESPACE) cached_response = request_cache.get_cached_response(REUSABLE_BLOCKS_CACHE_KEY) - reusable_transformed_blocks = cached_response.value if cached_response.is_found else None + reusable_transformed_blocks = cached_response.value.copy() if cached_response.is_found else None return reusable_transformed_blocks From 9565fc8169cfb989edd55fc49fe8467f7b88e73d Mon Sep 17 00:00:00 2001 From: Serhii Nanai Date: Thu, 30 Oct 2025 09:21:15 +0200 Subject: [PATCH 20/20] docs: caution against mutating cached blocks, revert .copy() --- lms/djangoapps/course_api/blocks/utils.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lms/djangoapps/course_api/blocks/utils.py b/lms/djangoapps/course_api/blocks/utils.py index c14530e37555..0686abc2fac1 100644 --- a/lms/djangoapps/course_api/blocks/utils.py +++ b/lms/djangoapps/course_api/blocks/utils.py @@ -74,9 +74,12 @@ def get_cached_transformed_blocks(): """ Helper function to get an unfiltered course structure from RequestCache, including blocks with start dates in the future. + + Caution: For performance reasons, the function returns the structure object itself, not its copy. + This means the retrieved structure is supposed to be read-only and should not be mutated by consumers. """ request_cache = RequestCache(COURSE_API_REQUEST_CACHE_NAMESPACE) cached_response = request_cache.get_cached_response(REUSABLE_BLOCKS_CACHE_KEY) - reusable_transformed_blocks = cached_response.value.copy() if cached_response.is_found else None + reusable_transformed_blocks = cached_response.value if cached_response.is_found else None return reusable_transformed_blocks