diff --git a/learning_resources/etl/loaders.py b/learning_resources/etl/loaders.py index 62389d29ba..0460777b29 100644 --- a/learning_resources/etl/loaders.py +++ b/learning_resources/etl/loaders.py @@ -576,7 +576,7 @@ def load_course( if config.fetch_only or not learning_resource: return learning_resource - Course.objects.get_or_create( + Course.objects.update_or_create( learning_resource=learning_resource, defaults=course_data ) diff --git a/learning_resources/etl/loaders_test.py b/learning_resources/etl/loaders_test.py index 9927387278..44f944ed1d 100644 --- a/learning_resources/etl/loaders_test.py +++ b/learning_resources/etl/loaders_test.py @@ -28,6 +28,7 @@ from learning_resources.etl import loaders from learning_resources.etl.constants import ( CourseLoaderConfig, + CourseNumberType, ETLSource, ProgramLoaderConfig, ) @@ -670,6 +671,50 @@ def test_load_course( # noqa: PLR0913, PLR0912, PLR0915 assert getattr(result, key) == value, f"Property {key} should equal {value}" +def test_load_course_updates_course_numbers(mock_upsert_tasks): + """load_course should replace course_numbers on an existing course""" + platform = LearningResourcePlatformFactory.create() + course = CourseFactory.create( + learning_resource__runs=[], + platform=platform.code, + course_numbers=[ + { + "value": "old-number", + "department": None, + "listing_type": CourseNumberType.primary.value, + "primary": True, + "sort_coursenum": "old-number", + } + ], + ) + learning_resource = course.learning_resource + + new_course_numbers = [ + { + "value": "18.03.1x", + "department": None, + "listing_type": CourseNumberType.primary.name, + "primary": True, + "sort_coursenum": "18.03.1x", + } + ] + props = { + "readable_id": learning_resource.readable_id, + "platform": platform.code, + "title": learning_resource.title, + "url": learning_resource.url, + "published": learning_resource.published, + "runs": [], + "course": {"course_numbers": new_course_numbers}, + } + + load_course(props, [], [], config=CourseLoaderConfig(prune=True)) + + assert Course.objects.count() == 1 + course.refresh_from_db() + assert course.course_numbers == new_course_numbers + + def test_load_course_bad_platform(mocker): """A bad platform should log an exception and not create the course""" mock_log = mocker.patch("learning_resources.etl.loaders.log.exception") diff --git a/learning_resources/etl/mitxonline.py b/learning_resources/etl/mitxonline.py index 6c53d4b816..4b5518a44e 100644 --- a/learning_resources/etl/mitxonline.py +++ b/learning_resources/etl/mitxonline.py @@ -4,7 +4,7 @@ import logging import re from collections.abc import Iterator -from datetime import UTC +from datetime import UTC, datetime from decimal import Decimal from urllib.parse import parse_qs, urljoin, urlparse @@ -388,6 +388,25 @@ def _transform_course(course): ] has_certification = parse_certification(OFFERED_BY["code"], runs) strip_enrollment_modes(runs) + # Course runs each carry a course_number. Order the runs by start date + # (latest first) so the latest run supplies the primary course number, then + # collect the distinct values with the remaining numbers as cross-listed. + # This replaces the course readable_id as the source of course numbers. + runs_by_recency = sorted( + (course_run for course_run in course["courseruns"] if course_run), + key=lambda course_run: ( + _parse_datetime( + course_run.get("start_date") or course_run.get("enrollment_start") + ) + or datetime.min.replace(tzinfo=UTC) + ), + reverse=True, + ) + course_numbers = [] + for course_run in runs_by_recency: + course_number = course_run.get("course_number") + if course_number and course_number not in course_numbers: + course_numbers.append(course_number) return { "readable_id": course["readable_id"], "platform": PlatformType.mitxonline.name, @@ -401,8 +420,12 @@ def _transform_course(course): "force_ingest": course.get("ingest_content_files_for_ai", False), "course": { "course_numbers": generate_course_numbers_json( - course["readable_id"], is_ocw=False - ), + course_numbers[0], + extra_nums=course_numbers[1:], + is_ocw=False, + ) + if course_numbers + else [], }, "published": bool( parse_page_attribute(course, "page_url") diff --git a/learning_resources/etl/mitxonline_test.py b/learning_resources/etl/mitxonline_test.py index 4653855059..8b9373ffd0 100644 --- a/learning_resources/etl/mitxonline_test.py +++ b/learning_resources/etl/mitxonline_test.py @@ -4,7 +4,6 @@ # pylint: disable=redefined-outer-name from datetime import UTC, datetime -from unittest.mock import ANY from urllib.parse import parse_qs, urlparse import pytest @@ -18,7 +17,7 @@ PlatformType, RunStatus, ) -from learning_resources.etl.constants import CourseNumberType, ETLSource +from learning_resources.etl.constants import ETLSource from learning_resources.etl.mitxonline import ( OFFERED_BY, _fetch_courses_by_ids, @@ -42,6 +41,7 @@ transform_topics, ) from learning_resources.etl.utils import ( + generate_course_numbers_json, get_department_id_by_name, parse_certification, parse_string_to_int, @@ -54,6 +54,30 @@ pytestmark = pytest.mark.django_db +def _expected_course_numbers(course_data): + """Build expected course_numbers json from a course's runs (latest run first).""" + runs_by_recency = sorted( + (course_run for course_run in course_data["courseruns"] if course_run), + key=lambda course_run: ( + _parse_datetime( + course_run.get("start_date") or course_run.get("enrollment_start") + ) + or datetime.min.replace(tzinfo=UTC) + ), + reverse=True, + ) + course_numbers = [] + for course_run in runs_by_recency: + course_number = course_run.get("course_number") + if course_number and course_number not in course_numbers: + course_numbers.append(course_number) + if not course_numbers: + return [] + return generate_course_numbers_json( + course_numbers[0], extra_nums=course_numbers[1:], is_ocw=False + ) + + @pytest.fixture def mock_mitxonline_programs_data(): """Mock mitxonline data""" @@ -718,17 +742,7 @@ def test_mitxonline_transform_programs( course_data["topics"], OFFERED_BY["code"] ), "runs": runs, - "course": { - "course_numbers": [ - { - "value": course_data["readable_id"], - "department": ANY, - "listing_type": CourseNumberType.primary.value, - "primary": True, - "sort_coursenum": course_data["readable_id"], - } - ] - }, + "course": {"course_numbers": _expected_course_numbers(course_data)}, "position": len(expected_courses), } ) @@ -896,17 +910,7 @@ def test_mitxonline_transform_courses(mock_mitxonline_courses_data, mocker, sett else None ), "runs": runs, - "course": { - "course_numbers": [ - { - "value": course_data["readable_id"], - "department": ANY, - "listing_type": CourseNumberType.primary.value, - "primary": True, - "sort_coursenum": course_data["readable_id"], - } - ] - }, + "course": {"course_numbers": _expected_course_numbers(course_data)}, "availability": course_data["availability"], "format": [Format.asynchronous.name], "pace": [Pace.instructor_paced.name],