From 916c452c3a4a69a685d1ded5974f1c8de31327b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=86=D0=B2=D0=B0=D0=BD=20=D0=9D=D1=94=D0=B4=D1=94=D0=BB?= =?UTF-8?q?=D1=8C=D0=BD=D1=96=D1=86=D0=B5=D0=B2?= Date: Thu, 21 Nov 2024 12:49:46 +0200 Subject: [PATCH 01/16] feat: [AXM-542] create xblock renderer --- .../contentstore/signals/handlers.py | 11 ++ .../mobile_api/course_info/views.py | 26 ++- lms/envs/common.py | 1 + .../courseware/courseware-chromeless.html | 42 +++++ lms/urls.py | 4 + openedx/features/offline_mode/__init__.py | 0 openedx/features/offline_mode/apps.py | 14 ++ .../offline_mode/assets_management.py | 167 ++++++++++++++++++ openedx/features/offline_mode/constants.py | 13 ++ .../features/offline_mode/html_manipulator.py | 91 ++++++++++ openedx/features/offline_mode/renderer.py | 139 +++++++++++++++ openedx/features/offline_mode/tasks.py | 36 ++++ openedx/features/offline_mode/toggles.py | 23 +++ openedx/features/offline_mode/urls.py | 10 ++ openedx/features/offline_mode/utils.py | 97 ++++++++++ openedx/features/offline_mode/views.py | 45 +++++ uwsgi.ini | 0 17 files changed, 718 insertions(+), 1 deletion(-) create mode 100644 openedx/features/offline_mode/__init__.py create mode 100644 openedx/features/offline_mode/apps.py create mode 100644 openedx/features/offline_mode/assets_management.py create mode 100644 openedx/features/offline_mode/constants.py create mode 100644 openedx/features/offline_mode/html_manipulator.py create mode 100644 openedx/features/offline_mode/renderer.py create mode 100644 openedx/features/offline_mode/tasks.py create mode 100644 openedx/features/offline_mode/toggles.py create mode 100644 openedx/features/offline_mode/urls.py create mode 100644 openedx/features/offline_mode/utils.py create mode 100644 openedx/features/offline_mode/views.py create mode 100755 uwsgi.ini diff --git a/cms/djangoapps/contentstore/signals/handlers.py b/cms/djangoapps/contentstore/signals/handlers.py index d756424bccaa..08347dd9cd53 100644 --- a/cms/djangoapps/contentstore/signals/handlers.py +++ b/cms/djangoapps/contentstore/signals/handlers.py @@ -2,9 +2,11 @@ import logging +import requests from datetime import datetime, timezone from functools import wraps from typing import Optional +from urllib.parse import urljoin from django.conf import settings from django.core.cache import cache @@ -27,6 +29,7 @@ from openedx.core.djangoapps.content.learning_sequences.api import key_supports_outlines from openedx.core.djangoapps.discussions.tasks import update_discussions_settings_from_course_task from openedx.core.lib.gating import api as gating_api +from openedx.features.offline_mode.toggles import is_offline_mode_enabled from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import SignalHandler, modulestore from .signals import GRADING_POLICY_CHANGED @@ -34,6 +37,7 @@ log = logging.getLogger(__name__) GRADING_POLICY_COUNTDOWN_SECONDS = 3600 +LMS_OFFLINE_HANDLER_URL = '/offline_mode/handle_course_published' def locked(expiry_seconds, key): # lint-amnesty, pylint: disable=missing-function-docstring @@ -155,6 +159,13 @@ def listen_for_course_publish(sender, course_key, **kwargs): # pylint: disable= # Send to a signal for catalog info changes as well, but only once we know the transaction is committed. transaction.on_commit(lambda: emit_catalog_info_changed_signal(course_key)) + if is_offline_mode_enabled(course_key): + requests.post( + url=urljoin(settings.LMS_ROOT_URL, LMS_OFFLINE_HANDLER_URL), + data={'course_id': str(course_key)}, + ) + log.info('Sent course_published event to offline mode handler') + @receiver(SignalHandler.course_deleted) def listen_for_course_delete(sender, course_key, **kwargs): # pylint: disable=unused-argument diff --git a/lms/djangoapps/mobile_api/course_info/views.py b/lms/djangoapps/mobile_api/course_info/views.py index 0a173863db13..844fa4cf77ea 100644 --- a/lms/djangoapps/mobile_api/course_info/views.py +++ b/lms/djangoapps/mobile_api/course_info/views.py @@ -2,13 +2,16 @@ Views for course info API """ +import os import logging from typing import Dict, Optional, Union import django +from django.conf import settings from django.contrib.auth import get_user_model +from django.core.files.storage import default_storage from opaque_keys import InvalidKeyError -from opaque_keys.edx.keys import CourseKey +from opaque_keys.edx.keys import CourseKey, UsageKey from rest_framework import generics, status from rest_framework.response import Response from rest_framework.reverse import reverse @@ -31,6 +34,8 @@ from openedx.core.djangoapps.video_pipeline.config.waffle import DEPRECATE_YOUTUBE from openedx.core.lib.api.view_utils import view_auth_classes from openedx.core.lib.xblock_utils import get_course_update_items +from openedx.features.offline_mode.assets_management import get_offline_block_content_path +from openedx.features.offline_mode.toggles import is_offline_mode_enabled from openedx.features.course_experience import ENABLE_COURSE_GOALS from ..decorators import mobile_course_access, mobile_view @@ -352,6 +357,8 @@ def list(self, request, **kwargs): # pylint: disable=W0221 course_key, response.data['blocks'], ) + if api_version == 'v4' and is_offline_mode_enabled(course_key): + self._extend_block_info_with_offline_data(response.data['blocks']) course_info_context = { 'user': requested_user, @@ -410,6 +417,23 @@ def _extend_sequential_info_with_assignment_progress( } ) + @staticmethod + def _extend_block_info_with_offline_data(blocks_info_data: Dict[str, Dict]) -> None: + """ + Extends block info with offline download data. + If offline content is available for the block, adds the offline download data to the block info. + """ + for block_id, block_info in blocks_info_data.items(): + if offline_content_path := get_offline_block_content_path(usage_key=UsageKey.from_string(block_id)): + file_url = os.path.join(settings.MEDIA_URL, offline_content_path) + block_info.update({ + 'offline_download': { + 'file_url': file_url, + 'last_modified': default_storage.get_created_time(offline_content_path), + 'file_size': default_storage.size(offline_content_path) + } + }) + @mobile_view() class CourseEnrollmentDetailsView(APIView): diff --git a/lms/envs/common.py b/lms/envs/common.py index d74c28e75687..f73147417125 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -3330,6 +3330,7 @@ def _make_locale_paths(settings): # pylint: disable=missing-function-docstring 'openedx.features.discounts', 'openedx.features.effort_estimation', 'openedx.features.name_affirmation_api.apps.NameAffirmationApiConfig', + 'openedx.features.offline_mode', 'lms.djangoapps.experiments', diff --git a/lms/templates/courseware/courseware-chromeless.html b/lms/templates/courseware/courseware-chromeless.html index deeda26c431d..18a8f333efad 100644 --- a/lms/templates/courseware/courseware-chromeless.html +++ b/lms/templates/courseware/courseware-chromeless.html @@ -219,3 +219,45 @@ }()); % endif + +% if is_offline_content: + +% endif diff --git a/lms/urls.py b/lms/urls.py index f97bc7f0d04e..02513e8684a2 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -1053,3 +1053,7 @@ urlpatterns += [ path('api/notifications/', include('openedx.core.djangoapps.notifications.urls')), ] + +urlpatterns += [ + path('offline_mode/', include('openedx.features.offline_mode.urls')), +] diff --git a/openedx/features/offline_mode/__init__.py b/openedx/features/offline_mode/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/openedx/features/offline_mode/apps.py b/openedx/features/offline_mode/apps.py new file mode 100644 index 000000000000..c504af09dd11 --- /dev/null +++ b/openedx/features/offline_mode/apps.py @@ -0,0 +1,14 @@ +""" +OfflineMode application configuration +""" + + +from django.apps import AppConfig + + +class OfflineModeConfig(AppConfig): + """ + Application Configuration for Offline Mode module. + """ + + name = 'openedx.features.offline_mode' diff --git a/openedx/features/offline_mode/assets_management.py b/openedx/features/offline_mode/assets_management.py new file mode 100644 index 000000000000..073d52a6b0ab --- /dev/null +++ b/openedx/features/offline_mode/assets_management.py @@ -0,0 +1,167 @@ +""" +This module contains utility functions for managing assets and files. +""" +import shutil +import logging +import os +import requests + +from django.conf import settings +from django.core.files.storage import default_storage + +from xmodule.assetstore.assetmgr import AssetManager +from xmodule.contentstore.content import StaticContent +from xmodule.exceptions import NotFoundError +from xmodule.modulestore.exceptions import ItemNotFoundError + +from .constants import MATHJAX_CDN_URL, MATHJAX_STATIC_PATH + + +log = logging.getLogger(__name__) + + +def get_static_file_path(relative_path): + """ + Constructs the absolute path for a static file based on its relative path. + """ + base_path = settings.STATIC_ROOT + return os.path.join(base_path, relative_path) + + +def read_static_file(path): + """ + Reads the contents of a static file in binary mode. + """ + with open(path, 'rb') as file: + return file.read() + + +def save_asset_file(temp_dir, xblock, path, filename): + """ + Saves an asset file to the default storage. + If the filename contains a '/', it reads the static file directly from the file system. + Otherwise, it fetches the asset from the AssetManager. + Args: + temp_dir (str): The temporary directory where the assets are stored. + xblock (XBlock): The XBlock instance + path (str): The path where the asset is located. + filename (str): The name of the file to be saved. + """ + try: + if filename.startswith('assets/'): + asset_filename = filename.split('/')[-1] + asset_key = StaticContent.get_asset_key_from_path(xblock.location.course_key, asset_filename) + content = AssetManager.find(asset_key).data + file_path = os.path.join(temp_dir, filename) + else: + static_path = get_static_file_path(filename) + content = read_static_file(static_path) + file_path = os.path.join(temp_dir, 'assets', filename) + except (ItemNotFoundError, NotFoundError): + log.info(f"Asset not found: {filename}") + + else: + create_subdirectories_for_asset(file_path) + with open(file_path, 'wb') as file: + file.write(content) + + +def create_subdirectories_for_asset(file_path): + out_dir_name = '/' + for dir_name in file_path.split('/')[:-1]: + out_dir_name = os.path.join(out_dir_name, dir_name) + if out_dir_name and not os.path.exists(out_dir_name): + os.mkdir(out_dir_name) + + +def remove_old_files(xblock): + """ + Removes the 'assets' directory, 'index.html', and 'offline_content.zip' files + in the specified base path directory. + Args: + (XBlock): The XBlock instance + """ + try: + base_path = block_storage_path(xblock) + assets_path = os.path.join(base_path, 'assets') + index_file_path = os.path.join(base_path, 'index.html') + offline_zip_path = os.path.join(base_path, f'{xblock.location.block_id}.zip') + + # Delete the 'assets' directory if it exists + if os.path.isdir(assets_path): + shutil.rmtree(assets_path) + log.info(f"Successfully deleted the directory: {assets_path}") + + # Delete the 'index.html' file if it exists + if default_storage.exists(index_file_path): + os.remove(index_file_path) + log.info(f"Successfully deleted the file: {index_file_path}") + + # Delete the 'offline_content.zip' file if it exists + if default_storage.exists(offline_zip_path): + os.remove(offline_zip_path) + log.info(f"Successfully deleted the file: {offline_zip_path}") + + except OSError as e: + log.error(f"Error occurred while deleting the files or directory: {e}") + + +def get_offline_block_content_path(xblock=None, usage_key=None): + """ + Checks whether 'offline_content.zip' file is present in the specified base path directory. + Args: + xblock (XBlock): The XBlock instance + usage_key (UsageKey): The UsageKey of the XBlock + Returns: + bool: True if the file is present, False otherwise + """ + usage_key = usage_key or getattr(xblock, 'location', None) + base_path = block_storage_path(usage_key=usage_key) + offline_zip_path = os.path.join(base_path, f'{usage_key.block_id}.zip') + return offline_zip_path if default_storage.exists(offline_zip_path) else None + + +def block_storage_path(xblock=None, usage_key=None): + """ + Generates the base storage path for the given XBlock. + The path is constructed based on the XBlock's location, which includes the organization, + course, block type, and block ID. + Args: + xblock (XBlock): The XBlock instance for which to generate the storage path. + usage_key (UsageKey): The UsageKey of the XBlock. + Returns: + str: The constructed base storage path. + """ + loc = usage_key or getattr(xblock, 'location', None) + return f'{str(loc.course_key)}/' if loc else '' + + +def is_modified(xblock): + """ + Check if the xblock has been modified since the last time the offline content was generated. + Args: + xblock (XBlock): The XBlock instance to check. + """ + file_path = os.path.join(block_storage_path(xblock), f'{xblock.location.block_id}.zip') + + try: + last_modified = default_storage.get_created_time(file_path) + except OSError: + return True + + return xblock.published_on > last_modified + + +def save_mathjax_to_xblock_assets(temp_dir): + """ + Saves MathJax to the local static directory. + If MathJax is not already saved, it fetches MathJax from + the CDN and saves it to the local static directory. + """ + file_path = os.path.join(temp_dir, MATHJAX_STATIC_PATH) + if not os.path.exists(file_path): + response = requests.get(MATHJAX_CDN_URL) + with open(file_path, 'wb') as file: + file.write(response.content) + + log.info(f"Successfully saved MathJax to {file_path}") diff --git a/openedx/features/offline_mode/constants.py b/openedx/features/offline_mode/constants.py new file mode 100644 index 000000000000..401d84d0a271 --- /dev/null +++ b/openedx/features/offline_mode/constants.py @@ -0,0 +1,13 @@ +""" +Constants for offline mode app. +""" +import os + +from django.conf import settings + +MATHJAX_VERSION = '2.7.5' +MATHJAX_CDN_URL = f'https://cdn.jsdelivr.net/npm/mathjax@{MATHJAX_VERSION}/MathJax.js' +MATHJAX_STATIC_PATH = os.path.join('assets', 'js', f'MathJax-{MATHJAX_VERSION}.js') + +DEFAULT_OFFLINE_SUPPORTED_XBLOCKS = ['html'] +OFFLINE_SUPPORTED_XBLOCKS = getattr(settings, 'OFFLINE_SUPPORTED_XBLOCKS', DEFAULT_OFFLINE_SUPPORTED_XBLOCKS) diff --git a/openedx/features/offline_mode/html_manipulator.py b/openedx/features/offline_mode/html_manipulator.py new file mode 100644 index 000000000000..9e5bff7ab396 --- /dev/null +++ b/openedx/features/offline_mode/html_manipulator.py @@ -0,0 +1,91 @@ +""" +Module to prepare HTML content for offline use. +""" +import os +import re + +from bs4 import BeautifulSoup + +from django.conf import settings + +from .assets_management import save_asset_file, save_mathjax_to_xblock_assets +from .constants import MATHJAX_CDN_URL, MATHJAX_STATIC_PATH + + +class HtmlManipulator: + """ + Class to prepare HTML content for offline use. + Changes links to static files to paths to pre-generated static files for offline use. + """ + + def __init__(self, xblock, html_data, temp_dir): + self.html_data = html_data + self.xblock = xblock + self.temp_dir = temp_dir + + def process_html(self): + """ + Prepares HTML content for local usage. + Changes links to static files to paths to pre-generated static files for offline use. + """ + self._replace_asset_links() + self._replace_static_links() + self._replace_mathjax_link() + + soup = BeautifulSoup(self.html_data, 'html.parser') + self._replace_iframe(soup) + return str(soup) + + def _replace_mathjax_link(self): + """ + Replace MathJax CDN link with local path to MathJax.js file. + """ + save_mathjax_to_xblock_assets(self.temp_dir) + mathjax_pattern = re.compile(fr'src="{MATHJAX_CDN_URL}[^"]*"') + self.html_data = mathjax_pattern.sub(f'src="{MATHJAX_STATIC_PATH}"', self.html_data) + + def _replace_static_links(self): + """ + Replace static links with local links. + """ + static_links_pattern = os.path.join(settings.STATIC_URL, r'[\w./-]+') + pattern = re.compile(fr'{static_links_pattern}') + self.html_data = pattern.sub(self._replace_link, self.html_data) + + def _replace_asset_links(self): + """ + Replace static links with local links. + """ + pattern = re.compile(r'/assets/[\w./@:+-]+') + self.html_data = pattern.sub(self._replace_asset_link, self.html_data) + + def _replace_asset_link(self, match): + """ + Returns the local path of the asset file. + """ + link = match.group() + filename = link[1:] if link.startswith('/') else link # Remove the leading '/' + save_asset_file(self.temp_dir, self.xblock, link, filename) + return filename + + def _replace_link(self, match): + """ + Returns the local path of the asset file. + """ + link = match.group() + filename = link.split(settings.STATIC_URL)[-1] + save_asset_file(self.temp_dir, self.xblock, link, filename) + return f'assets/{filename}' + + @staticmethod + def _replace_iframe(soup): + """ + Replace iframe tags with anchor tags. + """ + for node in soup.find_all('iframe'): + replacement = soup.new_tag('p') + tag_a = soup.new_tag('a') + tag_a['href'] = node.get('src') + tag_a.string = node.get('title', node.get('src')) + replacement.append(tag_a) + node.replace_with(replacement) diff --git a/openedx/features/offline_mode/renderer.py b/openedx/features/offline_mode/renderer.py new file mode 100644 index 000000000000..9a39bfee76c9 --- /dev/null +++ b/openedx/features/offline_mode/renderer.py @@ -0,0 +1,139 @@ +""" +This module contains the XBlockRenderer class, +which is responsible for rendering an XBlock HTML content from the LMS. +""" +import logging + +from django.conf import settings +from django.contrib.auth import get_user_model +from django.contrib.sessions.backends.db import SessionStore +from django.http import HttpRequest + +from opaque_keys.edx.keys import UsageKey +from xmodule.modulestore.django import modulestore + +from common.djangoapps.edxmako.shortcuts import render_to_string +from lms.djangoapps.courseware.block_render import get_block_by_usage_id +from lms.djangoapps.courseware.views.views import get_optimization_flags_for_content + +from openedx.core.lib.courses import get_course_by_id +from openedx.features.course_experience.utils import dates_banner_should_display +from openedx.features.course_experience.url_helpers import get_learning_mfe_home_url + +User = get_user_model() +log = logging.getLogger(__name__) + + +class XBlockRenderer: + """ + Renders an XBlock HTML content from the LMS. + Since imports from LMS are used here, XBlockRenderer can be called only in the LMS runtime. + :param usage_key_string: The string representation of the block UsageKey. + :param user: The user for whom the XBlock will be rendered. + """ + + SERVICE_USERNAME = 'offline_mode_worker' + + def __init__(self, usage_key_string, user=None, request=None): + self.usage_key = UsageKey.from_string(usage_key_string) + self.usage_key = self.usage_key.replace(course_key=modulestore().fill_in_run(self.usage_key.course_key)) + self.user = user or self.service_user + self.request = request or self.generate_request() + + @property + def service_user(self): + """ + Returns a valid user to be used as the service user. + """ + try: + return User.objects.get(username=self.SERVICE_USERNAME) + except User.DoesNotExist as e: + log.error(f'Service user with username {self.SERVICE_USERNAME} to render XBlock does not exist.') + raise e + + def generate_request(self): + """ + Generates a request object with the service user and a session. + """ + request = HttpRequest() + request.user = self.user + session = SessionStore() + session.create() + request.session = session + return request + + def render_xblock_from_lms(self): + """ + Returns a string representation of the HTML content of the XBlock as it appears in the LMS. + Blocks renders without header, footer and navigation. + Blocks view like a for regular user without staff or superuser access. + """ + course_key = self.usage_key.course_key + + with modulestore().bulk_operations(course_key): + course = get_course_by_id(course_key) + block, _ = get_block_by_usage_id( + self.request, + str(course_key), + str(self.usage_key), + disable_staff_debug_info=True, + course=course, + will_recheck_access='1', + ) + + enable_completion_on_view_service = False + wrap_xblock_data = None + completion_service = block.runtime.service(block, 'completion') + if completion_service and completion_service.completion_tracking_enabled(): + if completion_service.blocks_to_mark_complete_on_view({block}): + enable_completion_on_view_service = True + wrap_xblock_data = { + 'mark-completed-on-view-after-delay': completion_service.get_complete_on_view_delay_ms() + } + + fragment = self.get_fragment(block, wrap_xblock_data) + optimization_flags = get_optimization_flags_for_content(block, fragment) + missed_deadlines, missed_gated_content = dates_banner_should_display(course_key, self.user) + + context = { + 'fragment': fragment, + 'course': course, + 'block': block, + 'enable_completion_on_view_service': enable_completion_on_view_service, + 'xqa_server': settings.FEATURES.get('XQA_SERVER', 'http://your_xqa_server.com'), + 'missed_deadlines': missed_deadlines, + 'missed_gated_content': missed_gated_content, + 'has_ended': course.has_ended(), + 'web_app_course_url': get_learning_mfe_home_url(course_key=course.id, url_fragment='home'), + 'disable_accordion': True, + 'allow_iframing': True, + 'disable_header': True, + 'disable_footer': True, + 'disable_window_wrap': True, + 'edx_notes_enabled': False, + 'staff_access': False, + 'on_courseware_page': True, + 'is_learning_mfe': False, + 'is_mobile_app': True, + 'is_offline_content': True, + 'render_course_wide_assets': True, + 'LANGUAGE_CODE': 'en', + + **optimization_flags, + } + return render_to_string('courseware/courseware-chromeless.html', context, namespace='main') + + @staticmethod + def get_fragment(block, wrap_xblock_data=None): + """ + Returns the HTML fragment of the XBlock. + """ + student_view_context = { + 'show_bookmark_button': '0', + 'show_title': '1', + 'hide_access_error_blocks': True, + 'is_mobile_app': True, + } + if wrap_xblock_data: + student_view_context['wrap_xblock_data'] = wrap_xblock_data + return block.render('student_view', context=student_view_context) diff --git a/openedx/features/offline_mode/tasks.py b/openedx/features/offline_mode/tasks.py new file mode 100644 index 000000000000..75e786f43ca4 --- /dev/null +++ b/openedx/features/offline_mode/tasks.py @@ -0,0 +1,36 @@ +""" +Tasks for offline mode feature. +""" +from celery import shared_task +from edx_django_utils.monitoring import set_code_owner_attribute +from opaque_keys.edx.keys import CourseKey, UsageKey + +from xmodule.modulestore.django import modulestore + +from .constants import OFFLINE_SUPPORTED_XBLOCKS +from .renderer import XBlockRenderer +from .utils import generate_offline_content + + +@shared_task +@set_code_owner_attribute +def generate_offline_content_for_course(course_id): + """ + Generates offline content for all supported XBlocks in the course. + """ + course_key = CourseKey.from_string(course_id) + for offline_supported_block_type in OFFLINE_SUPPORTED_XBLOCKS: + for xblock in modulestore().get_items(course_key, qualifiers={'category': offline_supported_block_type}): + html_data = XBlockRenderer(str(xblock.location)).render_xblock_from_lms() + generate_offline_content_for_block.apply_async([str(xblock.location), html_data]) + + +@shared_task +@set_code_owner_attribute +def generate_offline_content_for_block(block_id, html_data): + """ + Generates offline content for the specified block. + """ + block_usage_key = UsageKey.from_string(block_id) + xblock = modulestore().get_item(block_usage_key) + generate_offline_content(xblock, html_data) diff --git a/openedx/features/offline_mode/toggles.py b/openedx/features/offline_mode/toggles.py new file mode 100644 index 000000000000..e76c5ce56803 --- /dev/null +++ b/openedx/features/offline_mode/toggles.py @@ -0,0 +1,23 @@ +""" +Feature toggles for the offline mode app. +""" +from openedx.core.djangoapps.waffle_utils import CourseWaffleFlag + +WAFFLE_FLAG_NAMESPACE = 'offline_mode' + +# .. toggle_name: offline_mode.enable_offline_mode +# .. toggle_implementation: CourseWaffleFlag +# .. toggle_default: False +# .. toggle_description: This feature toggle enables the offline mode course +# content generation for mobile devices. +# .. toggle_use_cases: opt_out, open_edx +# .. toggle_creation_date: 2024-06-06 +# .. toggle_target_removal_date: None +ENABLE_OFFLINE_MODE = CourseWaffleFlag(f'{WAFFLE_FLAG_NAMESPACE}.enable_offline_mode', __name__) + + +def is_offline_mode_enabled(course_key=None): + """ + Returns True if the offline mode is enabled for the course, False otherwise. + """ + return ENABLE_OFFLINE_MODE.is_enabled(course_key) diff --git a/openedx/features/offline_mode/urls.py b/openedx/features/offline_mode/urls.py new file mode 100644 index 000000000000..f5178a424316 --- /dev/null +++ b/openedx/features/offline_mode/urls.py @@ -0,0 +1,10 @@ +""" +URLs for the offline_mode feature. +""" +from django.urls import path + +from .views import SudioCoursePublishedEventHandler + +urlpatterns = [ + path('handle_course_published', SudioCoursePublishedEventHandler.as_view(), name='handle_course_published'), +] diff --git a/openedx/features/offline_mode/utils.py b/openedx/features/offline_mode/utils.py new file mode 100644 index 000000000000..6a0e561b656a --- /dev/null +++ b/openedx/features/offline_mode/utils.py @@ -0,0 +1,97 @@ +""" +Utility functions and classes for offline mode. +""" +import os +import logging +import shutil +from tempfile import mkdtemp + +from django.contrib.auth import get_user_model +from django.core.files.storage import default_storage + +from zipfile import ZipFile + +from .assets_management import block_storage_path, remove_old_files, is_modified +from .html_manipulator import HtmlManipulator + +User = get_user_model() +log = logging.getLogger(__name__) + + +def generate_offline_content(xblock, html_data): + """ + Generates archive with XBlock content for offline mode. + Args: + xblock (XBlock): The XBlock instance + html_data (str): The rendered HTML representation of the XBlock + """ + if not is_modified(xblock): + return + + base_path = block_storage_path(xblock) + remove_old_files(xblock) + tmp_dir = mkdtemp() + + try: + save_xblock_html(tmp_dir, xblock, html_data) + create_zip_file(tmp_dir, base_path, f'{xblock.location.block_id}.zip') + finally: + shutil.rmtree(tmp_dir, ignore_errors=True) + + +def save_xblock_html(tmp_dir, xblock, html_data): + """ + Saves the XBlock HTML content to a file. + Generates the 'index.html' file with the HTML added to use it locally. + Args: + tmp_dir (str): The temporary directory path to save the xblock content + xblock (XBlock): The XBlock instance + html_data (str): The rendered HTML representation of the XBlock + """ + html_manipulator = HtmlManipulator(xblock, html_data, tmp_dir) + updated_html = html_manipulator.process_html() + + with open(os.path.join(tmp_dir, 'index.html'), 'w') as file: + file.write(updated_html) + + +def create_zip_file(temp_dir, base_path, file_name): + """ + Creates a zip file with the content of the base_path directory. + Args: + temp_dir (str): The temporary directory path where the content is stored + base_path (str): The base path directory to save the zip file + file_name (str): The name of the zip file + """ + if not os.path.exists(default_storage.path(base_path)): + os.makedirs(default_storage.path(base_path)) + + with ZipFile(default_storage.path(base_path + file_name), 'w') as zip_file: + zip_file.write(os.path.join(temp_dir, 'index.html'), 'index.html') + add_files_to_zip_recursively( + zip_file, + current_base_path=os.path.join(temp_dir, 'assets'), + current_path_in_zip='assets', + ) + log.info(f'Offline content for {file_name} has been generated.') + + +def add_files_to_zip_recursively(zip_file, current_base_path, current_path_in_zip): + """ + Recursively adds files to the zip file. + Args: + zip_file (ZipFile): The zip file object + current_base_path (str): The current base path directory + current_path_in_zip (str): The current path in the zip file + """ + try: + for resource_path in os.listdir(current_base_path): + full_path = os.path.join(current_base_path, resource_path) + full_path_in_zip = os.path.join(current_path_in_zip, resource_path) + if os.path.isfile(full_path): + zip_file.write(full_path, full_path_in_zip) + else: + add_files_to_zip_recursively(zip_file, full_path, full_path_in_zip) + except OSError: + log.error(f'Error while reading the directory: {current_base_path}') + return diff --git a/openedx/features/offline_mode/views.py b/openedx/features/offline_mode/views.py new file mode 100644 index 000000000000..ad3412a636b6 --- /dev/null +++ b/openedx/features/offline_mode/views.py @@ -0,0 +1,45 @@ +""" +Views for the offline_mode app. +""" +from opaque_keys.edx.keys import CourseKey +from rest_framework import status +from rest_framework.response import Response +from rest_framework.views import APIView + +from .tasks import generate_offline_content_for_course +from .toggles import is_offline_mode_enabled + + +class SudioCoursePublishedEventHandler(APIView): + """ + Handle the event of a course being published in Studio. + This view is called by Studio when a course is published, + and it triggers the generation of offline content. + """ + + def post(self, request, *args, **kwargs): + """ + Trigger the generation of offline content task. + Args: + request (Request): The incoming request object. + args: Additional positional arguments. + kwargs: Additional keyword arguments. + Returns: + Response: The response object. + """ + + course_id = request.data.get('course_id') + if not course_id: + return Response( + data={'error': 'course_id is required'}, + status=status.HTTP_400_BAD_REQUEST + ) + course_key = CourseKey.from_string(course_id) + if is_offline_mode_enabled(course_key): + generate_offline_content_for_course.apply_async(args=[course_id]) + return Response(status=status.HTTP_200_OK) + else: + return Response( + data={'error': 'Offline mode is not enabled for this course'}, + status=status.HTTP_400_BAD_REQUEST, + ) diff --git a/uwsgi.ini b/uwsgi.ini new file mode 100755 index 000000000000..e69de29bb2d1 From 95dbc8e7b74858499d0e83d8dcd1a4535992aaf9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=86=D0=B2=D0=B0=D0=BD=20=D0=9D=D1=94=D0=B4=D1=94=D0=BB?= =?UTF-8?q?=D1=8C=D0=BD=D1=96=D1=86=D0=B5=D0=B2?= Date: Thu, 21 Nov 2024 12:51:01 +0200 Subject: [PATCH 02/16] feat: [AXM-589] Add retry to content generation task --- openedx/features/offline_mode/assets_management.py | 3 +++ openedx/features/offline_mode/constants.py | 3 +++ openedx/features/offline_mode/tasks.py | 9 ++++++--- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/openedx/features/offline_mode/assets_management.py b/openedx/features/offline_mode/assets_management.py index 073d52a6b0ab..739b7d049477 100644 --- a/openedx/features/offline_mode/assets_management.py +++ b/openedx/features/offline_mode/assets_management.py @@ -67,6 +67,9 @@ def save_asset_file(temp_dir, xblock, path, filename): def create_subdirectories_for_asset(file_path): + """ + Creates subdirectories for asset. + """ out_dir_name = '/' for dir_name in file_path.split('/')[:-1]: out_dir_name = os.path.join(out_dir_name, dir_name) diff --git a/openedx/features/offline_mode/constants.py b/openedx/features/offline_mode/constants.py index 401d84d0a271..a1337a7029c1 100644 --- a/openedx/features/offline_mode/constants.py +++ b/openedx/features/offline_mode/constants.py @@ -11,3 +11,6 @@ DEFAULT_OFFLINE_SUPPORTED_XBLOCKS = ['html'] OFFLINE_SUPPORTED_XBLOCKS = getattr(settings, 'OFFLINE_SUPPORTED_XBLOCKS', DEFAULT_OFFLINE_SUPPORTED_XBLOCKS) + +RETRY_BACKOFF_INITIAL_TIMEOUT = 60 # one minute +MAX_RETRY_ATTEMPTS = 5 diff --git a/openedx/features/offline_mode/tasks.py b/openedx/features/offline_mode/tasks.py index 75e786f43ca4..98c62b5add6f 100644 --- a/openedx/features/offline_mode/tasks.py +++ b/openedx/features/offline_mode/tasks.py @@ -7,7 +7,7 @@ from xmodule.modulestore.django import modulestore -from .constants import OFFLINE_SUPPORTED_XBLOCKS +from .constants import MAX_RETRY_ATTEMPTS, OFFLINE_SUPPORTED_XBLOCKS, RETRY_BACKOFF_INITIAL_TIMEOUT from .renderer import XBlockRenderer from .utils import generate_offline_content @@ -25,8 +25,11 @@ def generate_offline_content_for_course(course_id): generate_offline_content_for_block.apply_async([str(xblock.location), html_data]) -@shared_task -@set_code_owner_attribute +@shared_task( + autoretry_for=(Exception,), + retry_backoff=RETRY_BACKOFF_INITIAL_TIMEOUT, + retry_kwargs={'max_retries': MAX_RETRY_ATTEMPTS} +)@set_code_owner_attribute def generate_offline_content_for_block(block_id, html_data): """ Generates offline content for the specified block. From 8df43129fc2c1eeb67f64988d9af380eb584bf63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=86=D0=B2=D0=B0=D0=BD=20=D0=9D=D1=94=D0=B4=D1=94=D0=BB?= =?UTF-8?q?=D1=8C=D0=BD=D1=96=D1=86=D0=B5=D0=B2?= Date: Thu, 21 Nov 2024 12:57:44 +0200 Subject: [PATCH 03/16] feat: [AXM-644] Add authorization via cms worker for content generation view --- .../contentstore/signals/handlers.py | 5 +- cms/djangoapps/contentstore/utils.py | 17 ++++ cms/envs/common.py | 2 + cms/envs/production.py | 1 + lms/urls.py | 2 +- .../offline_mode/assets_management.py | 2 +- openedx/features/offline_mode/constants.py | 2 +- .../features/offline_mode/tests/__init__.py | 0 .../features/offline_mode/tests/test_views.py | 83 +++++++++++++++++++ openedx/features/offline_mode/urls.py | 1 + openedx/features/offline_mode/views.py | 19 ++++- 11 files changed, 127 insertions(+), 7 deletions(-) create mode 100644 openedx/features/offline_mode/tests/__init__.py create mode 100644 openedx/features/offline_mode/tests/test_views.py diff --git a/cms/djangoapps/contentstore/signals/handlers.py b/cms/djangoapps/contentstore/signals/handlers.py index 08347dd9cd53..d04a28a1d71d 100644 --- a/cms/djangoapps/contentstore/signals/handlers.py +++ b/cms/djangoapps/contentstore/signals/handlers.py @@ -2,7 +2,6 @@ import logging -import requests from datetime import datetime, timezone from functools import wraps from typing import Optional @@ -23,6 +22,7 @@ CoursewareSearchIndexer, LibrarySearchIndexer, ) +from cms.djangoapps.contentstore.utils import get_cms_api_client from common.djangoapps.track.event_transaction_utils import get_event_transaction_id, get_event_transaction_type from common.djangoapps.util.block_utils import yield_dynamic_block_descendants from lms.djangoapps.grades.api import task_compute_all_grades_for_course @@ -160,7 +160,8 @@ def listen_for_course_publish(sender, course_key, **kwargs): # pylint: disable= transaction.on_commit(lambda: emit_catalog_info_changed_signal(course_key)) if is_offline_mode_enabled(course_key): - requests.post( + client = get_cms_api_client() + client.post( url=urljoin(settings.LMS_ROOT_URL, LMS_OFFLINE_HANDLER_URL), data={'course_id': str(course_key)}, ) diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index c0f656ec7059..ff7dfb337653 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -6,6 +6,7 @@ import html import logging import re +import requests from collections import defaultdict from contextlib import contextmanager from datetime import datetime, timezone @@ -14,8 +15,10 @@ from bs4 import BeautifulSoup from django.conf import settings +from django.contrib.auth import get_user_model from django.core.exceptions import ValidationError from django.urls import reverse +from edx_rest_api_client.auth import SuppliedJwtAuth from django.utils import translation from django.utils.text import Truncator from django.utils.translation import gettext as _ @@ -96,6 +99,7 @@ from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from openedx.core.djangoapps.site_configuration.models import SiteConfiguration from openedx.core.djangoapps.models.course_details import CourseDetails +from openedx.core.djangoapps.oauth_dispatch.jwt import create_jwt_for_user from openedx.core.lib.courses import course_image_url from openedx.core.lib.html_to_text import html_to_text from openedx.features.content_type_gating.models import ContentTypeGatingConfig @@ -113,6 +117,7 @@ IMPORTABLE_FILE_TYPES = ('.tar.gz', '.zip') log = logging.getLogger(__name__) +User = get_user_model() def add_instructor(course_key, requesting_user, new_instructor): @@ -2344,3 +2349,15 @@ def get_xblock_render_context(request, block): return str(exc) return "" + + +def get_cms_api_client(): + """ + Returns an API client which can be used to make requests from the CMS service. + """ + user = User.objects.get(username=settings.EDXAPP_CMS_SERVICE_USER_NAME) + jwt = create_jwt_for_user(user) + client = requests.Session() + client.auth = SuppliedJwtAuth(jwt) + + return client diff --git a/cms/envs/common.py b/cms/envs/common.py index 00a384a359c6..c6eda1c1fffa 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -2523,6 +2523,8 @@ EXAMS_SERVICE_URL = 'http://localhost:18740/api/v1' EXAMS_SERVICE_USERNAME = 'edx_exams_worker' +CMS_SERVICE_USER_NAME = 'edxapp_cms_worker' + FINANCIAL_REPORTS = { 'STORAGE_TYPE': 'localfs', 'BUCKET': None, diff --git a/cms/envs/production.py b/cms/envs/production.py index ad7667772f9a..1b1e8fc6f876 100644 --- a/cms/envs/production.py +++ b/cms/envs/production.py @@ -168,6 +168,7 @@ def get_env_setting(setting): ENTERPRISE_CONSENT_API_URL = ENV_TOKENS.get('ENTERPRISE_CONSENT_API_URL', LMS_INTERNAL_ROOT_URL + '/consent/api/v1/') AUTHORING_API_URL = ENV_TOKENS.get('AUTHORING_API_URL', '') # Note that FEATURES['PREVIEW_LMS_BASE'] gets read in from the environment file. +CMS_SERVICE_USER_NAME = ENV_TOKENS.get('CMS_SERVICE_USER_NAME', CMS_SERVICE_USER_NAME) CHAT_COMPLETION_API = ENV_TOKENS.get('CHAT_COMPLETION_API', '') CHAT_COMPLETION_API_KEY = ENV_TOKENS.get('CHAT_COMPLETION_API_KEY', '') diff --git a/lms/urls.py b/lms/urls.py index 02513e8684a2..acffdd86e53e 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -1055,5 +1055,5 @@ ] urlpatterns += [ - path('offline_mode/', include('openedx.features.offline_mode.urls')), + path('offline_mode/', include('openedx.features.offline_mode.urls', namespace='offline_mode')), ] diff --git a/openedx/features/offline_mode/assets_management.py b/openedx/features/offline_mode/assets_management.py index 739b7d049477..c1753ca686a3 100644 --- a/openedx/features/offline_mode/assets_management.py +++ b/openedx/features/offline_mode/assets_management.py @@ -68,7 +68,7 @@ def save_asset_file(temp_dir, xblock, path, filename): def create_subdirectories_for_asset(file_path): """ - Creates subdirectories for asset. + Creates the subdirectories for the asset file path if they do not exist. """ out_dir_name = '/' for dir_name in file_path.split('/')[:-1]: diff --git a/openedx/features/offline_mode/constants.py b/openedx/features/offline_mode/constants.py index a1337a7029c1..771b341505aa 100644 --- a/openedx/features/offline_mode/constants.py +++ b/openedx/features/offline_mode/constants.py @@ -9,7 +9,7 @@ MATHJAX_CDN_URL = f'https://cdn.jsdelivr.net/npm/mathjax@{MATHJAX_VERSION}/MathJax.js' MATHJAX_STATIC_PATH = os.path.join('assets', 'js', f'MathJax-{MATHJAX_VERSION}.js') -DEFAULT_OFFLINE_SUPPORTED_XBLOCKS = ['html'] +DEFAULT_OFFLINE_SUPPORTED_XBLOCKS = ['html', 'problem'] OFFLINE_SUPPORTED_XBLOCKS = getattr(settings, 'OFFLINE_SUPPORTED_XBLOCKS', DEFAULT_OFFLINE_SUPPORTED_XBLOCKS) RETRY_BACKOFF_INITIAL_TIMEOUT = 60 # one minute diff --git a/openedx/features/offline_mode/tests/__init__.py b/openedx/features/offline_mode/tests/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/openedx/features/offline_mode/tests/test_views.py b/openedx/features/offline_mode/tests/test_views.py new file mode 100644 index 000000000000..a4478d8796ab --- /dev/null +++ b/openedx/features/offline_mode/tests/test_views.py @@ -0,0 +1,83 @@ +""" +Tests for view that handles course published event. +""" +from unittest.mock import patch + +from django.test import TestCase, RequestFactory +from django.test.client import Client +from django.urls import reverse +from edx_toggles.toggles.testutils import override_waffle_flag + +from common.djangoapps.student.tests.factories import UserFactory +from openedx.features.offline_mode.views import SudioCoursePublishedEventHandler + +from openedx.features.offline_mode.toggles import ENABLE_OFFLINE_MODE + + +class TestSudioCoursePublishedEventHandler(TestCase): + """ + Tests for the SudioCoursePublishedEventHandler view. + """ + + def setUp(self): + self.client = Client() + self.factory = RequestFactory() + self.view = SudioCoursePublishedEventHandler.as_view() + self.url = reverse('offline_mode:handle_course_published') + + self.user_password = 'Password1234' + self.user = UserFactory.create(password=self.user_password) + self.staff_user = UserFactory.create(is_staff=True, password=self.user_password) + + def staff_login(self): + self.client.login(username=self.staff_user.username, password=self.user_password) + + def test_unauthorized(self): + response = self.client.post(self.url, {}) + self.assertEqual(response.status_code, 401) + self.assertEqual(response.data, {'detail': 'Authentication credentials were not provided.'}) + + def test_not_admin(self): + self.client.login(username=self.user.username, password=self.user_password) + response = self.client.post(self.url, {}) + self.assertEqual(response.status_code, 403) + self.assertEqual(response.data, {'detail': 'You do not have permission to perform this action.'}) + + @override_waffle_flag(ENABLE_OFFLINE_MODE, active=True) + @patch('openedx.features.offline_mode.views.generate_offline_content_for_course.apply_async') + def test_admin_enabled_waffle_flag(self, mock_generate_offline_content_for_course_task): + self.staff_login() + course_id = 'course-v1:edX+DemoX+Demo_Course' + response = self.client.post(self.url, {'course_id': course_id}) + + self.assertEqual(response.status_code, 200) + self.assertEqual(response.data, None) + mock_generate_offline_content_for_course_task.assert_called_once_with(args=[course_id]) + + @override_waffle_flag(ENABLE_OFFLINE_MODE, active=False) + def test_admin_disabled_waffle_flag(self): + self.staff_login() + response = self.client.post(self.url, {'course_id': 'course-v1:edX+DemoX+Demo_Course'}) + + self.assertEqual(response.status_code, 400) + self.assertEqual(response.data, {'error': 'Offline mode is not enabled for this course'}) + + @override_waffle_flag(ENABLE_OFFLINE_MODE, active=True) + def test_admin_enabled_waffle_flag_no_course_id(self): + self.staff_login() + response = self.client.post(self.url, {}) + self.assertEqual(response.status_code, 400) + self.assertEqual(response.data, {'error': 'course_id is required'}) + + @override_waffle_flag(ENABLE_OFFLINE_MODE, active=False) + def test_admin_disabled_waffle_flag_no_course_id(self): + self.staff_login() + response = self.client.post(self.url, {}) + self.assertEqual(response.status_code, 400) + self.assertEqual(response.data, {'error': 'course_id is required'}) + + def test_invalid_course_id(self): + self.staff_login() + response = self.client.post(self.url, {'course_id': 'invalid_course_id'}) + self.assertEqual(response.status_code, 400) + self.assertEqual(response.data, {'error': 'Invalid course_id'}) diff --git a/openedx/features/offline_mode/urls.py b/openedx/features/offline_mode/urls.py index f5178a424316..d44d0d738f8a 100644 --- a/openedx/features/offline_mode/urls.py +++ b/openedx/features/offline_mode/urls.py @@ -5,6 +5,7 @@ from .views import SudioCoursePublishedEventHandler +app_name = 'offline_mode' urlpatterns = [ path('handle_course_published', SudioCoursePublishedEventHandler.as_view(), name='handle_course_published'), ] diff --git a/openedx/features/offline_mode/views.py b/openedx/features/offline_mode/views.py index ad3412a636b6..342fae92c80c 100644 --- a/openedx/features/offline_mode/views.py +++ b/openedx/features/offline_mode/views.py @@ -2,10 +2,15 @@ Views for the offline_mode app. """ from opaque_keys.edx.keys import CourseKey +from opaque_keys import InvalidKeyError +from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication from rest_framework import status +from rest_framework.authentication import SessionAuthentication +from rest_framework.permissions import IsAdminUser from rest_framework.response import Response from rest_framework.views import APIView +from openedx.core.lib.api.authentication import BearerAuthentication from .tasks import generate_offline_content_for_course from .toggles import is_offline_mode_enabled @@ -13,10 +18,14 @@ class SudioCoursePublishedEventHandler(APIView): """ Handle the event of a course being published in Studio. + This view is called by Studio when a course is published, and it triggers the generation of offline content. """ + authentication_classes = (JwtAuthentication, BearerAuthentication, SessionAuthentication) + permission_classes = (IsAdminUser,) + def post(self, request, *args, **kwargs): """ Trigger the generation of offline content task. @@ -27,14 +36,20 @@ def post(self, request, *args, **kwargs): Returns: Response: The response object. """ - course_id = request.data.get('course_id') if not course_id: return Response( data={'error': 'course_id is required'}, status=status.HTTP_400_BAD_REQUEST ) - course_key = CourseKey.from_string(course_id) + try: + course_key = CourseKey.from_string(course_id) + except InvalidKeyError: + return Response( + data={'error': 'Invalid course_id'}, + status=status.HTTP_400_BAD_REQUEST + ) + if is_offline_mode_enabled(course_key): generate_offline_content_for_course.apply_async(args=[course_id]) return Response(status=status.HTTP_200_OK) From 253ca5d030f5296b2b04d6e19f9396d41f3eb568 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=86=D0=B2=D0=B0=D0=BD=20=D0=9D=D1=94=D0=B4=D1=94=D0=BB?= =?UTF-8?q?=D1=8C=D0=BD=D1=96=D1=86=D0=B5=D0=B2?= Date: Thu, 21 Nov 2024 13:05:44 +0200 Subject: [PATCH 04/16] fix: [AXM-748] fix problem block offline generations --- openedx/features/offline_mode/assets_management.py | 4 ++-- openedx/features/offline_mode/renderer.py | 1 - openedx/features/offline_mode/tasks.py | 11 ++++++++--- openedx/features/offline_mode/utils.py | 10 ++++++++++ 4 files changed, 20 insertions(+), 6 deletions(-) diff --git a/openedx/features/offline_mode/assets_management.py b/openedx/features/offline_mode/assets_management.py index c1753ca686a3..b90c0332df02 100644 --- a/openedx/features/offline_mode/assets_management.py +++ b/openedx/features/offline_mode/assets_management.py @@ -57,8 +57,8 @@ def save_asset_file(temp_dir, xblock, path, filename): static_path = get_static_file_path(filename) content = read_static_file(static_path) file_path = os.path.join(temp_dir, 'assets', filename) - except (ItemNotFoundError, NotFoundError): - log.info(f"Asset not found: {filename}") + except (FileNotFoundError, ItemNotFoundError, NotFoundError): + log.warning(f"Asset not found: {filename}, during offline content generation.") else: create_subdirectories_for_asset(file_path) diff --git a/openedx/features/offline_mode/renderer.py b/openedx/features/offline_mode/renderer.py index 9a39bfee76c9..59f027243067 100644 --- a/openedx/features/offline_mode/renderer.py +++ b/openedx/features/offline_mode/renderer.py @@ -78,7 +78,6 @@ def render_xblock_from_lms(self): str(self.usage_key), disable_staff_debug_info=True, course=course, - will_recheck_access='1', ) enable_completion_on_view_service = False diff --git a/openedx/features/offline_mode/tasks.py b/openedx/features/offline_mode/tasks.py index 98c62b5add6f..79e991d652f8 100644 --- a/openedx/features/offline_mode/tasks.py +++ b/openedx/features/offline_mode/tasks.py @@ -3,6 +3,7 @@ """ from celery import shared_task from edx_django_utils.monitoring import set_code_owner_attribute +from django.http.response import Http404 from opaque_keys.edx.keys import CourseKey, UsageKey from xmodule.modulestore.django import modulestore @@ -17,16 +18,20 @@ def generate_offline_content_for_course(course_id): """ Generates offline content for all supported XBlocks in the course. + + Blocks that are closed to responses won't be processed. """ course_key = CourseKey.from_string(course_id) for offline_supported_block_type in OFFLINE_SUPPORTED_XBLOCKS: for xblock in modulestore().get_items(course_key, qualifiers={'category': offline_supported_block_type}): - html_data = XBlockRenderer(str(xblock.location)).render_xblock_from_lms() - generate_offline_content_for_block.apply_async([str(xblock.location), html_data]) + if not hasattr(xblock, 'closed') or not xblock.closed(): + block_id = str(xblock.location) + html_data = XBlockRenderer(block_id).render_xblock_from_lms() + generate_offline_content_for_block.apply_async([block_id, html_data]) @shared_task( - autoretry_for=(Exception,), + autoretry_for=(Exception, Http404), retry_backoff=RETRY_BACKOFF_INITIAL_TIMEOUT, retry_kwargs={'max_retries': MAX_RETRY_ATTEMPTS} )@set_code_owner_attribute diff --git a/openedx/features/offline_mode/utils.py b/openedx/features/offline_mode/utils.py index 6a0e561b656a..eeb4ff1efc7b 100644 --- a/openedx/features/offline_mode/utils.py +++ b/openedx/features/offline_mode/utils.py @@ -8,6 +8,7 @@ from django.contrib.auth import get_user_model from django.core.files.storage import default_storage +from django.http.response import Http404 from zipfile import ZipFile @@ -21,6 +22,7 @@ def generate_offline_content(xblock, html_data): """ Generates archive with XBlock content for offline mode. + Args: xblock (XBlock): The XBlock instance html_data (str): The rendered HTML representation of the XBlock @@ -35,6 +37,11 @@ def generate_offline_content(xblock, html_data): try: save_xblock_html(tmp_dir, xblock, html_data) create_zip_file(tmp_dir, base_path, f'{xblock.location.block_id}.zip') + except Http404: + log.error( + f'Block {xblock.location.block_id} cannot be fetched from course' + f' {xblock.location.course_key} during offline content generation.' + ) finally: shutil.rmtree(tmp_dir, ignore_errors=True) @@ -42,6 +49,7 @@ def generate_offline_content(xblock, html_data): def save_xblock_html(tmp_dir, xblock, html_data): """ Saves the XBlock HTML content to a file. + Generates the 'index.html' file with the HTML added to use it locally. Args: tmp_dir (str): The temporary directory path to save the xblock content @@ -58,6 +66,7 @@ def save_xblock_html(tmp_dir, xblock, html_data): def create_zip_file(temp_dir, base_path, file_name): """ Creates a zip file with the content of the base_path directory. + Args: temp_dir (str): The temporary directory path where the content is stored base_path (str): The base path directory to save the zip file @@ -79,6 +88,7 @@ def create_zip_file(temp_dir, base_path, file_name): def add_files_to_zip_recursively(zip_file, current_base_path, current_path_in_zip): """ Recursively adds files to the zip file. + Args: zip_file (ZipFile): The zip file object current_base_path (str): The current base path directory From bc39c7d226331bd8c3f7a3ed026f4cb981ec57cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=86=D0=B2=D0=B0=D0=BD=20=D0=9D=D1=94=D0=B4=D1=94=D0=BB?= =?UTF-8?q?=D1=8C=D0=BD=D1=96=D1=86=D0=B5=D0=B2?= Date: Thu, 21 Nov 2024 13:08:57 +0200 Subject: [PATCH 05/16] feat: [AXM-749] Implement s3 storage supporting --- .../mobile_api/course_info/views.py | 2 +- .../offline_mode/assets_management.py | 28 ++--- .../offline_mode/storage_management.py | 105 +++++++++++++++++ openedx/features/offline_mode/tasks.py | 4 +- openedx/features/offline_mode/utils.py | 107 ------------------ 5 files changed, 116 insertions(+), 130 deletions(-) create mode 100644 openedx/features/offline_mode/storage_management.py delete mode 100644 openedx/features/offline_mode/utils.py diff --git a/lms/djangoapps/mobile_api/course_info/views.py b/lms/djangoapps/mobile_api/course_info/views.py index 844fa4cf77ea..b046fb81d907 100644 --- a/lms/djangoapps/mobile_api/course_info/views.py +++ b/lms/djangoapps/mobile_api/course_info/views.py @@ -429,7 +429,7 @@ def _extend_block_info_with_offline_data(blocks_info_data: Dict[str, Dict]) -> N block_info.update({ 'offline_download': { 'file_url': file_url, - 'last_modified': default_storage.get_created_time(offline_content_path), + 'last_modified': default_storage.get_modified_time(offline_content_path), 'file_size': default_storage.size(offline_content_path) } }) diff --git a/openedx/features/offline_mode/assets_management.py b/openedx/features/offline_mode/assets_management.py index b90c0332df02..cab10f211ea0 100644 --- a/openedx/features/offline_mode/assets_management.py +++ b/openedx/features/offline_mode/assets_management.py @@ -1,11 +1,11 @@ """ This module contains utility functions for managing assets and files. """ -import shutil import logging import os import requests +from botocore.exceptions import ClientError from django.conf import settings from django.core.files.storage import default_storage @@ -77,35 +77,23 @@ def create_subdirectories_for_asset(file_path): os.mkdir(out_dir_name) -def remove_old_files(xblock): +def clean_outdated_xblock_files(xblock): """ - Removes the 'assets' directory, 'index.html', and 'offline_content.zip' files - in the specified base path directory. + Removes the old zip file with Offline Content from media storage. + Args: (XBlock): The XBlock instance """ try: base_path = block_storage_path(xblock) - assets_path = os.path.join(base_path, 'assets') - index_file_path = os.path.join(base_path, 'index.html') offline_zip_path = os.path.join(base_path, f'{xblock.location.block_id}.zip') - # Delete the 'assets' directory if it exists - if os.path.isdir(assets_path): - shutil.rmtree(assets_path) - log.info(f"Successfully deleted the directory: {assets_path}") - - # Delete the 'index.html' file if it exists - if default_storage.exists(index_file_path): - os.remove(index_file_path) - log.info(f"Successfully deleted the file: {index_file_path}") - # Delete the 'offline_content.zip' file if it exists if default_storage.exists(offline_zip_path): - os.remove(offline_zip_path) + default_storage.delete(offline_zip_path) log.info(f"Successfully deleted the file: {offline_zip_path}") - except OSError as e: + except ClientError as e: log.error(f"Error occurred while deleting the files or directory: {e}") @@ -148,8 +136,8 @@ def is_modified(xblock): file_path = os.path.join(block_storage_path(xblock), f'{xblock.location.block_id}.zip') try: - last_modified = default_storage.get_created_time(file_path) - except OSError: + last_modified = default_storage.get_modified_time(file_path) + except (OSError, ClientError): return True return xblock.published_on > last_modified diff --git a/openedx/features/offline_mode/storage_management.py b/openedx/features/offline_mode/storage_management.py new file mode 100644 index 000000000000..477aaa29a7af --- /dev/null +++ b/openedx/features/offline_mode/storage_management.py @@ -0,0 +1,105 @@ +""" +Utility functions and classes for offline mode. +""" +import os +import logging +import shutil +from tempfile import mkdtemp +from django.contrib.auth import get_user_model +from django.core.files.base import ContentFile +from django.http.response import Http404 +from openedx.core.storage import get_storage +from zipfile import ZipFile +from .assets_management import block_storage_path, clean_outdated_xblock_files, is_modified +from .html_manipulator import HtmlManipulator +User = get_user_model() +log = logging.getLogger(__name__) + + +class OfflineContentGenerator: + """ + Creates zip file with Offline Content in the media storage. + """ + def __init__(self, xblock, html_data, storage_class=None, storage_kwargs=None): + """ + Creates `SaveOfflineContentToStorage` object. + Args: + xblock (XBlock): The XBlock instance + html_data (str): The rendered HTML representation of the XBlock + storage_class: Used media storage class. + storage_kwargs (dict): Additional storage attributes. + """ + if storage_kwargs is None: + storage_kwargs = {} + self.xblock = xblock + self.html_data = html_data + self.storage = get_storage(storage_class, **storage_kwargs) + def generate_offline_content(self): + """ + Generates archive with XBlock content for offline mode. + """ + if not is_modified(self.xblock): + return + base_path = block_storage_path(self.xblock) + clean_outdated_xblock_files(self.xblock) + tmp_dir = mkdtemp() + try: + self.save_xblock_html(tmp_dir) + self.create_zip_file(tmp_dir, base_path, f'{self.xblock.location.block_id}.zip') + except Http404: + log.error( + f'Block {self.xblock.location.block_id} cannot be fetched from course' + f' {self.xblock.location.course_key} during offline content generation.' + ) + finally: + shutil.rmtree(tmp_dir, ignore_errors=True) + def save_xblock_html(self, tmp_dir): + """ + Saves the XBlock HTML content to a file. + Generates the 'index.html' file with the HTML added to use it locally. + Args: + tmp_dir (str): The temporary directory path to save the xblock content + """ + html_manipulator = HtmlManipulator(self.xblock, self.html_data, tmp_dir) + updated_html = html_manipulator.process_html() + with open(os.path.join(tmp_dir, 'index.html'), 'w') as file: + file.write(updated_html) + def create_zip_file(self, temp_dir, base_path, file_name): + """ + Creates a zip file with the Offline Content in the media storage. + Args: + temp_dir (str): The temporary directory path where the content is stored + base_path (str): The base path directory to save the zip file + file_name (str): The name of the zip file + """ + file_path = os.path.join(temp_dir, file_name) + with ZipFile(file_path, 'w') as zip_file: + zip_file.write(os.path.join(temp_dir, 'index.html'), 'index.html') + self.add_files_to_zip_recursively( + zip_file, + current_base_path=os.path.join(temp_dir, 'assets'), + current_path_in_zip='assets', + ) + with open(file_path, 'rb') as buffered_zip: + content_file = ContentFile(buffered_zip.read()) + self.storage.save(base_path + file_name, content_file) + log.info(f'Offline content for {file_name} has been generated.') + def add_files_to_zip_recursively(self, zip_file, current_base_path, current_path_in_zip): + """ + Recursively adds files to the zip file. + Args: + zip_file (ZipFile): The zip file object + current_base_path (str): The current base path directory + current_path_in_zip (str): The current path in the zip file + """ + try: + for resource_path in os.listdir(current_base_path): + full_path = os.path.join(current_base_path, resource_path) + full_path_in_zip = os.path.join(current_path_in_zip, resource_path) + if os.path.isfile(full_path): + zip_file.write(full_path, full_path_in_zip) + else: + self.add_files_to_zip_recursively(zip_file, full_path, full_path_in_zip) + except OSError: + log.error(f'Error while reading the directory: {current_base_path}') + return diff --git a/openedx/features/offline_mode/tasks.py b/openedx/features/offline_mode/tasks.py index 79e991d652f8..f1614213e7f1 100644 --- a/openedx/features/offline_mode/tasks.py +++ b/openedx/features/offline_mode/tasks.py @@ -10,7 +10,7 @@ from .constants import MAX_RETRY_ATTEMPTS, OFFLINE_SUPPORTED_XBLOCKS, RETRY_BACKOFF_INITIAL_TIMEOUT from .renderer import XBlockRenderer -from .utils import generate_offline_content +from .storage_management import OfflineContentGenerator @shared_task @@ -27,7 +27,7 @@ def generate_offline_content_for_course(course_id): if not hasattr(xblock, 'closed') or not xblock.closed(): block_id = str(xblock.location) html_data = XBlockRenderer(block_id).render_xblock_from_lms() - generate_offline_content_for_block.apply_async([block_id, html_data]) + OfflineContentGenerator(xblock, html_data).generate_offline_content() @shared_task( diff --git a/openedx/features/offline_mode/utils.py b/openedx/features/offline_mode/utils.py deleted file mode 100644 index eeb4ff1efc7b..000000000000 --- a/openedx/features/offline_mode/utils.py +++ /dev/null @@ -1,107 +0,0 @@ -""" -Utility functions and classes for offline mode. -""" -import os -import logging -import shutil -from tempfile import mkdtemp - -from django.contrib.auth import get_user_model -from django.core.files.storage import default_storage -from django.http.response import Http404 - -from zipfile import ZipFile - -from .assets_management import block_storage_path, remove_old_files, is_modified -from .html_manipulator import HtmlManipulator - -User = get_user_model() -log = logging.getLogger(__name__) - - -def generate_offline_content(xblock, html_data): - """ - Generates archive with XBlock content for offline mode. - - Args: - xblock (XBlock): The XBlock instance - html_data (str): The rendered HTML representation of the XBlock - """ - if not is_modified(xblock): - return - - base_path = block_storage_path(xblock) - remove_old_files(xblock) - tmp_dir = mkdtemp() - - try: - save_xblock_html(tmp_dir, xblock, html_data) - create_zip_file(tmp_dir, base_path, f'{xblock.location.block_id}.zip') - except Http404: - log.error( - f'Block {xblock.location.block_id} cannot be fetched from course' - f' {xblock.location.course_key} during offline content generation.' - ) - finally: - shutil.rmtree(tmp_dir, ignore_errors=True) - - -def save_xblock_html(tmp_dir, xblock, html_data): - """ - Saves the XBlock HTML content to a file. - - Generates the 'index.html' file with the HTML added to use it locally. - Args: - tmp_dir (str): The temporary directory path to save the xblock content - xblock (XBlock): The XBlock instance - html_data (str): The rendered HTML representation of the XBlock - """ - html_manipulator = HtmlManipulator(xblock, html_data, tmp_dir) - updated_html = html_manipulator.process_html() - - with open(os.path.join(tmp_dir, 'index.html'), 'w') as file: - file.write(updated_html) - - -def create_zip_file(temp_dir, base_path, file_name): - """ - Creates a zip file with the content of the base_path directory. - - Args: - temp_dir (str): The temporary directory path where the content is stored - base_path (str): The base path directory to save the zip file - file_name (str): The name of the zip file - """ - if not os.path.exists(default_storage.path(base_path)): - os.makedirs(default_storage.path(base_path)) - - with ZipFile(default_storage.path(base_path + file_name), 'w') as zip_file: - zip_file.write(os.path.join(temp_dir, 'index.html'), 'index.html') - add_files_to_zip_recursively( - zip_file, - current_base_path=os.path.join(temp_dir, 'assets'), - current_path_in_zip='assets', - ) - log.info(f'Offline content for {file_name} has been generated.') - - -def add_files_to_zip_recursively(zip_file, current_base_path, current_path_in_zip): - """ - Recursively adds files to the zip file. - - Args: - zip_file (ZipFile): The zip file object - current_base_path (str): The current base path directory - current_path_in_zip (str): The current path in the zip file - """ - try: - for resource_path in os.listdir(current_base_path): - full_path = os.path.join(current_base_path, resource_path) - full_path_in_zip = os.path.join(current_path_in_zip, resource_path) - if os.path.isfile(full_path): - zip_file.write(full_path, full_path_in_zip) - else: - add_files_to_zip_recursively(zip_file, full_path, full_path_in_zip) - except OSError: - log.error(f'Error while reading the directory: {current_base_path}') - return From 713bf2eba78193cc715cf7556c0520d107ba6baf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=86=D0=B2=D0=B0=D0=BD=20=D0=9D=D1=94=D0=B4=D1=94=D0=BB?= =?UTF-8?q?=D1=8C=D0=BD=D1=96=D1=86=D0=B5=D0=B2?= Date: Thu, 21 Nov 2024 13:13:47 +0200 Subject: [PATCH 06/16] fix: [AXM-791] fix error 404 handling and optimize for not modified blocks --- cms/djangoapps/contentstore/utils.py | 2 +- .../offline_mode/storage_management.py | 34 +++++++++++++------ openedx/features/offline_mode/tasks.py | 14 ++++---- 3 files changed, 32 insertions(+), 18 deletions(-) diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index ff7dfb337653..eef2b27f4355 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -2355,7 +2355,7 @@ def get_cms_api_client(): """ Returns an API client which can be used to make requests from the CMS service. """ - user = User.objects.get(username=settings.EDXAPP_CMS_SERVICE_USER_NAME) + user = User.objects.get(username=settings.CMS_SERVICE_USER_NAME) jwt = create_jwt_for_user(user) client = requests.Session() client.auth = SuppliedJwtAuth(jwt) diff --git a/openedx/features/offline_mode/storage_management.py b/openedx/features/offline_mode/storage_management.py index 477aaa29a7af..028f1ccf5ed8 100644 --- a/openedx/features/offline_mode/storage_management.py +++ b/openedx/features/offline_mode/storage_management.py @@ -10,8 +10,11 @@ from django.http.response import Http404 from openedx.core.storage import get_storage from zipfile import ZipFile -from .assets_management import block_storage_path, clean_outdated_xblock_files, is_modified + +from .assets_management import block_storage_path, clean_outdated_xblock_files from .html_manipulator import HtmlManipulator +from .renderer import XBlockRenderer + User = get_user_model() log = logging.getLogger(__name__) @@ -20,7 +23,8 @@ class OfflineContentGenerator: """ Creates zip file with Offline Content in the media storage. """ - def __init__(self, xblock, html_data, storage_class=None, storage_kwargs=None): + + def __init__(self, xblock, html_data=None, storage_class=None, storage_kwargs=None): """ Creates `SaveOfflineContentToStorage` object. Args: @@ -32,27 +36,35 @@ def __init__(self, xblock, html_data, storage_class=None, storage_kwargs=None): if storage_kwargs is None: storage_kwargs = {} self.xblock = xblock - self.html_data = html_data + self.html_data = html_data or self.render_block_html_data() self.storage = get_storage(storage_class, **storage_kwargs) + + def render_block_html_data(self): + """ + Renders the XBlock HTML content from the LMS. + """ + try: + return XBlockRenderer(str(self.xblock.location)).render_xblock_from_lms() + except Http404 as e: + log.error( + f'Block {str(self.xblock.location)} cannot be fetched from course' + f' {self.xblock.location.course_key} during offline content generation.' + ) + raise e + def generate_offline_content(self): """ Generates archive with XBlock content for offline mode. """ - if not is_modified(self.xblock): - return base_path = block_storage_path(self.xblock) clean_outdated_xblock_files(self.xblock) tmp_dir = mkdtemp() try: self.save_xblock_html(tmp_dir) self.create_zip_file(tmp_dir, base_path, f'{self.xblock.location.block_id}.zip') - except Http404: - log.error( - f'Block {self.xblock.location.block_id} cannot be fetched from course' - f' {self.xblock.location.course_key} during offline content generation.' - ) finally: shutil.rmtree(tmp_dir, ignore_errors=True) + def save_xblock_html(self, tmp_dir): """ Saves the XBlock HTML content to a file. @@ -64,6 +76,7 @@ def save_xblock_html(self, tmp_dir): updated_html = html_manipulator.process_html() with open(os.path.join(tmp_dir, 'index.html'), 'w') as file: file.write(updated_html) + def create_zip_file(self, temp_dir, base_path, file_name): """ Creates a zip file with the Offline Content in the media storage. @@ -84,6 +97,7 @@ def create_zip_file(self, temp_dir, base_path, file_name): content_file = ContentFile(buffered_zip.read()) self.storage.save(base_path + file_name, content_file) log.info(f'Offline content for {file_name} has been generated.') + def add_files_to_zip_recursively(self, zip_file, current_base_path, current_path_in_zip): """ Recursively adds files to the zip file. diff --git a/openedx/features/offline_mode/tasks.py b/openedx/features/offline_mode/tasks.py index f1614213e7f1..6ec13454edf0 100644 --- a/openedx/features/offline_mode/tasks.py +++ b/openedx/features/offline_mode/tasks.py @@ -8,6 +8,7 @@ from xmodule.modulestore.django import modulestore +from .assets_management import is_modified from .constants import MAX_RETRY_ATTEMPTS, OFFLINE_SUPPORTED_XBLOCKS, RETRY_BACKOFF_INITIAL_TIMEOUT from .renderer import XBlockRenderer from .storage_management import OfflineContentGenerator @@ -24,21 +25,20 @@ def generate_offline_content_for_course(course_id): course_key = CourseKey.from_string(course_id) for offline_supported_block_type in OFFLINE_SUPPORTED_XBLOCKS: for xblock in modulestore().get_items(course_key, qualifiers={'category': offline_supported_block_type}): - if not hasattr(xblock, 'closed') or not xblock.closed(): - block_id = str(xblock.location) - html_data = XBlockRenderer(block_id).render_xblock_from_lms() - OfflineContentGenerator(xblock, html_data).generate_offline_content() + is_not_closed = not hasattr(xblock, 'closed') or not xblock.closed() + if is_not_closed and is_modified(xblock): + generate_offline_content_for_block.apply_async([str(xblock.location)]) @shared_task( - autoretry_for=(Exception, Http404), + autoretry_for=(Http404,), retry_backoff=RETRY_BACKOFF_INITIAL_TIMEOUT, retry_kwargs={'max_retries': MAX_RETRY_ATTEMPTS} )@set_code_owner_attribute -def generate_offline_content_for_block(block_id, html_data): +def generate_offline_content_for_block(block_id): """ Generates offline content for the specified block. """ block_usage_key = UsageKey.from_string(block_id) xblock = modulestore().get_item(block_usage_key) - generate_offline_content(xblock, html_data) + OfflineContentGenerator(xblock).generate_offline_content() From f2044880898e0e090226a6a5c8734c33a5c370d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=86=D0=B2=D0=B0=D0=BD=20=D0=9D=D1=94=D0=B4=D1=94=D0=BB?= =?UTF-8?q?=D1=8C=D0=BD=D1=96=D1=86=D0=B5=D0=B2?= Date: Thu, 21 Nov 2024 13:15:36 +0200 Subject: [PATCH 07/16] refactor: [AXM-361] refactor JS bridge for IOS and Android --- lms/static/js/courseware/bridge.js | 72 ++++++++++++++ lms/static/js/spec/courseware/bridge_spec.js | 99 +++++++++++++++++++ .../courseware/courseware-chromeless.html | 42 -------- 3 files changed, 171 insertions(+), 42 deletions(-) create mode 100644 lms/static/js/courseware/bridge.js create mode 100644 lms/static/js/spec/courseware/bridge_spec.js diff --git a/lms/static/js/courseware/bridge.js b/lms/static/js/courseware/bridge.js new file mode 100644 index 000000000000..0c8241d31830 --- /dev/null +++ b/lms/static/js/courseware/bridge.js @@ -0,0 +1,72 @@ +/** + * JS bridge for communication between the native mobile apps and the xblock. + * + * This script is used to send data about student's answer to the native mobile apps (IOS and Android) + * and to receive data about student's answer from the native mobile apps to fill the form + * with the student's answer, disable xblock inputs and mark the problem as completed. + * + * Separate functions for each platform allow you to flexibly add platform-specific logic + * as needed without changing the naming on the mobile side. + */ + +/** + * Sends a data about student's answer to the IOS app. + * + * @param {string} message The stringified JSON object to be sent to the IOS app + */ +function sendMessageToIOS(message) { + window?.webkit?.messageHandlers?.IOSBridge?.postMessage(message); +} + +/** + * Sends a data about student's answer to the Android app. + * + * @param {string} message The stringified JSON object to be sent to the native Android app + */ +function sendMessageToAndroid(message) { + window?.AndroidBridge?.postMessage(message); +} + +/** + * Receives a message from the mobile apps and fills the form with the student's answer, + * disables xblock inputs and marks the problem as completed with appropriate message. + * + * @param {string} message The stringified JSON object about the student's answer from the native mobile app. + */ +function markProblemCompleted(message) { + const data = JSON.parse(message).data; + const problemContainer = $(".xblock-student_view"); + const submitButton = problemContainer.find(".submit-attempt-container .submit"); + const notificationContainer = problemContainer.find(".notification-gentle-alert"); + submitButton.attr({disabled: "disabled"}); + notificationContainer.find(".notification-message").text("Answer submitted."); + notificationContainer.show(); + + data.split("&").forEach(function (item) { + const [inputId, answer] = item.split('=', 2); + problemContainer.find(`input[id$="${answer}"], input[id$="${inputId}"]`).each(function () { + this.disabled = true; + if (this.type === "checkbox" || this.type === "radio") { + this.checked = true; + } else { + this.value = answer; + } + }) + }) +} + +/** + * Overrides the default $.ajax function to intercept the requests to the "handler/xmodule_handler/problem_check" + * endpoint and send the data to the native mobile apps. + * + * @param {Object} options The data object for the ajax request + */ +const originalAjax = $.ajax; +$.ajax = function (options) { + if (options.url && options.url.endsWith("handler/xmodule_handler/problem_check")) { + sendMessageToIOS(JSON.stringify(options)); + sendMessageToAndroid(JSON.stringify(options)); + } + return originalAjax.call(this, options); +} + diff --git a/lms/static/js/spec/courseware/bridge_spec.js b/lms/static/js/spec/courseware/bridge_spec.js new file mode 100644 index 000000000000..079a77445df7 --- /dev/null +++ b/lms/static/js/spec/courseware/bridge_spec.js @@ -0,0 +1,99 @@ +describe('JS bridge for communication between native mobile apps and the xblock', function() { + + beforeEach(function() { + // Mock objects for IOS and Android bridges + window.webkit = { + messageHandlers: { + IOSBridge: { + postMessage: jasmine.createSpy('postMessage') + } + } + }; + window.AndroidBridge = { + postMessage: jasmine.createSpy('postMessage') + }; + }); + + describe('sendMessageToIOS', function() { + it('should call postMessage on IOSBridge with the correct message', function() { + const message = JSON.stringify({answer: 'test'}); + sendMessageToIOS(message); + expect(window.webkit.messageHandlers.IOSBridge.postMessage).toHaveBeenCalledWith(message); + }); + }); + + describe('sendMessageToAndroid', function() { + it('should call postMessage on AndroidBridge with the correct message', function() { + const message = JSON.stringify({answer: 'test'}); + sendMessageToAndroid(message); + expect(window.AndroidBridge.postMessage).toHaveBeenCalledWith(message); + }); + }); + + describe('markProblemCompleted', function() { + it('should correctly parse the message and update the DOM elements', function() { + const message = JSON.stringify({ + data: 'input1=answer1&input2=answer2' + }); + const problemContainer = $('
' + + '
' + + '' + + '
' + + '
' + + '
' + + '
' + + '' + + '' + + '' + + '' + + '
'); + $('body').append(problemContainer); + + markProblemCompleted(message); + + expect(problemContainer.find('.submit-attempt-container .submit').attr('disabled')).toBe('disabled'); + expect(problemContainer.find('.notification-gentle-alert .notification-message').html()).toBe('Answer submitted.'); + expect(problemContainer.find('.notification-gentle-alert').css('display')).not.toBe('none'); + expect(problemContainer.find('#input1').val()).toBe('answer1'); + expect(problemContainer.find('#input2').val()).toBe('answer2'); + expect(problemContainer.find('#answer1').prop('disabled')).toBe(true); + expect(problemContainer.find('#answer2').prop('disabled')).toBe(true); + + problemContainer.remove(); + }); + }); + + describe('$.ajax', function() { + beforeEach(function() { + spyOn($, 'ajax').and.callThrough(); + }); + + it('should intercept the request to problem_check and send data to the native apps', function() { + const ajaxOptions = { + url: 'http://example.com/handler/xmodule_handler/problem_check', + data: {answer: 'test'} + }; + + $.ajax(ajaxOptions); + + expect(window.webkit.messageHandlers.IOSBridge.postMessage).toHaveBeenCalledWith(JSON.stringify(ajaxOptions)); + expect(window.AndroidBridge.postMessage).toHaveBeenCalledWith(JSON.stringify(ajaxOptions)); + }); + + it('should call the original $.ajax function', function() { + const ajaxOptions = { + url: 'http://example.com/handler/xmodule_handler/problem_check', + data: {answer: 'test'} + }; + + const originalAjax = spyOn($, 'ajax').and.callFake(function(options) { + return originalAjax.and.callThrough().call(this, options); + }); + + $.ajax(ajaxOptions); + + expect(originalAjax).toHaveBeenCalledWith(ajaxOptions); + }); + }); +}); + diff --git a/lms/templates/courseware/courseware-chromeless.html b/lms/templates/courseware/courseware-chromeless.html index 18a8f333efad..deeda26c431d 100644 --- a/lms/templates/courseware/courseware-chromeless.html +++ b/lms/templates/courseware/courseware-chromeless.html @@ -219,45 +219,3 @@ }()); % endif - -% if is_offline_content: - -% endif From f25bfa227ba744f2da28c155d181bcbab09e6b6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=86=D0=B2=D0=B0=D0=BD=20=D0=9D=D1=94=D0=B4=D1=94=D0=BB?= =?UTF-8?q?=D1=8C=D0=BD=D1=96=D1=86=D0=B5=D0=B2?= Date: Thu, 21 Nov 2024 13:18:12 +0200 Subject: [PATCH 08/16] feat: [AXM-755] save external files and fonts to offline block content --- .../offline_mode/assets_management.py | 23 +++++++++++- .../features/offline_mode/html_manipulator.py | 37 ++++++++++++++++++- 2 files changed, 58 insertions(+), 2 deletions(-) diff --git a/openedx/features/offline_mode/assets_management.py b/openedx/features/offline_mode/assets_management.py index cab10f211ea0..aa47addf7292 100644 --- a/openedx/features/offline_mode/assets_management.py +++ b/openedx/features/offline_mode/assets_management.py @@ -16,7 +16,6 @@ from .constants import MATHJAX_CDN_URL, MATHJAX_STATIC_PATH - log = logging.getLogger(__name__) @@ -39,8 +38,10 @@ def read_static_file(path): def save_asset_file(temp_dir, xblock, path, filename): """ Saves an asset file to the default storage. + If the filename contains a '/', it reads the static file directly from the file system. Otherwise, it fetches the asset from the AssetManager. + Args: temp_dir (str): The temporary directory where the assets are stored. xblock (XBlock): The XBlock instance @@ -100,6 +101,7 @@ def clean_outdated_xblock_files(xblock): def get_offline_block_content_path(xblock=None, usage_key=None): """ Checks whether 'offline_content.zip' file is present in the specified base path directory. + Args: xblock (XBlock): The XBlock instance usage_key (UsageKey): The UsageKey of the XBlock @@ -115,8 +117,10 @@ def get_offline_block_content_path(xblock=None, usage_key=None): def block_storage_path(xblock=None, usage_key=None): """ Generates the base storage path for the given XBlock. + The path is constructed based on the XBlock's location, which includes the organization, course, block type, and block ID. + Args: xblock (XBlock): The XBlock instance for which to generate the storage path. usage_key (UsageKey): The UsageKey of the XBlock. @@ -130,6 +134,7 @@ def block_storage_path(xblock=None, usage_key=None): def is_modified(xblock): """ Check if the xblock has been modified since the last time the offline content was generated. + Args: xblock (XBlock): The XBlock instance to check. """ @@ -146,6 +151,7 @@ def is_modified(xblock): def save_mathjax_to_xblock_assets(temp_dir): """ Saves MathJax to the local static directory. + If MathJax is not already saved, it fetches MathJax from the CDN and saves it to the local static directory. """ @@ -156,3 +162,18 @@ def save_mathjax_to_xblock_assets(temp_dir): file.write(response.content) log.info(f"Successfully saved MathJax to {file_path}") + + +def save_external_file(temp_dir, link, filename): + """ + Save external file to the local directory. + """ + file_path = os.path.join(temp_dir, filename) + try: + response = requests.get(link) + except requests.exceptions.RequestException as e: + log.error(f"Failed to download {link}: {e}") + else: + create_subdirectories_for_asset(file_path) + with open(file_path, 'wb') as file: + file.write(response.content) diff --git a/openedx/features/offline_mode/html_manipulator.py b/openedx/features/offline_mode/html_manipulator.py index 9e5bff7ab396..80d819c7b80c 100644 --- a/openedx/features/offline_mode/html_manipulator.py +++ b/openedx/features/offline_mode/html_manipulator.py @@ -3,12 +3,13 @@ """ import os import re +import uuid from bs4 import BeautifulSoup from django.conf import settings -from .assets_management import save_asset_file, save_mathjax_to_xblock_assets +from .assets_management import save_asset_file, save_external_file, save_mathjax_to_xblock_assets from .constants import MATHJAX_CDN_URL, MATHJAX_STATIC_PATH @@ -31,6 +32,8 @@ def process_html(self): self._replace_asset_links() self._replace_static_links() self._replace_mathjax_link() + self._replace_external_links() + self._copy_platform_fonts() soup = BeautifulSoup(self.html_data, 'html.parser') self._replace_iframe(soup) @@ -77,6 +80,38 @@ def _replace_link(self, match): save_asset_file(self.temp_dir, self.xblock, link, filename) return f'assets/{filename}' + def _replace_external_links(self): + """ + Replace external links to images and js files with local links. + """ + pattern = re.compile(r'https:\/\/[^"\s]+?\.(js|jpe?g|png|gif|bmp|svg)') + self.html_data = pattern.sub(self._replace_external_link, self.html_data) + + def _replace_external_link(self, match): + """ + Returns the local path of the external file. + Downloads the external file and saves it to the local directory. + """ + link = match.group() + file_extension = match.group(1) + unique_id = uuid.uuid4() + filename = f"assets/external/{unique_id}.{file_extension}" + save_external_file(self.temp_dir, link, filename) + + return filename + + def _copy_platform_fonts(self): + """ + Copy platform fonts to the block temp directory. + """ + platform_fonts_dir = "xmodule/js/common_static/fonts/vendor/" + block_fonts_path = os.path.join(self.temp_dir, "assets", "fonts", "vendor") + os.makedirs(block_fonts_path) + for font in os.listdir(platform_fonts_dir): + font_path = os.path.join(platform_fonts_dir, font) + if os.path.isfile(font_path): + os.system(f'cp {font_path} {block_fonts_path}') + @staticmethod def _replace_iframe(soup): """ From 7d8de391e8e852d64984590af41642a036aed236 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=86=D0=B2=D0=B0=D0=BD=20=D0=9D=D1=94=D0=B4=D1=94=D0=BB?= =?UTF-8?q?=D1=8C=D0=BD=D1=96=D1=86=D0=B5=D0=B2?= Date: Thu, 21 Nov 2024 13:19:34 +0200 Subject: [PATCH 09/16] fix: don't display icon in offline mode --- lms/static/js/courseware/bridge.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/lms/static/js/courseware/bridge.js b/lms/static/js/courseware/bridge.js index 0c8241d31830..03efb52e39b6 100644 --- a/lms/static/js/courseware/bridge.js +++ b/lms/static/js/courseware/bridge.js @@ -36,15 +36,20 @@ function sendMessageToAndroid(message) { function markProblemCompleted(message) { const data = JSON.parse(message).data; const problemContainer = $(".xblock-student_view"); + const submitButton = problemContainer.find(".submit-attempt-container .submit"); const notificationContainer = problemContainer.find(".notification-gentle-alert"); + submitButton.attr({disabled: "disabled"}); - notificationContainer.find(".notification-message").text("Answer submitted."); + notificationContainer.find(".notification-message").text("Answer submitted"); + notificationContainer.find(".icon").remove(); notificationContainer.show(); data.split("&").forEach(function (item) { const [inputId, answer] = item.split('=', 2); - problemContainer.find(`input[id$="${answer}"], input[id$="${inputId}"]`).each(function () { + problemContainer.find( + `input[id$="${answer}"], input[id$="${inputId}"]` + ).each(function () { this.disabled = true; if (this.type === "checkbox" || this.type === "radio") { this.checked = true; From 199399190316cb59ef213612b7c5d25288eb8700 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=86=D0=B2=D0=B0=D0=BD=20=D0=9D=D1=94=D0=B4=D1=94=D0=BB?= =?UTF-8?q?=D1=8C=D0=BD=D1=96=D1=86=D0=B5=D0=B2?= Date: Thu, 21 Nov 2024 13:26:33 +0200 Subject: [PATCH 10/16] test: [AXM-636] Cover Offline Mode API with unit tests --- .../tests/test_course_info_views.py | 87 +++- .../offline_mode/assets_management.py | 3 +- openedx/features/offline_mode/tests/base.py | 47 +++ .../tests/test_assets_management.py | 390 ++++++++++++++++++ .../tests/test_html_manipulator.py | 163 ++++++++ .../offline_mode/tests/test_renderer.py | 31 ++ .../tests/test_storage_management.py | 268 ++++++++++++ .../features/offline_mode/tests/test_tasks.py | 141 +++++++ 8 files changed, 1126 insertions(+), 4 deletions(-) create mode 100644 openedx/features/offline_mode/tests/base.py create mode 100644 openedx/features/offline_mode/tests/test_assets_management.py create mode 100644 openedx/features/offline_mode/tests/test_html_manipulator.py create mode 100644 openedx/features/offline_mode/tests/test_renderer.py create mode 100644 openedx/features/offline_mode/tests/test_storage_management.py create mode 100644 openedx/features/offline_mode/tests/test_tasks.py 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 56c020ec8fa3..eaebac0f27c8 100644 --- a/lms/djangoapps/mobile_api/tests/test_course_info_views.py +++ b/lms/djangoapps/mobile_api/tests/test_course_info_views.py @@ -2,7 +2,7 @@ Tests for course_info """ from datetime import datetime, timedelta -from unittest.mock import patch +from unittest.mock import MagicMock, patch import ddt from django.conf import settings @@ -17,12 +17,14 @@ from common.djangoapps.student.tests.factories import UserFactory # pylint: disable=unused-import from common.djangoapps.util.course import get_link_for_about_page -from lms.djangoapps.course_api.blocks.tests.test_views import TestBlocksInCourseView +from lms.djangoapps.mobile_api.utils import API_V05, API_V1, API_V2, API_V3, API_V4 from lms.djangoapps.mobile_api.course_info.views import BlocksInfoInCourseView from lms.djangoapps.mobile_api.testutils import MobileAPITestCase, MobileAuthTestMixin, MobileCourseAccessTestMixin from lms.djangoapps.mobile_api.utils import API_V1, API_V05 from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.features.course_experience import ENABLE_COURSE_GOALS +from openedx.features.offline_mode.constants import DEFAULT_OFFLINE_SUPPORTED_XBLOCKS +from openedx.features.offline_mode.toggles import ENABLE_OFFLINE_MODE from xmodule.html_block import CourseInfoBlock # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore import ModuleStoreEnum # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order @@ -430,6 +432,87 @@ 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) + @patch('lms.djangoapps.mobile_api.course_info.views.default_storage') + @patch('lms.djangoapps.mobile_api.course_info.views.get_offline_block_content_path') + @patch('lms.djangoapps.mobile_api.course_info.views.is_offline_mode_enabled') + def test_extend_block_info_with_offline_data( + self, + is_offline_mode_enabled_mock: MagicMock, + get_offline_block_content_path_mock: MagicMock, + default_storage_mock: MagicMock, + ) -> None: + url = reverse('blocks_info_in_course', kwargs={'api_version': API_V4}) + offline_content_path_mock = '/offline_content_path_mock/' + created_time_mock = 'created_time_mock' + size_mock = 'size_mock' + get_offline_block_content_path_mock.return_value = offline_content_path_mock + default_storage_mock.get_modified_time.return_value = created_time_mock + default_storage_mock.size.return_value = size_mock + + expected_offline_download_data = { + 'file_url': offline_content_path_mock, + 'last_modified': created_time_mock, + 'file_size': size_mock + } + + response = self.verify_response(url=url) + + is_offline_mode_enabled_mock.assert_called_once_with(self.course.course_id) + self.assertEqual(response.status_code, status.HTTP_200_OK) + for block_info in response.data['blocks'].values(): + self.assertDictEqual(block_info['offline_download'], expected_offline_download_data) + + @patch('lms.djangoapps.mobile_api.course_info.views.is_offline_mode_enabled') + @ddt.data( + (API_V05, True), + (API_V05, False), + (API_V1, True), + (API_V1, False), + (API_V2, True), + (API_V2, False), + (API_V3, True), + (API_V3, False), + ) + @ddt.unpack + def test_not_extend_block_info_with_offline_data_for_version_less_v4_and_any_waffle_flag( + self, + api_version: str, + offline_mode_waffle_flag_mock: MagicMock, + is_offline_mode_enabled_mock: MagicMock, + ) -> None: + url = reverse('blocks_info_in_course', kwargs={'api_version': api_version}) + is_offline_mode_enabled_mock.return_value = offline_mode_waffle_flag_mock + + response = self.verify_response(url=url) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + for block_info in response.data['blocks'].values(): + self.assertNotIn('offline_download', block_info) + + @override_waffle_flag(ENABLE_OFFLINE_MODE, active=True) + @patch('openedx.features.offline_mode.html_manipulator.save_mathjax_to_xblock_assets') + def test_create_offline_content_integration_test(self, save_mathjax_to_xblock_assets_mock: MagicMock) -> None: + UserFactory.create(username='offline_mode_worker', password='password', is_staff=True) + handle_course_published_url = reverse('offline_mode:handle_course_published') + self.client.login(username='offline_mode_worker', password='password') + + handler_response = self.client.post(handle_course_published_url, {'course_id': str(self.course.id)}) + self.assertEqual(handler_response.status_code, status.HTTP_200_OK) + + url = reverse('blocks_info_in_course', kwargs={'api_version': API_V4}) + + response = self.verify_response(url=url) + self.assertEqual(response.status_code, status.HTTP_200_OK) + for block_info in response.data['blocks'].values(): + if block_type := block_info.get('type'): + if block_type in DEFAULT_OFFLINE_SUPPORTED_XBLOCKS: + expected_offline_content_url = f'/uploads/{self.course.id}/{block_info["block_id"]}.zip' + self.assertIn('offline_download', block_info) + self.assertIn('file_url', block_info['offline_download']) + self.assertIn('last_modified', block_info['offline_download']) + self.assertIn('file_size', block_info['offline_download']) + self.assertEqual(expected_offline_content_url, block_info['offline_download']['file_url']) + class TestCourseEnrollmentDetailsView(MobileAPITestCase, MilestonesTestCaseMixin): # lint-amnesty, pylint: disable=test-inherits-tests """ diff --git a/openedx/features/offline_mode/assets_management.py b/openedx/features/offline_mode/assets_management.py index aa47addf7292..a8845935fcfa 100644 --- a/openedx/features/offline_mode/assets_management.py +++ b/openedx/features/offline_mode/assets_management.py @@ -37,7 +37,7 @@ def read_static_file(path): def save_asset_file(temp_dir, xblock, path, filename): """ - Saves an asset file to the default storage. + Saves an asset file to the temporary directory. If the filename contains a '/', it reads the static file directly from the file system. Otherwise, it fetches the asset from the AssetManager. @@ -89,7 +89,6 @@ def clean_outdated_xblock_files(xblock): base_path = block_storage_path(xblock) offline_zip_path = os.path.join(base_path, f'{xblock.location.block_id}.zip') - # Delete the 'offline_content.zip' file if it exists if default_storage.exists(offline_zip_path): default_storage.delete(offline_zip_path) log.info(f"Successfully deleted the file: {offline_zip_path}") diff --git a/openedx/features/offline_mode/tests/base.py b/openedx/features/offline_mode/tests/base.py new file mode 100644 index 000000000000..f8692400f267 --- /dev/null +++ b/openedx/features/offline_mode/tests/base.py @@ -0,0 +1,47 @@ +""" +Tests for the testing xBlock renderers for Offline Mode. +""" + +from xmodule.capa.tests.response_xml_factory import MultipleChoiceResponseXMLFactory +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory, BlockFactory + + +class CourseForOfflineTestCase(ModuleStoreTestCase): + """ + Base class for creation course for Offline Mode testing. + """ + + def setUp(self): + super().setUp() + default_store = self.store.default_modulestore.get_modulestore_type() + with self.store.default_store(default_store): + self.course = CourseFactory.create( # lint-amnesty, pylint: disable=attribute-defined-outside-init + display_name='Offline Course', + org='RaccoonGang', + number='1', + run='2024', + ) + chapter = BlockFactory.create(parent=self.course, category='chapter') + problem_xml = MultipleChoiceResponseXMLFactory().build_xml( + question_text='The correct answer is Choice 2', + choices=[False, False, True, False], + choice_names=['choice_0', 'choice_1', 'choice_2', 'choice_3'] + ) + self.vertical_block = BlockFactory.create( # lint-amnesty, pylint: disable=attribute-defined-outside-init + parent_location=chapter.location, + category='vertical', + display_name='Vertical' + ) + self.html_block = BlockFactory.create( # lint-amnesty, pylint: disable=attribute-defined-outside-init + parent=self.vertical_block, + category='html', + display_name='HTML xblock for Offline', + data='

