Feat/significant investigative info #1167
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
WalkthroughAdds investigation support across backend: new investigation data models/schemas, create/close HTTP handlers, datastore transactions, event publishing and SQS listeners, email notification services (TS + Python), CDK/API wiring, many tests, snapshot/schema updates, and dependency bumps. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant API as Provider-Data API
participant DC as DataClient
participant EB as EventBridge
participant SQS as SQS Listener
participant EmailAPI as EmailServiceClient
participant EmailSrv as InvestigationEmailSvc
participant SES as SES
Client->>API: POST /v1/provider/.../investigations
API->>DC: create_investigation(investigation)
DC-->>API: persisted (investigationId)
API->>EB: publish_investigation_event(detail)
EB->>SQS: route event to investigation listener queue
SQS->>EmailAPI: send_*_investigation_state_notification(templateVars)
EmailAPI->>EmailSrv: invoke investigation notification lambda
EmailSrv->>EmailSrv: resolve recipients & render template
EmailSrv->>SES: send email
SES-->>EmailSrv: send response
sequenceDiagram
autonumber
participant Client
participant API as Provider-Data API
participant DC as DataClient
participant Enc as Encumbrance flow
participant EB as EventBridge
participant SQS as SQS Listener
Client->>API: PATCH /v1/provider/.../investigations/{id} (close [+encumbrance])
API->>DC: close_investigation(..., investigationId)
alt encumbrance requested
API->>Enc: create_encumbrance(payload)
Enc->>DC: persist adverse action (adverseActionId)
API->>DC: update investigation.resultingEncumbranceId
end
DC-->>API: success
API->>EB: publish_investigation_closed_event(detail)
EB->>SQS: route closed event to listeners
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (9)📓 Common learnings📚 Learning: 2025-09-11T20:06:40.709ZApplied to files:
📚 Learning: 2025-09-03T22:29:24.535ZApplied to files:
📚 Learning: 2025-09-11T20:06:40.709ZApplied to files:
📚 Learning: 2025-08-14T20:56:56.938ZApplied to files:
📚 Learning: 2025-08-19T19:39:47.790ZApplied to files:
📚 Learning: 2025-06-26T16:42:00.781ZApplied to files:
📚 Learning: 2025-10-24T21:40:48.912ZApplied to files:
📚 Learning: 2025-10-29T19:11:33.003ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (5)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 31
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/compact-connect/lambdas/python/provider-data-v1/handlers/encumbrance.py (1)
69-127: Remove unreachable None check and use.valuefor enum string conversionVerification confirms both concerns:
LicenseUtility.get_license_type_by_abbreviationraisesCCInvalidRequestExceptionon all error paths (lines 33 and 36) and never returnsNone—the subsequent None check is unreachable.
ClinicalPrivilegeActionCategoryFieldis a String field that validates against[entry.value for entry in ClinicalPrivilegeActionCategory], expecting string values. Assigning enum objects causes a type mismatch.Apply the suggested diff to remove the dead code block and use
.valueto store strings in both branches:@@ def _generate_adverse_action_for_record_type( - if not license_type: - raise CCInvalidRequestException( - f'Could not find license type information based on provided parameters ' - f"compact: '{compact}' licenseType: '{license_type_abbr}'" - ) @@ - adverse_action.clinicalPrivilegeActionCategories = [ - ClinicalPrivilegeActionCategory(adverse_action_post_body['clinicalPrivilegeActionCategory']) - ] + adverse_action.clinicalPrivilegeActionCategories = [ + ClinicalPrivilegeActionCategory( + adverse_action_post_body['clinicalPrivilegeActionCategory'] + ).value + ] @@ - adverse_action.clinicalPrivilegeActionCategories = adverse_action_post_body[ - 'clinicalPrivilegeActionCategories' - ] + # Ensure list is of strings even if schema returns enums + categories = adverse_action_post_body['clinicalPrivilegeActionCategories'] + adverse_action.clinicalPrivilegeActionCategories = [ + c.value if isinstance(c, ClinicalPrivilegeActionCategory) else c for c in categories + ]
🧹 Nitpick comments (36)
backend/compact-connect/tests/resources/snapshots/PATCH_LICENSE_INVESTIGATION_REQUEST_SCHEMA.json (1)
10-10: Minor: Simplify redundant regex quantifier.Line 10 uses
[12]{1}to match the year's first digit; the explicit{1}quantifier is redundant since character classes match exactly one character by default.- "pattern": "^[12]{1}[0-9]{3}-(0[1-9]|1[0-2])-(0[1-9]|[12][0-9]|3[01])$", + "pattern": "^[12][0-9]{3}-(0[1-9]|1[0-2])-(0[1-9]|[12][0-9]|3[01])$",backend/compact-connect/bin/run_python_tests.py (1)
36-37: Good operational improvement for test isolation.Moving the Authorize.net tests to run last is a sensible approach to prevent runtime contamination from special dependencies. The script's
clean_modules()function provides additional isolation between test suites.Optional enhancement: Consider making the comment more specific about what "special dependencies" means for future maintainers:
- # Save Authorize.net tests for last, since special dependencies make that runtime different + # Save Authorize.net tests for last, since Authorize.net SDK dependencies can affect module state 'lambdas/python/purchases',backend/compact-connect/lambdas/python/common/requirements-dev.in (1)
1-1: Consider specifying explicit Moto extras instead of[all].Using
moto[all]pulls in dependencies for every AWS service Moto supports, which significantly increases installation time and dependency footprint. For better clarity and performance, explicitly list only the AWS services needed for testing.Based on the investigation system being added in this PR, you likely need extras like
[dynamodb,s3,ses,sns,sqs,events]or similar.Apply this diff if you can identify the specific services needed:
-moto[all]>=5.0.12, <6 +moto[dynamodb,s3,ses,sns,sqs,events]>=5.0.12, <6(Adjust the list based on actual AWS services being mocked in your tests)
backend/compact-connect/tests/app/test_api/test_investigation_api.py (2)
88-151: Reduce duplication in traversal helpers
_get_privilege_license_type_param_resource_idand_get_license_license_type_param_resource_idare nearly identical. Factor into a single helper parameterized by root path ('privileges' | 'licenses') to cut maintenance.Example:
def _get_license_type_param_resource_id(self, api_stack_template, api_stack, root: str) -> str: provider_resource = api_stack.api.v1_api.provider_management.provider_resource.node.default_child root_id = next(iter(api_stack_template.find_resources( type=CfnResource.CFN_RESOURCE_TYPE_NAME, props={'Properties': {'ParentId': {'Ref': api_stack.get_logical_id(provider_resource)}, 'PathPart': root}}, ))) jurisdiction_id = next(iter(api_stack_template.find_resources( type=CfnResource.CFN_RESOURCE_TYPE_NAME, props={'Properties': {'ParentId': {'Ref': root_id}, 'PathPart': 'jurisdiction'}}, ))) jurisdiction_param_id = next(iter(api_stack_template.find_resources( type=CfnResource.CFN_RESOURCE_TYPE_NAME, props={'Properties': {'ParentId': {'Ref': jurisdiction_id}, 'PathPart': '{jurisdiction}'}}, ))) license_type_id = next(iter(api_stack_template.find_resources( type=CfnResource.CFN_RESOURCE_TYPE_NAME, props={'Properties': {'ParentId': {'Ref': jurisdiction_param_id}, 'PathPart': 'licenseType'}}, ))) return next(iter(api_stack_template.find_resources( type=CfnResource.CFN_RESOURCE_TYPE_NAME, props={'Properties': {'ParentId': {'Ref': license_type_id}, 'PathPart': '{licenseType}'}}, )))Then wrap:
def _get_privilege_license_type_param_resource_id(...): return self._get_license_type_param_resource_id(..., 'privileges') def _get_license_license_type_param_resource_id(...): return self._get_license_type_param_resource_id(..., 'licenses')Also applies to: 153-215
37-38: Tiny nit: simplify dict key extractionUse
next(iter(d))instead ofnext(key for key in d.keys())for brevity.Also applies to: 53-55, 69-71, 85-87, 101-104, 115-116, 126-128, 138-140, 150-152, 165-167, 177-179, 189-191, 201-203, 213-215
backend/compact-connect/docs/postman/postman-collection.json (3)
824-824: Use valid sample dates to prevent misleading examplesSeveral examples contain impossible dates (e.g., "1608-02-30", "birthMonthDay": "09-00"). They may confuse users copying/pasting into tests.
Replace with valid values (e.g., "1608-02-29" in leap years, "09-01").
Also applies to: 961-961
151-168: Avoid hard‑coding authorization code in token exchangeThe authorization-code-grant-token request includes a fixed code that will expire.
Use a variable (e.g., {{authCode}}) populated by the authorize/login steps, or guide users to paste the code manually via a pre‑request script.
1075-1078: Make upload file path configurableHard-coded src ("AmVhGkArk/octp-nc-mock-data.csv") will break most environments.
Use a Postman variable (e.g., {{uploadFile}}) or leave file selection to the user.
- "src": "AmVhGkArk/octp-nc-mock-data.csv", + "src": "{{uploadFile}}",backend/compact-connect/tests/app/test_notification_stack.py (3)
749-819: Add queue properties and ESM asserts for privilege investigation listenerCurrently only handler and rule are asserted.
Use the same assertions as other tests to check:
- SQS queue props for privilege_investigation_listener_queue_logical_id (strip KmsMasterKeyId).
- ESM pointing to that queue and the handler logical ID (PrivilegeInvestigationNotificationListener).
821-893: Add queue properties and ESM asserts for privilege investigation closed listenerMirror the license closed assertions for consistency and regressions defense.
Propose identical pattern: assert queue props and ESM using privilege_investigation_closed_listener_queue_logical_id and privilege_investigation_closed_handler_logical_id.
677-747: Add queue properties and event source mapping assertions for parityThis test validates the rule but not the SQS queue properties or the SQS→Lambda event source mapping, unlike other listeners. Apply the suggested pattern to complete the test coverage.
backend/compact-connect/lambdas/python/provider-data-v1/handlers/encumbrance.py (2)
164-183: Surface created adverseActionId in logs (optional)Consider logging the returned ID for traceability (kept out of API response).
- _create_privilege_encumbrance_internal( + _id = _create_privilege_encumbrance_internal( @@ - return {'message': 'OK'} + logger.info('Privilege encumbrance created', extra={'adverseActionId': _id}) + return {'message': 'OK'}
220-238: Mirror optional ID logging for license encumbranceSame as privilege: log the created ID for ops debugging.
- _create_license_encumbrance_internal( + _id = _create_license_encumbrance_internal( @@ - return {'message': 'OK'} + logger.info('License encumbrance created', extra={'adverseActionId': _id}) + return {'message': 'OK'}backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/license/record.py (1)
34-42: Avoid duplicating InvestigationDetailsSchema; centralize once.This schema appears in both license and privilege modules. Move it to a shared module (e.g., cc_common.data_model.schema.common or fields-specific package) and import from there to prevent drift.
backend/compact-connect/lambdas/python/common/tests/unit/test_investigation_event_bus_client.py (1)
48-53: Avoid hard-coding EventBusName.Use config.event_bus_name to make tests resilient across environments.
Example:
from cc_common.config import config expected_event = { 'Source': 'test.source', 'DetailType': 'privilege.investigation', 'EventBusName': config.event_bus_name, }Also applies to: 102-108, 158-164, 214-218
backend/compact-connect/lambdas/python/common/common_test/test_data_generator.py (1)
154-180: Remove unknown createDate; schema expects creationDate.InvestigationRecordSchema requires creationDate (DateTime). createDate isn’t a field for this record and will be ignored; keeping it invites confusion.
Apply this diff:
default_investigation = { 'providerId': DEFAULT_PROVIDER_ID, 'compact': DEFAULT_COMPACT, 'type': 'investigation', 'jurisdiction': DEFAULT_PRIVILEGE_JURISDICTION, 'licenseTypeAbbreviation': DEFAULT_LICENSE_TYPE_ABBREVIATION, 'licenseType': DEFAULT_LICENSE_TYPE, 'investigationAgainst': DEFAULT_INVESTIGATION_AGAINST_PRIVILEGE, - 'createDate': date.fromisoformat(DEFAULT_INVESTIGATION_START_DATE), 'submittingUser': DEFAULT_AA_SUBMITTING_USER_ID, 'creationDate': datetime.fromisoformat(DEFAULT_DATE_OF_UPDATE_TIMESTAMP), 'investigationId': DEFAULT_INVESTIGATION_ID, }Optionally also drop licenseTypeAbbreviation here; the schema derives it from licenseType.
backend/compact-connect/lambdas/nodejs/tests/email-notification-service.test.ts (1)
1448-2080: Reduce duplication with small helpers.Extract tiny helpers to build events and assert SES subject/body fragments to cut repeated boilerplate and ease future changes.
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/investigation/record.py (1)
38-41: Use Date for closeDate to match model and semantics.InvestigationData.closeDate is a date; schema uses DateTime. Align to Date.
Apply this diff:
-from marshmallow.fields import UUID, DateTime, String +from marshmallow.fields import UUID, Date, DateTime, String @@ - closeDate = DateTime(required=False, allow_none=False) + closeDate = Date(required=False, allow_none=False)backend/compact-connect/lambdas/nodejs/email-notification-service/lambda.ts (1)
369-528: Implementation verified with test coverage for all eight templates.Tests exist for each template in both
investigation-notification-service.test.ts(service tests) andemail-notification-service.test.ts(lambda handler tests). All eight investigation notification cases are properly tested.The implementation is correct. For future maintenance, consider extracting a small guard helper to reduce repetition of the "missing template variables" checks—though this is optional since the current code is clear and functional.
backend/compact-connect/lambdas/python/data-events/tests/function/test_investigation_events.py (1)
149-170: DRY up expected InvestigationNotificationTemplateVariables constructionThe repeated expected_template_variables blocks are identical apart from investigation_jurisdiction; consider a tiny helper to reduce duplication and cut maintenance cost.
Also applies to: 305-325, 411-431, 521-541
backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_investigation.py (3)
121-125: Avoid hard‑coded licenseTypeAbbreviation ("slp") in SK prefixesUse the record’s abbreviation to avoid brittle tests tied to fixtures.
- & Key('sk').begins_with( - f'{test_privilege_record.compact}#PROVIDER#privilege/{test_privilege_record.jurisdiction}/slp#INVESTIGATION' - ), + & Key('sk').begins_with( + f'{test_privilege_record.compact}#PROVIDER#privilege/' + f'{test_privilege_record.jurisdiction}/{test_privilege_record.licenseTypeAbbreviation}#INVESTIGATION' + ),And similarly for license tests:
- & Key('sk').begins_with( - f'{test_license_record.compact}#PROVIDER#license/{test_license_record.jurisdiction}/slp#INVESTIGATION' - ), + & Key('sk').begins_with( + f'{test_license_record.compact}#PROVIDER#license/' + f'{test_license_record.jurisdiction}/{test_license_record.licenseTypeAbbreviation}#INVESTIGATION' + ),Also applies to: 141-146
31-34: Remove unused constantsTEST_INVESTIGATION_START_DATE and TEST_INVESTIGATION_CLOSE_DATE are declared but unused.
679-688: Strengthen encumbrance assertionsAlso assert encumbrance fields (e.g., effectiveStartDate equals TEST_ENCUMBRANCE_EFFECTIVE_DATE) to better validate close-with-encumbrance behavior.
Also applies to: 689-701
backend/compact-connect/lambdas/nodejs/tests/lib/email/investigation-notification-service.test.ts (2)
68-83: Prevent template capture write failuresWhen CAPTURE_EMAIL_TEMPLATES=true, EmailTemplateCapture.ensureOutputDirectory doesn’t create the directory before writeFileSync. Add fs.mkdirSync(outputDir, { recursive: true }) in ensureOutputDirectory to avoid intermittent failures. See tests/utils/email-template-capture.ts.
119-151: Reduce duplicated test scaffoldingConsider test.each for provider/state/closed variants to cut duplication and ease future template text updates.
Also applies to: 218-263, 300-345, 382-427, 429-461
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py (1)
1682-1694: Add idempotency guard when creating investigation recordTo avoid overwriting on retries, add a condition to the Put for the investigation item.
{ 'Put': { 'TableName': self.config.provider_table.table_name, - 'Item': TypeSerializer().serialize(investigation.serialize_to_database_record())['M'], + 'Item': TypeSerializer().serialize(investigation.serialize_to_database_record())['M'], + 'ConditionExpression': 'attribute_not_exists(pk)', } },backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py (3)
12-19: Import InvestigationAgainstEnum to avoid stringly-typed checksUse the enum’s values for filter comparisons below.
from cc_common.data_model.schema.common import ( ActiveInactiveStatus, AdverseActionAgainstEnum, CompactEligibilityStatus, HomeJurisdictionChangeStatusEnum, + InvestigationAgainstEnum, PrivilegeEncumberedStatusEnum, UpdateCategory, )
620-627: Use enum value instead of hardcoded string for investigationAgainstPrevents typos and keeps parity with schema.
- if record.investigationAgainst == 'privilege' + if record.investigationAgainst == InvestigationAgainstEnum.PRIVILEGE.value
639-646: Use enum value instead of hardcoded string for investigationAgainstSame rationale as above.
- if record.investigationAgainst == 'license' + if record.investigationAgainst == InvestigationAgainstEnum.LICENSE.valuebackend/compact-connect/lambdas/python/data-events/handlers/investigation_events.py (1)
82-100: Typing: ProviderNotificationMethod is provider-specific; state methods have a different signatureThese helpers accept both provider and state email methods. Either use a broader Callable or define a StateNotificationMethod Protocol. Functional behavior is fine; this is to reduce type-check noise.
Example adjustment:
-from cc_common.email_service_client import InvestigationNotificationTemplateVariables, ProviderNotificationMethod +from typing import Callable, Any +from cc_common.email_service_client import InvestigationNotificationTemplateVariables ... -def _send_primary_state_notification( - notification_method: ProviderNotificationMethod, +def _send_primary_state_notification( + notification_method: Callable[..., dict[str, Any]], ... -def _send_additional_state_notifications( - notification_method: ProviderNotificationMethod, +def _send_additional_state_notifications( + notification_method: Callable[..., dict[str, Any]],Also applies to: 117-139
backend/compact-connect/lambdas/python/common/tests/function/test_data_client.py (2)
1068-1141: Reduce brittleness of full-structure assertions.Asserting the entire update record dictionary is fragile across non-functional changes. Prefer targeted checks for invariant fields and factor a small helper to strip dynamic/optional keys (dateOfUpdate, sk, compactTransactionIdGSIPK) and then assert a subset. This will cut noisy test churn.
1413-1457: Centralize “closure update” matching and normalization.You scan update records for updateType == 'investigation' and removedValues containing 'investigationStatus', then pop dynamic keys. Extract this into a reusable helper used by both license and privilege closure tests to avoid duplication and ensure consistent normalization.
backend/compact-connect/lambdas/python/common/cc_common/email_service_client.py (1)
612-669: DRY up investigation email senders.Eight new methods differ mainly by template name, recipientType, and jurisdiction presence. Add a private helper like _send_investigation_email(..., to_state: bool, closed: bool, privilege: bool) to cut duplication and ensure consistent handling of optional providerId.
Also applies to: 671-699, 730-787, 789-846
backend/compact-connect/tests/smoke/investigation_smoke_tests.py (2)
451-456: Ensure staff user has scopes for target jurisdictions.License tests may operate on provider licenseJurisdiction (often 'oh'); with only ne/co/ky, authorization can 403. Add 'oh' (or derive from provider data) to avoid flakiness.
- permissions={'actions': {'admin'}, 'jurisdictions': {'ne': {'admin'}, 'co': {'admin'}, 'ky': {'admin'}}}, + permissions={ + 'actions': {'admin'}, + 'jurisdictions': {'ne': {'admin'}, 'co': {'admin'}, 'ky': {'admin'}, 'oh': {'admin'}}, + },
260-264: Replace fixed sleeps with polling to reduce flakes.Use a small wait loop polling DynamoDB/API until condition met or timeout. This improves stability under eventual consistency.
Also applies to: 295-299, 330-334, 365-369, 417-421
backend/compact-connect/lambdas/nodejs/lib/email/investigation-notification-service.ts (1)
87-460: Consider reducing duplication across notification methods.The eight public methods follow very similar patterns with only terminology differences (license/privilege, investigation/investigation closed, provider/state). Consider extracting common logic into helper methods or using a template method pattern to reduce duplication and improve maintainability.
For example, you could introduce:
- A method to build provider notification emails with parameterized text
- A method to build state notification emails with parameterized text
- Enums or constants for notification types (license/privilege, opened/closed)
This would reduce the ~370 lines of highly similar code to a more maintainable structure.
backend/compact-connect/lambdas/nodejs/lib/email/investigation-notification-service.ts
Outdated
Show resolved
Hide resolved
backend/compact-connect/lambdas/nodejs/lib/email/investigation-notification-service.ts
Outdated
Show resolved
Hide resolved
backend/compact-connect/lambdas/nodejs/lib/email/investigation-notification-service.ts
Outdated
Show resolved
Hide resolved
backend/compact-connect/lambdas/nodejs/lib/email/investigation-notification-service.ts
Outdated
Show resolved
Hide resolved
backend/compact-connect/tests/resources/snapshots/GET_PROVIDER_RESPONSE_SCHEMA.json
Show resolved
Hide resolved
…nt-investigative-info
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
backend/compact-connect/stacks/api_stack/v1_api/api_model.py (1)
1385-1403: Add missing investigation update types to API schema enumThe API schema at lines 1385-1403 is missing 'investigation' and 'closingInvestigation' enum values, even though the data model defines these update types (UpdateCategory.INVESTIGATION and UpdateCategory.CLOSING_INVESTIGATION) and the backend actively generates them. These values will flow through the privilege history endpoints and fail API schema validation. Include both to maintain consistency between the data model and API contract.
def _update_type_schema(self) -> JsonSchema: return JsonSchema( type=JsonSchemaType.STRING, enum=[ 'deactivation', 'expiration', 'issuance', 'other', 'renewal', 'encumbrance', + 'investigation', + 'closingInvestigation', 'homeJurisdictionChange', 'registration', 'lifting_encumbrance', # this is specific to privileges that are deactivated due to a state license deactivation, 'licenseDeactivation', 'emailChange', ], )backend/compact-connect/tests/resources/snapshots/PROVIDER_USER_RESPONSE_SCHEMA.json (1)
866-889: AddbirthMonthDayproperty definition to licenses items schema.
birthMonthDayis listed as required but not defined in the object's properties. While technically allowed (sinceadditionalPropertiesdefaults to true), all 22 other required fields are explicitly defined. AddbirthMonthDayas a property for consistency and clarity.
♻️ Duplicate comments (2)
backend/compact-connect/docs/postman/postman-collection.json (1)
427-427:⚠️ Replace realistic SSNs/phones in examples with unmistakably fake placeholdersThis is a reiteration of a prior review comment. The Postman collection still contains realistic-looking PII in sample payloads:
- Lines 427, 502, 563:
"ssn": "516-71-4118","phoneNumber": "+411669509084"- Line 824 onwards:
"phoneNumber": "+46030480739","+88477899480"Even if these are synthetic, using real-looking values in documentation creates a compliance risk. Replace all with unmistakably fake placeholders (e.g., SSN
"000-00-0000", phone"+15555550123", email"example@example.test"). Prefer showing onlyssnLastFourwhen the full SSN is not required for the example.Also applies to: 502-502, 563-563, 824-824, 844-844
backend/compact-connect/tests/app/test_api/test_investigation_api.py (1)
301-302: Fix: call the authorizer method (missing parentheses) across all test files, and memoize in api.py
staff_users_authorizeris a method; accessing.nodewithout calling it will fail. Apply the fix to all 22 occurrences across 5 test files:
- test_compact_api.py: line 151 (1 occurrence)
- test_compact_configuration_api.py: lines 69, 233, 284, 349, 401 (6 occurrences)
- test_investigation_api.py: lines 301, 340, 390, 429 (4 occurrences)
- test_staff_users_api.py: lines 88, 158, 297 (3 occurrences)
- test_provider_management_api.py: lines 232, 296, 372, 498, 596, 651, 719, 787 (8 occurrences)
Apply this diff pattern to all occurrences:
- 'Ref': api_stack.get_logical_id(api_stack.api.staff_users_authorizer.node.default_child), + 'Ref': api_stack.get_logical_id(api_stack.api.staff_users_authorizer().node.default_child),Also implement memoization in backend/compact-connect/stacks/api_stack/api.py to prevent duplicate constructs:
def staff_users_authorizer(self): - from aws_cdk.aws_apigateway import CognitoUserPoolsAuthorizer - - return CognitoUserPoolsAuthorizer( - self, 'StaffUsersPoolAuthorizer', cognito_user_pools=[self._persistent_stack.staff_users] - ) + from aws_cdk.aws_apigateway import CognitoUserPoolsAuthorizer + if hasattr(self, '_staff_users_authorizer'): + return self._staff_users_authorizer + self._staff_users_authorizer = CognitoUserPoolsAuthorizer( + self, 'StaffUsersPoolAuthorizer', cognito_user_pools=[self._persistent_stack.staff_users] + ) + return self._staff_users_authorizer
🧹 Nitpick comments (28)
backend/compact-connect/lambdas/python/provider-data-v1/tests/function/__init__.py (1)
25-30: LGTM! Clean test helper for partial dictionary matching.The implementation correctly validates that all keys in
expectedexist inactualwith matching values. This partial-matching behavior (not requiringactualto have only the keys inexpected) is appropriate for flexible test assertions on investigation data structures.Optional refinements to consider:
- Add null safety to provide clearer error messages:
def assertDictFieldsMatch(self, expected: dict, actual: dict): # noqa: N802 emulating TestCase style here + if expected is None or actual is None: + self.fail(f'Cannot compare None values: expected={expected}, actual={actual}') for key, value in expected.items():
- Simplify the error message to reduce redundancy:
- self.assertEqual(value, actual[key], f'Expected {key}: {value} but got {key}: {actual[key]}') + self.assertEqual(value, actual[key], f'Mismatch for key "{key}"')backend/compact-connect/tests/smoke/investigation_smoke_tests.py (1)
315-321: Minor inconsistency in parameter ordering.The PATCH requests have inconsistent keyword argument ordering: lines 315-321 use
jsonbeforeheaders, while lines 350-356 useheadersbeforejson. While this doesn't affect functionality, consistent ordering improves readability.For consistency, consider using the same parameter order for all requests:
response = requests.patch( f'{config.api_base_url}/v1/compacts/{compact}/providers/{provider_id}/licenses/jurisdiction/{jurisdiction}' f'/licenseType/{license_type_abbreviation}/investigation/{investigation_id}', - headers=auth_headers, json={}, + headers=auth_headers, timeout=30, )Also applies to: 350-356
backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py (1)
617-654: Consider using enum values instead of string literals.The methods use hard-coded string comparisons
'privilege'and'license'for theinvestigationAgainstfield. SinceInvestigationAgainstEnumis defined in the codebase (as shown in the relevant code snippets), consider using enum values for type safety:def get_investigation_records_for_privilege( self, privilege_jurisdiction: str, privilege_license_type_abbreviation: str, filter_condition: Callable[[InvestigationData], bool] | None = None, ) -> list[InvestigationData]: """ Get all investigation records for a given privilege. """ return [ record for record in self._investigation_records - if record.investigationAgainst == 'privilege' + if record.investigationAgainst == InvestigationAgainstEnum.PRIVILEGE.value and record.jurisdiction == privilege_jurisdiction and record.licenseTypeAbbreviation == privilege_license_type_abbreviation and record.closeDate is None # Only return active investigations and (filter_condition is None or filter_condition(record)) ]Apply similar changes to
get_investigation_records_for_license.backend/compact-connect/stacks/api_lambda_stack/provider_management.py (1)
61-92: Permissions and Nag reason look right; unify Nag call style
- Grants are minimal and correct: provider r/w, staff users read, event bus put. Nag reason accurately reflects scope.
- Minor style nit: other handlers use Stack.of(handler.role) for the Nag call; do the same here for consistency.
Apply:
- NagSuppressions.add_resource_suppressions_by_path( - self.stack, + NagSuppressions.add_resource_suppressions_by_path( + Stack.of(handler.role), path=f'{handler.role.node.path}/DefaultPolicy/Resource',backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/investigation/record.py (1)
41-55: Clarify lowercase comment or normalize inputThe “ensure this is passed in lowercase” comment is misleading—no normalization occurs. Either remove/adjust the comment or normalize as needed to match config keys.
- # ensure this is passed in lowercase + # licenseType must match keys in config.license_type_abbreviations for the given compactbackend/compact-connect/lambdas/python/common/tests/unit/test_investigation_event_bus_client.py (1)
66-74: Also assert eventTime equals the passed timestampYou’re dropping eventTime for equality, but it’s deterministic (from create_date/close_date). Assert it matches to catch regressions.
Example for the first test:
- actual_detail = json.loads(actual_event['Detail']) - actual_detail.pop('eventTime') + actual_detail = json.loads(actual_event['Detail']) + self.assertEqual('2024-02-15T12:00:00+00:00', actual_detail['eventTime']) + actual_detail.pop('eventTime')Repeat similarly for the other three tests using the corresponding timestamps.
Also applies to: 123-131, 180-188, 237-245
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/license/record.py (2)
35-43: Deduplicate InvestigationDetailsSchemaThis schema also exists under privilege/record.py. Move to a shared module (e.g., cc_common.data_model.schema.investigation.common) and import from both to avoid drift.
237-245: Validator logic — LGTM; consider name/coverage tweak
- Logic is correct. Rename to validate_investigation_details_present_if_investigation_update for clarity.
- Optional: also require investigationDetails for UpdateCategory.CLOSING_INVESTIGATION if your history emits that type.
- if data['updateType'] == UpdateCategory.INVESTIGATION and not data.get('investigationDetails'): + if data['updateType'] in (UpdateCategory.INVESTIGATION, UpdateCategory.CLOSING_INVESTIGATION) \ + and not data.get('investigationDetails'):backend/compact-connect/stacks/api_stack/v1_api/api_model.py (4)
1404-1434: Docstring nit: clarify where this schema is usedIt’s used by encumbrance endpoints and patch investigation requests. Reword to avoid implying POST investigations use it.
- """Common schema for encumbrance request data used in both POST and PATCH investigation endpoints""" + """Common schema for encumbrance request data used by encumbrance endpoints and PATCH investigation endpoints"""
1460-1491: Expose closure fields on investigation response objectsClosed investigations may include closeDate, closingUser, resultingEncumbranceId. Add as optional properties.
return JsonSchema( type=JsonSchemaType.OBJECT, required=[ 'type', 'compact', 'providerId', 'investigationId', 'jurisdiction', 'licenseType', 'dateOfUpdate', 'creationDate', 'submittingUser', ], properties={ 'type': JsonSchema(type=JsonSchemaType.STRING, enum=['investigation']), 'compact': JsonSchema(type=JsonSchemaType.STRING, enum=self.stack.node.get_context('compacts')), 'providerId': JsonSchema(type=JsonSchemaType.STRING, pattern=cc_api.UUID4_FORMAT), 'investigationId': JsonSchema(type=JsonSchemaType.STRING), 'jurisdiction': JsonSchema( type=JsonSchemaType.STRING, enum=self.stack.node.get_context('jurisdictions'), ), 'licenseType': JsonSchema(type=JsonSchemaType.STRING), 'dateOfUpdate': JsonSchema(type=JsonSchemaType.STRING, format='date-time'), 'creationDate': JsonSchema(type=JsonSchemaType.STRING, format='date-time'), 'submittingUser': JsonSchema(type=JsonSchemaType.STRING), + 'closeDate': JsonSchema(type=JsonSchemaType.STRING, format='date-time'), + 'closingUser': JsonSchema(type=JsonSchemaType.STRING), + 'resultingEncumbranceId': JsonSchema(type=JsonSchemaType.STRING), }, )
2822-2837: Lock down POST investigation request bodiesThese endpoints don’t need a body. Set additional_properties=False to reject stray payloads.
- schema=JsonSchema( - type=JsonSchemaType.OBJECT, - properties={}, - ), + schema=JsonSchema( + type=JsonSchemaType.OBJECT, + additional_properties=False, + ),Also applies to: 2838-2853
2854-2871: Constrain PATCH investigation request bodiesAllow only the encumbrance object; forbid extras.
- schema=JsonSchema( - type=JsonSchemaType.OBJECT, - properties={ - 'encumbrance': self._encumbrance_request_schema, - }, - ), + schema=JsonSchema( + type=JsonSchemaType.OBJECT, + additional_properties=False, + properties={ + 'encumbrance': self._encumbrance_request_schema, + }, + ),Also applies to: 2872-2888
backend/compact-connect/tests/app/base.py (3)
68-70: Good addition; minor polish.Init at class setup makes sense. Consider a type annotation for clarity:
- cls._overwrite_snapshots: bool = False
74-84: Broaden boolean parsing for OVERWRITE_SNAPSHOTS.Accept more truthy values ('1', 'yes', 'on') to reduce CI friction.
- cls._overwrite_snapshots = os.environ.get('OVERWRITE_SNAPSHOTS', 'false').lower() == 'true' + val = os.environ.get('OVERWRITE_SNAPSHOTS', 'false').strip().lower() + cls._overwrite_snapshots = val in ('1', 'true', 'yes', 'y', 'on')
624-649: Honor env in message and ensure snapshot dir exists.
- Create parent directory before writing.
- Update assertion message to mention OVERWRITE_SNAPSHOTS.
if snapshot != actual and overwrite_snapshot: - with open(snapshot_path, 'w') as f: + os.makedirs(os.path.dirname(snapshot_path), exist_ok=True) + with open(snapshot_path, 'w') as f: json.dump(actual, f, indent=2) # So the data files will end with a newline f.write('\n') sys.stdout.write(f"Snapshot '{snapshot_name}' has been overwritten.") else: self.maxDiff = None # pylint: disable=invalid-name,attribute-defined-outside-init self.assertEqual( snapshot, actual, f"Snapshot '{snapshot_name}' does not match the actual data. " - 'To overwrite the snapshot, set overwrite_snapshot=True.', + 'To overwrite, set overwrite_snapshot=True or export OVERWRITE_SNAPSHOTS=true.', )backend/compact-connect/lambdas/nodejs/tests/lib/email/investigation-notification-service.test.ts (1)
119-597: Reduce duplication via parameterized helpers.Provider/state open/closed tests repeat similar expectations. Extract a small helper to assert SES payloads and loop over variants to trim LOC.
backend/compact-connect/lambdas/nodejs/email-notification-service/lambda.ts (1)
370-529: <!-- [scratchpad_start]
[task verify_review_comment_consistency_and_repetition][observations]
- File: backend/compact-connect/lambdas/nodejs/email-notification-service/lambda.ts
- Review comment claims 8 new cases (lines 370-529) have duplicate validation and error message inconsistencies
- Script output confirms presence of messages in the target range:
- Lines 378, 398, 418, 438, 458, 478, 498, 518: "Missing required template variables for ... template." (with/without period variations)
- Lines 391, 431, 471, 511: "No jurisdiction provided for ... notification email" (differs from earlier pattern)
- Earlier code (lines 97, 134, 177) shows different jurisdiction error patterns
[analysis]
- Error message inconsistency confirmed:
- Jurisdiction checks use "No jurisdiction provided" (lines 391, 431, 471, 511) vs "Missing required jurisdiction field" (lines 134, 177)
- Template variable checks show formatting variations (period vs no period, phrasing)
- All 8 investigation cases (4 license + 4 privilege, each with provider/state notifications) contain repetitive validation blocks
- Refactoring suggestion is valid and would reduce code duplication
- This is an optional refactoring suggestion, not a blocking issue
[done]
Verification complete. Review comment's core observations about repetition and inconsistency are accurate. The suggested refactoring is reasonable.
[scratchpad_end]
-->Reduce repetition and standardize error messages across investigation notification cases.
The analysis confirms significant code duplication and message inconsistency. Across the eight investigation cases (lines 370–529), identical template validation blocks repeat, and jurisdiction error messages vary—some say "No jurisdiction provided" while earlier code uses "Missing required jurisdiction field." A helper function to centralize validation and enforce uniform error formatting would improve maintainability and prevent drift. The suggested helper and consistent messaging approach is sound.
backend/compact-connect/lambdas/python/common/cc_common/event_bus_client.py (1)
426-426: Inconsistent enum usage in detail_type construction.Line 378 constructs
detail_typewithout.value(relying on StrEnum's string representation), but this line uses.value. While both work correctly with StrEnum, using a consistent approach improves code clarity.Apply this diff for consistency with line 378:
- detail_type = f'{investigation_against.value}.investigationClosed' + detail_type = f'{investigation_against}.investigationClosed'backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py (1)
1858-1858: UpdateExpression clauses should follow DynamoDB convention (SET before REMOVE).While DynamoDB accepts clauses in any order, the conventional and documented order is SET, REMOVE, ADD, DELETE. Following this convention improves readability and consistency with other update expressions in the codebase.
Apply this diff to reorder the clauses:
- 'UpdateExpression': 'REMOVE investigationStatus SET dateOfUpdate = :dateOfUpdate', + 'UpdateExpression': 'SET dateOfUpdate = :dateOfUpdate REMOVE investigationStatus',backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_investigation.py (1)
174-193: Reduce brittleness: avoid relying on index [0] when asserting investigationsConsider selecting the relevant privilege/license by jurisdiction/licenseType instead of assuming index 0.
-privilege = provider_data['privileges'][0] +privilege = next(p for p in provider_data['privileges'] if p['jurisdiction']==test_privilege_record.jurisdiction and p['licenseType']==test_privilege_record.licenseType)Also applies to: 375-394, 1019-1040, 1284-1299
backend/compact-connect/tests/resources/snapshots/GET_PROVIDER_RESPONSE_SCHEMA.json (2)
678-680: Constrain investigationId to UUIDSchema currently allows any string; producers emit UUIDs. Add format to strengthen validation.
- "investigationId": { - "type": "string" - }, + "investigationId": { + "type": "string", + "format": "uuid" + },Also applies to: 1606-1607
657-775: DRY the investigations object via $refIdentical object appears under licenses and privileges. Define once (e.g., "#/definitions/Investigation") and $ref it in both places to reduce drift.
Example:
+ "definitions": { + "Investigation": { + "type": "object", + "properties": { "...": "..." }, + "required": [ "type","compact","providerId","investigationId","jurisdiction","licenseType","dateOfUpdate","creationDate","submittingUser" ] + } + }, ... - "investigations": { "items": { ... full object ... }, "type": "array" } + "investigations": { "items": { "$ref": "#/definitions/Investigation" }, "type": "array" }Also applies to: 1584-1702
backend/compact-connect/lambdas/python/provider-data-v1/handlers/investigation.py (1)
214-223: Serialize UUIDs when publishing closed events (consistency/safety)Event detail should be plain JSON scalars; pass str() for IDs.
- investigation_id=investigation_id, + investigation_id=str(investigation_id),Also applies to: 274-283
backend/compact-connect/lambdas/python/common/tests/function/test_data_client.py (1)
1036-1263: Factor common expectations/builders to reduce duplicationMany tests build near-identical expected investigation/update dicts. Extract small helpers (e.g., build_expected_investigation, build_expected_update) to cut noise and ease future schema changes.
Also applies to: 1322-1599, 1762-1912
backend/compact-connect/lambdas/python/common/cc_common/email_service_client.py (1)
65-92: Unify return type annotations for _invoke_lambda callers_invoke_lambda returns a boto3 response dict; annotate callers as dict[str, Any] for accuracy and consistency.
- ) -> dict[str, str]: + ) -> dict[str, Any]:Apply to investigation email methods for consistency with send_compact_transaction_report_email.
Also applies to: 624-870
backend/compact-connect/tests/app/test_api/test_investigation_api.py (3)
334-362: Strengthen PATCH method checks: require path paramAssert RequestParameters so /{investigationId} is required by API Gateway.
api_stack_template.has_resource_properties( type=CfnMethod.CFN_RESOURCE_TYPE_NAME, props={ 'HttpMethod': 'PATCH', 'AuthorizationType': 'COGNITO_USER_POOLS', 'AuthorizerId': { 'Ref': api_stack.get_logical_id(api_stack.api.staff_users_authorizer().node.default_child), }, 'ResourceId': {'Ref': self._get_privilege_investigation_id_resource_id(api_stack_template, api_stack)}, 'Integration': TestApi.generate_expected_integration_object_for_imported_lambda( api_lambda_stack, api_lambda_stack_template, api_lambda_stack.provider_management_lambdas.provider_investigation_handler, ), + 'RequestParameters': { + 'method.request.path.investigationId': True, + }, 'RequestModels': {'application/json': {'Ref': request_model_logical_id_capture}}, 'MethodResponses': [Repeat similarly for the license PATCH block.
Also applies to: 423-451
294-322: Optional: verify Integration type/method to avoid false positivesAdd Type and IntegrationHttpMethod to the Integration expectation.
- 'Integration': TestApi.generate_expected_integration_object_for_imported_lambda( + 'Integration': { + **TestApi.generate_expected_integration_object_for_imported_lambda( api_lambda_stack, api_lambda_stack_template, api_lambda_stack.provider_management_lambdas.provider_investigation_handler, - ), + ), + 'Type': 'AWS_PROXY', + 'IntegrationHttpMethod': 'POST', + },Apply similarly to the PATCH method blocks if desired.
Also applies to: 383-411
37-39: Nit: simplify “next(key for key in mapping.keys())”Prefer next(iter(mapping)) for clarity.
- return next(key for key in investigation_resource_logical_ids.keys()) + return next(iter(investigation_resource_logical_ids))Apply similarly to the other helpers.
Also applies to: 53-55, 69-71, 85-87, 102-104, 115-116, 126-128, 138-140, 150-152, 165-167, 177-179, 189-191, 201-203, 213-215
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (48)
backend/compact-connect/docs/postman/postman-collection.json(19 hunks)backend/compact-connect/lambdas/nodejs/cognito-emails/lambda.ts(1 hunks)backend/compact-connect/lambdas/nodejs/email-notification-service/lambda.ts(5 hunks)backend/compact-connect/lambdas/nodejs/eslint.config.mjs(1 hunks)backend/compact-connect/lambdas/nodejs/lib/email/email-notification-service.ts(1 hunks)backend/compact-connect/lambdas/nodejs/lib/email/investigation-notification-service.ts(1 hunks)backend/compact-connect/lambdas/nodejs/tests/email-notification-service.test.ts(1 hunks)backend/compact-connect/lambdas/nodejs/tests/ingest-event-reporter.test.ts(5 hunks)backend/compact-connect/lambdas/nodejs/tests/lib/email/email-notification-service.test.ts(0 hunks)backend/compact-connect/lambdas/nodejs/tests/lib/email/encumbrance-notification-service.test.ts(1 hunks)backend/compact-connect/lambdas/nodejs/tests/lib/email/investigation-notification-service.test.ts(1 hunks)backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py(2 hunks)backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py(9 hunks)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/common.py(2 hunks)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/data_event/api.py(1 hunks)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/investigation/__init__.py(1 hunks)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/investigation/record.py(1 hunks)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/license/record.py(7 hunks)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/record.py(6 hunks)backend/compact-connect/lambdas/python/common/cc_common/email_service_client.py(6 hunks)backend/compact-connect/lambdas/python/common/cc_common/event_bus_client.py(2 hunks)backend/compact-connect/lambdas/python/common/requirements-dev.in(1 hunks)backend/compact-connect/lambdas/python/common/tests/function/test_data_client.py(2 hunks)backend/compact-connect/lambdas/python/common/tests/unit/test_data_model/test_schema/test_investigation.py(1 hunks)backend/compact-connect/lambdas/python/common/tests/unit/test_data_model/test_schema/test_license.py(1 hunks)backend/compact-connect/lambdas/python/common/tests/unit/test_data_model/test_schema/test_privilege.py(1 hunks)backend/compact-connect/lambdas/python/common/tests/unit/test_investigation_event_bus_client.py(1 hunks)backend/compact-connect/lambdas/python/data-events/handlers/investigation_events.py(1 hunks)backend/compact-connect/lambdas/python/data-events/tests/function/test_investigation_events.py(1 hunks)backend/compact-connect/lambdas/python/provider-data-v1/handlers/investigation.py(1 hunks)backend/compact-connect/lambdas/python/provider-data-v1/requirements-dev.in(1 hunks)backend/compact-connect/lambdas/python/provider-data-v1/tests/function/__init__.py(1 hunks)backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_investigation.py(1 hunks)backend/compact-connect/lambdas/python/purchases/requirements-dev.in(1 hunks)backend/compact-connect/lambdas/python/staff-users/requirements-dev.in(1 hunks)backend/compact-connect/requirements-dev.in(1 hunks)backend/compact-connect/stacks/api_lambda_stack/provider_management.py(4 hunks)backend/compact-connect/stacks/api_stack/v1_api/api.py(1 hunks)backend/compact-connect/stacks/api_stack/v1_api/api_model.py(23 hunks)backend/compact-connect/tests/app/base.py(2 hunks)backend/compact-connect/tests/app/test_api/test_investigation_api.py(1 hunks)backend/compact-connect/tests/resources/snapshots/GET_PROVIDER_RESPONSE_SCHEMA.json(12 hunks)backend/compact-connect/tests/resources/snapshots/POST_PROVIDER_USERS_MILITARY_AFFILIATION_RESPONSE_SCHEMA.json(1 hunks)backend/compact-connect/tests/resources/snapshots/PROVIDER_USER_RESPONSE_SCHEMA.json(12 hunks)backend/compact-connect/tests/resources/snapshots/PUBLIC_GET_PROVIDER_RESPONSE_SCHEMA.json(6 hunks)backend/compact-connect/tests/resources/snapshots/PUBLIC_QUERY_PROVIDERS_RESPONSE_SCHEMA.json(1 hunks)backend/compact-connect/tests/resources/snapshots/QUERY_PROVIDERS_RESPONSE_SCHEMA.json(1 hunks)backend/compact-connect/tests/smoke/investigation_smoke_tests.py(1 hunks)
💤 Files with no reviewable changes (1)
- backend/compact-connect/lambdas/nodejs/tests/lib/email/email-notification-service.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- backend/compact-connect/lambdas/nodejs/lib/email/investigation-notification-service.ts
- backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/investigation/init.py
- backend/compact-connect/stacks/api_stack/v1_api/api.py
- backend/compact-connect/lambdas/python/purchases/requirements-dev.in
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-04-29T01:57:43.684Z
Learnt from: landonshumway-ia
PR: csg-org/CompactConnect#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/tests/resources/snapshots/PUBLIC_GET_PROVIDER_RESPONSE_SCHEMA.jsonbackend/compact-connect/tests/resources/snapshots/QUERY_PROVIDERS_RESPONSE_SCHEMA.jsonbackend/compact-connect/tests/resources/snapshots/GET_PROVIDER_RESPONSE_SCHEMA.jsonbackend/compact-connect/tests/resources/snapshots/PROVIDER_USER_RESPONSE_SCHEMA.json
📚 Learning: 2025-09-11T20:06:40.709Z
Learnt from: ChiefStief
PR: csg-org/CompactConnect#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/record.pybackend/compact-connect/stacks/api_stack/v1_api/api_model.py
📚 Learning: 2025-09-03T22:29:24.535Z
Learnt from: ChiefStief
PR: csg-org/CompactConnect#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/data_event/api.py
📚 Learning: 2025-10-24T21:40:48.885Z
Learnt from: jusdino
PR: csg-org/CompactConnect#1167
File: backend/compact-connect/lambdas/python/provider-data-v1/handlers/investigation.py:59-66
Timestamp: 2025-10-24T21:40:48.885Z
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/provider-data-v1/handlers/investigation.py
📚 Learning: 2025-06-04T17:38:22.392Z
Learnt from: landonshumway-ia
PR: csg-org/CompactConnect#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
🧬 Code graph analysis (22)
backend/compact-connect/lambdas/nodejs/tests/lib/email/investigation-notification-service.test.ts (3)
backend/compact-connect/lambdas/nodejs/lib/email/investigation-notification-service.ts (1)
InvestigationNotificationService(8-455)backend/compact-connect/lambdas/nodejs/lib/jurisdiction-client.ts (1)
JurisdictionClient(18-76)backend/compact-connect/lambdas/nodejs/tests/utils/email-template-capture.ts (1)
EmailTemplateCapture(9-141)
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/fields.py (2)
InvestigationStatusField(85-87)Jurisdiction(55-57)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/license/record.py (2)
InvestigationDetailsSchema(35-42)validate_investigation_details_present_if_investigation_status_updated(238-245)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/common.py (1)
UpdateCategory(296-310)
backend/compact-connect/lambdas/python/common/tests/unit/test_data_model/test_schema/test_investigation.py (5)
backend/compact-connect/lambdas/python/common/common_test/test_data_generator.py (1)
generate_default_investigation(155-179)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/investigation/record.py (1)
InvestigationRecordSchema(15-55)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/common.py (3)
serialize_to_database_record(205-208)CompactEligibilityStatus(318-320)from_database_record(124-139)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/investigation/__init__.py (17)
InvestigationData(12-110)investigationAgainst(57-58)investigationAgainst(61-62)licenseType(49-50)licenseType(53-54)providerId(33-34)providerId(37-38)jurisdiction(41-42)jurisdiction(45-46)submittingUser(73-74)submittingUser(77-78)investigationId(65-66)investigationId(69-70)compact(25-26)compact(29-30)creationDate(81-82)creationDate(85-86)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/investigation/api.py (1)
InvestigationPatchRequestSchema(13-25)
backend/compact-connect/lambdas/nodejs/email-notification-service/lambda.ts (3)
backend/compact-connect/lambdas/nodejs/lib/email/investigation-notification-service.ts (1)
InvestigationNotificationService(8-455)backend/compact-connect/lambdas/nodejs/lib/email/index.ts (1)
InvestigationNotificationService(4-4)backend/compact-connect/lambdas/nodejs/lib/models/email-notification-service-event.ts (2)
EmailNotificationEvent(10-19)EmailNotificationResponse(21-23)
backend/compact-connect/lambdas/python/common/tests/unit/test_data_model/test_schema/test_license.py (2)
backend/compact-connect/lambdas/python/common/tests/unit/test_data_model/test_schema/test_privilege.py (1)
test_invalid_if_missing_investigation_details_when_update_type_is_investigation(194-208)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/license/record.py (1)
LicenseUpdateRecordSchema(182-245)
backend/compact-connect/lambdas/python/data-events/handlers/investigation_events.py (8)
backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py (5)
ProviderUserRecords(412-853)get_provider_record(81-86)get_provider_record(655-664)get_license_records(503-510)get_privilege_records(493-501)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/data_event/api.py (1)
InvestigationEventDetailSchema(57-61)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/provider/__init__.py (2)
ProviderData(13-148)compactConnectRegisteredEmailAddress(77-83)backend/compact-connect/lambdas/python/common/cc_common/config.py (2)
email_service_client(298-303)data_client(42-45)backend/compact-connect/lambdas/python/common/cc_common/email_service_client.py (10)
InvestigationNotificationTemplateVariables(28-37)ProviderNotificationMethod(40-45)send_license_investigation_provider_notification_email(624-651)send_license_investigation_state_notification_email(653-684)send_license_investigation_closed_provider_notification_email(686-713)send_license_investigation_closed_state_notification_email(715-746)send_privilege_investigation_provider_notification_email(748-775)send_privilege_investigation_state_notification_email(777-808)send_privilege_investigation_closed_provider_notification_email(810-837)send_privilege_investigation_closed_state_notification_email(839-870)backend/compact-connect/lambdas/python/common/cc_common/license_util.py (2)
LicenseUtility(15-60)get_license_type_by_abbreviation(19-36)backend/compact-connect/lambdas/python/common/cc_common/utils.py (1)
sqs_handler(433-468)backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py (1)
get_provider_user_records(176-207)
backend/compact-connect/lambdas/nodejs/tests/email-notification-service.test.ts (2)
backend/compact-connect/lambdas/nodejs/lib/models/email-notification-service-event.ts (1)
EmailNotificationEvent(10-19)backend/compact-connect/lambdas/nodejs/tests/sample-records.ts (1)
SAMPLE_COMPACT_CONFIGURATION(326-342)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/license/record.py (3)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/common.py (1)
UpdateCategory(296-310)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/fields.py (3)
InvestigationStatusField(85-87)Jurisdiction(55-57)LicenseEncumberedStatusField(75-77)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/record.py (2)
InvestigationDetailsSchema(69-76)validate_investigation_details_present_if_investigation_status_updated(265-272)
backend/compact-connect/lambdas/python/common/cc_common/event_bus_client.py (3)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/common.py (1)
InvestigationAgainstEnum(287-293)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/data_event/api.py (1)
InvestigationEventDetailSchema(57-61)backend/compact-connect/lambdas/python/common/cc_common/event_batch_writer.py (1)
EventBatchWriter(4-49)
backend/compact-connect/lambdas/python/data-events/tests/function/test_investigation_events.py (3)
backend/compact-connect/lambdas/python/common/cc_common/email_service_client.py (1)
InvestigationNotificationTemplateVariables(28-37)backend/compact-connect/lambdas/python/data-events/handlers/investigation_events.py (4)
license_investigation_notification_listener(180-247)license_investigation_closed_notification_listener(251-318)privilege_investigation_notification_listener(322-389)privilege_investigation_closed_notification_listener(393-460)backend/compact-connect/lambdas/python/common/common_test/test_data_generator.py (3)
put_default_provider_record_in_provider_table(446-466)put_default_license_record_in_provider_table(275-285)put_default_privilege_record_in_provider_table(347-357)
backend/compact-connect/lambdas/python/common/tests/unit/test_investigation_event_bus_client.py (4)
backend/compact-connect/lambdas/python/common/cc_common/config.py (2)
event_bus_client(306-309)events_client(68-69)backend/compact-connect/lambdas/python/common/cc_common/event_bus_client.py (3)
EventBusClient(19-433)publish_investigation_event(339-385)publish_investigation_closed_event(387-433)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/common.py (1)
InvestigationAgainstEnum(287-293)backend/compact-connect/lambdas/python/common/cc_common/event_batch_writer.py (1)
put_event(42-49)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/data_event/api.py (3)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/investigation/__init__.py (6)
providerId(33-34)providerId(37-38)investigationId(65-66)investigationId(69-70)investigationAgainst(57-58)investigationAgainst(61-62)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/__init__.py (4)
providerId(35-36)providerId(39-40)licenseTypeAbbreviation(51-52)licenseTypeAbbreviation(55-56)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/common.py (1)
licenseTypeAbbreviation(156-167)
backend/compact-connect/stacks/api_lambda_stack/provider_management.py (2)
backend/compact-connect/lambdas/python/common/cc_common/config.py (2)
event_bus_name(84-85)provider_table(88-89)backend/compact-connect/common_constructs/python_function.py (1)
PythonFunction(19-138)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/investigation/record.py (4)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/base_record.py (2)
BaseRecordSchema(28-82)register_schema(68-75)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/fields.py (3)
Compact(50-52)InvestigationAgainstField(126-128)Jurisdiction(55-57)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/license/record.py (2)
generate_pk_sk(124-128)generate_pk_sk(212-229)backend/compact-connect/lambdas/python/common/cc_common/config.py (1)
license_types_for_compact(148-149)
backend/compact-connect/lambdas/python/common/tests/unit/test_data_model/test_schema/test_privilege.py (2)
backend/compact-connect/lambdas/python/common/tests/unit/test_data_model/test_schema/test_license.py (1)
test_invalid_if_missing_investigation_details_when_update_type_is_investigation(247-261)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/record.py (1)
PrivilegeUpdateRecordSchema(213-292)
backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_investigation.py (5)
backend/compact-connect/lambdas/python/provider-data-v1/tests/function/__init__.py (2)
TstFunction(22-443)assertDictFieldsMatch(25-30)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/common.py (5)
ClinicalPrivilegeActionCategory(391-404)EncumbranceType(369-388)InvestigationStatusEnum(335-337)update(182-203)UpdateCategory(296-310)backend/compact-connect/lambdas/python/common/common_test/test_data_generator.py (1)
generate_test_api_event(34-62)backend/compact-connect/lambdas/python/provider-data-v1/handlers/investigation.py (1)
investigation_handler(41-56)backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py (1)
get_provider(149-173)
backend/compact-connect/tests/smoke/investigation_smoke_tests.py (2)
backend/compact-connect/tests/smoke/smoke_common.py (8)
SmokeTestFailureException(13-17)call_provider_users_me_endpoint(227-235)create_test_staff_user(104-131)delete_test_staff_user(83-101)get_all_provider_database_records(238-248)get_license_type_abbreviation(183-190)get_provider_user_dynamodb_table(197-198)get_staff_user_auth_headers(176-180)backend/compact-connect/lambdas/python/common/tests/function/test_data_client.py (1)
test_close_privilege_investigation_with_encumbrance(1762-1836)
backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py (2)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/investigation/__init__.py (7)
InvestigationData(12-110)investigationAgainst(57-58)investigationAgainst(61-62)jurisdiction(41-42)jurisdiction(45-46)closeDate(89-90)closeDate(93-94)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/common.py (2)
UpdateCategory(296-310)licenseTypeAbbreviation(156-167)
backend/compact-connect/stacks/api_stack/v1_api/api_model.py (4)
backend/compact-connect/tests/app/test_transaction_monitoring.py (1)
get_context(27-34)backend/compact-connect/tests/app/test_pipeline.py (1)
get_context(22-31)backend/compact-connect/tests/app/test_api/__init__.py (1)
get_context(17-25)backend/compact-connect/tests/app/test_event_listener.py (1)
get_context(19-27)
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py (6)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/common.py (6)
InvestigationAgainstEnum(287-293)InvestigationStatusEnum(335-337)from_database_record(124-139)create_new(96-121)to_dict(169-180)serialize_to_database_record(205-208)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/investigation/__init__.py (15)
InvestigationData(12-110)compact(25-26)compact(29-30)providerId(33-34)providerId(37-38)jurisdiction(41-42)jurisdiction(45-46)investigationAgainst(57-58)investigationAgainst(61-62)investigationId(65-66)investigationId(69-70)creationDate(81-82)creationDate(85-86)licenseType(49-50)licenseType(53-54)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/__init__.py (10)
compact(28-29)compact(108-109)providerId(24-25)providerId(104-105)jurisdiction(32-33)jurisdiction(112-113)PrivilegeData(13-88)PrivilegeUpdateData(91-162)licenseType(40-41)licenseType(116-117)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/license/__init__.py (10)
compact(32-33)compact(164-165)providerId(28-29)providerId(160-161)jurisdiction(36-37)jurisdiction(168-169)LicenseData(9-137)LicenseUpdateData(140-193)licenseType(40-41)licenseType(172-173)backend/compact-connect/lambdas/python/common/cc_common/config.py (2)
provider_table(88-89)dynamodb_client(38-39)backend/compact-connect/lambdas/python/common/cc_common/exceptions.py (1)
CCNotFoundException(32-33)
backend/compact-connect/tests/app/test_api/test_investigation_api.py (5)
backend/compact-connect/tests/app/test_api/__init__.py (2)
TestApi(11-83)generate_expected_integration_object_for_imported_lambda(48-83)backend/compact-connect/lambdas/python/provider-data-v1/handlers/investigation.py (1)
investigation_handler(41-56)backend/compact-connect/tests/app/base.py (2)
get_resource_properties_by_logical_id(103-110)compare_snapshot(619-649)backend/compact-connect/stacks/api_stack/api.py (1)
staff_users_authorizer(50-55)backend/compact-connect/stacks/api_stack/v1_api/api_model.py (3)
message_response_model(21-39)patch_privilege_investigation_request_model(2855-2870)patch_license_investigation_request_model(2873-2888)
backend/compact-connect/lambdas/python/common/tests/function/test_data_client.py (5)
backend/compact-connect/lambdas/python/common/tests/function/__init__.py (2)
setUp(23-36)_load_provider_data(218-233)backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py (2)
create_investigation(1616-1709)close_investigation(1711-1881)backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_data_model/test_client.py (1)
_load_provider_data(215-230)backend/compact-connect/lambdas/python/common/common_test/test_data_generator.py (1)
generate_default_investigation(155-179)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/investigation/__init__.py (11)
investigationId(65-66)investigationId(69-70)submittingUser(73-74)submittingUser(77-78)creationDate(81-82)creationDate(85-86)InvestigationData(12-110)compact(25-26)compact(29-30)jurisdiction(41-42)jurisdiction(45-46)
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
Show resolved
Hide resolved
backend/compact-connect/lambdas/python/provider-data-v1/handlers/investigation.py
Show resolved
Hide resolved
...t-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_investigation.py
Outdated
Show resolved
Hide resolved
...t-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_investigation.py
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (7)
backend/compact-connect/lambdas/nodejs/email-notification-service/lambda.ts (1)
370-453: DRY the repeated validation for investigation email cases.Extract a small helper to assert jurisdiction and required templateVariables to reduce repetition and future misses.
// add near top of handler function requireVars(event: EmailNotificationEvent, keys: string[], missingMsg: string) { if (!event.jurisdiction) throw new Error('Missing required jurisdiction field.'); for (const k of keys) { if (!event.templateVariables?.[k]) throw new Error(missingMsg); } } // usage example case 'licenseInvestigationStateNotification': { requireVars(event, ['providerFirstName','providerLastName','providerId','investigationJurisdiction','licenseType'], 'Missing required template variables for licenseInvestigationStateNotification template.' ); await this.investigationService.sendLicenseInvestigationStateNotificationEmail( event.compact, event.jurisdiction, event.templateVariables.providerFirstName, event.templateVariables.providerLastName, event.templateVariables.providerId, event.templateVariables.investigationJurisdiction, event.templateVariables.licenseType ); break; }backend/compact-connect/lambdas/python/provider-data-v1/handlers/investigation.py (1)
78-83: Remove unreachable check after license type lookup.LicenseUtility.get_license_type_by_abbreviation raises on invalid; the subsequent if-not check is dead code.
- if not license_type: - raise CCInvalidRequestException( - f'Could not find license type information based on provided parameters ' - f"compact: '{compact}' licenseType: '{license_type_abbr}'" - )backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/data_event/api.py (1)
7-9: Validate investigationAgainst with an explicit enum set.Constrain to 'license' | 'privilege' to prevent malformed events.
-from marshmallow.validate import Length +from marshmallow.validate import Length, OneOf @@ class InvestigationEventDetailSchema(DataEventDetailBaseSchema): providerId = UUID(required=True, allow_none=False) investigationId = UUID(required=True, allow_none=False) licenseTypeAbbreviation = String(required=True, allow_none=False) - investigationAgainst = String(required=True, allow_none=False) + investigationAgainst = String(required=True, allow_none=False, validate=OneOf(['license','privilege'])) # Only present for investigationClosed events with encumbrance adverseActionId = UUID(required=False, allow_none=False)Also applies to: 57-64
backend/compact-connect/lambdas/python/common/cc_common/email_service_client.py (1)
624-631: Align return types with _invoke_lambda (dict[str, Any]).Methods return the raw boto3 invoke response; widen annotations for accuracy and consistency.
- ) -> dict[str, str]: + ) -> dict[str, Any]: @@ - ) -> dict[str, str]: + ) -> dict[str, Any]: @@ - ) -> dict[str, str]: + ) -> dict[str, Any]: @@ - ) -> dict[str, str]: + ) -> dict[str, Any]:Also applies to: 657-664, 690-697, 723-730
backend/compact-connect/lambdas/python/common/cc_common/event_bus_client.py (1)
377-383: Optional: use.valuefor readability, matching the closed-event path.Both are equivalent with StrEnum; using
.valuemirrors the closed variant and avoids ambiguity.- detail_type = f'{investigation_against}.investigation' + detail_type = f'{investigation_against.value}.investigation'backend/compact-connect/lambdas/python/data-events/handlers/investigation_events.py (2)
81-89: Fix type annotations for jurisdiction notifications.notification_method is typed as ProviderNotificationMethod (expects provider_email), but here you pass jurisdiction-based methods. Introduce a JurisdictionNotificationMethod Protocol (compact, jurisdiction, template_variables) or relax to Callable[...] to reflect actual usage and improve static checks.
Example:
# in cc_common/email_service_client.py from typing import Protocol, Any class JurisdictionNotificationMethod(Protocol): def __call__(self, *, compact: str, jurisdiction: str, template_variables: Any) -> dict[str, Any]: ... # in this file from cc_common.email_service_client import JurisdictionNotificationMethod def _send_primary_state_notification( notification_method: JurisdictionNotificationMethod, ... ): ... def _send_additional_state_notifications( notification_method: JurisdictionNotificationMethod, ... ): ...Also applies to: 116-126, 179-237, 241-307, 309-369, 371-436
139-151: Optional: consider filtering to active licenses/privileges.currently not filtering — jurisdictions with inactive/expired records will also be notified. If intent is “active only,” filter by status fields before adding to notification_jurisdictions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
backend/compact-connect/lambdas/nodejs/email-notification-service/lambda.ts(5 hunks)backend/compact-connect/lambdas/nodejs/lib/email/investigation-notification-service.ts(1 hunks)backend/compact-connect/lambdas/nodejs/tests/lib/email/investigation-notification-service.test.ts(1 hunks)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/data_event/api.py(1 hunks)backend/compact-connect/lambdas/python/common/cc_common/email_service_client.py(6 hunks)backend/compact-connect/lambdas/python/common/cc_common/event_bus_client.py(2 hunks)backend/compact-connect/lambdas/python/data-events/handlers/investigation_events.py(1 hunks)backend/compact-connect/lambdas/python/data-events/tests/function/test_investigation_events.py(1 hunks)backend/compact-connect/lambdas/python/provider-data-v1/handlers/investigation.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/compact-connect/lambdas/nodejs/tests/lib/email/investigation-notification-service.test.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-03T22:29:24.535Z
Learnt from: ChiefStief
PR: csg-org/CompactConnect#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/data_event/api.py
📚 Learning: 2025-10-24T21:40:48.885Z
Learnt from: jusdino
PR: csg-org/CompactConnect#1167
File: backend/compact-connect/lambdas/python/provider-data-v1/handlers/investigation.py:59-66
Timestamp: 2025-10-24T21:40:48.885Z
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/provider-data-v1/handlers/investigation.py
🧬 Code graph analysis (7)
backend/compact-connect/lambdas/nodejs/email-notification-service/lambda.ts (3)
backend/compact-connect/lambdas/nodejs/lib/email/investigation-notification-service.ts (1)
InvestigationNotificationService(8-287)backend/compact-connect/lambdas/nodejs/lib/email/index.ts (1)
InvestigationNotificationService(4-4)backend/compact-connect/lambdas/nodejs/lib/models/email-notification-service-event.ts (2)
EmailNotificationEvent(10-19)EmailNotificationResponse(21-23)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/data_event/api.py (3)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/investigation/__init__.py (6)
providerId(33-34)providerId(37-38)investigationId(65-66)investigationId(69-70)investigationAgainst(57-58)investigationAgainst(61-62)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/__init__.py (6)
providerId(35-36)providerId(39-40)licenseTypeAbbreviation(51-52)licenseTypeAbbreviation(55-56)adverseActionId(126-127)adverseActionId(130-131)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/common.py (1)
licenseTypeAbbreviation(156-167)
backend/compact-connect/lambdas/python/common/cc_common/event_bus_client.py (3)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/common.py (1)
InvestigationAgainstEnum(287-293)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/data_event/api.py (1)
InvestigationEventDetailSchema(57-63)backend/compact-connect/lambdas/python/common/cc_common/event_batch_writer.py (1)
EventBatchWriter(4-49)
backend/compact-connect/lambdas/python/data-events/handlers/investigation_events.py (8)
backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py (5)
ProviderUserRecords(412-853)get_provider_record(81-86)get_provider_record(655-664)get_license_records(503-510)get_privilege_records(493-501)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/data_event/api.py (1)
InvestigationEventDetailSchema(57-63)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/provider/__init__.py (2)
ProviderData(13-148)compactConnectRegisteredEmailAddress(77-83)backend/compact-connect/lambdas/python/common/cc_common/config.py (2)
email_service_client(298-303)data_client(42-45)backend/compact-connect/lambdas/python/common/cc_common/email_service_client.py (6)
InvestigationNotificationTemplateVariables(28-37)ProviderNotificationMethod(40-45)send_license_investigation_state_notification_email(624-655)send_license_investigation_closed_state_notification_email(657-688)send_privilege_investigation_state_notification_email(690-721)send_privilege_investigation_closed_state_notification_email(723-754)backend/compact-connect/lambdas/python/common/cc_common/license_util.py (2)
LicenseUtility(15-60)get_license_type_by_abbreviation(19-36)backend/compact-connect/lambdas/python/common/cc_common/utils.py (2)
sqs_handler(433-468)get(86-87)backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py (1)
get_provider_user_records(176-207)
backend/compact-connect/lambdas/python/data-events/tests/function/test_investigation_events.py (4)
backend/compact-connect/lambdas/python/common/cc_common/config.py (1)
email_service_client(298-303)backend/compact-connect/lambdas/python/common/cc_common/email_service_client.py (1)
InvestigationNotificationTemplateVariables(28-37)backend/compact-connect/lambdas/python/data-events/handlers/investigation_events.py (4)
license_investigation_notification_listener(180-238)license_investigation_closed_notification_listener(242-306)privilege_investigation_notification_listener(310-368)privilege_investigation_closed_notification_listener(372-436)backend/compact-connect/lambdas/python/common/common_test/test_data_generator.py (3)
put_default_provider_record_in_provider_table(446-466)put_default_license_record_in_provider_table(275-285)put_default_privilege_record_in_provider_table(347-357)
backend/compact-connect/lambdas/nodejs/lib/email/investigation-notification-service.ts (2)
backend/compact-connect/lambdas/nodejs/lib/email/index.ts (1)
InvestigationNotificationService(4-4)webroot/src/network/mocks/mock.data.ts (1)
compactConfig(1447-1493)
backend/compact-connect/lambdas/python/provider-data-v1/handlers/investigation.py (10)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/common.py (3)
CCPermissionsAction(224-234)InvestigationAgainstEnum(287-293)create_new(96-121)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/investigation/__init__.py (1)
InvestigationData(12-110)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/investigation/api.py (1)
InvestigationPatchRequestSchema(13-25)backend/compact-connect/lambdas/python/common/cc_common/exceptions.py (1)
CCInvalidRequestException(7-8)backend/compact-connect/lambdas/python/common/cc_common/license_util.py (2)
LicenseUtility(15-60)get_license_type_by_abbreviation(19-36)backend/compact-connect/lambdas/python/common/cc_common/utils.py (3)
api_handler(90-212)authorize_state_level_only_action(276-313)get(86-87)backend/compact-connect/lambdas/python/provider-data-v1/handlers/encumbrance.py (2)
_create_license_encumbrance_internal(185-217)_create_privilege_encumbrance_internal(130-161)backend/compact-connect/lambdas/python/common/cc_common/config.py (3)
current_standard_datetime(253-257)data_client(42-45)event_bus_client(306-309)backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py (2)
create_investigation(1616-1709)close_investigation(1711-1881)backend/compact-connect/lambdas/python/common/cc_common/event_bus_client.py (2)
publish_investigation_event(339-385)publish_investigation_closed_event(387-439)
⏰ 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
backend/compact-connect/lambdas/nodejs/lib/email/investigation-notification-service.ts
Show resolved
Hide resolved
backend/compact-connect/lambdas/nodejs/lib/email/investigation-notification-service.ts
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
backend/compact-connect/lambdas/nodejs/tests/email-notification-service.test.ts (2)
1657-1657: Use precise regex pattern for apostrophe.The
.in the regex matches any character, not just the apostrophe. This could lead to false positives.Apply this diff to use a more precise pattern:
- Data: expect.stringMatching(/Investigation on John Doe.s Audiologist license in Ohio has been closed/) + Data: expect.stringMatching(/Investigation on John Doe's Audiologist license in Ohio has been closed/)
1839-1839: Use precise regex pattern for apostrophe.The
.in the regex matches any character, not just the apostrophe. This is the same issue as line 1657.Apply this diff to use a more precise pattern:
- Data: expect.stringMatching(/Investigation on John Doe.s Audiologist privilege in Ohio has been closed/) + Data: expect.stringMatching(/Investigation on John Doe's Audiologist privilege in Ohio has been closed/)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/compact-connect/lambdas/nodejs/tests/email-notification-service.test.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
backend/compact-connect/lambdas/nodejs/tests/email-notification-service.test.ts (2)
backend/compact-connect/lambdas/nodejs/lib/models/email-notification-service-event.ts (1)
EmailNotificationEvent(10-19)backend/compact-connect/lambdas/nodejs/tests/sample-records.ts (1)
SAMPLE_COMPACT_CONFIGURATION(326-342)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: TestApp
🔇 Additional comments (2)
backend/compact-connect/lambdas/nodejs/tests/email-notification-service.test.ts (2)
1506-1594: LGTM! Comprehensive test coverage for license investigation notifications.The test suite properly covers:
- Successful email delivery with correct subject, body, and recipients
- Proper mock setup for both recipient and investigation jurisdictions
- Error handling for missing jurisdiction and template variables
The pattern aligns well with existing notification tests in the file.
1688-1776: LGTM! Well-structured privilege investigation notification tests.The test suite correctly:
- Differentiates "privilege" terminology from "license" in assertions
- Mocks both recipient and investigation jurisdiction configurations
- Tests all error conditions appropriately
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py (1)
1693-1706: Guard the investigation status update with a conditionIf the license/privilege record disappears between the read and this transaction, DynamoDB will happily materialize a brand-new item containing only
investigationStatus/dateOfUpdate. That leaves you with a phantom stub record and an inconsistent provider record. Addingattribute_exists(pk)(and letting the transaction cancel otherwise) keeps the write safe.'UpdateExpression': ( 'SET investigationStatus = :investigationStatus, dateOfUpdate = :dateOfUpdate' ), + 'ConditionExpression': 'attribute_exists(pk)', 'ExpressionAttributeValues': { ':investigationStatus': {'S': InvestigationStatusEnum.UNDER_INVESTIGATION}, ':dateOfUpdate': {'S': investigation.creationDate.isoformat()}, },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/check-compact-connect.yml(1 hunks)backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py(23 hunks)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/investigation/__init__.py(1 hunks)backend/compact-connect/lambdas/python/common/tests/function/test_data_client.py(9 hunks)backend/compact-connect/lambdas/python/common/tests/unit/test_data_model/test_schema/test_investigation.py(1 hunks)
🧰 Additional context used
🧠 Learnings (25)
📚 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-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:
.github/workflows/check-compact-connect.yml
📚 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:
.github/workflows/check-compact-connect.yml
📚 Learning: 2025-10-29T19:11:33.003Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1167
File: backend/compact-connect/lambdas/python/provider-data-v1/handlers/investigation.py:85-95
Timestamp: 2025-10-29T19:11:33.003Z
Learning: InvestigationData extends CCDataClass which provides the licenseTypeAbbreviation property as a computed property (returns license type abbreviation if the instance has both 'compact' and 'licenseType' fields). Therefore, InvestigationData instances have access to licenseTypeAbbreviation through inheritance, and it does not need to be explicitly set when creating new investigation records.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/investigation/__init__.pybackend/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/schema/investigation/__init__.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/cc_common/data_model/schema/investigation/__init__.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/common/cc_common/data_model/schema/investigation/__init__.pybackend/compact-connect/lambdas/python/common/tests/function/test_data_client.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/common/cc_common/data_model/schema/investigation/__init__.pybackend/compact-connect/lambdas/python/common/tests/function/test_data_client.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/common/cc_common/data_model/schema/investigation/__init__.pybackend/compact-connect/lambdas/python/common/tests/function/test_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.pybackend/compact-connect/lambdas/python/common/tests/function/test_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-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-10T13:50:30.047Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1069
File: backend/compact-connect/lambdas/python/purchases/handlers/transaction_reporting.py:18-22
Timestamp: 2025-09-10T13:50:30.047Z
Learning: When using Python StrEnum for filtering, the `in` operator correctly checks string values against enum members. `'settledSuccessfully' in ReportableTransactionStatuses` works properly and is more maintainable than using raw string sets. StrEnum members are both enum instances and strings, making them ideal for this use case.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-09-10T13:50:30.047Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1069
File: backend/compact-connect/lambdas/python/purchases/handlers/transaction_reporting.py:18-22
Timestamp: 2025-09-10T13:50:30.047Z
Learning: In Python 3.11, using `'string' in StrEnum` raises TypeError. StrEnum membership checking with strings only works starting in Python 3.12. For Python 3.11, convert StrEnum values to a set for membership testing or use explicit equality comparisons.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.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/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-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-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/cc_common/data_model/data_client.py
📚 Learning: 2025-09-03T23:06:45.015Z
Learnt from: ChiefStief
Repo: csg-org/CompactConnect PR: 1036
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py:2645-2651
Timestamp: 2025-09-03T23:06:45.015Z
Learning: PrivilegeData extends CCDataClass which provides the licenseTypeAbbreviation property, so PrivilegeData instances have access to licenseTypeAbbreviation through inheritance.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-09-03T23:06:45.015Z
Learnt from: ChiefStief
Repo: csg-org/CompactConnect PR: 1036
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py:2645-2651
Timestamp: 2025-09-03T23:06:45.015Z
Learning: PrivilegeData extends CCDataClass which provides the licenseTypeAbbreviation property as a computed property (returns license type abbreviation if the instance has both 'compact' and 'licenseType' fields). Therefore, PrivilegeData instances have access to licenseTypeAbbreviation through inheritance.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-06-10T03:16:16.896Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 848
File: backend/compact-connect/lambdas/python/migration/provider_user_pool_migration_551/main.py:35-39
Timestamp: 2025-06-10T03:16:16.896Z
Learning: In the provider user pool migration (provider_user_pool_migration_551), the FilterExpression intentionally only checks for the existence of compactConnectRegisteredEmailAddress. The migration should only remove currentHomeJurisdiction if compactConnectRegisteredEmailAddress is also present, targeting records that went through the old registration process. Records with only currentHomeJurisdiction but no compactConnectRegisteredEmailAddress should be left untouched.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.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/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: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-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
🧬 Code graph analysis (4)
backend/compact-connect/lambdas/python/common/tests/unit/test_data_model/test_schema/test_investigation.py (5)
backend/compact-connect/lambdas/python/common/common_test/test_data_generator.py (1)
generate_default_investigation(155-179)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/investigation/record.py (1)
InvestigationRecordSchema(16-56)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/common.py (3)
serialize_to_database_record(205-208)CompactEligibilityStatus(318-320)from_database_record(124-139)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/investigation/__init__.py (19)
InvestigationData(12-156)investigationAgainst(103-104)investigationAgainst(107-108)licenseType(95-96)licenseType(99-100)providerId(79-80)providerId(83-84)jurisdiction(87-88)jurisdiction(91-92)submittingUser(119-120)submittingUser(123-124)investigationId(111-112)investigationId(115-116)compact(71-72)compact(75-76)creationDate(127-128)creationDate(131-132)pk(42-48)sk(51-68)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/investigation/api.py (1)
InvestigationPatchRequestSchema(13-25)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/investigation/__init__.py (3)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/common.py (3)
CCDataClass(46-208)InvestigationAgainstEnum(287-293)licenseTypeAbbreviation(156-167)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/investigation/record.py (1)
InvestigationRecordSchema(16-56)backend/compact-connect/lambdas/python/common/cc_common/utils.py (1)
get(86-87)
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py (3)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/common.py (4)
InvestigationAgainstEnum(287-293)InvestigationStatusEnum(335-337)UpdateCategory(296-310)licenseTypeAbbreviation(156-167)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/investigation/__init__.py (19)
InvestigationData(12-156)compact(71-72)compact(75-76)providerId(79-80)providerId(83-84)jurisdiction(87-88)jurisdiction(91-92)investigationAgainst(103-104)investigationAgainst(107-108)investigationId(111-112)investigationId(115-116)creationDate(127-128)creationDate(131-132)licenseType(95-96)licenseType(99-100)closeDate(135-136)closeDate(139-140)pk(42-48)sk(51-68)backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py (5)
ProviderRecordType(29-42)get_specific_license_record(459-474)get_specific_privilege_record(476-491)get_investigation_records_for_license(636-653)get_investigation_records_for_privilege(617-634)
backend/compact-connect/lambdas/python/common/tests/function/test_data_client.py (4)
backend/compact-connect/lambdas/python/common/tests/function/__init__.py (1)
setUp(23-36)backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py (2)
create_investigation(1612-1713)close_investigation(1715-1898)backend/compact-connect/lambdas/python/common/common_test/test_data_generator.py (1)
generate_default_investigation(155-179)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/investigation/__init__.py (11)
investigationId(111-112)investigationId(115-116)submittingUser(119-120)submittingUser(123-124)creationDate(127-128)creationDate(131-132)InvestigationData(12-156)compact(71-72)compact(75-76)jurisdiction(87-88)jurisdiction(91-92)
🔇 Additional comments (6)
.github/workflows/check-compact-connect.yml (1)
105-106: Add an inline comment explaining the pip version constraint.pip 25.3 (released October 24, 2025) removed support for the legacy setup.py develop editable install method and removed the --global-option and --build-option CLI flags. The
pip<25.3constraint protects the dependency compilation and build process from these breaking changes.Add a brief comment to document this for future maintainers:
- name: Install all Python dependencies run: "cd backend/compact-connect; pip install -U 'pip<25.3'; bin/sync_deps.sh" # pip 25.3+ removed legacy build option flags; constraint protects pip-compile and build processbackend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py (1)
1875-1882: Fix invalid DynamoDB update expression orderingDynamoDB requires all
SETclauses to precede anyREMOVEclauses. The current"REMOVE … SET …"ordering triggers aValidationException, so the close-invocation never succeeds. Swap the order soSETcomes first.- 'UpdateExpression': 'REMOVE investigationStatus SET dateOfUpdate = :dateOfUpdate', + 'UpdateExpression': 'SET dateOfUpdate = :dateOfUpdate REMOVE investigationStatus',⛔ Skipped due to learnings
Learnt from: landonshumway-ia Repo: csg-org/CompactConnect PR: 1014 File: backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py:3058-3066 Timestamp: 2025-08-18T21:57:32.054Z Learning: DynamoDB UpdateExpression clauses (SET, REMOVE, ADD, DELETE) can appear in any syntactic order in the expression string. DynamoDB processes them in a fixed evaluation order (SET → REMOVE → ADD → DELETE) internally, but the textual order in the UpdateExpression string is flexible. Expressions like "REMOVE attr1 SET attr2 = :val" are syntactically valid and will not throw ValidationExceptions.backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/investigation/__init__.py (4)
1-22: LGTM! Class structure follows established patterns.The class setup correctly extends
CCDataClasswith proper schema initialization and allows incremental data population via setters.
24-39: LGTM! PK/SK generation aligns with schema.The static methods correctly generate database keys consistent with
InvestigationRecordSchema.pre_dumplogic. TheStrEnumusage in the f-string properly resolves to the enum value.
70-132: LGTM! Property accessors are well-implemented.All basic property getters and setters follow the established patterns. The
investigationAgainstproperty correctly handles enum-to-string conversion in the setter while returning the string value in the getter, maintaining consistency with database storage.
134-156: LGTM! Optional field properties correctly implemented.The type annotations for
closeDate,closingUser, andresultingEncumbranceIdare now correct (acknowledging the fix from previous review). The pattern is consistent: getters returnOptionaltypes via.get(), while setters accept non-None values matching the schema'sallow_none=Falseconstraint.
.../compact-connect/lambdas/python/common/cc_common/data_model/schema/investigation/__init__.py
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/compact-connect/lambdas/python/provider-data-v1/handlers/encumbrance.py (1)
69-127: Fix type mismatch for submittingUser.The
submitting_userparameter is typed asstr(line 74), but theAdverseActionData.submittingUserproperty expects aUUIDobject (line 123). This will cause a type validation error.Apply this diff to convert the string to UUID:
adverse_action.effectiveStartDate = encumbrance_effective_date - adverse_action.submittingUser = submitting_user + adverse_action.submittingUser = UUID(submitting_user) adverse_action.creationDate = config.current_standard_datetime
♻️ Duplicate comments (1)
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py (1)
1693-1707: Add ConditionExpression to prevent phantom item creation.The Update operation can create a partial item if the license/privilege record is deleted between the query and the transaction. Guard with
attribute_exists(pk)to ensure the transaction aborts if the item no longer exists.Apply this diff:
{ 'Update': { 'TableName': self.config.provider_table.table_name, 'Key': { 'pk': {'S': serialized_record['pk']}, 'sk': {'S': serialized_record['sk']}, }, 'UpdateExpression': ( 'SET investigationStatus = :investigationStatus, dateOfUpdate = :dateOfUpdate' ), + 'ConditionExpression': 'attribute_exists(pk)', 'ExpressionAttributeValues': { ':investigationStatus': {'S': InvestigationStatusEnum.UNDER_INVESTIGATION}, ':dateOfUpdate': {'S': investigation.creationDate.isoformat()}, }, } },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py(23 hunks)backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py(9 hunks)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/common.py(2 hunks)backend/compact-connect/lambdas/python/common/cc_common/utils.py(1 hunks)backend/compact-connect/lambdas/python/provider-data-v1/handlers/encumbrance.py(10 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/common.py
- backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py
🧰 Additional context used
🧠 Learnings (24)
📚 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/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). When working with encumbrance notes, use encumbranceDetails['clinicalPrivilegeActionCategory'], not encumbranceDetails['note'].
Applied to files:
backend/compact-connect/lambdas/python/provider-data-v1/handlers/encumbrance.pybackend/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/provider-data-v1/handlers/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/handlers/encumbrance.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/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/handlers/encumbrance.pybackend/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/provider-data-v1/handlers/encumbrance.pybackend/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/provider-data-v1/handlers/encumbrance.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.pybackend/compact-connect/lambdas/python/common/cc_common/utils.py
📚 Learning: 2025-08-15T22:26:08.128Z
Learnt from: jusdino
Repo: csg-org/CompactConnect PR: 1004
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/api.py:78-79
Timestamp: 2025-08-15T22:26:08.128Z
Learning: The encumbranceType field should not be included in public response schemas for adverse actions. It should only be available in internal/general response schemas, not in AdverseActionPublicResponseSchema.
Applied to files:
backend/compact-connect/lambdas/python/provider-data-v1/handlers/encumbrance.py
📚 Learning: 2025-06-18T05:57:18.225Z
Learnt from: jusdino
Repo: csg-org/CompactConnect PR: 864
File: backend/compact-connect/lambdas/python/common/cc_common/license_util.py:18-37
Timestamp: 2025-06-18T05:57:18.225Z
Learning: In the `get_license_type_by_abbreviation` method in `license_util.py`, KeyError exceptions from invalid compact codes are intentionally caught and re-raised as CCInvalidRequestException with `from e` to provide a consistent interface between invalid compact and invalid abbreviation scenarios, while preserving the original exception information in the chain.
Applied to files:
backend/compact-connect/lambdas/python/provider-data-v1/handlers/encumbrance.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/provider-data-v1/handlers/encumbrance.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/provider-data-v1/handlers/encumbrance.py
📚 Learning: 2025-10-29T19:11:33.003Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1167
File: backend/compact-connect/lambdas/python/provider-data-v1/handlers/investigation.py:85-95
Timestamp: 2025-10-29T19:11:33.003Z
Learning: InvestigationData extends CCDataClass which provides the licenseTypeAbbreviation property as a computed property (returns license type abbreviation if the instance has both 'compact' and 'licenseType' fields). Therefore, InvestigationData instances have access to licenseTypeAbbreviation through inheritance, and it does not need to be explicitly set when creating new investigation records.
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-10T13:50:30.047Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1069
File: backend/compact-connect/lambdas/python/purchases/handlers/transaction_reporting.py:18-22
Timestamp: 2025-09-10T13:50:30.047Z
Learning: When using Python StrEnum for filtering, the `in` operator correctly checks string values against enum members. `'settledSuccessfully' in ReportableTransactionStatuses` works properly and is more maintainable than using raw string sets. StrEnum members are both enum instances and strings, making them ideal for this use case.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-09-10T13:50:30.047Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1069
File: backend/compact-connect/lambdas/python/purchases/handlers/transaction_reporting.py:18-22
Timestamp: 2025-09-10T13:50:30.047Z
Learning: In Python 3.11, using `'string' in StrEnum` raises TypeError. StrEnum membership checking with strings only works starting in Python 3.12. For Python 3.11, convert StrEnum values to a set for membership testing or use explicit equality comparisons.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.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/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-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/cc_common/data_model/data_client.py
📚 Learning: 2025-09-03T23:06:45.015Z
Learnt from: ChiefStief
Repo: csg-org/CompactConnect PR: 1036
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py:2645-2651
Timestamp: 2025-09-03T23:06:45.015Z
Learning: PrivilegeData extends CCDataClass which provides the licenseTypeAbbreviation property, so PrivilegeData instances have access to licenseTypeAbbreviation through inheritance.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-09-03T23:06:45.015Z
Learnt from: ChiefStief
Repo: csg-org/CompactConnect PR: 1036
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py:2645-2651
Timestamp: 2025-09-03T23:06:45.015Z
Learning: PrivilegeData extends CCDataClass which provides the licenseTypeAbbreviation property as a computed property (returns license type abbreviation if the instance has both 'compact' and 'licenseType' fields). Therefore, PrivilegeData instances have access to licenseTypeAbbreviation through inheritance.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-06-10T03:16:16.896Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 848
File: backend/compact-connect/lambdas/python/migration/provider_user_pool_migration_551/main.py:35-39
Timestamp: 2025-06-10T03:16:16.896Z
Learning: In the provider user pool migration (provider_user_pool_migration_551), the FilterExpression intentionally only checks for the existence of compactConnectRegisteredEmailAddress. The migration should only remove currentHomeJurisdiction if compactConnectRegisteredEmailAddress is also present, targeting records that went through the old registration process. Records with only currentHomeJurisdiction but no compactConnectRegisteredEmailAddress should be left untouched.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.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/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
🧬 Code graph analysis (3)
backend/compact-connect/lambdas/python/provider-data-v1/handlers/encumbrance.py (6)
backend/compact-connect/lambdas/python/common/cc_common/utils.py (3)
api_handler(90-212)authorize_state_level_only_action(276-313)to_uuid(915-919)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/api.py (1)
AdverseActionPostRequestSchema(16-49)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/__init__.py (25)
compact(27-28)compact(31-32)jurisdiction(43-44)jurisdiction(47-48)AdverseActionData(14-147)licenseTypeAbbreviation(51-52)licenseTypeAbbreviation(55-56)licenseType(59-60)licenseType(63-64)actionAgainst(67-68)actionAgainst(71-72)encumbranceType(75-76)encumbranceType(79-80)clinicalPrivilegeActionCategory(84-85)clinicalPrivilegeActionCategory(88-91)clinicalPrivilegeActionCategories(94-95)clinicalPrivilegeActionCategories(98-99)effectiveStartDate(102-103)effectiveStartDate(106-107)submittingUser(110-111)submittingUser(114-115)creationDate(118-119)creationDate(122-123)adverseActionId(126-127)adverseActionId(130-131)backend/compact-connect/lambdas/python/common/cc_common/config.py (2)
expiration_resolution_date(221-228)current_standard_datetime(253-257)backend/compact-connect/lambdas/python/common/cc_common/license_util.py (2)
LicenseUtility(15-60)get_license_type_by_abbreviation(19-36)backend/compact-connect/lambdas/python/common/cc_common/feature_flag_client.py (1)
is_feature_enabled(47-117)
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py (6)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/common.py (3)
InvestigationAgainstEnum(287-293)InvestigationStatusEnum(337-339)licenseTypeAbbreviation(156-167)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/investigation/__init__.py (19)
InvestigationData(12-156)compact(71-72)compact(75-76)providerId(79-80)providerId(83-84)jurisdiction(87-88)jurisdiction(91-92)investigationAgainst(103-104)investigationAgainst(107-108)investigationId(111-112)investigationId(115-116)creationDate(127-128)creationDate(131-132)licenseType(95-96)licenseType(99-100)closeDate(135-136)closeDate(139-140)pk(42-48)sk(51-68)backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py (5)
ProviderRecordType(29-42)get_specific_license_record(459-474)get_specific_privilege_record(476-491)get_investigation_records_for_license(636-653)get_investigation_records_for_privilege(617-634)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/__init__.py (17)
AdverseActionData(14-147)adverseActionId(126-127)adverseActionId(130-131)effectiveLiftDate(134-135)effectiveLiftDate(138-139)compact(27-28)compact(31-32)providerId(35-36)providerId(39-40)jurisdiction(43-44)jurisdiction(47-48)licenseTypeAbbreviation(51-52)licenseTypeAbbreviation(55-56)creationDate(118-119)creationDate(122-123)licenseType(59-60)licenseType(63-64)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/__init__.py (9)
compact(28-29)compact(108-109)providerId(24-25)providerId(104-105)jurisdiction(32-33)jurisdiction(112-113)PrivilegeUpdateData(91-162)licenseType(40-41)licenseType(116-117)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/license/__init__.py (9)
compact(32-33)compact(164-165)providerId(28-29)providerId(160-161)jurisdiction(36-37)jurisdiction(168-169)LicenseUpdateData(140-193)licenseType(40-41)licenseType(172-173)
backend/compact-connect/lambdas/python/common/cc_common/utils.py (1)
backend/compact-connect/lambdas/python/common/cc_common/exceptions.py (1)
CCInvalidRequestException(7-8)
⏰ 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 (12)
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py (4)
5-5: LGTM!The new imports for investigation functionality are correctly added and properly organized.
Also applies to: 26-27, 33-33
180-180: LGTM!The UUID type updates improve type safety and are consistently applied across all affected methods.
Also applies to: 1226-1232, 1253-1266, 1903-1903, 2051-2063
596-596: LGTM!Excellent use of the
ProviderRecordTypeenum throughout the file instead of string literals. This improves type safety and maintainability.Also applies to: 737-737, 917-917, 1054-1054, 1454-1454, 1649-1649, 1662-1662, 1765-1765, 1789-1789, 2015-2015, 2533-2533, 2617-2617, 2693-2693, 2837-2837, 2995-2995, 3028-3028, 3150-3150, 3241-3241, 3386-3386
1875-1875: Fix UpdateExpression clause order.DynamoDB requires SET clauses to appear before REMOVE clauses. The current expression will cause a runtime error.
Apply this diff:
- 'UpdateExpression': 'REMOVE investigationStatus SET dateOfUpdate = :dateOfUpdate', + 'UpdateExpression': 'SET dateOfUpdate = :dateOfUpdate REMOVE investigationStatus',⛔ Skipped due to learnings
Learnt from: landonshumway-ia Repo: csg-org/CompactConnect PR: 1014 File: backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py:3058-3066 Timestamp: 2025-08-18T21:57:32.054Z Learning: DynamoDB UpdateExpression clauses (SET, REMOVE, ADD, DELETE) can appear in any syntactic order in the expression string. DynamoDB processes them in a fixed evaluation order (SET → REMOVE → ADD → DELETE) internally, but the textual order in the UpdateExpression string is flexible. Expressions like "REMOVE attr1 SET attr2 = :val" are syntactically valid and will not throw ValidationExceptions.backend/compact-connect/lambdas/python/common/cc_common/utils.py (1)
915-919: LGTM!Clean utility function that standardizes UUID parsing with consistent error handling across the codebase.
backend/compact-connect/lambdas/python/provider-data-v1/handlers/encumbrance.py (7)
57-62: LGTM!Proper request body validation with appropriate error handling.
65-66: LGTM!Clean helper function for extracting the submitting user ID.
130-161: LGTM with dependencies on fixes.The function structure and flow are correct. However, it depends on fixes to
_generate_adverse_action_for_record_typeto function properly.
164-182: LGTM!Good refactoring that separates parameter parsing from business logic. The internal function returns the adverse action ID for use by other flows (like investigation closure) while maintaining API compatibility with the existing
{'message': 'OK'}response.
185-217: LGTM with dependencies on fixes.The function structure mirrors the privilege encumbrance handler appropriately. However, it depends on fixes to
_generate_adverse_action_for_record_typeto function properly.
220-238: LGTM!Consistent refactoring pattern with the privilege encumbrance handler. Maintains API compatibility while enabling reuse of the internal function.
251-254: LGTM!Good improvement using the new
to_uuidutility for consistent UUID validation and error messaging in the lifting handlers.Also applies to: 303-306
backend/compact-connect/lambdas/python/provider-data-v1/handlers/encumbrance.py
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
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)
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py (1)
180-207: Type hint change to UUID for provider_id — handler call sites must convert strings to UUIDMethod signature requires
provider_id: UUID, but handlers pass strings from API Gateway path parameters. While f-string coercion masks this, the type mismatch creates type safety issues. Handler code instate_api.py:150and__init__.py:16must convert the string parameter to UUID before calling, matching the pattern intest_data_client.py:71.
🧹 Nitpick comments (4)
backend/compact-connect/lambdas/python/provider-data-v1/handlers/bulk_upload.py (1)
166-170: Consider using positional argument for consistency.For consistency with line 175 (which uses a positional argument), consider passing the message as a positional argument rather than using the
message=keyword.Apply this diff:
- raise ValidationError( - message=f'Duplicate License SSN detected. SSN matches with record ' - f'{matched_ssn_index}. Every record must have a unique SSN within the same ' - f'file.' - ) + raise ValidationError( + f'Duplicate License SSN detected. SSN matches with record ' + f'{matched_ssn_index}. Every record must have a unique SSN within the same ' + f'file.' + )backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py (1)
617-671: Consider using enum constants instead of magic strings.The methods correctly filter investigations by comparing
record.investigationAgainstagainst string literals'privilege'and'license'. While this works (since the property returns astr), importing and usingInvestigationAgainstEnum.PRIVILEGE.valueandInvestigationAgainstEnum.LICENSE.valuewould improve maintainability and reduce the risk of typos.The filtering logic for active vs. closed investigations is correct: the condition
include_closed or record.closeDate is Noneproperly excludes closed investigations by default.Apply this diff to use enum constants:
+from cc_common.data_model.schema.investigation import InvestigationAgainstEnum + class ProviderRecordUtility: ... def get_investigation_records_for_privilege( self, privilege_jurisdiction: str, privilege_license_type_abbreviation: str, filter_condition: Callable[[InvestigationData], bool] | None = None, include_closed: bool = False, ) -> list[InvestigationData]: ... return [ record for record in self._investigation_records - if record.investigationAgainst == 'privilege' + if record.investigationAgainst == InvestigationAgainstEnum.PRIVILEGE.value and record.jurisdiction == privilege_jurisdiction ... ] def get_investigation_records_for_license( self, license_jurisdiction: str, license_type_abbreviation: str, filter_condition: Callable[[InvestigationData], bool] | None = None, include_closed: bool = False, ) -> list[InvestigationData]: ... return [ record for record in self._investigation_records - if record.investigationAgainst == 'license' + if record.investigationAgainst == InvestigationAgainstEnum.LICENSE.value and record.jurisdiction == license_jurisdiction ... ]backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py (1)
175-183: Standardize TableName accessors for consistencyMix of self.config.provider_table_name, self.config.provider_table.name, and .table_name. Consider standardizing on one accessor to reduce cognitive load and avoid future drift.
Also applies to: 641-657, 1180-1182, 1694-1707, 1833-1841, 2550-2571, 2860-2893
backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_investigation.py (1)
418-475: Test helper reuse opportunity (optional)Repeated “create investigation → fetch id → PATCH close” setup could be a small helper to DRY tests and speed future additions.
Also applies to: 622-679, 835-915, 1133-1149, 980-999, 1231-1242
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
.github/workflows/check-compact-connect-ui-app.yml(1 hunks)backend/compact-connect-ui-app/cdk.json(1 hunks)backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py(23 hunks)backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py(9 hunks)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/license/__init__.py(1 hunks)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/__init__.py(1 hunks)backend/compact-connect/lambdas/python/common/cc_common/feature_flag_client.py(1 hunks)backend/compact-connect/lambdas/python/common/cc_common/utils.py(2 hunks)backend/compact-connect/lambdas/python/common/common_test/test_data_generator.py(2 hunks)backend/compact-connect/lambdas/python/common/tests/function/test_data_client.py(9 hunks)backend/compact-connect/lambdas/python/data-events/handlers/investigation_events.py(1 hunks)backend/compact-connect/lambdas/python/data-events/tests/function/test_investigation_events.py(1 hunks)backend/compact-connect/lambdas/python/provider-data-v1/handlers/bulk_upload.py(1 hunks)backend/compact-connect/lambdas/python/provider-data-v1/handlers/encumbrance.py(10 hunks)backend/compact-connect/lambdas/python/provider-data-v1/tests/function/__init__.py(1 hunks)backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_investigation.py(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- backend/compact-connect/lambdas/python/common/cc_common/feature_flag_client.py
🚧 Files skipped from review as they are similar to previous changes (2)
- backend/compact-connect/lambdas/python/common/common_test/test_data_generator.py
- backend/compact-connect/lambdas/python/provider-data-v1/tests/function/init.py
🧰 Additional context used
🧠 Learnings (42)
📓 Common learnings
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.
📚 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-ui-app/cdk.json
📚 Learning: 2025-06-09T22:03:03.232Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 852
File: backend/compact-connect/stacks/persistent_stack/__init__.py:545-549
Timestamp: 2025-06-09T22:03:03.232Z
Learning: In the CompactConnect codebase, production active_compact_member_jurisdictions configurations are very stable and unlikely to be removed. The vast majority of configuration errors in get_list_of_active_jurisdictions_for_compact_environment() will occur when developers are deploying sandbox environments, so error messages that primarily reference sandbox configuration are appropriate for the typical use case.
Applied to files:
backend/compact-connect-ui-app/cdk.json
📚 Learning: 2025-06-26T16:42:00.781Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 882
File: backend/compact-connect/lambdas/python/migration/compact_configured_states_871/main.py:127-130
Timestamp: 2025-06-26T16:42:00.781Z
Learning: In the compact_configured_states_871 migration, existing jurisdiction configurations that have licenseeRegistrationEnabled: true are migrated with isLive: true for backwards compatibility. This prevents breaking existing live functionality since those states are already operational. The migration preserves the current live state of jurisdictions to maintain service continuity, while new states added after migration can start with isLive: false and be controlled by compact admins.
Applied to files:
backend/compact-connect-ui-app/cdk.json
📚 Learning: 2025-08-29T19:41:38.201Z
Learnt from: jusdino
Repo: csg-org/CompactConnect PR: 1005
File: backend/compact-connect/app_clients/README.md:178-187
Timestamp: 2025-08-29T19:41:38.201Z
Learning: In the CompactConnect system, "state" and "jurisdiction" are functionally synonymous and can be used interchangeably in documentation and code. Users consider consistency between these terms to be a non-issue.
Applied to files:
backend/compact-connect-ui-app/cdk.json
📚 Learning: 2025-10-10T18:45:43.801Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 1114
File: backend/compact-connect-ui-app/stacks/frontend_deployment_stack/deployment.py:34-35
Timestamp: 2025-10-10T18:45:43.801Z
Learning: In backend/compact-connect-ui-app/stacks/frontend_deployment_stack/deployment.py, the cdk.context.deploy-example.json file is intentionally minimal and serves as a template, not for actual deployments. Actual deployment environments (test, beta, prod, sandbox) contain all required environment-specific keys like statsig_key, app_env, recaptcha_public_key, and robots_meta. This is by design.
Applied to files:
backend/compact-connect-ui-app/cdk.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).
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/__init__.pybackend/compact-connect/lambdas/python/provider-data-v1/handlers/encumbrance.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/schema/license/__init__.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-10-29T19:11:33.003Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1167
File: backend/compact-connect/lambdas/python/provider-data-v1/handlers/investigation.py:85-95
Timestamp: 2025-10-29T19:11:33.003Z
Learning: InvestigationData extends CCDataClass which provides the licenseTypeAbbreviation property as a computed property (returns license type abbreviation if the instance has both 'compact' and 'licenseType' fields). Therefore, InvestigationData instances have access to licenseTypeAbbreviation through inheritance, and it does not need to be explicitly set when creating new investigation records.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/__init__.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/schema/license/__init__.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-09-11T20:06:40.709Z
Learnt from: ChiefStief
Repo: csg-org/CompactConnect PR: 1036
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py:370-383
Timestamp: 2025-09-11T20:06:40.709Z
Learning: The EncumbranceDetailsSchema in backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/record.py does not contain a 'note' field. It only has clinicalPrivilegeActionCategory (String, optional), adverseActionId (UUID, required), and licenseJurisdiction (Jurisdiction, optional). When working with encumbrance notes, use encumbranceDetails['clinicalPrivilegeActionCategory'], not encumbranceDetails['note'].
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/__init__.pybackend/compact-connect/lambdas/python/provider-data-v1/handlers/encumbrance.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.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/provider-data-v1/tests/function/test_handlers/test_investigation.pybackend/compact-connect/lambdas/python/data-events/handlers/investigation_events.pybackend/compact-connect/lambdas/python/data-events/tests/function/test_investigation_events.pybackend/compact-connect/lambdas/python/provider-data-v1/handlers/encumbrance.pybackend/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/provider-data-v1/tests/function/test_handlers/test_investigation.pybackend/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/provider-data-v1/tests/function/test_handlers/test_investigation.pybackend/compact-connect/lambdas/python/data-events/handlers/investigation_events.pybackend/compact-connect/lambdas/python/provider-data-v1/handlers/encumbrance.pybackend/compact-connect/lambdas/python/common/tests/function/test_data_client.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-06-04T20:05:18.245Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 824
File: backend/compact-connect/tests/app/test_event_listener.py:18-27
Timestamp: 2025-06-04T20:05:18.245Z
Learning: In the CompactConnect test suite, the established pattern is to let file operations in test setup methods raise natural exceptions (FileNotFoundError, JSONDecodeError) rather than adding explicit error handling. This approach maintains consistency across tests and provides sufficient error information for debugging.
Applied to files:
backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_investigation.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/provider-data-v1/tests/function/test_handlers/test_investigation.py
📚 Learning: 2025-05-23T14:22:32.283Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 796
File: backend/compact-connect/lambdas/python/common/tests/function/__init__.py:32-37
Timestamp: 2025-05-23T14:22:32.283Z
Learning: TestDataGenerator is designed as a utility class with all static methods, and should be used by assigning the class itself rather than instantiating it.
Applied to files:
backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_investigation.py
📚 Learning: 2025-05-23T14:15:40.107Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 796
File: backend/compact-connect/lambdas/python/purchases/tests/function/__init__.py:27-31
Timestamp: 2025-05-23T14:15:40.107Z
Learning: The TestDataGenerator class in CompactConnect contains only static methods and should be used directly without instantiation (e.g., TestDataGenerator.method_name() rather than creating an instance first).
Applied to files:
backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_investigation.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/provider-data-v1/tests/function/test_handlers/test_investigation.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-08-19T02:07:26.373Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1014
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py:3037-3041
Timestamp: 2025-08-19T02:07:26.373Z
Learning: In the CompactConnect codebase, when provider records are expected to exist based on successful license record matching (such as in account recovery flows), missing provider records should raise CCInternalException rather than CCNotFoundException, as this indicates a data consistency issue that requires system alerts rather than treating it as a user error.
Applied to files:
backend/compact-connect/lambdas/python/data-events/handlers/investigation_events.py
📚 Learning: 2025-08-19T02:09:33.033Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1014
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py:3068-3071
Timestamp: 2025-08-19T02:09:33.033Z
Learning: In the CompactConnect account recovery flow, when clearing provider account recovery data after successful verification, if the provider record is unexpectedly missing (ConditionalCheckFailedException), it should raise CCInternalException rather than CCNotFoundException because the provider was confirmed to exist during the recovery token verification step, indicating a data consistency issue requiring system alerts.
Applied to files:
backend/compact-connect/lambdas/python/data-events/handlers/investigation_events.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/data-events/handlers/investigation_events.pybackend/compact-connect/lambdas/python/provider-data-v1/handlers/encumbrance.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 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:
.github/workflows/check-compact-connect-ui-app.yml
📚 Learning: 2025-08-25T22:39:05.553Z
Learnt from: jusdino
Repo: csg-org/CompactConnect PR: 1005
File: backend/compact-connect/tests/smoke/hmac_auth_smoke_tests.py:19-25
Timestamp: 2025-08-25T22:39:05.553Z
Learning: In the CompactConnect project, CLI scripts are written with the convention of assuming a specific current working directory (CWD), so CWD-relative paths in sys.path adjustments are intentional and acceptable within this codebase's standards.
Applied to files:
.github/workflows/check-compact-connect-ui-app.yml
📚 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/handlers/encumbrance.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/handlers/encumbrance.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/handlers/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/handlers/encumbrance.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-08-15T22:26:08.128Z
Learnt from: jusdino
Repo: csg-org/CompactConnect PR: 1004
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/api.py:78-79
Timestamp: 2025-08-15T22:26:08.128Z
Learning: The encumbranceType field should not be included in public response schemas for adverse actions. It should only be available in internal/general response schemas, not in AdverseActionPublicResponseSchema.
Applied to files:
backend/compact-connect/lambdas/python/provider-data-v1/handlers/encumbrance.py
📚 Learning: 2025-06-18T05:57:18.225Z
Learnt from: jusdino
Repo: csg-org/CompactConnect PR: 864
File: backend/compact-connect/lambdas/python/common/cc_common/license_util.py:18-37
Timestamp: 2025-06-18T05:57:18.225Z
Learning: In the `get_license_type_by_abbreviation` method in `license_util.py`, KeyError exceptions from invalid compact codes are intentionally caught and re-raised as CCInvalidRequestException with `from e` to provide a consistent interface between invalid compact and invalid abbreviation scenarios, while preserving the original exception information in the chain.
Applied to files:
backend/compact-connect/lambdas/python/provider-data-v1/handlers/encumbrance.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/provider-data-v1/handlers/encumbrance.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-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/common/tests/function/test_data_client.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/common/tests/function/test_data_client.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/common/tests/function/test_data_client.py
📚 Learning: 2025-09-10T13:50:30.047Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1069
File: backend/compact-connect/lambdas/python/purchases/handlers/transaction_reporting.py:18-22
Timestamp: 2025-09-10T13:50:30.047Z
Learning: When using Python StrEnum for filtering, the `in` operator correctly checks string values against enum members. `'settledSuccessfully' in ReportableTransactionStatuses` works properly and is more maintainable than using raw string sets. StrEnum members are both enum instances and strings, making them ideal for this use case.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-09-10T13:50:30.047Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1069
File: backend/compact-connect/lambdas/python/purchases/handlers/transaction_reporting.py:18-22
Timestamp: 2025-09-10T13:50:30.047Z
Learning: In Python 3.11, using `'string' in StrEnum` raises TypeError. StrEnum membership checking with strings only works starting in Python 3.12. For Python 3.11, convert StrEnum values to a set for membership testing or use explicit equality comparisons.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.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/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-09-03T23:06:45.015Z
Learnt from: ChiefStief
Repo: csg-org/CompactConnect PR: 1036
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py:2645-2651
Timestamp: 2025-09-03T23:06:45.015Z
Learning: PrivilegeData extends CCDataClass which provides the licenseTypeAbbreviation property, so PrivilegeData instances have access to licenseTypeAbbreviation through inheritance.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-09-03T23:06:45.015Z
Learnt from: ChiefStief
Repo: csg-org/CompactConnect PR: 1036
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py:2645-2651
Timestamp: 2025-09-03T23:06:45.015Z
Learning: PrivilegeData extends CCDataClass which provides the licenseTypeAbbreviation property as a computed property (returns license type abbreviation if the instance has both 'compact' and 'licenseType' fields). Therefore, PrivilegeData instances have access to licenseTypeAbbreviation through inheritance.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-06-10T03:16:16.896Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 848
File: backend/compact-connect/lambdas/python/migration/provider_user_pool_migration_551/main.py:35-39
Timestamp: 2025-06-10T03:16:16.896Z
Learning: In the provider user pool migration (provider_user_pool_migration_551), the FilterExpression intentionally only checks for the existence of compactConnectRegisteredEmailAddress. The migration should only remove currentHomeJurisdiction if compactConnectRegisteredEmailAddress is also present, targeting records that went through the old registration process. Records with only currentHomeJurisdiction but no compactConnectRegisteredEmailAddress should be left untouched.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.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/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
🧬 Code graph analysis (10)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/__init__.py (2)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/license/__init__.py (1)
investigationStatus(140-141)backend/compact-connect/lambdas/python/common/cc_common/utils.py (1)
get(86-87)
backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_investigation.py (5)
backend/compact-connect/lambdas/python/provider-data-v1/tests/function/__init__.py (2)
TstFunction(22-443)assertDictPartialMatch(25-30)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/common.py (5)
ClinicalPrivilegeActionCategory(393-406)EncumbranceType(371-390)serialize_to_database_record(205-208)InvestigationStatusEnum(337-339)update(182-203)backend/compact-connect/lambdas/python/common/common_test/test_data_generator.py (5)
put_default_provider_record_in_provider_table(440-460)generate_default_privilege(310-332)store_record_in_provider_table(335-338)generate_test_api_event(34-62)generate_default_license(233-266)backend/compact-connect/lambdas/python/provider-data-v1/handlers/investigation.py (1)
investigation_handler(41-56)backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py (7)
get_investigation_records_for_privilege(617-643)get_privilege_records(493-501)get_investigation_records_for_license(645-671)get_license_records(503-510)get_adverse_action_records_for_privilege(599-615)get_update_records_for_privilege(782-801)get_update_records_for_license(761-780)
backend/compact-connect/lambdas/python/data-events/handlers/investigation_events.py (8)
backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py (5)
ProviderUserRecords(412-871)get_provider_record(81-86)get_provider_record(673-682)get_license_records(503-510)get_privilege_records(493-501)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/data_event/api.py (1)
InvestigationEventDetailSchema(57-63)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/provider/__init__.py (1)
ProviderData(13-148)backend/compact-connect/lambdas/python/common/cc_common/config.py (2)
email_service_client(298-303)data_client(42-45)backend/compact-connect/lambdas/python/common/cc_common/email_service_client.py (5)
InvestigationNotificationTemplateVariables(28-37)send_license_investigation_state_notification_email(624-655)send_license_investigation_closed_state_notification_email(657-688)send_privilege_investigation_state_notification_email(690-721)send_privilege_investigation_closed_state_notification_email(723-754)backend/compact-connect/lambdas/python/common/cc_common/license_util.py (2)
LicenseUtility(15-60)get_license_type_by_abbreviation(19-36)backend/compact-connect/lambdas/python/common/cc_common/utils.py (2)
sqs_handler(433-468)get(86-87)backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py (1)
get_provider_user_records(176-207)
backend/compact-connect/lambdas/python/data-events/tests/function/test_investigation_events.py (5)
backend/compact-connect/lambdas/python/provider-data-v1/tests/function/__init__.py (1)
TstFunction(22-443)backend/compact-connect/lambdas/python/common/cc_common/config.py (1)
email_service_client(298-303)backend/compact-connect/lambdas/python/common/cc_common/email_service_client.py (1)
InvestigationNotificationTemplateVariables(28-37)backend/compact-connect/lambdas/python/data-events/handlers/investigation_events.py (4)
license_investigation_notification_listener(141-199)license_investigation_closed_notification_listener(203-267)privilege_investigation_notification_listener(271-329)privilege_investigation_closed_notification_listener(333-397)backend/compact-connect/lambdas/python/common/common_test/test_data_generator.py (3)
put_default_provider_record_in_provider_table(440-460)put_default_license_record_in_provider_table(269-279)put_default_privilege_record_in_provider_table(341-351)
backend/compact-connect/lambdas/python/provider-data-v1/handlers/encumbrance.py (1)
backend/compact-connect/lambdas/python/common/cc_common/utils.py (3)
api_handler(90-212)authorize_state_level_only_action(276-313)to_uuid(916-920)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/license/__init__.py (2)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/__init__.py (1)
investigationStatus(84-85)backend/compact-connect/lambdas/python/common/cc_common/utils.py (1)
get(86-87)
backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py (2)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/investigation/__init__.py (7)
InvestigationData(12-156)investigationAgainst(103-104)investigationAgainst(107-108)jurisdiction(87-88)jurisdiction(91-92)closeDate(135-136)closeDate(139-140)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/common.py (3)
update(182-203)UpdateCategory(296-312)licenseTypeAbbreviation(156-167)
backend/compact-connect/lambdas/python/common/cc_common/utils.py (1)
backend/compact-connect/lambdas/python/common/cc_common/exceptions.py (1)
CCInvalidRequestException(7-8)
backend/compact-connect/lambdas/python/common/tests/function/test_data_client.py (5)
backend/compact-connect/lambdas/python/common/tests/function/__init__.py (2)
setUp(23-36)_load_provider_data(218-233)backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_data_model/test_client.py (1)
_load_provider_data(215-230)backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py (2)
create_investigation(1612-1714)close_investigation(1716-1899)backend/compact-connect/lambdas/python/common/common_test/test_data_generator.py (1)
generate_default_investigation(155-173)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/investigation/__init__.py (11)
investigationId(111-112)investigationId(115-116)submittingUser(119-120)submittingUser(123-124)creationDate(127-128)creationDate(131-132)InvestigationData(12-156)compact(71-72)compact(75-76)jurisdiction(87-88)jurisdiction(91-92)
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py (4)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/common.py (4)
InvestigationAgainstEnum(287-293)InvestigationStatusEnum(337-339)UpdateCategory(296-312)licenseTypeAbbreviation(156-167)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/investigation/__init__.py (19)
InvestigationData(12-156)compact(71-72)compact(75-76)providerId(79-80)providerId(83-84)jurisdiction(87-88)jurisdiction(91-92)investigationAgainst(103-104)investigationAgainst(107-108)investigationId(111-112)investigationId(115-116)creationDate(127-128)creationDate(131-132)licenseType(95-96)licenseType(99-100)closeDate(135-136)closeDate(139-140)pk(42-48)sk(51-68)backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py (5)
ProviderRecordType(29-42)get_specific_license_record(459-474)get_specific_privilege_record(476-491)get_investigation_records_for_license(645-671)get_investigation_records_for_privilege(617-643)backend/compact-connect/lambdas/python/common/cc_common/exceptions.py (1)
CCNotFoundException(32-33)
🪛 GitHub Actions: Check-CompactConnect
backend/compact-connect/lambdas/python/common/tests/function/test_data_client.py
[error] 1085-1085: SubmittingUser in privilege investigation record is stored as a string, but test expects a UUID object (mismatch between data types). Investigate how 'submittingUser' is assigned in create_privilege_investigation.
🔇 Additional comments (15)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/license/__init__.py (1)
139-141: LGTM! Consistent implementation of optional property.The
investigationStatusproperty follows the established pattern for optional fields in this read-only class and matches the identical implementation in the privilege schema..github/workflows/check-compact-connect-ui-app.yml (1)
106-106: Verify necessity of expanded dependency scope in CI/CD workflow.The working directory changed to install all Python dependencies for
backend/compact-connectrather than justbackend/compact-connect-ui-app. While the pip constraint (pip<25.3) appears to address a compatibility issue, the scope expansion warrants verification.Can you confirm:
- Why all project Python dependencies are needed for UI app tests (previously scoped to ui-app only)?
- Whether the pip version constraint addresses a specific known issue or is a temporary workaround?
- Whether this change was necessary to support investigation-related tests?
backend/compact-connect-ui-app/cdk.json (1)
22-34: Configuration additions look well-formed.The jurisdictions array (53 entries covering all US states/territories) and compacts array (aslp, octp, coun) appear correctly formatted and complete. These values should support the investigation feature's jurisdiction and compact validation requirements.
Please verify that these context values are actually consumed by investigation-related code (e.g., in Lambda handlers or resource definitions).
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/__init__.py (1)
83-85: LGTM!The
investigationStatusproperty correctly implements an optional read-only field following the established pattern for other status properties in this class (encumberedStatus,homeJurisdictionChangeStatus,licenseDeactivatedStatus). The implementation is consistent with the identical property added toLicenseDataand properly typed asstr | None.backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py (3)
20-20: LGTM! Investigation record type integration.The integration of investigation support follows the established pattern for other record types (adverse actions, licenses, privileges). The import, enum addition, field declaration, and initialization logic are all consistent with the existing architecture.
Also applies to: 42-42, 427-427, 443-444
260-267: LGTM! Appropriate filtering of investigation events from privilege history.Investigation updates are correctly excluded from the privilege history API response since investigations are displayed separately in the
investigationsarray. This prevents duplicate information and keeps the history focused on status changes. The comment clearly documents this design decision.
814-838: LGTM! API response correctly includes investigations.The
generate_api_response_objectmethod now properly populates theinvestigationsarray for both licenses and privileges. By calling the getter methods without theinclude_closedparameter, only active investigations (those without acloseDate) are returned in the API response, which aligns with the PR objectives.The implementation follows the same pattern as the existing
adverseActionspopulation.Also applies to: 840-858
backend/compact-connect/lambdas/python/common/tests/function/test_data_client.py (1)
1079-1081: Fix expectedsubmittingUsertype in privilege investigation test.
InvestigationData.serialize_to_database_record()storessubmittingUseras a string, so the Dynamo record under test contains a string. The current assertion compares it to the UUID object returned frominvestigation.submittingUser, which triggers the pipeline failure (“SubmittingUser … stored as a string, but test expects a UUID object”). Cast tostr(...)here, just as the license investigation test already does, so the expectation matches the persisted value.- 'submittingUser': investigation.submittingUser, + 'submittingUser': str(investigation.submittingUser),⛔ Skipped due to learnings
Learnt from: landonshumway-ia Repo: csg-org/CompactConnect PR: 1135 File: backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py:2642-2654 Timestamp: 2025-10-15T19:53:00.422Z Learning: In backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py and provider_record_util.py, the get_adverse_action_by_id method correctly handles UUID vs string comparison as written. Attempting to add explicit type conversion causes the mapping to fail. The current implementation should not be changed to add UUID/string conversion logic.backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py (4)
1612-1715: create_investigation flow looks solidGood use of ProviderUserRecords, typed AVs, and ConditionExpression to prevent phantom writes. StrEnum usage aligns with repo conventions. LGTM.
1752-1866: close_investigation: transactional guards and last-open logic look correctConditionally removing investigationStatus only when last open is appropriate; investigation record update guarded by attribute_not_exists(closeDate) is good. After fixing UpdateExpression ordering (see separate comment), this block looks good.
Please confirm handlers create any resulting encumbrance in the same overall operation (before removing investigationStatus) to meet the “no gap” objective.
596-596: Enum-typed record “type” fields and update records — LGTMReplacing string literals with ProviderRecordType and UpdateCategory improves safety and readability; record serialization patterns are consistent.
Also applies to: 736-736, 917-917, 1054-1071, 1454-1473, 2016-2031, 2534-2543, 2694-2705, 2838-2849, 2997-3010, 3030-3041, 3151-3164
1876-1881: Fix DynamoDB UpdateExpression order: SET must precede REMOVEDynamoDB requires clause order: SET, REMOVE, ADD, DELETE. Current expressions use REMOVE before SET and will fail at runtime.
Apply these diffs:
@@ - 'UpdateExpression': 'REMOVE investigationStatus SET dateOfUpdate = :dateOfUpdate', + 'UpdateExpression': 'SET dateOfUpdate = :dateOfUpdate REMOVE investigationStatus',@@ - UpdateExpression=( - 'REMOVE pendingEmailAddress, emailVerificationCode, emailVerificationExpiry ' - 'SET dateOfUpdate = :date_of_update' - ), + UpdateExpression=( + 'SET dateOfUpdate = :date_of_update ' + 'REMOVE pendingEmailAddress, emailVerificationCode, emailVerificationExpiry' + ),@@ - UpdateExpression=('REMOVE recoveryToken, recoveryExpiry SET dateOfUpdate = :date_of_update'), + UpdateExpression=('SET dateOfUpdate = :date_of_update REMOVE recoveryToken, recoveryExpiry'),Also applies to: 3348-3353, 3497-3501
⛔ Skipped due to learnings
Learnt from: landonshumway-ia Repo: csg-org/CompactConnect PR: 1014 File: backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py:3058-3066 Timestamp: 2025-08-18T21:57:32.054Z Learning: DynamoDB UpdateExpression clauses (SET, REMOVE, ADD, DELETE) can appear in any syntactic order in the expression string. DynamoDB processes them in a fixed evaluation order (SET → REMOVE → ADD → DELETE) internally, but the textual order in the UpdateExpression string is flexible. Expressions like "REMOVE attr1 SET attr2 = :val" are syntactically valid and will not throw ValidationExceptions.backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_investigation.py (3)
54-61: Good shift to TestDataGenerator and in-repo fixturesTests consistently use TestDataGenerator utilities; avoids JSON fixture churn and matches team practice. LGTM.
Also applies to: 229-236, 410-416, 615-621, 818-824, 1051-1057
84-101: Event publishing assertions are precise and resilientPartial matching dynamic IDs and asserting detail payloads keep contracts tight. Nice coverage of failure paths with patched _publish_event.
Also applies to: 263-279, 476-491, 680-695, 826-844, 1059-1082
131-132: Enum assertions on dataclass properties — LGTMComparing InvestigationStatusEnum to dataclass fields (not API dicts) is type-correct here.
Also applies to: 311-312, 927-930, 1161-1163
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
backend/compact-connect/lambdas/python/common/tests/function/test_data_client.py (1)
1036-1918: Excellent test coverage for investigation lifecycle.These tests comprehensively cover the investigation creation and closing workflows for both privileges and licenses, including error cases and the encumbrance workflow. The test structure is clear and assertions are thorough.
One minor observation: In
test_close_privilege_investigation_success(lines 1456-1458), there's a conditional pop forcompactTransactionIdGSIPK, but the corresponding license testtest_close_license_investigation_success(lines 1600-1602) doesn't include this safeguard. For consistency and to prevent potential test flakiness if the field is sometimes present, consider adding the conditional pop to the license closure test as well:# Pop dynamic fields that we don't want to assert on closure_update.pop('dateOfUpdate') closure_update.pop('sk') # Only pop compactTransactionIdGSIPK if it exists if 'compactTransactionIdGSIPK' in closure_update: closure_update.pop('compactTransactionIdGSIPK')
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
backend/compact-connect-ui-app/cdk.json(1 hunks)backend/compact-connect/lambdas/python/common/tests/function/test_data_client.py(9 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/compact-connect-ui-app/cdk.json
🧰 Additional context used
🧠 Learnings (4)
📚 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/function/test_data_client.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/common/tests/function/test_data_client.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/common/tests/function/test_data_client.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/common/tests/function/test_data_client.py
🧬 Code graph analysis (1)
backend/compact-connect/lambdas/python/common/tests/function/test_data_client.py (4)
backend/compact-connect/lambdas/python/common/tests/function/__init__.py (2)
setUp(23-36)_load_provider_data(218-233)backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py (2)
create_investigation(1612-1714)close_investigation(1716-1899)backend/compact-connect/lambdas/python/common/common_test/test_data_generator.py (1)
generate_default_investigation(155-173)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/investigation/__init__.py (11)
investigationId(111-112)investigationId(115-116)submittingUser(119-120)submittingUser(123-124)creationDate(127-128)creationDate(131-132)InvestigationData(12-156)compact(71-72)compact(75-76)jurisdiction(87-88)jurisdiction(91-92)
⏰ 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: TestApp
🔇 Additional comments (2)
backend/compact-connect/lambdas/python/common/tests/function/test_data_client.py (2)
19-21: LGTM! Helpful addition for test debugging.Setting
maxDiff = Noneenables full diff output in assertion failures, which is valuable for debugging tests with complex data structures like these investigation records.
68-87: LGTM! Consistent UUID handling.The return type change from
strtoUUIDaligns with the broader type-safety improvements in this PR. The string conversions at lines 732, 776, 797, 873, and 895 correctly handle the UUID-to-string conversion needed for DynamoDB storage.
landonshumway-ia
requested changes
Nov 5, 2025
Collaborator
landonshumway-ia
left a comment
landonshumway-ia
left a comment
There was a problem hiding this comment.
Looks awesome. Just a couple of nits with using keywords for the args.
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
Outdated
Show resolved
Hide resolved
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
Outdated
Show resolved
Hide resolved
.../compact-connect/lambdas/python/common/cc_common/data_model/schema/investigation/__init__.py
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
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)
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py (1)
2046-2055: Type mismatch:provider_idshould beUUID.The
lift_license_encumbrancemethod signature declaresprovider_id: str(line 2049), but it's passed toget_provider_user_recordson line 2082, which now expectsUUID(line 180). This creates a type inconsistency.Apply this diff to align with the updated signature:
def lift_license_encumbrance( self, compact: str, - provider_id: str, + provider_id: UUID, jurisdiction: str, license_type_abbreviation: str, adverse_action_id: UUID, effective_lift_date: date, lifting_user: str, ) -> None: """ Lift an encumbrance from a license record by updating the adverse action record and potentially updating the license record's encumbered status. - :param str provider_id: The provider ID + :param UUID provider_id: The provider ID
🧹 Nitpick comments (1)
backend/compact-connect/lambdas/python/common/cc_common/utils.py (1)
916-923: Add explanatory comment per previous review.The implementation is correct and provides a clean, reusable way to validate UUID path parameters. However, the previous reviewer requested documentation explaining why this should be used for all IDs in path params to prevent bogus values from breaking things.
Consider enhancing the docstring:
def to_uuid(uuid: str, on_error: str) -> UUID: """ - Parse a str to a UUID, raising CCInvalidRequestException if invalid. + Parse a str to a UUID, raising CCInvalidRequestException if invalid. + + This should be used for all UUID path parameters to validate and normalize + input before processing, preventing malformed UUIDs from causing unexpected + errors deeper in the application. + + :param str uuid: The string representation of a UUID to parse + :param str on_error: Custom error message to include in the exception + :return: A validated UUID object + :raises CCInvalidRequestException: If the string is not a valid UUID """Based on past review comments.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py(23 hunks)backend/compact-connect/lambdas/python/common/cc_common/utils.py(2 hunks)
🧰 Additional context used
🧠 Learnings (19)
📚 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-29T19:11:33.003Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1167
File: backend/compact-connect/lambdas/python/provider-data-v1/handlers/investigation.py:85-95
Timestamp: 2025-10-29T19:11:33.003Z
Learning: InvestigationData extends CCDataClass which provides the licenseTypeAbbreviation property as a computed property (returns license type abbreviation if the instance has both 'compact' and 'licenseType' fields). Therefore, InvestigationData instances have access to licenseTypeAbbreviation through inheritance, and it does not need to be explicitly set when creating new investigation records.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-09-11T20:06:40.709Z
Learnt from: ChiefStief
Repo: csg-org/CompactConnect PR: 1036
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py:370-383
Timestamp: 2025-09-11T20:06:40.709Z
Learning: The EncumbranceDetailsSchema in backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/record.py does not contain a 'note' field. It only has clinicalPrivilegeActionCategory (String, optional), adverseActionId (UUID, required), and licenseJurisdiction (Jurisdiction, optional). When working with encumbrance notes, use encumbranceDetails['clinicalPrivilegeActionCategory'], not encumbranceDetails['note'].
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.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-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-10T13:50:30.047Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1069
File: backend/compact-connect/lambdas/python/purchases/handlers/transaction_reporting.py:18-22
Timestamp: 2025-09-10T13:50:30.047Z
Learning: When using Python StrEnum for filtering, the `in` operator correctly checks string values against enum members. `'settledSuccessfully' in ReportableTransactionStatuses` works properly and is more maintainable than using raw string sets. StrEnum members are both enum instances and strings, making them ideal for this use case.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-09-10T13:50:30.047Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1069
File: backend/compact-connect/lambdas/python/purchases/handlers/transaction_reporting.py:18-22
Timestamp: 2025-09-10T13:50:30.047Z
Learning: In Python 3.11, using `'string' in StrEnum` raises TypeError. StrEnum membership checking with strings only works starting in Python 3.12. For Python 3.11, convert StrEnum values to a set for membership testing or use explicit equality comparisons.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.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/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-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-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/cc_common/data_model/data_client.py
📚 Learning: 2025-09-03T23:06:45.015Z
Learnt from: ChiefStief
Repo: csg-org/CompactConnect PR: 1036
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py:2645-2651
Timestamp: 2025-09-03T23:06:45.015Z
Learning: PrivilegeData extends CCDataClass which provides the licenseTypeAbbreviation property, so PrivilegeData instances have access to licenseTypeAbbreviation through inheritance.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-09-03T23:06:45.015Z
Learnt from: ChiefStief
Repo: csg-org/CompactConnect PR: 1036
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py:2645-2651
Timestamp: 2025-09-03T23:06:45.015Z
Learning: PrivilegeData extends CCDataClass which provides the licenseTypeAbbreviation property as a computed property (returns license type abbreviation if the instance has both 'compact' and 'licenseType' fields). Therefore, PrivilegeData instances have access to licenseTypeAbbreviation through inheritance.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-06-10T03:16:16.896Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 848
File: backend/compact-connect/lambdas/python/migration/provider_user_pool_migration_551/main.py:35-39
Timestamp: 2025-06-10T03:16:16.896Z
Learning: In the provider user pool migration (provider_user_pool_migration_551), the FilterExpression intentionally only checks for the existence of compactConnectRegisteredEmailAddress. The migration should only remove currentHomeJurisdiction if compactConnectRegisteredEmailAddress is also present, targeting records that went through the old registration process. Records with only currentHomeJurisdiction but no compactConnectRegisteredEmailAddress should be left untouched.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.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/cc_common/data_model/data_client.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/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: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-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
🧬 Code graph analysis (2)
backend/compact-connect/lambdas/python/common/cc_common/utils.py (1)
backend/compact-connect/lambdas/python/common/cc_common/exceptions.py (1)
CCInvalidRequestException(7-8)
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py (8)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/common.py (9)
InvestigationAgainstEnum(287-293)InvestigationStatusEnum(337-339)LicenseEncumberedStatusEnum(325-327)PrivilegeEncumberedStatusEnum(330-334)UpdateCategory(296-312)licenseTypeAbbreviation(156-167)create_new(96-121)to_dict(169-180)serialize_to_database_record(205-208)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/investigation/__init__.py (19)
InvestigationData(12-156)compact(71-72)compact(75-76)providerId(79-80)providerId(83-84)jurisdiction(87-88)jurisdiction(91-92)investigationAgainst(103-104)investigationAgainst(107-108)investigationId(111-112)investigationId(115-116)creationDate(127-128)creationDate(131-132)licenseType(95-96)licenseType(99-100)closeDate(135-136)closeDate(139-140)pk(42-48)sk(51-68)backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py (5)
ProviderRecordType(29-42)get_specific_license_record(459-474)get_specific_privilege_record(476-491)get_investigation_records_for_license(645-671)get_investigation_records_for_privilege(617-643)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/__init__.py (17)
AdverseActionData(14-147)adverseActionId(126-127)adverseActionId(130-131)effectiveLiftDate(134-135)effectiveLiftDate(138-139)compact(27-28)compact(31-32)providerId(35-36)providerId(39-40)jurisdiction(43-44)jurisdiction(47-48)licenseTypeAbbreviation(51-52)licenseTypeAbbreviation(55-56)creationDate(118-119)creationDate(122-123)licenseType(59-60)licenseType(63-64)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/license/__init__.py (9)
compact(32-33)compact(168-169)providerId(28-29)providerId(164-165)jurisdiction(36-37)jurisdiction(172-173)LicenseUpdateData(144-197)licenseType(40-41)licenseType(176-177)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/__init__.py (9)
compact(28-29)compact(112-113)providerId(24-25)providerId(108-109)jurisdiction(32-33)jurisdiction(116-117)PrivilegeUpdateData(95-166)licenseType(40-41)licenseType(120-121)backend/compact-connect/lambdas/python/common/cc_common/exceptions.py (1)
CCNotFoundException(32-33)backend/compact-connect/lambdas/python/common/cc_common/config.py (2)
provider_table(88-89)dynamodb_client(38-39)
⏰ 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: TestApp
🔇 Additional comments (7)
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py (7)
5-5: LGTM! Imports support new investigation functionality.The added imports for UUID types, investigation enums, and InvestigationData are appropriate for the new investigation create/close features.
Also applies to: 26-27, 33-33
176-182: LGTM! UUID type strengthens type safety.The parameter type change from
strtoUUIDforprovider_idis consistent with the investigation feature's UUID usage and improves type safety. The UUID will be automatically stringified in the f-string on line 193.
596-596: LGTM! Enum usage eliminates magic strings.Replacing string literals with
ProviderRecordTypeenum members throughout the file improves type safety and maintainability. This change aligns with best practices and reduces the risk of typos.Also applies to: 737-737, 917-917, 1054-1054, 1454-1454, 2016-2016, 2534-2534, 2618-2618, 2694-2694, 2838-2838, 2996-2996, 3029-3029, 3151-3151, 3242-3242, 3387-3387
1612-1714: LGTM! Investigation creation is well-structured.The
create_investigationmethod correctly:
- Fetches all provider records to validate the target exists
- Branches appropriately for license vs. privilege investigations
- Uses transaction items with proper marshalling via helper methods
- Includes
ConditionExpressionto prevent phantom item creation- Uses
StrEnummembers directly per the codebase conventions
1887-1899: LGTM! Error handling correctly maps transaction failures.The exception handling appropriately converts
ConditionalCheckFailedfrom transaction cancellations intoCCNotFoundException, which provides clear feedback when an investigation doesn't exist or is already closed.
1225-1267: LGTM! UUID type consistency in helper methods.The type changes to use
UUIDforadverse_action_idandtarget_adverse_action_idparameters improve type safety and consistency. UUID comparisons work correctly throughout.
1876-1876: Fix UpdateExpression syntax: SET must precede REMOVE.DynamoDB requires
SETclauses beforeREMOVEclauses inUpdateExpression. The current order will cause a syntax error at runtime.Apply this diff:
- 'UpdateExpression': 'REMOVE investigationStatus SET dateOfUpdate = :dateOfUpdate', + 'UpdateExpression': 'SET dateOfUpdate = :dateOfUpdate REMOVE investigationStatus',⛔ Skipped due to learnings
Learnt from: landonshumway-ia Repo: csg-org/CompactConnect PR: 1014 File: backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py:3058-3066 Timestamp: 2025-08-18T21:57:32.054Z Learning: DynamoDB UpdateExpression clauses (SET, REMOVE, ADD, DELETE) can appear in any syntactic order in the expression string. DynamoDB processes them in a fixed evaluation order (SET → REMOVE → ADD → DELETE) internally, but the textual order in the UpdateExpression string is flexible. Expressions like "REMOVE attr1 SET attr2 = :val" are syntactically valid and will not throw ValidationExceptions.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py (2)
1777-1777: Use keyword argument for filter_condition for consistency.Lines 1771 and 1795 use the
filter_condition=keyword argument when callingget_investigation_records_for_licenseandget_investigation_records_for_privilege, but lines 1777 and 1801 pass the lambda as a positional argument. For clarity and consistency, specify the keyword argument.Apply this change to line 1777:
( inv for inv in provider_records.get_investigation_records_for_license( - jurisdiction, license_type_abbreviation, lambda inv: inv.investigationId == investigation_id + jurisdiction, license_type_abbreviation, filter_condition=lambda inv: inv.investigationId == investigation_id ) ),Apply this change to line 1801:
( inv for inv in provider_records.get_investigation_records_for_privilege( - jurisdiction, license_type_abbreviation, lambda inv: inv.investigationId == investigation_id + jurisdiction, license_type_abbreviation, filter_condition=lambda inv: inv.investigationId == investigation_id ) ),Also applies to: 1801-1801
1876-1876: Consider conventional UpdateExpression ordering for consistency.The UpdateExpression uses
REMOVE investigationStatus SET dateOfUpdate = :dateOfUpdate. While both orderings are valid in DynamoDB, the conventional ordering is to use SET before REMOVE for consistency with typical UpdateExpression patterns.Apply this change:
- 'UpdateExpression': 'REMOVE investigationStatus SET dateOfUpdate = :dateOfUpdate', + 'UpdateExpression': 'SET dateOfUpdate = :dateOfUpdate REMOVE investigationStatus',
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py(23 hunks)backend/compact-connect/lambdas/python/common/cc_common/utils.py(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/compact-connect/lambdas/python/common/cc_common/utils.py
🧰 Additional context used
🧠 Learnings (19)
📚 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-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-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-10T13:50:30.047Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1069
File: backend/compact-connect/lambdas/python/purchases/handlers/transaction_reporting.py:18-22
Timestamp: 2025-09-10T13:50:30.047Z
Learning: When using Python StrEnum for filtering, the `in` operator correctly checks string values against enum members. `'settledSuccessfully' in ReportableTransactionStatuses` works properly and is more maintainable than using raw string sets. StrEnum members are both enum instances and strings, making them ideal for this use case.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-09-10T13:50:30.047Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1069
File: backend/compact-connect/lambdas/python/purchases/handlers/transaction_reporting.py:18-22
Timestamp: 2025-09-10T13:50:30.047Z
Learning: In Python 3.11, using `'string' in StrEnum` raises TypeError. StrEnum membership checking with strings only works starting in Python 3.12. For Python 3.11, convert StrEnum values to a set for membership testing or use explicit equality comparisons.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.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/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-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-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/cc_common/data_model/data_client.py
📚 Learning: 2025-09-03T23:06:45.015Z
Learnt from: ChiefStief
Repo: csg-org/CompactConnect PR: 1036
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py:2645-2651
Timestamp: 2025-09-03T23:06:45.015Z
Learning: PrivilegeData extends CCDataClass which provides the licenseTypeAbbreviation property, so PrivilegeData instances have access to licenseTypeAbbreviation through inheritance.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-09-03T23:06:45.015Z
Learnt from: ChiefStief
Repo: csg-org/CompactConnect PR: 1036
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py:2645-2651
Timestamp: 2025-09-03T23:06:45.015Z
Learning: PrivilegeData extends CCDataClass which provides the licenseTypeAbbreviation property as a computed property (returns license type abbreviation if the instance has both 'compact' and 'licenseType' fields). Therefore, PrivilegeData instances have access to licenseTypeAbbreviation through inheritance.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-06-10T03:16:16.896Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 848
File: backend/compact-connect/lambdas/python/migration/provider_user_pool_migration_551/main.py:35-39
Timestamp: 2025-06-10T03:16:16.896Z
Learning: In the provider user pool migration (provider_user_pool_migration_551), the FilterExpression intentionally only checks for the existence of compactConnectRegisteredEmailAddress. The migration should only remove currentHomeJurisdiction if compactConnectRegisteredEmailAddress is also present, targeting records that went through the old registration process. Records with only currentHomeJurisdiction but no compactConnectRegisteredEmailAddress should be left untouched.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.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/cc_common/data_model/data_client.py
📚 Learning: 2025-10-29T19:11:33.003Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1167
File: backend/compact-connect/lambdas/python/provider-data-v1/handlers/investigation.py:85-95
Timestamp: 2025-10-29T19:11:33.003Z
Learning: InvestigationData extends CCDataClass which provides the licenseTypeAbbreviation property as a computed property (returns license type abbreviation if the instance has both 'compact' and 'licenseType' fields). Therefore, InvestigationData instances have access to licenseTypeAbbreviation through inheritance, and it does not need to be explicitly set when creating new investigation records.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.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/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: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-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-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
🧬 Code graph analysis (1)
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py (4)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/common.py (4)
InvestigationAgainstEnum(287-293)InvestigationStatusEnum(337-339)UpdateCategory(296-312)licenseTypeAbbreviation(156-167)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/investigation/__init__.py (19)
InvestigationData(12-156)compact(71-72)compact(75-76)providerId(79-80)providerId(83-84)jurisdiction(87-88)jurisdiction(91-92)investigationAgainst(103-104)investigationAgainst(107-108)investigationId(111-112)investigationId(115-116)creationDate(127-128)creationDate(131-132)licenseType(95-96)licenseType(99-100)closeDate(135-136)closeDate(139-140)pk(42-48)sk(51-68)backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py (5)
ProviderRecordType(29-42)get_specific_license_record(459-474)get_specific_privilege_record(476-491)get_investigation_records_for_license(645-671)get_investigation_records_for_privilege(617-643)backend/compact-connect/lambdas/python/common/cc_common/config.py (2)
provider_table(88-89)dynamodb_client(38-39)
⏰ 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: TestApp
🔇 Additional comments (1)
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py (1)
1612-1714: LGTM! Investigation creation logic is well-structured.The method correctly:
- Fetches records using the ProviderUserRecords abstraction
- Branches appropriately for LICENSE vs PRIVILEGE investigation types
- Creates all necessary records atomically in a transaction
- Uses ProviderRecordType enum consistently
- Passes StrEnum members directly to DynamoDB without
.value(correct per learnings)
landonshumway-ia
approved these changes
Nov 6, 2025
Collaborator
|
@jlkravitz This is ready for your review. Thanks |
jlkravitz
requested changes
Nov 7, 2025
Collaborator
jlkravitz
left a comment
jlkravitz
left a comment
There was a problem hiding this comment.
Agreed with Landon. I would prefer if you had edited one more file to make it 100. (Just kidding.)
Left several comments!
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
Show resolved
Hide resolved
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
Outdated
Show resolved
Hide resolved
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
Show resolved
Hide resolved
backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py
Show resolved
Hide resolved
backend/compact-connect/lambdas/python/common/cc_common/email_service_client.py
Outdated
Show resolved
Hide resolved
backend/compact-connect/tests/smoke/investigation_smoke_tests.py
Outdated
Show resolved
Hide resolved
backend/compact-connect/lambdas/python/data-events/tests/function/test_investigation_events.py
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/fields.py (1)
72-76: Consider simplifying the validator merging logic.The current implementation uses both
get()andpop()on the same key, which works but is unnecessarily complex.Apply this diff for a cleaner implementation:
- # Merge any provided validators with our new desired one - validate = kwargs.get('validate', []) - if isinstance(kwargs.pop('validate', []), Validator): - validate = [validate] - super().__init__(*args, validate=[OneOf([entry.value for entry in UpdateCategory]), *validate], **kwargs) + # Merge any provided validators with our new desired one + validate = kwargs.pop('validate', []) + if isinstance(validate, Validator): + validate = [validate] + super().__init__(*args, validate=[OneOf([entry.value for entry in UpdateCategory]), *validate], **kwargs)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py(23 hunks)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/fields.py(5 hunks)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/api.py(6 hunks)backend/compact-connect/lambdas/python/common/cc_common/email_service_client.py(6 hunks)backend/compact-connect/lambdas/python/data-events/tests/function/test_investigation_events.py(1 hunks)
🧰 Additional context used
🧠 Learnings (25)
📓 Common learnings
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.234Z
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.
<!--
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.414Z
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.
📚 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/schema/fields.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-05-13T21:20:51.133Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 798
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/fields.py:86-90
Timestamp: 2025-05-13T21:20:51.133Z
Learning: The implementation of `PositiveDecimal` in `cc_common/data_model/schema/fields.py` is intentionally kept simple without support for stacked validators. If more complex validation is needed, the preferred approach is to create another custom field class.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/fields.py
📚 Learning: 2025-10-29T19:11:33.003Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1167
File: backend/compact-connect/lambdas/python/provider-data-v1/handlers/investigation.py:85-95
Timestamp: 2025-10-29T19:11:33.003Z
Learning: InvestigationData extends CCDataClass which provides the licenseTypeAbbreviation property as a computed property (returns license type abbreviation if the instance has both 'compact' and 'licenseType' fields). Therefore, InvestigationData instances have access to licenseTypeAbbreviation through inheritance, and it does not need to be explicitly set when creating new investigation records.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/fields.pybackend/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/schema/privilege/api.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-09-11T20:06:40.709Z
Learnt from: ChiefStief
Repo: csg-org/CompactConnect PR: 1036
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py:370-383
Timestamp: 2025-09-11T20:06:40.709Z
Learning: The EncumbranceDetailsSchema in backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/record.py does not contain a 'note' field. It only has clinicalPrivilegeActionCategory (String, optional), adverseActionId (UUID, required), and licenseJurisdiction (Jurisdiction, optional). When working with encumbrance notes, use encumbranceDetails['clinicalPrivilegeActionCategory'], not encumbranceDetails['note'].
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/api.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-11-06T18:11:58.234Z
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.234Z
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/lambdas/python/common/cc_common/data_model/schema/privilege/api.pybackend/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/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/cc_common/email_service_client.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.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/cc_common/email_service_client.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-09-10T13:50:30.047Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1069
File: backend/compact-connect/lambdas/python/purchases/handlers/transaction_reporting.py:18-22
Timestamp: 2025-09-10T13:50:30.047Z
Learning: When using Python StrEnum for filtering, the `in` operator correctly checks string values against enum members. `'settledSuccessfully' in ReportableTransactionStatuses` works properly and is more maintainable than using raw string sets. StrEnum members are both enum instances and strings, making them ideal for this use case.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-09-10T13:50:30.047Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1069
File: backend/compact-connect/lambdas/python/purchases/handlers/transaction_reporting.py:18-22
Timestamp: 2025-09-10T13:50:30.047Z
Learning: In Python 3.11, using `'string' in StrEnum` raises TypeError. StrEnum membership checking with strings only works starting in Python 3.12. For Python 3.11, convert StrEnum values to a set for membership testing or use explicit equality comparisons.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.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/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-11-06T18:10:16.414Z
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.414Z
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:
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-09-03T23:06:45.015Z
Learnt from: ChiefStief
Repo: csg-org/CompactConnect PR: 1036
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py:2645-2651
Timestamp: 2025-09-03T23:06:45.015Z
Learning: PrivilegeData extends CCDataClass which provides the licenseTypeAbbreviation property, so PrivilegeData instances have access to licenseTypeAbbreviation through inheritance.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-09-03T23:06:45.015Z
Learnt from: ChiefStief
Repo: csg-org/CompactConnect PR: 1036
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py:2645-2651
Timestamp: 2025-09-03T23:06:45.015Z
Learning: PrivilegeData extends CCDataClass which provides the licenseTypeAbbreviation property as a computed property (returns license type abbreviation if the instance has both 'compact' and 'licenseType' fields). Therefore, PrivilegeData instances have access to licenseTypeAbbreviation through inheritance.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-06-10T03:16:16.896Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 848
File: backend/compact-connect/lambdas/python/migration/provider_user_pool_migration_551/main.py:35-39
Timestamp: 2025-06-10T03:16:16.896Z
Learning: In the provider user pool migration (provider_user_pool_migration_551), the FilterExpression intentionally only checks for the existence of compactConnectRegisteredEmailAddress. The migration should only remove currentHomeJurisdiction if compactConnectRegisteredEmailAddress is also present, targeting records that went through the old registration process. Records with only currentHomeJurisdiction but no compactConnectRegisteredEmailAddress should be left untouched.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.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/cc_common/data_model/data_client.pybackend/compact-connect/lambdas/python/data-events/tests/function/test_investigation_events.py
📚 Learning: 2025-11-06T18:14:40.602Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 1196
File: webroot/src/models/Investigation/Investigation.model.ts:67-86
Timestamp: 2025-11-06T18:14:40.602Z
Learning: In the Investigation model (webroot/src/models/Investigation/Investigation.model.ts), the isActive() method intentionally includes defensive logic to handle potential backend bug states where the start date (startDate) might be missing. If an investigation record exists but lacks a start date, it should still be considered active as long as it hasn't ended (updateDate is null or in the future), because the record's existence implies it was created and just has missing date metadata.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.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/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:35:42.943Z
Learnt from: ChiefStief
Repo: csg-org/CompactConnect PR: 1036
File: backend/compact-connect/lambdas/python/data-events/handlers/encumbrance_events.py:200-204
Timestamp: 2025-09-03T22:35:42.943Z
Learning: The adverseActionCategory and adverseActionId fields in encumbrance events are only needed when the event flow creates privilege update database records. Some privilege encumbrance event publications don't create database records and don't need these fields, which is why they're optional in the EncumbranceEventDetailSchema.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-08-29T21:45:05.792Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1016
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py:2702-2710
Timestamp: 2025-08-29T21:45:05.792Z
Learning: In the lift_home_jurisdiction_license_privilege_encumbrances method, when latest_effective_lift_date is None, the method always returns an empty list for affected_privileges, so the existing if affected_privileges guard in the calling code already prevents event publishing without needing explicit None checks for the date.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
🧬 Code graph analysis (4)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/fields.py (1)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/common.py (3)
InvestigationAgainstEnum(287-293)InvestigationStatusEnum(337-339)UpdateCategory(296-312)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/api.py (4)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/common.py (1)
UpdateCategory(296-312)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/fields.py (4)
Compact(50-52)InvestigationStatusField(89-91)Jurisdiction(55-57)UpdateType(70-76)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/investigation/api.py (1)
InvestigationGeneralResponseSchema(28-45)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/__init__.py (2)
investigationStatus(84-85)updateType(104-105)
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py (4)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/common.py (5)
InvestigationAgainstEnum(287-293)InvestigationStatusEnum(337-339)licenseTypeAbbreviation(156-167)create_new(96-121)serialize_to_database_record(205-208)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/investigation/__init__.py (19)
InvestigationData(12-156)compact(71-72)compact(75-76)providerId(79-80)providerId(83-84)jurisdiction(87-88)jurisdiction(91-92)investigationAgainst(103-104)investigationAgainst(107-108)investigationId(111-112)investigationId(115-116)creationDate(127-128)creationDate(131-132)licenseType(95-96)licenseType(99-100)closeDate(135-136)closeDate(139-140)pk(42-48)sk(51-68)backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py (5)
ProviderRecordType(29-42)get_specific_license_record(459-474)get_specific_privilege_record(476-491)get_investigation_records_for_license(645-671)get_investigation_records_for_privilege(617-643)backend/compact-connect/lambdas/python/common/cc_common/exceptions.py (1)
CCNotFoundException(32-33)
backend/compact-connect/lambdas/python/data-events/tests/function/test_investigation_events.py (5)
backend/compact-connect/lambdas/python/provider-data-v1/tests/function/__init__.py (1)
TstFunction(22-443)backend/compact-connect/lambdas/python/common/cc_common/config.py (1)
email_service_client(298-303)backend/compact-connect/lambdas/python/common/cc_common/email_service_client.py (1)
InvestigationNotificationTemplateVariables(28-37)backend/compact-connect/lambdas/python/data-events/handlers/investigation_events.py (4)
license_investigation_notification_listener(141-199)license_investigation_closed_notification_listener(203-267)privilege_investigation_notification_listener(271-329)privilege_investigation_closed_notification_listener(333-397)backend/compact-connect/lambdas/python/common/common_test/test_data_generator.py (3)
put_default_provider_record_in_provider_table(440-460)put_default_license_record_in_provider_table(269-279)put_default_privilege_record_in_provider_table(341-351)
⏰ 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: TestApp
🔇 Additional comments (7)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/api.py (4)
4-4: LGTM!The new imports are properly organized and necessary for the investigation functionality being added.
Also applies to: 11-19
79-79: LGTM!The investigation fields are added consistently with the existing
adverseActionspattern and include appropriate field constraints.Also applies to: 91-92
114-114: LGTM!The investigation fields are consistently added to the private response schema, matching the structure in the general response schema.
Also applies to: 126-127
170-176: LGTM!The
ContainsNoneOfvalidator correctly prevents investigation-related updates from being exposed in privilege history responses, providing defense-in-depth protection against accidental disclosure.backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/fields.py (2)
2-2: LGTM!The new imports are necessary for the investigation field classes and UpdateType validator merging logic.
Also applies to: 11-12
89-92: LGTM!Both new field classes follow the established pattern for enum-validated string fields and are implemented consistently with existing field classes in this file.
Also applies to: 130-133
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py (1)
1874-1884: Fix invalid DynamoDB UpdateExpression ordering
UpdateExpressioncurrently reads'REMOVE investigationStatus SET dateOfUpdate = :dateOfUpdate', but DynamoDB requiresSET(and other clauses) to appear beforeREMOVE. As written, this call will throw aValidationException, causingclose_investigationto fail whenever we close the last open investigation and try to update the license/privilege record. Please reorder the clauses so the statement is syntactically valid.Apply this diff to correct the ordering:
- 'UpdateExpression': 'REMOVE investigationStatus SET dateOfUpdate = :dateOfUpdate', + 'UpdateExpression': 'SET dateOfUpdate = :dateOfUpdate REMOVE investigationStatus',⛔ Skipped due to learnings
Learnt from: landonshumway-ia Repo: csg-org/CompactConnect PR: 1014 File: backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py:3058-3066 Timestamp: 2025-08-18T21:57:32.054Z Learning: DynamoDB UpdateExpression clauses (SET, REMOVE, ADD, DELETE) can appear in any syntactic order in the expression string. DynamoDB processes them in a fixed evaluation order (SET → REMOVE → ADD → DELETE) internally, but the textual order in the UpdateExpression string is flexible. Expressions like "REMOVE attr1 SET attr2 = :val" are syntactically valid and will not throw ValidationExceptions.
jusdino
force-pushed
the
feat/significant-investigative-info
branch
from
083987d to
55148a0
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
backend/compact-connect/lambdas/python/common/cc_common/email_service_client.py (2)
326-327: Standardize error message punctuation.The validation error messages in the encumbrance methods have inconsistent punctuation - lines 327 and 391 lack periods, while lines 455 and 519 include them. For consistency, either add or remove the trailing period across all four messages.
Apply this diff to standardize (removing periods for consistency with investigation methods below):
- raise ValueError('Provider ID is required for state notification emails') + raise ValueError('Provider ID is required for state notification emails') - raise ValueError('Provider ID is required for state notification emails') + raise ValueError('Provider ID is required for state notification emails') - raise ValueError('Provider ID is required for state notification emails.') + raise ValueError('Provider ID is required for state notification emails') - raise ValueError('Provider ID is required for state notification emails.') + raise ValueError('Provider ID is required for state notification emails')Also applies to: 390-391, 454-455, 518-519
624-754: Consider aligning error message wording across notification methods.The four new investigation notification methods use the error message
"provider_id must be provided for state notifications", while the encumbrance methods above use"Provider ID is required for state notification emails". For consistency, consider standardizing the wording across all state notification methods.Otherwise, the implementation is solid - all four methods correctly:
- Validate provider_id presence
- Build payloads with distinct template names
- Map template variables consistently
- Convert UUID to string for serialization
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py (1)
1768-1798: Inconsistent filtering: redundant closeDate check for privileges.The privilege branch (line 1797) includes
inv.closeDate is Nonein the filter condition, but this is redundant becauseget_investigation_records_for_privilegealready excludes closed investigations by default. The license branch (line 1771) correctly omits this check.Apply this diff for consistency:
# Count open investigations (those without closeDate), excluding the one we're closing open_investigations = provider_records.get_investigation_records_for_privilege( jurisdiction, license_type_abbreviation, - filter_condition=lambda inv: inv.closeDate is None and inv.investigationId != investigation_id, + filter_condition=lambda inv: inv.investigationId != investigation_id, )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py(23 hunks)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/fields.py(5 hunks)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/api.py(6 hunks)backend/compact-connect/lambdas/python/common/cc_common/email_service_client.py(6 hunks)backend/compact-connect/lambdas/python/data-events/tests/function/test_investigation_events.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/fields.py
🧰 Additional context used
🧠 Learnings (24)
📓 Common learnings
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.234Z
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.
<!--
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.414Z
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.
📚 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/email_service_client.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.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/cc_common/email_service_client.pybackend/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/schema/privilege/api.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-09-11T20:06:40.709Z
Learnt from: ChiefStief
Repo: csg-org/CompactConnect PR: 1036
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py:370-383
Timestamp: 2025-09-11T20:06:40.709Z
Learning: The EncumbranceDetailsSchema in backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/record.py does not contain a 'note' field. It only has clinicalPrivilegeActionCategory (String, optional), adverseActionId (UUID, required), and licenseJurisdiction (Jurisdiction, optional). When working with encumbrance notes, use encumbranceDetails['clinicalPrivilegeActionCategory'], not encumbranceDetails['note'].
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/api.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-11-06T18:11:58.234Z
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.234Z
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/lambdas/python/common/cc_common/data_model/schema/privilege/api.pybackend/compact-connect/lambdas/python/data-events/tests/function/test_investigation_events.pybackend/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/schema/privilege/api.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/data-events/tests/function/test_investigation_events.pybackend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-10-29T19:11:33.003Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1167
File: backend/compact-connect/lambdas/python/provider-data-v1/handlers/investigation.py:85-95
Timestamp: 2025-10-29T19:11:33.003Z
Learning: InvestigationData extends CCDataClass which provides the licenseTypeAbbreviation property as a computed property (returns license type abbreviation if the instance has both 'compact' and 'licenseType' fields). Therefore, InvestigationData instances have access to licenseTypeAbbreviation through inheritance, and it does not need to be explicitly set when creating new investigation records.
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-10T13:50:30.047Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1069
File: backend/compact-connect/lambdas/python/purchases/handlers/transaction_reporting.py:18-22
Timestamp: 2025-09-10T13:50:30.047Z
Learning: When using Python StrEnum for filtering, the `in` operator correctly checks string values against enum members. `'settledSuccessfully' in ReportableTransactionStatuses` works properly and is more maintainable than using raw string sets. StrEnum members are both enum instances and strings, making them ideal for this use case.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-09-10T13:50:30.047Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1069
File: backend/compact-connect/lambdas/python/purchases/handlers/transaction_reporting.py:18-22
Timestamp: 2025-09-10T13:50:30.047Z
Learning: In Python 3.11, using `'string' in StrEnum` raises TypeError. StrEnum membership checking with strings only works starting in Python 3.12. For Python 3.11, convert StrEnum values to a set for membership testing or use explicit equality comparisons.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.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/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-11-06T18:10:16.414Z
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.414Z
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:
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-09-03T23:06:45.015Z
Learnt from: ChiefStief
Repo: csg-org/CompactConnect PR: 1036
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py:2645-2651
Timestamp: 2025-09-03T23:06:45.015Z
Learning: PrivilegeData extends CCDataClass which provides the licenseTypeAbbreviation property, so PrivilegeData instances have access to licenseTypeAbbreviation through inheritance.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-09-03T23:06:45.015Z
Learnt from: ChiefStief
Repo: csg-org/CompactConnect PR: 1036
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py:2645-2651
Timestamp: 2025-09-03T23:06:45.015Z
Learning: PrivilegeData extends CCDataClass which provides the licenseTypeAbbreviation property as a computed property (returns license type abbreviation if the instance has both 'compact' and 'licenseType' fields). Therefore, PrivilegeData instances have access to licenseTypeAbbreviation through inheritance.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-06-10T03:16:16.896Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 848
File: backend/compact-connect/lambdas/python/migration/provider_user_pool_migration_551/main.py:35-39
Timestamp: 2025-06-10T03:16:16.896Z
Learning: In the provider user pool migration (provider_user_pool_migration_551), the FilterExpression intentionally only checks for the existence of compactConnectRegisteredEmailAddress. The migration should only remove currentHomeJurisdiction if compactConnectRegisteredEmailAddress is also present, targeting records that went through the old registration process. Records with only currentHomeJurisdiction but no compactConnectRegisteredEmailAddress should be left untouched.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-11-06T18:14:40.602Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 1196
File: webroot/src/models/Investigation/Investigation.model.ts:67-86
Timestamp: 2025-11-06T18:14:40.602Z
Learning: In the Investigation model (webroot/src/models/Investigation/Investigation.model.ts), the isActive() method intentionally includes defensive logic to handle potential backend bug states where the start date (startDate) might be missing. If an investigation record exists but lacks a start date, it should still be considered active as long as it hasn't ended (updateDate is null or in the future), because the record's existence implies it was created and just has missing date metadata.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.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/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:35:42.943Z
Learnt from: ChiefStief
Repo: csg-org/CompactConnect PR: 1036
File: backend/compact-connect/lambdas/python/data-events/handlers/encumbrance_events.py:200-204
Timestamp: 2025-09-03T22:35:42.943Z
Learning: The adverseActionCategory and adverseActionId fields in encumbrance events are only needed when the event flow creates privilege update database records. Some privilege encumbrance event publications don't create database records and don't need these fields, which is why they're optional in the EncumbranceEventDetailSchema.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
📚 Learning: 2025-08-29T21:45:05.792Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1016
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py:2702-2710
Timestamp: 2025-08-29T21:45:05.792Z
Learning: In the lift_home_jurisdiction_license_privilege_encumbrances method, when latest_effective_lift_date is None, the method always returns an empty list for affected_privileges, so the existing if affected_privileges guard in the calling code already prevents event publishing without needing explicit None checks for the date.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
🧬 Code graph analysis (3)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/api.py (4)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/base_record.py (1)
ForgivingSchema(21-25)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/common.py (1)
UpdateCategory(296-312)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/fields.py (5)
ActiveInactive(60-62)Compact(50-52)InvestigationStatusField(89-91)Jurisdiction(55-57)UpdateType(70-76)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/investigation/api.py (1)
InvestigationGeneralResponseSchema(28-45)
backend/compact-connect/lambdas/python/data-events/tests/function/test_investigation_events.py (4)
backend/compact-connect/lambdas/python/provider-data-v1/tests/function/__init__.py (1)
TstFunction(22-443)backend/compact-connect/lambdas/python/common/cc_common/email_service_client.py (1)
InvestigationNotificationTemplateVariables(28-37)backend/compact-connect/lambdas/python/data-events/handlers/investigation_events.py (4)
license_investigation_notification_listener(141-199)license_investigation_closed_notification_listener(203-267)privilege_investigation_notification_listener(271-329)privilege_investigation_closed_notification_listener(333-397)backend/compact-connect/lambdas/python/common/common_test/test_data_generator.py (3)
put_default_provider_record_in_provider_table(440-460)put_default_license_record_in_provider_table(269-279)put_default_privilege_record_in_provider_table(341-351)
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py (7)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/common.py (4)
InvestigationAgainstEnum(287-293)InvestigationStatusEnum(337-339)UpdateCategory(296-312)licenseTypeAbbreviation(156-167)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/investigation/__init__.py (19)
InvestigationData(12-156)compact(71-72)compact(75-76)providerId(79-80)providerId(83-84)jurisdiction(87-88)jurisdiction(91-92)investigationAgainst(103-104)investigationAgainst(107-108)investigationId(111-112)investigationId(115-116)creationDate(127-128)creationDate(131-132)licenseType(95-96)licenseType(99-100)closeDate(135-136)closeDate(139-140)pk(42-48)sk(51-68)backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py (5)
ProviderRecordType(29-42)get_specific_license_record(459-474)get_specific_privilege_record(476-491)get_investigation_records_for_license(645-671)get_investigation_records_for_privilege(617-643)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/__init__.py (14)
adverseActionId(126-127)adverseActionId(130-131)compact(27-28)compact(31-32)providerId(35-36)providerId(39-40)jurisdiction(43-44)jurisdiction(47-48)licenseTypeAbbreviation(51-52)licenseTypeAbbreviation(55-56)creationDate(118-119)creationDate(122-123)licenseType(59-60)licenseType(63-64)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/license/__init__.py (9)
compact(32-33)compact(168-169)providerId(28-29)providerId(164-165)jurisdiction(36-37)jurisdiction(172-173)LicenseUpdateData(144-197)licenseType(40-41)licenseType(176-177)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/__init__.py (9)
compact(28-29)compact(112-113)providerId(24-25)providerId(108-109)jurisdiction(32-33)jurisdiction(116-117)PrivilegeUpdateData(95-166)licenseType(40-41)licenseType(120-121)backend/compact-connect/lambdas/python/common/cc_common/exceptions.py (1)
CCNotFoundException(32-33)
⏰ 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: TestApp
🔇 Additional comments (12)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/api.py (4)
4-4: LGTM! Imports are necessary and correctly used.All new imports support the investigation functionality:
ContainsNoneOffor validation protection in history eventsUpdateCategoryto reference investigation enum valuesInvestigationStatusFieldfor the new status fieldInvestigationGeneralResponseSchemafor nested investigation dataAlso applies to: 11-11, 12-18, 19-19
79-79: LGTM! Investigation fields properly added.The
investigationslist andinvestigationStatusfields follow the same pattern asadverseActions(optional, non-nullable). The comment clearly explains wheninvestigationStatusis populated. This is appropriate for internal staff access with readGeneral permissions.Also applies to: 91-92
114-114: LGTM! Consistent investigation support across staff schemas.Both
PrivilegeGeneralResponseSchemaandPrivilegeReadPrivateResponseSchemanow include matching investigation fields, which is appropriate since both are for internal staff use (with different permission levels).Also applies to: 126-127
170-176: LGTM! Strong protection against investigation disclosure.The
ContainsNoneOfvalidator ensures investigation and closing investigation updates cannot be returned via the public history endpoint. This provides validation-level protection consistent with the intentional exclusion of investigation fields fromPrivilegePublicResponseSchema. The comment clearly documents the security intent.backend/compact-connect/lambdas/python/common/cc_common/email_service_client.py (1)
27-38: LGTM! Dataclass design is appropriate.The
InvestigationNotificationTemplateVariablesdataclass correctly models the required fields for investigation notifications. Makingprovider_ida requiredUUIDfield (notOptional) aligns with the validation logic in the state notification methods below.backend/compact-connect/lambdas/python/data-events/tests/function/test_investigation_events.py (4)
19-94: LGTM! Well-structured test helpers.The test class setup and helper methods follow best practices:
- Deterministic timestamp patching ensures reproducible tests
- Message generation helpers eliminate duplication
- Consistent event structure across all test scenarios
96-462: Excellent test coverage for notification scenarios.The four happy path tests comprehensively verify:
- State notifications to investigation jurisdiction plus all jurisdictions with active licenses/privileges
- Correct template variable construction for each notification
- Proper sorting eliminates non-deterministic test failures
The explicit comments confirming that provider notifications are NOT sent for investigations add helpful clarity.
464-536: Robust error handling test coverage.The error handling tests properly verify graceful degradation:
- Missing provider records return batch item failures (allowing retry)
- Email service exceptions return batch item failures (allowing retry)
This ensures the system won't lose messages when transient failures occur.
538-598: Well-designed tests for investigation-to-encumbrance flow.These tests correctly verify the business logic where investigation closed notifications are skipped when an encumbrance is created. This prevents duplicate notifications and ensures states receive the more important encumbrance notification instead.
Using
assert_not_called()provides clear verification that the skip logic works as intended.backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py (3)
1612-1714: LGTM! Investigation creation implements best practices.The method correctly:
- Uses
get_provider_user_recordsto fetch all records in a single query- Abstracts DB details through data class serialization for pk/sk generation
- Applies
ConditionExpressionto prevent phantom record creation- Ensures atomicity with a DynamoDB transaction
- Uses
ProviderRecordTypeenum values throughout
180-180: LGTM! UUID type annotations improve type safety.Consistent use of UUID type annotations for provider_id and adverse_action_id parameters improves type safety and makes the API contracts clearer.
Also applies to: 1226-1266, 1908-1911, 2053-2068
596-596: LGTM! Consistent use of ProviderRecordType enum values.Using
ProviderRecordTypeenum members instead of hardcoded strings improves maintainability and reduces the risk of typos. Based on learnings.Also applies to: 737-737, 917-917, 1054-1054, 1454-1454, 2020-2020, 2538-2538, 2622-2622, 2698-2698, 2842-2842, 3000-3000, 3033-3033, 3155-3155, 3246-3246, 3391-3391
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
backend/compact-connect/docs/postman/postman-collection.json (1)
427-427: Replace realistic SSNs/phones/emails in examples with unmistakably fake placeholdersThis mirrors a previous concern that was marked addressed but remains in the collection. Full SSNs (e.g., "151-51-8414", "971-92-2380") and realistic phone numbers ("+3849258700", "+925079022127") pose a compliance risk even as synthetic/example data. Replace with standardized placeholders:
"ssn": "000-00-0000"and"phoneNumber": "+15555550123"(incrementing last digit for each instance).- "ssn": "151-51-8414", + "ssn": "000-00-0000", - "phoneNumber": "+3849258700", + "phoneNumber": "+15555550123",Apply similar changes to lines 502 and 563, and any other SSN/phone examples. Consider adding a note in collection docs: "All PII shown here is placeholder data."
Also applies to: 502-502, 563-563
🧹 Nitpick comments (6)
backend/compact-connect/tests/smoke/investigation_smoke_tests.py (6)
238-240: Add defensive check for empty privileges list.Direct index access to
provider_data['privileges'][0]will raise anIndexErrorif no privileges exist. Consider adding a bounds check with a descriptive error message to improve debuggability when the test environment isn't set up correctly.Apply this pattern:
provider_data = call_provider_users_me_endpoint() provider_id = provider_data['providerId'] compact = provider_data['compact'] +if not provider_data['privileges']: + raise SmokeTestFailureException('No privileges found for provider - test environment may not be set up correctly') jurisdiction = provider_data['privileges'][0]['jurisdiction'] license_type = provider_data['privileges'][0]['licenseType'] license_type_abbreviation = get_license_type_abbreviation(license_type)
273-275: Add defensive check for empty licenses list.Similar to the privileges test, add a bounds check before accessing
provider_data['licenses'][0]to provide a clearer error message if no licenses exist.provider_data = call_provider_users_me_endpoint() provider_id = provider_data['providerId'] compact = provider_data['compact'] jurisdiction = provider_data['licenseJurisdiction'] +if not provider_data['licenses']: + raise SmokeTestFailureException('No licenses found for provider - test environment may not be set up correctly') license_type = provider_data['licenses'][0]['licenseType'] license_type_abbreviation = get_license_type_abbreviation(license_type)
305-310: Add defensive check for empty privileges list.Add bounds checking before accessing
provider_data['privileges'][0].provider_data = call_provider_users_me_endpoint() provider_id = provider_data['providerId'] compact = provider_data['compact'] +if not provider_data['privileges']: + raise SmokeTestFailureException('No privileges found for provider - test environment may not be set up correctly') jurisdiction = provider_data['privileges'][0]['jurisdiction'] license_type = provider_data['privileges'][0]['licenseType'] license_type_abbreviation = get_license_type_abbreviation(license_type)
340-345: Add defensive check for empty licenses list.Add bounds checking before accessing
provider_data['licenses'][0].provider_data = call_provider_users_me_endpoint() provider_id = provider_data['providerId'] compact = provider_data['compact'] jurisdiction = provider_data['licenseJurisdiction'] +if not provider_data['licenses']: + raise SmokeTestFailureException('No licenses found for provider - test environment may not be set up correctly') license_type = provider_data['licenses'][0]['licenseType'] license_type_abbreviation = get_license_type_abbreviation(license_type)
375-380: Add defensive check for empty privileges list.Add bounds checking before accessing
provider_data['privileges'][0].provider_data = call_provider_users_me_endpoint() provider_id = provider_data['providerId'] compact = provider_data['compact'] +if not provider_data['privileges']: + raise SmokeTestFailureException('No privileges found for provider - test environment may not be set up correctly') jurisdiction = provider_data['privileges'][0]['jurisdiction'] license_type = provider_data['privileges'][0]['licenseType'] license_type_abbreviation = get_license_type_abbreviation(license_type)
260-261: Consider retry logic for more robust eventual consistency handling.While the fixed 5-second sleep is acceptable for smoke tests, a retry-with-polling pattern would be more robust and potentially faster. This could reduce flakiness if DynamoDB replication is slower than expected or faster than 5 seconds.
Example pattern:
# Instead of fixed sleep, poll until condition is met max_attempts = 10 for attempt in range(max_attempts): try: _verify_investigation_exists('privilege', jurisdiction, license_type) break except SmokeTestFailureException: if attempt == max_attempts - 1: raise time.sleep(1)Note: This same pattern appears in lines 295-296, 330-331, 365-366, and 418-419.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
backend/compact-connect/docs/api-specification/latest-oas30.json(0 hunks)backend/compact-connect/docs/postman/postman-collection.json(19 hunks)backend/compact-connect/stacks/api_stack/v1_api/api_model.py(23 hunks)backend/compact-connect/tests/resources/snapshots/PATCH_LICENSE_INVESTIGATION_REQUEST_SCHEMA.json(1 hunks)backend/compact-connect/tests/resources/snapshots/PATCH_PRIVILEGE_INVESTIGATION_REQUEST_SCHEMA.json(1 hunks)backend/compact-connect/tests/smoke/investigation_smoke_tests.py(1 hunks)
💤 Files with no reviewable changes (1)
- backend/compact-connect/docs/api-specification/latest-oas30.json
🚧 Files skipped from review as they are similar to previous changes (2)
- backend/compact-connect/tests/resources/snapshots/PATCH_LICENSE_INVESTIGATION_REQUEST_SCHEMA.json
- backend/compact-connect/tests/resources/snapshots/PATCH_PRIVILEGE_INVESTIGATION_REQUEST_SCHEMA.json
🧰 Additional context used
🧠 Learnings (16)
📓 Common learnings
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.234Z
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.
<!--
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.414Z
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.
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.
📚 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/docs/postman/postman-collection.json
📚 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-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-08-18T21:57:32.054Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1014
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py:3058-3066
Timestamp: 2025-08-18T21:57:32.054Z
Learning: DynamoDB UpdateExpression clauses (SET, REMOVE, ADD, DELETE) can appear in any syntactic order in the expression string. DynamoDB processes them in a fixed evaluation order (SET → REMOVE → ADD → DELETE) internally, but the textual order in the UpdateExpression string is flexible. Expressions like "REMOVE attr1 SET attr2 = :val" are syntactically valid and will not throw ValidationExceptions.
Applied to files:
backend/compact-connect/tests/smoke/investigation_smoke_tests.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/tests/smoke/investigation_smoke_tests.py
📚 Learning: 2025-06-10T03:16:16.896Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 848
File: backend/compact-connect/lambdas/python/migration/provider_user_pool_migration_551/main.py:35-39
Timestamp: 2025-06-10T03:16:16.896Z
Learning: In the provider user pool migration (provider_user_pool_migration_551), the FilterExpression intentionally only checks for the existence of compactConnectRegisteredEmailAddress. The migration should only remove currentHomeJurisdiction if compactConnectRegisteredEmailAddress is also present, targeting records that went through the old registration process. Records with only currentHomeJurisdiction but no compactConnectRegisteredEmailAddress should be left untouched.
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/tests/smoke/investigation_smoke_tests.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/tests/smoke/investigation_smoke_tests.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-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-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-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/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-06-26T16:42:00.781Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 882
File: backend/compact-connect/lambdas/python/migration/compact_configured_states_871/main.py:127-130
Timestamp: 2025-06-26T16:42:00.781Z
Learning: In the compact_configured_states_871 migration, existing jurisdiction configurations that have licenseeRegistrationEnabled: true are migrated with isLive: true for backwards compatibility. This prevents breaking existing live functionality since those states are already operational. The migration preserves the current live state of jurisdictions to maintain service continuity, while new states added after migration can start with isLive: false and be controlled by compact admins.
Applied to files:
backend/compact-connect/stacks/api_stack/v1_api/api_model.py
📚 Learning: 2025-10-29T19:11:33.003Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1167
File: backend/compact-connect/lambdas/python/provider-data-v1/handlers/investigation.py:85-95
Timestamp: 2025-10-29T19:11:33.003Z
Learning: InvestigationData extends CCDataClass which provides the licenseTypeAbbreviation property as a computed property (returns license type abbreviation if the instance has both 'compact' and 'licenseType' fields). Therefore, InvestigationData instances have access to licenseTypeAbbreviation through inheritance, and it does not need to be explicitly set when creating new investigation records.
Applied to files:
backend/compact-connect/stacks/api_stack/v1_api/api_model.py
🧬 Code graph analysis (2)
backend/compact-connect/tests/smoke/investigation_smoke_tests.py (1)
backend/compact-connect/tests/smoke/smoke_common.py (8)
SmokeTestFailureException(13-17)call_provider_users_me_endpoint(227-235)create_test_staff_user(104-131)delete_test_staff_user(83-101)get_all_provider_database_records(238-248)get_license_type_abbreviation(183-190)get_provider_user_dynamodb_table(197-198)get_staff_user_auth_headers(176-180)
backend/compact-connect/stacks/api_stack/v1_api/api_model.py (2)
backend/compact-connect/tests/app/test_transaction_monitoring.py (1)
get_context(27-34)backend/compact-connect/tests/app/test_api/__init__.py (1)
get_context(17-25)
⏰ 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: TestApp
🔇 Additional comments (8)
backend/compact-connect/tests/smoke/investigation_smoke_tests.py (5)
1-24: LGTM!Clean imports and clear documentation. The module structure follows the established smoke test patterns.
27-118: LGTM!The cleanup logic is comprehensive and correctly removes investigation/encumbrance data from multiple record types. The UpdateExpression syntax on lines 44-47 is now correct following the previous fix.
121-229: LGTM!The helper functions provide clean abstractions for data extraction and verification. The dual verification (database + API) ensures consistency across storage and presentation layers.
397-397: Verify that past encumbrance effective dates are accepted.The hard-coded date
'2024-01-15'is in the past (current date is November 2025). Confirm that the API validation accepts past dates forencumbranceEffectiveDate, or update to use a current/future date if required.
435-490: LGTM!The test orchestration is well-designed with proper cleanup in the
finallyblock and good test isolation through repeatedsetup_test_environment()calls between test sequences. The error handling ensures cleanup occurs even if tests fail.backend/compact-connect/stacks/api_stack/v1_api/api_model.py (3)
414-414: LGTM! Good refactoring to share encumbrance schema.Replacing inline schemas with a shared
_encumbrance_request_schemaeliminates duplication and makes the schema easier to maintain.Also applies to: 427-427
1460-1490: LGTM! Well-structured investigation schema.The schema properly defines all required fields for investigation objects with appropriate types, formats, and validations.
1174-1182: LGTM! Consistent addition of investigation fields.The
investigationsarray andinvestigationStatusfields are properly integrated into both license and privilege schemas, enabling API consumers to check investigation status and retrieve investigation details.Also applies to: 1320-1328
jlkravitz
approved these changes
Nov 10, 2025
Collaborator
jlkravitz
left a comment
jlkravitz
left a comment
There was a problem hiding this comment.
LGTM! @isabeleliassen Good to merge
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description List
Testing List
Closes #1133
Summary by CodeRabbit
New Features
Tests
Documentation
Chores