-
Notifications
You must be signed in to change notification settings - Fork 7
Remove deprecated 'clinicalPrivilegeActionCategory' from backend #1243
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.
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
Remove deprecated 'clinicalPrivilegeActionCategory' from backend #1243
Conversation
📝 WalkthroughWalkthroughRemoves the deprecated Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py (1)
379-389: Encumbrance note derivation from multi-category list is appropriateUsing
', '.join(encumbranceDetails['clinicalPrivilegeActionCategories'])to populateevent['note']is a clean way to surface the new list-based categories while keeping the history note a simple string for consumers. Given the migration away from the singular field, this is consistent with the updated EncumbranceDetails schema.If you expect any legacy or malformed records to survive the migration, you could defensively use
.get('clinicalPrivilegeActionCategories') or []to avoid a KeyError; otherwise this is fine as-is.backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/record.py (1)
48-52: Pre-dump hook behavior updated; docstring now out of dateRouting
pre_dump_serializationthroughgenerate_pk_skand then returningin_datais consistent with removing the legacy clinicalPrivilegeActionCategory migration logic and relying solely on the new list-based field. The PK/SK composition itself looks coherent with other provider table key patterns for adverse actions.The docstring on Line 50 still mentions “ensure the clinicalPrivilegeActionCategories list is serialized correctly”, which no longer reflects what this hook does. Consider updating it to describe PK/SK generation instead.
backend/compact-connect/lambdas/python/common/tests/unit/test_data_model/test_schema/test_investigation.py (1)
138-151: Patch schema test aligned with list-based encumbrance categoriesThe PATCH payload now uses
clinicalPrivilegeActionCategoriesas a list, which matches the updated schema and the rest of the encumbrance flow. The existing assertion (thatload()returns a dict) is still valid; if you ever want stronger coverage, you could additionally assert thatresult['encumbrance']['clinicalPrivilegeActionCategories']matches the input list, but that’s not strictly necessary for this PR.backend/compact-connect/lambdas/python/provider-data-v1/handlers/encumbrance.py (1)
68-107: EnsureclinicalPrivilegeActionCategoriesis required (or guard against it being absent)The generator now unconditionally does:
adverse_action.clinicalPrivilegeActionCategories = adverse_action_post_body['clinicalPrivilegeActionCategories']This is perfect if
AdverseActionPostRequestSchemahas been updated soclinicalPrivilegeActionCategoriesis a required field for encumbrance requests, because missing data will surface as a 400 via the schema instead of silently proceeding.If that field is still optional in the POST schema (mirroring the old, optional singular field), this line would raise a
KeyErrorand produce a 500 for valid-but-no-note requests. In that case, either:
- Make the field required in
AdverseActionPostRequestSchema, or- Switch to a guarded assignment (e.g., only set the property when the key is present) depending on product requirements.
Please double-check the schema to ensure its required/optional semantics match this new direct access.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/__init__.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/api.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/record.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/record.pybackend/compact-connect/lambdas/python/common/tests/resources/api/adverse-action-post.jsonbackend/compact-connect/lambdas/python/common/tests/unit/test_data_model/test_schema/test_investigation.pybackend/compact-connect/lambdas/python/common/tests/unit/test_provider_record_util.pybackend/compact-connect/lambdas/python/data-events/tests/function/test_encumbrance_events.pybackend/compact-connect/lambdas/python/provider-data-v1/handlers/encumbrance.pybackend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_encumbrance.pybackend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_investigation.pybackend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_privilege_history.pybackend/compact-connect/stacks/api_stack/v1_api/api_model.pybackend/compact-connect/tests/resources/snapshots/GET_PROVIDER_RESPONSE_SCHEMA.jsonbackend/compact-connect/tests/resources/snapshots/LICENSE_ENCUMBRANCE_REQUEST_SCHEMA.jsonbackend/compact-connect/tests/resources/snapshots/PATCH_LICENSE_INVESTIGATION_REQUEST_SCHEMA.jsonbackend/compact-connect/tests/resources/snapshots/PATCH_PRIVILEGE_INVESTIGATION_REQUEST_SCHEMA.jsonbackend/compact-connect/tests/resources/snapshots/PRIVILEGE_ENCUMBRANCE_REQUEST_SCHEMA.jsonbackend/compact-connect/tests/resources/snapshots/PROVIDER_USER_RESPONSE_SCHEMA.jsonbackend/compact-connect/tests/smoke/investigation_smoke_tests.py
💤 Files with no reviewable changes (9)
- backend/compact-connect/tests/resources/snapshots/PRIVILEGE_ENCUMBRANCE_REQUEST_SCHEMA.json
- backend/compact-connect/tests/resources/snapshots/LICENSE_ENCUMBRANCE_REQUEST_SCHEMA.json
- backend/compact-connect/tests/resources/snapshots/PATCH_LICENSE_INVESTIGATION_REQUEST_SCHEMA.json
- backend/compact-connect/tests/resources/snapshots/GET_PROVIDER_RESPONSE_SCHEMA.json
- backend/compact-connect/tests/resources/snapshots/PATCH_PRIVILEGE_INVESTIGATION_REQUEST_SCHEMA.json
- backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/init.py
- backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/record.py
- backend/compact-connect/tests/resources/snapshots/PROVIDER_USER_RESPONSE_SCHEMA.json
- backend/compact-connect/stacks/api_stack/v1_api/api_model.py
🧰 Additional context used
🧠 Learnings (22)
📓 Common learnings
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.
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'].
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).
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 769
File: backend/compact-connect/stacks/api_stack/v1_api/api_model.py:397-401
Timestamp: 2025-04-29T01:59:51.222Z
Learning: In the CompactConnect project, validation constraints like enum values should be defined only in runtime code (Lambda) rather than duplicating them in CDK API schema definitions. This applies to fields like clinicalPrivilegeActionCategory and other similar enumerations.
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.
📚 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/provider-data-v1/tests/function/test_handlers/test_encumbrance.pybackend/compact-connect/lambdas/python/common/tests/unit/test_provider_record_util.pybackend/compact-connect/lambdas/python/provider-data-v1/handlers/encumbrance.pybackend/compact-connect/lambdas/python/common/tests/unit/test_data_model/test_schema/test_investigation.pybackend/compact-connect/lambdas/python/data-events/tests/function/test_encumbrance_events.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/api.pybackend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_privilege_history.pybackend/compact-connect/lambdas/python/common/tests/resources/api/adverse-action-post.jsonbackend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.pybackend/compact-connect/tests/smoke/investigation_smoke_tests.pybackend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_investigation.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/record.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.pybackend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_encumbrance.pybackend/compact-connect/lambdas/python/common/tests/unit/test_provider_record_util.pybackend/compact-connect/lambdas/python/provider-data-v1/handlers/encumbrance.pybackend/compact-connect/lambdas/python/common/tests/unit/test_data_model/test_schema/test_investigation.pybackend/compact-connect/lambdas/python/data-events/tests/function/test_encumbrance_events.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/api.pybackend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_privilege_history.pybackend/compact-connect/lambdas/python/common/tests/resources/api/adverse-action-post.jsonbackend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.pybackend/compact-connect/tests/smoke/investigation_smoke_tests.pybackend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_investigation.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/record.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/provider-data-v1/tests/function/test_handlers/test_encumbrance.pybackend/compact-connect/lambdas/python/provider-data-v1/handlers/encumbrance.pybackend/compact-connect/lambdas/python/common/tests/unit/test_data_model/test_schema/test_investigation.pybackend/compact-connect/lambdas/python/data-events/tests/function/test_encumbrance_events.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/api.pybackend/compact-connect/lambdas/python/common/tests/resources/api/adverse-action-post.jsonbackend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/record.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.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.pybackend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_encumbrance.pybackend/compact-connect/lambdas/python/provider-data-v1/handlers/encumbrance.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.pybackend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_encumbrance.pybackend/compact-connect/lambdas/python/common/tests/unit/test_provider_record_util.pybackend/compact-connect/lambdas/python/provider-data-v1/handlers/encumbrance.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.pybackend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_investigation.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.pybackend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_encumbrance.pybackend/compact-connect/lambdas/python/provider-data-v1/handlers/encumbrance.pybackend/compact-connect/lambdas/python/common/tests/unit/test_data_model/test_schema/test_investigation.pybackend/compact-connect/lambdas/python/data-events/tests/function/test_encumbrance_events.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/api.pybackend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_privilege_history.pybackend/compact-connect/lambdas/python/common/tests/resources/api/adverse-action-post.jsonbackend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.pybackend/compact-connect/tests/smoke/investigation_smoke_tests.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/record.py
📚 Learning: 2025-10-28T15:05:49.315Z
Learnt from: jusdino
Repo: csg-org/CompactConnect PR: 1167
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py:1698-1701
Timestamp: 2025-10-28T15:05:49.315Z
Learning: In the CompactConnect Python codebase, enums such as InvestigationStatusEnum, InvestigationAgainstEnum, ActiveInactiveStatus, LicenseEncumberedStatusEnum, and PrivilegeEncumberedStatusEnum derive from StrEnum. When writing DynamoDB AttributeValues (e.g., {'S': ...}) in files like backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py, pass the enum member directly (without .value). Future reviews should not recommend using .value for these enums.
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.pybackend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_encumbrance.pybackend/compact-connect/lambdas/python/common/tests/unit/test_provider_record_util.pybackend/compact-connect/lambdas/python/provider-data-v1/handlers/encumbrance.pybackend/compact-connect/lambdas/python/data-events/tests/function/test_encumbrance_events.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/api.pybackend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_privilege_history.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py
📚 Learning: 2025-08-26T15:30:29.956Z
Learnt from: ChiefStief
Repo: csg-org/CompactConnect PR: 1012
File: backend/compact-connect/docs/design/README.md:503-512
Timestamp: 2025-08-26T15:30:29.956Z
Learning: In CompactConnect, the use of UTC-4:00 for privilege timeline events (encumbrances and expirations) was an agreed-upon business decision, not a technical choice made by individual developers. This fixed offset approach should be respected as an intentional organizational requirement.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-12-16T21:43:07.408Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1219
File: backend/compact-connect/lambdas/python/search/handlers/search.py:131-140
Timestamp: 2025-12-16T21:43:07.408Z
Learning: In backend/compact-connect/lambdas/python/search/handlers/search.py, avoid logging the full request body. Do not log sensitive content by default. If logging is required for security investigations, redact or mask sensitive fields (e.g., credentials, tokens, PII) and log only safe metadata (method, path, status, user identifier). Use a secure, access-controlled audit log or feature flag to enable such logs, ensuring minimal exposure and compliance with security policies. This guideline targets Python backend handlers handling external requests and should be considered for similar files with request processing.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.pybackend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_encumbrance.pybackend/compact-connect/lambdas/python/common/tests/unit/test_provider_record_util.pybackend/compact-connect/lambdas/python/provider-data-v1/handlers/encumbrance.pybackend/compact-connect/lambdas/python/common/tests/unit/test_data_model/test_schema/test_investigation.pybackend/compact-connect/lambdas/python/data-events/tests/function/test_encumbrance_events.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/api.pybackend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_privilege_history.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.pybackend/compact-connect/tests/smoke/investigation_smoke_tests.pybackend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_investigation.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/record.py
📚 Learning: 2025-04-29T02:10:38.400Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 769
File: backend/compact-connect/lambdas/python/provider-data-v1/handlers/encumbrance.py:53-60
Timestamp: 2025-04-29T02:10:38.400Z
Learning: Lambda functions in the provider-data-v1 module that are exposed via API Gateway endpoints (like encumbrance_handler) should only be called through the API, not directly. Therefore, they don't need to handle various body formats as the API Gateway will consistently provide event['body'] as a JSON string.
Applied to files:
backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_encumbrance.py
📚 Learning: 2025-04-29T01:57:43.684Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 769
File: backend/compact-connect/lambdas/python/common/tests/resources/dynamo/provider.json:5-5
Timestamp: 2025-04-29T01:57:43.684Z
Learning: The provider.json test data contains both "providerDateOfUpdate" and "dateOfUpdate" fields which serve different purposes, and both should be maintained in the test files.
Applied to files:
backend/compact-connect/lambdas/python/common/tests/unit/test_provider_record_util.py
📚 Learning: 2025-06-03T16:41:07.757Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 796
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py:58-64
Timestamp: 2025-06-03T16:41:07.757Z
Learning: The static `get_provider_record` method in `ProviderRecordUtility` is legacy code being phased out in favor of the `ProviderUserRecords` class. During this migration, they are keeping the static method backwards compatible (returning `None`) while the new `ProviderUserRecords` class implements the better pattern of raising exceptions.
Applied to files:
backend/compact-connect/lambdas/python/common/tests/unit/test_provider_record_util.py
📚 Learning: 2025-10-24T21:40:48.912Z
Learnt from: jusdino
Repo: csg-org/CompactConnect PR: 1167
File: backend/compact-connect/lambdas/python/provider-data-v1/handlers/investigation.py:59-66
Timestamp: 2025-10-24T21:40:48.912Z
Learning: For the investigation endpoints in backend/compact-connect/lambdas/python/provider-data-v1/handlers/investigation.py, API Gateway validators are configured to guarantee that the request body exists and is non-empty, so defensive checks like event.get('body') or '{}' are not needed in _load_investigation_patch_body.
Applied to files:
backend/compact-connect/lambdas/python/common/tests/unit/test_data_model/test_schema/test_investigation.pybackend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_investigation.py
📚 Learning: 2025-04-29T01:59:51.222Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 769
File: backend/compact-connect/stacks/api_stack/v1_api/api_model.py:397-401
Timestamp: 2025-04-29T01:59:51.222Z
Learning: In the CompactConnect project, validation constraints like enum values should be defined only in runtime code (Lambda) rather than duplicating them in CDK API schema definitions. This applies to fields like clinicalPrivilegeActionCategory and other similar enumerations.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/api.py
📚 Learning: 2025-04-29T02:18:11.099Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 769
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/record.py:40-43
Timestamp: 2025-04-29T02:18:11.099Z
Learning: In the CompactConnect codebase, API design follows a pattern where fields are either provided with valid values or omitted entirely. Null values are not allowed, even for optional fields. This applies to Marshmallow schemas where fields are configured with `required=False, allow_none=False`.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/api.py
📚 Learning: 2025-08-15T19:59:09.332Z
Learnt from: jusdino
Repo: csg-org/CompactConnect PR: 1004
File: backend/compact-connect/docs/internal/api-specification/latest-oas30.json:2305-2354
Timestamp: 2025-08-15T19:59:09.332Z
Learning: In the CompactConnect API, the history endpoint paths are intentionally different between internal and public APIs. Internal compacts history includes "/privileges/" in the path (/v1/compacts/{compact}/providers/{providerId}/privileges/jurisdiction/{jurisdiction}/licenseType/{licenseType}/history) while public compacts history omits it (/v1/public/compacts/{compact}/providers/{providerId}/jurisdiction/{jurisdiction}/licenseType/{licenseType}/history). This inconsistency is a deliberate product design choice, not an oversight.
Applied to files:
backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_privilege_history.py
📚 Learning: 2025-09-04T17:55:02.692Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1058
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py:785-786
Timestamp: 2025-09-04T17:55:02.692Z
Learning: In backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py, the sanitize_provider_account_profile_fields_from_response method is called within generate_api_response_object, which serves as the central method that all endpoints use to construct provider response objects. This ensures that sensitive profile fields are sanitized at the source, eliminating the need for additional sanitization calls in downstream methods like those in utils.py.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py
📚 Learning: 2025-11-06T18:11:58.272Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 1196
File: webroot/src/network/mocks/mock.data.ts:1919-1944
Timestamp: 2025-11-06T18:11:58.272Z
Learning: In CompactConnect, the investigation event type (`updateType: 'investigation'`) in privilege/license history is intentionally not yet fully supported in the frontend UI translation and categorization logic (webroot/src/locales/*.json and webroot/src/models/LicenseHistoryItem/LicenseHistoryItem.model.ts). The server license history API needs to be updated first to return sufficient information about investigation events. Until then, the UI gracefully falls back to displaying "Unknown" for investigation events, which is acceptable as an interim solution.
<!--
Applied to files:
backend/compact-connect/tests/smoke/investigation_smoke_tests.py
📚 Learning: 2025-06-23T20:19:18.952Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 877
File: backend/compact-connect/tests/smoke/practitioner_email_update_smoke_tests.py:22-192
Timestamp: 2025-06-23T20:19:18.952Z
Learning: In the CompactConnect project, smoke test functions (like test_practitioner_email_update in backend/compact-connect/tests/smoke/practitioner_email_update_smoke_tests.py) should remain as single large functions rather than being broken down into smaller helper functions, based on the project maintainer's preference and reasoning.
Applied to files:
backend/compact-connect/tests/smoke/investigation_smoke_tests.py
🧬 Code graph analysis (5)
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py (1)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/__init__.py (4)
clinicalPrivilegeActionCategories(82-83)clinicalPrivilegeActionCategories(86-87)adverseActionId(114-115)adverseActionId(118-119)
backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_encumbrance.py (1)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/common.py (1)
ClinicalPrivilegeActionCategory(401-414)
backend/compact-connect/lambdas/python/provider-data-v1/handlers/encumbrance.py (1)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/__init__.py (2)
clinicalPrivilegeActionCategories(82-83)clinicalPrivilegeActionCategories(86-87)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/api.py (1)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/fields.py (1)
ClinicalPrivilegeActionCategoryField(125-127)
backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_investigation.py (1)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/common.py (1)
ClinicalPrivilegeActionCategory(401-414)
⏰ 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). (2)
- GitHub Check: TestApp
- GitHub Check: TestAndLintPurchases
🔇 Additional comments (10)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/api.py (1)
26-30: Required, non-emptyclinicalPrivilegeActionCategorieson POST looks correctMaking
clinicalPrivilegeActionCategoriesrequired withLength(min=1)aligns the POST contract with the finalized multi-category model and the removal of the deprecated singular field, while response schemas can still omit it when not applicable. This matches the migration plan captured in the prior learnings.Based on learnings, this completes the planned transition from the singular field.
backend/compact-connect/lambdas/python/common/tests/resources/api/adverse-action-post.json (1)
2-5: Fixture updated to plural list field matches schemaSwitching the test payload to
clinicalPrivilegeActionCategoriesas a single-element list aligns this resource with the new POST schema and enum-backed field.backend/compact-connect/lambdas/python/data-events/tests/function/test_encumbrance_events.py (1)
288-326: Tests correctly migrated toclinicalPrivilegeActionCategoriesin encumbrance detailsUpdating both the adverse action test fixture and the expected
encumbranceDetailsdict to useclinicalPrivilegeActionCategoriesas a list keeps these tests in sync with the new encumbrance schema and the note-building logic inProviderRecordUtility. The remaining TODO around the feature flag test can be addressed separately under its linked issue.backend/compact-connect/tests/smoke/investigation_smoke_tests.py (1)
393-401: Smoke test encumbrance payload updated to plural list fieldUsing
clinicalPrivilegeActionCategoriesas a list in the close-with-encumbrance payload keeps this end-to-end investigation smoke test aligned with the updated encumbrance API contract and downstream data model.backend/compact-connect/lambdas/python/common/tests/unit/test_provider_record_util.py (1)
920-992: Encumbrance test data now correctly uses the plural list fieldSwitching
encumbranceDetailsto useclinicalPrivilegeActionCategorieswith a single-item list matches the new schema while still exercising the note extraction logic (note: 'Non-compliance With Requirements'). Looks consistent with the multi-category design.backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_investigation.py (1)
37-46: Investigation close body correctly migrated to list-based categoriesUsing
clinicalPrivilegeActionCategoriesas a list ofClinicalPrivilegeActionCategoryvalues matches the new encumbrance representation and keeps the request payload strongly typed via the enum.backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_privilege_history.py (1)
21-60: History tests now seed encumbrances with list-based categoriesBoth
_when_testing_provider_user_event_with_custom_claimsand_when_testing_public_endpointnow populateencumbranceDetails['clinicalPrivilegeActionCategories']as a single-item list. This lines up with the multi-category data model and the expectation that downstream history construction still surfaces a singlenotestring where appropriate.backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_encumbrance.py (1)
43-52: Encumbrance tests correctly exercise the new list-based categories in all flag pathsUpdating
_generate_test_body()and the flag-off expectedencumbranceDetailsto useclinicalPrivilegeActionCategoriesas a list keeps the privilege/ license encumbrance tests in sync with the new AdverseAction model and confirms that behavior is consistent regardless of the feature-flag mock. No functional issues here.Also applies to: 197-207
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py (2)
1530-1533: LGTM! Clean removal of deprecated field.The encumbrance_details construction now consistently uses
clinicalPrivilegeActionCategories(plural, list-based) without feature flag branching. This simplifies the code and aligns with the completed migration.Based on learnings, the migration from singular to plural field was intentional, and this PR completes the cleanup phase.
3064-3068: LGTM! Consistent use of migrated field.The encumbrance_details construction here also correctly uses
clinicalPrivilegeActionCategories(plural) and appropriately includeslicenseJurisdictionfor home jurisdiction license privilege encumbrances. The structure is consistent with the privilege encumbrance method while including license-specific context.Based on learnings, the deprecated singular field has been fully removed across the codebase as part of the migration completion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
backend/compact-connect/lambdas/python/common/tests/unit/test_provider_record_util.py (1)
914-990: LGTM! Consider adding multi-category test case.The migration from
clinicalPrivilegeActionCategory(singular string) toclinicalPrivilegeActionCategories(list) is correctly reflected, and the feature flag decorator removal aligns with the PR objectives. However, this test only validates a single-item list. Consider adding a test case with multiple categories to ensure the join logic (that produces the comma-separatednotestring) works correctly.💡 Example test case with multiple categories
'encumbranceDetails': { 'clinicalPrivilegeActionCategories': [ 'Non-compliance With Requirements', 'Criminal Conviction', 'Substance Abuse' ], 'licenseJurisdiction': 'oh', },Expected note:
'note': 'Non-compliance With Requirements, Criminal Conviction, Substance Abuse'
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
backend/compact-connect/lambdas/python/common/cc_common/feature_flag_enum.pybackend/compact-connect/lambdas/python/common/tests/unit/test_provider_record_util.pybackend/compact-connect/lambdas/python/data-events/tests/function/test_encumbrance_events.pybackend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_encumbrance.pybackend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_privilege_history.pybackend/compact-connect/stacks/feature_flag_stack/__init__.py
💤 Files with no reviewable changes (3)
- backend/compact-connect/stacks/feature_flag_stack/init.py
- backend/compact-connect/lambdas/python/data-events/tests/function/test_encumbrance_events.py
- backend/compact-connect/lambdas/python/common/cc_common/feature_flag_enum.py
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_privilege_history.py
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
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.
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'].
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).
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 769
File: backend/compact-connect/stacks/api_stack/v1_api/api_model.py:397-401
Timestamp: 2025-04-29T01:59:51.222Z
Learning: In the CompactConnect project, validation constraints like enum values should be defined only in runtime code (Lambda) rather than duplicating them in CDK API schema definitions. This applies to fields like clinicalPrivilegeActionCategory and other similar enumerations.
📚 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/provider-data-v1/tests/function/test_handlers/test_encumbrance.pybackend/compact-connect/lambdas/python/common/tests/unit/test_provider_record_util.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/provider-data-v1/tests/function/test_handlers/test_encumbrance.pybackend/compact-connect/lambdas/python/common/tests/unit/test_provider_record_util.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/provider-data-v1/tests/function/test_handlers/test_encumbrance.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/provider-data-v1/tests/function/test_handlers/test_encumbrance.pybackend/compact-connect/lambdas/python/common/tests/unit/test_provider_record_util.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/provider-data-v1/tests/function/test_handlers/test_encumbrance.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/provider-data-v1/tests/function/test_handlers/test_encumbrance.pybackend/compact-connect/lambdas/python/common/tests/unit/test_provider_record_util.py
📚 Learning: 2025-12-16T21:43:07.408Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1219
File: backend/compact-connect/lambdas/python/search/handlers/search.py:131-140
Timestamp: 2025-12-16T21:43:07.408Z
Learning: In backend/compact-connect/lambdas/python/search/handlers/search.py, avoid logging the full request body. Do not log sensitive content by default. If logging is required for security investigations, redact or mask sensitive fields (e.g., credentials, tokens, PII) and log only safe metadata (method, path, status, user identifier). Use a secure, access-controlled audit log or feature flag to enable such logs, ensuring minimal exposure and compliance with security policies. This guideline targets Python backend handlers handling external requests and should be considered for similar files with request processing.
Applied to files:
backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_encumbrance.pybackend/compact-connect/lambdas/python/common/tests/unit/test_provider_record_util.py
🧬 Code graph analysis (1)
backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_encumbrance.py (1)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/common.py (1)
ClinicalPrivilegeActionCategory(401-414)
⏰ 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 (4)
backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_encumbrance.py (3)
50-50: LGTM! Correctly migrated to plural field with list format.The test body generator now uses
clinicalPrivilegeActionCategories(plural) with a list value, properly reflecting the removal of the deprecated singular field.
137-171: LGTM! Test correctly validates the plural field in privilege update records.The test expectation at line 161 properly uses
clinicalPrivilegeActionCategoriesas a list, matching the new standard implementation. The validation confirms that encumbrance details are correctly stored with the plural field structure.
1-1199: Excellent migration! Test suite successfully updated to use the plural field exclusively.The test file has been cleanly migrated from the deprecated
clinicalPrivilegeActionCategory(singular) toclinicalPrivilegeActionCategories(plural list). All test scenarios maintain comprehensive coverage while validating the correct implementation of the new field structure. The removal of feature-flag-related tests is appropriate given that the database migration is complete.backend/compact-connect/lambdas/python/common/tests/unit/test_provider_record_util.py (1)
992-1068: Verify encumbranceDetails schema consistency.The test uses a
'note'field inencumbranceDetails(line 1032), but according to the schema definition and migration pattern,encumbranceDetailsshould useclinicalPrivilegeActionCategories(a list), not a standalone'note'field. The first test correctly usesclinicalPrivilegeActionCategories; this test should likely follow the same pattern for consistency.Based on learnings, EncumbranceDetailsSchema contains
clinicalPrivilegeActionCategories,adverseActionId, andlicenseJurisdiction. Consider updating the test data to:'encumbranceDetails': { 'clinicalPrivilegeActionCategories': ['Non-compliance With Requirements'], 'licenseJurisdiction': 'oh', },
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (6)
webroot/src/network/mocks/mock.data.api.ts (2)
229-255: LGTM - Mock response correctly unified.The removal of feature-flag conditional logic ensures mock responses consistently return
npdbCategories. This aligns with the backend standardization on the array-based field.Optional: Consider removing unused parameter
The
npdbCategoryparameter (line 235) is no longer used in the response. Consider removing it from the function signature in a future cleanup to reduce confusion:public encumberLicense( compact, licenseeId, licenseState, licenseType, encumbranceType, - npdbCategory, npdbCategories, startDate ) {This would make the signature match the actual behavior and reduce maintenance burden.
327-353: LGTM - Mock response correctly unified.The removal of feature-flag conditional logic in
encumberPrivilegeensures consistent mock responses withnpdbCategories.Optional: Consider removing unused parameter
Similar to
encumberLicense, thenpdbCategoryparameter (line 333) is no longer used. Consider removing it from the function signature in a future cleanup:public encumberPrivilege( compact, licenseeId, privilegeState, licenseType, encumbranceType, - npdbCategory, npdbCategories, startDate ) {webroot/src/network/licenseApi/data.api.ts (4)
267-284: LGTM - Request payload correctly unified.The
encumberLicensemethod now consistently sendsclinicalPrivilegeActionCategories: npdbCategorieswithout feature-flag branching, aligning with the backend standardization.Optional: Consider removing unused parameter
The
npdbCategoryparameter (line 273) is no longer used in the request payload. Consider removing it from the function signature in a future cleanup:public async encumberLicense( compact: string, licenseeId: string, licenseState: string, licenseType: string, encumbranceType: string, - npdbCategory: string, npdbCategories: Array<string>, startDate: string ) {This would also require updating call sites in component code (not visible in this PR).
344-372: LGTM - Investigation encumbrance payload unified.The
updateLicenseInvestigationmethod correctly usesclinicalPrivilegeActionCategories: encumbrance.npdbCategorieswhen an encumbrance is provided, removing feature-flag branching.Optional: Consider updating encumbrance type
The
encumbranceparameter's type definition (lines 350-355) includes an unusednpdbCategoryfield. Consider removing it for clarity:encumbrance?: { encumbranceType: string, - npdbCategory: string, npdbCategories: Array<string>, startDate: string }
409-426: LGTM - Privilege encumbrance payload unified.The
encumberPrivilegemethod correctly standardizes onclinicalPrivilegeActionCategories: npdbCategories, consistent with license encumbrance changes.Optional: Consider removing unused parameter
The
npdbCategoryparameter (line 415) is no longer used. Consider removing it from the function signature:public async encumberPrivilege( compact: string, licenseeId: string, privilegeState: string, licenseType: string, encumbranceType: string, - npdbCategory: string, npdbCategories: Array<string>, startDate: string ) {
486-514: LGTM - Privilege investigation payload unified.The
updatePrivilegeInvestigationmethod correctly usesclinicalPrivilegeActionCategories: encumbrance.npdbCategories, maintaining consistency with the license investigation changes.Optional: Consider updating encumbrance type
The
encumbranceparameter's type definition (lines 492-497) includes an unusednpdbCategoryfield:encumbrance?: { encumbranceType: string, - npdbCategory: string, npdbCategories: Array<string>, startDate: string }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
webroot/src/app.config.tswebroot/src/components/LicenseCard/LicenseCard.tswebroot/src/components/LicenseCard/LicenseCard.vuewebroot/src/components/PrivilegeCard/PrivilegeCard.tswebroot/src/components/PrivilegeCard/PrivilegeCard.vuewebroot/src/models/AdverseAction/AdverseAction.model.spec.tswebroot/src/models/AdverseAction/AdverseAction.model.tswebroot/src/network/licenseApi/data.api.tswebroot/src/network/mocks/mock.data.api.tswebroot/src/network/mocks/mock.data.ts
💤 Files with no reviewable changes (3)
- webroot/src/app.config.ts
- webroot/src/models/AdverseAction/AdverseAction.model.spec.ts
- webroot/src/models/AdverseAction/AdverseAction.model.ts
🧰 Additional context used
🧠 Learnings (17)
📓 Common learnings
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.
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'].
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).
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: 769
File: backend/compact-connect/stacks/api_stack/v1_api/api_model.py:397-401
Timestamp: 2025-04-29T01:59:51.222Z
Learning: In the CompactConnect project, validation constraints like enum values should be defined only in runtime code (Lambda) rather than duplicating them in CDK API schema definitions. This applies to fields like clinicalPrivilegeActionCategory and other similar enumerations.
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.
📚 Learning: 2025-08-21T16:36:48.723Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 1019
File: webroot/src/components/PrivilegeCard/PrivilegeCard.vue:270-278
Timestamp: 2025-08-21T16:36:48.723Z
Learning: In PrivilegeCard component's unencumber modal (webroot/src/components/PrivilegeCard/PrivilegeCard.vue), the unencumber-select wrapper element intentionally uses space key handling to catch bubbled events from child InputCheckbox elements for custom handling actions. When adverseAction.hasEndDate() is true, items show an inactive-category div and are designed to be non-interactive (not focusable), while items without end dates contain focusable InputCheckbox child elements. This design pattern is consistent with prior implementation and represents intentional UX behavior.
Applied to files:
webroot/src/components/PrivilegeCard/PrivilegeCard.vuewebroot/src/components/LicenseCard/LicenseCard.vuewebroot/src/components/PrivilegeCard/PrivilegeCard.tswebroot/src/components/LicenseCard/LicenseCard.ts
📚 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:
webroot/src/components/PrivilegeCard/PrivilegeCard.vuewebroot/src/network/mocks/mock.data.tswebroot/src/components/PrivilegeCard/PrivilegeCard.ts
📚 Learning: 2025-06-24T00:02:39.944Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 873
File: webroot/src/components/LicenseCard/LicenseCard.ts:414-443
Timestamp: 2025-06-24T00:02:39.944Z
Learning: In LicenseCard component's clickUnencumberItem method (webroot/src/components/LicenseCard/LicenseCard.ts), complex event handling for checkbox interactions is intentionally designed to ensure consistent behavior across checkbox input, wrapper label, and outer selection parent elements for custom UI requirements. This complexity should be preserved rather than simplified.
Applied to files:
webroot/src/components/PrivilegeCard/PrivilegeCard.vuewebroot/src/components/LicenseCard/LicenseCard.vuewebroot/src/components/PrivilegeCard/PrivilegeCard.tswebroot/src/components/LicenseCard/LicenseCard.ts
📚 Learning: 2025-08-12T22:51:48.937Z
Learnt from: rmolinares
Repo: csg-org/CompactConnect PR: 1009
File: webroot/src/pages/LicenseeProof/LicenseeProof.less:158-165
Timestamp: 2025-08-12T22:51:48.937Z
Learning: In webroot/src/pages/LicenseeProof/LicenseeProof.vue, the .max-gap elements inside .licenses-container are intentionally hard-coded as empty elements that serve as space placeholders for tablet+ screen widths. On mobile, they are hidden completely using the :empty pseudo-class. This is an intentional design pattern where the developers have full control over keeping these elements truly empty.
Applied to files:
webroot/src/components/LicenseCard/LicenseCard.vue
📚 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:
webroot/src/network/licenseApi/data.api.ts
📚 Learning: 2025-11-06T18:11:58.272Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 1196
File: webroot/src/network/mocks/mock.data.ts:1919-1944
Timestamp: 2025-11-06T18:11:58.272Z
Learning: In CompactConnect, the investigation event type (`updateType: 'investigation'`) in privilege/license history is intentionally not yet fully supported in the frontend UI translation and categorization logic (webroot/src/locales/*.json and webroot/src/models/LicenseHistoryItem/LicenseHistoryItem.model.ts). The server license history API needs to be updated first to return sufficient information about investigation events. Until then, the UI gracefully falls back to displaying "Unknown" for investigation events, which is acceptable as an interim solution.
<!--
Applied to files:
webroot/src/network/licenseApi/data.api.tswebroot/src/network/mocks/mock.data.ts
📚 Learning: 2025-11-12T21:06:06.981Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 1196
File: webroot/src/components/LicenseCard/LicenseCard.ts:388-396
Timestamp: 2025-11-12T21:06:06.981Z
Learning: In the CompactConnect investigation creation flow, the backend APIs for creating investigations (both `createLicenseInvestigation` and `createPrivilegeInvestigation` in webroot/src/network/licenseApi/data.api.ts) do not accept a `startDate` parameter. The backend automatically sets the investigation creation date to the timestamp when the API request is received, so the frontend UI does not need to capture or submit an investigation start date during the creation workflow.
Applied to files:
webroot/src/network/licenseApi/data.api.ts
📚 Learning: 2025-12-19T16:04:23.341Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 1234
File: webroot/src/network/searchApi/data.api.ts:83-95
Timestamp: 2025-12-19T16:04:23.341Z
Learning: In the CompactConnect codebase, all network API files under webroot/src/network/*/data.api.ts use a specific axios.create header configuration pattern with nested get/post/put objects inside the headers config. Reviewers should not flag this pattern as an issue, since it aligns with the project’s Axios version and conventions. If enforcing general axios patterns, treat this as a project-specific exception and focus on consistency within this path.
Applied to files:
webroot/src/network/licenseApi/data.api.ts
📚 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:
webroot/src/network/mocks/mock.data.ts
📚 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:
webroot/src/network/mocks/mock.data.ts
📚 Learning: 2025-11-06T18:10:16.437Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 1196
File: webroot/src/network/mocks/mock.data.ts:772-793
Timestamp: 2025-11-06T18:10:16.437Z
Learning: In CompactConnect, licenses and privileges can have investigations from different jurisdictions. For example, a license in Nevada (NV) can legitimately have investigations with jurisdiction Wyoming (WY). This cross-jurisdiction investigation pattern is valid in the data model and represents real-world scenarios where a practitioner licensed in one state may be under investigation by another state's authorities.
Applied to files:
webroot/src/network/mocks/mock.data.ts
📚 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:
webroot/src/network/mocks/mock.data.ts
📚 Learning: 2025-06-24T00:07:10.463Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 873
File: webroot/src/components/LicenseCard/LicenseCard.ts:509-528
Timestamp: 2025-06-24T00:07:10.463Z
Learning: In the CompactConnect frontend codebase, the project prefers to avoid early returns in frontend code when possible, as mentioned by jsandoval81 in webroot/src/components/LicenseCard/LicenseCard.ts.
Applied to files:
webroot/src/components/PrivilegeCard/PrivilegeCard.tswebroot/src/components/LicenseCard/LicenseCard.ts
📚 Learning: 2025-07-03T15:35:57.893Z
Learnt from: rmolinares
Repo: csg-org/CompactConnect PR: 905
File: webroot/src/components/UpdateHomeJurisdiction/UpdateHomeJurisdiction.vue:32-41
Timestamp: 2025-07-03T15:35:57.893Z
Learning: In the CompactConnect frontend codebase, the team prefers to keep non-dynamic text directly in Vue templates rather than moving it to computed properties in TypeScript modules, as this approach prevents cluttering the TS files with template labels.
Applied to files:
webroot/src/components/PrivilegeCard/PrivilegeCard.ts
📚 Learning: 2025-06-04T22:04:14.373Z
Learnt from: rmolinares
Repo: csg-org/CompactConnect PR: 843
File: webroot/src/components/Forms/InputDate/InputDate.ts:0-0
Timestamp: 2025-06-04T22:04:14.373Z
Learning: In the InputDate component (webroot/src/components/Forms/InputDate/InputDate.ts), immediate validation on every keystroke is intentional design behavior. The team prefers to alert users to encourage expected date format completion rather than deferring validation until the date is complete. This provides immediate feedback to guide users toward proper MM/dd/yyyy format completion.
Applied to files:
webroot/src/components/PrivilegeCard/PrivilegeCard.tswebroot/src/components/LicenseCard/LicenseCard.ts
📚 Learning: 2025-06-24T00:17:31.188Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 873
File: webroot/src/styles.common/_inputs.less:115-118
Timestamp: 2025-06-24T00:17:31.188Z
Learning: The team intentionally uses broad CSS selectors like `.dp__input_wrap div { position: static; }` to fix styling issues with the Vue Date-Picker library. They have experience with these styles working well, keep the library version pinned in yarn.lock, and have established processes to re-test everything when updating the library version. This approach is acceptable given their testing discipline and version control.
Applied to files:
webroot/src/components/LicenseCard/LicenseCard.ts
🧬 Code graph analysis (2)
webroot/src/components/PrivilegeCard/PrivilegeCard.ts (1)
webroot/src/models/FormInput/FormInput.model.ts (1)
FormInput(59-170)
webroot/src/components/LicenseCard/LicenseCard.ts (1)
webroot/src/models/FormInput/FormInput.model.ts (1)
FormInput(59-170)
⏰ 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). (3)
- GitHub Check: TestApp
- GitHub Check: TestAndLintPurchases
- GitHub Check: CheckWebroot
🔇 Additional comments (14)
webroot/src/components/PrivilegeCard/PrivilegeCard.vue (1)
217-219: LGTM - Feature flag removal successful.The template simplification correctly removes the conditional rendering that was gated by
ENCUMBER_MULTI_CATEGORY. Now always renderingInputSelectMultiplefor NPDB categories aligns with the standardization on the array-basedclinicalPrivilegeActionCategoriesfield throughout the codebase.webroot/src/components/LicenseCard/LicenseCard.vue (1)
157-159: LGTM - Consistent feature flag removal.The template simplification mirrors the change in
PrivilegeCard.vue, removing conditional rendering and standardizing onInputSelectMultiplefor NPDB categories. This ensures consistent UX across license and privilege encumbrance workflows.webroot/src/network/mocks/mock.data.ts (1)
743-743: LGTM - Mock data correctly migrated.The transformation from
clinicalPrivilegeActionCategory(string) toclinicalPrivilegeActionCategories(array) correctly maintains semantic meaning while aligning with the new backend schema. Each mock adverse action now uses the array-based field consistently.Also applies to: 760-760, 879-879, 916-916, 933-933
webroot/src/network/licenseApi/data.api.ts (1)
8-8: LGTM - Feature flag import removed.The removal of
FeatureGatesfrom imports is correct and consistent with eliminating feature-flag gating logic throughout the file.webroot/src/components/PrivilegeCard/PrivilegeCard.ts (5)
19-19: LGTM – Import cleanup is correct.The
FeatureGatesimport removal aligns with the deprecation ofENCUMBER_MULTI_CATEGORY_FLAG. OnlydateFormatPatternsis retained as it's still used for date validation.
231-236: LGTM – Simplified getter without feature gate branching.The getter now directly returns the mapped array, which is appropriate for the multi-select input component (
InputSelectMultiple). No default empty option is needed since multi-select inputs handle empty state differently than single-select dropdowns.
306-313: LGTM – Array-based form input is correctly configured.The
encumberModalNpdbCategoriesFormInput is properly set up:
- Uses
Joi.array().min(1)validation to require at least one selection- Initializes
valueas empty array[]- Uses
valueOptionsfor the multi-select optionsThis completes the migration from the single-value approach to the array-based approach. Based on learnings, the deprecated field is now safe to remove.
556-557: LGTM – Payload uses consistent array-based field.Both submission paths (investigation update and standalone encumbrance) now consistently use
npdbCategoriesas an array, matching the backend's expected schema after theclinicalPrivilegeActionCategorydeprecation.Also applies to: 571-572
1010-1010: LGTM – Mock populate correctly wraps value in array.The mock data correctly wraps the single option value in an array
[this.npdbCategoryOptions[1]?.value]to match the array-based form input.webroot/src/components/LicenseCard/LicenseCard.ts (5)
19-19: LGTM – Import cleanup matches PrivilegeCard.Consistent with PrivilegeCard.ts, only
dateFormatPatternsis retained after removingFeatureGates.
259-264: LGTM – Getter simplified consistently with PrivilegeCard.The
npdbCategoryOptionsgetter now directly returns the mapped array without feature gate branching.
314-321: LGTM – Array-based form input configuration is consistent.The
encumberModalNpdbCategoriesFormInput configuration matches PrivilegeCard.ts exactly, ensuring consistent behavior across both components.
500-501: LGTM – Payload structure aligned with backend expectations.Both submission paths correctly use
npdbCategoriesas an array, consistent with PrivilegeCard.ts and the backend schema after deprecation.Also applies to: 515-516
955-955: LGTM – Mock populate correctly uses array format.Matches PrivilegeCard.ts mock populate behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
backend/compact-connect/tests/resources/snapshots/PATCH_PRIVILEGE_INVESTIGATION_REQUEST_SCHEMA.json (1)
48-52: Schema migration completed successfully.The required field addition for
clinicalPrivilegeActionCategoriesis consistent with the broader migration from the deprecated singular field. The change correctly enforces the new plural field across privilege investigation requests.Note: This schema permits empty arrays for
clinicalPrivilegeActionCategories. If at least one category is semantically required, consider adding"minItems": 1to the array definition at lines 40-46.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
backend/compact-connect/tests/resources/snapshots/LICENSE_ENCUMBRANCE_REQUEST_SCHEMA.jsonbackend/compact-connect/tests/resources/snapshots/PATCH_LICENSE_INVESTIGATION_REQUEST_SCHEMA.jsonbackend/compact-connect/tests/resources/snapshots/PATCH_PRIVILEGE_INVESTIGATION_REQUEST_SCHEMA.jsonbackend/compact-connect/tests/resources/snapshots/PRIVILEGE_ENCUMBRANCE_REQUEST_SCHEMA.json
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/compact-connect/tests/resources/snapshots/LICENSE_ENCUMBRANCE_REQUEST_SCHEMA.json
🧰 Additional context used
🧠 Learnings (12)
📓 Common learnings
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.
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'].
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).
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: 769
File: backend/compact-connect/stacks/api_stack/v1_api/api_model.py:397-401
Timestamp: 2025-04-29T01:59:51.222Z
Learning: In the CompactConnect project, validation constraints like enum values should be defined only in runtime code (Lambda) rather than duplicating them in CDK API schema definitions. This applies to fields like clinicalPrivilegeActionCategory and other similar enumerations.
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.
📚 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/tests/resources/snapshots/PRIVILEGE_ENCUMBRANCE_REQUEST_SCHEMA.jsonbackend/compact-connect/tests/resources/snapshots/PATCH_PRIVILEGE_INVESTIGATION_REQUEST_SCHEMA.jsonbackend/compact-connect/tests/resources/snapshots/PATCH_LICENSE_INVESTIGATION_REQUEST_SCHEMA.json
📚 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/tests/resources/snapshots/PRIVILEGE_ENCUMBRANCE_REQUEST_SCHEMA.jsonbackend/compact-connect/tests/resources/snapshots/PATCH_PRIVILEGE_INVESTIGATION_REQUEST_SCHEMA.jsonbackend/compact-connect/tests/resources/snapshots/PATCH_LICENSE_INVESTIGATION_REQUEST_SCHEMA.json
📚 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/tests/resources/snapshots/PRIVILEGE_ENCUMBRANCE_REQUEST_SCHEMA.jsonbackend/compact-connect/tests/resources/snapshots/PATCH_PRIVILEGE_INVESTIGATION_REQUEST_SCHEMA.jsonbackend/compact-connect/tests/resources/snapshots/PATCH_LICENSE_INVESTIGATION_REQUEST_SCHEMA.json
📚 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/tests/resources/snapshots/PRIVILEGE_ENCUMBRANCE_REQUEST_SCHEMA.jsonbackend/compact-connect/tests/resources/snapshots/PATCH_PRIVILEGE_INVESTIGATION_REQUEST_SCHEMA.json
📚 Learning: 2025-04-29T01:59:51.222Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 769
File: backend/compact-connect/stacks/api_stack/v1_api/api_model.py:397-401
Timestamp: 2025-04-29T01:59:51.222Z
Learning: In the CompactConnect project, validation constraints like enum values should be defined only in runtime code (Lambda) rather than duplicating them in CDK API schema definitions. This applies to fields like clinicalPrivilegeActionCategory and other similar enumerations.
Applied to files:
backend/compact-connect/tests/resources/snapshots/PRIVILEGE_ENCUMBRANCE_REQUEST_SCHEMA.json
📚 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/tests/resources/snapshots/PRIVILEGE_ENCUMBRANCE_REQUEST_SCHEMA.json
📚 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/tests/resources/snapshots/PRIVILEGE_ENCUMBRANCE_REQUEST_SCHEMA.jsonbackend/compact-connect/tests/resources/snapshots/PATCH_PRIVILEGE_INVESTIGATION_REQUEST_SCHEMA.jsonbackend/compact-connect/tests/resources/snapshots/PATCH_LICENSE_INVESTIGATION_REQUEST_SCHEMA.json
📚 Learning: 2025-08-14T20:56:56.938Z
Learnt from: jusdino
Repo: csg-org/CompactConnect PR: 1004
File: backend/compact-connect/docs/internal/api-specification/latest-oas30.json:5698-5700
Timestamp: 2025-08-14T20:56:56.938Z
Learning: In the CompactConnect project, there is a deliberate design pattern for API schemas: request schemas should include enum validation constraints (like encumbranceType with its allowed values), while response schemas should remain unconstrained (just "type": "string") to allow flexibility in API responses.
Applied to files:
backend/compact-connect/tests/resources/snapshots/PRIVILEGE_ENCUMBRANCE_REQUEST_SCHEMA.jsonbackend/compact-connect/tests/resources/snapshots/PATCH_PRIVILEGE_INVESTIGATION_REQUEST_SCHEMA.json
📚 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/tests/resources/snapshots/PRIVILEGE_ENCUMBRANCE_REQUEST_SCHEMA.json
📚 Learning: 2025-08-22T21:20:35.260Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1029
File: backend/compact-connect/docs/api-specification/latest-oas30.json:468-471
Timestamp: 2025-08-22T21:20:35.260Z
Learning: The file backend/compact-connect/docs/api-specification/latest-oas30.json is auto-generated by API Gateway and should not be modified inline. Any schema changes would need to be addressed at the source in the CDK/CloudFormation definitions.
Applied to files:
backend/compact-connect/tests/resources/snapshots/PRIVILEGE_ENCUMBRANCE_REQUEST_SCHEMA.jsonbackend/compact-connect/tests/resources/snapshots/PATCH_LICENSE_INVESTIGATION_REQUEST_SCHEMA.json
📚 Learning: 2025-11-06T18:11:58.272Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 1196
File: webroot/src/network/mocks/mock.data.ts:1919-1944
Timestamp: 2025-11-06T18:11:58.272Z
Learning: In CompactConnect, the investigation event type (`updateType: 'investigation'`) in privilege/license history is intentionally not yet fully supported in the frontend UI translation and categorization logic (webroot/src/locales/*.json and webroot/src/models/LicenseHistoryItem/LicenseHistoryItem.model.ts). The server license history API needs to be updated first to return sufficient information about investigation events. Until then, the UI gracefully falls back to displaying "Unknown" for investigation events, which is acceptable as an interim solution.
<!--
Applied to files:
backend/compact-connect/tests/resources/snapshots/PATCH_LICENSE_INVESTIGATION_REQUEST_SCHEMA.json
⏰ 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). (3)
- GitHub Check: TestApp
- GitHub Check: TestAndLintPurchases
- GitHub Check: CheckWebroot
🔇 Additional comments (2)
backend/compact-connect/tests/resources/snapshots/PRIVILEGE_ENCUMBRANCE_REQUEST_SCHEMA.json (1)
40-44: LGTM! Consistent migration across schemas.The required field addition aligns with the other investigation request schemas, completing the migration from the deprecated singular
clinicalPrivilegeActionCategoryfield.backend/compact-connect/tests/resources/snapshots/PATCH_LICENSE_INVESTIGATION_REQUEST_SCHEMA.json (1)
48-52: Reconsider makingclinicalPrivilegeActionCategoriesrequired while deprecated singular field remains.The deprecated singular field
clinicalPrivilegeActionCategorystill exists in multiple places (manage_opensearch_indices.py, latest-oas30.json). Making the plural field required at this stage contradicts the backwards compatibility migration pattern documented in the codebase, where both fields are intentionally kept optional during the transition. This change should wait until the deprecated field is fully removed via the planned migration scripts.The minItems suggestion is valid: the Python schema enforces
Length(min=1), so adding"minItems": 1to the array definition for consistency is recommended.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In
@backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py:
- Around line 1518-1521: The encumbrance_details dict in encumber_privilege() is
missing the licenseJurisdiction field; update the encumbrance_details
construction in the encumber_privilege() method to include
'licenseJurisdiction': adverse_action.jurisdiction (same shape as
encumber_home_jurisdiction_license_privileges()), so it matches
EncumbranceDetailsSchema and existing tests that expect
clinicalPrivilegeActionCategories, licenseJurisdiction, and adverseActionId.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.pybackend/compact-connect/lambdas/python/common/tests/function/test_data_client.py
💤 Files with no reviewable changes (1)
- backend/compact-connect/lambdas/python/common/tests/function/test_data_client.py
🧰 Additional context used
🧠 Learnings (12)
📓 Common learnings
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.
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'].
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).
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: 769
File: backend/compact-connect/stacks/api_stack/v1_api/api_model.py:397-401
Timestamp: 2025-04-29T01:59:51.222Z
Learning: In the CompactConnect project, validation constraints like enum values should be defined only in runtime code (Lambda) rather than duplicating them in CDK API schema definitions. This applies to fields like clinicalPrivilegeActionCategory and other similar enumerations.
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.
📚 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-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-10-28T15:05:49.315Z
Learnt from: jusdino
Repo: csg-org/CompactConnect PR: 1167
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py:1698-1701
Timestamp: 2025-10-28T15:05:49.315Z
Learning: In the CompactConnect Python codebase, enums such as InvestigationStatusEnum, InvestigationAgainstEnum, ActiveInactiveStatus, LicenseEncumberedStatusEnum, and PrivilegeEncumberedStatusEnum derive from StrEnum. When writing DynamoDB AttributeValues (e.g., {'S': ...}) in files like backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py, pass the enum member directly (without .value). Future reviews should not recommend using .value for these enums.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-09-03T22:27:56.621Z
Learnt from: ChiefStief
Repo: csg-org/CompactConnect PR: 1036
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/provider/api.py:71-71
Timestamp: 2025-09-03T22:27:56.621Z
Learning: The privilegeJurisdictions field in provider schemas consistently uses load_default=set() pattern across multiple schema files in the CompactConnect codebase (provider/record.py and provider/api.py), and developers prioritize maintaining this consistency.
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-26T15:30:29.956Z
Learnt from: ChiefStief
Repo: csg-org/CompactConnect PR: 1012
File: backend/compact-connect/docs/design/README.md:503-512
Timestamp: 2025-08-26T15:30:29.956Z
Learning: In CompactConnect, the use of UTC-4:00 for privilege timeline events (encumbrances and expirations) was an agreed-upon business decision, not a technical choice made by individual developers. This fixed offset approach should be respected as an intentional organizational requirement.
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.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-12-16T21:43:07.408Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1219
File: backend/compact-connect/lambdas/python/search/handlers/search.py:131-140
Timestamp: 2025-12-16T21:43:07.408Z
Learning: In backend/compact-connect/lambdas/python/search/handlers/search.py, avoid logging the full request body. Do not log sensitive content by default. If logging is required for security investigations, redact or mask sensitive fields (e.g., credentials, tokens, PII) and log only safe metadata (method, path, status, user identifier). Use a secure, access-controlled audit log or feature flag to enable such logs, ensuring minimal exposure and compliance with security policies. This guideline targets Python backend handlers handling external requests and should be considered for similar files with request processing.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
🧬 Code graph analysis (1)
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py (2)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/__init__.py (8)
compact(26-27)compact(30-31)jurisdiction(42-43)jurisdiction(46-47)clinicalPrivilegeActionCategories(82-83)clinicalPrivilegeActionCategories(86-87)adverseActionId(114-115)adverseActionId(118-119)backend/compact-connect/lambdas/python/common/cc_common/data_model/update_tier_enum.py (1)
UpdateTierEnum(4-40)
⏰ 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). (3)
- GitHub Check: TestApp
- GitHub Check: LintNode
- GitHub Check: CheckWebroot
🔇 Additional comments (2)
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py (2)
3052-3056: Clean removal of feature flag and deprecated field.The
encumbrance_detailsdictionary now unconditionally uses the standardizedclinicalPrivilegeActionCategorieslist field along withlicenseJurisdictionandadverseActionId. This correctly removes the deprecated conditional logic based on theENCUMBRANCE_MULTI_CATEGORY_FLAG.The change is consistent with the same migration applied in the
encumber_privilegemethod and maintains proper tracking of encumbrance details for home jurisdiction license privileges.
1032-1053: Tier-based SK pattern is correctly implemented.The method properly constructs the
TIER_ONEprefix query and handles pagination correctly usingLastEvaluatedKey. This simplification improves code maintainability.This change removes queries for any legacy SK patterns. If privilege update records using old SK formats still exist in the database, they will not be returned by this query. Ensure the migration to tier-based SK format is complete before this code reaches production.
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
Show resolved
Hide resolved
c3342ad to
fa1e471
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
webroot/src/components/PrivilegeCard/PrivilegeCard.ts (1)
1003-1013: Dev-onlymockPopulatenow conflicts with updated date validation pattern
encumberModalStartDateis populated with aYYYY-MM-DDstring:this.formData.encumberModalStartDate.value = moment().format('YYYY-MM-DD');but the Joi validation for this field now uses
dateFormatPatterns.MM_DD_YYYY, which expects anMM/DD/YYYY-style value. In development, callingmockPopulate()will therefore leave the start date failing validation.Based on learnings about InputDate’s expected
MM/dd/yyyyUX format, it would be safer to align the mock value with the same pattern (for example, using anMM/DD/YYYY-formatted string) so the dev helper continues to yield a valid, submittable form.
🧹 Nitpick comments (3)
webroot/src/network/licenseApi/data.api.ts (1)
267-284: Consider removing the unusednpdbCategoryparameter.The
encumberLicensemethod now exclusively usesnpdbCategories(line 279). ThenpdbCategoryparameter is no longer referenced in the implementation and could be removed for clarity.🔎 Proposed cleanup
public async encumberLicense( compact: string, licenseeId: string, licenseState: string, licenseType: string, encumbranceType: string, - npdbCategory: string, npdbCategories: Array<string>, startDate: string ) {Note: This same cleanup applies to
encumberPrivilege(lines 409-426),updateLicenseInvestigation(lines 344-372), andupdatePrivilegeInvestigation(lines 486-514).backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_encumbrance.py (2)
43-51: Tests correctly exercise newclinicalPrivilegeActionCategoriesfor privilege encumbrance
_generate_test_body()now postsclinicalPrivilegeActionCategoriesas a one-element list of the NPDB enum.value, matching how the backend stores categories.test_privilege_encumbrance_handler_adds_adverse_action_record_in_provider_data_tableand
test_privilege_encumbrance_handler_adds_privilege_update_record_in_provider_data_tableverify that:
- the
AdverseActionrow is created as expected, and- the privilege update record’s
encumbranceDetailsincludesclinicalPrivilegeActionCategorieswith the expected label.This gives good end-to-end coverage of the new list field.
For symmetry, you might consider adding a small assertion in the license encumbrance tests to confirm that license-side encumbrance details (or adverse actions, if modeled there) also carry the expected
clinicalPrivilegeActionCategorieswhere applicable.Also applies to: 101-135, 137-172
304-383: License encumbrance tests remain consistent; consider mirroring privilege NPDB assertionsThe license POST encumbrance tests correctly validate that:
- an
AdverseActionrecord is created with the expected core fields, and- the corresponding license update record is present and shaped as expected.
They do not currently assert on NPDB categories, which is fine functionally but slightly less strict than the privilege tests.
If license encumbrance flows are also expected to persist
clinicalPrivilegeActionCategories, you could add a focused assertion here (similar to the privilege test) to catch any future regressions in that path.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (36)
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/__init__.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/api.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/record.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/record.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/schema/provider/record.pybackend/compact-connect/lambdas/python/common/cc_common/feature_flag_enum.pybackend/compact-connect/lambdas/python/common/tests/function/test_data_client.pybackend/compact-connect/lambdas/python/common/tests/resources/api/adverse-action-post.jsonbackend/compact-connect/lambdas/python/common/tests/unit/test_data_model/test_schema/test_investigation.pybackend/compact-connect/lambdas/python/common/tests/unit/test_provider_record_util.pybackend/compact-connect/lambdas/python/data-events/tests/function/test_encumbrance_events.pybackend/compact-connect/lambdas/python/provider-data-v1/handlers/encumbrance.pybackend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_encumbrance.pybackend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_investigation.pybackend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_privilege_history.pybackend/compact-connect/stacks/api_stack/v1_api/api_model.pybackend/compact-connect/stacks/feature_flag_stack/__init__.pybackend/compact-connect/tests/resources/snapshots/GET_PROVIDER_RESPONSE_SCHEMA.jsonbackend/compact-connect/tests/resources/snapshots/LICENSE_ENCUMBRANCE_REQUEST_SCHEMA.jsonbackend/compact-connect/tests/resources/snapshots/PATCH_LICENSE_INVESTIGATION_REQUEST_SCHEMA.jsonbackend/compact-connect/tests/resources/snapshots/PATCH_PRIVILEGE_INVESTIGATION_REQUEST_SCHEMA.jsonbackend/compact-connect/tests/resources/snapshots/PRIVILEGE_ENCUMBRANCE_REQUEST_SCHEMA.jsonbackend/compact-connect/tests/resources/snapshots/PROVIDER_USER_RESPONSE_SCHEMA.jsonbackend/compact-connect/tests/smoke/investigation_smoke_tests.pywebroot/src/app.config.tswebroot/src/components/LicenseCard/LicenseCard.tswebroot/src/components/LicenseCard/LicenseCard.vuewebroot/src/components/PrivilegeCard/PrivilegeCard.tswebroot/src/components/PrivilegeCard/PrivilegeCard.vuewebroot/src/models/AdverseAction/AdverseAction.model.spec.tswebroot/src/models/AdverseAction/AdverseAction.model.tswebroot/src/network/licenseApi/data.api.tswebroot/src/network/mocks/mock.data.api.tswebroot/src/network/mocks/mock.data.ts
💤 Files with no reviewable changes (11)
- webroot/src/app.config.ts
- backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/provider/record.py
- webroot/src/models/AdverseAction/AdverseAction.model.spec.ts
- backend/compact-connect/lambdas/python/common/tests/function/test_data_client.py
- backend/compact-connect/stacks/feature_flag_stack/init.py
- backend/compact-connect/lambdas/python/common/cc_common/feature_flag_enum.py
- backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/init.py
- backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/record.py
- backend/compact-connect/tests/resources/snapshots/GET_PROVIDER_RESPONSE_SCHEMA.json
- webroot/src/models/AdverseAction/AdverseAction.model.ts
- backend/compact-connect/tests/resources/snapshots/PROVIDER_USER_RESPONSE_SCHEMA.json
🚧 Files skipped from review as they are similar to previous changes (12)
- webroot/src/components/PrivilegeCard/PrivilegeCard.vue
- webroot/src/network/mocks/mock.data.ts
- webroot/src/components/LicenseCard/LicenseCard.vue
- backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_privilege_history.py
- backend/compact-connect/lambdas/python/data-events/tests/function/test_encumbrance_events.py
- backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_investigation.py
- backend/compact-connect/lambdas/python/common/tests/unit/test_provider_record_util.py
- webroot/src/network/mocks/mock.data.api.ts
- backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py
- backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/api.py
- backend/compact-connect/lambdas/python/common/tests/resources/api/adverse-action-post.json
- backend/compact-connect/tests/resources/snapshots/LICENSE_ENCUMBRANCE_REQUEST_SCHEMA.json
🧰 Additional context used
🧠 Learnings (35)
📓 Common learnings
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.
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'].
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 769
File: backend/compact-connect/stacks/api_stack/v1_api/api_model.py:397-401
Timestamp: 2025-04-29T01:59:51.222Z
Learning: In the CompactConnect project, validation constraints like enum values should be defined only in runtime code (Lambda) rather than duplicating them in CDK API schema definitions. This applies to fields like clinicalPrivilegeActionCategory and other similar enumerations.
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).
📚 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/provider-data-v1/tests/function/test_handlers/test_encumbrance.pybackend/compact-connect/tests/resources/snapshots/PATCH_PRIVILEGE_INVESTIGATION_REQUEST_SCHEMA.jsonbackend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.pybackend/compact-connect/tests/resources/snapshots/PRIVILEGE_ENCUMBRANCE_REQUEST_SCHEMA.jsonbackend/compact-connect/tests/smoke/investigation_smoke_tests.pybackend/compact-connect/lambdas/python/common/tests/unit/test_data_model/test_schema/test_investigation.pybackend/compact-connect/stacks/api_stack/v1_api/api_model.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/record.pybackend/compact-connect/tests/resources/snapshots/PATCH_LICENSE_INVESTIGATION_REQUEST_SCHEMA.jsonbackend/compact-connect/lambdas/python/provider-data-v1/handlers/encumbrance.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/provider-data-v1/tests/function/test_handlers/test_encumbrance.pybackend/compact-connect/tests/resources/snapshots/PATCH_PRIVILEGE_INVESTIGATION_REQUEST_SCHEMA.jsonbackend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.pybackend/compact-connect/tests/resources/snapshots/PRIVILEGE_ENCUMBRANCE_REQUEST_SCHEMA.jsonbackend/compact-connect/tests/smoke/investigation_smoke_tests.pybackend/compact-connect/lambdas/python/common/tests/unit/test_data_model/test_schema/test_investigation.pybackend/compact-connect/stacks/api_stack/v1_api/api_model.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/record.pybackend/compact-connect/tests/resources/snapshots/PATCH_LICENSE_INVESTIGATION_REQUEST_SCHEMA.jsonbackend/compact-connect/lambdas/python/provider-data-v1/handlers/encumbrance.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/provider-data-v1/tests/function/test_handlers/test_encumbrance.pybackend/compact-connect/tests/resources/snapshots/PATCH_PRIVILEGE_INVESTIGATION_REQUEST_SCHEMA.jsonbackend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.pybackend/compact-connect/tests/resources/snapshots/PRIVILEGE_ENCUMBRANCE_REQUEST_SCHEMA.jsonbackend/compact-connect/tests/smoke/investigation_smoke_tests.pybackend/compact-connect/stacks/api_stack/v1_api/api_model.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/record.pybackend/compact-connect/tests/resources/snapshots/PATCH_LICENSE_INVESTIGATION_REQUEST_SCHEMA.jsonbackend/compact-connect/lambdas/python/provider-data-v1/handlers/encumbrance.pywebroot/src/components/PrivilegeCard/PrivilegeCard.ts
📚 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/provider-data-v1/tests/function/test_handlers/test_encumbrance.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.pybackend/compact-connect/tests/resources/snapshots/PRIVILEGE_ENCUMBRANCE_REQUEST_SCHEMA.jsonbackend/compact-connect/lambdas/python/common/tests/unit/test_data_model/test_schema/test_investigation.pybackend/compact-connect/stacks/api_stack/v1_api/api_model.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/record.pybackend/compact-connect/tests/resources/snapshots/PATCH_LICENSE_INVESTIGATION_REQUEST_SCHEMA.jsonbackend/compact-connect/lambdas/python/provider-data-v1/handlers/encumbrance.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/provider-data-v1/tests/function/test_handlers/test_encumbrance.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/record.pybackend/compact-connect/lambdas/python/provider-data-v1/handlers/encumbrance.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/provider-data-v1/tests/function/test_handlers/test_encumbrance.pybackend/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/provider-data-v1/tests/function/test_handlers/test_encumbrance.pybackend/compact-connect/tests/resources/snapshots/PATCH_PRIVILEGE_INVESTIGATION_REQUEST_SCHEMA.jsonbackend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.pybackend/compact-connect/tests/resources/snapshots/PRIVILEGE_ENCUMBRANCE_REQUEST_SCHEMA.jsonbackend/compact-connect/stacks/api_stack/v1_api/api_model.pybackend/compact-connect/tests/resources/snapshots/PATCH_LICENSE_INVESTIGATION_REQUEST_SCHEMA.jsonbackend/compact-connect/lambdas/python/provider-data-v1/handlers/encumbrance.py
📚 Learning: 2025-04-29T21:14:36.205Z
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:38-40
Timestamp: 2025-04-29T21:14:36.205Z
Learning: In the CompactConnect codebase, `cc_common.config._Config.current_standard_datetime` is implemented as a property that returns `datetime.now(tz=UTC).replace(microsecond=0)`. The correct way to patch it in tests is by directly providing the datetime value, not wrapping it in a lambda: `patch('cc_common.config._Config.current_standard_datetime', datetime.fromisoformat(DEFAULT_DATE_OF_UPDATE_TIMESTAMP))`.
Applied to files:
backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_encumbrance.py
📚 Learning: 2025-04-29T21:14:36.205Z
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:38-40
Timestamp: 2025-04-29T21:14:36.205Z
Learning: In the CompactConnect codebase, `cc_common.config._Config.current_standard_datetime` is a property (not a method), and can be properly patched in tests by directly supplying a datetime value rather than wrapping it in a lambda: `patch('cc_common.config._Config.current_standard_datetime', datetime.fromisoformat(DEFAULT_DATE_OF_UPDATE_TIMESTAMP))`.
Applied to files:
backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_encumbrance.py
📚 Learning: 2025-04-29T21:14:36.205Z
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:38-40
Timestamp: 2025-04-29T21:14:36.205Z
Learning: In the CompactConnect codebase, `cc_common.config._Config.current_standard_datetime` is a property rather than a method, and should be patched in tests by directly providing a datetime value: `patch('cc_common.config._Config.current_standard_datetime', datetime.fromisoformat(DEFAULT_DATE_OF_UPDATE_TIMESTAMP))`.
Applied to files:
backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_encumbrance.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/provider-data-v1/tests/function/test_handlers/test_encumbrance.pybackend/compact-connect/tests/resources/snapshots/PATCH_PRIVILEGE_INVESTIGATION_REQUEST_SCHEMA.jsonbackend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.pybackend/compact-connect/tests/resources/snapshots/PRIVILEGE_ENCUMBRANCE_REQUEST_SCHEMA.jsonbackend/compact-connect/stacks/api_stack/v1_api/api_model.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/record.pybackend/compact-connect/tests/resources/snapshots/PATCH_LICENSE_INVESTIGATION_REQUEST_SCHEMA.jsonbackend/compact-connect/lambdas/python/provider-data-v1/handlers/encumbrance.py
📚 Learning: 2025-12-16T21:43:07.408Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1219
File: backend/compact-connect/lambdas/python/search/handlers/search.py:131-140
Timestamp: 2025-12-16T21:43:07.408Z
Learning: In backend/compact-connect/lambdas/python/search/handlers/search.py, avoid logging the full request body. Do not log sensitive content by default. If logging is required for security investigations, redact or mask sensitive fields (e.g., credentials, tokens, PII) and log only safe metadata (method, path, status, user identifier). Use a secure, access-controlled audit log or feature flag to enable such logs, ensuring minimal exposure and compliance with security policies. This guideline targets Python backend handlers handling external requests and should be considered for similar files with request processing.
Applied to files:
backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_encumbrance.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.pybackend/compact-connect/tests/smoke/investigation_smoke_tests.pybackend/compact-connect/lambdas/python/common/tests/unit/test_data_model/test_schema/test_investigation.pybackend/compact-connect/stacks/api_stack/v1_api/api_model.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/record.pybackend/compact-connect/lambdas/python/provider-data-v1/handlers/encumbrance.py
📚 Learning: 2025-08-14T20:56:56.938Z
Learnt from: jusdino
Repo: csg-org/CompactConnect PR: 1004
File: backend/compact-connect/docs/internal/api-specification/latest-oas30.json:5698-5700
Timestamp: 2025-08-14T20:56:56.938Z
Learning: In the CompactConnect project, there is a deliberate design pattern for API schemas: request schemas should include enum validation constraints (like encumbranceType with its allowed values), while response schemas should remain unconstrained (just "type": "string") to allow flexibility in API responses.
Applied to files:
backend/compact-connect/tests/resources/snapshots/PATCH_PRIVILEGE_INVESTIGATION_REQUEST_SCHEMA.jsonbackend/compact-connect/tests/resources/snapshots/PRIVILEGE_ENCUMBRANCE_REQUEST_SCHEMA.jsonbackend/compact-connect/tests/resources/snapshots/PATCH_LICENSE_INVESTIGATION_REQUEST_SCHEMA.json
📚 Learning: 2025-04-29T01:59:51.222Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 769
File: backend/compact-connect/stacks/api_stack/v1_api/api_model.py:397-401
Timestamp: 2025-04-29T01:59:51.222Z
Learning: In the CompactConnect project, validation constraints like enum values should be defined only in runtime code (Lambda) rather than duplicating them in CDK API schema definitions. This applies to fields like clinicalPrivilegeActionCategory and other similar enumerations.
Applied to files:
backend/compact-connect/tests/resources/snapshots/PATCH_PRIVILEGE_INVESTIGATION_REQUEST_SCHEMA.jsonbackend/compact-connect/tests/resources/snapshots/PRIVILEGE_ENCUMBRANCE_REQUEST_SCHEMA.jsonbackend/compact-connect/stacks/api_stack/v1_api/api_model.py
📚 Learning: 2025-11-06T18:11:58.272Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 1196
File: webroot/src/network/mocks/mock.data.ts:1919-1944
Timestamp: 2025-11-06T18:11:58.272Z
Learning: In CompactConnect, the investigation event type (`updateType: 'investigation'`) in privilege/license history is intentionally not yet fully supported in the frontend UI translation and categorization logic (webroot/src/locales/*.json and webroot/src/models/LicenseHistoryItem/LicenseHistoryItem.model.ts). The server license history API needs to be updated first to return sufficient information about investigation events. Until then, the UI gracefully falls back to displaying "Unknown" for investigation events, which is acceptable as an interim solution.
<!--
Applied to files:
webroot/src/network/licenseApi/data.api.tsbackend/compact-connect/tests/smoke/investigation_smoke_tests.pybackend/compact-connect/tests/resources/snapshots/PATCH_LICENSE_INVESTIGATION_REQUEST_SCHEMA.json
📚 Learning: 2025-11-12T21:06:06.981Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 1196
File: webroot/src/components/LicenseCard/LicenseCard.ts:388-396
Timestamp: 2025-11-12T21:06:06.981Z
Learning: In the CompactConnect investigation creation flow, the backend APIs for creating investigations (both `createLicenseInvestigation` and `createPrivilegeInvestigation` in webroot/src/network/licenseApi/data.api.ts) do not accept a `startDate` parameter. The backend automatically sets the investigation creation date to the timestamp when the API request is received, so the frontend UI does not need to capture or submit an investigation start date during the creation workflow.
Applied to files:
webroot/src/network/licenseApi/data.api.ts
📚 Learning: 2025-11-06T18:10:16.437Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 1196
File: webroot/src/network/mocks/mock.data.ts:772-793
Timestamp: 2025-11-06T18:10:16.437Z
Learning: In CompactConnect, licenses and privileges can have investigations from different jurisdictions. For example, a license in Nevada (NV) can legitimately have investigations with jurisdiction Wyoming (WY). This cross-jurisdiction investigation pattern is valid in the data model and represents real-world scenarios where a practitioner licensed in one state may be under investigation by another state's authorities.
Applied to files:
webroot/src/network/licenseApi/data.api.ts
📚 Learning: 2025-12-19T16:04:23.341Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 1234
File: webroot/src/network/searchApi/data.api.ts:83-95
Timestamp: 2025-12-19T16:04:23.341Z
Learning: In the CompactConnect codebase, all network API files under webroot/src/network/*/data.api.ts use a specific axios.create header configuration pattern with nested get/post/put objects inside the headers config. Reviewers should not flag this pattern as an issue, since it aligns with the project’s Axios version and conventions. If enforcing general axios patterns, treat this as a project-specific exception and focus on consistency within this path.
Applied to files:
webroot/src/network/licenseApi/data.api.ts
📚 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.pybackend/compact-connect/tests/resources/snapshots/PRIVILEGE_ENCUMBRANCE_REQUEST_SCHEMA.jsonbackend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/record.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.py
📚 Learning: 2025-08-13T20:28:33.191Z
Learnt from: rmolinares
Repo: csg-org/CompactConnect PR: 1011
File: webroot/src/models/Licensee/Licensee.model.ts:276-279
Timestamp: 2025-08-13T20:28:33.191Z
Learning: The two-year encumbrance wait period feature only applies to privileges, not licenses. The logic should only check if privilege encumbrances that are no longer active have lift dates within the past two years.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-11-13T22:13:48.430Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1188
File: backend/compact-connect/tests/smoke/rollback_license_upload_smoke_tests.py:354-372
Timestamp: 2025-11-13T22:13:48.430Z
Learning: In the CompactConnect codebase, privilege records use datetime fields (not date fields) for dateOfIssuance, dateOfRenewal, and dateOfExpiration, which is intentionally different from license records that use date fields for these same attributes.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-09-03T22:27:56.621Z
Learnt from: ChiefStief
Repo: csg-org/CompactConnect PR: 1036
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/provider/api.py:71-71
Timestamp: 2025-09-03T22:27:56.621Z
Learning: The privilegeJurisdictions field in provider schemas consistently uses load_default=set() pattern across multiple schema files in the CompactConnect codebase (provider/record.py and provider/api.py), and developers prioritize maintaining this consistency.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-08-26T15:30:29.956Z
Learnt from: ChiefStief
Repo: csg-org/CompactConnect PR: 1012
File: backend/compact-connect/docs/design/README.md:503-512
Timestamp: 2025-08-26T15:30:29.956Z
Learning: In CompactConnect, the use of UTC-4:00 for privilege timeline events (encumbrances and expirations) was an agreed-upon business decision, not a technical choice made by individual developers. This fixed offset approach should be respected as an intentional organizational requirement.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-06-23T20:19:18.952Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 877
File: backend/compact-connect/tests/smoke/practitioner_email_update_smoke_tests.py:22-192
Timestamp: 2025-06-23T20:19:18.952Z
Learning: In the CompactConnect project, smoke test functions (like test_practitioner_email_update in backend/compact-connect/tests/smoke/practitioner_email_update_smoke_tests.py) should remain as single large functions rather than being broken down into smaller helper functions, based on the project maintainer's preference and reasoning.
Applied to files:
backend/compact-connect/tests/smoke/investigation_smoke_tests.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/tests/smoke/investigation_smoke_tests.py
📚 Learning: 2025-10-24T21:40:48.912Z
Learnt from: jusdino
Repo: csg-org/CompactConnect PR: 1167
File: backend/compact-connect/lambdas/python/provider-data-v1/handlers/investigation.py:59-66
Timestamp: 2025-10-24T21:40:48.912Z
Learning: For the investigation endpoints in backend/compact-connect/lambdas/python/provider-data-v1/handlers/investigation.py, API Gateway validators are configured to guarantee that the request body exists and is non-empty, so defensive checks like event.get('body') or '{}' are not needed in _load_investigation_patch_body.
Applied to files:
backend/compact-connect/lambdas/python/common/tests/unit/test_data_model/test_schema/test_investigation.py
📚 Learning: 2025-04-29T21:09:04.842Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 769
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/common.py:87-90
Timestamp: 2025-04-29T21:09:04.842Z
Learning: In the CCDataClass implementation, validation is performed during both the dump() and load() processes in Marshmallow, so calling dump() first still results in ValidationError if the provided data is invalid.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/record.py
📚 Learning: 2025-06-24T00:02:39.944Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 873
File: webroot/src/components/LicenseCard/LicenseCard.ts:414-443
Timestamp: 2025-06-24T00:02:39.944Z
Learning: In LicenseCard component's clickUnencumberItem method (webroot/src/components/LicenseCard/LicenseCard.ts), complex event handling for checkbox interactions is intentionally designed to ensure consistent behavior across checkbox input, wrapper label, and outer selection parent elements for custom UI requirements. This complexity should be preserved rather than simplified.
Applied to files:
webroot/src/components/LicenseCard/LicenseCard.tswebroot/src/components/PrivilegeCard/PrivilegeCard.ts
📚 Learning: 2025-06-24T00:07:10.463Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 873
File: webroot/src/components/LicenseCard/LicenseCard.ts:509-528
Timestamp: 2025-06-24T00:07:10.463Z
Learning: In the CompactConnect frontend codebase, the project prefers to avoid early returns in frontend code when possible, as mentioned by jsandoval81 in webroot/src/components/LicenseCard/LicenseCard.ts.
Applied to files:
webroot/src/components/LicenseCard/LicenseCard.tswebroot/src/components/PrivilegeCard/PrivilegeCard.ts
📚 Learning: 2025-06-24T00:17:31.188Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 873
File: webroot/src/styles.common/_inputs.less:115-118
Timestamp: 2025-06-24T00:17:31.188Z
Learning: The team intentionally uses broad CSS selectors like `.dp__input_wrap div { position: static; }` to fix styling issues with the Vue Date-Picker library. They have experience with these styles working well, keep the library version pinned in yarn.lock, and have established processes to re-test everything when updating the library version. This approach is acceptable given their testing discipline and version control.
Applied to files:
webroot/src/components/LicenseCard/LicenseCard.ts
📚 Learning: 2025-06-04T22:04:14.373Z
Learnt from: rmolinares
Repo: csg-org/CompactConnect PR: 843
File: webroot/src/components/Forms/InputDate/InputDate.ts:0-0
Timestamp: 2025-06-04T22:04:14.373Z
Learning: In the InputDate component (webroot/src/components/Forms/InputDate/InputDate.ts), immediate validation on every keystroke is intentional design behavior. The team prefers to alert users to encourage expected date format completion rather than deferring validation until the date is complete. This provides immediate feedback to guide users toward proper MM/dd/yyyy format completion.
Applied to files:
webroot/src/components/LicenseCard/LicenseCard.tswebroot/src/components/PrivilegeCard/PrivilegeCard.ts
📚 Learning: 2025-08-21T16:36:48.723Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 1019
File: webroot/src/components/PrivilegeCard/PrivilegeCard.vue:270-278
Timestamp: 2025-08-21T16:36:48.723Z
Learning: In PrivilegeCard component's unencumber modal (webroot/src/components/PrivilegeCard/PrivilegeCard.vue), the unencumber-select wrapper element intentionally uses space key handling to catch bubbled events from child InputCheckbox elements for custom handling actions. When adverseAction.hasEndDate() is true, items show an inactive-category div and are designed to be non-interactive (not focusable), while items without end dates contain focusable InputCheckbox child elements. This design pattern is consistent with prior implementation and represents intentional UX behavior.
Applied to files:
webroot/src/components/LicenseCard/LicenseCard.tswebroot/src/components/PrivilegeCard/PrivilegeCard.ts
📚 Learning: 2025-07-03T15:35:57.893Z
Learnt from: rmolinares
Repo: csg-org/CompactConnect PR: 905
File: webroot/src/components/UpdateHomeJurisdiction/UpdateHomeJurisdiction.vue:32-41
Timestamp: 2025-07-03T15:35:57.893Z
Learning: In the CompactConnect frontend codebase, the team prefers to keep non-dynamic text directly in Vue templates rather than moving it to computed properties in TypeScript modules, as this approach prevents cluttering the TS files with template labels.
Applied to files:
webroot/src/components/PrivilegeCard/PrivilegeCard.ts
🧬 Code graph analysis (4)
backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_encumbrance.py (1)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/common.py (1)
ClinicalPrivilegeActionCategory(401-414)
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py (1)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/__init__.py (8)
compact(26-27)compact(30-31)jurisdiction(42-43)jurisdiction(46-47)clinicalPrivilegeActionCategories(82-83)clinicalPrivilegeActionCategories(86-87)adverseActionId(114-115)adverseActionId(118-119)
backend/compact-connect/lambdas/python/provider-data-v1/handlers/encumbrance.py (1)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/__init__.py (2)
clinicalPrivilegeActionCategories(82-83)clinicalPrivilegeActionCategories(86-87)
webroot/src/components/PrivilegeCard/PrivilegeCard.ts (1)
webroot/src/models/FormInput/FormInput.model.ts (1)
FormInput(59-170)
⏰ 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). (2)
- GitHub Check: TestApp
- GitHub Check: CheckWebroot
🔇 Additional comments (20)
backend/compact-connect/tests/smoke/investigation_smoke_tests.py (1)
394-401: LGTM! Encumbrance payload correctly updated to use plural field.The test payload correctly uses
clinicalPrivilegeActionCategoriesas a list instead of the deprecated singularclinicalPrivilegeActionCategorystring field. This aligns with the PR objective to remove the deprecated field and ensures the smoke test validates the updated API contract.backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py (3)
1518-1521: Encumbrance details correctly use plural field.The
encumbrance_detailsdictionary correctly usesclinicalPrivilegeActionCategories(plural, array) instead of the deprecated singular field. The intentional exclusion oflicenseJurisdictionfor direct privilege encumbrance is appropriate.Note:
clinicalPrivilegeActionCategoriescan beNoneper the schema (list[str] | None). If adverse actions are expected to always have this field populated for encumbrances, the current code is fine. Otherwise, consider defensive handling ifNonevalues could cause issues downstream.Based on learnings: The distinction between direct privilege encumbrance (without
licenseJurisdiction) and license-driven encumbrance (withlicenseJurisdiction) is an intentional design decision that has been previously clarified.
3052-3058: Encumbrance details correctly use plural field with clear documentation.The
encumbrance_detailsdictionary correctly usesclinicalPrivilegeActionCategories(plural, array) and includeslicenseJurisdictionfor license-driven encumbrances. The added comment (lines 3054-3056) clearly documents whylicenseJurisdictionis included in this case but not in direct privilege encumbrances, which improves code maintainability.Note: Same as with direct privilege encumbrance,
clinicalPrivilegeActionCategoriescan beNoneper the schema. Consider whether defensive handling is appropriate for your use case.Based on learnings: The inclusion of
licenseJurisdictionspecifically for license-driven privilege encumbrances is an intentional design pattern that helps distinguish between different encumbrance paths in the data model.
1032-1053: The query simplification is safe and correctly reflects the completed migration to the tier-based sort key pattern. All privilege update records in the system are stored under the{compact}#UPDATE#1#privilege/prefix following the migration from old patterns, as evidenced by the migration infrastructure and consistent test data using the new pattern throughout the codebase.backend/compact-connect/lambdas/python/common/tests/unit/test_data_model/test_schema/test_investigation.py (1)
142-147: LGTM! Test correctly updated to use the plural array field.The test now validates
clinicalPrivilegeActionCategoriesas an array, aligning with the schema migration from the deprecated singular field.webroot/src/network/licenseApi/data.api.ts (1)
8-8: LGTM! Feature gate removed.The removal of
FeatureGatesfrom the imports aligns with the PR objective to eliminate theENCUMBRANCE_MULTI_CATEGORY_FLAGand consistently use the plural array field across all encumbrance operations.backend/compact-connect/tests/resources/snapshots/PATCH_PRIVILEGE_INVESTIGATION_REQUEST_SCHEMA.json (1)
40-52: LGTM! Schema correctly migrated to plural field.The snapshot now requires
clinicalPrivilegeActionCategoriesas an array, replacing the deprecated singular field. This aligns with the API schema evolution and ensures clients must provide categories as an array.backend/compact-connect/tests/resources/snapshots/PRIVILEGE_ENCUMBRANCE_REQUEST_SCHEMA.json (1)
32-44: LGTM! Encumbrance schema aligned with investigation schema.The required
clinicalPrivilegeActionCategoriesarray field is now consistently defined across both privilege encumbrance and investigation request schemas.backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/record.py (3)
2-2: LGTM! Cleaned up unused import.The
pre_loadimport is no longer needed after removing the migration hooks, correctly reducing unused imports.
47-50: LGTM! Migration logic correctly removed.The
pre_dump_serializationmethod is now simplified to only handle PK/SK generation, as the migration fromclinicalPrivilegeActionCategorytoclinicalPrivilegeActionCategoriesis complete.
37-37: The verification confirms that the migration is complete and the deprecated singular field has been removed from Python schemas. However, do not markclinicalPrivilegeActionCategoriesasrequired=Trueyet. The current optional status inAdverseActionRecordSchemais intentional for backwards compatibility—it allows loading existing records that may lack this field, while the API request schema already enforces the requirement at input time. Removing the optional status should only be done in a follow-up migration with data cleanup scripts, consistent with the codebase's migration strategy.backend/compact-connect/tests/resources/snapshots/PATCH_LICENSE_INVESTIGATION_REQUEST_SCHEMA.json (1)
40-52: Encumbrancerequiredlist now correctly aligned with API modelThe snapshot’s
encumbrance.requiredlist matches the CDK_encumbrance_request_schema(date, type, and categories all required whenencumbranceis present) while keepingencumbranceitself optional at the top level. This preserves the “close-only” flow and enforces completeness when an encumbrance is supplied.backend/compact-connect/stacks/api_stack/v1_api/api_model.py (1)
1118-1168: Encumbrance request and response schemas are internally consistent with new multi-category field
- Response side: both license and privilege
adverseActionsnow exposeclinicalPrivilegeActionCategoriesas an optionalstring[], which is safe for older records that may not yet carry this field.- Request side:
_encumbrance_request_schema.requirednow includesclinicalPrivilegeActionCategoriesalongsideencumbranceEffectiveDateandencumbranceType, aligning with the JSON-schema snapshots and backend request validation.This looks consistent across models and reinforces that all new encumbrances must include at least one NPDB category.
Please double-check that any non-UI API consumers (if they exist) have already been migrated to send
clinicalPrivilegeActionCategories, since the API model will now reject encumbrance payloads that omit it.Also applies to: 1261-1312, 1401-1421
backend/compact-connect/lambdas/python/provider-data-v1/handlers/encumbrance.py (1)
68-82: Directly mappingclinicalPrivilegeActionCategoriesfrom validated request body is appropriateThe handler now simply assigns:
adverse_action.clinicalPrivilegeActionCategories = adverse_action_post_body['clinicalPrivilegeActionCategories']after
AdverseActionPostRequestSchemahas validated and deserialized the body. This keeps category validation in the schema layer and removes the now-unneeded feature-flag/migration logic.Please ensure
AdverseActionPostRequestSchemaalways definesclinicalPrivilegeActionCategories(with the expected list-of-strings shape) for encumbrance requests, so this dictionary access cannot raise aKeyError.Also applies to: 97-106
webroot/src/components/PrivilegeCard/PrivilegeCard.ts (1)
19-20: Privilege encumbrance UI now consistently uses multi-select NPDB categories
npdbCategoryOptionsderives options fromlicensing.npdbTypes, andinitFormInputsEncumberPrivilegedefinesencumberModalNpdbCategoriesas a multi-selectFormInputvalidated viaJoi.array().min(1),- both encumbrance submission paths now always include
npdbCategoriesfromencumberModalNpdbCategories.value.This cleanly removes the old single-category path and keeps the UI, Vuex actions, and backend in sync around an array-based NPDB category contract, while reusing shared
dateFormatPatternsfor start-date validation.Also applies to: 231-236, 297-314, 530-573
webroot/src/components/LicenseCard/LicenseCard.ts (5)
19-19: LGTM: Feature gate dependency removed.The simplified import correctly removes the feature gate dependency as part of the migration to exclusively using array-based NPDB categories.
314-321: LGTM: Array-based form input properly configured.The form input correctly migrates to array-based NPDB categories with appropriate validation. The
Joi.array().min(1)validation maintains the required constraint while supporting multiple selections.
500-500: LGTM: Submission payloads consistently use array field.Both submission paths correctly use
npdbCategories(plural) and reference the array-based form input value. The migration from singular to plural field is complete across all submission scenarios.Also applies to: 515-515
955-955: LGTM: Mock populate correctly sets array value.The mock populate function properly wraps the selected value in an array, consistent with the array-based form input.
259-264: No changes needed—the removal of the default empty option is correct.The template properly uses
InputSelectMultiplefor the NPDB categories field (encumberModalNpdbCategories), which is the appropriate component for this multi-select scenario. Removing the default empty "select option" is the right approach for this component type.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
backend/compact-connect/tests/smoke/smoke_common.py (2)
481-485: Consider adding pagination for consistency.The privilege records query lacks pagination handling while the privilege update records query (lines 491-500) properly handles it. If a provider has many privilege records exceeding DynamoDB's 1MB response limit, some records could be missed.
For smoke tests this is unlikely to be an issue, but for consistency and defensive coding, consider using the same pagination pattern for both queries.
🔎 Proposed fix
- # Query for all privilege records in the specified jurisdiction - original_privilege_records = dynamodb_table.query( - KeyConditionExpression=Key('pk').eq(pk) - & Key('sk').begins_with(f'{compact}#PROVIDER#privilege/{jurisdiction}/') - ).get('Items', []) + # Query for all privilege records in the specified jurisdiction + privilege_sk_prefix = f'{compact}#PROVIDER#privilege/{jurisdiction}/' + original_privilege_records = [] + last_evaluated_key = None + while True: + pagination = {'ExclusiveStartKey': last_evaluated_key} if last_evaluated_key else {} + query_resp = dynamodb_table.query( + KeyConditionExpression=Key('pk').eq(pk) & Key('sk').begins_with(privilege_sk_prefix), + **pagination, + ) + original_privilege_records.extend(query_resp.get('Items', [])) + last_evaluated_key = query_resp.get('LastEvaluatedKey') + if not last_evaluated_key: + break
519-522: Log message could be more accurate.The message says "Deleted privilege record" (singular) but multiple privilege records may be deleted. Consider tracking both counts for clearer debugging output.
🔎 Proposed fix
+ privilege_count = len(original_privilege_records) - len(original_privilege_update_records) logger.info( - f'Deleted privilege record and {len(original_privilege_update_records)} privilege update records for ' + f'Deleted {privilege_count} privilege records and {len(original_privilege_update_records)} privilege update records for ' f'jurisdiction {jurisdiction}' )Note: This requires capturing the privilege count before extending, or adjusting the calculation as shown above.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
backend/compact-connect/tests/smoke/purchasing_privileges_smoke_tests.pybackend/compact-connect/tests/smoke/smoke_common.py
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
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.
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'].
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 769
File: backend/compact-connect/stacks/api_stack/v1_api/api_model.py:397-401
Timestamp: 2025-04-29T01:59:51.222Z
Learning: In the CompactConnect project, validation constraints like enum values should be defined only in runtime code (Lambda) rather than duplicating them in CDK API schema definitions. This applies to fields like clinicalPrivilegeActionCategory and other similar enumerations.
📚 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/tests/smoke/purchasing_privileges_smoke_tests.pybackend/compact-connect/tests/smoke/smoke_common.py
📚 Learning: 2025-08-22T22:02:41.865Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1029
File: backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_licenses.py:40-73
Timestamp: 2025-08-22T22:02:41.865Z
Learning: The test framework in backend/compact-connect/lambdas/python uses self.addCleanup(self.delete_resources) in the base TstFunction/TstLambdas classes to automatically clean up all test resources (DynamoDB tables, SQS queues, S3 buckets, Cognito user pools) after each test. The delete_resources() method provides comprehensive cleanup that prevents cross-test interference, eliminating the need for additional tearDown logic.
Applied to files:
backend/compact-connect/tests/smoke/purchasing_privileges_smoke_tests.pybackend/compact-connect/tests/smoke/smoke_common.py
📚 Learning: 2025-06-23T20:19:18.952Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 877
File: backend/compact-connect/tests/smoke/practitioner_email_update_smoke_tests.py:22-192
Timestamp: 2025-06-23T20:19:18.952Z
Learning: In the CompactConnect project, smoke test functions (like test_practitioner_email_update in backend/compact-connect/tests/smoke/practitioner_email_update_smoke_tests.py) should remain as single large functions rather than being broken down into smaller helper functions, based on the project maintainer's preference and reasoning.
Applied to files:
backend/compact-connect/tests/smoke/purchasing_privileges_smoke_tests.pybackend/compact-connect/tests/smoke/smoke_common.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/tests/smoke/purchasing_privileges_smoke_tests.py
📚 Learning: 2025-08-22T22:02:41.865Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1029
File: backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_licenses.py:40-73
Timestamp: 2025-08-22T22:02:41.865Z
Learning: The TstLambdas base class in backend/compact-connect/lambdas/python/*/tests/function/__init__.py uses self.addCleanup(self.delete_resources) to automatically clean up test resources (including SQS queues) after each test, preventing cross-test interference. No additional tearDown logic is needed for queue cleanup.
Applied to files:
backend/compact-connect/tests/smoke/purchasing_privileges_smoke_tests.py
📚 Learning: 2025-12-16T21:43:07.408Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1219
File: backend/compact-connect/lambdas/python/search/handlers/search.py:131-140
Timestamp: 2025-12-16T21:43:07.408Z
Learning: In backend/compact-connect/lambdas/python/search/handlers/search.py, avoid logging the full request body. Do not log sensitive content by default. If logging is required for security investigations, redact or mask sensitive fields (e.g., credentials, tokens, PII) and log only safe metadata (method, path, status, user identifier). Use a secure, access-controlled audit log or feature flag to enable such logs, ensuring minimal exposure and compliance with security policies. This guideline targets Python backend handlers handling external requests and should be considered for similar files with request processing.
Applied to files:
backend/compact-connect/tests/smoke/purchasing_privileges_smoke_tests.pybackend/compact-connect/tests/smoke/smoke_common.py
📚 Learning: 2025-09-03T22:27:56.621Z
Learnt from: ChiefStief
Repo: csg-org/CompactConnect PR: 1036
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/provider/api.py:71-71
Timestamp: 2025-09-03T22:27:56.621Z
Learning: The privilegeJurisdictions field in provider schemas consistently uses load_default=set() pattern across multiple schema files in the CompactConnect codebase (provider/record.py and provider/api.py), and developers prioritize maintaining this consistency.
Applied to files:
backend/compact-connect/tests/smoke/smoke_common.py
🧬 Code graph analysis (2)
backend/compact-connect/tests/smoke/purchasing_privileges_smoke_tests.py (1)
backend/compact-connect/tests/smoke/smoke_common.py (1)
delete_existing_privilege_records(468-522)
backend/compact-connect/tests/smoke/smoke_common.py (2)
backend/compact-connect/lambdas/python/common/cc_common/data_model/update_tier_enum.py (1)
UpdateTierEnum(4-40)backend/compact-connect/tests/smoke/config.py (1)
provider_user_dynamodb_table(44-45)
⏰ 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). (2)
- GitHub Check: CheckWebroot
- GitHub Check: TestApp
🔇 Additional comments (4)
backend/compact-connect/tests/smoke/purchasing_privileges_smoke_tests.py (2)
10-17: LGTM!The import addition aligns with the new helper function in
smoke_common.py. The consolidated import block is clean.
144-145: LGTM!Good refactor to delegate cleanup to the shared helper function. This improves maintainability by centralizing the privilege record cleanup logic and ensures consistent cleanup behavior across smoke tests. Based on learnings, smoke test functions should remain linear and readable, and this helper extraction appropriately supports that goal by removing low-level DynamoDB boilerplate from the test flow.
backend/compact-connect/tests/smoke/smoke_common.py (2)
4-4: LGTM!The
timeimport at module level is appropriate since it's used by the newdelete_existing_privilege_recordsfunction. TheUpdateTierEnumimport correctly follows the noqa pattern used for other deferred imports in this file.Also applies to: 38-38
468-522: Good helper function implementation.The new helper correctly:
- Queries both privilege records and privilege update records using the proper SK patterns
- Uses
UpdateTierEnum.TIER_ONEto match the privilege update tier structure- Includes propagation delay between deletions
- Centralizes cleanup logic that was previously inline in tests
This supports the PR goal of cleaning up test artifacts related to privilege changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In @backend/compact-connect/tests/smoke/smoke_common.py:
- Around line 503-521: The privilege records query using dynamodb_table.query to
populate original_privilege_records can miss items because it doesn't handle
LastEvaluatedKey; replace the single call with a paginated loop like the one
used for original_privilege_update_records: build privilege_sk_prefix as before
(Key('pk').eq(pk) &
Key('sk').begins_with(f'{compact}#PROVIDER#privilege/{jurisdiction}/')), then
repeatedly call dynamodb_table.query with ExclusiveStartKey until
LastEvaluatedKey is falsy, extending original_privilege_records with each
response's Items.
🧹 Nitpick comments (1)
backend/compact-connect/tests/smoke/smoke_common.py (1)
539-542: Consider improving log message clarity.The log message uses singular "privilege record" but could delete multiple. Consider logging both counts for better observability.
🔎 Suggested improvement
+ privilege_count = len(original_privilege_records) - len(original_privilege_update_records) logger.info( - f'Deleted privilege record and {len(original_privilege_update_records)} privilege update records for ' + f'Deleted {privilege_count} privilege record(s) and {len(original_privilege_update_records)} privilege update record(s) for ' f'jurisdiction {jurisdiction}' )Note: You'd need to capture
privilege_countbefore line 523 extends the list, or track the original length beforehand.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
backend/compact-connect/lambdas/python/common/tests/function/test_data_client.pybackend/compact-connect/tests/smoke/encumbrance_smoke_tests.pybackend/compact-connect/tests/smoke/purchasing_privileges_smoke_tests.pybackend/compact-connect/tests/smoke/smoke_common.py
💤 Files with no reviewable changes (1)
- backend/compact-connect/lambdas/python/common/tests/function/test_data_client.py
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/compact-connect/tests/smoke/purchasing_privileges_smoke_tests.py
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
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.
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'].
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).
📚 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/tests/smoke/encumbrance_smoke_tests.pybackend/compact-connect/tests/smoke/smoke_common.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/tests/smoke/encumbrance_smoke_tests.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/tests/smoke/encumbrance_smoke_tests.py
📚 Learning: 2025-08-13T20:28:33.191Z
Learnt from: rmolinares
Repo: csg-org/CompactConnect PR: 1011
File: webroot/src/models/Licensee/Licensee.model.ts:276-279
Timestamp: 2025-08-13T20:28:33.191Z
Learning: The two-year encumbrance wait period feature only applies to privileges, not licenses. The logic should only check if privilege encumbrances that are no longer active have lift dates within the past two years.
Applied to files:
backend/compact-connect/tests/smoke/encumbrance_smoke_tests.py
📚 Learning: 2025-12-16T21:43:07.408Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1219
File: backend/compact-connect/lambdas/python/search/handlers/search.py:131-140
Timestamp: 2025-12-16T21:43:07.408Z
Learning: In backend/compact-connect/lambdas/python/search/handlers/search.py, avoid logging the full request body. Do not log sensitive content by default. If logging is required for security investigations, redact or mask sensitive fields (e.g., credentials, tokens, PII) and log only safe metadata (method, path, status, user identifier). Use a secure, access-controlled audit log or feature flag to enable such logs, ensuring minimal exposure and compliance with security policies. This guideline targets Python backend handlers handling external requests and should be considered for similar files with request processing.
Applied to files:
backend/compact-connect/tests/smoke/encumbrance_smoke_tests.pybackend/compact-connect/tests/smoke/smoke_common.py
📚 Learning: 2025-06-23T20:19:18.952Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 877
File: backend/compact-connect/tests/smoke/practitioner_email_update_smoke_tests.py:22-192
Timestamp: 2025-06-23T20:19:18.952Z
Learning: In the CompactConnect project, smoke test functions (like test_practitioner_email_update in backend/compact-connect/tests/smoke/practitioner_email_update_smoke_tests.py) should remain as single large functions rather than being broken down into smaller helper functions, based on the project maintainer's preference and reasoning.
Applied to files:
backend/compact-connect/tests/smoke/smoke_common.py
📚 Learning: 2025-09-03T22:27:56.621Z
Learnt from: ChiefStief
Repo: csg-org/CompactConnect PR: 1036
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/provider/api.py:71-71
Timestamp: 2025-09-03T22:27:56.621Z
Learning: The privilegeJurisdictions field in provider schemas consistently uses load_default=set() pattern across multiple schema files in the CompactConnect codebase (provider/record.py and provider/api.py), and developers prioritize maintaining this consistency.
Applied to files:
backend/compact-connect/tests/smoke/smoke_common.py
🧬 Code graph analysis (1)
backend/compact-connect/tests/smoke/smoke_common.py (2)
backend/compact-connect/lambdas/python/common/cc_common/data_model/update_tier_enum.py (1)
UpdateTierEnum(4-40)backend/compact-connect/tests/smoke/config.py (1)
provider_user_dynamodb_table(44-45)
⏰ 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 (4)
backend/compact-connect/tests/smoke/encumbrance_smoke_tests.py (2)
686-690: Clarify the purpose of this wait after test completion.This wait is added after all verifications have passed (line 682 confirms privilege is unencumbered). If the purpose is to prevent interference with subsequent tests, consider whether:
- A shorter wait would suffice for inter-test isolation
- The notification handlers should be awaited more explicitly rather than using a time-based wait
- Test cleanup/teardown is the more appropriate place for this synchronization
The 60-second wait may add unnecessary overhead to the smoke test suite if it's only for cleanup between tests.
Can you confirm if notification handlers from this test could interfere with subsequent tests, and whether there's a more targeted way to ensure they complete?
936-939: LGTM - Appropriate synchronization for eventual consistency.This wait is correctly placed between the license lift operation and the subsequent privilege status verification. It allows time for downstream processing to propagate the license lift to the privilege's encumbered status, which is essential for verifying the "licenseEncumbered → unencumbered" transition in line 943.
The placement and pattern are consistent with existing waits in this file (lines 631, 795, 884).
Based on learnings, smoke tests prioritize linear narrative readability, and this inline wait maintains that flow.
backend/compact-connect/tests/smoke/smoke_common.py (2)
4-4: LGTM!The top-level
timeimport andUpdateTierEnumimport are correctly placed and appropriately used by the newdelete_existing_privilege_recordsfunction.Also applies to: 42-42
232-262: LGTM!The 403 handling with automatic token refresh and single retry is a robust pattern for handling expired tokens in smoke tests. The implementation correctly clears the cached token before retrying and avoids infinite retry loops.
backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
webroot/src/network/mocks/mock.data.api.ts (1)
229-252: Fix method signature mismatch between mock and callers.The
encumberLicensemock method was simplified to accept onlynpdbCategories(line 235), but the action handler and tests still pass the deprecated singularnpdbCategoryparameter. The real API methods still accept both parameters, making the mock interface now inconsistent with both its callers and the actual API layer.Either restore
npdbCategoryas an optional parameter in the mock methods (to maintain backwards compatibility during migration), or update the action handler (webroot/src/store/users/users.actions.tslines 134-146) and all tests (webroot/src/store/users/users.spec.tslines 738, 975, etc.) to remove references to the singular form.
🤖 Fix all issues with AI agents
In
@backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py:
- Around line 386-387: The assignment to event['npdbCategories'] directly
indexes event['encumbranceDetails']['clinicalPrivilegeActionCategories'] which
can raise KeyError if clinicalPrivilegeActionCategories is absent; change this
to defensively read encumbranceDetails via event.get('encumbranceDetails') and
then use .get('clinicalPrivilegeActionCategories') (or default to an empty list)
before assigning to event['npdbCategories'], ensuring you reference the same
keys: event, encumbranceDetails, clinicalPrivilegeActionCategories, and
npdbCategories.
In
@backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/api.py:
- Around line 181-182: npdbCategories is currently defined with required and
allow_none passed to String() instead of the List(); change the marshmallow
field so that the inner type is simply String() and the List(...) receives
required=True/False and allow_none=True/False as appropriate (e.g.,
List(String(), required=False, allow_none=True)) so the list itself is
optional/nullable rather than each category string.
🧹 Nitpick comments (2)
webroot/src/models/LicenseHistoryItem/LicenseHistoryItem.model.spec.ts (2)
154-178: LGTM: Comprehensive test for npdbCategories array.The test thoroughly validates:
- Serialization via
fromServerpreserves thenpdbCategoriesarray- Display logic correctly maps and joins multiple category names
Consider adding an explicit test case for a single-item
npdbCategoriesarray (e.g.,npdbCategories: ['Misconduct or Abuse']) to ensure that scenario works correctly, distinct from the backwards-compatibilityserverNotepath tested later.
180-204: Consider clarifying test name for backwards compatibility.The test name mentions "single category" but the test actually validates the backwards-compatibility path using the
serverNotefield (line 187) rather than a single-itemnpdbCategoriesarray. Consider renaming to something like:it('should create a LicenseHistoryItem with correct display values for an encumbrance using legacy serverNote field', () => {This would more clearly communicate that it's testing backwards compatibility with the deprecated field format.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/api.pybackend/compact-connect/lambdas/python/migration/migrate_update_sort_keys/main.pybackend/compact-connect/lambdas/python/migration/tests/function/test_migrate_update_sort_keys.pybackend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_privilege_history.pybackend/compact-connect/pipeline/backend_stage.pybackend/compact-connect/stacks/data_migration_stack/__init__.pywebroot/src/models/LicenseHistoryItem/LicenseHistoryItem.model.spec.tswebroot/src/models/LicenseHistoryItem/LicenseHistoryItem.model.tswebroot/src/network/mocks/mock.data.api.ts
💤 Files with no reviewable changes (4)
- backend/compact-connect/stacks/data_migration_stack/init.py
- backend/compact-connect/lambdas/python/migration/tests/function/test_migrate_update_sort_keys.py
- backend/compact-connect/pipeline/backend_stage.py
- backend/compact-connect/lambdas/python/migration/migrate_update_sort_keys/main.py
🧰 Additional context used
🧠 Learnings (15)
📓 Common learnings
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.
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'].
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).
📚 Learning: 2025-11-06T18:11:58.272Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 1196
File: webroot/src/network/mocks/mock.data.ts:1919-1944
Timestamp: 2025-11-06T18:11:58.272Z
Learning: In CompactConnect, the investigation event type (`updateType: 'investigation'`) in privilege/license history is intentionally not yet fully supported in the frontend UI translation and categorization logic (webroot/src/locales/*.json and webroot/src/models/LicenseHistoryItem/LicenseHistoryItem.model.ts). The server license history API needs to be updated first to return sufficient information about investigation events. Until then, the UI gracefully falls back to displaying "Unknown" for investigation events, which is acceptable as an interim solution.
<!--
Applied to files:
webroot/src/models/LicenseHistoryItem/LicenseHistoryItem.model.spec.tswebroot/src/models/LicenseHistoryItem/LicenseHistoryItem.model.tsbackend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.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:
webroot/src/network/mocks/mock.data.api.tsbackend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.pybackend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_privilege_history.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/schema/privilege/api.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.pybackend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_privilege_history.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/schema/privilege/api.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.pybackend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_privilege_history.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/schema/privilege/api.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.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/schema/privilege/api.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.pybackend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_privilege_history.py
📚 Learning: 2025-12-16T21:43:07.408Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1219
File: backend/compact-connect/lambdas/python/search/handlers/search.py:131-140
Timestamp: 2025-12-16T21:43:07.408Z
Learning: In backend/compact-connect/lambdas/python/search/handlers/search.py, avoid logging the full request body. Do not log sensitive content by default. If logging is required for security investigations, redact or mask sensitive fields (e.g., credentials, tokens, PII) and log only safe metadata (method, path, status, user identifier). Use a secure, access-controlled audit log or feature flag to enable such logs, ensuring minimal exposure and compliance with security policies. This guideline targets Python backend handlers handling external requests and should be considered for similar files with request processing.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/api.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.pybackend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_privilege_history.py
📚 Learning: 2026-01-05T22:50:09.696Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1243
File: backend/compact-connect/tests/smoke/smoke_common.py:503-521
Timestamp: 2026-01-05T22:50:09.696Z
Learning: Enforce the rule: there is only one privilege record per provider per jurisdiction, and do not paginate when querying privilege records by jurisdiction. This applies across the codebase wherever privilege records are queried or tested (implementation and tests). Note that privilege update records can have multiple rows and require pagination. Implement checks and queries to assume a unique constraint for (provider, jurisdiction) on privilege records, and ensure any list endpoints or test smoke checks reflect no pagination for jurisdiction-based privilege queries while preserving pagination for privilege updates.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/api.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.pybackend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_privilege_history.py
📚 Learning: 2025-09-04T17:55:02.692Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1058
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py:785-786
Timestamp: 2025-09-04T17:55:02.692Z
Learning: In backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py, the sanitize_provider_account_profile_fields_from_response method is called within generate_api_response_object, which serves as the central method that all endpoints use to construct provider response objects. This ensures that sensitive profile fields are sanitized at the source, eliminating the need for additional sanitization calls in downstream methods like those in utils.py.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.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/provider_record_util.py
📚 Learning: 2025-09-04T17:55:02.692Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1058
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py:785-786
Timestamp: 2025-09-04T17:55:02.692Z
Learning: In the CompactConnect backend, provider data sanitization follows a centralized architecture: the ProviderRecordUtility.generate_api_response_object() method serves as the single source for constructing provider response objects and includes sanitization of sensitive profile fields. Downstream functions like sanitize_provider_data_based_on_caller_scopes() in utils.py receive already-sanitized data, making additional sanitization calls redundant. This ensures sensitive fields are removed at the source rather than requiring multiple sanitization points throughout the codebase.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.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/provider_record_util.py
📚 Learning: 2025-08-29T18:29:16.953Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 1042
File: webroot/src/pages/MfaResetStartLicensee/MfaResetStartLicensee.vue:141-146
Timestamp: 2025-08-29T18:29:16.953Z
Learning: In the MfaResetStartLicensee component, license type keys (like "audiologist") are displayed directly in the summary using CSS `text-transform: capitalize` because the keys are human-readable strings, rather than using separate display name lookups.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py
📚 Learning: 2025-08-15T19:59:09.332Z
Learnt from: jusdino
Repo: csg-org/CompactConnect PR: 1004
File: backend/compact-connect/docs/internal/api-specification/latest-oas30.json:2305-2354
Timestamp: 2025-08-15T19:59:09.332Z
Learning: In the CompactConnect API, the history endpoint paths are intentionally different between internal and public APIs. Internal compacts history includes "/privileges/" in the path (/v1/compacts/{compact}/providers/{providerId}/privileges/jurisdiction/{jurisdiction}/licenseType/{licenseType}/history) while public compacts history omits it (/v1/public/compacts/{compact}/providers/{providerId}/jurisdiction/{jurisdiction}/licenseType/{licenseType}/history). This inconsistency is a deliberate product design choice, not an oversight.
Applied to files:
backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_privilege_history.py
🧬 Code graph analysis (2)
webroot/src/models/LicenseHistoryItem/LicenseHistoryItem.model.spec.ts (1)
webroot/src/models/LicenseHistoryItem/LicenseHistoryItem.model.ts (2)
LicenseHistoryItemSerializer(121-135)LicenseHistoryItem(26-116)
webroot/src/models/LicenseHistoryItem/LicenseHistoryItem.model.ts (2)
webroot/src/models/Compact/Compact.model.ts (1)
name(109-115)webroot/src/models/State/State.model.ts (1)
name(41-48)
⏰ 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). (2)
- GitHub Check: TestApp
- GitHub Check: CheckWebroot
🔇 Additional comments (9)
webroot/src/models/LicenseHistoryItem/LicenseHistoryItem.model.ts (4)
20-20: LGTM: npdbCategories property addition.The optional
npdbCategoriesfield properly supports the migration from the deprecated singular field while maintaining backwards compatibility.
35-35: LGTM: Class property initialization.The default
nullvalue ensures backwards compatibility with existing data that doesn't include npdbCategories.
98-111: LGTM: Encumbrance note display logic with backwards compatibility.The implementation correctly:
- Prioritizes the new
npdbCategoriesarray format- Maps category keys to human-readable translated names
- Falls back to the key itself if translation is missing
- Maintains backwards compatibility by using
serverNotewhennpdbCategoriesis absent or empty
130-130: LGTM: Serializer mapping for npdbCategories.The serializer correctly preserves
npdbCategoriesfrom the server payload.webroot/src/models/LicenseHistoryItem/LicenseHistoryItem.model.spec.ts (1)
45-45: LGTM: Default value assertion.The test correctly verifies that
npdbCategoriesdefaults tonull.webroot/src/network/mocks/mock.data.api.ts (1)
324-347: LGTM! Consistent with encumberLicense changes.The method correctly mirrors the
encumberLicensechanges by consistently usingnpdbCategories(plural) in both the parameter and return object. This completes the migration from the deprecated singular field for privilege encumbrance.backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_privilege_history.py (3)
27-31: LGTM!Test data correctly uses
clinicalPrivilegeActionCategories(plural) as a list, aligning with the schema changes.
225-225: LGTM!Removal of the
mock_flagparameter aligns with the PR objective to remove deprecated feature flag handling now that migration is complete.
258-258: LGTM!The assertion correctly expects
npdbCategoriesas a list with multiple items, providing good test coverage for the multi-category scenario.
backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py
Show resolved
Hide resolved
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/api.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/compact-connect/stacks/api_stack/v1_api/api_model.py
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
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.
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'].
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).
📚 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/stacks/api_stack/v1_api/api_model.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/stacks/api_stack/v1_api/api_model.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/stacks/api_stack/v1_api/api_model.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/stacks/api_stack/v1_api/api_model.py
📚 Learning: 2025-04-29T01:59:51.222Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 769
File: backend/compact-connect/stacks/api_stack/v1_api/api_model.py:397-401
Timestamp: 2025-04-29T01:59:51.222Z
Learning: In the CompactConnect project, validation constraints like enum values should be defined only in runtime code (Lambda) rather than duplicating them in CDK API schema definitions. This applies to fields like clinicalPrivilegeActionCategory and other similar enumerations.
Applied to files:
backend/compact-connect/stacks/api_stack/v1_api/api_model.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/stacks/api_stack/v1_api/api_model.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/stacks/api_stack/v1_api/api_model.py
📚 Learning: 2025-12-16T21:43:07.408Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1219
File: backend/compact-connect/lambdas/python/search/handlers/search.py:131-140
Timestamp: 2025-12-16T21:43:07.408Z
Learning: In backend/compact-connect/lambdas/python/search/handlers/search.py, avoid logging the full request body. Do not log sensitive content by default. If logging is required for security investigations, redact or mask sensitive fields (e.g., credentials, tokens, PII) and log only safe metadata (method, path, status, user identifier). Use a secure, access-controlled audit log or feature flag to enable such logs, ensuring minimal exposure and compliance with security policies. This guideline targets Python backend handlers handling external requests and should be considered for similar files with request processing.
Applied to files:
backend/compact-connect/stacks/api_stack/v1_api/api_model.py
📚 Learning: 2026-01-05T22:50:09.696Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1243
File: backend/compact-connect/tests/smoke/smoke_common.py:503-521
Timestamp: 2026-01-05T22:50:09.696Z
Learning: Enforce the rule: there is only one privilege record per provider per jurisdiction, and do not paginate when querying privilege records by jurisdiction. This applies across the codebase wherever privilege records are queried or tested (implementation and tests). Note that privilege update records can have multiple rows and require pagination. Implement checks and queries to assume a unique constraint for (provider, jurisdiction) on privilege records, and ensure any list endpoints or test smoke checks reflect no pagination for jurisdiction-based privilege queries while preserving pagination for privilege updates.
Applied to files:
backend/compact-connect/stacks/api_stack/v1_api/api_model.py
🧬 Code graph analysis (1)
backend/compact-connect/stacks/api_stack/v1_api/api_model.py (1)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/jurisdiction/__init__.py (1)
required(17-18)
⏰ 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). (2)
- GitHub Check: CheckWebroot
- GitHub Check: TestApp
🔇 Additional comments (1)
backend/compact-connect/stacks/api_stack/v1_api/api_model.py (1)
1416-1420: LGTM!The array field definition is correct and the description is clear.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @backend/compact-connect/lambdas/python/staff-users/requirements-dev.txt:
- Line 7: The pinned boto3==1.42.24 (and corresponding botocore pin mentioned in
the review) does not exist on PyPI and will break installs; update the pins in
requirements-dev.txt to an available release (e.g., boto3==1.42.21 and matching
botocore==1.42.21) or to a verified current version, ensure boto3 and botocore
versions are mutually compatible, run a quick pip index versions / pip install
-r to verify resolution, and update any lockfile or CI dependency cache
accordingly.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
backend/compact-connect/lambdas/python/cognito-backup/requirements-dev.txtbackend/compact-connect/lambdas/python/cognito-backup/requirements.txtbackend/compact-connect/lambdas/python/common/requirements-dev.txtbackend/compact-connect/lambdas/python/common/requirements.txtbackend/compact-connect/lambdas/python/compact-configuration/requirements-dev.txtbackend/compact-connect/lambdas/python/custom-resources/requirements-dev.txtbackend/compact-connect/lambdas/python/data-events/requirements-dev.txtbackend/compact-connect/lambdas/python/disaster-recovery/requirements-dev.txtbackend/compact-connect/lambdas/python/provider-data-v1/requirements-dev.txtbackend/compact-connect/lambdas/python/search/requirements-dev.txtbackend/compact-connect/lambdas/python/search/requirements.txtbackend/compact-connect/lambdas/python/staff-user-pre-token/requirements-dev.txtbackend/compact-connect/lambdas/python/staff-users/requirements-dev.txtbackend/compact-connect/requirements-dev.txtbackend/compact-connect/requirements.txt
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
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.
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'].
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).
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1243
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py:386-387
Timestamp: 2026-01-07T21:20:07.472Z
Learning: In backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py, the field clinicalPrivilegeActionCategories is required for encumbrance events in encumbranceDetails. The code should raise an exception if this field is missing rather than using defensive coding with .get(), as its absence indicates a data integrity issue that should be caught immediately with a fail-fast approach.
📚 Learning: 2025-07-22T03:52:25.934Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 907
File: backend/compact-connect/lambdas/python/provider-data-v1/requirements.txt:2-2
Timestamp: 2025-07-22T03:52:25.934Z
Learning: In CompactConnect, the Python version used by pip-compile to generate requirements.txt files (shown in the header comment) is separate from the actual Lambda runtime environment. Dependencies are installed by a Python 3.12 container during the CI/CD pipeline, ensuring runtime compatibility regardless of the Python version used for pip-compile dependency resolution.
Applied to files:
backend/compact-connect/lambdas/python/search/requirements.txtbackend/compact-connect/lambdas/python/custom-resources/requirements-dev.txtbackend/compact-connect/requirements.txtbackend/compact-connect/lambdas/python/staff-users/requirements-dev.txtbackend/compact-connect/lambdas/python/data-events/requirements-dev.txtbackend/compact-connect/requirements-dev.txtbackend/compact-connect/lambdas/python/common/requirements.txtbackend/compact-connect/lambdas/python/compact-configuration/requirements-dev.txtbackend/compact-connect/lambdas/python/staff-user-pre-token/requirements-dev.txtbackend/compact-connect/lambdas/python/disaster-recovery/requirements-dev.txtbackend/compact-connect/lambdas/python/search/requirements-dev.txtbackend/compact-connect/lambdas/python/cognito-backup/requirements-dev.txtbackend/compact-connect/lambdas/python/cognito-backup/requirements.txtbackend/compact-connect/lambdas/python/common/requirements-dev.txtbackend/compact-connect/lambdas/python/provider-data-v1/requirements-dev.txt
📚 Learning: 2025-07-22T03:36:17.137Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 907
File: backend/compact-connect/lambdas/python/purchases/requirements-dev.txt:15-0
Timestamp: 2025-07-22T03:36:17.137Z
Learning: In CompactConnect, requirements-dev.txt files for Lambda functions are used exclusively for running tests and development, not for actual Lambda runtime environments. Concerns about runtime compatibility (like OpenSSL versions) don't apply to these development dependency files.
Applied to files:
backend/compact-connect/lambdas/python/search/requirements.txtbackend/compact-connect/lambdas/python/custom-resources/requirements-dev.txtbackend/compact-connect/requirements.txtbackend/compact-connect/lambdas/python/staff-users/requirements-dev.txtbackend/compact-connect/lambdas/python/data-events/requirements-dev.txtbackend/compact-connect/requirements-dev.txtbackend/compact-connect/lambdas/python/common/requirements.txtbackend/compact-connect/lambdas/python/compact-configuration/requirements-dev.txtbackend/compact-connect/lambdas/python/staff-user-pre-token/requirements-dev.txtbackend/compact-connect/lambdas/python/disaster-recovery/requirements-dev.txtbackend/compact-connect/lambdas/python/search/requirements-dev.txtbackend/compact-connect/lambdas/python/cognito-backup/requirements-dev.txtbackend/compact-connect/lambdas/python/cognito-backup/requirements.txtbackend/compact-connect/lambdas/python/common/requirements-dev.txtbackend/compact-connect/lambdas/python/provider-data-v1/requirements-dev.txt
📚 Learning: 2025-08-12T19:49:24.999Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1001
File: backend/compact-connect/lambdas/python/disaster-recovery/requirements.in:1-1
Timestamp: 2025-08-12T19:49:24.999Z
Learning: In CompactConnect disaster-recovery Lambda functions, runtime dependencies like boto3, aws-lambda-powertools, and botocore are provided by lambda layers at deploy time rather than being specified in requirements.in files. The requirements.in file intentionally contains only a comment explaining this approach.
Applied to files:
backend/compact-connect/lambdas/python/custom-resources/requirements-dev.txtbackend/compact-connect/requirements.txtbackend/compact-connect/lambdas/python/staff-users/requirements-dev.txtbackend/compact-connect/lambdas/python/data-events/requirements-dev.txtbackend/compact-connect/lambdas/python/common/requirements.txtbackend/compact-connect/lambdas/python/compact-configuration/requirements-dev.txtbackend/compact-connect/lambdas/python/staff-user-pre-token/requirements-dev.txtbackend/compact-connect/lambdas/python/disaster-recovery/requirements-dev.txtbackend/compact-connect/lambdas/python/search/requirements-dev.txtbackend/compact-connect/lambdas/python/cognito-backup/requirements-dev.txtbackend/compact-connect/lambdas/python/cognito-backup/requirements.txtbackend/compact-connect/lambdas/python/common/requirements-dev.txtbackend/compact-connect/lambdas/python/provider-data-v1/requirements-dev.txt
📚 Learning: 2025-08-12T19:49:48.235Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1001
File: backend/compact-connect/lambdas/python/disaster-recovery/requirements.txt:1-6
Timestamp: 2025-08-12T19:49:48.235Z
Learning: The disaster-recovery Lambda functions in CompactConnect get their aws-lambda-powertools dependency from the shared lambda layer rather than individual requirements.txt files, which is why their requirements.txt files can be empty or header-only.
Applied to files:
backend/compact-connect/lambdas/python/custom-resources/requirements-dev.txtbackend/compact-connect/requirements.txtbackend/compact-connect/lambdas/python/staff-users/requirements-dev.txtbackend/compact-connect/lambdas/python/data-events/requirements-dev.txtbackend/compact-connect/lambdas/python/common/requirements.txtbackend/compact-connect/lambdas/python/compact-configuration/requirements-dev.txtbackend/compact-connect/lambdas/python/staff-user-pre-token/requirements-dev.txtbackend/compact-connect/lambdas/python/disaster-recovery/requirements-dev.txtbackend/compact-connect/lambdas/python/search/requirements-dev.txtbackend/compact-connect/lambdas/python/cognito-backup/requirements-dev.txtbackend/compact-connect/lambdas/python/cognito-backup/requirements.txtbackend/compact-connect/lambdas/python/common/requirements-dev.txtbackend/compact-connect/lambdas/python/provider-data-v1/requirements-dev.txt
📚 Learning: 2025-07-21T20:40:56.491Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 907
File: backend/compact-connect/lambdas/python/common/requirements.txt:7-0
Timestamp: 2025-07-21T20:40:56.491Z
Learning: In CompactConnect, there is only one lambda layer in use for Python lambdas, and this single layer manages the versions of aws-lambda-powertools, boto3, and botocore dependencies. This eliminates concerns about version skew across multiple lambda layers since all Python lambdas share the same dependency management through this single layer.
Applied to files:
backend/compact-connect/lambdas/python/custom-resources/requirements-dev.txtbackend/compact-connect/requirements.txtbackend/compact-connect/lambdas/python/staff-users/requirements-dev.txtbackend/compact-connect/lambdas/python/data-events/requirements-dev.txtbackend/compact-connect/lambdas/python/common/requirements.txtbackend/compact-connect/lambdas/python/compact-configuration/requirements-dev.txtbackend/compact-connect/lambdas/python/staff-user-pre-token/requirements-dev.txtbackend/compact-connect/lambdas/python/disaster-recovery/requirements-dev.txtbackend/compact-connect/lambdas/python/search/requirements-dev.txtbackend/compact-connect/lambdas/python/cognito-backup/requirements-dev.txtbackend/compact-connect/lambdas/python/cognito-backup/requirements.txtbackend/compact-connect/lambdas/python/common/requirements-dev.txtbackend/compact-connect/lambdas/python/provider-data-v1/requirements-dev.txt
📚 Learning: 2025-08-22T21:20:35.260Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1029
File: backend/compact-connect/docs/api-specification/latest-oas30.json:468-471
Timestamp: 2025-08-22T21:20:35.260Z
Learning: The file backend/compact-connect/docs/api-specification/latest-oas30.json is auto-generated by API Gateway and should not be modified inline. Any schema changes would need to be addressed at the source in the CDK/CloudFormation definitions.
Applied to files:
backend/compact-connect/requirements.txtbackend/compact-connect/lambdas/python/common/requirements.txtbackend/compact-connect/lambdas/python/cognito-backup/requirements-dev.txtbackend/compact-connect/lambdas/python/cognito-backup/requirements.txtbackend/compact-connect/lambdas/python/provider-data-v1/requirements-dev.txt
📚 Learning: 2025-12-11T17:30:43.367Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1219
File: backend/compact-connect/lambdas/python/search/handlers/provider_update_ingest.py:15-20
Timestamp: 2025-12-11T17:30:43.367Z
Learning: In the CompactConnect project, Lambda functions use AWS CDK's PythonFunction construct for bundling. The bundling process handles module resolution such that handlers in backend/compact-connect/lambdas/python/search/handlers/ can use bare imports (e.g., `from opensearch_client import OpenSearchClient`) to reference modules at the parent search level, even without an __init__.py file in the search directory. The imports work correctly at runtime despite static analysis concerns.
Applied to files:
backend/compact-connect/requirements.txt
⏰ 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). (2)
- GitHub Check: TestApp
- GitHub Check: CheckWebroot
🔇 Additional comments (21)
backend/compact-connect/lambdas/python/cognito-backup/requirements.txt (1)
7-7: Verify these dependency bumps are intentional and aligned with PR objectives.This file shows dependency version updates (aws-lambda-powertools, boto3, botocore, urllib3) that appear disconnected from the PR objective of removing the deprecated
clinicalPrivilegeActionCategoryfield. Additionally, per learnings from prior PRs, runtime dependencies like boto3 and botocore are typically provided by lambda layers rather than individual requirements.txt files in this codebase.Questions:
- Are these dependency changes intentional, or did they get bundled accidentally?
- Is the lambda layer (
backend/compact-connect/lambdas/python/layer) also being updated to maintain version consistency and avoid skew?- Were Python unit tests run to validate these dependency upgrades in the cognito-backup Lambda? (The PR testing checklist mentions
yarn test:unit:all, but this is Python code.)Also applies to: 9-11, 29-29
backend/compact-connect/lambdas/python/cognito-backup/requirements-dev.txt (1)
7-87: Verify that dependency updates belong in this PR.This file contains only routine dependency version bumps (aws-lambda-powertools, boto3, botocore, certifi, joserfc, moto, urllib3) and appears to be auto-generated via pip-compile. These updates are orthogonal to the core PR objective of removing deprecated backend fields (
clinicalPrivilegeActionCategory).Clarify whether:
- These dependency updates are necessary to support the deprecated field removal.
- This file was regenerated as an incidental side effect, and the change should be excluded (or reverted) in favor of a separate dependency-management PR.
- These updates carry any breaking changes or compatibility implications for cognito-backup Lambda tests.
backend/compact-connect/lambdas/python/staff-user-pre-token/requirements-dev.txt (1)
7-7: Routine development dependency updates—safe and appropriate.These are patch and minor version bumps for development/test dependencies only. All updates are low-risk:
boto3andbotocorepatch bumps (1.42.11 → 1.42.24) are safe; per your learnings, version management is centralized via the single lambda layer, so no version skew concerns apply.certifiandurllib3are transitive dependencies with standard patch updates.motopatch bump (5.1.18 → 5.1.19) poses no compatibility risk for testing.The file is autogenerated by pip-compile, so these updates are expected when dependencies are refreshed. No action required unless you encounter test failures unrelated to the main PR objective.
Also applies to: 9-9, 14-14, 36-36, 61-61
backend/compact-connect/lambdas/python/disaster-recovery/requirements-dev.txt (1)
7-7: Routine autogenerated dependency updates for development.These patch-version updates to development dependencies (boto3, botocore, certifi, moto, urllib3) are standard maintenance, likely from re-running pip-compile as part of this PR. Since
requirements-dev.txtis development-only, runtime compatibility concerns don't apply—these updates will be validated by the CI test suite.Also applies to: 9-9, 14-14, 36-36, 61-61
backend/compact-connect/lambdas/python/search/requirements.txt (2)
2-2: Verify Python version used for pip-compile.The header indicates pip-compile was run with Python 3.14, but per project learnings, a Python 3.12 container is used during CI/CD for actual Lambda deployment. Verify that this version mismatch is intentional and doesn't create compatibility issues with the resulting dependency pins.
7-7: Verify dependency version legitimacy and security.This file shows version bumps for certifi (2026.1.4) and urllib3 (2.6.3). Since this PR's primary objective is to remove deprecated backend fields, confirm:
- Whether these dependency updates are required for this change or should be split into a separate dependency-management PR.
- That the pinned versions are legitimate and free of known security vulnerabilities.
- Compatibility with other dependencies (e.g., opensearch-py, requests).
Note: This is an autogenerated file; changes typically indicate
requirements.inwas modified or pip-compile was re-run. Verify the reason for re-running pip-compile.Also applies to: 33-33
backend/compact-connect/lambdas/python/common/requirements-dev.txt (3)
15-18: Verify the aws-sam-translator downgrade and dependency resolution.The aws-sam-translator version was downgraded from 1.105.0 to 1.103.0 (lines 15–18), which is unusual. The updated "via" comments now reference cfn-lint and moto, suggesting a dependency conflict resolution or a deliberate pin adjustment. Since this is an auto-generated file from
pip-compile, the downgrade may reflect constraint resolution; however, confirm that:
- This downgrade was intentional in the source
requirements-dev.infile- It does not negatively impact test execution or the build pipeline
- It is not a regression from the previous version
41-41: Verify the cfn-lint and pydantic downgrades.Both cfn-lint (1.43.0 → 1.41.0, line 41) and pydantic (2.12.5 → 2.12.4, line 110) were downgraded, which is atypical for a routine refresh of development dependencies. The PR objective focuses on removing deprecated code—not dependency version adjustments—so clarify:
- Are these downgrades intentional constraints in
requirements-dev.in, or artifacts of pip-compile's conflict resolution?- Do they relate to test or compatibility requirements for the deprecated field removal?
- Have tests been run successfully with these pinned versions?
Also applies to: 110-110
1-184: Confirm scope alignment of dependency updates.This PR's primary objective is to remove the deprecated
clinicalPrivilegeActionCategoryfield and associated logic. The requirements-dev.txt file shows multiple version pin changes—mostly minor upgrades with some downgrades—but no connection to the PR objective is evident. Verify that:
- These dependency changes are necessary for the codebase changes in this PR (e.g., to satisfy new test requirements or resolve conflicts introduced by schema/logic changes)
- They were generated via
pip-compileagainst an updatedrequirements-dev.inintentionallyIf these changes are incidental (e.g., from running
pip-compilewithout targeted updates), consider reverting them to keep the PR focused on the core objective of removing deprecated fields.backend/compact-connect/requirements-dev.txt (1)
15-15: Routine dependency version updates — verify pyparsing minor bump.The dependency updates are mostly micro/patch versions and are appropriate for a development-only requirements file. However, Line 73 updates
pyparsingfrom 3.2.5 to 3.3.1 (a minor version bump), which could introduce behavioral changes. Sincepip-requirements-parserdepends on this library, verify that the update doesn't cause compatibility issues with tooling that parses requirements.Based on learnings, these are development dependencies only, so runtime compatibility is not a concern. However, if you encounter any issues running the test suite (
yarn test:unit:all) after this update, the pyparsing bump would be the first place to investigate.Also applies to: 21-21, 31-31, 73-73, 91-91, 101-101
backend/compact-connect/lambdas/python/common/requirements.txt (1)
1-55: Verify dependency updates align with PR deprecation removal goals and maintain compatibility.This auto-generated requirements.txt shows routine patch/minor version bumps across AWS and utility dependencies (aws-lambda-powertools 3.24.0, boto3/botocore 1.42.24, certifi 2026.1.4, marshmallow 3.26.2, urllib3 2.6.3). Since this file is auto-generated by pip-compile, these updates likely resulted from either changes to
requirements.inor a rebuild. However, confirm:
- Whether these dependency updates are intentional for the deprecation removal or incidental from regeneration.
- Whether boto3/botocore 1.42.24 remain compatible with the Lambda 3.12 runtime (per learnings, the runtime differs from pip-compile's Python 3.14).
- Whether marshmallow 3.26.2 changes impact the schema refactoring (clinicalPrivilegeActionCategory removal) mentioned in the PR.
backend/compact-connect/lambdas/python/data-events/requirements-dev.txt (1)
7-7: Verify that dependency version bumps are intentional and tested.This file is autogenerated by pip-compile. The version updates appear to be incremental patch/minor bumps (boto3, botocore, moto, certifi, urllib3). Since this is a development-only file, runtime compatibility concerns don't apply. However, confirm that:
- These version bumps are intentional (not accidental side effects)
- Tests pass with these versions (per PR testing requirements:
yarn test:unit:allwithout errors)- They don't introduce any incompatibilities with changes elsewhere in the PR
Also applies to: 9-9, 14-14, 36-36, 61-61
backend/compact-connect/lambdas/python/compact-configuration/requirements-dev.txt (2)
1-6: Autogenerated file—no manual edits detected.The header confirms this file is pip-compile output. The Python version (3.14) used for resolution differs from the Lambda runtime (3.12), but this is expected per the build process.
7-7: Version bumps are minor/patch updates—routine and appropriate for dev dependencies.All changed lines are version increments:
- boto3 & botocore: 1.42.11 → 1.42.24 (lockstep minor bump; correct)
- certifi: 2025.11.12 → 2026.1.4 (CA certificate rotation; typical security practice)
- moto: 5.1.18 → 5.1.19 (test library patch)
- urllib3: 2.6.2 → 2.6.3 (HTTP library patch; often includes stability fixes)
Since this is a requirements-dev.txt file (dev/testing only, per learnings), these updates affect test execution only, not production Lambda runtime. The single lambda layer provides boto3/botocore at runtime.
Verify that the input
requirements-dev.infile was not manually edited in ways that would introduce unexpected constraints or incompatibilities. If it was, confirm that the generated lock file is consistent and resolves without conflicts.Also applies to: 9-9, 14-14, 36-36, 61-61
backend/compact-connect/lambdas/python/search/requirements-dev.txt (1)
7-7: Development dependency updates look good.This autogenerated file shows routine patch and minor version updates to dev-only dependencies (boto3, botocore, certifi, moto, urllib3), which are appropriate for testing and development. The certifi 2026.1.4 version is current and reasonable for a January 2026 codebase.
Please confirm these dependency updates were intentional (regenerated via pip-compile), rather than accidental. You can verify by checking if
requirements-dev.inwas modified or if pip-compile was manually run.Also applies to: 9-9, 14-14, 36-36, 59-59
backend/compact-connect/requirements.txt (1)
1-74: No action required—version bumps are expected from pip-compile's resolution of loose constraints.This auto-generated file reflects pip-compile resolving the loose version constraints in
requirements.in(e.g.,aws-cdk-lib>=2.140.0) to the latest compatible versions. The regeneration withpip-compile --no-emit-index-url --no-strip-extras requirements.inis a normal operation and was intentionally run (git log: "update requirements to latest"). No changes to version pins inrequirements.inwere needed to trigger these updates.Ensure the test runs (
yarn test:unit:all,yarn serve,yarn build) validate compatibility with these newer versions as part of routine verification.backend/compact-connect/lambdas/python/provider-data-v1/requirements-dev.txt (3)
14-14: LGTM!The certifi update (2025.11.12 → 2026.1.4) is a routine security and certificate authority bundle refresh. This is appropriate for dev dependencies and aligns with the January 2026 release timeline.
38-38: LGTM!The patch-level version bumps for moto (5.1.18 → 5.1.19) and urllib3 (2.6.2 → 2.6.3) are routine maintenance updates with no anticipated breaking changes. These are appropriate for dev/test dependencies.
Also applies to: 65-65
7-7: The versions are already appropriately synchronized. Both the shared lambda layer (common/requirements.txt) and this file specify boto3==1.42.24 and botocore==1.42.24, so there is no version mismatch between development and production environments.backend/compact-connect/lambdas/python/custom-resources/requirements-dev.txt (2)
2-2: Note: Python version in header appears ahead of current timeline.The file header indicates
Python 3.14, which is a pip-compile artifact. This is not a functional issue for development-only dependencies but worth noting that the actual Lambda runtime uses Python 3.12 (per learnings). No action required.
7-7: This is an intentional dependency update—no action needed.The commit history shows "update requirements to latest" and verification confirms no critical security issues. The urllib3 version includes security fixes (CVE-2025-66418). Development-only dependencies carry minimal risk, and the Python 3.14 mentioned in the pip-compile header is a tool artifact, not a runtime concern.
Likely an incorrect or invalid review comment.
backend/compact-connect/lambdas/python/staff-users/requirements-dev.txt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
backend/compact-connect/lambdas/python/common/tests/unit/test_provider_record_util.py (1)
992-1068: Update test data to use new field format for consistency.The encumbranceDetails at line 1032 still uses the deprecated
notefield instead of the newclinicalPrivilegeActionCategoriesarray format. While this test verifies that notes are excluded from output (whenFalseis passed), the input data should reflect the current schema for consistency with the first test (line 953) and the broader migration away from deprecated fields.♻️ Suggested update
'encumbranceDetails': { - 'note': 'Non-compliance With Requirements', + 'clinicalPrivilegeActionCategories': ['Non-compliance With Requirements'], 'licenseJurisdiction': 'oh', },Based on learnings,
clinicalPrivilegeActionCategoriesis the required field for encumbrance events after the migration.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/api.pybackend/compact-connect/lambdas/python/common/tests/unit/test_provider_record_util.py
🧰 Additional context used
🧠 Learnings (14)
📓 Common learnings
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.
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'].
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1243
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py:386-387
Timestamp: 2026-01-07T21:20:07.472Z
Learning: In backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py, the field clinicalPrivilegeActionCategories is required for encumbrance events in encumbranceDetails. The code should raise an exception if this field is missing rather than using defensive coding with .get(), as its absence indicates a data integrity issue that should be caught immediately with a fail-fast approach.
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).
📚 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/tests/unit/test_provider_record_util.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/api.py
📚 Learning: 2026-01-07T21:20:07.472Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1243
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py:386-387
Timestamp: 2026-01-07T21:20:07.472Z
Learning: In backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py, the field clinicalPrivilegeActionCategories is required for encumbrance events in encumbranceDetails. The code should raise an exception if this field is missing rather than using defensive coding with .get(), as its absence indicates a data integrity issue that should be caught immediately with a fail-fast approach.
Applied to files:
backend/compact-connect/lambdas/python/common/tests/unit/test_provider_record_util.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/api.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/tests/unit/test_provider_record_util.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/api.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/tests/unit/test_provider_record_util.py
📚 Learning: 2025-04-29T01:57:43.684Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 769
File: backend/compact-connect/lambdas/python/common/tests/resources/dynamo/provider.json:5-5
Timestamp: 2025-04-29T01:57:43.684Z
Learning: The provider.json test data contains both "providerDateOfUpdate" and "dateOfUpdate" fields which serve different purposes, and both should be maintained in the test files.
Applied to files:
backend/compact-connect/lambdas/python/common/tests/unit/test_provider_record_util.py
📚 Learning: 2025-08-21T19:57:13.276Z
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:0-0
Timestamp: 2025-08-21T19:57:13.276Z
Learning: In the CompactConnect codebase, ProviderData.recoveryExpiry property returns `datetime | None` after the type hint fix. The marshmallow DateTime field in the record schema handles conversion between ISO strings (database storage) and datetime objects (application use), so test assertions can directly compare datetime objects.
Applied to files:
backend/compact-connect/lambdas/python/common/tests/unit/test_provider_record_util.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/common/tests/unit/test_provider_record_util.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/tests/unit/test_provider_record_util.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/api.py
📚 Learning: 2025-12-16T21:43:07.408Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1219
File: backend/compact-connect/lambdas/python/search/handlers/search.py:131-140
Timestamp: 2025-12-16T21:43:07.408Z
Learning: In backend/compact-connect/lambdas/python/search/handlers/search.py, avoid logging the full request body. Do not log sensitive content by default. If logging is required for security investigations, redact or mask sensitive fields (e.g., credentials, tokens, PII) and log only safe metadata (method, path, status, user identifier). Use a secure, access-controlled audit log or feature flag to enable such logs, ensuring minimal exposure and compliance with security policies. This guideline targets Python backend handlers handling external requests and should be considered for similar files with request processing.
Applied to files:
backend/compact-connect/lambdas/python/common/tests/unit/test_provider_record_util.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/api.py
📚 Learning: 2026-01-05T22:50:09.696Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1243
File: backend/compact-connect/tests/smoke/smoke_common.py:503-521
Timestamp: 2026-01-05T22:50:09.696Z
Learning: Enforce the rule: there is only one privilege record per provider per jurisdiction, and do not paginate when querying privilege records by jurisdiction. This applies across the codebase wherever privilege records are queried or tested (implementation and tests). Note that privilege update records can have multiple rows and require pagination. Implement checks and queries to assume a unique constraint for (provider, jurisdiction) on privilege records, and ensure any list endpoints or test smoke checks reflect no pagination for jurisdiction-based privilege queries while preserving pagination for privilege updates.
Applied to files:
backend/compact-connect/lambdas/python/common/tests/unit/test_provider_record_util.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/api.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/schema/privilege/api.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/schema/privilege/api.py
📚 Learning: 2025-04-29T02:18:11.099Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 769
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/record.py:40-43
Timestamp: 2025-04-29T02:18:11.099Z
Learning: In the CompactConnect codebase, API design follows a pattern where fields are either provided with valid values or omitted entirely. Null values are not allowed, even for optional fields. This applies to Marshmallow schemas where fields are configured with `required=False, allow_none=False`.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/api.py
⏰ 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). (2)
- GitHub Check: CheckWebroot
- GitHub Check: TestApp
🔇 Additional comments (2)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/api.py (1)
181-182: Past issue resolved - marshmallow syntax is now correct.The marshmallow field definition syntax has been corrected as requested in the previous review. The
requiredandallow_noneparameters are now properly on theList()field rather than the innerString()type. The implementation is also consistent with the existingnotefield pattern (line 180).backend/compact-connect/lambdas/python/common/tests/unit/test_provider_record_util.py (1)
914-990: LGTM! Test correctly updated for field migration.The test properly validates the migration from singular
clinicalPrivilegeActionCategoryto array-basedclinicalPrivilegeActionCategoriesin the input, and the transformation tonpdbCategoriesin the output. The feature flag removal is also appropriate for this phase of the migration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
@backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/record.py:
- Line 37: The schema change making clinicalPrivilegeActionCategories required
is breaking for existing DB records; either ensure the migration that populates
this field has completed for all adverse action records before deploying, or
revert to optional behavior by setting clinicalPrivilegeActionCategories =
List(ClinicalPrivilegeActionCategoryField(), required=False, allow_none=True)
and add a defensive fallback in the loader/property (the property getter
referenced in __init__.py around line 83) to return [] when absent, or add a
custom marshmallow pre_load to supply a default empty list—pick one approach and
update the schema/class (clinicalPrivilegeActionCategories) and the __init__
property logic consistently so deserialization does not raise ValidationError
for records missing the field.
🧹 Nitpick comments (1)
webroot/src/models/AdverseAction/AdverseAction.model.spec.ts (1)
147-198: Consider adding test coverage for multiple NPDB categories.While the existing tests provide good coverage, consider adding a test case where
clinicalPrivilegeActionCategoriescontains multiple elements to explicitly verify thatgetFirstNpdbTypeName()returns only the first element and not other elements from the array. This would provide stronger validation of the method's contract.📋 Example test case
Add after line 198:
it('should return only the first NPDB category when multiple categories are present', () => { const data = { clinicalPrivilegeActionCategories: [ 'Non-compliance With Requirements', 'Substandard Care', 'Misuse of Drugs' ], }; const adverseAction = AdverseActionSerializer.fromServer(data); // Test that only first element is returned expect(adverseAction.getFirstNpdbTypeName()).to.equal('Non-compliance With Requirements'); expect(adverseAction.npdbTypes).to.matchPattern(data.clinicalPrivilegeActionCategories); });
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/record.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/record.pybackend/compact-connect/lambdas/python/purchases/requirements-dev.inbackend/compact-connect/lambdas/python/purchases/requirements-dev.txtbackend/compact-connect/lambdas/python/purchases/requirements.inbackend/compact-connect/lambdas/python/purchases/requirements.txtwebroot/src/components/LicenseCard/LicenseCard.vuewebroot/src/components/PrivilegeCard/PrivilegeCard.vuewebroot/src/models/AdverseAction/AdverseAction.model.spec.tswebroot/src/models/AdverseAction/AdverseAction.model.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- webroot/src/components/LicenseCard/LicenseCard.vue
- webroot/src/models/AdverseAction/AdverseAction.model.ts
🧰 Additional context used
🧠 Learnings (20)
📓 Common learnings
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.
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'].
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).
📚 Learning: 2025-07-22T03:52:25.934Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 907
File: backend/compact-connect/lambdas/python/provider-data-v1/requirements.txt:2-2
Timestamp: 2025-07-22T03:52:25.934Z
Learning: In CompactConnect, the Python version used by pip-compile to generate requirements.txt files (shown in the header comment) is separate from the actual Lambda runtime environment. Dependencies are installed by a Python 3.12 container during the CI/CD pipeline, ensuring runtime compatibility regardless of the Python version used for pip-compile dependency resolution.
Applied to files:
backend/compact-connect/lambdas/python/purchases/requirements.inbackend/compact-connect/lambdas/python/purchases/requirements.txtbackend/compact-connect/lambdas/python/purchases/requirements-dev.inbackend/compact-connect/lambdas/python/purchases/requirements-dev.txt
📚 Learning: 2025-08-21T02:51:28.199Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1014
File: backend/compact-connect/lambdas/python/common/requirements.in:4-4
Timestamp: 2025-08-21T02:51:28.199Z
Learning: In CompactConnect, the purchases lambda contains requests as a transitive dependency from the Authorize.net SDK, which is automatically resolved by pip-compile. This should not be manually removed even when requests is also available in the common layer, as it's managed automatically by the dependency resolver.
Applied to files:
backend/compact-connect/lambdas/python/purchases/requirements.inbackend/compact-connect/lambdas/python/purchases/requirements.txtbackend/compact-connect/lambdas/python/purchases/requirements-dev.inbackend/compact-connect/lambdas/python/purchases/requirements-dev.txt
📚 Learning: 2025-07-22T03:36:17.137Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 907
File: backend/compact-connect/lambdas/python/purchases/requirements-dev.txt:15-0
Timestamp: 2025-07-22T03:36:17.137Z
Learning: In CompactConnect, requirements-dev.txt files for Lambda functions are used exclusively for running tests and development, not for actual Lambda runtime environments. Concerns about runtime compatibility (like OpenSSL versions) don't apply to these development dependency files.
Applied to files:
backend/compact-connect/lambdas/python/purchases/requirements.inbackend/compact-connect/lambdas/python/purchases/requirements.txtbackend/compact-connect/lambdas/python/purchases/requirements-dev.inbackend/compact-connect/lambdas/python/purchases/requirements-dev.txt
📚 Learning: 2025-08-21T16:36:48.723Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 1019
File: webroot/src/components/PrivilegeCard/PrivilegeCard.vue:270-278
Timestamp: 2025-08-21T16:36:48.723Z
Learning: In PrivilegeCard component's unencumber modal (webroot/src/components/PrivilegeCard/PrivilegeCard.vue), the unencumber-select wrapper element intentionally uses space key handling to catch bubbled events from child InputCheckbox elements for custom handling actions. When adverseAction.hasEndDate() is true, items show an inactive-category div and are designed to be non-interactive (not focusable), while items without end dates contain focusable InputCheckbox child elements. This design pattern is consistent with prior implementation and represents intentional UX behavior.
Applied to files:
webroot/src/components/PrivilegeCard/PrivilegeCard.vue
📚 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:
webroot/src/components/PrivilegeCard/PrivilegeCard.vuebackend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/record.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/record.py
📚 Learning: 2025-06-24T00:02:39.944Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 873
File: webroot/src/components/LicenseCard/LicenseCard.ts:414-443
Timestamp: 2025-06-24T00:02:39.944Z
Learning: In LicenseCard component's clickUnencumberItem method (webroot/src/components/LicenseCard/LicenseCard.ts), complex event handling for checkbox interactions is intentionally designed to ensure consistent behavior across checkbox input, wrapper label, and outer selection parent elements for custom UI requirements. This complexity should be preserved rather than simplified.
Applied to files:
webroot/src/components/PrivilegeCard/PrivilegeCard.vue
📚 Learning: 2025-08-12T19:49:24.999Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1001
File: backend/compact-connect/lambdas/python/disaster-recovery/requirements.in:1-1
Timestamp: 2025-08-12T19:49:24.999Z
Learning: In CompactConnect disaster-recovery Lambda functions, runtime dependencies like boto3, aws-lambda-powertools, and botocore are provided by lambda layers at deploy time rather than being specified in requirements.in files. The requirements.in file intentionally contains only a comment explaining this approach.
Applied to files:
backend/compact-connect/lambdas/python/purchases/requirements.txtbackend/compact-connect/lambdas/python/purchases/requirements-dev.inbackend/compact-connect/lambdas/python/purchases/requirements-dev.txt
📚 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:
webroot/src/models/AdverseAction/AdverseAction.model.spec.tsbackend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/record.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/record.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:
webroot/src/models/AdverseAction/AdverseAction.model.spec.tsbackend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/record.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/record.py
📚 Learning: 2026-01-07T21:20:07.472Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1243
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py:386-387
Timestamp: 2026-01-07T21:20:07.472Z
Learning: In backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py, the field clinicalPrivilegeActionCategories is required for encumbrance events in encumbranceDetails. The code should raise an exception if this field is missing rather than using defensive coding with .get(), as its absence indicates a data integrity issue that should be caught immediately with a fail-fast approach.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/record.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/record.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/schema/privilege/record.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/record.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/schema/privilege/record.py
📚 Learning: 2025-04-29T01:59:51.222Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 769
File: backend/compact-connect/stacks/api_stack/v1_api/api_model.py:397-401
Timestamp: 2025-04-29T01:59:51.222Z
Learning: In the CompactConnect project, validation constraints like enum values should be defined only in runtime code (Lambda) rather than duplicating them in CDK API schema definitions. This applies to fields like clinicalPrivilegeActionCategory and other similar enumerations.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/record.py
📚 Learning: 2025-09-03T22:27:56.621Z
Learnt from: ChiefStief
Repo: csg-org/CompactConnect PR: 1036
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/provider/api.py:71-71
Timestamp: 2025-09-03T22:27:56.621Z
Learning: The privilegeJurisdictions field in provider schemas consistently uses load_default=set() pattern across multiple schema files in the CompactConnect codebase (provider/record.py and provider/api.py), and developers prioritize maintaining this consistency.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/record.py
📚 Learning: 2025-12-16T21:43:07.408Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1219
File: backend/compact-connect/lambdas/python/search/handlers/search.py:131-140
Timestamp: 2025-12-16T21:43:07.408Z
Learning: In backend/compact-connect/lambdas/python/search/handlers/search.py, avoid logging the full request body. Do not log sensitive content by default. If logging is required for security investigations, redact or mask sensitive fields (e.g., credentials, tokens, PII) and log only safe metadata (method, path, status, user identifier). Use a secure, access-controlled audit log or feature flag to enable such logs, ensuring minimal exposure and compliance with security policies. This guideline targets Python backend handlers handling external requests and should be considered for similar files with request processing.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/record.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/record.py
📚 Learning: 2026-01-05T22:50:09.696Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1243
File: backend/compact-connect/tests/smoke/smoke_common.py:503-521
Timestamp: 2026-01-05T22:50:09.696Z
Learning: Enforce the rule: there is only one privilege record per provider per jurisdiction, and do not paginate when querying privilege records by jurisdiction. This applies across the codebase wherever privilege records are queried or tested (implementation and tests). Note that privilege update records can have multiple rows and require pagination. Implement checks and queries to assume a unique constraint for (provider, jurisdiction) on privilege records, and ensure any list endpoints or test smoke checks reflect no pagination for jurisdiction-based privilege queries while preserving pagination for privilege updates.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/record.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/record.py
📚 Learning: 2025-04-29T21:09:04.842Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 769
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/common.py:87-90
Timestamp: 2025-04-29T21:09:04.842Z
Learning: In the CCDataClass implementation, validation is performed during both the dump() and load() processes in Marshmallow, so calling dump() first still results in ValidationError if the provided data is invalid.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/record.py
📚 Learning: 2025-08-12T19:49:48.235Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1001
File: backend/compact-connect/lambdas/python/disaster-recovery/requirements.txt:1-6
Timestamp: 2025-08-12T19:49:48.235Z
Learning: The disaster-recovery Lambda functions in CompactConnect get their aws-lambda-powertools dependency from the shared lambda layer rather than individual requirements.txt files, which is why their requirements.txt files can be empty or header-only.
Applied to files:
backend/compact-connect/lambdas/python/purchases/requirements-dev.txt
📚 Learning: 2025-07-21T20:40:56.491Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 907
File: backend/compact-connect/lambdas/python/common/requirements.txt:7-0
Timestamp: 2025-07-21T20:40:56.491Z
Learning: In CompactConnect, there is only one lambda layer in use for Python lambdas, and this single layer manages the versions of aws-lambda-powertools, boto3, and botocore dependencies. This eliminates concerns about version skew across multiple lambda layers since all Python lambdas share the same dependency management through this single layer.
Applied to files:
backend/compact-connect/lambdas/python/purchases/requirements-dev.txt
🧬 Code graph analysis (1)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/record.py (3)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/__init__.py (2)
clinicalPrivilegeActionCategories(82-83)clinicalPrivilegeActionCategories(86-87)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/fields.py (1)
ClinicalPrivilegeActionCategoryField(125-127)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/jurisdiction/__init__.py (1)
required(17-18)
⏰ 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). (2)
- GitHub Check: TestApp
- GitHub Check: CheckWebroot
🔇 Additional comments (12)
webroot/src/models/AdverseAction/AdverseAction.model.spec.ts (3)
61-61: LGTM - Default case handled correctly.The test correctly validates that
getFirstNpdbTypeName()returns an empty string whennpdbTypesis an empty array.
185-185: LGTM - First element extraction validated.The test correctly validates that
getFirstNpdbTypeName()returns the first NPDB category from theclinicalPrivilegeActionCategoriesarray sent by the server. This aligns with the PR objective of migrating from the deprecated singular field to the array-based field while preserving display compatibility.
189-198: Good defensive test for invalid data.The test correctly validates graceful handling when the server sends an invalid data type (string instead of array). This defensive programming prevents potential runtime errors.
webroot/src/components/PrivilegeCard/PrivilegeCard.vue (2)
217-219: LGTM: Standardized on multi-select for NPDB categories.The unconditional use of
InputSelectMultiplecorrectly aligns with the migration to array-based NPDB categories. Removing the feature-flag gating simplifies the code and confirms the migration is complete.
393-393: No action required. ThegetFirstNpdbTypeName()method inAdverseAction.model.tsproperly handles empty arrays using optional chaining (this.npdbTypes?.[0]) with a fallback to an empty string, ensuring the method always returns a string value without risk of runtime errors.backend/compact-connect/lambdas/python/purchases/requirements-dev.in (1)
17-17: LGTM - Routine dependency update.The urllib3 constraint has been appropriately tightened to pick up the latest patch version, consistent with the security-motivated update in requirements.in.
backend/compact-connect/lambdas/python/purchases/requirements-dev.txt (1)
5-5: LGTM - Development dependency updates properly regenerated.The auto-generated file correctly reflects the requirements-dev.in changes, with multiple minor version bumps for development dependencies (aws-lambda-powertools, boto3, botocore, moto, ruff, urllib3, etc.). All updates are routine maintenance.
Also applies to: 11-11, 15-15, 19-19, 30-30, 40-40, 54-56, 81-81, 118-118, 150-150, 168-168
backend/compact-connect/lambdas/python/purchases/requirements.in (1)
4-4: LGTM - Security patch update.The urllib3 constraint has been appropriately tightened to ensure the vulnerability patch in 2.6.3 is picked up. The explicit transitive dependency management is a sound practice for security-critical libraries.
backend/compact-connect/lambdas/python/purchases/requirements.txt (1)
5-5: Approve dependency updates; urllib3 2.6.3 confirms security fixes.The auto-generated updates reflect the requirements.in changes correctly. The certifi update provides current root certificates, and urllib3 2.6.3 addresses documented vulnerabilities including CVE-2025-66418 (unbounded decompression-chain DoS) and CVE-2025-66471 (improper handling of highly compressed data).
Also applies to: 9-9, 21-24
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/record.py (2)
2-2: LGTM: Import cleanup is correct.Removing
pre_loadis appropriate since the migration hook that used this decorator has been removed. The remaining imports are still actively used in the schema.
47-50: LGTM: Method simplification improves clarity.Removing the migration logic from
pre_dump_serializationis appropriate now that the deprecated field has been removed. The method now has a single, clear responsibility: generating partition and sort keys for serialization.backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/record.py (1)
63-63: Change is safe and properly addresses data integrity requirements.Making
clinicalPrivilegeActionCategoriesrequired inEncumbranceDetailsSchemais validated. All code paths instantiating the schema populate this field fromAdverseActionRecordSchema, which also enforces the field as required. No breaking changes will occur.
...d/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/record.py
Show resolved
Hide resolved
jusdino
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Just one non-blocking question.
ChiefStief
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me!
|
@jlkravitz This is ready for your review. Thanks |
jlkravitz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one small question!
webroot/src/models/LicenseHistoryItem/LicenseHistoryItem.model.ts
Outdated
Show resolved
Hide resolved
This can be added back later if other migrations need to be performed
This was previously displaying the deprecated field, which we no longer support. This keeps the change compatible with the previous implementation by grabbing the first category from the list.
cc51f74 to
8c34f76
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (9)
backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_encumbrance.py (1)
43-51: Consider adding test coverage for multiple categories.The current tests only use a single category in the
clinicalPrivilegeActionCategorieslist. Since the migration changed from a single-value field to a list, consider adding a test case with multiple categories to ensure the handler correctly processes lists with more than one element.Example test body with multiple categories
def _generate_test_body_with_multiple_categories(): from cc_common.data_model.schema.common import ClinicalPrivilegeActionCategory, EncumbranceType return { 'encumbranceEffectiveDate': TEST_ENCUMBRANCE_EFFECTIVE_DATE, 'encumbranceType': EncumbranceType.SUSPENSION.value, 'clinicalPrivilegeActionCategories': [ ClinicalPrivilegeActionCategory.UNSAFE_PRACTICE.value, ClinicalPrivilegeActionCategory.FRAUD.value, ], }backend/compact-connect/tests/smoke/smoke_common.py (1)
4-4: Module-level import makes local import redundant.With
import timeadded at module level, the local import at line 405 insidewait_for_provider_creationis now redundant and can be removed.♻️ Suggested cleanup
def wait_for_provider_creation( staff_headers: dict, compact: str, given_name: str, family_name: str, max_wait_time: int = 300 ): """Poll for provider creation after license upload. ... """ - import time - logger.info(f'Waiting for provider creation for {given_name} {family_name}...')backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py (1)
380-388: Fail-fast is fine here, but add context (or a legacy fallback) to avoid opaque KeyErrors in privilege history.Right now a missing
encumbranceDetails['clinicalPrivilegeActionCategories']will raise a rawKeyErrorduring history construction. If that’s intentional, consider raising aCCInternalExceptionwith enough context (provider/privilege/update identifiers) so it’s actionable; if any legacy records can still exist, consider a fallback from the deprecated singular field for read-compat. Based on learnings, fail-fast is preferred when categories are required.Proposed fail-fast-with-context tweak
- event['npdbCategories'] = event['encumbranceDetails']['clinicalPrivilegeActionCategories'] + try: + event['npdbCategories'] = event['encumbranceDetails']['clinicalPrivilegeActionCategories'] + except KeyError as e: + raise CCInternalException( + 'Encumbrance history record missing clinicalPrivilegeActionCategories in encumbranceDetails' + ) from ebackend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py (2)
1517-1521: Good simplification; consider asserting categories are present to keep the “fail-fast” intent explicit.If
adverse_action.clinicalPrivilegeActionCategoriescan ever beNone(legacy record), you’ll serialize encumbranceDetails with a missing/invalid value and likely fail later. An explicit guard here would make the failure mode clearer. Based on learnings, fail-fast is preferred for missing categories.Example guard
now = config.current_standard_datetime + if not adverse_action.clinicalPrivilegeActionCategories: + raise CCInternalException('Missing clinicalPrivilegeActionCategories on adverse action') encumbrance_details = { 'clinicalPrivilegeActionCategories': adverse_action.clinicalPrivilegeActionCategories, 'adverseActionId': adverse_action.adverseActionId, }
3052-3058: EnsurelicenseJurisdictioncasing/shape is consistent with the rest of encumbranceDetails.If other code assumes lowercase jurisdictions inside encumbranceDetails, consider normalizing
licenseJurisdictionhere (e.g.,jurisdiction.lower()) to avoid mismatches during comparisons/filtering.webroot/src/network/licenseApi/data.api.ts (4)
267-284: Consider removing unusednpdbCategoryparameter.The
npdbCategory(singular) parameter is no longer used in the request payload since the feature flag has been removed and onlynpdbCategories(array) is sent. Consider removing this parameter from the method signature to avoid confusion.♻️ Suggested cleanup
public async encumberLicense( compact: string, licenseeId: string, licenseState: string, licenseType: string, encumbranceType: string, - npdbCategory: string, npdbCategories: Array<string>, startDate: string )Note: This would require updating all callers of this method.
344-372: Same cleanup opportunity forupdateLicenseInvestigation.The
encumbrance.npdbCategoryproperty in the method's optional encumbrance parameter is unused.♻️ Suggested cleanup
encumbrance?: { encumbranceType: string, - npdbCategory: string, npdbCategories: Array<string>, startDate: string }
409-426: Same cleanup opportunity forencumberPrivilege.The
npdbCategoryparameter is unused in the request payload.
486-514: Same cleanup opportunity forupdatePrivilegeInvestigation.The
encumbrance.npdbCategoryproperty is unused in the request payload.
jlkravitz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@isabeleliassen This is good to go!
We have several deprecated fields as well as deprecated logic that is now ready to be removed from the codebase.
Requirements List
Description List
Testing List
yarn test:unit:allshould run without errors or warningsyarn serveshould run without errors or warningsyarn buildshould run without errors or warningsbackend/compact-connect/tests/unit/test_api.pyrun compact-connect/bin/download_oas30.pyCloses #1241 #1136
Summary by CodeRabbit
Release Notes
New Features
Refactor
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.