Add LearningResource.delivery field (eventually to replace learning_format) (#1510)

Author: mbertrand

frontends/api/src/generated/v1/api.ts

@@ -270,13 +270,25 @@ class LevelType(ExtendedEnum):
 
 
 class LearningResourceFormat(ExtendedEnum):
-    """Enum for resource formats"""
+    """Enum for resource learning_format"""
 
     online = "Online"
     hybrid = "Hybrid"
     in_person = "In person"
 
 
+class LearningResourceDelivery(ExtendedEnum):
+    """
+    Enum for resource delivery methods. This
+    will eventually replace LearningResourceFormat
+    """
+
+    online = "Online"
+    hybrid = "Hybrid"
+    in_person = "In person"
+    offline = "Offline"
+
+
 class CertificationType(ExtendedEnum):
     """Enum for resource certification types"""
 

learning_resources/constants.py

@@ -20,6 +20,7 @@
     CONTENT_TYPE_VIDEO,
     VALID_TEXT_FILE_TYPES,
     Availability,
+    LearningResourceDelivery,
     LearningResourceType,
     OfferedBy,
     PlatformType,
@@ -33,7 +34,11 @@
     transform_levels,
     transform_topics,
 )
-from learning_resources.models import ContentFile, LearningResource
+from learning_resources.models import (
+    ContentFile,
+    LearningResource,
+    default_learning_format,
+)
 from learning_resources.utils import (
     get_s3_object_and_read,
     parse_instructors,
@@ -48,6 +53,22 @@
 UNIQUE_FIELD = "url"
 
 
+def parse_delivery(course_data: dict) -> list[str]:
+    """
+    Parse delivery methods
+
+    Args:
+        url (str): The course url
+
+    Returns:
+        list[str]: The delivery method(s)
+    """
+    delivery = default_learning_format()
+    if not course_data.get("hide_download"):
+        delivery.append(LearningResourceDelivery.offline.name)
+    return delivery
+
+
 def transform_content_files(
     s3_resource: boto3.resource,
     course_prefix: str,
@@ -343,6 +364,7 @@ def transform_course(course_data: dict) -> dict:
         "resource_type": LearningResourceType.course.name,
         "unique_field": UNIQUE_FIELD,
         "availability": Availability.anytime.name,
+        "delivery": parse_delivery(course_data),
     }
 
 

learning_resources/etl/ocw.py

@@ -9,7 +9,7 @@
 from moto import mock_s3
 
 from learning_resources.conftest import OCW_TEST_PREFIX, setup_s3_ocw
-from learning_resources.constants import DEPARTMENTS
+from learning_resources.constants import DEPARTMENTS, LearningResourceDelivery
 from learning_resources.etl.constants import CourseNumberType, ETLSource
 from learning_resources.etl.ocw import (
     transform_content_files,
@@ -174,19 +174,37 @@ def test_transform_content_file_needs_text_update(
         "term",
         "year",
         "expected_id",
+        "hide_download",
     ),
     [
-        ("legacy-uid", None, "legacyuid", False, "Spring", "2005", "16.01+spring_2005"),
-        (None, "site-uid", "siteuid", True, "", 2005, "16.01_2005"),
-        (None, "site-uid", "siteuid", True, "Fall", 2005, "16.01+fall_2005"),
-        (None, "site-uid", "siteuid", True, "Fall", None, "16.01+fall"),
-        (None, "site-uid", "siteuid", True, "", "", "16.01"),
-        (None, "site-uid", "siteuid", True, None, None, "16.01"),
-        (None, None, None, True, "Spring", "2005", None),
+        (
+            "legacy-uid",
+            None,
+            "legacyuid",
+            False,
+            "Spring",
+            "2005",
+            "16.01+spring_2005",
+            False,
+        ),
+        (None, "site-uid", "siteuid", True, "", 2005, "16.01_2005", True),
+        (None, "site-uid", "siteuid", True, "Fall", 2005, "16.01+fall_2005", None),
+        (None, "site-uid", "siteuid", True, "Fall", None, "16.01+fall", False),
+        (None, "site-uid", "siteuid", True, "", "", "16.01", True),
+        (None, "site-uid", "siteuid", True, None, None, "16.01", False),
+        (None, None, None, True, "Spring", "2005", None, None),
     ],
 )
 def test_transform_course(  # noqa: PLR0913
-    settings, legacy_uid, site_uid, expected_uid, has_extra_num, term, year, expected_id
+    settings,
+    legacy_uid,
+    site_uid,
+    expected_uid,
+    has_extra_num,
+    term,
+    year,
+    expected_id,
+    hide_download,
 ):
     """transform_course should return expected data"""
     settings.OCW_BASE_URL = "http://test.edu/"
@@ -201,17 +219,22 @@ def test_transform_course(  # noqa: PLR0913
     course_json["year"] = year
     course_json["legacy_uid"] = legacy_uid
     course_json["site_uid"] = site_uid
+    course_json["hide_download"] = hide_download
     course_json["extra_course_numbers"] = "1, 2" if has_extra_num else None
     extracted_json = {
         **course_json,
         "last_modified": now_in_utc(),
         "slug": "slug",
         "url": "http://test.edu/slug",
     }
+    expected_delivery = [LearningResourceDelivery.online.name] + (
+        [] if hide_download else [LearningResourceDelivery.offline.name]
+    )
     transformed_json = transform_course(extracted_json)
     if expected_uid:
         assert transformed_json["readable_id"] == expected_id
         assert transformed_json["etl_source"] == ETLSource.ocw.name
+        assert transformed_json["delivery"] == expected_delivery
         assert transformed_json["runs"][0]["run_id"] == expected_uid
         assert transformed_json["runs"][0]["level"] == ["undergraduate", "high_school"]
         assert transformed_json["runs"][0]["semester"] == (term if term else None)

learning_resources/etl/ocw_test.py

@@ -11,7 +11,7 @@
 
 from learning_resources.constants import Availability, CertificationType
 from learning_resources.etl.constants import ETLSource
-from learning_resources.etl.utils import transform_format, transform_topics
+from learning_resources.etl.utils import transform_delivery, transform_topics
 from learning_resources.models import LearningResourceOfferor, LearningResourcePlatform
 from main.utils import clean_data, now_in_utc
 
@@ -189,6 +189,7 @@ def update_format(unique_resource: dict, resource_format: list[str]):
     unique_resource["learning_format"] = sorted(
         set(unique_resource["learning_format"] + resource_format)
     )
+    unique_resource["delivery"] = unique_resource["learning_format"]
 
 
 def extract_data(course_or_program: str) -> list[dict]:
@@ -265,7 +266,7 @@ def transform_programs(programs: list[dict]) -> list[dict]:
                 "professional": True,
                 "certification": True,
                 "certification_type": CertificationType.professional.name,
-                "learning_format": transform_format(program["format_name"]),
+                "learning_format": transform_delivery(program["format_name"]),
                 "runs": runs,
                 "topics": parse_topic(program, offered_by.code) if offered_by else None,
                 "courses": [
@@ -355,7 +356,7 @@ def _transform_course(
             "course": {
                 "course_numbers": [],
             },
-            "learning_format": transform_format(course["format_name"]),
+            "learning_format": transform_delivery(course["format_name"]),
             "published": True,
             "topics": parse_topic(course, offered_by.code) if offered_by else None,
             "runs": runs,

learning_resources/etl/prolearn.py

@@ -32,7 +32,7 @@
     transform_programs,
     update_format,
 )
-from learning_resources.etl.utils import transform_format
+from learning_resources.etl.utils import transform_delivery
 from learning_resources.factories import (
     LearningResourceOfferorFactory,
     LearningResourcePlatformFactory,
@@ -157,7 +157,8 @@ def test_prolearn_transform_programs(mock_csail_programs_data):
             else None,
             "etl_source": ETLSource.prolearn.name,
             "professional": True,
-            "learning_format": transform_format(program["format_name"]),
+            "learning_format": transform_delivery(program["format_name"]),
+            "delivery": transform_delivery(program["format_name"]),
             "certification": True,
             "certification_type": CertificationType.professional.name,
             "runs": [
@@ -226,7 +227,8 @@ def test_prolearn_transform_courses(mock_mitpe_courses_data):
             "professional": True,
             "certification": True,
             "certification_type": CertificationType.professional.name,
-            "learning_format": transform_format(course["format_name"]),
+            "learning_format": transform_delivery(course["format_name"]),
+            "delivery": transform_delivery(course["format_name"]),
             "topics": parse_topic(course, "mitpe"),
             "url": course["course_link"]
             if urlparse(course["course_link"]).path
@@ -395,6 +397,7 @@ def test_update_format(
     first_course["learning_format"] = old_format
     update_format(first_course, new_format)
     assert first_course["learning_format"] == sorted(expected_format)
+    assert first_course["delivery"] == sorted(expected_format)
 
 
 @pytest.mark.parametrize("sloan_api_enabled", [True, False])

learning_resources/etl/prolearn_test.py

@@ -18,7 +18,7 @@
 )
 from learning_resources.etl.constants import ETLSource
 from learning_resources.etl.utils import (
-    transform_format,
+    transform_delivery,
     transform_topics,
 )
 
@@ -202,7 +202,9 @@ def transform_course(course_data: dict, runs_data: dict) -> dict:
     course_runs_data = [
         run for run in runs_data if run["Course_Id"] == course_data["Course_Id"]
     ]
-
+    format_delivery = list(
+        {transform_delivery(run["Delivery"])[0] for run in course_runs_data}
+    )
     transformed_course = {
         "readable_id": course_data["Course_Id"],
         "title": course_data["Title"],
@@ -216,9 +218,8 @@ def transform_course(course_data: dict, runs_data: dict) -> dict:
         "certification_type": CertificationType.professional.name,
         "professional": True,
         "published": True,
-        "learning_format": list(
-            {transform_format(run["Delivery"])[0] for run in course_runs_data}
-        ),
+        "learning_format": format_delivery,
+        "delivery": format_delivery,
         "topics": parse_topics(course_data["Topics"]),
         "course": {
             "course_numbers": [],

learning_resources/etl/sloan.py

@@ -15,7 +15,7 @@
     parse_datetime,
     parse_image,
     transform_course,
-    transform_format,
+    transform_delivery,
     transform_run,
 )
 from learning_resources.factories import (
@@ -139,8 +139,9 @@ def test_transform_course(mock_sloan_courses_data, mock_sloan_runs_data):
         transform_run(run, course_data) for run in course_runs_data
     ]
     assert transformed["learning_format"] == list(
-        {transform_format(run["Delivery"])[0] for run in course_runs_data}
+        {transform_delivery(run["Delivery"])[0] for run in course_runs_data}
     )
+    assert transformed["delivery"] == transformed["learning_format"]
     assert transformed["image"] == parse_image(course_data)
     assert transformed["availability"] == parse_availability(course_runs_data)
     assert sorted(transformed.keys()) == sorted(
@@ -158,6 +159,7 @@ def test_transform_course(mock_sloan_courses_data, mock_sloan_runs_data):
             "professional",
             "published",
             "learning_format",
+            "delivery",
             "topics",
             "course",
             "runs",

learning_resources/etl/sloan_test.py

@@ -687,7 +687,7 @@ def most_common_topics(
     return [{"name": topic} for topic in common_topics]
 
 
-def transform_format(resource_format: str) -> list[str]:
+def transform_delivery(resource_delivery: str) -> list[str]:
     """
     Return the correct format of the resource
 
@@ -699,9 +699,9 @@ def transform_format(resource_format: str) -> list[str]:
 
     """
     try:
-        return [RESOURCE_FORMAT_MAPPING[resource_format]]
+        return [RESOURCE_FORMAT_MAPPING[resource_delivery]]
     except KeyError:
-        log.exception("Invalid format %s", resource_format)
+        log.exception("Invalid format %s", resource_delivery)
         return [LearningResourceFormat.online.name]
 
 

learning_resources/etl/utils.py

@@ -371,13 +371,15 @@ def test_most_common_topics():
 )
 def test_parse_format(original, expected):
     """parse_format should return expected format"""
-    assert utils.transform_format(original) == [expected]
+    assert utils.transform_delivery(original) == [expected]
 
 
 def test_parse_bad_format(mocker):
     """An exception log should be called for invalid formats"""
     mock_log = mocker.patch("learning_resources.etl.utils.log.exception")
-    assert utils.transform_format("bad_format") == [LearningResourceFormat.online.name]
+    assert utils.transform_delivery("bad_format") == [
+        LearningResourceFormat.online.name
+    ]
     mock_log.assert_called_once_with("Invalid format %s", "bad_format")