Confirm de-dupe of license encumbrance privilege update records #1050 #1194
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
WalkthroughAdds UUID typing for adverse_action_id across utilities and adds pre-checks that query existing ENCUMBRANCE privilege-update records by adverse_action_id to skip creating duplicate updates; extensive tests added to assert idempotent behavior and stable update identity across retries. Changes
Sequence Diagram(s)sequenceDiagram
participant Event as Event Source
participant Handler as Encumbrance Handler
participant DB as Database
participant Logger as Logger
Note over Handler,DB: Input includes adverse_action_id (UUID)
Event->>Handler: Deliver encumbrance/lift event (adverse_action_id: UUID)
Handler->>DB: Query privilege_update\n(type=ENCUMBRANCE, adverse_action_id=UUID)
DB-->>Handler: Return matching records (0..n)
alt match found
Handler->>Logger: Log duplicate ENCUMBRANCE detected
Handler-->>Event: Acknowledge (no new update)
else no match
Handler->>DB: Create privilege_update records\n(apply encumbrance or lift)
DB-->>Handler: Created
Handler-->>Event: Acknowledge (updates created)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (10)📓 Common learnings📚 Learning: 2025-05-30T13:48:25.375ZApplied to files:
📚 Learning: 2025-07-08T18:40:24.408ZApplied to files:
📚 Learning: 2025-08-25T19:04:48.800ZApplied to files:
📚 Learning: 2025-06-04T20:07:09.680ZApplied to files:
📚 Learning: 2025-09-09T19:28:53.260ZApplied to files:
📚 Learning: 2025-04-29T02:52:40.532ZApplied to files:
📚 Learning: 2025-06-04T17:38:22.392ZApplied to files:
📚 Learning: 2025-08-29T21:45:05.792ZApplied to files:
📚 Learning: 2025-08-13T20:28:33.191ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (1)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
backend/compact-connect/lambdas/python/data-events/tests/function/test_encumbrance_events.py (1)
3265-3270: Refresh provider snapshot before helper assertion
provider_recordsis captured before the second handler invocation, so this helper check reuses a stale snapshot. The Dynamo query above already guards against duplicates, but if that query ever changes, this section would no longer detect a regression. Consider re-fetchingprovider_recordsafter the retry so the helper-based assertion inspects the post-retry state.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py(2 hunks)backend/compact-connect/lambdas/python/data-events/tests/function/test_encumbrance_events.py(1 hunks)
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 824
File: backend/compact-connect/lambdas/python/provider-data-v1/handlers/encumbrance.py:116-201
Timestamp: 2025-05-30T13:48:25.375Z
Learning: In encumbrance handling code, prefer to keep privilege and license encumbrance lifting implementations decoupled rather than extracting shared logic, as requirements between these implementations are likely to change in the future.
Learnt from: ChiefStief
Repo: csg-org/CompactConnect PR: 1036
File: webroot/src/components/PrivilegeEventNode/PrivilegeEventNode.ts:51-53
Timestamp: 2025-09-03T22:33:32.687Z
Learning: For encumbrance and lifting_encumbrance events in the privilege system, createDate should always be present. If createDate is absent, it indicates a bug in the data pipeline that should be allowed to surface rather than being defensively handled.
Learnt from: ChiefStief
Repo: csg-org/CompactConnect PR: 1036
File: backend/compact-connect/lambdas/python/data-events/handlers/encumbrance_events.py:200-204
Timestamp: 2025-09-03T22:35:42.943Z
Learning: The adverseActionCategory and adverseActionId fields in encumbrance events are only needed when the event flow creates privilege update database records. Some privilege encumbrance event publications don't create database records and don't need these fields, which is why they're optional in the EncumbranceEventDetailSchema.
Learnt from: ChiefStief
Repo: csg-org/CompactConnect PR: 1093
File: webroot/src/locales/en.json:591-592
Timestamp: 2025-09-16T18:31:34.428Z
Learning: ChiefStief prefers to keep approved verbiage in CompactConnect even when there are minor technical inaccuracies, particularly for timezone-related text that has already gone through the approval process.
📚 Learning: 2025-09-11T20:06:40.709Z
Learnt from: ChiefStief
Repo: csg-org/CompactConnect PR: 1036
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py:370-383
Timestamp: 2025-09-11T20:06:40.709Z
Learning: The EncumbranceDetailsSchema in backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/record.py does not contain a 'note' field. It only has clinicalPrivilegeActionCategory (String, optional), adverseActionId (UUID, required), and licenseJurisdiction (Jurisdiction, optional). When working with encumbrance notes, use encumbranceDetails['clinicalPrivilegeActionCategory'], not encumbranceDetails['note'].
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-09-11T20:06:40.709Z
Learnt from: ChiefStief
Repo: csg-org/CompactConnect PR: 1036
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py:370-383
Timestamp: 2025-09-11T20:06:40.709Z
Learning: The EncumbranceDetailsSchema in backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/record.py does not contain a 'note' field. It only has clinicalPrivilegeActionCategory (String, optional), adverseActionId (UUID, required), and licenseJurisdiction (Jurisdiction, optional).
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-10-15T19:53:00.422Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1135
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py:2642-2654
Timestamp: 2025-10-15T19:53:00.422Z
Learning: In backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py and provider_record_util.py, the get_adverse_action_by_id method correctly handles UUID vs string comparison as written. Attempting to add explicit type conversion causes the mapping to fail. The current implementation should not be changed to add UUID/string conversion logic.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-05-30T13:48:25.375Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 824
File: backend/compact-connect/lambdas/python/provider-data-v1/handlers/encumbrance.py:116-201
Timestamp: 2025-05-30T13:48:25.375Z
Learning: In encumbrance handling code, prefer to keep privilege and license encumbrance lifting implementations decoupled rather than extracting shared logic, as requirements between these implementations are likely to change in the future.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.pybackend/compact-connect/lambdas/python/data-events/tests/function/test_encumbrance_events.py
📚 Learning: 2025-09-03T22:29:24.535Z
Learnt from: ChiefStief
Repo: csg-org/CompactConnect PR: 1036
File: backend/compact-connect/lambdas/python/data-events/handlers/encumbrance_events.py:0-0
Timestamp: 2025-09-03T22:29:24.535Z
Learning: The EncumbranceEventDetailSchema in backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/data_event/api.py is used across multiple instances/contexts where adverseActionCategory and adverseActionId fields may be required in some cases and not present in others. This is an intentional design pattern for handling different event variants with a single schema.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.pybackend/compact-connect/lambdas/python/data-events/tests/function/test_encumbrance_events.py
📚 Learning: 2025-09-03T22:35:42.943Z
Learnt from: ChiefStief
Repo: csg-org/CompactConnect PR: 1036
File: backend/compact-connect/lambdas/python/data-events/handlers/encumbrance_events.py:200-204
Timestamp: 2025-09-03T22:35:42.943Z
Learning: The adverseActionCategory and adverseActionId fields in encumbrance events are only needed when the event flow creates privilege update database records. Some privilege encumbrance event publications don't create database records and don't need these fields, which is why they're optional in the EncumbranceEventDetailSchema.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-08-29T21:45:05.792Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1016
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py:2702-2710
Timestamp: 2025-08-29T21:45:05.792Z
Learning: In the lift_home_jurisdiction_license_privilege_encumbrances method, when latest_effective_lift_date is None, the method always returns an empty list for affected_privileges, so the existing if affected_privileges guard in the calling code already prevents event publishing without needing explicit None checks for the date.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.pybackend/compact-connect/lambdas/python/data-events/tests/function/test_encumbrance_events.py
🧬 Code graph analysis (2)
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py (3)
backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py (1)
get_update_records_for_privilege(714-733)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/__init__.py (6)
jurisdiction(32-33)jurisdiction(112-113)licenseType(40-41)licenseType(116-117)updateType(100-101)encumbranceDetails(151-155)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/common.py (2)
update(182-203)UpdateCategory(287-301)
backend/compact-connect/lambdas/python/data-events/tests/function/test_encumbrance_events.py (6)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/common.py (4)
UpdateCategory(287-301)serialize_to_database_record(205-208)update(182-203)PrivilegeEncumberedStatusEnum(319-323)backend/compact-connect/lambdas/python/data-events/handlers/encumbrance_events.py (2)
license_encumbrance_listener(186-243)license_encumbrance_lifted_listener(247-307)backend/compact-connect/lambdas/python/common/common_test/test_data_generator.py (4)
put_default_provider_record_in_provider_table(409-429)put_default_privilege_record_in_provider_table(310-320)put_default_adverse_action_record_in_provider_table(154-160)put_default_license_record_in_provider_table(238-248)backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py (1)
get_provider_user_records(173-204)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/__init__.py (7)
compact(28-29)compact(108-109)jurisdiction(32-33)jurisdiction(112-113)updateType(100-101)encumbranceDetails(151-155)encumberedStatus(72-73)backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py (2)
get_update_records_for_privilege(714-733)get_privilege_records(481-489)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: TestApp
🔇 Additional comments (2)
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py (2)
2703-2722: Deduplication logic correctly implemented for unencumbered privileges.The deduplication check prevents creating duplicate ENCUMBRANCE update records when the event flow is re-run, which is essential for idempotency. The implementation is safe:
- Short-circuit evaluation prevents AttributeError on None
- Filtering by both updateType and adverseActionId is precise
- Skipping both the update record and status update transaction maintains consistency
2757-2776: Deduplication logic correctly implemented for previously encumbered privileges.This parallel deduplication check for already-encumbered privileges ensures that audit trail records aren't duplicated when events are replayed. The implementation mirrors the unencumbered privilege logic and correctly handles the case where no status change is needed (only history tracking).
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/compact-connect/lambdas/python/data-events/tests/function/test_encumbrance_events.py(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: ChiefStief
Repo: csg-org/CompactConnect PR: 1036
File: backend/compact-connect/lambdas/python/data-events/handlers/encumbrance_events.py:200-204
Timestamp: 2025-09-03T22:35:42.943Z
Learning: The adverseActionCategory and adverseActionId fields in encumbrance events are only needed when the event flow creates privilege update database records. Some privilege encumbrance event publications don't create database records and don't need these fields, which is why they're optional in the EncumbranceEventDetailSchema.
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 824
File: backend/compact-connect/lambdas/python/provider-data-v1/handlers/encumbrance.py:116-201
Timestamp: 2025-05-30T13:48:25.375Z
Learning: In encumbrance handling code, prefer to keep privilege and license encumbrance lifting implementations decoupled rather than extracting shared logic, as requirements between these implementations are likely to change in the future.
Learnt from: ChiefStief
Repo: csg-org/CompactConnect PR: 1036
File: webroot/src/components/PrivilegeEventNode/PrivilegeEventNode.ts:51-53
Timestamp: 2025-09-03T22:33:32.687Z
Learning: For encumbrance and lifting_encumbrance events in the privilege system, createDate should always be present. If createDate is absent, it indicates a bug in the data pipeline that should be allowed to surface rather than being defensively handled.
Learnt from: ChiefStief
Repo: csg-org/CompactConnect PR: 1093
File: webroot/src/locales/en.json:591-592
Timestamp: 2025-09-16T18:31:34.428Z
Learning: ChiefStief prefers to keep approved verbiage in CompactConnect even when there are minor technical inaccuracies, particularly for timezone-related text that has already gone through the approval process.
📚 Learning: 2025-05-30T13:48:25.375Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 824
File: backend/compact-connect/lambdas/python/provider-data-v1/handlers/encumbrance.py:116-201
Timestamp: 2025-05-30T13:48:25.375Z
Learning: In encumbrance handling code, prefer to keep privilege and license encumbrance lifting implementations decoupled rather than extracting shared logic, as requirements between these implementations are likely to change in the future.
Applied to files:
backend/compact-connect/lambdas/python/data-events/tests/function/test_encumbrance_events.py
📚 Learning: 2025-08-29T21:45:05.792Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1016
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py:2702-2710
Timestamp: 2025-08-29T21:45:05.792Z
Learning: In the lift_home_jurisdiction_license_privilege_encumbrances method, when latest_effective_lift_date is None, the method always returns an empty list for affected_privileges, so the existing if affected_privileges guard in the calling code already prevents event publishing without needing explicit None checks for the date.
Applied to files:
backend/compact-connect/lambdas/python/data-events/tests/function/test_encumbrance_events.py
🧬 Code graph analysis (1)
backend/compact-connect/lambdas/python/data-events/tests/function/test_encumbrance_events.py (6)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/common.py (4)
UpdateCategory(287-301)serialize_to_database_record(205-208)update(182-203)PrivilegeEncumberedStatusEnum(319-323)backend/compact-connect/lambdas/python/data-events/handlers/encumbrance_events.py (2)
license_encumbrance_listener(186-243)license_encumbrance_lifted_listener(247-307)backend/compact-connect/lambdas/python/common/common_test/test_data_generator.py (3)
put_default_privilege_record_in_provider_table(310-320)put_default_adverse_action_record_in_provider_table(154-160)put_default_license_record_in_provider_table(238-248)backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py (1)
get_provider_user_records(173-204)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/__init__.py (6)
compact(28-29)compact(108-109)jurisdiction(32-33)jurisdiction(112-113)encumbranceDetails(151-155)encumberedStatus(72-73)backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py (2)
get_update_records_for_privilege(714-733)get_privilege_records(481-489)
🪛 GitHub Actions: Check-CompactConnect
backend/compact-connect/lambdas/python/data-events/tests/function/test_encumbrance_events.py
[error] 1-1: Test failure: expected 0 matching_updates but got 1 in test_license_encumbrance_listener_does_not_create_duplicate_update_records_for_already_encumbered_privileges. Root cause observed: PutEvents on event bus 'license-data-events' raised ResourceNotFoundException (Event bus does not exist), leading to duplication logic not behaving as expected.
[error] 1-1: Test failure: expected 0 matching_updates but got 1 in test_license_encumbrance_listener_does_not_create_duplicate_update_records_for_unencumbered_privileges_on_retry. Root cause observed: PutEvents on event bus 'license-data-events' raised ResourceNotFoundException (Event bus does not exist), causing processing to treat as error and affecting update records duplication check.
backend/compact-connect/lambdas/python/data-events/tests/function/test_encumbrance_events.py
Outdated
Show resolved
Hide resolved
added 2 commits
November 5, 2025 15:03
ChiefStief
changed the title
ChiefStief
changed the title
ChiefStief
changed the title
jusdino
suggested changes
Nov 16, 2025
Collaborator
jusdino
left a comment
jusdino
left a comment
There was a problem hiding this comment.
Looks good generally. You ran across a couple of items we've been trying to tidy up in the codebase that hopefully you can tidy up for us here:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
Outdated
Show resolved
Hide resolved
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
Show resolved
Hide resolved
backend/compact-connect/lambdas/python/data-events/tests/function/test_encumbrance_events.py
Outdated
Show resolved
Hide resolved
Collaborator
|
Also, now that we've converted to the git-tag workflow, you'll need to rebase your branch off from CSG's |
added 2 commits
November 19, 2025 17:26
ChiefStief
force-pushed
the
feat/confirm-no-duplicate-events
branch
from
7f2607b to
2716381
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py (1)
2994-3015: Privilege encumbrance update de‑dupe logic is sound and idempotentThe pre‑check that queries
get_update_records_for_privilegefor existingENCUMBRANCEupdates with matchingencumbranceDetails['adverseActionId']correctly prevents duplicate privilege update records on retried flows, while still allowing one record per privilege/adverse_action_id pair. TheencumbranceDetails is not Noneguard also keeps this safe with older records. The same pattern on both unencumbered and already‑encumbered privilege sets maintains consistent behavior.If you want to trim duplication, you could extract a small helper like
_has_encumbrance_update_for_adverse_action(...)and reuse it in both loops, but that’s purely optional.Also applies to: 3048-3068
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py(4 hunks)backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py(2 hunks)backend/compact-connect/lambdas/python/data-events/tests/function/test_encumbrance_events.py(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py
🧰 Additional context used
🧠 Learnings (19)
📓 Common learnings
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1135
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py:2642-2654
Timestamp: 2025-10-15T19:53:00.422Z
Learning: In backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py and provider_record_util.py, the get_adverse_action_by_id method correctly handles UUID vs string comparison as written. Attempting to add explicit type conversion causes the mapping to fail. The current implementation should not be changed to add UUID/string conversion logic.
Learnt from: ChiefStief
Repo: csg-org/CompactConnect PR: 1036
File: backend/compact-connect/lambdas/python/data-events/handlers/encumbrance_events.py:200-204
Timestamp: 2025-09-03T22:35:42.943Z
Learning: The adverseActionCategory and adverseActionId fields in encumbrance events are only needed when the event flow creates privilege update database records. Some privilege encumbrance event publications don't create database records and don't need these fields, which is why they're optional in the EncumbranceEventDetailSchema.
Learnt from: ChiefStief
Repo: csg-org/CompactConnect PR: 1093
File: webroot/src/locales/en.json:591-592
Timestamp: 2025-09-16T18:31:34.428Z
Learning: ChiefStief prefers to keep approved verbiage in CompactConnect even when there are minor technical inaccuracies, particularly for timezone-related text that has already gone through the approval process.
📚 Learning: 2025-10-15T19:53:00.422Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1135
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py:2642-2654
Timestamp: 2025-10-15T19:53:00.422Z
Learning: In backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py and provider_record_util.py, the get_adverse_action_by_id method correctly handles UUID vs string comparison as written. Attempting to add explicit type conversion causes the mapping to fail. The current implementation should not be changed to add UUID/string conversion logic.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-09-11T20:06:40.709Z
Learnt from: ChiefStief
Repo: csg-org/CompactConnect PR: 1036
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py:370-383
Timestamp: 2025-09-11T20:06:40.709Z
Learning: The EncumbranceDetailsSchema in backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/record.py does not contain a 'note' field. It only has clinicalPrivilegeActionCategory (String, optional), adverseActionId (UUID, required), and licenseJurisdiction (Jurisdiction, optional). When working with encumbrance notes, use encumbranceDetails['clinicalPrivilegeActionCategory'], not encumbranceDetails['note'].
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.pybackend/compact-connect/lambdas/python/data-events/tests/function/test_encumbrance_events.py
📚 Learning: 2025-09-11T20:06:40.709Z
Learnt from: ChiefStief
Repo: csg-org/CompactConnect PR: 1036
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py:370-383
Timestamp: 2025-09-11T20:06:40.709Z
Learning: The EncumbranceDetailsSchema in backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/record.py does not contain a 'note' field. It only has clinicalPrivilegeActionCategory (String, optional), adverseActionId (UUID, required), and licenseJurisdiction (Jurisdiction, optional).
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-09-03T22:29:24.535Z
Learnt from: ChiefStief
Repo: csg-org/CompactConnect PR: 1036
File: backend/compact-connect/lambdas/python/data-events/handlers/encumbrance_events.py:0-0
Timestamp: 2025-09-03T22:29:24.535Z
Learning: The EncumbranceEventDetailSchema in backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/data_event/api.py is used across multiple instances/contexts where adverseActionCategory and adverseActionId fields may be required in some cases and not present in others. This is an intentional design pattern for handling different event variants with a single schema.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-08-21T20:24:39.042Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1014
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/provider/api.py:0-0
Timestamp: 2025-08-21T20:24:39.042Z
Learning: When validating identifier fields like providerId in Marshmallow schemas, prefer using the UUID field type over String with Length validation, as UUID provides automatic format validation and is more semantically appropriate.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-09-03T22:35:42.943Z
Learnt from: ChiefStief
Repo: csg-org/CompactConnect PR: 1036
File: backend/compact-connect/lambdas/python/data-events/handlers/encumbrance_events.py:200-204
Timestamp: 2025-09-03T22:35:42.943Z
Learning: The adverseActionCategory and adverseActionId fields in encumbrance events are only needed when the event flow creates privilege update database records. Some privilege encumbrance event publications don't create database records and don't need these fields, which is why they're optional in the EncumbranceEventDetailSchema.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-08-15T22:26:08.128Z
Learnt from: jusdino
Repo: csg-org/CompactConnect PR: 1004
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/api.py:78-79
Timestamp: 2025-08-15T22:26:08.128Z
Learning: The encumbranceType field should not be included in public response schemas for adverse actions. It should only be available in internal/general response schemas, not in AdverseActionPublicResponseSchema.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-08-19T19:39:47.790Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 1019
File: webroot/src/network/licenseApi/data.api.ts:256-276
Timestamp: 2025-08-19T19:39:47.790Z
Learning: The backend API for CompactConnect will accept and require the new `encumbranceType` field in encumbrance requests, even though this field may not appear in the current (incomplete) API documentation.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-05-30T13:48:25.375Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 824
File: backend/compact-connect/lambdas/python/provider-data-v1/handlers/encumbrance.py:116-201
Timestamp: 2025-05-30T13:48:25.375Z
Learning: In encumbrance handling code, prefer to keep privilege and license encumbrance lifting implementations decoupled rather than extracting shared logic, as requirements between these implementations are likely to change in the future.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.pybackend/compact-connect/lambdas/python/data-events/tests/function/test_encumbrance_events.py
📚 Learning: 2025-09-03T22:33:32.687Z
Learnt from: ChiefStief
Repo: csg-org/CompactConnect PR: 1036
File: webroot/src/components/PrivilegeEventNode/PrivilegeEventNode.ts:51-53
Timestamp: 2025-09-03T22:33:32.687Z
Learning: For encumbrance and lifting_encumbrance events in the privilege system, createDate should always be present. If createDate is absent, it indicates a bug in the data pipeline that should be allowed to surface rather than being defensively handled.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-10-15T19:56:58.050Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1135
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/record.py:60-0
Timestamp: 2025-10-15T19:56:58.050Z
Learning: In the CompactConnect codebase, when migrating from a single field to a list field (e.g., clinicalPrivilegeActionCategory to clinicalPrivilegeActionCategories), both fields are intentionally kept as optional during the migration period to support backwards compatibility. Mutual exclusivity validation is not enforced during this phase, as the deprecated field will be removed in a follow-up PR with migration scripts.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-08-29T21:45:05.792Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1016
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py:2702-2710
Timestamp: 2025-08-29T21:45:05.792Z
Learning: In the lift_home_jurisdiction_license_privilege_encumbrances method, when latest_effective_lift_date is None, the method always returns an empty list for affected_privileges, so the existing if affected_privileges guard in the calling code already prevents event publishing without needing explicit None checks for the date.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.pybackend/compact-connect/lambdas/python/data-events/tests/function/test_encumbrance_events.py
📚 Learning: 2025-07-08T18:40:24.408Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 882
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/compact_configuration_client.py:287-359
Timestamp: 2025-07-08T18:40:24.408Z
Learning: In the CompactConnect codebase, landonshumway-ia prefers to avoid extraneous unit tests when existing test coverage is already sufficient to catch bugs. For the get_privilege_purchase_options method's live-jurisdiction filtering logic, the existing tests in the purchases test suite provide adequate coverage without needing additional edge case tests.
Applied to files:
backend/compact-connect/lambdas/python/data-events/tests/function/test_encumbrance_events.py
📚 Learning: 2025-08-25T19:04:48.800Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1014
File: backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_account_recovery.py:288-293
Timestamp: 2025-08-25T19:04:48.800Z
Learning: In the CompactConnect project, landonshumway-ia has already provided feedback on patch cleanup patterns in test setup methods for the account recovery tests, so similar suggestions about stopping patches in setUp/tearDown should not be repeated.
Applied to files:
backend/compact-connect/lambdas/python/data-events/tests/function/test_encumbrance_events.py
📚 Learning: 2025-06-04T20:07:09.680Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 824
File: backend/compact-connect/tests/app/test_event_listener.py:29-222
Timestamp: 2025-06-04T20:07:09.680Z
Learning: In the CompactConnect project, test methods that include property snapshots for different AWS resource configurations (like event listeners) should be kept decoupled rather than extracted into common helper methods, even if they appear similar. This avoids complex parameter passing and maintains test independence when snapshots may vary between configurations.
Applied to files:
backend/compact-connect/lambdas/python/data-events/tests/function/test_encumbrance_events.py
📚 Learning: 2025-09-09T19:28:53.260Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 1071
File: webroot/tests/helpers/setup.ts:153-155
Timestamp: 2025-09-09T19:28:53.260Z
Learning: In the CompactConnect project, component tests are kept minimal and focus primarily on basic functionality. The main testing effort is concentrated on the data layer (models, store, etc.). For component tests, API stub methods only need to exist to prevent runtime errors - they don't need to return specific mock data since components aren't being tested for full API integration scenarios.
Applied to files:
backend/compact-connect/lambdas/python/data-events/tests/function/test_encumbrance_events.py
📚 Learning: 2025-04-29T02:52:40.532Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 769
File: backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_encumbrance.py:138-147
Timestamp: 2025-04-29T02:52:40.532Z
Learning: In CompactConnect tests, hardcoded values (like license type abbreviations 'slp') in test queries are sometimes used intentionally rather than using dynamic lookups. This is a deliberate design decision to make tests fail if default test data changes, requiring developers to consciously update related tests.
Applied to files:
backend/compact-connect/lambdas/python/data-events/tests/function/test_encumbrance_events.py
📚 Learning: 2025-06-04T17:38:22.392Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 824
File: backend/compact-connect/tests/smoke/encumbrance_smoke_tests.py:461-640
Timestamp: 2025-06-04T17:38:22.392Z
Learning: For smoke tests in backend/compact-connect/tests/smoke/, prioritize linear narrative readability over reducing complexity metrics. These tests are designed to be run manually by developers for verification, and should be readable from top to bottom like a story, allowing developers to follow the complete workflow without jumping between helper methods.
Applied to files:
backend/compact-connect/lambdas/python/data-events/tests/function/test_encumbrance_events.py
🧬 Code graph analysis (2)
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py (3)
backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py (1)
get_update_records_for_privilege(783-802)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/__init__.py (5)
jurisdiction(32-33)jurisdiction(116-117)licenseType(40-41)licenseType(120-121)encumbranceDetails(155-159)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/common.py (2)
update(182-203)UpdateCategory(296-312)
backend/compact-connect/lambdas/python/data-events/tests/function/test_encumbrance_events.py (4)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/common.py (3)
UpdateCategory(296-312)update(182-203)PrivilegeEncumberedStatusEnum(330-334)backend/compact-connect/lambdas/python/data-events/handlers/encumbrance_events.py (2)
license_encumbrance_listener(287-344)license_encumbrance_lifted_listener(348-408)backend/compact-connect/lambdas/python/common/common_test/test_data_generator.py (5)
put_default_provider_record_in_provider_table(441-461)put_default_privilege_record_in_provider_table(342-352)put_default_adverse_action_record_in_provider_table(186-192)query_privilege_update_records_for_given_record_from_database(99-115)put_default_license_record_in_provider_table(270-280)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/__init__.py (7)
updateType(104-105)updatedValues(128-129)createDate(132-133)createDate(136-137)compact(28-29)compact(112-113)encumberedStatus(72-73)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: TestApp
🔇 Additional comments (2)
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py (1)
2903-2912: Adverse action ID typed as UUID is consistent with the data modelChanging
adverse_action_idtoUUIDand updating the docstring matches theAdverseActionData/EncumbranceDetailsSchemadefinition whereadverseActionIdis a UUID, and aligns with the existing pattern of handling identifiers as UUIDs inside the data layer without extra casting. No issues from a correctness standpoint. Based on learnings.Also applies to: 2918-2924
backend/compact-connect/lambdas/python/data-events/tests/function/test_encumbrance_events.py (1)
3036-3261: Excellent deduplication test coverage!These three tests correctly validate the idempotent behavior described in issue #1050 by confirming that retry/reprocessing of encumbrance events does not create duplicate privilege update records. The tests appropriately:
- Reuse the same event object (same
messageId) for both handler invocations to simulate genuine retry scenarios- Use the
query_privilege_update_records_for_given_record_from_databasehelper method, following the established interface pattern rather than hard-coded pk/sk queries- Verify record identity using
createDateas a reliable proxy to confirm the same record persists rather than a new duplicate being created- Cover the key scenarios: unencumbered privileges becoming encumbered, already-encumbered privileges receiving additional tracking, and license-encumbered privileges being lifted
The filtering logic correctly isolates the specific update records by
updateTypeandadverseActionId(where applicable), ensuring precise assertions.
Contributor
Author
|
@jlkravitz Ready for you to take a look. |
jlkravitz
reviewed
Nov 21, 2025
backend/compact-connect/lambdas/python/data-events/tests/function/test_encumbrance_events.py
Outdated
Show resolved
Hide resolved
jlkravitz
approved these changes
Dec 2, 2025
Collaborator
jlkravitz
left a comment
jlkravitz
left a comment
There was a problem hiding this comment.
@isabeleliassen Good to merge!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Requirements List
Description List
Testing List
backend/compact-connect/tests/unit/test_api.pyCloses #1050
Summary by CodeRabbit
Bug Fixes
Tests
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.