Skip to content

cache invalidation for Facilityretreive cache on identifier updation #3501

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

Open
praffq wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

cache invalidation for Facilityretreive cache on identifier updation #3501

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 2 additions & 1 deletion care/emr/signals/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
from .patient import * # noqa F403
from .patient import * # noqa: F403
from .patient_identifier import * # noqa: F403
1 change: 1 addition & 0 deletions care/emr/signals/patient_identifier/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
from .cache_invalidation import * # noqa: F403
24 changes: 24 additions & 0 deletions care/emr/signals/patient_identifier/cache_invalidation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
from django.core.cache import cache
from django.db.models.signals import post_save

from care.emr.models.patient import PatientIdentifierConfig
from care.emr.resources.base import model_cache_key, model_string
from care.facility.models import Facility


def invalidate_facility_cache_on_identifier_change(sender, instance, **kwargs):
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 30, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

fd "cache_invalidation.py" --type f

Repository: ohcnetwork/care

Length of output: 161

🏁 Script executed:

cat -n care/emr/signals/patient_identifier/cache_invalidation.py

Repository: ohcnetwork/care

Length of output: 1028

🏁 Script executed:

# Check if Ruff is configured and if there are any lint configurations
fd "ruff.toml|pyproject.toml|setup.cfg" --type f | head -5

Repository: ohcnetwork/care

Length of output: 83

🏁 Script executed:

cat ruff.toml

Repository: ohcnetwork/care

Length of output: 965

🏁 Script executed:

# Check if there are other signal handlers in the codebase to see how they handle this
rg "post_save.connect|post_delete.connect|pre_save.connect" -A 5 --type py | head -40

Repository: ohcnetwork/care

Length of output: 1210

🏁 Script executed:

# Look at the other signal handler functions to see how they handle this
rg "def invalidate_tag_config_cache|def delete_model_cache" -A 8 --type py

Repository: ohcnetwork/care

Length of output: 1473

🏁 Script executed:

# Check the full invalidate_tag_config_cache function
cat -n care/emr/resources/tag/cache_invalidation.py | head -30

Repository: ohcnetwork/care

Length of output: 1190

🏁 Script executed:

cat -n care/emr/resources/tag/cache_invalidation.py | grep -A 15 "def invalidate_tag_config_cache"

Repository: ohcnetwork/care

Length of output: 664

🏁 Script executed:

# Verify the delete_model_cache function uses sender
cat -n care/emr/resources/base.py | grep -A 8 "def delete_model_cache"

Repository: ohcnetwork/care

Length of output: 550

Silence Ruff's unused argument warnings for signal handler parameters.
Line 9 defines sender and **kwargs but never uses them; Ruff's ARG rule will flag this. Rename to _sender and **_kwargs (the Django signal handler signature requires these, even when unused).

🔧 Lint-friendly fix 
-def invalidate_facility_cache_on_identifier_change(sender, instance, **kwargs):
+def invalidate_facility_cache_on_identifier_change(_sender, instance, **_kwargs):
📝 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 invalidate_facility_cache_on_identifier_change(sender, instance, **kwargs):
def invalidate_facility_cache_on_identifier_change(_sender, instance, **_kwargs):
🧰 Tools
🪛 Ruff (0.14.14)

[warning] 9-9: Unused function argument: sender

(ARG001)

[warning] 9-9: Unused function argument: kwargs

(ARG001)

🤖 Prompt for AI Agents 
In `@care/emr/signals/patient_identifier/cache_invalidation.py` at line 9, The
signal handler invalidate_facility_cache_on_identifier_change declares unused
parameters sender and **kwargs which triggers Ruff's ARG unused-argument
warnings; update the function signature to use _sender and **_kwargs (preserving
the Django signal signature) so the parameters are recognized as intentionally
unused and the linter stops flagging them.

facility_model_string = model_string(Facility)

if instance.facility_id:
cache.delete_pattern(
model_cache_key(facility_model_string, pk=instance.facility.external_id)
)
else:
cache.delete_pattern(model_cache_key(facility_model_string))
Comment on lines +9 to +17
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 30, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# Find model_cache_key function definition
rg -n "def model_cache_key" --type=py

Repository: ohcnetwork/care

Length of output: 110

🏁 Script executed:

# Find model_string function definition
rg -n "def model_string" --type=py

Repository: ohcnetwork/care

Length of output: 135

🏁 Script executed:

# Find Facility model definition and external_id field
rg -n "class Facility" care/facility/models/facility.py -A 30 | head -50

Repository: ohcnetwork/care

Length of output: 2300

