From 78187946350bc0f347c3ef05c1a117bee81db238 Mon Sep 17 00:00:00 2001 From: Matt Bertrand Date: Wed, 12 Nov 2025 14:38:00 -0500 Subject: [PATCH 1/5] Refactor next_start_date, add best_run_id --- learning_resources/etl/loaders.py | 8 +-- learning_resources/etl/loaders_test.py | 71 ++++++++++++++++++---- learning_resources/models.py | 81 ++++++++++++++++++-------- learning_resources/serializers.py | 12 +++- learning_resources/serializers_test.py | 8 ++- learning_resources/views.py | 27 +++++++-- learning_resources_search/constants.py | 1 + 7 files changed, 162 insertions(+), 46 deletions(-) diff --git a/learning_resources/etl/loaders.py b/learning_resources/etl/loaders.py index 301eeede58..fd41c89284 100644 --- a/learning_resources/etl/loaders.py +++ b/learning_resources/etl/loaders.py @@ -46,7 +46,6 @@ Video, VideoChannel, VideoPlaylist, - now_in_utc, ) from learning_resources.utils import ( add_parent_topics_to_learning_resource, @@ -139,11 +138,12 @@ def load_run_dependent_values( Returns: tuple[datetime.time | None, list[Decimal], str]: date, prices, and availability """ - 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.next_start_date = ( + max(filter(None, [best_run.start_date, best_run.enrollment_start])) + if best_run.start_date or best_run.enrollment_start + else None ) resource.availability = best_run.availability resource.prices = ( diff --git a/learning_resources/etl/loaders_test.py b/learning_resources/etl/loaders_test.py index 2af2c9b3a2..0b70d899d1 100644 --- a/learning_resources/etl/loaders_test.py +++ b/learning_resources/etl/loaders_test.py @@ -1,6 +1,6 @@ """Tests for ETL loaders""" -from datetime import timedelta +from datetime import datetime, timedelta from decimal import Decimal from pathlib import Path @@ -382,7 +382,7 @@ def test_load_program_bad_platform(mocker): @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, C901 +def test_load_course( # noqa: PLR0913, PLR0912, PLR0915 mock_upsert_tasks, course_exists, is_published, @@ -498,10 +498,9 @@ def test_load_course( # noqa: PLR0913,PLR0912,PLR0915, C901 assert result.professional is True 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.next_start_date == start_date + else: + assert result.next_start_date is None assert result.prices == ( [Decimal("0.00"), Decimal("49.00")] if is_run_published and result.certification @@ -1748,15 +1747,17 @@ def test_load_run_dependent_values(certification): course = LearningResourceFactory.create( is_course=True, certification=certification, runs=[] ) + assert course.runs.count() == 0 closest_date = now_in_utc() + timedelta(days=1) furthest_date = now_in_utc() + timedelta(days=2) - run = LearningResourceRunFactory.create( + best_run = LearningResourceRunFactory.create( learning_resource=course, published=True, availability=Availability.dated.name, prices=[Decimal("0.00"), Decimal("20.00")], resource_prices=LearningResourcePriceFactory.create_batch(2), start_date=closest_date, + enrollment_start=None, location="Portland, ME", duration="3 - 4 weeks", min_weeks=3, @@ -1772,6 +1773,7 @@ def test_load_run_dependent_values(certification): prices=[Decimal("0.00"), Decimal("50.00")], resource_prices=LearningResourcePriceFactory.create_batch(2), start_date=furthest_date, + enrollment_start=None, location="Portland, OR", duration="7 - 9 weeks", min_weeks=7, @@ -1781,15 +1783,23 @@ def test_load_run_dependent_values(certification): max_weekly_hours=19, ) result = load_run_dependent_values(course) - assert result.next_start_date == course.next_start_date == closest_date - assert result.prices == course.prices == ([] if not certification else run.prices) + course.refresh_from_db() + assert ( + result.prices == course.prices == ([] if not certification else best_run.prices) + ) + assert ( + result.next_start_date + == course.next_start_date + == best_run.start_date + == closest_date + ) assert ( list(result.resource_prices) == list(course.resource_prices.all()) - == ([] if not certification else list(run.resource_prices.all())) + == ([] if not certification else list(best_run.resource_prices.all())) ) assert result.availability == course.availability == Availability.dated.name - assert result.location == course.location == run.location + assert result.location == course.location == best_run.location for key in [ "duration", "time_commitment", @@ -1798,7 +1808,7 @@ def test_load_run_dependent_values(certification): "min_weekly_hours", "max_weekly_hours", ]: - assert getattr(result, key) == getattr(course, key) == getattr(run, key) + assert getattr(result, key) == getattr(course, key) == getattr(best_run, key) def test_load_run_dependent_values_resets_next_start_date(): @@ -1829,6 +1839,43 @@ def test_load_run_dependent_values_resets_next_start_date(): assert course.next_start_date is None +@pytest.mark.parametrize( + ("start_dt", "enrollnment_start", "expected_next"), + [ + ("2025-01-01T10:00:00Z", "2024-12-15T10:00:00Z", "2025-01-01T10:00:00Z"), + ("2025-01-01T10:00:00Z", None, "2025-01-01T10:00:00Z"), + (None, "2024-12-15T10:00:00Z", "2024-12-15T10:00:00Z"), + (None, None, None), + ], +) +def test_load_run_dependent_values_next_start_date( + start_dt, enrollnment_start, expected_next +): + """Test that next_start_date is correctly set from the best_run""" + course = LearningResourceFactory.create(is_course=True, published=True, runs=[]) + + # Create multiple runs with different start dates + LearningResourceRunFactory.create( + learning_resource=course, + published=True, + start_date=datetime.fromisoformat(start_dt) if start_dt else None, + enrollment_start=datetime.fromisoformat(enrollnment_start) + if enrollnment_start + else None, + ) + + # Call load_run_dependent_values + result = load_run_dependent_values(course) + + # Refresh course from database + course.refresh_from_db() + + # Verify that next_start_date matches the earliest run's start date + assert result.next_start_date == ( + datetime.fromisoformat(expected_next) if expected_next else None + ) + + @pytest.mark.parametrize( ("is_scholar_course", "tag_counts", "expected_score"), [ diff --git a/learning_resources/models.py b/learning_resources/models.py index ef5391b82b..a3cbf58be1 100644 --- a/learning_resources/models.py +++ b/learning_resources/models.py @@ -367,6 +367,7 @@ def for_serialization(self, *, user: Optional["User"] = None): queryset=LearningResourceRun.objects.filter(published=True) .order_by("start_date", "enrollment_start", "id") .for_serialization(), + to_attr="_published_runs", ), Prefetch( "parents", @@ -537,36 +538,68 @@ def audience(self) -> str | None: @cached_property def best_run(self) -> Optional["LearningResourceRun"]: """Returns the most current/upcoming enrollable run for the learning resource""" - published_runs = self.runs.filter(published=True) + if hasattr(self, "_published_runs"): + published_runs = self._published_runs + else: + published_runs = list(self.runs.filter(published=True)) + + if not published_runs: + return None + 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)) + enrollable_runs = [ + run + for run in published_runs + if ( + (run.enrollment_start and run.enrollment_start <= now) + or ( + not run.enrollment_start + and run.start_date + and run.start_date <= 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() + and ( + (run.enrollment_end and run.enrollment_end > now) + or (not run.enrollment_end and run.end_date and run.end_date > now) ) - 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() + ] + if enrollable_runs: + return min( + enrollable_runs, + key=lambda r: ( + r.start_date or timezone.now(), + r.end_date or timezone.now(), + ), ) - return best_lr_run + + # If no current enrollable run found, find the next upcoming run + upcoming_runs = [ + run + for run in published_runs + if run.start_date and run.start_date >= timezone.now() + ] + if upcoming_runs: + return min(upcoming_runs, key=lambda r: r.start_date) + + # If current_run is still null, return the run with the latest start date + runs_with_dates = [run for run in published_runs if run.start_date] + if runs_with_dates: + return max(runs_with_dates, key=lambda r: r.start_date) + + return published_runs[0] if published_runs else None + + @cached_property + def published_runs(self) -> list["LearningResourceRun"]: + """Return a list of published runs for the resource""" + if hasattr(self, "_published_runs"): + return self._published_runs + return list( + self.runs.filter(published=True) + .order_by("start_date", "enrollment_start", "id") + .for_serialization() + ) @cached_property def views_count(self) -> int: diff --git a/learning_resources/serializers.py b/learning_resources/serializers.py index cd91b5e582..a8200abc8d 100644 --- a/learning_resources/serializers.py +++ b/learning_resources/serializers.py @@ -900,7 +900,9 @@ class LearningResourceBaseSerializer(serializers.ModelSerializer, WriteableTopic read_only=True, ) resource_prices = LearningResourcePriceSerializer(read_only=True, many=True) - runs = LearningResourceRunSerializer(read_only=True, many=True, allow_null=True) + runs = LearningResourceRunSerializer( + source="published_runs", read_only=True, many=True, allow_null=True + ) image = serializers.SerializerMethodField() learning_path_parents = MicroLearningPathRelationshipSerializer( many=True, read_only=True @@ -915,6 +917,7 @@ class LearningResourceBaseSerializer(serializers.ModelSerializer, WriteableTopic format = serializers.ListField(child=FormatSerializer(), read_only=True) pace = serializers.ListField(child=PaceSerializer(), read_only=True) children = serializers.SerializerMethodField(allow_null=True) + best_run_id = serializers.SerializerMethodField(allow_null=True) @extend_schema_field(LearningResourceRelationshipChildField(allow_null=True)) def get_children(self, instance): @@ -922,6 +925,13 @@ def get_children(self, instance): instance.children, many=True, read_only=True ).data + def get_best_run_id(self, instance) -> int | None: + """Return the best run id for the resource, if it has runs""" + best_run = instance.best_run + if best_run: + return best_run.id + return None + def get_resource_category(self, instance) -> str: """Return the resource category of the resource""" if instance.resource_type in [ diff --git a/learning_resources/serializers_test.py b/learning_resources/serializers_test.py index 8a5eb81a85..75cb04d8e8 100644 --- a/learning_resources/serializers_test.py +++ b/learning_resources/serializers_test.py @@ -324,7 +324,7 @@ def test_learning_resource_serializer( # noqa: PLR0913 "ocw_topics": sorted(resource.ocw_topics), "runs": [ serializers.LearningResourceRunSerializer(instance=run).data - for run in resource.runs.all() + for run in resource.published_runs ], detail_key: detail_serializer_cls(instance=getattr(resource, detail_key)).data, "views": resource.views.count(), @@ -351,6 +351,7 @@ def test_learning_resource_serializer( # noqa: PLR0913 "max_weekly_hours": resource.max_weekly_hours, "min_weeks": resource.min_weeks, "max_weeks": resource.max_weeks, + "best_run_id": resource.best_run.id if resource.best_run else None, } @@ -857,6 +858,11 @@ def test_instructors_display(): load_instructors( run, [{"full_name": instructor.full_name} for instructor in instructors] ) + # Clear cached properties so they pick up the new run with instructors + if hasattr(resource, "_published_runs"): + delattr(resource, "_published_runs") + if hasattr(resource, "published_runs"): + del resource.__dict__["published_runs"] serialized_resource = serializers.LearningResourceSerializer(resource).data metadata_serializer = serializers.LearningResourceMetadataDisplaySerializer( serialized_resource diff --git a/learning_resources/views.py b/learning_resources/views.py index 2df588382c..2a9e2123e8 100644 --- a/learning_resources/views.py +++ b/learning_resources/views.py @@ -597,9 +597,17 @@ class LearningResourceListRelationshipViewSet(viewsets.GenericViewSet): permission_classes = (AnonymousAccessReadonlyPermission,) filter_backends = [MultipleOptionsFilterBackend] - queryset = LearningResourceRelationship.objects.select_related("parent", "child") http_method_names = ["patch"] + def get_queryset(self): + """Return queryset with properly prefetched child learning resources""" + return LearningResourceRelationship.objects.prefetch_related( + Prefetch( + "child", + queryset=LearningResource.objects.for_serialization(), + ) + ) + def get_serializer_class(self): if self.action == "userlists": return UserListRelationshipSerializer @@ -963,15 +971,26 @@ class UserListItemViewSet(NestedViewSetMixin, viewsets.ModelViewSet): Viewset for UserListRelationships """ - queryset = UserListRelationship.objects.prefetch_related("child").order_by( - "position" - ) + queryset = UserListRelationship.objects.order_by("position") serializer_class = UserListRelationshipSerializer pagination_class = DefaultPagination permission_classes = (HasUserListItemPermissions,) http_method_names = VALID_HTTP_METHODS parent_lookup_kwargs = {"userlist_id": "parent"} + def get_queryset(self): + """Return queryset with properly prefetched child learning resources""" + user = self.request.user if hasattr(self, "request") else None + # Start with the base queryset which gets filtered by NestedViewSetMixin + qs = super().get_queryset() + # Add prefetch for child learning resources + return qs.prefetch_related( + Prefetch( + "child", + queryset=LearningResource.objects.for_serialization(user=user), + ) + ) + def create(self, request, *args, **kwargs): user_list_id = kwargs.get("userlist_id") request.data["parent"] = user_list_id diff --git a/learning_resources_search/constants.py b/learning_resources_search/constants.py index a1922ca39f..7ceaf6548a 100644 --- a/learning_resources_search/constants.py +++ b/learning_resources_search/constants.py @@ -300,6 +300,7 @@ class FilterConfig: "location": {"type": "keyword"}, }, }, + "best_run_id": {"type": "long"}, "next_start_date": {"type": "date"}, "resource_age_date": {"type": "date"}, "featured_rank": {"type": "float"}, From d99a97bc2282e1d42982306a839987c65efc9f28 Mon Sep 17 00:00:00 2001 From: Matt Bertrand Date: Wed, 12 Nov 2025 14:53:10 -0500 Subject: [PATCH 2/5] OpenAPI spec --- frontends/api/src/generated/v0/api.ts | 48 +++++++++++++++++++ frontends/api/src/generated/v1/api.ts | 48 +++++++++++++++++++ learning_resources_search/serializers_test.py | 8 ++++ openapi/specs/v0.yaml | 48 +++++++++++++++++++ openapi/specs/v1.yaml | 48 +++++++++++++++++++ 5 files changed, 200 insertions(+) diff --git a/frontends/api/src/generated/v0/api.ts b/frontends/api/src/generated/v0/api.ts index fd4558e580..297e53a94b 100644 --- a/frontends/api/src/generated/v0/api.ts +++ b/frontends/api/src/generated/v0/api.ts @@ -177,6 +177,12 @@ export interface ArticleResource { * @memberof ArticleResource */ children: LearningResourceRelationshipChildField | null + /** + * Return the best run id for the resource, if it has runs + * @type {number} + * @memberof ArticleResource + */ + best_run_id: number | null /** * * @type {ArticleResourceResourceTypeEnum} @@ -1338,6 +1344,12 @@ export interface CourseResource { * @memberof CourseResource */ children: LearningResourceRelationshipChildField | null + /** + * Return the best run id for the resource, if it has runs + * @type {number} + * @memberof CourseResource + */ + best_run_id: number | null /** * * @type {CourseResourceResourceTypeEnum} @@ -2383,6 +2395,12 @@ export interface LearningPathResource { * @memberof LearningPathResource */ children: LearningResourceRelationshipChildField | null + /** + * Return the best run id for the resource, if it has runs + * @type {number} + * @memberof LearningPathResource + */ + best_run_id: number | null /** * * @type {LearningPathResourceResourceTypeEnum} @@ -4189,6 +4207,12 @@ export interface PodcastEpisodeResource { * @memberof PodcastEpisodeResource */ children: LearningResourceRelationshipChildField | null + /** + * Return the best run id for the resource, if it has runs + * @type {number} + * @memberof PodcastEpisodeResource + */ + best_run_id: number | null /** * * @type {PodcastEpisodeResourceResourceTypeEnum} @@ -4502,6 +4526,12 @@ export interface PodcastResource { * @memberof PodcastResource */ children: LearningResourceRelationshipChildField | null + /** + * Return the best run id for the resource, if it has runs + * @type {number} + * @memberof PodcastResource + */ + best_run_id: number | null /** * * @type {PodcastResourceResourceTypeEnum} @@ -5217,6 +5247,12 @@ export interface ProgramResource { * @memberof ProgramResource */ children: LearningResourceRelationshipChildField | null + /** + * Return the best run id for the resource, if it has runs + * @type {number} + * @memberof ProgramResource + */ + best_run_id: number | null /** * * @type {ProgramResourceResourceTypeEnum} @@ -6222,6 +6258,12 @@ export interface VideoPlaylistResource { * @memberof VideoPlaylistResource */ children: LearningResourceRelationshipChildField | null + /** + * Return the best run id for the resource, if it has runs + * @type {number} + * @memberof VideoPlaylistResource + */ + best_run_id: number | null /** * * @type {VideoPlaylistResourceResourceTypeEnum} @@ -6535,6 +6577,12 @@ export interface VideoResource { * @memberof VideoResource */ children: LearningResourceRelationshipChildField | null + /** + * Return the best run id for the resource, if it has runs + * @type {number} + * @memberof VideoResource + */ + best_run_id: number | null /** * * @type {VideoResourceResourceTypeEnum} diff --git a/frontends/api/src/generated/v1/api.ts b/frontends/api/src/generated/v1/api.ts index 1886393d3c..f63826786b 100644 --- a/frontends/api/src/generated/v1/api.ts +++ b/frontends/api/src/generated/v1/api.ts @@ -257,6 +257,12 @@ export interface ArticleResource { * @memberof ArticleResource */ children: LearningResourceRelationshipChildField | null + /** + * Return the best run id for the resource, if it has runs + * @type {number} + * @memberof ArticleResource + */ + best_run_id: number | null /** * * @type {ArticleResourceResourceTypeEnum} @@ -1218,6 +1224,12 @@ export interface CourseResource { * @memberof CourseResource */ children: LearningResourceRelationshipChildField | null + /** + * Return the best run id for the resource, if it has runs + * @type {number} + * @memberof CourseResource + */ + best_run_id: number | null /** * * @type {CourseResourceResourceTypeEnum} @@ -2108,6 +2120,12 @@ export interface LearningPathResource { * @memberof LearningPathResource */ children: LearningResourceRelationshipChildField | null + /** + * Return the best run id for the resource, if it has runs + * @type {number} + * @memberof LearningPathResource + */ + best_run_id: number | null /** * * @type {LearningPathResourceResourceTypeEnum} @@ -5514,6 +5532,12 @@ export interface PodcastEpisodeResource { * @memberof PodcastEpisodeResource */ children: LearningResourceRelationshipChildField | null + /** + * Return the best run id for the resource, if it has runs + * @type {number} + * @memberof PodcastEpisodeResource + */ + best_run_id: number | null /** * * @type {PodcastEpisodeResourceResourceTypeEnum} @@ -6004,6 +6028,12 @@ export interface PodcastResource { * @memberof PodcastResource */ children: LearningResourceRelationshipChildField | null + /** + * Return the best run id for the resource, if it has runs + * @type {number} + * @memberof PodcastResource + */ + best_run_id: number | null /** * * @type {PodcastResourceResourceTypeEnum} @@ -6726,6 +6756,12 @@ export interface ProgramResource { * @memberof ProgramResource */ children: LearningResourceRelationshipChildField | null + /** + * Return the best run id for the resource, if it has runs + * @type {number} + * @memberof ProgramResource + */ + best_run_id: number | null /** * * @type {ProgramResourceResourceTypeEnum} @@ -7823,6 +7859,12 @@ export interface VideoPlaylistResource { * @memberof VideoPlaylistResource */ children: LearningResourceRelationshipChildField | null + /** + * Return the best run id for the resource, if it has runs + * @type {number} + * @memberof VideoPlaylistResource + */ + best_run_id: number | null /** * * @type {VideoPlaylistResourceResourceTypeEnum} @@ -8301,6 +8343,12 @@ export interface VideoResource { * @memberof VideoResource */ children: LearningResourceRelationshipChildField | null + /** + * Return the best run id for the resource, if it has runs + * @type {number} + * @memberof VideoResource + */ + best_run_id: number | null /** * * @type {VideoResourceResourceTypeEnum} diff --git a/learning_resources_search/serializers_test.py b/learning_resources_search/serializers_test.py index 0c56a0466f..004abe67bd 100644 --- a/learning_resources_search/serializers_test.py +++ b/learning_resources_search/serializers_test.py @@ -157,6 +157,8 @@ "resource_type": "course", "platform": {"name": "globalalumni"}, "is_learning_material": False, + "next_start_date": "2023-09-26T06:00:00Z", + "best_run_id": 633, }, } ], @@ -302,6 +304,8 @@ "resource_type": "course", "platform": {"name": "globalalumni"}, "is_learning_material": False, + "next_start_date": "2023-09-26T06:00:00Z", + "best_run_id": 633, } ], "metadata": { @@ -377,6 +381,8 @@ "course_feature": [], "is_learning_material": True, "user_list_parents": [], + "next_start_date": None, + "best_run_id": None, }, } ], @@ -551,6 +557,8 @@ "course_feature": [], "is_learning_material": True, "user_list_parents": [], + "next_start_date": None, + "best_run_id": None, } ], "metadata": { diff --git a/openapi/specs/v0.yaml b/openapi/specs/v0.yaml index 5c23c5dbb1..7a0ceee17b 100644 --- a/openapi/specs/v0.yaml +++ b/openapi/specs/v0.yaml @@ -1650,6 +1650,11 @@ components: - $ref: '#/components/schemas/LearningResourceRelationshipChildField' nullable: true readOnly: true + best_run_id: + type: integer + nullable: true + description: Return the best run id for the resource, if it has runs + readOnly: true resource_type: allOf: - $ref: '#/components/schemas/ArticleResourceResourceTypeEnum' @@ -1746,6 +1751,7 @@ components: type: boolean readOnly: true required: + - best_run_id - certification - certification_type - children @@ -2475,6 +2481,11 @@ components: - $ref: '#/components/schemas/LearningResourceRelationshipChildField' nullable: true readOnly: true + best_run_id: + type: integer + nullable: true + description: Return the best run id for the resource, if it has runs + readOnly: true resource_type: allOf: - $ref: '#/components/schemas/CourseResourceResourceTypeEnum' @@ -2575,6 +2586,7 @@ components: type: boolean readOnly: true required: + - best_run_id - certification - certification_type - children @@ -3130,6 +3142,11 @@ components: - $ref: '#/components/schemas/LearningResourceRelationshipChildField' nullable: true readOnly: true + best_run_id: + type: integer + nullable: true + description: Return the best run id for the resource, if it has runs + readOnly: true resource_type: allOf: - $ref: '#/components/schemas/LearningPathResourceResourceTypeEnum' @@ -3228,6 +3245,7 @@ components: require_summaries: type: boolean required: + - best_run_id - certification - certification_type - children @@ -4493,6 +4511,11 @@ components: - $ref: '#/components/schemas/LearningResourceRelationshipChildField' nullable: true readOnly: true + best_run_id: + type: integer + nullable: true + description: Return the best run id for the resource, if it has runs + readOnly: true resource_type: allOf: - $ref: '#/components/schemas/PodcastEpisodeResourceResourceTypeEnum' @@ -4593,6 +4616,7 @@ components: type: boolean readOnly: true required: + - best_run_id - certification - certification_type - children @@ -4779,6 +4803,11 @@ components: - $ref: '#/components/schemas/LearningResourceRelationshipChildField' nullable: true readOnly: true + best_run_id: + type: integer + nullable: true + description: Return the best run id for the resource, if it has runs + readOnly: true resource_type: allOf: - $ref: '#/components/schemas/PodcastResourceResourceTypeEnum' @@ -4879,6 +4908,7 @@ components: type: boolean readOnly: true required: + - best_run_id - certification - certification_type - children @@ -5343,6 +5373,11 @@ components: - $ref: '#/components/schemas/LearningResourceRelationshipChildField' nullable: true readOnly: true + best_run_id: + type: integer + nullable: true + description: Return the best run id for the resource, if it has runs + readOnly: true resource_type: allOf: - $ref: '#/components/schemas/ProgramResourceResourceTypeEnum' @@ -5443,6 +5478,7 @@ components: type: boolean readOnly: true required: + - best_run_id - certification - certification_type - children @@ -6067,6 +6103,11 @@ components: - $ref: '#/components/schemas/LearningResourceRelationshipChildField' nullable: true readOnly: true + best_run_id: + type: integer + nullable: true + description: Return the best run id for the resource, if it has runs + readOnly: true resource_type: allOf: - $ref: '#/components/schemas/VideoPlaylistResourceResourceTypeEnum' @@ -6167,6 +6208,7 @@ components: type: boolean readOnly: true required: + - best_run_id - certification - certification_type - children @@ -6353,6 +6395,11 @@ components: - $ref: '#/components/schemas/LearningResourceRelationshipChildField' nullable: true readOnly: true + best_run_id: + type: integer + nullable: true + description: Return the best run id for the resource, if it has runs + readOnly: true resource_type: allOf: - $ref: '#/components/schemas/VideoResourceResourceTypeEnum' @@ -6459,6 +6506,7 @@ components: type: boolean readOnly: true required: + - best_run_id - certification - certification_type - children diff --git a/openapi/specs/v1.yaml b/openapi/specs/v1.yaml index 59c2904dde..9d7ea6c9b7 100644 --- a/openapi/specs/v1.yaml +++ b/openapi/specs/v1.yaml @@ -9757,6 +9757,11 @@ components: - $ref: '#/components/schemas/LearningResourceRelationshipChildField' nullable: true readOnly: true + best_run_id: + type: integer + nullable: true + description: Return the best run id for the resource, if it has runs + readOnly: true resource_type: allOf: - $ref: '#/components/schemas/ArticleResourceResourceTypeEnum' @@ -9853,6 +9858,7 @@ components: type: boolean readOnly: true required: + - best_run_id - certification - certification_type - children @@ -10435,6 +10441,11 @@ components: - $ref: '#/components/schemas/LearningResourceRelationshipChildField' nullable: true readOnly: true + best_run_id: + type: integer + nullable: true + description: Return the best run id for the resource, if it has runs + readOnly: true resource_type: allOf: - $ref: '#/components/schemas/CourseResourceResourceTypeEnum' @@ -10535,6 +10546,7 @@ components: type: boolean readOnly: true required: + - best_run_id - certification - certification_type - children @@ -11007,6 +11019,11 @@ components: - $ref: '#/components/schemas/LearningResourceRelationshipChildField' nullable: true readOnly: true + best_run_id: + type: integer + nullable: true + description: Return the best run id for the resource, if it has runs + readOnly: true resource_type: allOf: - $ref: '#/components/schemas/LearningPathResourceResourceTypeEnum' @@ -11105,6 +11122,7 @@ components: require_summaries: type: boolean required: + - best_run_id - certification - certification_type - children @@ -13629,6 +13647,11 @@ components: - $ref: '#/components/schemas/LearningResourceRelationshipChildField' nullable: true readOnly: true + best_run_id: + type: integer + nullable: true + description: Return the best run id for the resource, if it has runs + readOnly: true resource_type: allOf: - $ref: '#/components/schemas/PodcastEpisodeResourceResourceTypeEnum' @@ -13729,6 +13752,7 @@ components: type: boolean readOnly: true required: + - best_run_id - certification - certification_type - children @@ -14041,6 +14065,11 @@ components: - $ref: '#/components/schemas/LearningResourceRelationshipChildField' nullable: true readOnly: true + best_run_id: + type: integer + nullable: true + description: Return the best run id for the resource, if it has runs + readOnly: true resource_type: allOf: - $ref: '#/components/schemas/PodcastResourceResourceTypeEnum' @@ -14141,6 +14170,7 @@ components: type: boolean readOnly: true required: + - best_run_id - certification - certification_type - children @@ -14591,6 +14621,11 @@ components: - $ref: '#/components/schemas/LearningResourceRelationshipChildField' nullable: true readOnly: true + best_run_id: + type: integer + nullable: true + description: Return the best run id for the resource, if it has runs + readOnly: true resource_type: allOf: - $ref: '#/components/schemas/ProgramResourceResourceTypeEnum' @@ -14691,6 +14726,7 @@ components: type: boolean readOnly: true required: + - best_run_id - certification - certification_type - children @@ -15350,6 +15386,11 @@ components: - $ref: '#/components/schemas/LearningResourceRelationshipChildField' nullable: true readOnly: true + best_run_id: + type: integer + nullable: true + description: Return the best run id for the resource, if it has runs + readOnly: true resource_type: allOf: - $ref: '#/components/schemas/VideoPlaylistResourceResourceTypeEnum' @@ -15450,6 +15491,7 @@ components: type: boolean readOnly: true required: + - best_run_id - certification - certification_type - children @@ -15748,6 +15790,11 @@ components: - $ref: '#/components/schemas/LearningResourceRelationshipChildField' nullable: true readOnly: true + best_run_id: + type: integer + nullable: true + description: Return the best run id for the resource, if it has runs + readOnly: true resource_type: allOf: - $ref: '#/components/schemas/VideoResourceResourceTypeEnum' @@ -15854,6 +15901,7 @@ components: type: boolean readOnly: true required: + - best_run_id - certification - certification_type - children From 0dd8683fc581d1bdf20dbd0fc7bf7a66a3e246dc Mon Sep 17 00:00:00 2001 From: Matt Bertrand Date: Thu, 13 Nov 2025 09:14:19 -0500 Subject: [PATCH 3/5] Revert some changes meant for another PR --- learning_resources/views.py | 27 ++++----------------------- 1 file changed, 4 insertions(+), 23 deletions(-) diff --git a/learning_resources/views.py b/learning_resources/views.py index 2a9e2123e8..2df588382c 100644 --- a/learning_resources/views.py +++ b/learning_resources/views.py @@ -597,17 +597,9 @@ class LearningResourceListRelationshipViewSet(viewsets.GenericViewSet): permission_classes = (AnonymousAccessReadonlyPermission,) filter_backends = [MultipleOptionsFilterBackend] + queryset = LearningResourceRelationship.objects.select_related("parent", "child") http_method_names = ["patch"] - def get_queryset(self): - """Return queryset with properly prefetched child learning resources""" - return LearningResourceRelationship.objects.prefetch_related( - Prefetch( - "child", - queryset=LearningResource.objects.for_serialization(), - ) - ) - def get_serializer_class(self): if self.action == "userlists": return UserListRelationshipSerializer @@ -971,26 +963,15 @@ class UserListItemViewSet(NestedViewSetMixin, viewsets.ModelViewSet): Viewset for UserListRelationships """ - queryset = UserListRelationship.objects.order_by("position") + queryset = UserListRelationship.objects.prefetch_related("child").order_by( + "position" + ) serializer_class = UserListRelationshipSerializer pagination_class = DefaultPagination permission_classes = (HasUserListItemPermissions,) http_method_names = VALID_HTTP_METHODS parent_lookup_kwargs = {"userlist_id": "parent"} - def get_queryset(self): - """Return queryset with properly prefetched child learning resources""" - user = self.request.user if hasattr(self, "request") else None - # Start with the base queryset which gets filtered by NestedViewSetMixin - qs = super().get_queryset() - # Add prefetch for child learning resources - return qs.prefetch_related( - Prefetch( - "child", - queryset=LearningResource.objects.for_serialization(user=user), - ) - ) - def create(self, request, *args, **kwargs): user_list_id = kwargs.get("userlist_id") request.data["parent"] = user_list_id From ae56d0868782cd7903525b7910534e518ef4507b Mon Sep 17 00:00:00 2001 From: Matt Bertrand Date: Fri, 14 Nov 2025 11:01:54 -0500 Subject: [PATCH 4/5] Revcert changes to next_start_date (will be done in subsequent PR) --- learning_resources/etl/loaders.py | 8 +-- learning_resources/etl/loaders_test.py | 71 +++++--------------------- 2 files changed, 16 insertions(+), 63 deletions(-) diff --git a/learning_resources/etl/loaders.py b/learning_resources/etl/loaders.py index fd41c89284..301eeede58 100644 --- a/learning_resources/etl/loaders.py +++ b/learning_resources/etl/loaders.py @@ -46,6 +46,7 @@ Video, VideoChannel, VideoPlaylist, + now_in_utc, ) from learning_resources.utils import ( add_parent_topics_to_learning_resource, @@ -138,12 +139,11 @@ def load_run_dependent_values( Returns: tuple[datetime.time | None, list[Decimal], str]: date, prices, and availability """ + now = now_in_utc() best_run = resource.best_run if resource.published and best_run: - resource.next_start_date = ( - max(filter(None, [best_run.start_date, best_run.enrollment_start])) - if best_run.start_date or best_run.enrollment_start - else None + resource.next_start_date = max( + best_run.start_date or best_run.enrollment_start or now, now ) resource.availability = best_run.availability resource.prices = ( diff --git a/learning_resources/etl/loaders_test.py b/learning_resources/etl/loaders_test.py index 0b70d899d1..2af2c9b3a2 100644 --- a/learning_resources/etl/loaders_test.py +++ b/learning_resources/etl/loaders_test.py @@ -1,6 +1,6 @@ """Tests for ETL loaders""" -from datetime import datetime, timedelta +from datetime import timedelta from decimal import Decimal from pathlib import Path @@ -382,7 +382,7 @@ def test_load_program_bad_platform(mocker): @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, @@ -498,9 +498,10 @@ def test_load_course( # noqa: PLR0913, PLR0912, PLR0915 assert result.professional is True if is_published and is_run_published and not blocklisted: - assert result.next_start_date == start_date - else: - assert result.next_start_date is None + 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 @@ -1747,17 +1748,15 @@ def test_load_run_dependent_values(certification): course = LearningResourceFactory.create( is_course=True, certification=certification, runs=[] ) - assert course.runs.count() == 0 closest_date = now_in_utc() + timedelta(days=1) furthest_date = now_in_utc() + timedelta(days=2) - best_run = LearningResourceRunFactory.create( + run = LearningResourceRunFactory.create( learning_resource=course, published=True, availability=Availability.dated.name, prices=[Decimal("0.00"), Decimal("20.00")], resource_prices=LearningResourcePriceFactory.create_batch(2), start_date=closest_date, - enrollment_start=None, location="Portland, ME", duration="3 - 4 weeks", min_weeks=3, @@ -1773,7 +1772,6 @@ def test_load_run_dependent_values(certification): prices=[Decimal("0.00"), Decimal("50.00")], resource_prices=LearningResourcePriceFactory.create_batch(2), start_date=furthest_date, - enrollment_start=None, location="Portland, OR", duration="7 - 9 weeks", min_weeks=7, @@ -1783,23 +1781,15 @@ def test_load_run_dependent_values(certification): max_weekly_hours=19, ) result = load_run_dependent_values(course) - course.refresh_from_db() - assert ( - result.prices == course.prices == ([] if not certification else best_run.prices) - ) - assert ( - result.next_start_date - == course.next_start_date - == best_run.start_date - == closest_date - ) + assert result.next_start_date == course.next_start_date == closest_date + assert result.prices == course.prices == ([] if not certification else run.prices) assert ( list(result.resource_prices) == list(course.resource_prices.all()) - == ([] if not certification else list(best_run.resource_prices.all())) + == ([] if not certification else list(run.resource_prices.all())) ) assert result.availability == course.availability == Availability.dated.name - assert result.location == course.location == best_run.location + assert result.location == course.location == run.location for key in [ "duration", "time_commitment", @@ -1808,7 +1798,7 @@ def test_load_run_dependent_values(certification): "min_weekly_hours", "max_weekly_hours", ]: - assert getattr(result, key) == getattr(course, key) == getattr(best_run, key) + assert getattr(result, key) == getattr(course, key) == getattr(run, key) def test_load_run_dependent_values_resets_next_start_date(): @@ -1839,43 +1829,6 @@ def test_load_run_dependent_values_resets_next_start_date(): assert course.next_start_date is None -@pytest.mark.parametrize( - ("start_dt", "enrollnment_start", "expected_next"), - [ - ("2025-01-01T10:00:00Z", "2024-12-15T10:00:00Z", "2025-01-01T10:00:00Z"), - ("2025-01-01T10:00:00Z", None, "2025-01-01T10:00:00Z"), - (None, "2024-12-15T10:00:00Z", "2024-12-15T10:00:00Z"), - (None, None, None), - ], -) -def test_load_run_dependent_values_next_start_date( - start_dt, enrollnment_start, expected_next -): - """Test that next_start_date is correctly set from the best_run""" - course = LearningResourceFactory.create(is_course=True, published=True, runs=[]) - - # Create multiple runs with different start dates - LearningResourceRunFactory.create( - learning_resource=course, - published=True, - start_date=datetime.fromisoformat(start_dt) if start_dt else None, - enrollment_start=datetime.fromisoformat(enrollnment_start) - if enrollnment_start - else None, - ) - - # Call load_run_dependent_values - result = load_run_dependent_values(course) - - # Refresh course from database - course.refresh_from_db() - - # Verify that next_start_date matches the earliest run's start date - assert result.next_start_date == ( - datetime.fromisoformat(expected_next) if expected_next else None - ) - - @pytest.mark.parametrize( ("is_scholar_course", "tag_counts", "expected_score"), [ From 7365408a2c3d52835acf856751d7798e3d3b84ef Mon Sep 17 00:00:00 2001 From: Matt Bertrand Date: Wed, 19 Nov 2025 12:30:42 -0500 Subject: [PATCH 5/5] Clearer docstrings --- learning_resources/models.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/learning_resources/models.py b/learning_resources/models.py index a3cbf58be1..33317c4169 100644 --- a/learning_resources/models.py +++ b/learning_resources/models.py @@ -574,7 +574,7 @@ def best_run(self) -> Optional["LearningResourceRun"]: ), ) - # If no current enrollable run found, find the next upcoming run + # If no enrollable runs found, find the next upcoming run upcoming_runs = [ run for run in published_runs @@ -583,7 +583,7 @@ def best_run(self) -> Optional["LearningResourceRun"]: if upcoming_runs: return min(upcoming_runs, key=lambda r: r.start_date) - # If current_run is still null, return the run with the latest start date + # No enrollable/upcoming runs, return run with the latest start date runs_with_dates = [run for run in published_runs if run.start_date] if runs_with_dates: return max(runs_with_dates, key=lambda r: r.start_date)