style: [AXM-349] fix style issues

openedx/features/offline_mode/assets_management.py

@@ -1,3 +1,6 @@
+"""
+This module contains utility functions for managing assets and files.
+"""
 import shutil
 import logging
 import os
@@ -12,7 +15,7 @@
 from xmodule.exceptions import NotFoundError
 from xmodule.modulestore.exceptions import ItemNotFoundError
 
-from .constants import MATHJAX_CDN_URL, MATHJAX_STATIC_PATH
+from .constants import MATHJAX_CDN_URL, MATHJAX_STATIC_PATH, OFFLINE_CONTENT_ARCHIVE_NAME
 
 
 log = logging.getLogger(__name__)
@@ -37,15 +40,14 @@ def read_static_file(path):
 def save_asset_file(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:
         xblock (XBlock): The XBlock instance
         path (str): The path where the asset is located.
         filename (str): The name of the file to be saved.
     """
-    if filename.endswith('djangojs.js'):
-        return
     try:
         if '/' in filename:
             static_path = get_static_file_path(filename)
@@ -70,77 +72,64 @@ def remove_old_files(xblock):
     """
     try:
         base_path = base_storage_path(xblock)
-
-        # Define the paths to the specific items to delete
         asset_path = os.path.join(base_path, 'asset')
         index_file_path = os.path.join(base_path, 'index.html')
-        # FIXME: change filename to block_id or move to constants
-        offline_zip_path = os.path.join(base_path, 'offline_content.zip')
+        offline_zip_path = os.path.join(base_path, OFFLINE_CONTENT_ARCHIVE_NAME)
 
         # Delete the 'asset' directory if it exists
         if os.path.isdir(asset_path):
             shutil.rmtree(asset_path)
             log.info(f"Successfully deleted the directory: {asset_path}")
 
         # Delete the 'index.html' file if it exists
-        if os.path.isfile(index_file_path):
+        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 os.path.isfile(offline_zip_path):
+        if default_storage.exists(offline_zip_path):
             os.remove(offline_zip_path)
             log.info(f"Successfully deleted the file: {offline_zip_path}")
 
-    except Exception as e:
+    except OSError as e:
         log.error(f"Error occurred while deleting the files or directory: {e}")
 
 
 def is_offline_content_present(xblock):
     """
     Checks whether 'offline_content.zip' file is present in the specified base path directory.
+
     Args:
         xblock (XBlock): The XBlock instance
     Returns:
         bool: True if the file is present, False otherwise
     """
-    try:
-        base_path = base_storage_path(xblock)
-        # FIXME: change filename to block_id or move to constants
-        # Define the path to the 'offline_content.zip' file
-        offline_zip_path = os.path.join(base_path, 'offline_content.zip')
-
-        # Check if the file exists
-        if os.path.isfile(offline_zip_path):
-            return True
-        else:
-            return False
-
-    except Exception as e:
-        log.error(f"Error occurred while checking the file: {e}")
-        return False
+    base_path = base_storage_path(xblock)
+    offline_zip_path = os.path.join(base_path, OFFLINE_CONTENT_ARCHIVE_NAME)
+    return default_storage.exists(offline_zip_path)
 
 
 def base_storage_path(xblock):
     """
     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.
     Returns:
         str: The constructed base storage path.
     """
-    # FIXME: change to os.path.join?
     loc = xblock.location
     return f'{loc.org}/{loc.course}/{loc.block_type}/{loc.block_id}/'
 
 
 def is_modified(xblock):
     """
     Check if the xblock has been modified since the last time the offline content was generated.
-    :param xblock:
-    :return:
+
+    Args:
+        xblock (XBlock): The XBlock instance to check.
     """
     file_path = os.path.join(base_storage_path(xblock), 'content_html.zip')
 
@@ -153,6 +142,12 @@ def is_modified(xblock):
 
 
 def save_mathjax_to_local_static():
+    """
+    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.
+    """
     if not default_storage.exists(MATHJAX_STATIC_PATH):
         response = requests.get(MATHJAX_CDN_URL)
         default_storage.save(MATHJAX_STATIC_PATH, ContentFile(response.content))

openedx/features/offline_mode/constants.py

@@ -1,5 +1,15 @@
+"""
+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('offline_mode_shared_static', 'js', f'MathJax-{MATHJAX_VERSION}.js')
+
+OFFLINE_CONTENT_ARCHIVE_NAME = 'offline_content.zip'
+
+DEFAULT_OFFLINE_SUPPORTED_XBLOCKS = ['html', 'problem']
+OFFLINE_SUPPORTED_XBLOCKS = getattr(settings, 'OFFLINE_SUPPORTED_XBLOCKS', DEFAULT_OFFLINE_SUPPORTED_XBLOCKS)

openedx/features/offline_mode/html_manipulator.py

@@ -1,3 +1,6 @@
+"""
+Module to prepare HTML content for offline use.
+"""
 import os
 import re
 
@@ -8,14 +11,16 @@
 from .assets_management import save_asset_file, save_mathjax_to_local_static
 from .constants import MATHJAX_CDN_URL, MATHJAX_STATIC_PATH
 
+
+# Relative path difference between the HTML content and the shared static files.
 RELATIVE_PATH_DIFF = '../../../../'
 
 
 class HtmlManipulator:
     """
-    Class to manipulate the HTML content of an XBlock.
-
+    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):
@@ -36,7 +41,7 @@ def _replace_static_links(self):
         """
         Replace static links with local links.
         """
