Skip to content
Closed
Show file tree
Hide file tree
Changes from 5 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
66 changes: 65 additions & 1 deletion care/emr/api/viewsets/facility.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from django.db import transaction
from django.db.models import Q
from django.utils.decorators import method_decorator
from django_filters import CharFilter, FilterSet, NumberFilter
Expand All @@ -11,11 +12,22 @@
from rest_framework.parsers import MultiPartParser
from rest_framework.response import Response

from care.emr.api.viewsets.base import EMRModelReadOnlyViewSet, EMRModelViewSet
from care.emr.api.viewsets.base import (
EMRBaseViewSet,
EMRCreateMixin,
EMRDestroyMixin,
EMRListMixin,
EMRModelReadOnlyViewSet,
EMRModelViewSet,
EMRRetrieveMixin,
)
from care.emr.models import Organization, SchedulableUserResource
from care.emr.models.facility import FacilityFlag
from care.emr.models.organization import FacilityOrganizationUser, OrganizationUser
from care.emr.resources.facility.spec import (
FacilityCreateSpec,
FacilityFlagCreateSpec,
FacilityFlagReadSpec,
FacilityReadSpec,
FacilityRetrieveSpec,
)
Expand All @@ -28,6 +40,7 @@
cover_image_validator,
custom_image_extension_validator,
)
from care.utils.registries.feature_flag import FlagNotFoundError, FlagRegistry, FlagType


class FacilityImageUploadSerializer(serializers.ModelSerializer):
Expand Down Expand Up @@ -180,3 +193,54 @@ class AllFacilityViewSet(EMRModelReadOnlyViewSet):

def get_queryset(self):
return Facility.objects.filter(is_public=True).select_related()


class FacilityFlagFilter(filters.FilterSet):
flag = filters.CharFilter(field_name="flag", lookup_expr="exact")
facility = filters.UUIDFilter(field_name="facility__external_id")


class FacilityFlagViewSet(
EMRDestroyMixin, EMRCreateMixin, EMRRetrieveMixin, EMRListMixin, EMRBaseViewSet
):
database_model = FacilityFlag
pydantic_model = FacilityFlagCreateSpec
pydantic_read_model = FacilityFlagReadSpec
filter_backends = [filters.DjangoFilterBackend]
filterset_class = FacilityFlagFilter

def permissions_controller(self, request):
return request.user.is_superuser

def perform_create(self, instance):
with transaction.atomic():
FlagRegistry.register(FlagType.FACILITY.value, instance.flag)
super().perform_create(instance)

def perform_destroy(self, instance):
with transaction.atomic():
super().perform_destroy(instance)
FacilityFlag.invalidate_cache(instance.facility, instance.flag)
transaction.on_commit(
lambda: self._safe_unregister_flag_if_unused(instance.flag, instance.id)
)
Comment on lines +258 to +264
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Wrong argument type passed to invalidate_cache.

Line 261 passes instance.facility (a Facility object) to invalidate_cache, which expects entity_id: int. This will cause a type error or incorrect cache key formatting.

         with transaction.atomic():
             super().perform_destroy(instance)
-            FacilityFlag.invalidate_cache(instance.facility, instance.flag)
+            FacilityFlag.invalidate_cache(instance.facility_id, instance.flag)
             transaction.on_commit(
                 lambda: self._safe_unregister_flag_if_unused(instance.flag, instance.id)
             )
🤖 Prompt for AI Agents
In care/emr/api/viewsets/facility.py around lines 258 to 264, perform_destroy
calls FacilityFlag.invalidate_cache with instance.facility (a Facility object)
but the function expects entity_id: int; change the call to pass the facility's
integer id (use instance.facility_id or instance.facility.id) so the correct
type/format is provided, keeping the transaction.atomic and on_commit logic
unchanged.


def _safe_unregister_flag_if_unused(self, flag_name: str, deleted_instance_id: int):
still_used = (
FacilityFlag.objects.filter(flag=flag_name, deleted=False)
.exclude(id=deleted_instance_id)
.exists()
)

if not still_used:
FlagRegistry.unregister(FlagType.FACILITY.value, flag_name)

@action(detail=False, methods=["get"], url_path="available-flags")
def list_available_flags(self, request):
try:
flags = FlagRegistry.get_all_flags(FlagType.FACILITY.value)
return Response({"available_flags": list(flags)})
except FlagNotFoundError:
return Response(
{"message": "No registered flag type 'facility' found."}, status=400
)
64 changes: 63 additions & 1 deletion care/emr/api/viewsets/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,21 @@
from rest_framework.permissions import IsAuthenticated
from rest_framework.response import Response

