Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions learning_resources/etl/edx_shared_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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()


Expand All @@ -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()
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
19 changes: 9 additions & 10 deletions learning_resources/etl/loaders.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
Video,
VideoChannel,
VideoPlaylist,
now_in_utc,
)
from learning_resources.utils import (
add_parent_topics_to_learning_resource,
Expand Down Expand Up @@ -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
Expand All @@ -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,
Expand Down
77 changes: 60 additions & 17 deletions learning_resources/etl/loaders_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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:
Expand Down Expand Up @@ -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},
Expand All @@ -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
Expand Down Expand Up @@ -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"),
[
Expand Down
2 changes: 1 addition & 1 deletion learning_resources/etl/openedx.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
37 changes: 31 additions & 6 deletions learning_resources/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
8 changes: 4 additions & 4 deletions vector_search/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down
10 changes: 5 additions & 5 deletions vector_search/tasks_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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=[])
Expand All @@ -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
Expand All @@ -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
)
Expand Down
Loading