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.

Sign up for GitHub

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

chore: Add total doctors count to hospital doctor list response #2629

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from
Open

chore: Add total doctors count to hospital doctor list response #2629

Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
42350b4
chore: Add total doctors count to hospital doctor list response
shauryag2002 Dec 2, 2024
5f85647
Handle potential None value from aggregation
shauryag2002 Dec 2, 2024
7074cf0
Merge branch 'ohcnetwork:develop' into backend/auto-scroll-card-number
shauryag2002 Dec 7, 2024
83a2f2b
Reverting hospital_doctor.py
shauryag2002 Dec 7, 2024
bfd4f75
Update hospital_doctor.py
shauryag2002 Dec 7, 2024
fa6f6bd
Update hospital_doctor.py
shauryag2002 Dec 7, 2024
80aefde
Update hospital_doctor.py
shauryag2002 Dec 7, 2024
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
11 changes: 11 additions & 0 deletions care/facility/api/serializers/hospital_doctor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,22 @@
class HospitalDoctorSerializer(serializers.ModelSerializer):
area_text = ChoiceField(choices=DOCTOR_TYPES, read_only=True, source="area")
id = serializers.UUIDField(source="external_id", read_only=True)
total_doctors = serializers.IntegerField(read_only=True)

class Meta:
model = HospitalDoctors
read_only_fields = (
"id",
"area_text",
"total_doctors"
)
exclude = (*TIMESTAMP_FIELDS, "facility", "external_id")

def to_representation(self, instance):
representation = super().to_representation(instance)
representation["total_doctors"] = (
HospitalDoctors.objects.filter(facility=instance.facility)
.aggregate(total_doctors=Sum("count"))["total_doctors"]
or 0
)
return representation
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Several issues need attention in the to_representation method

  1. The Sum import is missing
  2. The method indentation is incorrect (currently inside Meta class)
  3. Missing error handling for database operations

Please apply these fixes:

+ from django.db.models import Sum
  from rest_framework import serializers
  
  from care.facility.api.serializers import TIMESTAMP_FIELDS
  from care.facility.models import DOCTOR_TYPES, HospitalDoctors
  from care.utils.serializers.fields import ChoiceField

  class HospitalDoctorSerializer(serializers.ModelSerializer):
      # ... other fields ...

      class Meta:
          model = HospitalDoctors
          read_only_fields = (
              "id",
              "area_text",
              "total_doctors"
          )
          exclude = (*TIMESTAMP_FIELDS, "facility", "external_id")

-         def to_representation(self, instance):
-             representation = super().to_representation(instance)
-             representation["total_doctors"] = (
-                 HospitalDoctors.objects.filter(facility=instance.facility)
-                 .aggregate(total_doctors=Sum("count"))["total_doctors"]
-                 or 0
-             )
-             return representation

+     def to_representation(self, instance):
+         representation = super().to_representation(instance)
+         try:
+             representation["total_doctors"] = (
+                 HospitalDoctors.objects.filter(facility=instance.facility)
+                 .aggregate(total_doctors=Sum("count"))["total_doctors"]
+                 or 0
+             )
+         except Exception as e:
+             representation["total_doctors"] = 0
+             # Consider logging the error here
+         return representation
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def to_representation(self, instance):
representation = super().to_representation(instance)
representation["total_doctors"] = (
HospitalDoctors.objects.filter(facility=instance.facility)
.aggregate(total_doctors=Sum("count"))["total_doctors"]
or 0
)
return representation
from django.db.models import Sum
from rest_framework import serializers
from care.facility.api.serializers import TIMESTAMP_FIELDS
from care.facility.models import DOCTOR_TYPES, HospitalDoctors
from care.utils.serializers.fields import ChoiceField
class HospitalDoctorSerializer(serializers.ModelSerializer):
# ... other fields ...
class Meta:
model = HospitalDoctors
read_only_fields = (
"id",
"area_text",
"total_doctors"
)
exclude = (*TIMESTAMP_FIELDS, "facility", "external_id")
def to_representation(self, instance):
representation = super().to_representation(instance)
try:
representation["total_doctors"] = (
HospitalDoctors.objects.filter(facility=instance.facility)
.aggregate(total_doctors=Sum("count"))["total_doctors"]
or 0
)
except Exception as e:
representation["total_doctors"] = 0
# Consider logging the error here
return representation
🧰 Tools
🪛 Ruff (0.8.0)

26-26: Undefined name Sum

(F821)