from care.emr.api.viewsets.base import EMRModelViewSet
from care.emr.api.viewsets.base import (
EMRBaseViewSet,
EMRCreateMixin,
EMRDestroyMixin,
EMRListMixin,
EMRModelViewSet,
EMRRetrieveMixin,
)
from care.emr.models import Organization
from care.emr.models.organization import OrganizationUser
from care.emr.models.user import UserFlag
from care.emr.resources.user.spec import (
UserCreateSpec,
UserFlagCreateSpec,
UserFlagReadSpec,
UserRetrieveSpec,
UserSpec,
UserTypeRoleMapping,
Expand All @@ -24,6 +34,7 @@
from care.users.api.serializers.user import UserImageUploadSerializer, UserSerializer
from care.users.models import User
from care.utils.file_uploads.cover_image import delete_cover_image
from care.utils.registries.feature_flag import FlagNotFoundError, FlagRegistry, FlagType


class UserFilter(filters.FilterSet):
Expand Down Expand Up @@ -159,3 +170,54 @@ def pnconfig(self, request, *args, **kwargs):
setattr(user, field, request.data[field])
user.save()
return Response({})


class UserFlagFilter(filters.FilterSet):
flag = filters.CharFilter(field_name="flag", lookup_expr="exact")
user = filters.UUIDFilter(field_name="user__external_id")


class UserFlagViewSet(
EMRDestroyMixin, EMRCreateMixin, EMRRetrieveMixin, EMRListMixin, EMRBaseViewSet
):
database_model = UserFlag
pydantic_model = UserFlagCreateSpec
pydantic_read_model = UserFlagReadSpec
filter_backends = [filters.DjangoFilterBackend]
filterset_class = UserFlagFilter

def permissions_controller(self, request):
return request.user.is_superuser

def perform_create(self, instance):
with transaction.atomic():
FlagRegistry.register(FlagType.USER.value, instance.flag)
super().perform_create(instance)

def perform_destroy(self, instance):
with transaction.atomic():
super().perform_destroy(instance)
UserFlag.invalidate_cache(instance.user, instance.flag)
transaction.on_commit(
lambda: self._safe_unregister_flag_if_unused(instance.flag, instance.id)
)
Comment on lines +230 to +236
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Wrong argument type passed to invalidate_cache.

Line 233 passes instance.user (a User object) instead of instance.user_id (int) to invalidate_cache. This is the same issue present in the facility viewset.

         with transaction.atomic():
             super().perform_destroy(instance)
-            UserFlag.invalidate_cache(instance.user, instance.flag)
+            UserFlag.invalidate_cache(instance.user_id, instance.flag)
             transaction.on_commit(
                 lambda: self._safe_unregister_flag_if_unused(instance.flag, instance.id)
             )
🤖 Prompt for AI Agents
In care/emr/api/viewsets/user.py around lines 230 to 236, the call to
UserFlag.invalidate_cache is passing instance.user (a User object) instead of
the user id; change the call to pass instance.user_id (int). Update the same
pattern where found (e.g., the facility viewset) to use instance.user_id, and
keep the surrounding transaction.atomic and transaction.on_commit logic
unchanged.


def _safe_unregister_flag_if_unused(self, flag_name: str, deleted_instance_id: int):
still_used = (
UserFlag.objects.filter(flag=flag_name)
.exclude(id=deleted_instance_id)
.exists()
)

if not still_used:
FlagRegistry.unregister(FlagType.USER.value, flag_name)

@action(detail=False, methods=["get"], url_path="available-flags")
def list_available_flags(self, request):
try:
flags = FlagRegistry.get_all_flags(FlagType.USER.value)
return Response({"available_flags": list(flags)})
except FlagNotFoundError:
return Response(
{"message": "No registered flag type 'user' found."}, status=400
)
58 changes: 58 additions & 0 deletions care/emr/migrations/0030_facilityflag_userflag.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
# Generated by Django 5.1.4 on 2025-05-15 15:33

import django.db.models.deletion
import uuid
from django.conf import settings
from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
('emr', '0029_encounter_discharge_summary_advice'),
('facility', '0477_delete_facilityflag'),
migrations.swappable_dependency(settings.AUTH_USER_MODEL),
]

