Fix N+1 issue in ops-learning and fix test

Author: szabozoltan69

api/test_views.py

@@ -22,6 +22,7 @@
 from deployments.factories.user import UserFactory
 from dref.models import DrefFile
 from main.test_case import APITestCase
+from per.factories import OpsLearningFactory
 
 
 class AuthPowerBITest(APITestCase):
@@ -144,14 +145,19 @@ def setUp(self):
 
         # Create public field reports
         event_pub = EventFactory.create(visibility=VisibilityChoices.PUBLIC, parent_event=None)
-        FieldReportFactory.create_batch(4, event=event_pub, visibility=VisibilityChoices.PUBLIC)
+        self.public_field_report = FieldReportFactory.create(event=event_pub, visibility=VisibilityChoices.PUBLIC)
+        FieldReportFactory.create_batch(3, event=event_pub, visibility=VisibilityChoices.PUBLIC)
         # Create non-public field reports
         event_non_pub = EventFactory.create(visibility=VisibilityChoices.IFRC, parent_event=None)
         FieldReportFactory.create_batch(5, event=event_non_pub, visibility=VisibilityChoices.IFRC)
 
+        self.ops_learning = OpsLearningFactory.create(is_validated=True)
+
     def test_guest_user_permission(self):
         body = {}
         id = 1  # NOTE: id is used just to test api that requires id, it doesnot indicate real id. It can be any number.
+        field_report_id = self.public_field_report.id
+        ops_learning_id = self.ops_learning.id
 
         guest_apis = [
             "/api/v2/add_subscription/",
@@ -161,12 +167,12 @@ def test_guest_user_permission(self):
         guest_get_apis = [
             "/api/v2/user/me/",
             "/api/v2/field-report/",
-            f"/api/v2/field-report/{id}/",
+            f"/api/v2/field-report/{field_report_id}/",
             "/api/v2/language/",
             f"/api/v2/language/{id}/",
             "/api/v2/event/",
             "/api/v2/ops-learning/",
-            f"/api/v2/ops-learning/{id}/",
+            f"/api/v2/ops-learning/{ops_learning_id}/",
         ]
 
         go_post_apis = [

assets

@@ -20,7 +20,7 @@
 from rest_framework.response import Response
 from rest_framework.settings import api_settings
 
-from api.models import Country, Region
+from api.models import AppealDocument, Country, Region
 from deployments.models import SectorTag
 from main.permissions import DenyGuestUserMutationPermission, DenyGuestUserPermission
 from main.utils import SpreadSheetContentNegotiation
@@ -1202,6 +1202,7 @@ def get_queryset(self):
             return qs.select_related(
                 "appeal_code",
             ).prefetch_related(
+                "appeal_code__event__appeals",
                 "sector",
                 "sector_validated",
                 "organization",
@@ -1216,6 +1217,7 @@ def get_queryset(self):
                 "appeal_code",
             )
             .prefetch_related(
+                "appeal_code__event__appeals",
                 "sector",
                 "sector_validated",
                 "organization",
@@ -1226,6 +1228,26 @@ def get_queryset(self):
             )
         )
 
+    def get_serializer_context(self):
+        context = super().get_serializer_context()
+        if getattr(self, "swagger_fake_view", False):
+            return context
+        view_action = getattr(self, "action", None)
+        if view_action == "list":
+            queryset = self.filter_queryset(self.get_queryset())
+            appeal_document_ids = list(
+                queryset.exclude(appeal_document_id__isnull=True).values_list("appeal_document_id", flat=True).distinct()
+            )
+            if appeal_document_ids:
+                context["appeal_documents_map"] = AppealDocument.objects.in_bulk(appeal_document_ids)
+        elif view_action == "retrieve":
+            lookup_kwarg = self.lookup_url_kwarg or self.lookup_field
+            if lookup_kwarg in self.kwargs:
+                obj = self.get_object()
+                if obj.appeal_document_id:
+                    context["appeal_documents_map"] = AppealDocument.objects.in_bulk([obj.appeal_document_id])
+        return context
+
     def get_serializer_class(self):
         if self.request.method == "GET":
             request_format = self.request.GET.get("format", "json")

per/drf_views.py

@@ -1054,14 +1054,20 @@ class Meta:
         fields = "__all__"
         read_only_fields = ("created_at", "modified_at")
 
-    @staticmethod
-    def get_document_url(obj):
-        if obj.appeal_document_id and (document := AppealDocument.objects.filter(id=obj.appeal_document_id).first()):
+    def _get_document(self, obj):
+        if not obj.appeal_document_id:
+            return None
+        document_map = self.context.get("appeal_documents_map")
+        if document_map is not None:
+            return document_map.get(obj.appeal_document_id)
+        return AppealDocument.objects.filter(id=obj.appeal_document_id).first()
+
+    def get_document_url(self, obj):
+        if document := self._get_document(obj):
             return document.document_url
 
-    @staticmethod
-    def get_document_name(obj):
-        if obj.appeal_document_id and (document := AppealDocument.objects.filter(id=obj.appeal_document_id).first()):
+    def get_document_name(self, obj):
+        if document := self._get_document(obj):
             return document.name
 
 
@@ -1088,14 +1094,20 @@ class Meta:
         )
         exclude = ("learning", "type", "organization", "sector", "per_component")
 
-    @staticmethod
-    def get_document_url(obj):
-        if obj.appeal_document_id and (document := AppealDocument.objects.filter(id=obj.appeal_document_id).first()):
+    def _get_document(self, obj):
+        if not obj.appeal_document_id:
+            return None
+        document_map = self.context.get("appeal_documents_map")
+        if document_map is not None:
+            return document_map.get(obj.appeal_document_id)
+        return AppealDocument.objects.filter(id=obj.appeal_document_id).first()
+
+    def get_document_url(self, obj):
+        if document := self._get_document(obj):
             return document.document_url
 
-    @staticmethod
-    def get_document_name(obj):
-        if obj.appeal_document_id and (document := AppealDocument.objects.filter(id=obj.appeal_document_id).first()):
+    def get_document_name(self, obj):
+        if document := self._get_document(obj):
             return document.name