From ce2734e907e01120543349dd5b3e23fd69259215 Mon Sep 17 00:00:00 2001 From: Kristin Aoki <42981026+KristinAoki@users.noreply.github.com> Date: Wed, 22 Nov 2023 12:11:35 -0500 Subject: [PATCH] feat: update video and asset api to include usage loactions (#33746) --- .../contentstore/asset_storage_handlers.py | 78 ++++++++++++------- .../rest_api/v1/serializers/videos.py | 3 + .../contentstore/rest_api/v1/views/videos.py | 2 +- .../contentstore/video_storage_handlers.py | 15 ++-- cms/djangoapps/contentstore/views/assets.py | 4 +- .../contentstore/views/tests/test_assets.py | 4 +- .../contentstore/views/tests/test_videos.py | 9 ++- 7 files changed, 72 insertions(+), 43 deletions(-) diff --git a/cms/djangoapps/contentstore/asset_storage_handlers.py b/cms/djangoapps/contentstore/asset_storage_handlers.py index 82aef8d5ec76..1e4dbd61240b 100644 --- a/cms/djangoapps/contentstore/asset_storage_handlers.py +++ b/cms/djangoapps/contentstore/asset_storage_handlers.py @@ -91,7 +91,7 @@ def handle_assets(request, course_key_string=None, asset_key_string=None): return HttpResponseNotFound() -def get_asset_usage_path(request, course_key, asset_key_string): +def get_asset_usage_path_json(request, course_key, asset_key_string): """ Get a list of units with ancestors that use given asset. """ @@ -99,9 +99,16 @@ def get_asset_usage_path(request, course_key, asset_key_string): if not has_course_author_access(request.user, course_key): raise PermissionDenied() asset_location = AssetKey.from_string(asset_key_string) if asset_key_string else None + usage_locations = _get_asset_usage_path(course_key, [{'asset_key': asset_location}]) + return JsonResponse({'usage_locations': usage_locations}) + + +def _get_asset_usage_path(course_key, assets): + """ + Get a list of units with ancestors that use given asset. + """ store = modulestore() - usage_locations = [] - static_path = StaticContent.get_static_path_from_location(asset_location) + usage_locations = {str(asset['asset_key']): [] for asset in assets} verticals = store.get_items( course_key, qualifiers={ @@ -114,26 +121,34 @@ def get_asset_usage_path(request, course_key, asset_key_string): blocks.extend(vertical.get_children()) for block in blocks: - is_video_block = getattr(block, 'category', '') == 'video' - if is_video_block: - handout = getattr(block, 'handout', '') - if handout and str(asset_location) in handout: - unit = block.get_parent() - subsection = unit.get_parent() - subsection_display_name = getattr(subsection, 'display_name', '') - unit_display_name = getattr(unit, 'display_name', '') - xblock_display_name = getattr(block, 'display_name', '') - usage_locations.append(f'{subsection_display_name} - {unit_display_name} / {xblock_display_name}') - else: - data = getattr(block, 'data', '') - if static_path in data or str(asset_location) in data: - unit = block.get_parent() - subsection = unit.get_parent() - subsection_display_name = getattr(subsection, 'display_name', '') - unit_display_name = getattr(unit, 'display_name', '') - xblock_display_name = getattr(block, 'display_name', '') - usage_locations.append(f'{subsection_display_name} - {unit_display_name} / {xblock_display_name}') - return JsonResponse({'usage_locations': usage_locations}) + for asset in assets: + asset_key = asset['asset_key'] + asset_key_string = str(asset_key) + static_path = StaticContent.get_static_path_from_location(asset_key) + is_video_block = getattr(block, 'category', '') == 'video' + if is_video_block: + handout = getattr(block, 'handout', '') + if handout and asset_key_string in handout: + unit = block.get_parent() + subsection = unit.get_parent() + subsection_display_name = getattr(subsection, 'display_name', '') + unit_display_name = getattr(unit, 'display_name', '') + xblock_display_name = getattr(block, 'display_name', '') + current_locations = usage_locations[asset_key_string] + new_location = f'{subsection_display_name} - {unit_display_name} / {xblock_display_name}' + usage_locations[asset_key_string] = [*current_locations, new_location] + else: + data = getattr(block, 'data', '') + if static_path in data or asset_key_string in data: + unit = block.get_parent() + subsection = unit.get_parent() + subsection_display_name = getattr(subsection, 'display_name', '') + unit_display_name = getattr(unit, 'display_name', '') + xblock_display_name = getattr(block, 'display_name', '') + current_locations = usage_locations[asset_key_string] + new_location = f'{subsection_display_name} - {unit_display_name} / {xblock_display_name}' + usage_locations[asset_key_string] = [*current_locations, new_location] + return usage_locations def _asset_index(request, course_key): @@ -196,6 +211,8 @@ def _assets_json(request, course_key): assets, total_count = _get_assets_for_page(course_key, query_options) + assets_usage_locations_map = _get_asset_usage_path(course_key, assets) + if request_options['requested_page'] > 0 and first_asset_to_display_index >= total_count and total_count > 0: # lint-amnesty, pylint: disable=chained-comparison _update_options_to_requery_final_page(query_options, total_count) current_page = query_options['current_page'] @@ -203,7 +220,7 @@ def _assets_json(request, course_key): assets, total_count = _get_assets_for_page(course_key, query_options) last_asset_to_display_index = first_asset_to_display_index + len(assets) - assets_in_json_format = _get_assets_in_json_format(assets, course_key) + assets_in_json_format = _get_assets_in_json_format(assets, course_key, assets_usage_locations_map) response_payload = { 'start': first_asset_to_display_index, @@ -412,10 +429,13 @@ def _update_options_to_requery_final_page(query_options, total_asset_count): query_options['current_page'] = int(math.floor((total_asset_count - 1) / query_options['page_size'])) -def _get_assets_in_json_format(assets, course_key): +def _get_assets_in_json_format(assets, course_key, assets_usage_locations_map): """returns assets information in JSON Format""" assets_in_json_format = [] for asset in assets: + asset_key = asset['asset_key'] + asset_key_string = str(asset_key) + usage_locations = getattr(assets_usage_locations_map, 'asset_key_string', []) thumbnail_asset_key = _get_thumbnail_asset_key(asset, course_key) asset_is_locked = asset.get('locked', False) asset_file_size = asset.get('length', None) @@ -424,11 +444,12 @@ def _get_assets_in_json_format(assets, course_key): asset['displayname'], asset['contentType'], asset['uploadDate'], - asset['asset_key'], + asset_key, thumbnail_asset_key, asset_is_locked, course_key, asset_file_size, + usage_locations, ) assets_in_json_format.append(asset_in_json) @@ -670,13 +691,15 @@ def _delete_thumbnail(thumbnail_location, course_key, asset_key): # lint-amnest logging.warning('Could not delete thumbnail: %s', thumbnail_location) -def get_asset_json(display_name, content_type, date, location, thumbnail_location, locked, course_key, file_size=None): +def get_asset_json(display_name, content_type, date, location, thumbnail_location, + locked, course_key, file_size=None, usage=None): ''' Helper method for formatting the asset information to send to client. ''' asset_url = StaticContent.serialize_asset_key_with_slash(location) external_url = urljoin(configuration_helpers.get_value('LMS_ROOT_URL', settings.LMS_ROOT_URL), asset_url) portable_url = StaticContent.get_static_path_from_location(location) + usage_locations = [] if usage is None else usage return { 'display_name': display_name, 'content_type': content_type, @@ -690,4 +713,5 @@ def get_asset_json(display_name, content_type, date, location, thumbnail_locatio # needed for Backbone delete/update. 'id': str(location), 'file_size': file_size, + 'usage_locations': usage_locations, } diff --git a/cms/djangoapps/contentstore/rest_api/v1/serializers/videos.py b/cms/djangoapps/contentstore/rest_api/v1/serializers/videos.py index 89f566f0abfb..04f508eedfd7 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/serializers/videos.py +++ b/cms/djangoapps/contentstore/rest_api/v1/serializers/videos.py @@ -56,6 +56,9 @@ class VideoModelSerializer(serializers.Serializer): transcripts = serializers.ListField( child=serializers.CharField() ) + usage_locations = serializers.ListField( + child=serializers.CharField() + ) class VideoActiveTranscriptPreferencesSerializer(serializers.Serializer): diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/videos.py b/cms/djangoapps/contentstore/rest_api/v1/views/videos.py index b6d6dcfbae20..c5e7ae1bb263 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/videos.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/videos.py @@ -177,6 +177,6 @@ def get(self, request: Request, course_id: str, edx_video_id: str): if not has_studio_read_access(request.user, course_key): self.permission_denied(request) - usage_locations = get_video_usage_path(request, course_key, edx_video_id) + usage_locations = get_video_usage_path(course_key, edx_video_id) serializer = VideoUsageSerializer(usage_locations) return Response(serializer.data) diff --git a/cms/djangoapps/contentstore/video_storage_handlers.py b/cms/djangoapps/contentstore/video_storage_handlers.py index 318a91b4bb79..4d546aab3de5 100644 --- a/cms/djangoapps/contentstore/video_storage_handlers.py +++ b/cms/djangoapps/contentstore/video_storage_handlers.py @@ -15,7 +15,6 @@ from boto import s3 from django.conf import settings from django.contrib.staticfiles.storage import staticfiles_storage -from django.core.exceptions import PermissionDenied from django.http import FileResponse, HttpResponseNotFound from django.shortcuts import redirect from django.utils.translation import gettext as _ @@ -42,7 +41,6 @@ from rest_framework.response import Response from common.djangoapps.edxmako.shortcuts import render_to_response -from common.djangoapps.student.auth import has_course_author_access from common.djangoapps.util.json_request import JsonResponse from openedx.core.djangoapps.video_config.models import VideoTranscriptEnabledFlag from openedx.core.djangoapps.video_config.toggles import PUBLIC_VIDEO_SHARE @@ -223,13 +221,11 @@ def handle_videos(request, course_key_string, edx_video_id=None): return JsonResponse(data, status=status) -def get_video_usage_path(request, course_key, edx_video_id): +def get_video_usage_path(course_key, edx_video_id): """ API for fetching the locations a specific video is used in a course. Returns a list of paths to a video. """ - if not has_course_author_access(request.user, course_key): - raise PermissionDenied() store = modulestore() usage_locations = [] videos = store.get_items( @@ -616,15 +612,16 @@ def _get_index_videos(course, pagination_conf=None): 'transcript_urls', 'error_description' ] - def _get_values(video): + def _get_values(video, course): """ Get data for predefined video attributes. """ values = {} + values["usage_locations"] = get_video_usage_path(course.id, video["edx_video_id"])['usage_locations'] for attr in attrs: if attr == 'courses': - course = [c for c in video['courses'] if course_id in c] - (__, values['course_video_image_url']), = list(course[0].items()) + current_course = [c for c in video['courses'] if course_id in c] + (__, values['course_video_image_url']), = list(current_course[0].items()) elif attr == 'encoded_videos': values['download_link'] = '' values['file_size'] = 0 @@ -637,7 +634,7 @@ def _get_values(video): return values videos, pagination_context = _get_videos(course, pagination_conf) - return [_get_values(video) for video in videos], pagination_context + return [_get_values(video, course) for video in videos], pagination_context def get_all_transcript_languages(): diff --git a/cms/djangoapps/contentstore/views/assets.py b/cms/djangoapps/contentstore/views/assets.py index cca58e8c6b03..c9ae16a1402c 100644 --- a/cms/djangoapps/contentstore/views/assets.py +++ b/cms/djangoapps/contentstore/views/assets.py @@ -5,7 +5,7 @@ from django.views.decorators.csrf import ensure_csrf_cookie from cms.djangoapps.contentstore.asset_storage_handlers import ( handle_assets, - get_asset_usage_path, + get_asset_usage_path_json, update_course_run_asset as update_course_run_asset_source_function, get_file_size as get_file_size_source_function, delete_asset as delete_asset_source_function, @@ -57,7 +57,7 @@ def assets_handler(request, course_key_string=None, asset_key_string=None): @login_required @ensure_csrf_cookie def asset_usage_path_handler(request, course_key_string, asset_key_string): - return get_asset_usage_path(request, course_key_string, asset_key_string) + return get_asset_usage_path_json(request, course_key_string, asset_key_string) def update_course_run_asset(course_key, upload_file): diff --git a/cms/djangoapps/contentstore/views/tests/test_assets.py b/cms/djangoapps/contentstore/views/tests/test_assets.py index 914f660f61d3..a6dafab55c00 100644 --- a/cms/djangoapps/contentstore/views/tests/test_assets.py +++ b/cms/djangoapps/contentstore/views/tests/test_assets.py @@ -56,6 +56,7 @@ def upload_asset(self, name="asset-1", asset_type='text'): Post to the asset upload url """ asset = self.get_sample_asset(name, asset_type) + print(asset) response = self.client.post(self.url, {"name": name, "file": asset}) return response @@ -242,7 +243,8 @@ def test_mocked_filtered_response(self, mock_get_all_content_for_course): "thumbnail": None, "thumbnail_location": thumbnail_location, "locked": None, - "static_full_url": "/assets/courseware/v1/asset-v1:org+class+run+type@asset+block@my_file_name.jpg" + "static_full_url": "/assets/courseware/v1/asset-v1:org+class+run+type@asset+block@my_file_name.jpg", + "usage_locations": {str(asset_key): []} } ], 1 diff --git a/cms/djangoapps/contentstore/views/tests/test_videos.py b/cms/djangoapps/contentstore/views/tests/test_videos.py index 7d2f6e36e524..1eadd49c50a8 100644 --- a/cms/djangoapps/contentstore/views/tests/test_videos.py +++ b/cms/djangoapps/contentstore/views/tests/test_videos.py @@ -373,7 +373,8 @@ def test_get_json(self): 'transcripts', 'transcription_status', 'transcript_urls', - 'error_description' + 'error_description', + 'usage_locations' } ) dateutil.parser.parse(response_video['created']) @@ -389,7 +390,8 @@ def test_get_json(self): [ 'edx_video_id', 'client_video_id', 'created', 'duration', 'status', 'course_video_image_url', 'file_size', 'download_link', - 'transcripts', 'transcription_status', 'transcript_urls', 'error_description' + 'transcripts', 'transcription_status', 'transcript_urls', + 'error_description', 'usage_locations' ], [ { @@ -406,7 +408,8 @@ def test_get_json(self): [ 'edx_video_id', 'client_video_id', 'created', 'duration', 'status', 'course_video_image_url', 'file_size', 'download_link', - 'transcripts', 'transcription_status', 'transcript_urls', 'error_description' + 'transcripts', 'transcription_status', 'transcript_urls', + 'error_description', 'usage_locations' ], [ {