operations = [
migrations.CreateModel(
name='FacilityFlag',
fields=[
('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
('external_id', models.UUIDField(db_index=True, default=uuid.uuid4, unique=True)),
('created_date', models.DateTimeField(auto_now_add=True, db_index=True, null=True)),
('modified_date', models.DateTimeField(auto_now=True, db_index=True, null=True)),
('deleted', models.BooleanField(db_index=True, default=False)),
('history', models.JSONField(default=dict)),
('meta', models.JSONField(default=dict)),
('flag', models.CharField(max_length=1024)),
('created_by', models.ForeignKey(blank=True, default=None, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='%(app_label)s_%(class)s_created_by', to=settings.AUTH_USER_MODEL)),
('facility', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='facility.facility')),
('updated_by', models.ForeignKey(blank=True, default=None, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='%(app_label)s_%(class)s_updated_by', to=settings.AUTH_USER_MODEL)),
],
options={
'verbose_name': 'Facility Flag',
'constraints': [models.UniqueConstraint(condition=models.Q(('deleted', False)), fields=('facility', 'flag'), name='emr_unique_facility_flag')],
},
),
migrations.CreateModel(
name='UserFlag',
fields=[
('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
('external_id', models.UUIDField(db_index=True, default=uuid.uuid4, unique=True)),
('created_date', models.DateTimeField(auto_now_add=True, db_index=True, null=True)),
('modified_date', models.DateTimeField(auto_now=True, db_index=True, null=True)),
('deleted', models.BooleanField(db_index=True, default=False)),
('history', models.JSONField(default=dict)),
('meta', models.JSONField(default=dict)),
('flag', models.CharField(max_length=1024)),
('created_by', models.ForeignKey(blank=True, default=None, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='%(app_label)s_%(class)s_created_by', to=settings.AUTH_USER_MODEL)),
('updated_by', models.ForeignKey(blank=True, default=None, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='%(app_label)s_%(class)s_updated_by', to=settings.AUTH_USER_MODEL)),
('user', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL)),
],
options={
'verbose_name': 'User Flag',
'constraints': [models.UniqueConstraint(condition=models.Q(('deleted', False)), fields=('user', 'flag'), name='emr_unique_user_flag')],
},
),
]
69 changes: 69 additions & 0 deletions care/emr/models/base.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
from django.core.cache import cache
from django.db import models

from care.utils.models.base import BaseModel
from care.utils.registries.feature_flag import FlagName, FlagRegistry


class EMRBaseModel(BaseModel):
Expand All @@ -25,3 +27,70 @@ class EMRBaseModel(BaseModel):

class Meta:
abstract = True


FLAGS_CACHE_TTL = 60 * 60 * 24 # 1 Day


class BaseFlag(EMRBaseModel):
flag = models.CharField(max_length=1024)

cache_key_template = ""
all_flags_cache_key_template = ""
flag_type = None
entity_field_name = ""

class Meta:
abstract = True

def save(self, *args, **kwargs):
self.validate_flag(self.flag)
cache.delete(
self.cache_key_template.format(
entity_id=self.entity_id, flag_name=self.flag
)
)
cache.delete(self.all_flags_cache_key_template.format(entity_id=self.entity_id))
return super().save(*args, **kwargs)
Comment on lines +48 to +56
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Cache invalidation happens before persistence completes.

Currently, you delete cache entries before super().save(), which means if the save fails (e.g., validation error, DB constraint), the cache is already invalidated. Consider moving invalidation after the save or using a post-save signal.

Apply this approach:

 def save(self, *args, **kwargs):
     self.validate_flag(self.flag)
+    result = super().save(*args, **kwargs)
     cache.delete(
         self.cache_key_template.format(
             entity_id=self.entity_id, flag_name=self.flag
         )
     )
     cache.delete(self.all_flags_cache_key_template.format(entity_id=self.entity_id))
-    return super().save(*args, **kwargs)
+    return result
📝 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 save(self, *args, **kwargs):
self.validate_flag(self.flag)
cache.delete(
self.cache_key_template.format(
entity_id=self.entity_id, flag_name=self.flag
)
)
cache.delete(self.all_flags_cache_key_template.format(entity_id=self.entity_id))
return super().save(*args, **kwargs)
def save(self, *args, **kwargs):
self.validate_flag(self.flag)
result = super().save(*args, **kwargs)
cache.delete(
self.cache_key_template.format(
entity_id=self.entity_id, flag_name=self.flag
)
)
cache.delete(self.all_flags_cache_key_template.format(entity_id=self.entity_id))
return result
🤖 Prompt for AI Agents
In care/emr/models/base.py around lines 48 to 56, cache entries are being
deleted before the model is persisted which can leave cache stale if save()
fails; move the cache invalidation to occur after calling super().save(*args,
**kwargs) (or register a post-save signal handler) so deletion only happens on
successful persistence, ensuring you still call validate_flag(self.flag) prior
to saving and delete both the per-entity flag key and the all-flags key after
the save completes.


