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

Merge branch 'master' into arslan/fix-validation-api

Browse Source
This commit is contained in:
Arslan Ashraf
2025-11-03 14:03:22 +05:00
committed by GitHub
parent 2179109bc2 7f0375b6b9
commit 4df43229bc
10 changed files with 173 additions and 47 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,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':

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

@@ -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

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

@@ -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)

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

@@ -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 = []

25
lms/djangoapps/courseware/tests/test_video_mongo.py
View File

@@ -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,

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,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,

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

@@ -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
"""

6
lms/templates/video.html
View File

@@ -140,12 +140,6 @@ from openedx.core.djangolib.js_utils import (
<a class="btn-link" href="${handout}">${_('Download Handout')}</a>
</div>
% endif
% if branding_info:
<div class="branding">
<span class="host-tag">${branding_info['logo_tag']}</span>
<a href="${branding_info['url']}"><img class="brand-logo" src="${branding_info['logo_src']}" alt="${branding_info['logo_tag']}" /></a>
</div>
% endif
</div>
% endif
% if transcript_feedback_enabled and video_id:

8
xmodule/static/css-builtin-blocks/VideoBlockDisplay.css
View File

@@ -78,7 +78,7 @@
.xmodule_display.xmodule_VideoBlock .video .wrapper-video-bottom-section .wrapper-download-video,
.xmodule_display.xmodule_VideoBlock .video .wrapper-video-bottom-section .wrapper-download-transcripts,
.xmodule_display.xmodule_VideoBlock .video .wrapper-video-bottom-section .wrapper-handouts,
.xmodule_display.xmodule_VideoBlock .video .wrapper-video-bottom-section .branding,
.xmodule_display.xmodule_VideoBlock .video .wrapper-video-bottom-section ,
.xmodule_display.xmodule_VideoBlock .video .wrapper-video-bottom-section .wrapper-transcript-feedback {
margin-top: var(--baseline, 20px);
padding-right: var(--baseline, 20px);
@@ -118,11 +118,11 @@
padding-left: 4px;
}
.xmodule_display.xmodule_VideoBlock .video .wrapper-downloads .branding {
.xmodule_display.xmodule_VideoBlock .video .wrapper-downloads {
padding-right: 0;
}
.xmodule_display.xmodule_VideoBlock .video .wrapper-downloads .branding .host-tag {
.xmodule_display.xmodule_VideoBlock .video .wrapper-downloads .host-tag {
position: absolute;
left: -9999em;
display: inline-block;
@@ -130,7 +130,7 @@
color: var(--body-color, #313131);
}
.xmodule_display.xmodule_VideoBlock .video .wrapper-downloads .branding .brand-logo {
.xmodule_display.xmodule_VideoBlock .video .wrapper-downloads .brand-logo {
display: inline-block;
max-width: 100%;
max-height: calc((var(--baseline, 20px) * 2));

8
xmodule/video_block/video_block.py
View File

@@ -103,11 +103,6 @@ try:
except ImportError:
edxval_api = None
try:
from lms.djangoapps.branding.models import BrandingInfoConfig
except ImportError:
BrandingInfoConfig = None
log = logging.getLogger(__name__)
# Make '_' a no-op so we can scrape strings. Using lambda instead of
@@ -294,7 +289,6 @@ class _BuiltInVideoBlock(
sources = [source for source in self.html5_sources if source]
download_video_link = None
branding_info = None
youtube_streams = ""
video_duration = None
video_status = None
@@ -358,7 +352,6 @@ class _BuiltInVideoBlock(
# Video caching is disabled for Studio. User_location is always None in Studio.
# CountryMiddleware disabled for Studio.
if getattr(self, 'video_speed_optimizations', True) and cdn_url:
branding_info = BrandingInfoConfig.get_config().get(user_location)
if self.edx_video_id and edxval_api and video_status != 'external':
for index, source_url in enumerate(sources):
@@ -477,7 +470,6 @@ class _BuiltInVideoBlock(
template_context = {
'autoadvance_enabled': autoadvance_enabled,
'branding_info': branding_info,
'bumper_metadata': json.dumps(self.bumper['metadata']), # pylint: disable=E1101
'cdn_eval': cdn_eval,
'cdn_exp_group': cdn_exp_group,