cache invalidation for Facilityretreive cache on identifier updation#3501
cache invalidation for Facilityretreive cache on identifier updation#3501
Conversation
📝 WalkthroughWalkthroughIntroduces Django signal handler to invalidate cached Facility data when PatientIdentifierConfig is saved. The handler is registered to the post_save signal and deletes cache entries based on Facility external_id or general Facility patterns. The implementation is exposed through the signals package's public API. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 🧪 Unit Test Generation v2 is now available!We have significantly improved our unit test generation capabilities. To enable: Add this to your reviews:
finishing_touches:
unit_tests:
enabled: trueTry it out by using the Have feedback? Share your thoughts on our Discord thread! Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@care/emr/signals/patient_identifier/cache_invalidation.py`:
- 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.
- Around line 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.
| from care.facility.models import Facility | ||
|
|
||
|
|
||
| def invalidate_facility_cache_on_identifier_change(sender, instance, **kwargs): |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd "cache_invalidation.py" --type fRepository: ohcnetwork/care
Length of output: 161
🏁 Script executed:
cat -n care/emr/signals/patient_identifier/cache_invalidation.pyRepository: 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 -5Repository: ohcnetwork/care
Length of output: 83
🏁 Script executed:
cat ruff.tomlRepository: 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 -40Repository: 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 pyRepository: 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 -30Repository: 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.
| 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.
| def invalidate_facility_cache_on_identifier_change(sender, instance, **kwargs): | ||
| 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)) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find model_cache_key function definition
rg -n "def model_cache_key" --type=pyRepository: ohcnetwork/care
Length of output: 110
🏁 Script executed:
# Find model_string function definition
rg -n "def model_string" --type=pyRepository: 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 -50Repository: 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 2Repository: 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 2Repository: 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 -20Repository: ohcnetwork/care
Length of output: 41
🏁 Script executed:
# Read model_cache_key implementation
sed -n '205,230p' care/emr/resources/base.pyRepository: ohcnetwork/care
Length of output: 1063
🏁 Script executed:
# Read model_string implementation
sed -n '196,210p' care/emr/resources/base.pyRepository: 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/nullRepository: ohcnetwork/care
Length of output: 50747
🏁 Script executed:
# Look at the full Facility model
wc -l care/facility/models/facility.pyRepository: ohcnetwork/care
Length of output: 95
🏁 Script executed:
# Check the base cache invalidation logic
sed -n '196,240p' care/emr/resources/base.pyRepository: 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 -100Repository: 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 10Repository: 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 2Repository: 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/ -iRepository: ohcnetwork/care
Length of output: 41
🏁 Script executed:
# Check what PatientIdentifierConfig is and how it relates to Facility
rg "class PatientIdentifierConfig" --type=py -A 10Repository: 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 -80Repository: 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.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #3501 +/- ##
===========================================
+ Coverage 74.43% 74.44% +0.01%
===========================================
Files 471 473 +2
Lines 21734 21747 +13
Branches 2264 2265 +1
===========================================
+ Hits 16177 16189 +12
- Misses 5055 5056 +1
Partials 502 502 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
1 participant
Proposed Changes
Associated Issue
Architecture changes
Merge Checklist
/docsOnly PR's with test cases included and passing lint and test pipelines will be reviewed
@ohcnetwork/care-backend-maintainers @ohcnetwork/care-backend-admins
Summary by CodeRabbit
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.