-        static_links_pattern = os.path.join(settings.STATIC_URL, '[\w./-]+')
+        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)
 
@@ -70,41 +75,8 @@ def _add_js_bridge(soup):
         :return:
         """
         script_tag = soup.new_tag('script')
-        # FIXME: this script should be loaded from a file
-        script_tag.string = """
-        // JS bridge script
-        function sendMessageToiOS(message) {
-            window?.webkit?.messageHandlers?.iOSBridge?.postMessage(message);
-        }
-        function sendMessageToAndroid(message) {
-            window?.AndroidBridge?.postMessage(message);
-        }
-        function receiveMessageFromiOS(message) {
-            console.log("Message received from iOS:", message);
-        }
-        function receiveMessageFromAndroid(message) {
-            console.log("Message received from Android:", message);
-        }
-        if (window.webkit && window.webkit.messageHandlers && window.webkit.messageHandlers.iOSBridge) {
-            window.addEventListener("messageFromiOS", function(event) {
-                receiveMessageFromiOS(event.data);
-            });
-        }
-        if (window.AndroidBridge) {
-            window.addEventListener("messageFromAndroid", function(event) {
-                receiveMessageFromAndroid(event.data);
-            });
-        }
-        const originalAjax = $.ajax;
-        $.ajax = function(options) {
-            sendMessageToiOS(options);
-            sendMessageToiOS(JSON.stringify(options));
-            sendMessageToAndroid(options);
-            sendMessageToAndroid(JSON.stringify(options));
-            console.log(options, JSON.stringify(options));
-            return originalAjax.call(this, options);
-        };
-        """
+        with open('openedx/features/offline_mode/static/offline_mode/js/bridge.js', 'r') as file:
+            script_tag.string = file.read()
         if soup.body:
             soup.body.append(script_tag)
         else:

openedx/features/offline_mode/renderer.py

@@ -9,7 +9,7 @@
 from django.contrib.sessions.backends.db import SessionStore
 from django.http import HttpRequest
 
-from opaque_keys.edx.keys import CourseKey, UsageKey
+from opaque_keys.edx.keys import UsageKey
 from xmodule.modulestore.django import modulestore
 
 from common.djangoapps.edxmako.shortcuts import render_to_string

openedx/features/offline_mode/static/offline_mode/js/bridge.js

@@ -0,0 +1,35 @@
+function sendMessageToiOS(message) {
+  window?.webkit?.messageHandlers?.iOSBridge?.postMessage(message);
+}
+
+function sendMessageToAndroid(message) {
+  window?.AndroidBridge?.postMessage(message);
+}
+
+function receiveMessageFromiOS(message) {
+  console.log("Message received from iOS:", message);
+}
+
+function receiveMessageFromAndroid(message) {
+  console.log("Message received from Android:", message);
+}
+
+if (window.webkit && window.webkit.messageHandlers && window.webkit.messageHandlers.iOSBridge) {
+  window.addEventListener("messageFromiOS", function (event) {
+    receiveMessageFromiOS(event.data);
+  });
+}
+if (window.AndroidBridge) {
+  window.addEventListener("messageFromAndroid", function (event) {
+    receiveMessageFromAndroid(event.data);
+  });
+}
+const originalAjax = $.ajax;
+$.ajax = function (options) {
+  sendMessageToiOS(options);
+  sendMessageToiOS(JSON.stringify(options));
+  sendMessageToAndroid(options);
+  sendMessageToAndroid(JSON.stringify(options));
+  console.log(options, JSON.stringify(options));
+  return originalAjax.call(this, options);
+};

openedx/features/offline_mode/tasks.py

@@ -1,17 +1,25 @@
+"""
+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
 
 from xmodule.modulestore.django import modulestore
 