Test HTML Content

' + ) + self.problem_block = BlockFactory.create( # lint-amnesty, pylint: disable=attribute-defined-outside-init + parent=self.vertical_block, + category='problem', + display_name='Problem xblock for Offline', + data=problem_xml + ) diff --git a/openedx/features/offline_mode/tests/test_assets_management.py b/openedx/features/offline_mode/tests/test_assets_management.py new file mode 100644 index 000000000000..e386d10e2b57 --- /dev/null +++ b/openedx/features/offline_mode/tests/test_assets_management.py @@ -0,0 +1,390 @@ +""" +Tests for the testing utility functions for managing assets and files for Offline Mode. +""" + +import os + +from datetime import datetime +from unittest import TestCase +from unittest.mock import MagicMock, Mock, call, patch + +from botocore.exceptions import ClientError +from django.conf import settings +from path import Path +from pytz import UTC + +from openedx.features.offline_mode.assets_management import ( + block_storage_path, + clean_outdated_xblock_files, + create_subdirectories_for_asset, + get_offline_block_content_path, + get_static_file_path, + is_modified, + save_asset_file, + save_mathjax_to_xblock_assets, +) +from openedx.features.offline_mode.constants import MATHJAX_CDN_URL, MATHJAX_STATIC_PATH +from xmodule.modulestore.exceptions import ItemNotFoundError + + +class AssetsManagementTestCase(TestCase): + """ + Test case for the testing utility functions for managing assets and files. + """ + + def test_get_static_file_path(self) -> None: + relative_path_mock = 'relative_path_mock' + expected_result = Path(f'{settings.STATIC_ROOT}/{relative_path_mock}') + + result = get_static_file_path(relative_path_mock) + + self.assertEqual(result, expected_result) + + @patch('openedx.features.offline_mode.assets_management.open') + @patch('openedx.features.offline_mode.assets_management.create_subdirectories_for_asset') + @patch('openedx.features.offline_mode.assets_management.os.path.join') + @patch('openedx.features.offline_mode.assets_management.AssetManager.find') + @patch('openedx.features.offline_mode.assets_management.StaticContent.get_asset_key_from_path') + def test_save_asset_file_if_filename_contains_slash( + self, + get_asset_key_from_path_mock: MagicMock, + asset_manager_find_mock: MagicMock, + os_path_join_mock: MagicMock, + create_subdirectories_for_asset_mock: MagicMock, + context_manager_mock: MagicMock, + ) -> None: + temp_dir_mock = 'temp_dir_mock' + xblock_mock = Mock() + path_mock = 'path_mock' + filename_mock = 'assets/filename_mock' + + save_asset_file(temp_dir_mock, xblock_mock, path_mock, filename_mock) + + get_asset_key_from_path_mock.assert_called_once_with( + xblock_mock.location.course_key, filename_mock.split('/')[-1] + ) + asset_manager_find_mock.assert_called_once_with(get_asset_key_from_path_mock.return_value) + os_path_join_mock.assert_called_once_with(temp_dir_mock, filename_mock) + create_subdirectories_for_asset_mock.assert_called_once_with(os_path_join_mock.return_value) + context_manager_mock.assert_called_once_with(os_path_join_mock.return_value, 'wb') + context_manager_mock.return_value.__enter__.return_value.write.assert_called_once_with( + asset_manager_find_mock.return_value.data + ) + + @patch('openedx.features.offline_mode.assets_management.open') + @patch('openedx.features.offline_mode.assets_management.create_subdirectories_for_asset') + @patch('openedx.features.offline_mode.assets_management.os.path.join') + @patch('openedx.features.offline_mode.assets_management.read_static_file') + @patch('openedx.features.offline_mode.assets_management.get_static_file_path') + def test_save_asset_file_no_slash_in_filename( + self, + get_static_file_path_mock: MagicMock, + read_static_file_mock: MagicMock, + os_path_join_mock: MagicMock, + create_subdirectories_for_asset_mock: MagicMock, + context_manager_mock: MagicMock, + ) -> None: + temp_dir_mock = 'temp_dir_mock' + xblock_mock = Mock() + path_mock = 'path_mock' + filename_mock = 'filename_mock' + + save_asset_file(temp_dir_mock, xblock_mock, path_mock, filename_mock) + + get_static_file_path_mock.assert_called_once_with(filename_mock) + read_static_file_mock.assert_called_once_with(get_static_file_path_mock.return_value) + os_path_join_mock.assert_called_once_with( + temp_dir_mock, 'assets', filename_mock, + ) + create_subdirectories_for_asset_mock.assert_called_once_with(os_path_join_mock.return_value) + context_manager_mock.assert_called_once_with(os_path_join_mock.return_value, 'wb') + context_manager_mock.return_value.__enter__.return_value.write.assert_called_once_with( + read_static_file_mock.return_value + ) + + @patch('openedx.features.offline_mode.assets_management.log.warning') + @patch( + 'openedx.features.offline_mode.assets_management.get_static_file_path', side_effect=ItemNotFoundError + ) + def test_save_asset_file_can_not_find( + self, + get_static_file_path_mock: MagicMock, + log_warning_mock: MagicMock, + ) -> None: + temp_dir_mock = 'temp_dir_mock' + xblock_mock = Mock() + path_mock = 'path_mock' + filename_mock = 'filename_mock' + + save_asset_file(temp_dir_mock, xblock_mock, path_mock, filename_mock) + + get_static_file_path_mock.assert_called_once_with(filename_mock) + log_warning_mock.assert_called_once_with( + f'Asset not found: {filename_mock}, during offline content generation.' + ) + + @patch('openedx.features.offline_mode.assets_management.os') + def test_create_subdirectories_for_asset_subdirectories_does_not_exist(self, os_mock: MagicMock) -> None: + file_path_mock = 'file/path/mock/' + os_mock.path.exists.return_value = False + + expected_os_path_join_call_args_list = [ + call('/', 'file'), + call(os_mock.path.join.return_value, 'path'), + call(os_mock.path.join.return_value, 'mock'), + ] + expected_os_mock_mkdir_call_args_list = [ + call(os_mock.path.join.return_value), + call(os_mock.path.join.return_value), + call(os_mock.path.join.return_value), + ] + + create_subdirectories_for_asset(file_path_mock) + + self.assertListEqual(os_mock.path.join.call_args_list, expected_os_path_join_call_args_list) + self.assertListEqual(os_mock.mkdir.call_args_list, expected_os_mock_mkdir_call_args_list) + + @patch('openedx.features.offline_mode.assets_management.os') + def test_create_subdirectories_for_asset_subdirectories_exist(self, os_mock: MagicMock) -> None: + file_path_mock = 'file/path/mock/' + + expected_os_path_join_call_args_list = [ + call('/', 'file'), + call(os_mock.path.join.return_value, 'path'), + call(os_mock.path.join.return_value, 'mock'), + ] + + create_subdirectories_for_asset(file_path_mock) + + self.assertListEqual(os_mock.path.join.call_args_list, expected_os_path_join_call_args_list) + os_mock.mkdir.assert_not_called() + + @patch('openedx.features.offline_mode.assets_management.log') + @patch('openedx.features.offline_mode.assets_management.default_storage') + @patch('openedx.features.offline_mode.assets_management.block_storage_path') + def test_clean_outdated_xblock_files_successful( + self, + block_storage_path_mock: MagicMock, + default_storage_mock: MagicMock, + logger_mock: MagicMock, + ) -> None: + xblock_mock = Mock() + default_storage_mock.exists.return_value = True + expected_offline_zip_path = os.path.join( + block_storage_path_mock.return_value, f'{xblock_mock.location.block_id}.zip' + ) + + clean_outdated_xblock_files(xblock_mock) + + block_storage_path_mock.assert_called_once_with(xblock_mock) + default_storage_mock.exists.assert_called_once_with(expected_offline_zip_path) + default_storage_mock.delete.assert_called_once_with(expected_offline_zip_path) + logger_mock.info.assert_called_once_with(f'Successfully deleted the file: {expected_offline_zip_path}') + + @patch('openedx.features.offline_mode.assets_management.log') + @patch('openedx.features.offline_mode.assets_management.default_storage') + @patch('openedx.features.offline_mode.assets_management.block_storage_path') + def test_clean_outdated_xblock_files_does_not_exist( + self, + block_storage_path_mock: MagicMock, + default_storage_mock: MagicMock, + logger_mock: MagicMock, + ) -> None: + xblock_mock = Mock() + default_storage_mock.exists.return_value = False + expected_offline_zip_path = os.path.join( + block_storage_path_mock.return_value, f'{xblock_mock.location.block_id}.zip' + ) + + clean_outdated_xblock_files(xblock_mock) + + block_storage_path_mock.assert_called_once_with(xblock_mock) + default_storage_mock.exists.assert_called_once_with(expected_offline_zip_path) + default_storage_mock.delete.assert_not_called() + logger_mock.info.assert_not_called() + + @patch('openedx.features.offline_mode.assets_management.log.error') + @patch('openedx.features.offline_mode.assets_management.default_storage.exists') + @patch('openedx.features.offline_mode.assets_management.block_storage_path') + def test_remove_old_files_client_error( + self, + block_storage_path_mock: MagicMock, + default_storage_exists_mock: MagicMock, + log_error_mock: MagicMock, + ) -> None: + xblock_mock = Mock() + default_storage_exists_mock.side_effect = ClientError( + operation_name='InvalidKeyPair.Duplicate', error_response={ + 'Error': {'Code': 'Duplicate', 'Message': 'Invalid File Path'} + } + ) + expected_error_message = ( + 'An error occurred (Duplicate) when calling the InvalidKeyPair.Duplicate operation: Invalid File Path' + ) + + clean_outdated_xblock_files(xblock_mock) + block_storage_path_mock.assert_called_once_with(xblock_mock) + log_error_mock.assert_called_once_with( + f'Error occurred while deleting the files or directory: {expected_error_message}' + ) + + @patch('openedx.features.offline_mode.assets_management.default_storage.exists') + @patch('openedx.features.offline_mode.assets_management.os.path.join', return_value='offline_zip_path_mock') + @patch('openedx.features.offline_mode.assets_management.block_storage_path') + def test_get_offline_block_content_path_offline_content_exists( + self, + block_storage_path_mock: MagicMock, + os_path_join_mock: MagicMock, + default_storage_exists_mock: MagicMock, + ) -> None: + xblock_mock = Mock() + + result = get_offline_block_content_path(xblock_mock) + + block_storage_path_mock.assert_called_once_with(usage_key=xblock_mock.location) + os_path_join_mock.assert_called_once_with( + block_storage_path_mock.return_value, f'{xblock_mock.location.block_id}.zip' + ) + default_storage_exists_mock.assert_called_once_with(os_path_join_mock.return_value) + self.assertEqual(result, 'offline_zip_path_mock') + + @patch('openedx.features.offline_mode.assets_management.default_storage.exists', return_value=False) + @patch('openedx.features.offline_mode.assets_management.os.path.join', return_value='offline_zip_path_mock') + @patch('openedx.features.offline_mode.assets_management.block_storage_path') + def test_get_offline_block_content_path_does_not_exist( + self, + block_storage_path_mock: MagicMock, + os_path_join_mock: MagicMock, + default_storage_exists_mock: MagicMock, + ) -> None: + xblock_mock = Mock() + + result = get_offline_block_content_path(xblock_mock) + + block_storage_path_mock.assert_called_once_with(usage_key=xblock_mock.location) + os_path_join_mock.assert_called_once_with( + block_storage_path_mock.return_value, f'{xblock_mock.location.block_id}.zip' + ) + default_storage_exists_mock.assert_called_once_with(os_path_join_mock.return_value) + self.assertEqual(result, None) + + def test_block_storage_path_exists(self) -> None: + xblock_mock = Mock(location=Mock(course_key='course_key_mock')) + + result = block_storage_path(xblock_mock) + + self.assertEqual(result, 'course_key_mock/') + + def test_block_storage_path_does_not_exists(self) -> None: + result = block_storage_path() + + self.assertEqual(result, '') + + @patch( + 'openedx.features.offline_mode.assets_management.default_storage.get_modified_time', + return_value=datetime(2024, 6, 12, tzinfo=UTC) + ) + @patch('openedx.features.offline_mode.assets_management.block_storage_path') + @patch('openedx.features.offline_mode.assets_management.os.path.join') + def test_is_modified_true( + self, + os_path_join_mock: MagicMock, + block_storage_path_mock: MagicMock, + get_created_time_mock: MagicMock, + ) -> None: + xblock_mock = Mock(published_on=datetime(2024, 6, 13, tzinfo=UTC)) + + result = is_modified(xblock_mock) + + os_path_join_mock.assert_called_once_with( + block_storage_path_mock.return_value, f'{xblock_mock.location.block_id}.zip') + get_created_time_mock.assert_called_once_with(os_path_join_mock.return_value) + self.assertEqual(result, True) + + @patch( + 'openedx.features.offline_mode.assets_management.default_storage.get_modified_time', + return_value=datetime(2024, 6, 12, tzinfo=UTC) + ) + @patch('openedx.features.offline_mode.assets_management.block_storage_path') + @patch('openedx.features.offline_mode.assets_management.os.path.join') + def test_is_modified_false( + self, + os_path_join_mock: MagicMock, + block_storage_path_mock: MagicMock, + get_created_time_mock: MagicMock, + ) -> None: + xblock_mock = Mock(published_on=datetime(2024, 6, 1, tzinfo=UTC)) + + result = is_modified(xblock_mock) + + os_path_join_mock.assert_called_once_with( + block_storage_path_mock.return_value, f'{xblock_mock.location.block_id}.zip') + get_created_time_mock.assert_called_once_with(os_path_join_mock.return_value) + self.assertEqual(result, False) + + @patch( + 'openedx.features.offline_mode.assets_management.default_storage.get_modified_time', + side_effect=OSError + ) + @patch('openedx.features.offline_mode.assets_management.block_storage_path') + @patch('openedx.features.offline_mode.assets_management.os.path.join') + def test_is_modified_os_error( + self, + os_path_join_mock: MagicMock, + block_storage_path_mock: MagicMock, + get_created_time_mock: MagicMock, + ) -> None: + xblock_mock = Mock() + + result = is_modified(xblock_mock) + + os_path_join_mock.assert_called_once_with( + block_storage_path_mock.return_value, f'{xblock_mock.location.block_id}.zip') + get_created_time_mock.assert_called_once_with(os_path_join_mock.return_value) + self.assertEqual(result, True) + + @patch('openedx.features.offline_mode.assets_management.log.info') + @patch('openedx.features.offline_mode.assets_management.open') + @patch('openedx.features.offline_mode.assets_management.requests.get') + @patch('openedx.features.offline_mode.assets_management.os') + def test_save_mathjax_to_xblock_assets_successfully( + self, + os_mock: MagicMock, + requests_get_mock: MagicMock, + context_manager_mock: MagicMock, + logger_mock: MagicMock, + ) -> None: + temp_dir_mock = 'temp_dir_mock' + os_mock.path.exists.return_value = False + + save_mathjax_to_xblock_assets(temp_dir_mock) + + os_mock.path.join.assert_called_once_with(temp_dir_mock, MATHJAX_STATIC_PATH) + os_mock.path.exists.assert_called_once_with(os_mock.path.join.return_value) + requests_get_mock.assert_called_once_with(MATHJAX_CDN_URL) + context_manager_mock.assert_called_once_with(os_mock.path.join.return_value, 'wb') + context_manager_mock.return_value.__enter__.return_value.write.assert_called_once_with( + requests_get_mock.return_value.content + ) + logger_mock.assert_called_once_with(f'Successfully saved MathJax to {os_mock.path.join.return_value}') + + @patch('openedx.features.offline_mode.assets_management.log.info') + @patch('openedx.features.offline_mode.assets_management.open') + @patch('openedx.features.offline_mode.assets_management.requests.get') + @patch('openedx.features.offline_mode.assets_management.os') + def test_save_mathjax_to_xblock_assets_already_exists( + self, + os_mock: MagicMock, + requests_get_mock: MagicMock, + context_manager_mock: MagicMock, + logger_mock: MagicMock, + ) -> None: + temp_dir_mock = 'temp_dir_mock' + + save_mathjax_to_xblock_assets(temp_dir_mock) + + os_mock.path.join.assert_called_once_with(temp_dir_mock, MATHJAX_STATIC_PATH) + os_mock.path.exists.assert_called_once_with(os_mock.path.join.return_value) + requests_get_mock.assert_not_called() + context_manager_mock.assert_not_called() + logger_mock.assert_not_called() diff --git a/openedx/features/offline_mode/tests/test_html_manipulator.py b/openedx/features/offline_mode/tests/test_html_manipulator.py new file mode 100644 index 000000000000..cf280900177e --- /dev/null +++ b/openedx/features/offline_mode/tests/test_html_manipulator.py @@ -0,0 +1,163 @@ +""" +Tests for the testing methods for prepare HTML content for offline using. +""" + +from bs4 import BeautifulSoup +from unittest import TestCase +from unittest.mock import MagicMock, Mock, call, patch + +from openedx.features.offline_mode.constants import MATHJAX_CDN_URL, MATHJAX_STATIC_PATH +from openedx.features.offline_mode.html_manipulator import HtmlManipulator + + +class HtmlManipulatorTestCase(TestCase): + """ + Test case for the testing `HtmlManipulator` methods. + """ + + @patch('openedx.features.offline_mode.html_manipulator.HtmlManipulator._replace_iframe') + @patch('openedx.features.offline_mode.html_manipulator.BeautifulSoup', return_value='soup_mock') + @patch('openedx.features.offline_mode.html_manipulator.HtmlManipulator._copy_platform_fonts') + @patch('openedx.features.offline_mode.html_manipulator.HtmlManipulator._replace_external_links') + @patch('openedx.features.offline_mode.html_manipulator.HtmlManipulator._replace_mathjax_link') + @patch('openedx.features.offline_mode.html_manipulator.HtmlManipulator._replace_static_links') + @patch('openedx.features.offline_mode.html_manipulator.HtmlManipulator._replace_asset_links') + def test_process_html( + self, + replace_asset_links_mock: MagicMock, + replace_static_links_mock: MagicMock, + replace_mathjax_link_mock: MagicMock, + replace_external_links: MagicMock, + copy_platform_fonts: MagicMock, + beautiful_soup_mock: MagicMock, + replace_iframe_mock: MagicMock, + ) -> None: + html_data_mock = 'html_data_mock' + xblock_mock = Mock() + temp_dir_mock = 'temp_dir_mock' + html_manipulator = HtmlManipulator(xblock_mock, html_data_mock, temp_dir_mock) + expected_result = 'soup_mock' + + result = html_manipulator.process_html() + + replace_asset_links_mock.assert_called_once_with() + replace_static_links_mock.assert_called_once_with() + replace_mathjax_link_mock.assert_called_once_with() + replace_external_links.assert_called_once_with() + copy_platform_fonts.assert_called_once_with() + beautiful_soup_mock.assert_called_once_with(html_manipulator.html_data, 'html.parser') + replace_iframe_mock.assert_called_once_with(beautiful_soup_mock.return_value) + self.assertEqual(result, expected_result) + + @patch('openedx.features.offline_mode.html_manipulator.save_mathjax_to_xblock_assets') + def test_replace_mathjax_link(self, save_mathjax_to_xblock_assets: MagicMock) -> None: + html_data_mock = f'' + xblock_mock = Mock() + temp_dir_mock = 'temp_dir_mock' + html_manipulator = HtmlManipulator(xblock_mock, html_data_mock, temp_dir_mock) + + expected_html_data_after_replacing = f'' + + self.assertEqual(html_manipulator.html_data, html_data_mock) + + html_manipulator._replace_mathjax_link() # lint-amnesty, pylint: disable=protected-access + + save_mathjax_to_xblock_assets.assert_called_once_with(html_manipulator.temp_dir) + self.assertEqual(html_manipulator.html_data, expected_html_data_after_replacing) + + @patch('openedx.features.offline_mode.html_manipulator.save_asset_file') + def test_replace_static_links(self, save_asset_file_mock: MagicMock) -> None: + html_data_mock = '

