diff --git a/learning_resources/etl/edx_shared_test.py b/learning_resources/etl/edx_shared_test.py index d924c6d058..2469d212ab 100644 --- a/learning_resources/etl/edx_shared_test.py +++ b/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 diff --git a/learning_resources/etl/loaders.py b/learning_resources/etl/loaders.py index ef218b8aea..56921dde04 100644 --- a/learning_resources/etl/loaders.py +++ b/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, diff --git a/learning_resources/etl/loaders_test.py b/learning_resources/etl/loaders_test.py index 5bd4f5e99b..0971bafe25 100644 --- a/learning_resources/etl/loaders_test.py +++ b/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"), [ diff --git a/learning_resources/etl/openedx.py b/learning_resources/etl/openedx.py index 6ddd567b22..2f5f574dce 100644 --- a/learning_resources/etl/openedx.py +++ b/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: diff --git a/learning_resources/models.py b/learning_resources/models.py index 91b269d786..08c9efe3f6 100644 --- a/learning_resources/models.py +++ b/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: diff --git a/vector_search/tasks.py b/vector_search/tasks.py index d80bf91373..f5cdaac3b8 100644 --- a/vector_search/tasks.py +++ b/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() diff --git a/vector_search/tasks_test.py b/vector_search/tasks_test.py index 204f5a410c..86135bc07e 100644 --- a/vector_search/tasks_test.py +++ b/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 )