Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix facility n+1 #2089

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions care/facility/api/serializers/facility.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,13 @@ class FacilityBasicInfoSerializer(serializers.ModelSerializer):
bed_count = serializers.SerializerMethodField()

def get_bed_count(self, facility):
if hasattr(facility, "bed_count"):
return facility.bed_count
return Bed.objects.filter(facility=facility).count()

def get_patient_count(self, facility):
if hasattr(facility, "patientregistrations_count"):
return facility.patientregistrations_count
return PatientRegistration.objects.filter(
facility=facility, is_active=True
).count()
Expand Down
31 changes: 29 additions & 2 deletions care/facility/api/viewsets/facility.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
from django.conf import settings
from django.db.models import Count, OuterRef, Subquery
from django.db.models.functions import Coalesce
from django_filters import rest_framework as filters
from djqscsv import render_to_csv_response
from drf_spectacular.utils import extend_schema, extend_schema_view
Expand All @@ -21,7 +23,9 @@
FacilityPatientStatsHistory,
HospitalDoctors,
)
from care.facility.models.bed import Bed
from care.facility.models.facility import FacilityUser
from care.facility.models.patient import PatientRegistration
from care.users.models import User


Expand Down Expand Up @@ -71,8 +75,31 @@ class FacilityViewSet(
):
"""Viewset for facility CRUD operations."""

queryset = Facility.objects.all().select_related(
"ward", "local_body", "district", "state"
patient_registration_count_subquery = Subquery(
PatientRegistration.objects.filter(facility=OuterRef("pk"))
.values("facility")
.annotate(count=Count("id"))
.values("count")
)
bed_count_subquery = Subquery(
Bed.objects.filter(facility=OuterRef("pk"))
.values("facility")
.annotate(count=Count("id"))
.values("count")
)

queryset = (
Facility.objects.all()
.select_related(
"ward",
"local_body",
"district",
"state",
)
.annotate(
patientregistrations_count=Coalesce(patient_registration_count_subquery, 0)
)
.annotate(bed_count=Coalesce(bed_count_subquery, 0))
)
permission_classes = (
IsAuthenticated,
Expand Down
22 changes: 22 additions & 0 deletions care/facility/tests/test_facility_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,3 +91,25 @@ def test_delete_with_active_patients(self):
self.client.force_authenticate(user=state_admin)
response = self.client.delete(f"/api/v1/facility/{facility.external_id}/")
self.assertIs(response.status_code, status.HTTP_400_BAD_REQUEST)

def test_listing_db_queries(self):
"""This test case is to test the get api endpoint for facility and to ensure that only one db query is made for fetching the list of facilities"""
facility_1 = self.create_facility(
self.super_user, self.district, self.local_body
)
facility_2 = self.create_facility(
self.super_user, self.district, self.local_body
)
facility_3 = self.create_facility(
self.super_user, self.district, self.local_body
)
self.create_patient(self.district, facility_1)
self.create_patient(self.district, facility_2)
self.create_patient(self.district, facility_3)

self.client.force_authenticate(user=self.super_user)

with self.assertNumQueries(2):
response = self.client.get("/api/v1/facility/")

self.assertIs(response.status_code, status.HTTP_200_OK)
Loading