+from .constants import OFFLINE_SUPPORTED_XBLOCKS
 from .renderer import XBlockRenderer
-from .utils import generate_offline_content
+from .utils import generate_offline_content, is_offline_supported_block
 
 
 @shared_task
-def get_rendered_xblock_from_lms(course_id):
+@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 xblock in modulestore().get_items(course_key, qualifiers={'category': 'problem'}):
-        # if is_offline_supported(xblock):
-        #     continue
-        html_data = XBlockRenderer(str(xblock.id)).render_xblock_from_lms()
-        generate_offline_content(xblock, html_data)
+    for xblock in modulestore().get_items(course_key, qualifiers={'category': OFFLINE_SUPPORTED_XBLOCKS}):
+        if is_offline_supported_block(xblock):
+            html_data = XBlockRenderer(str(xblock.id)).render_xblock_from_lms()
+            generate_offline_content(xblock, html_data)

openedx/features/offline_mode/utils.py

@@ -1,3 +1,6 @@
+"""
+Utility functions and classes for offline mode.
+"""
 import os
 import logging
 
@@ -9,17 +12,24 @@
 
 
 from .assets_management import base_storage_path, remove_old_files, is_modified
+from .constants import OFFLINE_CONTENT_ARCHIVE_NAME, OFFLINE_SUPPORTED_XBLOCKS
 from .html_manipulator import HtmlManipulator
 
 User = get_user_model()
 log = logging.getLogger(__name__)
 
 
 def create_zip_file(base_path, file_name):
+    """
+    Creates a zip file with the content of the base_path directory.
+    """
     zf = ZipFile(default_storage.path(base_path + file_name), 'w')
     zf.write(default_storage.path(base_path + 'index.html'), 'index.html')
 
     def add_files_to_zip(zip_file, current_base_path, current_path_in_zip):
+        """
+        Recursively adds files to the zip file.
+        """
         try:
             directories, files = default_storage.listdir(current_base_path)
         except OSError:
@@ -38,6 +48,13 @@ def add_files_to_zip(zip_file, current_base_path, current_path_in_zip):
 
 
 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 HTML data of the XBlock
+    """
     if not is_modified(xblock):
         return
 
@@ -50,4 +67,11 @@ def generate_offline_content(xblock, html_data):
         os.path.join(base_path, 'index.html'),
         ContentFile(updated_html),
     )
-    create_zip_file(base_path, 'offline_content.zip')
+    create_zip_file(base_path, OFFLINE_CONTENT_ARCHIVE_NAME)
+
+
+def is_offline_supported_block(xblock):
+    """
+    Returns True if the block is supported for offline mode, False otherwise.
+    """
+    return xblock.location.block_type in OFFLINE_SUPPORTED_XBLOCKS