Skip to content

Commit

Permalink
Fixed N+1 queries issue in /api/v1/bed/ (#1908)
Browse files Browse the repository at this point in the history
* Fixed N+1 queries issue in /api/v1/bed/

* Feat: PR review changes for issue #1338

Removed is_occupied property from Bed model as it is not used anywhere

Annotated is_occupied field in the queryset instead of _is_occupied

* Feat: PR review changes

Changed is_occupied from SerializerMethodField to Boolean Field

* Feat: PR Review Changes

Annotated is_occupied field irrespective of action

* Added the test case for listing beds API

* add comment explaining number of queries

---------

Co-authored-by: Aakash Singh <[email protected]>
  • Loading branch information
dhruv-goyal-10 and sainak authored Mar 4, 2024
1 parent 48f1d89 commit 8ca6e55
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 6 deletions.
13 changes: 11 additions & 2 deletions care/facility/api/viewsets/bed.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from django.core.exceptions import ValidationError as DjangoValidationError
from django.db import IntegrityError, transaction
from django.db.models import OuterRef, Subquery
from django.db.models import Exists, OuterRef, Subquery
from django_filters import rest_framework as filters
from drf_spectacular.utils import extend_schema, extend_schema_view
from rest_framework import filters as drf_filters
Expand Down Expand Up @@ -50,7 +50,7 @@ class BedViewSet(
):
queryset = (
Bed.objects.all()
.select_related("facility", "location")
.select_related("facility", "location", "location__facility")
.order_by("-created_date")
)
serializer_class = BedSerializer
Expand All @@ -63,6 +63,15 @@ class BedViewSet(
def get_queryset(self):
user = self.request.user
queryset = self.queryset

queryset = queryset.annotate(
is_occupied=Exists(
ConsultationBed.objects.filter(
bed__id=OuterRef("id"), end_date__isnull=True
)
)
)

if user.is_superuser:
pass
elif user.user_type >= User.TYPE_VALUE_MAP["StateLabAdmin"]:
Expand Down
4 changes: 0 additions & 4 deletions care/facility/models/bed.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,6 @@ class Meta:
)
]

@property
def is_occupied(self) -> bool:
return ConsultationBed.objects.filter(bed=self, end_date__isnull=True).exists()

def __str__(self):
return self.name

Expand Down
38 changes: 38 additions & 0 deletions care/facility/tests/test_bed_api.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
from rest_framework import status
from rest_framework.test import APITestCase

from care.facility.models import Bed
from care.utils.tests.test_utils import TestUtils


class BedViewSetTestCase(TestUtils, APITestCase):
@classmethod
def setUpTestData(cls) -> None:
cls.state = cls.create_state()
cls.district = cls.create_district(cls.state)
cls.local_body = cls.create_local_body(cls.district)
cls.super_user = cls.create_super_user("su", cls.district)
cls.facility = cls.create_facility(cls.super_user, cls.district, cls.local_body)
cls.asset_location = cls.create_asset_location(cls.facility)
cls.user = cls.create_user("staff", cls.district, home_facility=cls.facility)
cls.patient = cls.create_patient(
cls.district, cls.facility, local_body=cls.local_body
)

def setUp(self) -> None:
super().setUp()
self.bed1 = Bed.objects.create(
name="bed1", location=self.asset_location, facility=self.facility
)
self.bed2 = Bed.objects.create(
name="bed2", location=self.asset_location, facility=self.facility
)
self.bed3 = Bed.objects.create(
name="bed3", location=self.asset_location, facility=self.facility
)

def test_list_beds(self):
# includes 3 queries for auth and 1 for pagination count
with self.assertNumQueries(5):
response = self.client.get("/api/v1/bed/")
self.assertEqual(response.status_code, status.HTTP_200_OK)

0 comments on commit 8ca6e55

Please sign in to comment.