' + xblock_mock = Mock() + temp_dir_mock = 'temp_dir_mock' + html_manipulator = HtmlManipulator(xblock_mock, html_data_mock, temp_dir_mock) + + expected_html_data_after_replacing = ( + '
' + ) + + self.assertEqual(html_manipulator.html_data, html_data_mock) + + html_manipulator._replace_static_links() # lint-amnesty, pylint: disable=protected-access + + save_asset_file_mock.assert_called_once_with( + html_manipulator.temp_dir, + html_manipulator.xblock, + '/static/images/professor-sandel.jpg', + 'images/professor-sandel.jpg', + ) + self.assertEqual(html_manipulator.html_data, expected_html_data_after_replacing) + + @patch('openedx.features.offline_mode.html_manipulator.save_asset_file') + def test_replace_asset_links(self, save_asset_file_mock: MagicMock) -> None: + html_data_mock = '/assets/courseware/v1/5b628a18f2ee3303081ffe4d6ab64ee4/asset-v1:OpenedX+DemoX+DemoCourse+type@asset+block/Pendleton_Sinking_Ship.jpeg' # lint-amnesty, pylint: disable=line-too-long + xblock_mock = Mock() + temp_dir_mock = 'temp_dir_mock' + html_manipulator = HtmlManipulator(xblock_mock, html_data_mock, temp_dir_mock) + + expected_html_data_after_replacing = ( + 'assets/courseware/v1/5b628a18f2ee3303081ffe4d6ab64ee4/asset-v1:OpenedX+DemoX+DemoCourse+type@asset+block/Pendleton_Sinking_Ship.jpeg' # lint-amnesty, pylint: disable=line-too-long + ) + + self.assertEqual(html_manipulator.html_data, html_data_mock) + + html_manipulator._replace_asset_links() # lint-amnesty, pylint: disable=protected-access + + save_asset_file_mock.assert_called_once_with( + html_manipulator.temp_dir, + html_manipulator.xblock, + html_data_mock, + expected_html_data_after_replacing, + ) + self.assertEqual(html_manipulator.html_data, expected_html_data_after_replacing) + + def test_replace_iframe(self): + html_data_mock = """ + + """ + soup = BeautifulSoup(html_data_mock, 'html.parser') + expected_html_markup = """

