refactor!: remove learner_downloadable field

olx_importer/management/commands/load_components.py

@@ -126,7 +126,6 @@ def create_content(self, static_local_path, now, component_version):
             component_version,
             content.id,
             key=key,
-            learner_downloadable=True,
         )
 
     def import_block_type(self, block_type_name, now): # , publish_log_entry):
@@ -177,7 +176,6 @@ def import_block_type(self, block_type_name, now): # , publish_log_entry):
                 component_version,
                 text_content.pk,
                 key="block.xml",
-                learner_downloadable=False
             )
 
             # Cycle through static assets references and add those as well...

openedx_learning/__init__.py

@@ -2,4 +2,4 @@
 Open edX Learning ("Learning Core").
 """
 
-__version__ = "0.16.3"
+__version__ = "0.17.0"

openedx_learning/apps/authoring/components/admin.py

@@ -71,7 +71,6 @@ def get_queryset(self, request):
     fields = [
         "key",
         "format_size",
-        "learner_downloadable",
         "rendered_data",
     ]
     readonly_fields = [

openedx_learning/apps/authoring/components/api.py

@@ -215,7 +215,6 @@ def create_next_component_version(
                     content_id=content_pk,
                     component_version=component_version,
                     key=key,
-                    learner_downloadable=False,
                 )
         # Now copy any old associations that existed, as long as they aren't
         # in conflict with the new stuff or marked for deletion.
@@ -227,7 +226,6 @@ def create_next_component_version(
                     content_id=cvrc.content_id,
                     component_version=component_version,
                     key=cvrc.key,
-                    learner_downloadable=cvrc.learner_downloadable,
                 )
 
         return component_version
@@ -422,7 +420,6 @@ def create_component_version_content(
     content_id: int,
     /,
     key: str,
-    learner_downloadable: bool = False,
 ) -> ComponentVersionContent:
     """
     Add a Content to the given ComponentVersion
@@ -445,15 +442,13 @@ def create_component_version_content(
         component_version_id=component_version_id,
         content_id=content_id,
         key=key,
-        learner_downloadable=learner_downloadable,
     )
     return cvrc
 
 
 class AssetError(StrEnum):
     """Error codes related to fetching ComponentVersion assets."""
     ASSET_PATH_NOT_FOUND_FOR_COMPONENT_VERSION = auto()
-    ASSET_NOT_LEARNER_DOWNLOADABLE = auto()
     ASSET_HAS_NO_DOWNLOAD_FILE = auto()
 
 
@@ -484,7 +479,6 @@ def get_redirect_response_for_component_asset(
     component_version_uuid: UUID,
     asset_path: Path,
     public: bool = False,
-    learner_downloadable_only: bool = True,
 ) -> HttpResponse:
     """
     ``HttpResponse`` for a reverse-proxy to serve a ``ComponentVersion`` asset.
@@ -498,11 +492,6 @@ def get_redirect_response_for_component_asset(
         If ``True``, this will return an ``HttpResponse`` that can be cached in
         a CDN and shared across many clients.
 
-    :param learner_downloadable_only: Only return assets that are meant to be
-        downloadable by Learners, i.e. in the LMS experience. If this is
-        ``True``, then requests for assets that are not meant for student
-        download will return a ``404`` error response.
-
     **Response Codes**
 
     If the asset exists for this ``ComponentVersion``, this function will return
@@ -512,15 +501,10 @@ def get_redirect_response_for_component_asset(
     the ``ComponentVersion`` itself does not exist, the response code will be
     ``404``.
 
-    Other than checking the coarse-grained ``learner_downloadable_only`` flag,
-    *this function does not do auth checking of any sort*–it will never return
+    This function does not do auth checking of any sort. It will never return
     a ``401`` or ``403`` response code. That is by design. Figuring out who is
     making the request and whether they have permission to do so is the
-    responsiblity of whatever is calling this function. The
-    ``learner_downloadable_only`` flag is intended to be a filter for the entire
-    view. When it's True, not even staff can download component-internal assets.
-    This is intended to protect us from accidentally allowing sensitive grading
-    code to get leaked out.
+    responsiblity of whatever is calling this function.
 
     **Metadata Headers**
 
@@ -596,24 +580,6 @@ def _error_header(error: AssetError) -> dict[str, str]:
         )
         return HttpResponseNotFound(headers=info_headers)
 
-    # Check: If we're asking only for Learner Downloadable assets, and the asset
-    # in question is not supposed to be downloadable by learners, then we give a
-    # 404 error. Even staff members are not expected to be able to download
-    # these assets via the LMS endpoint that serves students. Studio would be
-    # expected to have an entirely different view to serve these assets in that
-    # context (along with different timeouts, auth, and cache settings). So in
-    # that sense, the asset doesn't exist for that particular endpoint.
-    if learner_downloadable_only and (not cv_content.learner_downloadable):
-        logger.error(
-           f"ComponentVersion {component_version_uuid} has asset {asset_path}, "
-           "but it is not meant to be downloadable by learners "
-           "(ComponentVersionContent.learner_downloadable=False)."
-        )
-        info_headers.update(
-            _error_header(AssetError.ASSET_NOT_LEARNER_DOWNLOADABLE)
-        )
-        return HttpResponseNotFound(headers=info_headers)
-
     # At this point, we know that there is valid Content that we want to send.
     # This adds Content-level headers, like the hash/etag and content type.
     info_headers.update(contents_api.get_content_info_headers(content))

openedx_learning/apps/authoring/components/migrations/0003_remove_componentversioncontent_learner_downloadable.py

@@ -0,0 +1,17 @@
+# Generated by Django 4.2.16 on 2024-11-06 17:14
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('oel_components', '0002_alter_componentversioncontent_key'),
+    ]
+
+    operations = [
+        migrations.RemoveField(
+            model_name='componentversioncontent',
+            name='learner_downloadable',
+        ),
+    ]

openedx_learning/apps/authoring/components/models.py

@@ -254,43 +254,6 @@ class ComponentVersionContent(models.Model):
     # identifiers that don't map as cleanly to file paths at some point.
     key = key_field(db_column="_key")
 
-    # Long explanation for the ``learner_downloadable`` field:
-    #
-    # Is this Content downloadable during the learning experience? This is
-    # NOT about public vs. private permissions on course assets, as that will be
-    # a policy that can be changed independently of new versions of the content.
-    # For instance, a course team could decide to flip their course assets from
-    # private to public for CDN caching reasons, and that should not require
-    # new ComponentVersions to be created.
-    #
-    # What the ``learner_downloadable`` field refers to is whether this asset is
-    # supposed to *ever* be directly downloadable by browsers during the
-    # learning experience. This will be True for things like images, PDFs, and
-    # video transcript files. This field will be False for things like:
-    #
-    # * Problem Block OLX will contain the answers to the problem. The XBlock
-    #   runtime and ProblemBlock will use this information to generate HTML and
-    #   grade responses, but the the user's browser is never permitted to
-    #   actually download the raw OLX itself.
-    # * Many courses include a python_lib.zip file holding custom Python code
-    #   to be used by codejail to assess student answers. This code will also
-    #   potentially reveal answers, and is never intended to be downloadable by
-    #   the student's browser.
-    # * Some course teams will upload other file formats that their OLX is
-    #   derived from (e.g. specially formatted LaTeX files). These files will
-    #   likewise contain answers and should never be downloadable by the
-    #   student.
-    # * Other custom metadata may be attached as files in the import, such as
-    #   custom identifiers, author information, etc.
-    #
-    # Even if ``learner_downloadble`` is True, the LMS may decide that this
-    # particular student isn't allowed to see this particular piece of content
-    # yet–e.g. because they are not enrolled, or because the exam this Component
-    # is a part of hasn't started yet. That's a matter of LMS permissions and
-    # policy that is not intrinsic to the content itself, and exists at a layer
-    # above this.
-    learner_downloadable = models.BooleanField(default=False)
-
     class Meta:
         constraints = [
             # Uniqueness is only by ComponentVersion and key. If for some reason

openedx_learning/contrib/media_server/views.py

@@ -5,7 +5,7 @@
 """
 from pathlib import Path
 
