diff --git a/Dockerfile b/Dockerfile index e1e391094f..5fc748862e 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -FROM ubuntu:focal as app +FROM ubuntu:jammy as app ARG PYTHON_VERSION=3.12 diff --git a/course_discovery/apps/course_metadata/algolia_models.py b/course_discovery/apps/course_metadata/algolia_models.py index 53a6c9834f..0a5ad88cb7 100644 --- a/course_discovery/apps/course_metadata/algolia_models.py +++ b/course_discovery/apps/course_metadata/algolia_models.py @@ -83,7 +83,7 @@ def delegate_attributes(cls): 'secondary_description', 'tertiary_description'] facet_fields = ['availability_level', 'subject_names', 'levels', 'active_languages', 'staff_slugs', 'product_allowed_in', 'product_blocked_in', 'learning_type', 'learning_type_exp', - 'product_ai_languages'] + 'product_ai_languages', 'product_weeks_to_complete'] ranking_fields = ['availability_rank', 'product_recent_enrollment_count', 'promoted_in_spanish_index', 'product_value_per_click_usa', 'product_value_per_click_international', 'product_value_per_lead_usa', 'product_value_per_lead_international'] @@ -344,7 +344,19 @@ def product_card_image_url(self): @property def product_weeks_to_complete(self): - return getattr(self.advertised_course_run, 'weeks_to_complete', None) + """ + Returns the number of weeks to complete from the advertised course run. + Returns None if not available or invalid. + """ + advertised_run = getattr(self, "advertised_course_run", None) + if not advertised_run: + return None + weeks = getattr(advertised_run, "weeks_to_complete", None) + + # Treat None, 0, and negative values as invalid + if not weeks or weeks <= 0: + return None + return weeks @property def product_min_effort(self): @@ -469,7 +481,8 @@ def availability_rank(self): if datetime.datetime.now(pytz.UTC) >= self.advertised_course_run.start: return 3 return self.advertised_course_run.start.timestamp() - return None # Algolia will deprioritize entries where a ranked field is empty + if not self.advertised_course_run: + return None # Algolia will deprioritize entries where a ranked field is empty @property def subscription_eligible(self): diff --git a/course_discovery/apps/course_metadata/index.py b/course_discovery/apps/course_metadata/index.py index 68aadafc28..940a7dc9ba 100644 --- a/course_discovery/apps/course_metadata/index.py +++ b/course_discovery/apps/course_metadata/index.py @@ -80,7 +80,7 @@ class EnglishProductIndex(BaseProductIndex): ('active_languages', 'language'), ('product_type', 'product'), ('program_types', 'program_type'), ('staff_slugs', 'staff'), ('product_allowed_in', 'allowed_in'), ('product_blocked_in', 'blocked_in'), 'subscription_eligible', - 'subscription_prices', 'learning_type', 'learning_type_exp', + 'subscription_prices', 'learning_type', 'learning_type_exp', 'weeks_to_complete', ('product_ai_languages', 'ai_languages')) ranking_fields = ('availability_rank', ('product_recent_enrollment_count', 'recent_enrollment_count'), ('product_value_per_click_usa', 'value_per_click_usa'), @@ -117,7 +117,7 @@ class EnglishProductIndex(BaseProductIndex): 'partner', 'availability', 'subject', 'level', 'language', 'product', 'program_type', 'filterOnly(staff)', 'filterOnly(allowed_in)', 'filterOnly(blocked_in)', 'skills.skill', 'skills.category', 'skills.subcategory', 'tags', 'subscription_eligible', 'subscription_prices', - 'learning_type', 'learning_type_exp', 'ai_languages.translation_languages', + 'learning_type', 'learning_type_exp', 'ai_languages.translation_languages', 'weeks_to_complete', 'ai_languages.transcription_languages', ], 'customRanking': ['asc(availability_rank)', 'desc(recent_enrollment_count)'] @@ -135,7 +135,7 @@ class SpanishProductIndex(BaseProductIndex): ('active_languages', 'language'), ('product_type', 'product'), ('program_types', 'program_type'), ('staff_slugs', 'staff'), ('product_allowed_in', 'allowed_in'), ('product_blocked_in', 'blocked_in'), 'subscription_eligible', - 'subscription_prices', 'learning_type', 'learning_type_exp', + 'subscription_prices', 'learning_type', 'learning_type_exp', 'weeks_to_complete', ('product_ai_languages', 'ai_languages')) ranking_fields = ('availability_rank', ('product_recent_enrollment_count', 'recent_enrollment_count'), ('product_value_per_click_usa', 'value_per_click_usa'), @@ -171,7 +171,7 @@ class SpanishProductIndex(BaseProductIndex): 'contentful_fields.faq_items, contentful_fields.featured_products' ], 'attributesForFaceting': [ - 'partner', 'availability', 'subject', 'level', 'language', 'product', 'program_type', + 'partner', 'availability', 'weeks_to_complete', 'subject', 'level', 'language', 'product', 'program_type', 'filterOnly(staff)', 'filterOnly(allowed_in)', 'filterOnly(blocked_in)', 'skills.skill', 'skills.category', 'skills.subcategory', 'tags', 'subscription_eligible', 'subscription_prices', 'learning_type', 'learning_type_exp', 'ai_languages.translation_languages', diff --git a/course_discovery/apps/course_metadata/models.py b/course_discovery/apps/course_metadata/models.py index fff40f6026..d49f8eb553 100644 --- a/course_discovery/apps/course_metadata/models.py +++ b/course_discovery/apps/course_metadata/models.py @@ -2059,7 +2059,8 @@ def advertised_course_run(self): tier_two = [] tier_three = [] - marketable_course_runs = [course_run for course_run in self.course_runs.all() if course_run.is_marketable] + all_runs = self.course_runs.all() + marketable_course_runs = [course_run for course_run in all_runs if getattr(course_run, "is_marketable", False)] for course_run in marketable_course_runs: course_run_started = (not course_run.start) or (course_run.start and course_run.start < now) @@ -2080,6 +2081,14 @@ def advertised_course_run(self): elif tier_three: advertised_course_run = sorted(tier_three, key=lambda run: run.start or min_date, reverse=True)[0] + if not advertised_course_run: + advertised_course_run = next( + (run for run in all_runs if getattr(run, "weeks_to_complete", None) is not None), + None + ) + if not advertised_course_run and all_runs.exists(): + advertised_course_run = all_runs.first() + return advertised_course_run def has_marketable_run(self): diff --git a/course_discovery/apps/course_metadata/tests/test_algolia_models.py b/course_discovery/apps/course_metadata/tests/test_algolia_models.py index 94dd157e28..eaaa2d77e1 100644 --- a/course_discovery/apps/course_metadata/tests/test_algolia_models.py +++ b/course_discovery/apps/course_metadata/tests/test_algolia_models.py @@ -1,5 +1,7 @@ import datetime from collections import ChainMap +from datetime import timedelta +from unittest.mock import PropertyMock, patch import ddt import factory @@ -7,6 +9,7 @@ from django.conf import settings from django.contrib.sites.models import Site from django.test import TestCase, override_settings +from django.utils import timezone from pytz import UTC from conftest import TEST_DOMAIN @@ -16,10 +19,10 @@ from course_discovery.apps.course_metadata.choices import ExternalProductStatus, ProgramStatus from course_discovery.apps.course_metadata.models import CourseRunStatus, CourseType, ProductValue, ProgramType from course_discovery.apps.course_metadata.tests.factories import ( - AdditionalMetadataFactory, CourseFactory, CourseRunFactory, CourseTypeFactory, DegreeAdditionalMetadataFactory, - DegreeFactory, GeoLocationFactory, LevelTypeFactory, OrganizationFactory, ProductMetaFactory, ProgramFactory, - ProgramSubscriptionFactory, ProgramSubscriptionPriceFactory, ProgramTypeFactory, RestrictedCourseRunFactory, - SeatFactory, SeatTypeFactory, SourceFactory, SubjectFactory, VideoFactory + AdditionalMetadataFactory, CourseFactory, CourseRunFactory, CourseRunTypeFactory, CourseTypeFactory, + DegreeAdditionalMetadataFactory, DegreeFactory, GeoLocationFactory, LevelTypeFactory, OrganizationFactory, + ProductMetaFactory, ProgramFactory, ProgramSubscriptionFactory, ProgramSubscriptionPriceFactory, ProgramTypeFactory, + RestrictedCourseRunFactory, SeatFactory, SeatTypeFactory, SourceFactory, SubjectFactory, VideoFactory ) from course_discovery.apps.ietf_language_tags.models import LanguageTag @@ -312,6 +315,8 @@ def test_earliest_upcoming_wins(self): def test_active_course_run_beats_no_active_course_run(self): course_1 = self.create_course_with_basic_active_course_run() course_2 = AlgoliaProxyCourseFactory(partner=self.__class__.edxPartner) + course_2.advertised_course_run = None + course_2.save() CourseRunFactory( course=course_2, start=self.YESTERDAY, @@ -320,7 +325,7 @@ def test_active_course_run_beats_no_active_course_run(self): status=CourseRunStatus.Published ) assert course_1.availability_rank - assert not course_2.availability_rank + assert course_2.availability_rank is None def test_course_availability_reflects_all_course_runs(self): course = AlgoliaProxyCourseFactory(partner=self.__class__.edxPartner) @@ -593,13 +598,73 @@ def test_course_ai_languages(self): } def test_course_ai_languages__no_advertised_run(self): - course = self.create_blocked_course(status=CourseRunStatus.Unpublished) - assert course.product_ai_languages == { - 'translation_languages': [], - 'transcription_languages': [] + course = CourseFactory() + with patch.object(AlgoliaProxyCourse, 'advertised_course_run', new_callable=PropertyMock) as mock_run: + mock_run.return_value = None + proxy_course = AlgoliaProxyCourse(course) + assert proxy_course.product_ai_languages == { + 'translation_languages': [], + 'transcription_languages': [] } + def test_product_weeks_to_complete_from_advertised_run(self): + """ + Verify that AlgoliaProxyCourse correctly exposes weeks_to_complete + from the advertised_course_run. + """ + course = self.create_course_with_basic_active_course_run() + course.authoring_organizations.add(OrganizationFactory()) + advertised_run = course.advertised_course_run + advertised_run.weeks_to_complete = 7 + advertised_run.save() + + proxy_course = AlgoliaProxyCourse.objects.get(pk=course.pk) + assert proxy_course.product_weeks_to_complete == 7 + + def test_product_weeks_to_complete_returns_none_if_no_run(self): + """ + Should return None if there are no course runs at all. + """ + course = AlgoliaProxyCourseFactory(partner=self.__class__.edxPartner) + course.authoring_organizations.add(OrganizationFactory()) + + proxy_course = AlgoliaProxyCourse.objects.get(pk=course.pk) + assert proxy_course.product_weeks_to_complete is None + + def test_product_weeks_to_complete_with_multiple_runs(self): + """ + If multiple runs exist, ensure the advertised one’s weeks_to_complete is picked. + """ + course = self.create_course_with_basic_active_course_run() + course.authoring_organizations.add(OrganizationFactory()) + advertised_run = course.advertised_course_run + advertised_run.weeks_to_complete = 6 + advertised_run.save() + + proxy_course = AlgoliaProxyCourse.objects.get(pk=course.pk) + assert proxy_course.product_weeks_to_complete == 6 + + def test_product_weeks_to_complete_ignores_invalid_or_none_values(self): + """ + Ensure that product_weeks_to_complete returns None + when the advertised course run has an invalid or None weeks_to_complete value. + """ + course = self.create_course_with_basic_active_course_run() + course.authoring_organizations.add(OrganizationFactory()) + + advertised_run = course.advertised_course_run + advertised_run.weeks_to_complete = None + advertised_run.save() + + proxy_course = AlgoliaProxyCourse.objects.get(pk=course.pk) + assert proxy_course.product_weeks_to_complete is None + + # Now test with invalid value (e.g., 0 weeks) + advertised_run.weeks_to_complete = 0 + advertised_run.save() + proxy_course = AlgoliaProxyCourse.objects.get(pk=course.pk) + assert proxy_course.product_weeks_to_complete is None @ddt.ddt @pytest.mark.django_db class TestAlgoliaProxyProgram(TestAlgoliaProxyWithEdxPartner): diff --git a/docs/conf.py b/docs/conf.py index f81226a16b..ce3357696f 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -11,8 +11,6 @@ import datetime import os - - # on_rtd is whether we are on readthedocs.org on_rtd = os.environ.get('READTHEDOCS', None) == 'True'