Change "next_run" to "best_run" and refactor how next_start_date is calculated (#2586)

Author: mbertrand

learning_resources/etl/edx_shared_test.py

@@ -80,7 +80,7 @@ def test_sync_edx_course_files(  # noqa: PLR0913
         )
         course.refresh_from_db()
         if published:
-            assert course.next_run in runs
+            assert course.best_run in runs
         keys.extend(
             [f"20220101/{s3_prefix}/{run.run_id}.tar.gz" for run in runs]
             if source != ETLSource.oll.name
@@ -102,7 +102,7 @@ def test_sync_edx_course_files(  # noqa: PLR0913
     assert mock_load_content_files.call_count == (4 if published else 0)
     if published:
         for course in courses:
-            mock_load_content_files.assert_any_call(course.next_run, fake_data)
+            mock_load_content_files.assert_any_call(course.best_run, fake_data)
     mock_log.assert_not_called()
 
 
@@ -113,7 +113,7 @@ def test_sync_edx_course_files_matching_checksum(
 
     run = LearningResourceFactory.create(
         is_course=True, create_runs=True, etl_source=ETLSource.mitxonline.name
-    ).next_run
+    ).best_run
     run.learning_resource.runs.exclude(id=run.id).first()
     run.checksum = "123"
     run.save()
@@ -156,7 +156,7 @@ def test_sync_edx_course_files_invalid_tarfile(
         published=True,
         create_runs=True,
     )
-    run = course.next_run
+    run = course.best_run
     key = f"20220101/courses/{run.run_id}.tar.gz"
     bucket = (
         mock_mitxonline_learning_bucket
@@ -231,7 +231,7 @@ def test_sync_edx_course_files_error(
         published=True,
         create_runs=True,
     )
-    run = course.next_run
+    run = course.best_run
     key = f"20220101/courses/{run.run_id}.tar.gz"
     bucket = (
         mock_mitxonline_learning_bucket

learning_resources/etl/loaders.py

@@ -45,6 +45,7 @@
     Video,
     VideoChannel,
     VideoPlaylist,
+    now_in_utc,
 )
 from learning_resources.utils import (
     add_parent_topics_to_learning_resource,
@@ -137,16 +138,12 @@ def load_run_dependent_values(
     Returns:
         tuple[datetime.time | None, list[Decimal], str]: date, prices, and availability
     """
-    next_upcoming_run = resource.next_run
-    if next_upcoming_run:
-        resource.next_start_date = next_upcoming_run.start_date
-    else:
-        resource.next_start_date = None
-    best_run = (
-        next_upcoming_run
-        or resource.runs.filter(published=True).order_by("-start_date").first()
-    )
-    if best_run:
+    now = now_in_utc()
+    best_run = resource.best_run
+    if resource.published and best_run:
+        resource.next_start_date = max(
+            best_run.start_date or best_run.enrollment_start or now, now
+        )
         resource.availability = best_run.availability
         resource.prices = (
             best_run.prices
@@ -165,6 +162,8 @@ def load_run_dependent_values(
         resource.time_commitment = best_run.time_commitment
         resource.min_weekly_hours = best_run.min_weekly_hours
         resource.max_weekly_hours = best_run.max_weekly_hours
+    else:
+        resource.next_start_date = None
     resource.save()
     return ResourceNextRunConfig(
         next_start_date=resource.next_start_date,

learning_resources/etl/loaders_test.py

@@ -9,7 +9,6 @@
 
 import pytest
 from django.forms.models import model_to_dict
-from django.utils import timezone
 
 from learning_resources.constants import (
     CURRENCY_USD,
@@ -364,11 +363,11 @@ def test_load_program_bad_platform(mocker):
 @pytest.mark.parametrize("course_exists", [True, False])
 @pytest.mark.parametrize("is_published", [True, False])
 @pytest.mark.parametrize("is_run_published", [True, False])
-@pytest.mark.parametrize("blocklisted", [True, False])
+@pytest.mark.parametrize("blocklisted", [False, True])
 @pytest.mark.parametrize("delivery", [LearningResourceDelivery.hybrid.name, None])
 @pytest.mark.parametrize("has_upcoming_run", [True, False])
 @pytest.mark.parametrize("has_departments", [True, False])
-def test_load_course(  # noqa: PLR0913,PLR0912,PLR0915
+def test_load_course(  # noqa: PLR0913,PLR0912,PLR0915, C901
     mock_upsert_tasks,
     course_exists,
     is_published,
@@ -391,30 +390,45 @@ def test_load_course(  # noqa: PLR0913,PLR0912,PLR0915
 
     learning_resource.published = is_published
 
-    start_date = (
-        timezone.now() + timedelta(10)
-        if has_upcoming_run
-        else timezone.now() - timedelta(1)
-    )
-    old_start_date = timezone.now() - timedelta(365)
+    now = now_in_utc()
+    start_date = now + timedelta(10) if has_upcoming_run else now - timedelta(10)
+    end_date = start_date + timedelta(30)
+    old_start_date = now - timedelta(365)
+    old_end_date = old_start_date + timedelta(30)
 
     if course_exists:
         run = LearningResourceRunFactory.create(
-            learning_resource=learning_resource, published=True, start_date=start_date
+            learning_resource=learning_resource,
+            published=True,
+            enrollment_start=start_date - timedelta(30),
+            start_date=start_date,
+            end_date=end_date,
+            enrollment_end=end_date - timedelta(30),
         )
         old_run = LearningResourceRunFactory.create(
             learning_resource=learning_resource,
             published=True,
             start_date=old_start_date,
+            end_date=old_end_date,
+            enrollment_start=old_start_date - timedelta(30),
+            enrollment_end=old_end_date - timedelta(30),
         )
-        learning_resource.runs.set([run])
+        learning_resource.runs.set([run, old_run])
         learning_resource.save()
     else:
-        run = LearningResourceRunFactory.build(start_date=start_date)
+        run = LearningResourceRunFactory.build(
+            start_date=start_date,
+            end_date=end_date,
+            enrollment_start=start_date - timedelta(30),
+            enrollment_end=end_date - timedelta(30),
+        )
         old_run = LearningResourceRunFactory.build(
             learning_resource=learning_resource,
             published=True,
             start_date=old_start_date,
+            end_date=old_end_date,
+            enrollment_start=old_start_date - timedelta(30),
+            enrollment_end=old_end_date - timedelta(30),
         )
     assert Course.objects.count() == (1 if course_exists else 0)
     if has_departments:
@@ -451,7 +465,7 @@ def test_load_course(  # noqa: PLR0913,PLR0912,PLR0915
             {
                 "run_id": run.run_id,
                 "enrollment_start": run.enrollment_start,
-                "start_date": start_date,
+                "start_date": run.start_date,
                 "end_date": run.end_date,
                 "prices": [
                     {"amount": Decimal("0.00"), "currency": CURRENCY_USD},
@@ -468,10 +482,11 @@ def test_load_course(  # noqa: PLR0913,PLR0912,PLR0915
     result = load_course(props, blocklist, [], config=CourseLoaderConfig(prune=True))
     assert result.professional is True
 
-    expected_next_start_date = (
-        start_date if has_upcoming_run and is_run_published else None
-    )
-    assert result.next_start_date == expected_next_start_date
+    if is_published and is_run_published and not blocklisted:
+        if has_upcoming_run:
+            assert result.next_start_date == start_date
+        else:
+            assert result.next_start_date.date() == now.date()
     assert result.prices == (
         [Decimal("0.00"), Decimal("49.00")]
         if is_run_published and result.certification
@@ -1769,6 +1784,34 @@ def test_load_run_dependent_values(certification):
         assert getattr(result, key) == getattr(course, key) == getattr(run, key)
 
 
+def test_load_run_dependent_values_resets_next_start_date():
+    """Test that next_start_date is reset to None when best_run becomes None"""
+    # Create a published course with an existing next_start_date
+    previous_date = now_in_utc() + timedelta(days=5)
+    course = LearningResourceFactory.create(
+        is_course=True,
+        published=True,
+        next_start_date=previous_date,  # Course previously had a start date
+    )
+
+    # Ensure course has no runs, so best_run will return None
+    course.runs.all().update(published=False)
+    assert course.best_run is None
+
+    # Verify the course initially has a next_start_date
+    assert course.next_start_date == previous_date
+
+    # Call load_run_dependent_values
+    result = load_run_dependent_values(course)
+
+    # Refresh course from database
+    course.refresh_from_db()
+
+    # Verify that next_start_date was reset to None
+    assert result.next_start_date is None
+    assert course.next_start_date is None
+
+
 @pytest.mark.parametrize(
     ("is_scholar_course", "tag_counts", "expected_score"),
     [

learning_resources/etl/openedx.py

@@ -503,7 +503,7 @@ def _parse_program_instructors_topics(program):
     for course in courses:
         topics.extend([{"name": topic.name} for topic in course.topics.all()])
         run = (
-            course.next_run
+            course.best_run
             or course.runs.filter(published=True).order_by("-start_date").first()
         )
         if run:

learning_resources/models.py

@@ -31,7 +31,7 @@
     PrivacyLevel,
 )
 from main.models import TimestampedModel, TimestampedModelQuerySet
-from main.utils import checksum_for_content
+from main.utils import checksum_for_content, now_in_utc
 
 if TYPE_CHECKING:
     from django.contrib.auth import get_user_model
@@ -526,13 +526,38 @@ def audience(self) -> str | None:
         return None
 
     @cached_property
-    def next_run(self) -> Optional["LearningResourceRun"]:
-        """Returns the next run for the learning resource"""
-        return (
-            self.runs.filter(Q(published=True) & Q(start_date__gt=timezone.now()))
-            .order_by("start_date")
+    def best_run(self) -> Optional["LearningResourceRun"]:
+        """Returns the most current/upcoming enrollable run for the learning resource"""
+        published_runs = self.runs.filter(published=True)
+        now = now_in_utc()
+        # Find the most recent run with a currently active enrollment period
+        best_lr_run = (
+            published_runs.filter(
+                (
+                    Q(enrollment_start__lte=now)
+                    | (Q(enrollment_start__isnull=True) & Q(start_date__lte=now))
+                )
+                & (
+                    Q(enrollment_end__gt=now)
+                    | (Q(enrollment_end__isnull=True) & Q(end_date__gt=now))
+                )
+            )
+            .order_by("start_date", "end_date")
             .first()
         )
+        if not best_lr_run:
+            # If no current enrollable run found, find the next upcoming run
+            best_lr_run = (
+                self.runs.filter(Q(published=True) & Q(start_date__gte=timezone.now()))
+                .order_by("start_date")
+                .first()
+            )
+        if not best_lr_run:
+            # If current_run is still null, return the run with the latest start date
+            best_lr_run = (
+                self.runs.filter(Q(published=True)).order_by("-start_date").first()
+            )
+        return best_lr_run
 
     @cached_property
     def views_count(self) -> int:

vector_search/tasks.py

@@ -135,8 +135,8 @@ def start_embed_resources(self, indexes, skip_content_files, overwrite):
                     .order_by("id")
                 ):
                     run = (
-                        course.next_run
-                        if course.next_run
+                        course.best_run
+                        if course.best_run
                         else course.runs.filter(published=True)
                         .order_by("-start_date")
                         .first()
@@ -225,8 +225,8 @@ def embed_learning_resources_by_id(self, ids, skip_content_files, overwrite):
             if not skip_content_files and resource_type == COURSE_TYPE:
                 for course in embed_resources.order_by("id"):
                     run = (
-                        course.next_run
-                        if course.next_run
+                        course.best_run
+                        if course.best_run
                         else course.runs.filter(published=True)
                         .order_by("-start_date")
                         .first()

vector_search/tasks_test.py

@@ -249,9 +249,9 @@ def test_embed_learning_resources_by_id(mocker, mocked_celery):
     assert sorted(resource_ids) == sorted(embedded_resource_ids)
 
 
-def test_embedded_content_from_next_run(mocker, mocked_celery):
+def test_embedded_content_from_best_run(mocker, mocked_celery):
     """
-    Content files to embed should come from next course run
+    Content files to embed should come from best course run
     """
 
     mocker.patch("vector_search.tasks.load_course_blocklist", return_value=[])
@@ -267,10 +267,10 @@ def test_embedded_content_from_next_run(mocker, mocked_celery):
         start_date=datetime.datetime.now(tz=datetime.UTC) + datetime.timedelta(days=2),
     )
 
-    next_run_contentfiles = [
+    best_run_contentfiles = [
         cf.id
         for cf in ContentFileFactory.create_batch(
-            3, run=course.learning_resource.next_run
+            3, run=course.learning_resource.best_run
         )
     ]
     # create contentfiles using the other run
@@ -286,7 +286,7 @@ def test_embedded_content_from_next_run(mocker, mocked_celery):
         )
 
     generate_embeddings_mock.si.assert_called_with(
-        next_run_contentfiles,
+        best_run_contentfiles,
         "content_file",
         True,  # noqa: FBT003
     )