From ee747c6850e9e8a0c94f5c9f55ceddc28701a17d Mon Sep 17 00:00:00 2001 From: Aakash Singh Date: Thu, 8 Aug 2024 13:29:48 +0530 Subject: [PATCH 1/2] events module: serialize only specific fields of related objects (#2324) --- care/facility/api/serializers/daily_round.py | 1 + .../api/serializers/patient_consultation.py | 4 +- .../api/viewsets/consultation_diagnosis.py | 35 +++++++++++- care/facility/api/viewsets/events.py | 15 ++++- care/facility/events/handler.py | 56 ++++++++----------- .../management/commands/load_event_types.py | 15 ++--- .../0447_patientconsultationevent_taken_at.py | 30 ++++++++++ care/facility/models/events.py | 1 + care/users/models.py | 4 ++ care/utils/event_utils.py | 24 ++++++-- scripts/celery_beat-ecs.sh | 1 + scripts/celery_beat.sh | 1 + 12 files changed, 136 insertions(+), 51 deletions(-) create mode 100644 care/facility/migrations/0447_patientconsultationevent_taken_at.py diff --git a/care/facility/api/serializers/daily_round.py b/care/facility/api/serializers/daily_round.py index eea3e21d61..46fea14d05 100644 --- a/care/facility/api/serializers/daily_round.py +++ b/care/facility/api/serializers/daily_round.py @@ -253,6 +253,7 @@ def create(self, validated_data): daily_round_obj, daily_round_obj.created_by_id, daily_round_obj.created_date, + taken_at=daily_round_obj.taken_at, ) return daily_round_obj diff --git a/care/facility/api/serializers/patient_consultation.py b/care/facility/api/serializers/patient_consultation.py index 3344680370..a316153caa 100644 --- a/care/facility/api/serializers/patient_consultation.py +++ b/care/facility/api/serializers/patient_consultation.py @@ -273,7 +273,7 @@ def update(self, instance, validated_data): consultation, self.context["request"].user.id, consultation.modified_date, - old_instance, + old=old_instance, ) if "assigned_to" in validated_data: @@ -819,7 +819,7 @@ def update(self, instance: PatientConsultation, validated_data): instance, self.context["request"].user.id, instance.modified_date, - old_instance, + old=old_instance, ) return instance diff --git a/care/facility/api/viewsets/consultation_diagnosis.py b/care/facility/api/viewsets/consultation_diagnosis.py index 73d218bc3e..cc76358be6 100644 --- a/care/facility/api/viewsets/consultation_diagnosis.py +++ b/care/facility/api/viewsets/consultation_diagnosis.py @@ -1,13 +1,17 @@ +from copy import copy + from django.shortcuts import get_object_or_404 from django_filters import rest_framework as filters from dry_rest_permissions.generics import DRYPermissions from rest_framework import mixins from rest_framework.permissions import IsAuthenticated +from rest_framework.response import Response from rest_framework.viewsets import GenericViewSet from care.facility.api.serializers.consultation_diagnosis import ( ConsultationDiagnosisSerializer, ) +from care.facility.events.handler import create_consultation_events from care.facility.models import ( ConditionVerificationStatus, ConsultationDiagnosis, @@ -52,4 +56,33 @@ def get_queryset(self): def perform_create(self, serializer): consultation = self.get_consultation_obj() - serializer.save(consultation=consultation, created_by=self.request.user) + diagnosis = serializer.save( + consultation=consultation, created_by=self.request.user + ) + create_consultation_events( + consultation.id, + diagnosis, + caused_by=self.request.user.id, + created_date=diagnosis.created_date, + ) + + def perform_update(self, serializer): + return serializer.save() + + def update(self, request, *args, **kwargs): + partial = kwargs.pop("partial", False) + instance = self.get_object() + old_instance = copy(instance) + serializer = self.get_serializer(instance, data=request.data, partial=partial) + serializer.is_valid(raise_exception=True) + instance = self.perform_update(serializer) + + create_consultation_events( + instance.consultation_id, + instance, + caused_by=self.request.user.id, + created_date=instance.created_date, + old=old_instance, + ) + + return Response(serializer.data) diff --git a/care/facility/api/viewsets/events.py b/care/facility/api/viewsets/events.py index 4e0a31a5fe..32b81d3ab4 100644 --- a/care/facility/api/viewsets/events.py +++ b/care/facility/api/viewsets/events.py @@ -47,11 +47,20 @@ def roots(self, request): class PatientConsultationEventFilterSet(filters.FilterSet): + ordering = filters.OrderingFilter( + fields=( + "created_date", + "taken_at", + ) + ) + class Meta: model = PatientConsultationEvent - fields = { - "event_type": ["exact"], - } + fields = [ + "event_type", + "caused_by", + "is_latest", + ] class PatientConsultationEventViewSet(ReadOnlyModelViewSet): diff --git a/care/facility/events/handler.py b/care/facility/events/handler.py index 66525603c0..4c5193aa0f 100644 --- a/care/facility/events/handler.py +++ b/care/facility/events/handler.py @@ -1,7 +1,9 @@ +from contextlib import suppress from datetime import datetime +from django.core.exceptions import FieldDoesNotExist from django.db import transaction -from django.db.models import Field, Model +from django.db.models import Model from django.db.models.query import QuerySet from django.utils.timezone import now @@ -9,63 +11,46 @@ from care.utils.event_utils import get_changed_fields, serialize_field -def transform( - object_instance: Model, - old_instance: Model, - fields_to_store: set[str] | None = None, -) -> dict[str, any]: - fields: set[Field] = set() - if old_instance: - changed_fields = get_changed_fields(old_instance, object_instance) - fields = { - field - for field in object_instance._meta.fields - if field.name in changed_fields - } - else: - fields = set(object_instance._meta.fields) - - if fields_to_store: - fields = {field for field in fields if field.name in fields_to_store} - - return {field.name: serialize_field(object_instance, field) for field in fields} - - def create_consultation_event_entry( consultation_id: int, object_instance: Model, caused_by: int, created_date: datetime, + taken_at: datetime, old_instance: Model | None = None, fields_to_store: set[str] | None = None, ): change_type = ChangeType.UPDATED if old_instance else ChangeType.CREATED - data = transform(object_instance, old_instance, fields_to_store) - fields_to_store = fields_to_store or set(data.keys()) + fields: set[str] = ( + get_changed_fields(old_instance, object_instance) + if old_instance + else {field.name for field in object_instance._meta.fields} + ) + + fields_to_store = fields_to_store & fields if fields_to_store else fields batch = [] groups = EventType.objects.filter( model=object_instance.__class__.__name__, fields__len__gt=0, is_active=True ).values_list("id", "fields") for group_id, group_fields in groups: - if set(group_fields) & fields_to_store: + if fields_to_store & {field.split("__", 1)[0] for field in group_fields}: value = {} for field in group_fields: - try: - value[field] = data[field] - except KeyError: - value[field] = getattr(object_instance, field, None) - # if all values in the group are Falsy, skip creating the event for this group + with suppress(FieldDoesNotExist): + value[field] = serialize_field(object_instance, field) + if all(not v for v in value.values()): continue + PatientConsultationEvent.objects.select_for_update().filter( consultation_id=consultation_id, event_type=group_id, is_latest=True, object_model=object_instance.__class__.__name__, object_id=object_instance.id, - created_date__lt=created_date, + taken_at__lt=taken_at, ).update(is_latest=False) batch.append( PatientConsultationEvent( @@ -74,6 +59,7 @@ def create_consultation_event_entry( event_type_id=group_id, is_latest=True, created_date=created_date, + taken_at=taken_at, object_model=object_instance.__class__.__name__, object_id=object_instance.id, value=value, @@ -94,12 +80,16 @@ def create_consultation_events( objects: list | QuerySet | Model, caused_by: int, created_date: datetime = None, + taken_at: datetime = None, old: Model | None = None, fields_to_store: list[str] | set[str] | None = None, ): if created_date is None: created_date = now() + if taken_at is None: + taken_at = created_date + with transaction.atomic(): if isinstance(objects, (QuerySet, list, tuple)): if old is not None: @@ -112,6 +102,7 @@ def create_consultation_events( obj, caused_by, created_date, + taken_at, fields_to_store=set(fields_to_store) if fields_to_store else None, ) else: @@ -120,6 +111,7 @@ def create_consultation_events( objects, caused_by, created_date, + taken_at, old, fields_to_store=set(fields_to_store) if fields_to_store else None, ) diff --git a/care/facility/management/commands/load_event_types.py b/care/facility/management/commands/load_event_types.py index 7e5d689f48..4aecbfc6fc 100644 --- a/care/facility/management/commands/load_event_types.py +++ b/care/facility/management/commands/load_event_types.py @@ -57,11 +57,13 @@ class Command(BaseCommand): "name": "INVESTIGATION", "fields": ("investigation",), }, - # disabling until we have a better way to serialize user objects - # { - # "name": "TREATING_PHYSICIAN", - # "fields": ("treating_physician",), - # }, + { + "name": "TREATING_PHYSICIAN", + "fields": ( + "treating_physician__username", + "treating_physician__full_name", + ), + }, ), }, { @@ -225,7 +227,7 @@ class Command(BaseCommand): { "name": "DIAGNOSIS", "model": "ConsultationDiagnosis", - "fields": ("diagnosis", "verification_status", "is_principal"), + "fields": ("diagnosis__label", "verification_status", "is_principal"), }, { "name": "SYMPTOMS", @@ -246,7 +248,6 @@ class Command(BaseCommand): "VENTILATOR_MODES", "SYMPTOMS", "ROUND_SYMPTOMS", - "TREATING_PHYSICIAN", ) def create_objects( diff --git a/care/facility/migrations/0447_patientconsultationevent_taken_at.py b/care/facility/migrations/0447_patientconsultationevent_taken_at.py new file mode 100644 index 0000000000..95f52ea646 --- /dev/null +++ b/care/facility/migrations/0447_patientconsultationevent_taken_at.py @@ -0,0 +1,30 @@ +# Generated by Django 4.2.10 on 2024-07-02 10:44 + +from django.db import migrations, models + + +def backfill_taken_at(apps, schema_editor): + PatientConsultationEvent = apps.get_model("facility", "PatientConsultationEvent") + PatientConsultationEvent.objects.filter(taken_at__isnull=True).update( + taken_at=models.F("created_date") + ) + + +class Migration(migrations.Migration): + dependencies = [ + ("facility", "0446_alter_notification_event"), + ] + + operations = [ + migrations.AddField( + model_name="patientconsultationevent", + name="taken_at", + field=models.DateTimeField(db_index=True, null=True), + ), + migrations.RunPython(backfill_taken_at, reverse_code=migrations.RunPython.noop), + migrations.AlterField( + model_name="patientconsultationevent", + name="taken_at", + field=models.DateTimeField(db_index=True), + ), + ] diff --git a/care/facility/models/events.py b/care/facility/models/events.py index e502677ff5..9eafe47388 100644 --- a/care/facility/models/events.py +++ b/care/facility/models/events.py @@ -55,6 +55,7 @@ class PatientConsultationEvent(models.Model): ) caused_by = models.ForeignKey(User, on_delete=models.PROTECT) created_date = models.DateTimeField(db_index=True) + taken_at = models.DateTimeField(db_index=True) object_model = models.CharField( max_length=50, db_index=True, null=False, blank=False ) diff --git a/care/users/models.py b/care/users/models.py index c06ae3e43c..5ea7e17e6e 100644 --- a/care/users/models.py +++ b/care/users/models.py @@ -311,6 +311,10 @@ class User(AbstractUser): CSV_MAKE_PRETTY = {"user_type": (lambda x: User.REVERSE_TYPE_MAP[x])} + @property + def full_name(self): + return self.get_full_name() + @staticmethod def has_read_permission(request): return True diff --git a/care/utils/event_utils.py b/care/utils/event_utils.py index 6105d07e26..c5032e25bd 100644 --- a/care/utils/event_utils.py +++ b/care/utils/event_utils.py @@ -2,7 +2,7 @@ from json import JSONEncoder from logging import getLogger -from django.core.serializers import serialize +from django.core.exceptions import FieldDoesNotExist from django.db.models import Field, Model from multiselectfield.db.fields import MSFList, MultiSelectField @@ -27,11 +27,23 @@ def get_changed_fields(old: Model, new: Model) -> set[str]: return changed_fields -def serialize_field(object: Model, field: Field): - value = getattr(object, field.name) - if isinstance(value, Model): - # serialize the fields of the related model - return serialize("python", [value])[0]["fields"] +def serialize_field(object: Model, field_name: str): + if "__" in field_name: + field_name, sub_field = field_name.split("__", 1) + related_object = getattr(object, field_name) + return serialize_field(related_object, sub_field) + + field = None + try: + field = object._meta.get_field(field_name) + except FieldDoesNotExist as e: + try: + # try to get property field + return getattr(object, field_name) + except AttributeError: + raise e + + value = getattr(object, field.name, None) if issubclass(field.__class__, Field) and field.choices: # serialize choice fields with display value return getattr(object, f"get_{field.name}_display", lambda: value)() diff --git a/scripts/celery_beat-ecs.sh b/scripts/celery_beat-ecs.sh index e664458132..540915a8b2 100755 --- a/scripts/celery_beat-ecs.sh +++ b/scripts/celery_beat-ecs.sh @@ -29,6 +29,7 @@ done python manage.py migrate --noinput python manage.py load_redis_index +python manage.py load_event_types touch /tmp/healthy diff --git a/scripts/celery_beat.sh b/scripts/celery_beat.sh index e4aa5d083f..9cc34f8075 100755 --- a/scripts/celery_beat.sh +++ b/scripts/celery_beat.sh @@ -29,6 +29,7 @@ done python manage.py migrate --noinput python manage.py load_redis_index +python manage.py load_event_types touch /tmp/healthy From 1e955a52d7a8e69d8a14734160890109d505423e Mon Sep 17 00:00:00 2001 From: Aakash Singh Date: Thu, 8 Aug 2024 18:49:52 +0530 Subject: [PATCH 2/2] fix attribute error on null values in field serialization --- care/facility/events/handler.py | 4 ++-- care/utils/event_utils.py | 31 ++++++++++++++++++------------- 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/care/facility/events/handler.py b/care/facility/events/handler.py index 4c5193aa0f..53c3ffc6ba 100644 --- a/care/facility/events/handler.py +++ b/care/facility/events/handler.py @@ -79,8 +79,8 @@ def create_consultation_events( consultation_id: int, objects: list | QuerySet | Model, caused_by: int, - created_date: datetime = None, - taken_at: datetime = None, + created_date: datetime | None = None, + taken_at: datetime | None = None, old: Model | None = None, fields_to_store: list[str] | set[str] | None = None, ): diff --git a/care/utils/event_utils.py b/care/utils/event_utils.py index c5032e25bd..12d0dca91a 100644 --- a/care/utils/event_utils.py +++ b/care/utils/event_utils.py @@ -30,23 +30,28 @@ def get_changed_fields(old: Model, new: Model) -> set[str]: def serialize_field(object: Model, field_name: str): if "__" in field_name: field_name, sub_field = field_name.split("__", 1) - related_object = getattr(object, field_name) + related_object = getattr(object, field_name, None) return serialize_field(related_object, sub_field) - field = None + value = None + try: + value = getattr(object, field_name) + except AttributeError: + if object is not None: + logger.warning( + f"Field {field_name} not found in {object.__class__.__name__}" + ) + return None + try: - field = object._meta.get_field(field_name) - except FieldDoesNotExist as e: - try: - # try to get property field - return getattr(object, field_name) - except AttributeError: - raise e - - value = getattr(object, field.name, None) - if issubclass(field.__class__, Field) and field.choices: # serialize choice fields with display value - return getattr(object, f"get_{field.name}_display", lambda: value)() + field = object._meta.get_field(field_name) + if issubclass(field.__class__, Field) and field.choices: + value = getattr(object, f"get_{field_name}_display", lambda: value)() + except FieldDoesNotExist: + # the required field is a property and not a model field + pass + return value