diff --git a/lms/djangoapps/course_api/blocks/api.py b/lms/djangoapps/course_api/blocks/api.py
index a79e9759e1..80c2bb7fc5 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 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,6 +14,7 @@ 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(
@@ -29,6 +30,7 @@ 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.
@@ -61,6 +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.
+ cache_with_future_dates (bool): When True, will use the block caching logic using RequestCache
"""
if HIDE_ACCESS_DENIALS_FLAG.is_enabled():
@@ -118,6 +121,10 @@ 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,
@@ -128,6 +135,19 @@ 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 = []
@@ -142,7 +162,7 @@ def get_blocks(
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/utils.py b/lms/djangoapps/course_api/blocks/utils.py
index 6f371624b7..0686abc2fa 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 @@ 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.
@@ -63,3 +68,18 @@ 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
diff --git a/lms/djangoapps/course_api/blocks/views.py b/lms/djangoapps/course_api/blocks/views.py
index 96679a5629..7f81861b9b 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
@@ -237,6 +238,7 @@ 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
@@ -339,9 +341,50 @@ 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)
diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py
index bbf9d53947..3b527cda4d 100644
--- a/lms/djangoapps/courseware/courses.py
+++ b/lms/djangoapps/courseware/courses.py
@@ -26,6 +26,7 @@ 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 (
@@ -636,7 +637,10 @@ 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)
+
+ block_data = get_cached_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 = []
diff --git a/lms/djangoapps/courseware/tests/test_video_mongo.py b/lms/djangoapps/courseware/tests/test_video_mongo.py
index a1829f4e0d..48f45267ca 100644
--- a/lms/djangoapps/courseware/tests/test_video_mongo.py
+++ b/lms/djangoapps/courseware/tests/test_video_mongo.py
@@ -88,7 +88,6 @@ class TestVideoYouTube(TestVideo): # lint-amnesty, pylint: disable=missing-clas
expected_context = {
'autoadvance_enabled': False,
- 'branding_info': None,
'license': None,
'bumper_metadata': 'null',
'block_id': str(self.block.location),
@@ -177,7 +176,6 @@ class TestVideoNonYouTube(TestVideo): # pylint: disable=test-inherits-tests
expected_context = {
'autoadvance_enabled': False,
- 'branding_info': None,
'license': None,
'bumper_metadata': 'null',
'block_id': str(self.block.location),
@@ -454,7 +452,6 @@ class TestGetHtmlMethod(BaseTestVideoXBlock):
expected_context = {
'autoadvance_enabled': False,
- 'branding_info': None,
'license': None,
'bumper_metadata': 'null',
'block_id': str(self.block.location),
@@ -587,7 +584,6 @@ class TestGetHtmlMethod(BaseTestVideoXBlock):
initial_context = {
'autoadvance_enabled': False,
- 'branding_info': None,
'license': None,
'bumper_metadata': 'null',
'block_id': str(self.block.location),
@@ -726,7 +722,6 @@ class TestGetHtmlMethod(BaseTestVideoXBlock):
initial_context = {
'autoadvance_enabled': False,
- 'branding_info': None,
'license': None,
'bumper_metadata': 'null',
'block_id': str(self.block.location),
@@ -914,7 +909,6 @@ class TestGetHtmlMethod(BaseTestVideoXBlock):
initial_context = {
'autoadvance_enabled': False,
- 'branding_info': None,
'license': None,
'bumper_metadata': 'null',
'block_id': str(self.block.location),
@@ -971,21 +965,12 @@ class TestGetHtmlMethod(BaseTestVideoXBlock):
return context, expected_context
# pylint: disable=invalid-name
- @patch('xmodule.video_block.video_block.BrandingInfoConfig')
@patch('xmodule.video_block.video_block.rewrite_video_url')
- def test_get_html_cdn_source(self, mocked_get_video, mock_BrandingInfoConfig):
+ def test_get_html_cdn_source(self, mocked_get_video):
"""
Test if sources got from CDN
"""
- mock_BrandingInfoConfig.get_config.return_value = {
- "CN": {
- 'url': 'http://www.xuetangx.com',
- 'logo_src': 'http://www.xuetangx.com/static/images/logo.png',
- 'logo_tag': 'Video hosted by XuetangX.com'
- }
- }
-
def side_effect(*args, **kwargs): # lint-amnesty, pylint: disable=unused-argument
cdn = {
'http://example.com/example.mp4': 'http://cdn-example.com/example.mp4',
@@ -1031,11 +1016,6 @@ class TestGetHtmlMethod(BaseTestVideoXBlock):
initial_context = {
'autoadvance_enabled': False,
- 'branding_info': {
- 'logo_src': 'http://www.xuetangx.com/static/images/logo.png',
- 'logo_tag': 'Video hosted by XuetangX.com',
- 'url': 'http://www.xuetangx.com'
- },
'license': None,
'bumper_metadata': 'null',
'block_id': str(self.block.location),
@@ -1138,7 +1118,6 @@ class TestGetHtmlMethod(BaseTestVideoXBlock):
initial_context = {
'autoadvance_enabled': False,
- 'branding_info': None,
'license': None,
'bumper_metadata': 'null',
'cdn_eval': False,
@@ -2380,7 +2359,6 @@ class TestVideoWithBumper(TestVideo): # pylint: disable=test-inherits-tests
expected_context = {
'autoadvance_enabled': False,
- 'branding_info': None,
'license': None,
'bumper_metadata': json.dumps(OrderedDict({
'saveStateUrl': self.block.ajax_url + '/save_user_state',
@@ -2480,7 +2458,6 @@ class TestAutoAdvanceVideo(TestVideo): # lint-amnesty, pylint: disable=test-inh
context = {
'autoadvance_enabled': autoadvanceenabled_flag,
- 'branding_info': None,
'block_id': str(self.block.location),
'course_id': str(self.block.location.course_key),
'license': None,
diff --git a/lms/djangoapps/grades/course_data.py b/lms/djangoapps/grades/course_data.py
index 5464c4f881..523d6e6df3 100644
--- a/lms/djangoapps/grades/course_data.py
+++ b/lms/djangoapps/grades/course_data.py
@@ -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,7 +56,11 @@ class CourseData:
@property
def structure(self): # lint-amnesty, pylint: disable=missing-function-docstring
if self._structure is None:
- self._structure = get_course_blocks(
+ # 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.user,
self.location,
collected_block_structure=self._collected_block_structure,
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 efb3f7d9fd..b7cdc6b039 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,78 @@ 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
"""
diff --git a/lms/templates/video.html b/lms/templates/video.html
index f2506e3613..66f369798c 100644
--- a/lms/templates/video.html
+++ b/lms/templates/video.html
@@ -140,12 +140,6 @@ from openedx.core.djangolib.js_utils import (
${_('Download Handout')}
% endif
- % if branding_info:
-
-
${branding_info['logo_tag']}
-
![${branding_info['logo_tag']}](${branding_info['logo_src']})
-