From 8af2a7f1ed1d13744515b59386c3b7d1f877f04f Mon Sep 17 00:00:00 2001 From: Matt Bertrand Date: Mon, 25 Aug 2025 09:36:19 -0400 Subject: [PATCH 1/2] . --- learning_resources/models.py | 18 +++++++++++++++++ learning_resources/serializers.py | 32 +++++++++++++++++++++++++------ 2 files changed, 44 insertions(+), 6 deletions(-) diff --git a/learning_resources/models.py b/learning_resources/models.py index b6670ed1d2..35d8ffcc03 100644 --- a/learning_resources/models.py +++ b/learning_resources/models.py @@ -384,6 +384,13 @@ def for_serialization(self, *, user: Optional["User"] = None): ), to_attr="_podcasts", ), + Prefetch( + "children", + queryset=LearningResourceRelationship.objects.filter( + relation_type=LearningResourceRelationTypes.PROGRAM_COURSES.value, + ), + to_attr="_courses", + ), Prefetch( "user_lists", queryset=UserListRelationship.objects.filter(parent__author=user) @@ -545,6 +552,17 @@ def podcasts(self) -> list["LearningResourceRelationship"]: ), ) + @cached_property + def courses(self) -> list["LearningResourceRelationship"]: + """Return a list of courses that a resource contains""" + return getattr( + self, + "_courses", + self.children.filter( + relation_type=LearningResourceRelationTypes.PROGRAM_COURSES.value, + ).order_by("position"), + ) + class Meta: unique_together = (("platform", "readable_id", "resource_type"),) diff --git a/learning_resources/serializers.py b/learning_resources/serializers.py index 2dafad3aef..296a2d73ad 100644 --- a/learning_resources/serializers.py +++ b/learning_resources/serializers.py @@ -444,6 +444,26 @@ class Meta: fields = ("id", "parent", "child") +class MicroProgramRelationshipSerializer(serializers.ModelSerializer): + """ + Serializer containing only parent and child ids for a program_course_relationship + """ + + child = serializers.ReadOnlyField(source="child_id") + readable_id = serializers.ReadOnlyField(source="child.readable_id") + title = serializers.ReadOnlyField(source="child.title") + + class Meta: + model = models.LearningResourceRelationship + fields = ( + "child", + "position", + "relation_type", + "title", + "readable_id", + ) + + class LearningResourceMetadataDisplaySerializer(serializers.Serializer): """ Serializer to render course information as a text document @@ -910,13 +930,13 @@ class LearningResourceBaseSerializer(serializers.ModelSerializer, WriteableTopic resource_category = serializers.SerializerMethodField() format = serializers.ListField(child=FormatSerializer(), read_only=True) pace = serializers.ListField(child=PaceSerializer(), read_only=True) - children = serializers.SerializerMethodField(allow_null=True) + children = MicroProgramRelationshipSerializer(many=True, read_only=True) - @extend_schema_field(LearningResourceRelationshipChildField(allow_null=True)) - def get_children(self, instance): - return LearningResourceRelationshipChildField( - instance.children.order_by("position"), many=True, read_only=True - ).data + # @extend_schema_field(LearningResourceRelationshipChildField(allow_null=True)) + # def get_children(self, instance): + # return LearningResourceRelationshipChildField( + # instance.courses, many=True, read_only=True + # ).data def get_resource_category(self, instance) -> str: """Return the resource category of the resource""" From ea06f4046095cf189eb518d6e3a8cc5796e4fda6 Mon Sep 17 00:00:00 2001 From: Matt Bertrand Date: Tue, 26 Aug 2025 14:31:32 -0400 Subject: [PATCH 2/2] Get rid of n+1 queries --- learning_resources/models.py | 18 +++-------------- learning_resources/serializers.py | 32 ++++++------------------------- learning_resources/views_test.py | 5 +++-- 3 files changed, 12 insertions(+), 43 deletions(-) diff --git a/learning_resources/models.py b/learning_resources/models.py index 35d8ffcc03..c75119311e 100644 --- a/learning_resources/models.py +++ b/learning_resources/models.py @@ -386,10 +386,9 @@ def for_serialization(self, *, user: Optional["User"] = None): ), Prefetch( "children", - queryset=LearningResourceRelationship.objects.filter( - relation_type=LearningResourceRelationTypes.PROGRAM_COURSES.value, - ), - to_attr="_courses", + queryset=LearningResourceRelationship.objects.select_related( + "child" + ).order_by("position"), ), Prefetch( "user_lists", @@ -552,17 +551,6 @@ def podcasts(self) -> list["LearningResourceRelationship"]: ), ) - @cached_property - def courses(self) -> list["LearningResourceRelationship"]: - """Return a list of courses that a resource contains""" - return getattr( - self, - "_courses", - self.children.filter( - relation_type=LearningResourceRelationTypes.PROGRAM_COURSES.value, - ).order_by("position"), - ) - class Meta: unique_together = (("platform", "readable_id", "resource_type"),) diff --git a/learning_resources/serializers.py b/learning_resources/serializers.py index 296a2d73ad..d3efd1676b 100644 --- a/learning_resources/serializers.py +++ b/learning_resources/serializers.py @@ -444,26 +444,6 @@ class Meta: fields = ("id", "parent", "child") -class MicroProgramRelationshipSerializer(serializers.ModelSerializer): - """ - Serializer containing only parent and child ids for a program_course_relationship - """ - - child = serializers.ReadOnlyField(source="child_id") - readable_id = serializers.ReadOnlyField(source="child.readable_id") - title = serializers.ReadOnlyField(source="child.title") - - class Meta: - model = models.LearningResourceRelationship - fields = ( - "child", - "position", - "relation_type", - "title", - "readable_id", - ) - - class LearningResourceMetadataDisplaySerializer(serializers.Serializer): """ Serializer to render course information as a text document @@ -930,13 +910,13 @@ class LearningResourceBaseSerializer(serializers.ModelSerializer, WriteableTopic resource_category = serializers.SerializerMethodField() format = serializers.ListField(child=FormatSerializer(), read_only=True) pace = serializers.ListField(child=PaceSerializer(), read_only=True) - children = MicroProgramRelationshipSerializer(many=True, read_only=True) + children = serializers.SerializerMethodField(allow_null=True) - # @extend_schema_field(LearningResourceRelationshipChildField(allow_null=True)) - # def get_children(self, instance): - # return LearningResourceRelationshipChildField( - # instance.courses, many=True, read_only=True - # ).data + @extend_schema_field(LearningResourceRelationshipChildField(allow_null=True)) + def get_children(self, instance): + return LearningResourceRelationshipChildField( + instance.children, many=True, read_only=True + ).data def get_resource_category(self, instance) -> str: """Return the resource category of the resource""" diff --git a/learning_resources/views_test.py b/learning_resources/views_test.py index a0d5f70786..688313114c 100644 --- a/learning_resources/views_test.py +++ b/learning_resources/views_test.py @@ -174,7 +174,8 @@ def test_program_endpoint(client, url, params): def test_program_detail_endpoint(client, django_assert_num_queries, url): """Test program endpoint""" program = ProgramFactory.create() - with django_assert_num_queries(18 + program.learning_resource.children.count()): + assert program.learning_resource.children.count() > 0 + with django_assert_num_queries(18): # should be same # regardless of child count resp = client.get(reverse(url, args=[program.learning_resource.id])) assert resp.data.get("title") == program.learning_resource.title assert resp.data.get("resource_type") == LearningResourceType.program.name @@ -219,7 +220,7 @@ def test_no_excess_queries(rf, user, mocker, django_assert_num_queries, course_c request = rf.get("/") request.user = user - with django_assert_num_queries(19): + with django_assert_num_queries(20): view = CourseViewSet(request=request) results = view.get_queryset().all() assert len(results) == course_count