-from django.core.exceptions import ObjectDoesNotExist, PermissionDenied
+from django.core.exceptions import ObjectDoesNotExist
 from django.http import FileResponse, Http404
 
 from openedx_learning.apps.authoring.components.api import look_up_component_version_content
@@ -34,11 +34,6 @@ def component_asset(
     except ObjectDoesNotExist:
         raise Http404("File not found")  # pylint: disable=raise-missing-from
 
-    if not cvc.learner_downloadable and not (
-        request.user and request.user.is_superuser
-    ):
-        raise PermissionDenied("This file is not publicly downloadable.")
-
     response = FileResponse(cvc.raw_content.file, filename=Path(asset_path).name)
     response["Content-Type"] = cvc.raw_content.mime_type
 

tests/openedx_learning/apps/authoring/components/test_api.py

@@ -388,7 +388,6 @@ def test_add(self):
             new_version.pk,
             new_content.pk,
             key="my/path/to/hello.txt",
-            learner_downloadable=False,
         )
         # re-fetch from the database to check to see if we wrote it correctly
         new_version = components_api.get_component(self.problem.pk) \
@@ -405,7 +404,6 @@ def test_add(self):
             new_version.pk,
             new_content.pk,
             key="//nested/path/hello.txt",
-            learner_downloadable=False,
         )
         new_version = components_api.get_component(self.problem.pk) \
                                     .versions \

tests/openedx_learning/apps/authoring/components/test_assets.py

@@ -75,7 +75,6 @@ def setUpTestData(cls) -> None:
             cls.component_version.pk,
             cls.problem_content.id,
             key="block.xml",
-            learner_downloadable=False,
         )
 
         # Python source file, stored as a file. This is hypothetical, as we
@@ -90,7 +89,6 @@ def setUpTestData(cls) -> None:
             cls.component_version.pk,
             cls.python_source_asset.id,
             key="src/grader.py",
-            learner_downloadable=False,
         )
 
         # An HTML file that is student downloadable
@@ -104,7 +102,6 @@ def setUpTestData(cls) -> None:
             cls.component_version.pk,
             cls.html_asset_content.id,
             key="static/hello.html",
-            learner_downloadable=True,
         )
 
     def test_no_component_version(self):
@@ -145,10 +142,6 @@ def test_404s_with_component_version_info(self):
             # This is testing that asset paths are case sensitive
             "static/HELLO.html": AssetError.ASSET_PATH_NOT_FOUND_FOR_COMPONENT_VERSION,
 
-            # Files that want to guarantee can never be downloaded (they're for
-            # backend usage only).
-            "src/grader.py": AssetError.ASSET_NOT_LEARNER_DOWNLOADABLE,
-
             # Text stored in the database directly instead of file storage.
             "block.xml": AssetError.ASSET_HAS_NO_DOWNLOAD_FILE,
         }