🏁 Script executed:

# Search for external_id field on Facility model
rg -n "external_id\s*=" care/facility/models/facility.py -B 2 -A 2

Repository: ohcnetwork/care

Length of output: 41

🏁 Script executed:

# Look for how cache keys are used elsewhere with Facility
rg -n "model_cache_key.*Facility" --type=py -B 2 -A 2

Repository: ohcnetwork/care

Length of output: 41

🏁 Script executed:

# Check if external_id has null constraints
rg -n "external_id" care/facility/models/facility.py | head -20

Repository: ohcnetwork/care

Length of output: 41

🏁 Script executed:

# Read model_cache_key implementation
sed -n '205,230p' care/emr/resources/base.py

Repository: ohcnetwork/care

Length of output: 1063

🏁 Script executed:

# Read model_string implementation
sed -n '196,210p' care/emr/resources/base.py

Repository: ohcnetwork/care

Length of output: 514

🏁 Script executed:

# Find BaseModel definition
rg -n "class BaseModel" --type=py care/

Repository: ohcnetwork/care

Length of output: 118

🏁 Script executed:

# Search for external_id more broadly in facility-related files
fd -e py care/facility | xargs rg "external_id" 2>/dev/null

Repository: ohcnetwork/care

Length of output: 50747

🏁 Script executed:

# Look at the full Facility model
wc -l care/facility/models/facility.py

Repository: ohcnetwork/care

Length of output: 95

🏁 Script executed:

# Check the base cache invalidation logic
sed -n '196,240p' care/emr/resources/base.py

Repository: ohcnetwork/care

Length of output: 1723

🏁 Script executed:

# Search for other places where Facility cache is accessed/invalidated
rg "cache.*Facility\|Facility.*cache" care/emr/resources/ -B 2 -A 2 | head -100

Repository: ohcnetwork/care

Length of output: 41

🏁 Script executed:

# Look at the actual signal handler connection for patient_identifier
rg -n "invalidate_facility_cache_on_identifier_change" --type=py -B 5 -A 10

Repository: ohcnetwork/care

Length of output: 2051

🏁 Script executed:

# Look for how cache keys are actually generated and used
rg "model_cache_key\(" care/emr/resources/base.py -A 2 -B 2

Repository: ohcnetwork/care

Length of output: 943

🏁 Script executed:

# Check if there are any other examples of facility cache invalidation
rg "invalidate.*facility\|Facility.*cache" care/emr/signals/ -i

Repository: ohcnetwork/care

Length of output: 41

🏁 Script executed:

# Check what PatientIdentifierConfig is and how it relates to Facility
rg "class PatientIdentifierConfig" --type=py -A 10

Repository: ohcnetwork/care

Length of output: 5702

🏁 Script executed:

# See if there are similar cache invalidation patterns elsewhere
rg "post_save.*connect" care/emr/signals/ -B 5 -A 5 | head -80

Repository: ohcnetwork/care

Length of output: 910

Cache invalidation should follow the established pattern: invalidate by both id and external_id.

The concern about external_id being null is unfounded—it's a UUID field with default=uuid4, so it's always populated. The cache key alignment is correct. However, the standard invalidation pattern elsewhere in care/emr/resources/base.py invalidates both by primary key (id) and by external_id:

cache.delete_pattern(model_cache_key(sender_model_string, pk=instance.id))
if external_id := getattr(instance, "external_id", None):
    cache.delete_pattern(model_cache_key(sender_model_string, pk=external_id))

Align with this pattern to avoid stale cache entries keyed by id.

🧰 Tools
🪛 Ruff (0.14.14)

[warning] 9-9: Unused function argument: sender

(ARG001)

[warning] 9-9: Unused function argument: kwargs

(ARG001)

🤖 Prompt for AI Agents 
In `@care/emr/signals/patient_identifier/cache_invalidation.py` around lines 9 -
17, The invalidate_facility_cache_on_identifier_change function currently only
deletes the cache by external_id; change it to follow the standard pattern used
in care/emr/resources/base.py by deleting both the id and external_id keyed
caches: call cache.delete_pattern(model_cache_key(facility_model_string,
pk=instance.id)) and then, since external_id is present, also call
cache.delete_pattern(model_cache_key(facility_model_string,
pk=instance.external_id)) (or use getattr(instance, "external_id", None) if you
prefer the existing guard) so both possible cache keys are invalidated.



post_save.connect(
invalidate_facility_cache_on_identifier_change,
sender=PatientIdentifierConfig,
dispatch_uid="invalidate_facility_cache_on_identifier_config_save",
)