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

revert: feat: [FC-0092] Optimize Course Info Blocks API (#37122) (#37661)

Browse Source 
This reverts commit 7cd4170ca7.
This commit is contained in:
Asad Ali
2025-11-20 22:05:14 +05:00
committed by GitHub
parent 1253831c52
commit ab6cf6e85e
6 changed files with 5 additions and 168 deletions
Download Patch File Download Diff File Expand all files Collapse all files

24
lms/djangoapps/course_api/blocks/api.py
View File

@@ -1,7 +1,7 @@
"""
API function for retrieving course blocks data
"""
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
@@ -14,7 +14,6 @@ from .serializers import BlockDictSerializer, BlockSerializer
from .toggles import HIDE_ACCESS_DENIALS_FLAG
from .transformers.blocks_api import BlocksAPITransformer
from .transformers.milestones import MilestonesAndSpecialExamsTransformer
from .utils import COURSE_API_REQUEST_CACHE_NAMESPACE, REUSABLE_BLOCKS_CACHE_KEY
def get_blocks(
@@ -30,7 +29,6 @@ def get_blocks(
block_types_filter=None,
hide_access_denials=False,
allow_start_dates_in_future=False,
cache_with_future_dates=False,
):
"""
Return a serialized representation of the course blocks.
@@ -63,7 +61,6 @@ 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.
cache_with_future_dates (bool): When True, will use the block caching logic using RequestCache
"""
if HIDE_ACCESS_DENIALS_FLAG.is_enabled():
@@ -121,10 +118,6 @@ def get_blocks(
),
]
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
# transform
blocks = course_blocks_api.get_course_blocks(
user,
@@ -135,19 +128,6 @@ def get_blocks(
include_has_scheduled_content=include_has_scheduled_content
)
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(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,
# 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 = []
@@ -162,7 +142,7 @@ def get_blocks(
serializer_context = {
'request': request,
'block_structure': blocks,
'requested_fields': requested_fields,
'requested_fields': requested_fields or [],
}
if return_type == 'dict':

20
lms/djangoapps/course_api/blocks/utils.py
View File

@@ -1,7 +1,6 @@
"""
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 (
@@ -10,10 +9,6 @@ from openedx.core.djangoapps.discussions.models import (
)
COURSE_API_REQUEST_CACHE_NAMESPACE = "course_api"
REUSABLE_BLOCKS_CACHE_KEY = "reusable_transformed_blocks"
def filter_discussion_xblocks_from_response(response, course_key):
"""
Removes discussion xblocks if discussion provider is openedx.
@@ -68,18 +63,3 @@ 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.
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 if cached_response.is_found else None
return reusable_transformed_blocks

43
lms/djangoapps/course_api/blocks/views.py
View File

@@ -2,7 +2,6 @@
CourseBlocks API views
"""
from datetime import datetime, timezone
from django.core.exceptions import ValidationError
from django.db import transaction
@@ -238,7 +237,6 @@ class BlocksView(DeveloperErrorViewMixin, ListAPIView):
params.cleaned_data['return_type'],
params.cleaned_data.get('block_types_filter', None),
hide_access_denials=hide_access_denials,
cache_with_future_dates=True
)
)
# If the username is an empty string, and not None, then we are requesting
@@ -341,50 +339,9 @@ class BlocksInCourseView(BlocksView):
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)
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
"""
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():
get_field = block.get if include_start else block.pop
start = get_field("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]
return course_blocks
@method_decorator(transaction.non_atomic_requests, name='dispatch')
@view_auth_classes(is_authenticated=False)

6
lms/djangoapps/courseware/courses.py
View File

@@ -26,7 +26,6 @@ from common.djangoapps.edxmako.shortcuts import render_to_string
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 (
@@ -637,10 +636,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)
block_data = get_cached_transformed_blocks() or get_course_blocks(
user, course_usage_key, allow_start_dates_in_future=True, include_completion=True
)
block_data = get_course_blocks(user, course_usage_key, allow_start_dates_in_future=True, include_completion=True)
now = datetime.now(pytz.UTC)
assignments = []

8
lms/djangoapps/grades/course_data.py
View File

@@ -2,12 +2,12 @@
Code used to get and cache the requested course-data
"""
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:
@@ -56,11 +56,7 @@ class CourseData:
@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 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.
self._structure = get_cached_transformed_blocks() or get_course_blocks(
self._structure = get_course_blocks(
self.user,
self.location,
collected_block_structure=self._collected_block_structure,

72
lms/djangoapps/mobile_api/tests/test_course_info_views.py
View File

@@ -432,78 +432,6 @@ class TestBlocksInfoInCourseView(TestBlocksInCourseView, MilestonesTestCaseMixin
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)
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
"""