${_('YouTube Video')}

""" + + HtmlManipulator._replace_iframe(soup) # lint-amnesty, pylint: disable=protected-access + + self.assertEqual(f'{soup.find_all("p")[0]}', expected_html_markup) + + @patch('openedx.features.offline_mode.html_manipulator.save_external_file') + def test_replace_external_links(self, save_external_file_mock: MagicMock) -> None: + xblock_mock = Mock() + temp_dir_mock = 'temp_dir_mock' + html_data_mock = """ + Example Image + + """ + + html_manipulator = HtmlManipulator(xblock_mock, html_data_mock, temp_dir_mock) + html_manipulator._replace_external_links() # lint-amnesty, pylint: disable=protected-access + + self.assertEqual(save_external_file_mock.call_count, 2) + + @patch('openedx.features.offline_mode.html_manipulator.uuid.uuid4') + @patch('openedx.features.offline_mode.html_manipulator.save_external_file') + def test_replace_external_link( + self, + save_external_file_mock: MagicMock, + uuid_mock: MagicMock, + ) -> None: + xblock_mock = Mock() + temp_dir_mock = 'temp_dir_mock' + html_data_mock = 'html_data_mock' + external_url_mock = 'https://cdn.example.com/image.jpg' + uuid_result_mock = '123e4567-e89b-12d3-a456-426655440000' + uuid_mock.return_value = uuid_result_mock + mock_match = MagicMock() + mock_match.group.side_effect = [external_url_mock, 'jpg'] + + expected_result = 'assets/external/123e4567-e89b-12d3-a456-426655440000.jpg' + expected_save_external_file_args = [call(temp_dir_mock, external_url_mock, expected_result)] + + html_manipulator = HtmlManipulator(xblock_mock, html_data_mock, temp_dir_mock) + result = html_manipulator._replace_external_link(mock_match) # lint-amnesty, pylint: disable=protected-access + + self.assertListEqual(save_external_file_mock.call_args_list, expected_save_external_file_args) + self.assertEqual(result, expected_result) diff --git a/openedx/features/offline_mode/tests/test_renderer.py b/openedx/features/offline_mode/tests/test_renderer.py new file mode 100644 index 000000000000..b46d75431cf4 --- /dev/null +++ b/openedx/features/offline_mode/tests/test_renderer.py @@ -0,0 +1,31 @@ +""" +Tests for the testing xBlock renderers for Offline Mode. +""" + +from openedx.features.offline_mode.renderer import XBlockRenderer +from openedx.features.offline_mode.tests.base import CourseForOfflineTestCase + + +class XBlockRendererTestCase(CourseForOfflineTestCase): + """ + Test case for the testing `XBlockRenderer`. + """ + + def test_render_xblock_from_lms_html_block(self): + xblock_renderer = XBlockRenderer(str(self.html_block.location), user=self.user) + + result = xblock_renderer.render_xblock_from_lms() + + self.assertIsNotNone(result) + self.assertEqual(type(result), str) + self.assertIn('HTML xblock for Offline', result) + self.assertIn('

Test HTML Content

', result) + + def test_render_xblock_from_lms_problem_block(self): + xblock_renderer = XBlockRenderer(str(self.problem_block.location), user=self.user) + + result = xblock_renderer.render_xblock_from_lms() + + self.assertIsNotNone(result) + self.assertEqual(type(result), str) + self.assertIn('Problem xblock for Offline', result) diff --git a/openedx/features/offline_mode/tests/test_storage_management.py b/openedx/features/offline_mode/tests/test_storage_management.py new file mode 100644 index 000000000000..f77f2ba614cd --- /dev/null +++ b/openedx/features/offline_mode/tests/test_storage_management.py @@ -0,0 +1,268 @@ +""" +Tests for the testing Offline Mode storage management. +""" + +import os +import shutil +from unittest import TestCase +from unittest.mock import MagicMock, Mock, call, patch + +from django.http.response import Http404 + +from openedx.features.offline_mode.constants import MATHJAX_STATIC_PATH +from openedx.features.offline_mode.storage_management import OfflineContentGenerator +from openedx.features.offline_mode.tests.base import CourseForOfflineTestCase + + +class OfflineContentGeneratorTestCase(TestCase): + """ + Test case for the testing Offline Mode utils. + """ + @patch('openedx.features.offline_mode.storage_management.XBlockRenderer') + def test_render_block_html_data_successful(self, xblock_renderer_mock: MagicMock) -> None: + xblock_mock = Mock() + html_data_mock = 'html_markup_data_mock' + + result = OfflineContentGenerator(xblock_mock, html_data_mock).render_block_html_data() + + xblock_renderer_mock.assert_called_once_with(str(xblock_mock.location)) + xblock_renderer_mock.return_value.render_xblock_from_lms.assert_called_once_with() + self.assertEqual(result, xblock_renderer_mock.return_value.render_xblock_from_lms.return_value) + + @patch('openedx.features.offline_mode.storage_management.XBlockRenderer') + def test_render_block_html_data_successful_no_html_data(self, xblock_renderer_mock: MagicMock) -> None: + xblock_mock = Mock() + expected_xblock_renderer_args_list = [call(str(xblock_mock.location)), call(str(xblock_mock.location))] + + result = OfflineContentGenerator(xblock_mock).render_block_html_data() + + self.assertListEqual(xblock_renderer_mock.call_args_list, expected_xblock_renderer_args_list) + self.assertListEqual( + xblock_renderer_mock.return_value.render_xblock_from_lms.call_args_list, [call(), call()] + ) + self.assertEqual(result, xblock_renderer_mock.return_value.render_xblock_from_lms.return_value) + + @patch('openedx.features.offline_mode.storage_management.log.error') + @patch('openedx.features.offline_mode.storage_management.XBlockRenderer', side_effect=Http404) + def test_render_block_html_data_http404( + self, + xblock_renderer_mock: MagicMock, + logger_mock: MagicMock, + ) -> None: + xblock_mock = Mock() + html_data_mock = 'html_markup_data_mock' + + with self.assertRaises(Http404): + OfflineContentGenerator(xblock_mock, html_data_mock).render_block_html_data() + + xblock_renderer_mock.assert_called_once_with(str(xblock_mock.location)) + logger_mock.assert_called_once_with( + f'Block {str(xblock_mock.location)} cannot be fetched from course' + f' {xblock_mock.location.course_key} during offline content generation.' + ) + + @patch('openedx.features.offline_mode.storage_management.shutil.rmtree') + @patch('openedx.features.offline_mode.storage_management.OfflineContentGenerator.create_zip_file') + @patch('openedx.features.offline_mode.storage_management.OfflineContentGenerator.save_xblock_html') + @patch('openedx.features.offline_mode.storage_management.mkdtemp') + @patch('openedx.features.offline_mode.storage_management.clean_outdated_xblock_files') + @patch('openedx.features.offline_mode.storage_management.block_storage_path') + def test_generate_offline_content_for_modified_xblock( + self, + block_storage_path_mock: MagicMock, + clean_outdated_xblock_files_mock: MagicMock, + mkdtemp_mock: MagicMock, + save_xblock_html_mock: MagicMock, + create_zip_file_mock: MagicMock, + shutil_rmtree_mock: MagicMock, + ) -> None: + xblock_mock = Mock() + html_data_mock = 'html_markup_data_mock' + + OfflineContentGenerator(xblock_mock, html_data_mock).generate_offline_content() + + block_storage_path_mock.assert_called_once_with(xblock_mock) + clean_outdated_xblock_files_mock.assert_called_once_with(xblock_mock) + mkdtemp_mock.assert_called_once_with() + save_xblock_html_mock.assert_called_once_with(mkdtemp_mock.return_value) + create_zip_file_mock.assert_called_once_with( + mkdtemp_mock.return_value, + block_storage_path_mock.return_value, + f'{xblock_mock.location.block_id}.zip' + ) + shutil_rmtree_mock.assert_called_once_with(mkdtemp_mock.return_value, ignore_errors=True) + + @patch('openedx.features.offline_mode.storage_management.os.path.join') + @patch('openedx.features.offline_mode.storage_management.open') + @patch('openedx.features.offline_mode.storage_management.HtmlManipulator') + def test_save_xblock_html( + self, + html_manipulator_mock: MagicMock, + context_manager_mock: MagicMock, + os_path_join_mock: MagicMock, + ) -> None: + tmp_dir_mock = Mock() + xblock_mock = Mock() + html_data_mock = 'html_markup_data_mock' + + OfflineContentGenerator(xblock_mock, html_data_mock).save_xblock_html(tmp_dir_mock) + + html_manipulator_mock.assert_called_once_with(xblock_mock, html_data_mock, tmp_dir_mock) + html_manipulator_mock.return_value.process_html.assert_called_once_with() + context_manager_mock.assert_called_once_with(os_path_join_mock.return_value, 'w') + os_path_join_mock.assert_called_once_with(tmp_dir_mock, 'index.html') + context_manager_mock.return_value.__enter__.return_value.write.assert_called_once_with( + html_manipulator_mock.return_value.process_html.return_value + ) + + @patch('openedx.features.offline_mode.storage_management.log.info') + @patch('openedx.features.offline_mode.storage_management.ContentFile') + @patch('openedx.features.offline_mode.storage_management.open') + @patch('openedx.features.offline_mode.storage_management.get_storage') + @patch('openedx.features.offline_mode.storage_management.OfflineContentGenerator.add_files_to_zip_recursively') + @patch('openedx.features.offline_mode.storage_management.ZipFile') + def test_create_zip_file( + self, + zip_file_context_manager: MagicMock, + add_files_to_zip_recursively_mock: MagicMock, + storage_mock: MagicMock, + open_context_manager_mock: MagicMock, + content_file_mock: MagicMock, + log_info_mock: MagicMock, + ) -> None: + xblock_mock = Mock() + html_data_mock = 'html_markup_data_mock' + temp_dir_mock = 'temp_dir_mock' + base_path_mock = 'base_path_mock' + file_name_mock = 'file_name_mock' + + OfflineContentGenerator(xblock_mock, html_data_mock).create_zip_file( + temp_dir_mock, base_path_mock, file_name_mock + ) + + zip_file_context_manager.assert_called_once_with(os.path.join(temp_dir_mock, file_name_mock), 'w') + zip_file_context_manager.return_value.__enter__.return_value.write.assert_called_once_with( + os.path.join(temp_dir_mock, 'index.html'), 'index.html' + ) + add_files_to_zip_recursively_mock.assert_called_once_with( + zip_file_context_manager.return_value.__enter__.return_value, + current_base_path=os.path.join(temp_dir_mock, 'assets'), + current_path_in_zip='assets', + ) + open_context_manager_mock.assert_called_once_with(os.path.join(temp_dir_mock, file_name_mock), 'rb') + content_file_mock.assert_called_once_with( + open_context_manager_mock.return_value.__enter__.return_value.read.return_value + ) + storage_mock.return_value.save.assert_called_once_with( + os.path.join(base_path_mock + file_name_mock), content_file_mock.return_value + ) + log_info_mock.assert_called_once_with( + f'Offline content for {file_name_mock} has been generated.' + ) + + @patch('openedx.features.offline_mode.storage_management.os') + def test_add_files_to_zip_recursively_successfully_for_file( + self, + os_mock: MagicMock, + ) -> None: + xblock_mock = Mock() + html_data_mock = 'html_markup_data_mock' + zip_file_mock = Mock() + current_base_path_mock = 'current_base_path_mock' + current_path_in_zip_mock = 'current_path_in_zip_mock' + resource_path_mock = 'resource_path_mock' + os_mock.listdir.return_value = [resource_path_mock] + + expected_os_mock_path_join_calls = [ + call(current_base_path_mock, resource_path_mock), + call(current_path_in_zip_mock, resource_path_mock) + ] + + OfflineContentGenerator(xblock_mock, html_data_mock).add_files_to_zip_recursively( + zip_file_mock, current_base_path_mock, current_path_in_zip_mock + ) + + os_mock.listdir.assert_called_once_with(current_base_path_mock) + self.assertListEqual(os_mock.path.join.call_args_list, expected_os_mock_path_join_calls) + zip_file_mock.write.assert_called_once_with(os_mock.path.join.return_value, os_mock.path.join.return_value) + + @patch('openedx.features.offline_mode.storage_management.OfflineContentGenerator.add_files_to_zip_recursively') + @patch('openedx.features.offline_mode.storage_management.os.listdir') + def test_add_files_to_zip_recursively_successfully_recursively_path( + self, + os_listdir_mock: MagicMock, + add_files_to_zip_recursively_mock: MagicMock, + ) -> None: + xblock_mock = Mock() + html_data_mock = 'html_markup_data_mock' + zip_file_mock = Mock() + current_base_path_mock = 'current_base_path_mock' + current_path_in_zip_mock = 'current_path_in_zip_mock' + resource_path_mock = 'resource_path_mock' + os_listdir_mock.listdir.return_value = [resource_path_mock] + + OfflineContentGenerator(xblock_mock, html_data_mock).add_files_to_zip_recursively( + zip_file_mock, current_base_path_mock, current_path_in_zip_mock + ) + + add_files_to_zip_recursively_mock.assert_called_once_with( + zip_file_mock, current_base_path_mock, current_path_in_zip_mock + ) + + @patch('openedx.features.offline_mode.storage_management.log.error') + @patch('openedx.features.offline_mode.storage_management.os.listdir', side_effect=OSError) + def test_add_files_to_zip_recursively_with_os_error( + self, + os_mock: MagicMock, + log_error_mock: MagicMock, + ) -> None: + xblock_mock = Mock() + html_data_mock = 'html_markup_data_mock' + zip_file_mock = Mock() + current_base_path_mock = 'current_base_path_mock' + current_path_in_zip_mock = 'current_path_in_zip_mock' + + OfflineContentGenerator(xblock_mock, html_data_mock).add_files_to_zip_recursively( + zip_file_mock, current_base_path_mock, current_path_in_zip_mock + ) + + os_mock.assert_called_once_with(current_base_path_mock) + log_error_mock.assert_called_once_with(f'Error while reading the directory: {current_base_path_mock}') + + +class OfflineContentGeneratorFunctionalTestCase(CourseForOfflineTestCase): + """ + Tests creating Offline Content in storage. + """ + + def setUp(self): + super().setUp() + self.html_data = '

Test HTML Content

' # lint-amnesty, pylint: disable=attribute-defined-outside-init + + @patch('openedx.features.offline_mode.html_manipulator.save_mathjax_to_xblock_assets') + def test_generate_offline_content(self, save_mathjax_to_xblock_assets_mock): + OfflineContentGenerator(self.html_block, self.html_data).generate_offline_content() + + expected_offline_content_path = 'test_root/uploads/course-v1:RaccoonGang+1+2024/HTML_xblock_for_Offline.zip' + + save_mathjax_to_xblock_assets_mock.assert_called_once() + self.assertTrue(os.path.exists(expected_offline_content_path)) + shutil.rmtree('test_root/uploads/course-v1:RaccoonGang+1+2024', ignore_errors=True) + + def test_save_xblock_html_to_temp_dir(self): + shutil.rmtree('test_root/assets', ignore_errors=True) + temp_dir = 'test_root/' + os.makedirs('test_root/assets/js/') + OfflineContentGenerator(self.html_block, self.html_data).save_xblock_html(temp_dir) + + expected_index_html_path = 'test_root/index.html' + expected_mathjax_static_path = os.path.join(temp_dir, MATHJAX_STATIC_PATH) + + self.assertTrue(os.path.exists(expected_index_html_path)) + self.assertTrue(os.path.exists(expected_mathjax_static_path)) + with open(expected_index_html_path, 'r') as content: + html_data = content.read() + self.assertIn(self.html_data, html_data) + + shutil.rmtree('test_root/assets', ignore_errors=True) + os.remove(expected_index_html_path) diff --git a/openedx/features/offline_mode/tests/test_tasks.py b/openedx/features/offline_mode/tests/test_tasks.py new file mode 100644 index 000000000000..ef468d825d7b --- /dev/null +++ b/openedx/features/offline_mode/tests/test_tasks.py @@ -0,0 +1,141 @@ +""" +Tests for the testing Offline Mode tacks. +""" + +from unittest import TestCase +from unittest.mock import MagicMock, Mock, call, patch + +from ddt import data, ddt, unpack +from django.http.response import Http404 +from opaque_keys.edx.keys import CourseKey, UsageKey +from openedx.features.offline_mode.constants import MAX_RETRY_ATTEMPTS, OFFLINE_SUPPORTED_XBLOCKS +from openedx.features.offline_mode.tasks import ( + generate_offline_content_for_block, + generate_offline_content_for_course, +) + + +@ddt +class GenerateOfflineContentTasksTestCase(TestCase): + """ + Test case for the testing generating offline content tacks. + """ + + @patch('openedx.features.offline_mode.tasks.OfflineContentGenerator') + @patch('openedx.features.offline_mode.tasks.modulestore') + def test_generate_offline_content_for_block_success( + self, + modulestore_mock: MagicMock, + offline_content_generator_mock: MagicMock, + ) -> None: + block_id_mock = 'block-v1:a+a+a+type@problem+block@fb81e4dbfd4945cb9318d6bc460a956c' + + generate_offline_content_for_block(block_id_mock) + + modulestore_mock.assert_called_once_with() + modulestore_mock.return_value.get_item.assert_called_once_with(UsageKey.from_string(block_id_mock)) + offline_content_generator_mock.assert_called_once_with(modulestore_mock.return_value.get_item.return_value) + offline_content_generator_mock.return_value.generate_offline_content.assert_called_once_with() + + @patch('openedx.features.offline_mode.tasks.OfflineContentGenerator') + @patch('openedx.features.offline_mode.tasks.modulestore', side_effect=Http404) + def test_generate_offline_content_for_block_with_exception_in_modulestore( + self, + modulestore_mock: MagicMock, + offline_content_generator_mock: MagicMock, + ) -> None: + block_id_mock = 'block-v1:a+a+a+type@problem+block@fb81e4dbfd4945cb9318d6bc460a956c' + + generate_offline_content_for_block.delay(block_id_mock) + + self.assertEqual(modulestore_mock.call_count, MAX_RETRY_ATTEMPTS + 1) + offline_content_generator_mock.assert_not_called() + + @patch('openedx.features.offline_mode.tasks.OfflineContentGenerator', side_effect=Http404) + @patch('openedx.features.offline_mode.tasks.modulestore') + def test_generate_offline_content_for_block_with_exception_in_offline_content_generation( + self, + modulestore_mock: MagicMock, + offline_content_generator_mock: MagicMock, + ) -> None: + block_id_mock = 'block-v1:a+a+a+type@problem+block@fb81e4dbfd4945cb9318d6bc460a956c' + + generate_offline_content_for_block.delay(block_id_mock) + + self.assertEqual(modulestore_mock.call_count, MAX_RETRY_ATTEMPTS + 1) + self.assertEqual(offline_content_generator_mock.call_count, MAX_RETRY_ATTEMPTS + 1) + + @patch('openedx.features.offline_mode.tasks.generate_offline_content_for_block') + @patch('openedx.features.offline_mode.tasks.is_modified') + @patch('openedx.features.offline_mode.tasks.modulestore') + def test_generate_offline_content_for_course_supported_block_types( + self, + modulestore_mock: MagicMock, + is_modified_mock: MagicMock, + generate_offline_content_for_block_mock: MagicMock, + ) -> None: + course_id_mock = 'course-v1:a+a+a' + xblock_location_mock = 'xblock_location_mock' + modulestore_mock.return_value.get_items.return_value = [ + Mock(location=xblock_location_mock, closed=Mock(return_value=False)) + ] + + expected_call_args_for_modulestore_get_items = [ + call(CourseKey.from_string(course_id_mock), qualifiers={'category': offline_supported_block_type}) + for offline_supported_block_type in OFFLINE_SUPPORTED_XBLOCKS + ] + expected_call_args_is_modified_mock = [ + call(modulestore_mock.return_value.get_items.return_value[0]) for _ in OFFLINE_SUPPORTED_XBLOCKS + ] + expected_call_args_for_generate_offline_content_for_block_mock = [ + call([xblock_location_mock]) for _ in OFFLINE_SUPPORTED_XBLOCKS + ] + + generate_offline_content_for_course(course_id_mock) + + self.assertEqual(modulestore_mock.call_count, len(OFFLINE_SUPPORTED_XBLOCKS)) + self.assertListEqual( + modulestore_mock.return_value.get_items.call_args_list, expected_call_args_for_modulestore_get_items + ) + self.assertListEqual(is_modified_mock.call_args_list, expected_call_args_is_modified_mock) + self.assertListEqual( + generate_offline_content_for_block_mock.apply_async.call_args_list, + expected_call_args_for_generate_offline_content_for_block_mock + ) + + @patch('openedx.features.offline_mode.tasks.generate_offline_content_for_block') + @patch('openedx.features.offline_mode.tasks.is_modified') + @patch('openedx.features.offline_mode.tasks.modulestore') + @data( + (False, False), + (True, False), + (False, True), + ) + @unpack + def test_generate_offline_content_for_course_supported_block_types_for_closed_or_not_modified_xblock( + self, + is_modified_value_mock: bool, + is_closed_value_mock: bool, + modulestore_mock: MagicMock, + is_modified_mock: MagicMock, + generate_offline_content_for_block_mock: MagicMock, + ) -> None: + course_id_mock = 'course-v1:a+a+a' + xblock_location_mock = 'xblock_location_mock' + modulestore_mock.return_value.get_items.return_value = [ + Mock(location=xblock_location_mock, closed=Mock(return_value=is_closed_value_mock)) + ] + is_modified_mock.return_value = is_modified_value_mock + + expected_call_args_for_modulestore_get_items = [ + call(CourseKey.from_string(course_id_mock), qualifiers={'category': offline_supported_block_type}) + for offline_supported_block_type in OFFLINE_SUPPORTED_XBLOCKS + ] + + generate_offline_content_for_course(course_id_mock) + + self.assertEqual(modulestore_mock.call_count, len(OFFLINE_SUPPORTED_XBLOCKS)) + self.assertListEqual( + modulestore_mock.return_value.get_items.call_args_list, expected_call_args_for_modulestore_get_items + ) + generate_offline_content_for_block_mock.assert_not_called() From 24131535db10f437b34143d49f75a7c86c19750f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=86=D0=B2=D0=B0=D0=BD=20=D0=9D=D1=94=D0=B4=D1=94=D0=BB?= =?UTF-8?q?=D1=8C=D0=BD=D1=96=D1=86=D0=B5=D0=B2?= Date: Thu, 21 Nov 2024 13:29:01 +0200 Subject: [PATCH 11/16] fix: encode spaces as %20 --- lms/static/js/courseware/bridge.js | 46 +++++++++++++++++++++++------- 1 file changed, 35 insertions(+), 11 deletions(-) diff --git a/lms/static/js/courseware/bridge.js b/lms/static/js/courseware/bridge.js index 03efb52e39b6..33d22f2999d1 100644 --- a/lms/static/js/courseware/bridge.js +++ b/lms/static/js/courseware/bridge.js @@ -10,21 +10,33 @@ */ /** - * Sends a data about student's answer to the IOS app. - * - * @param {string} message The stringified JSON object to be sent to the IOS app + * Sends a JSON-formatted message to the iOS bridge if available. + * @param {string} message - The JSON message to send. */ function sendMessageToIOS(message) { - window?.webkit?.messageHandlers?.IOSBridge?.postMessage(message); + try { + if (window?.webkit?.messageHandlers?.IOSBridge) { + window.webkit.messageHandlers.IOSBridge.postMessage(message); + console.log("Message sent to iOS:", message); + } + } catch (error) { + console.error("Failed to send message to iOS:", error); + } } /** - * Sends a data about student's answer to the Android app. - * - * @param {string} message The stringified JSON object to be sent to the native Android app + * Sends a JSON-formatted message to the Android bridge if available. + * @param {string} message - The JSON message to send. */ function sendMessageToAndroid(message) { - window?.AndroidBridge?.postMessage(message); + try { + if (window?.AndroidBridge) { + window.AndroidBridge.postMessage(message); + console.log("Message sent to Android:", message); + } + } catch (error) { + console.error("Failed to send message to Android:", error); + } } /** @@ -34,7 +46,13 @@ function sendMessageToAndroid(message) { * @param {string} message The stringified JSON object about the student's answer from the native mobile app. */ function markProblemCompleted(message) { - const data = JSON.parse(message).data; + let data; + try { + data = JSON.parse(message).data + } catch (error) { + console.error("Failed to parse message:", error) + return + } const problemContainer = $(".xblock-student_view"); const submitButton = problemContainer.find(".submit-attempt-container .submit"); @@ -69,8 +87,14 @@ function markProblemCompleted(message) { const originalAjax = $.ajax; $.ajax = function (options) { if (options.url && options.url.endsWith("handler/xmodule_handler/problem_check")) { - sendMessageToIOS(JSON.stringify(options)); - sendMessageToAndroid(JSON.stringify(options)); + if (options.data) { + // Replace spaces with URLEncoded value to ensure correct parsing on the backend + let formattedData = options.data.replace(/\+/g, '%20'); + let jsonMessage = JSON.stringify(formattedData) + + sendMessageToIOS(jsonMessage) + sendMessageToAndroid(jsonMessage) + } } return originalAjax.call(this, options); } From 3033a6b64d5d46d1d9cc054b81aa993eb16fe188 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=86=D0=B2=D0=B0=D0=BD=20=D0=9D=D1=94=D0=B4=D1=94=D0=BB?= =?UTF-8?q?=D1=8C=D0=BD=D1=96=D1=86=D0=B5=D0=B2?= Date: Thu, 21 Nov 2024 13:29:34 +0200 Subject: [PATCH 12/16] refactor: update path to store blocks --- openedx/features/offline_mode/assets_management.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openedx/features/offline_mode/assets_management.py b/openedx/features/offline_mode/assets_management.py index a8845935fcfa..d4adf2e6a1bb 100644 --- a/openedx/features/offline_mode/assets_management.py +++ b/openedx/features/offline_mode/assets_management.py @@ -127,7 +127,7 @@ def block_storage_path(xblock=None, usage_key=None): str: The constructed base storage path. """ loc = usage_key or getattr(xblock, 'location', None) - return f'{str(loc.course_key)}/' if loc else '' + return f'offline_content/{str(loc.course_key)}/' if loc else '' def is_modified(xblock): From c82412eb272f1f6dc68a43f409fcdb588707678f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=86=D0=B2=D0=B0=D0=BD=20=D0=9D=D1=94=D0=B4=D1=94=D0=BB?= =?UTF-8?q?=D1=8C=D0=BD=D1=96=D1=86=D0=B5=D0=B2?= Date: Thu, 21 Nov 2024 13:30:20 +0200 Subject: [PATCH 13/16] fix: provide a file URL by using url method --- lms/djangoapps/mobile_api/course_info/views.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lms/djangoapps/mobile_api/course_info/views.py b/lms/djangoapps/mobile_api/course_info/views.py index b046fb81d907..cce2d661f3cf 100644 --- a/lms/djangoapps/mobile_api/course_info/views.py +++ b/lms/djangoapps/mobile_api/course_info/views.py @@ -425,10 +425,9 @@ def _extend_block_info_with_offline_data(blocks_info_data: Dict[str, Dict]) -> N """ for block_id, block_info in blocks_info_data.items(): if offline_content_path := get_offline_block_content_path(usage_key=UsageKey.from_string(block_id)): - file_url = os.path.join(settings.MEDIA_URL, offline_content_path) block_info.update({ 'offline_download': { - 'file_url': file_url, + 'file_url': default_storage.url(offline_content_path), 'last_modified': default_storage.get_modified_time(offline_content_path), 'file_size': default_storage.size(offline_content_path) } From 8fe54a3c79fa8f98b6cbb8e9ad06c352bce4df8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=86=D0=B2=D0=B0=D0=BD=20=D0=9D=D1=94=D0=B4=D1=94=D0=BB?= =?UTF-8?q?=D1=8C=D0=BD=D1=96=D1=86=D0=B5=D0=B2?= Date: Thu, 21 Nov 2024 13:58:44 +0200 Subject: [PATCH 14/16] style: remote extra import --- openedx/features/offline_mode/tasks.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openedx/features/offline_mode/tasks.py b/openedx/features/offline_mode/tasks.py index 6ec13454edf0..0fe4de0f51cd 100644 --- a/openedx/features/offline_mode/tasks.py +++ b/openedx/features/offline_mode/tasks.py @@ -10,7 +10,6 @@ from .assets_management import is_modified from .constants import MAX_RETRY_ATTEMPTS, OFFLINE_SUPPORTED_XBLOCKS, RETRY_BACKOFF_INITIAL_TIMEOUT -from .renderer import XBlockRenderer from .storage_management import OfflineContentGenerator @@ -34,7 +33,8 @@ def generate_offline_content_for_course(course_id): autoretry_for=(Http404,), retry_backoff=RETRY_BACKOFF_INITIAL_TIMEOUT, retry_kwargs={'max_retries': MAX_RETRY_ATTEMPTS} -)@set_code_owner_attribute +) +@set_code_owner_attribute def generate_offline_content_for_block(block_id): """ Generates offline content for the specified block. From ad379026ba89c4735cf6645a3fb6e179130a4ec1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=86=D0=B2=D0=B0=D0=BD=20=D0=9D=D1=94=D0=B4=D1=94=D0=BB?= =?UTF-8?q?=D1=8C=D0=BD=D1=96=D1=86=D0=B5=D0=B2?= Date: Fri, 29 Nov 2024 15:11:10 +0200 Subject: [PATCH 15/16] style: fix code style issues --- lms/djangoapps/mobile_api/course_info/views.py | 3 --- lms/djangoapps/mobile_api/tests/test_course_info_views.py | 4 ++-- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/lms/djangoapps/mobile_api/course_info/views.py b/lms/djangoapps/mobile_api/course_info/views.py index cce2d661f3cf..62b4713a2db1 100644 --- a/lms/djangoapps/mobile_api/course_info/views.py +++ b/lms/djangoapps/mobile_api/course_info/views.py @@ -1,13 +1,10 @@ """ Views for course info API """ - -import os import logging from typing import Dict, Optional, Union import django -from django.conf import settings from django.contrib.auth import get_user_model from django.core.files.storage import default_storage from opaque_keys import InvalidKeyError 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 eaebac0f27c8..979f7df24d2c 100644 --- a/lms/djangoapps/mobile_api/tests/test_course_info_views.py +++ b/lms/djangoapps/mobile_api/tests/test_course_info_views.py @@ -17,10 +17,10 @@ from common.djangoapps.student.tests.factories import UserFactory # pylint: disable=unused-import from common.djangoapps.util.course import get_link_for_about_page -from lms.djangoapps.mobile_api.utils import API_V05, API_V1, API_V2, API_V3, API_V4 +from lms.djangoapps.course_api.blocks.tests.test_views import TestBlocksInCourseView from lms.djangoapps.mobile_api.course_info.views import BlocksInfoInCourseView from lms.djangoapps.mobile_api.testutils import MobileAPITestCase, MobileAuthTestMixin, MobileCourseAccessTestMixin -from lms.djangoapps.mobile_api.utils import API_V1, API_V05 +from lms.djangoapps.mobile_api.utils import API_V05, API_V1, API_V2, API_V3, API_V4 from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.features.course_experience import ENABLE_COURSE_GOALS from openedx.features.offline_mode.constants import DEFAULT_OFFLINE_SUPPORTED_XBLOCKS From 00e2cd559956fe76d11ec1e71328e5719c2965db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=86=D0=B2=D0=B0=D0=BD=20=D0=9D=D1=94=D0=B4=D1=94=D0=BB?= =?UTF-8?q?=D1=8C=D0=BD=D1=96=D1=86=D0=B5=D0=B2?= Date: Wed, 4 Dec 2024 14:29:55 +0200 Subject: [PATCH 16/16] fix: add js bridge to xblock html --- lms/templates/courseware/courseware-chromeless.html | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lms/templates/courseware/courseware-chromeless.html b/lms/templates/courseware/courseware-chromeless.html index deeda26c431d..ff961ac716bf 100644 --- a/lms/templates/courseware/courseware-chromeless.html +++ b/lms/templates/courseware/courseware-chromeless.html @@ -26,6 +26,9 @@ <%static:include path="common/templates/${template_name}.underscore" /> % endfor +% if is_offline_content: + +% endif <% header_file = None %>