@property
def entity(self):
return getattr(self, self.entity_field_name)

@property
def entity_id(self):
return getattr(self, f"{self.entity_field_name}_id")

@classmethod
def validate_flag(cls, flag_name: FlagName):
FlagRegistry.validate_flag_name(cls.flag_type, flag_name)

@classmethod
def check_entity_has_flag(cls, entity_id: int, flag_name: FlagName) -> bool:
cls.validate_flag(flag_name)
return cache.get_or_set(
cls.cache_key_template.format(entity_id=entity_id, flag_name=flag_name),
default=lambda: cls.objects.filter(
**{f"{cls.entity_field_name}_id": entity_id, "flag": flag_name}
).exists(),
timeout=FLAGS_CACHE_TTL,
)

@classmethod
def get_all_flags(cls, entity_id: int) -> tuple[FlagName]:
return cache.get_or_set(
cls.all_flags_cache_key_template.format(entity_id=entity_id),
default=lambda: tuple(
cls.objects.filter(
**{f"{cls.entity_field_name}_id": entity_id}
).values_list("flag", flat=True)
),
timeout=FLAGS_CACHE_TTL,
)

@classmethod
def invalidate_cache(cls, entity_id: int, flag_name: str):
cache.delete(
cls.cache_key_template.format(entity_id=entity_id, flag_name=flag_name)
)
cache.delete(cls.all_flags_cache_key_template.format(entity_id=entity_id))
Original file line number Diff line number Diff line change
@@ -1,12 +1,8 @@
from django.db import models

from care.utils.models.base import BaseFlag
from care.emr.models.base import BaseFlag
from care.utils.registries.feature_flag import FlagName, FlagType

FACILITY_FLAG_CACHE_KEY = "facility_flag_cache:{facility_id}:{flag_name}"
FACILITY_ALL_FLAGS_CACHE_KEY = "facility_all_flags_cache:{facility_id}"
FACILITY_FLAG_CACHE_TTL = 60 * 60 * 24 # 1 Day


class FacilityFlag(BaseFlag):
facility = models.ForeignKey(
Expand All @@ -15,7 +11,7 @@ class FacilityFlag(BaseFlag):

cache_key_template = "facility_flag_cache:{entity_id}:{flag_name}"
all_flags_cache_key_template = "facility_all_flags_cache:{entity_id}"
flag_type = FlagType.FACILITY
flag_type = FlagType.FACILITY.value
entity_field_name = "facility"

def __str__(self) -> str:
Expand All @@ -27,7 +23,7 @@ class Meta:
models.UniqueConstraint(
fields=["facility", "flag"],
condition=models.Q(deleted=False),
name="unique_facility_flag",
name="emr_unique_facility_flag",
)
]

Expand Down
3 changes: 1 addition & 2 deletions care/emr/models/file_upload.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

from care.emr.models import EMRBaseModel
from care.emr.utils.file_manager import S3FilesManager
from care.users.models import User
from care.utils.csp.config import BucketType
from care.utils.models.validators import parse_file_extension

Expand All @@ -23,7 +22,7 @@ class FileUpload(EMRBaseModel):
archive_reason = models.TextField(blank=True)
archived_datetime = models.DateTimeField(blank=True, null=True)
archived_by = models.ForeignKey(
User,
"users.User",
on_delete=models.PROTECT,
null=True,
blank=True,
Expand Down
3 changes: 1 addition & 2 deletions care/emr/models/patient.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
from django.utils import timezone

from care.emr.models import EMRBaseModel
from care.users.models import User
from care.utils.models.validators import mobile_or_landline_number_validator


Expand Down Expand Up @@ -115,7 +114,7 @@ class PatientUser(EMRBaseModel):
Add a user that can access the patient
"""

user = models.ForeignKey(User, on_delete=models.CASCADE)
user = models.ForeignKey("users.User", on_delete=models.CASCADE)
patient = models.ForeignKey(Patient, on_delete=models.CASCADE)
role = models.ForeignKey("security.RoleModel", on_delete=models.PROTECT)

Expand Down
Loading
Loading