Skip to content

Commit 9f5720b

Browse files
authored
Avoid n+1 queries on video.playlists serializer field (#2662)
1 parent 0c13fef commit 9f5720b

File tree

5 files changed

+62
-11
lines changed

5 files changed

+62
-11
lines changed

learning_resources/filters_test.py

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -446,15 +446,18 @@ def test_learning_resource_filter_course_features(client):
446446
"""Test that the resource_content_tag filter works"""
447447

448448
resource_with_exams = LearningResourceFactory.create(
449-
content_tags=LearningResourceContentTagFactory.create_batch(1, name="Exams")
449+
content_tags=LearningResourceContentTagFactory.create_batch(1, name="Exams"),
450+
resource_type=LearningResourceType.video.name,
450451
)
451452
resource_with_notes = LearningResourceFactory.create(
452453
content_tags=LearningResourceContentTagFactory.create_batch(
453454
1, name="Lecture Notes"
454-
)
455+
),
456+
resource_type=LearningResourceType.video.name,
455457
)
456458
LearningResourceFactory.create(
457-
content_tags=LearningResourceContentTagFactory.create_batch(1, name="Other")
459+
content_tags=LearningResourceContentTagFactory.create_batch(1, name="Other"),
460+
resource_type=LearningResourceType.video.name,
458461
)
459462

460463
results = client.get(f"{RESOURCE_API_URL}?course_feature=exams").json()["results"]

learning_resources/models.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -390,6 +390,13 @@ def for_serialization(self, *, user: Optional["User"] = None):
390390
),
391391
to_attr="_podcasts",
392392
),
393+
Prefetch(
394+
"parents",
395+
queryset=LearningResourceRelationship.objects.filter(
396+
relation_type=LearningResourceRelationTypes.PLAYLIST_VIDEOS.value,
397+
),
398+
to_attr="_playlists",
399+
),
393400
Prefetch(
394401
"children",
395402
queryset=LearningResourceRelationship.objects.select_related(
@@ -604,6 +611,17 @@ def podcasts(self) -> list["LearningResourceRelationship"]:
604611
),
605612
)
606613

614+
@cached_property
615+
def playlists(self) -> list["LearningResourceRelationship"]:
616+
"""Return a list of playlists that the resource is in"""
617+
return getattr(
618+
self,
619+
"_playlists",
620+
self.parents.filter(
621+
relation_type=LearningResourceRelationTypes.PLAYLIST_VIDEOS.value,
622+
),
623+
)
624+
607625
class Meta:
608626
unique_together = (("platform", "readable_id", "resource_type"),)
609627

learning_resources/serializers.py

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1097,11 +1097,7 @@ class VideoResourceSerializer(LearningResourceBaseSerializer):
10971097

10981098
def get_playlists(self, instance) -> list[str]:
10991099
"""Get the playlist id(s) the video belongs to"""
1100-
return list(
1101-
instance.parents.filter(
1102-
relation_type=constants.LearningResourceRelationTypes.PLAYLIST_VIDEOS.value
1103-
).values_list("parent__id", flat=True)
1104-
)
1100+
return [playlist.parent_id for playlist in instance.playlists]
11051101

11061102

11071103
class VideoPlaylistResourceSerializer(LearningResourceBaseSerializer):

learning_resources/serializers_test.py

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,11 @@
3333
LearningResourcePriceFactory,
3434
LearningResourceRunFactory,
3535
)
36-
from learning_resources.models import ContentFile, LearningResource
36+
from learning_resources.models import (
37+
ContentFile,
38+
LearningResource,
39+
LearningResourceRelationship,
40+
)
3741
from main.test_utils import assert_json_equal, drf_datetime
3842
from main.utils import frontend_absolute_url
3943

@@ -141,6 +145,36 @@ def test_serialize_podcast_episode_to_json():
141145
)
142146

143147

148+
def test_serialize_video_resource_playlists_to_json():
149+
"""
150+
Verify that a serialized video resource has the correct playlist data
151+
"""
152+
playlist = factories.VideoPlaylistFactory.create()
153+
video = factories.VideoFactory.create()
154+
LearningResourceRelationship.objects.get_or_create(
155+
parent=playlist.learning_resource,
156+
child=video.learning_resource,
157+
relation_type=LearningResourceRelationTypes.PLAYLIST_VIDEOS.value,
158+
)
159+
serializer = serializers.VideoResourceSerializer(instance=video.learning_resource)
160+
assert serializer.data["playlists"] == [playlist.learning_resource.id]
161+
162+
163+
def test_serialize_podcast_episode_playlists_to_json():
164+
"""
165+
Verify that a serialized podcast episode resource has the correct podcast data
166+
"""
167+
podcast = factories.PodcastFactory.create()
168+
podcast_episode = factories.PodcastEpisodeFactory.create()
169+
LearningResourceRelationship.objects.get_or_create(
170+
parent=podcast.learning_resource,
171+
child=podcast_episode.learning_resource,
172+
relation_type=LearningResourceRelationTypes.PODCAST_EPISODES.value,
173+
)
174+
serializer = serializers.PodcastEpisodeSerializer(instance=podcast_episode)
175+
assert serializer.data["podcasts"] == [podcast.learning_resource.id]
176+
177+
144178
@pytest.mark.parametrize("has_context", [True, False])
145179
@pytest.mark.parametrize(
146180
("params", "detail_key", "specific_serializer_cls", "detail_serializer_cls"),

learning_resources/views_test.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ def test_program_detail_endpoint(client, django_assert_num_queries, url):
194194
"""Test program endpoint"""
195195
program = ProgramFactory.create()
196196
assert program.learning_resource.children.count() > 0
197-
with django_assert_num_queries(20): # should be same # regardless of child count
197+
with django_assert_num_queries(21): # should be same # regardless of child count
198198
resp = client.get(reverse(url, args=[program.learning_resource.id]))
199199
assert resp.data.get("title") == program.learning_resource.title
200200
assert resp.data.get("resource_type") == LearningResourceType.program.name
@@ -239,7 +239,7 @@ def test_no_excess_queries(rf, user, mocker, django_assert_num_queries, course_c
239239
request = rf.get("/")
240240
request.user = user
241241

242-
with django_assert_num_queries(22):
242+
with django_assert_num_queries(23):
243243
view = CourseViewSet(request=request)
244244
results = view.get_queryset().all()
245245
assert len(results) == course_count

0 commit comments

Comments
 (0)