Feat/home state change notifications#1279
Merged
isabeleliassen merged 18 commits intocsg-org:mainfrom Feb 2, 2026
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughPublishes provider home-jurisdiction-change events, adds idempotent listener that notifies old/new jurisdiction operations teams via new email templates and services, gates publishing with a feature flag, updates data client to return previous jurisdiction/no-op, and adds tests plus infra wiring across Node.js and Python components. Changes
Sequence Diagram(s)sequenceDiagram
participant ProviderAPI as "Provider API"
participant Data as "DataClient"
participant EventBus as "EventBus"
participant Handler as "HomeJurisdictionHandler"
participant Config as "CompactConfigClient"
participant EmailSvc as "EmailServiceClient"
participant OldOps as "Old Jurisdiction Ops"
participant NewOps as "New Jurisdiction Ops"
ProviderAPI->>Data: update_provider_home_state_jurisdiction(compact, provider_id, selected_jurisdiction)
Data-->>ProviderAPI: previous_home_jurisdiction
alt previous != new and feature flag enabled
ProviderAPI->>EventBus: publish_home_jurisdiction_change_event(detail)
EventBus->>Handler: deliver event
Handler->>Data: fetch provider record
Data-->>Handler: provider details
alt previous is compact and has ops emails
Handler->>Config: get_jurisdiction_operations_team_emails(previous)
Config-->>Handler: old_ops_emails
Handler->>EmailSvc: send_home_jurisdiction_change_old_state_notification(...)
EmailSvc->>OldOps: email
end
alt new is compact and has ops emails
Handler->>Config: get_jurisdiction_operations_team_emails(new)
Config-->>Handler: new_ops_emails
Handler->>EmailSvc: send_home_jurisdiction_change_new_state_notification(...)
EmailSvc->>NewOps: email
end
Handler-->>EventBus: record success (NotificationTracker)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
landonshumway-ia
force-pushed
the
feat/home-state-change-notifications
branch
2 times, most recently
from
764a760 to
bac9c63
Compare
![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: 7
🤖 Fix all issues with AI agents
In
`@backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py`:
- Around line 2596-2598: The code currently raises CCInvalidRequestException
when selected_jurisdiction equals
top_level_provider_record.currentHomeJurisdiction
(home_jurisdiction_before_update), making PUT non-idempotent; change this to
treat the request as a no-op: detect equality of selected_jurisdiction and
home_jurisdiction_before_update, do not raise, return the existing
top_level_provider_record/current home jurisdiction immediately, and ensure that
any downstream notification or side-effect logic is executed only when an actual
change occurs (i.e., gate notifications on selected_jurisdiction !=
home_jurisdiction_before_update); update any callers or response path to expect
a no-op return instead of an exception.
In
`@backend/compact-connect/lambdas/python/common/cc_common/email_service_client.py`:
- Around line 848-882: In send_home_jurisdiction_change_new_state_notification,
previous_jurisdiction is typed as str | None but the code calls
previous_jurisdiction.upper() unconditionally; either handle the None case or
enforce non-None. Fix by updating the payload construction in
send_home_jurisdiction_change_new_state_notification so 'previousJurisdiction'
is computed safely (e.g., previous_jurisdiction.upper() if previous_jurisdiction
is not None else '' or a sentinel like 'N/A'), or alternatively change the
function signature/type and add an explicit validation (raise ValueError) if
previous_jurisdiction is None; ensure the change is applied to the
templateVariables entry for previousJurisdiction so no AttributeError can occur.
In
`@backend/compact-connect/lambdas/python/data-events/handlers/home_jurisdiction_events.py`:
- Around line 167-177: Guard against previous_jurisdiction being None before
calling
config.email_service_client.send_home_jurisdiction_change_new_state_notification:
normalize the variable referenced as previous_jurisdiction (used in the call) to
a safe string (e.g. empty string or explicit "UNKNOWN") when it is None so the
email client’s uppercase operation won’t raise; update the block inside the
tracker.should_send_state_notification(new_jurisdiction) branch (where
send_home_jurisdiction_change_new_state_notification is invoked) to replace or
set previous_jurisdiction to a non-None value before passing it.
In
`@backend/compact-connect/lambdas/python/provider-data-v1/handlers/provider_users.py`:
- Around line 111-126: The code currently calls
config.event_bus_client.publish_home_jurisdiction_change_event after
update_provider_home_state_jurisdiction and will surface any publish failure as
a request error; change this to a best‑effort publish by first comparing
previous_home_jurisdiction to selected_jurisdiction and only attempting to
publish if they differ, then wrap the publish call in a try/except that logs the
exception (using the existing logger or config.logger) rather than re-raising so
the API does not fail when event publishing fails; keep the feature flag check
with
is_feature_enabled(FeatureFlagEnum.HOME_JURISDICTION_CHANGE_NOTIFICATION_FLAG,
fail_default=False) intact.
In
`@backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_provider_users_home_jurisdiction_change.py`:
- Around line 1084-1088: The test docstring/comment in
test_provider_users_home_jurisdiction_change.py is misleading: it mentions
"publishes" and "True" while the test sets mock_is_feature_enabled.return_value
= False; update the docstring/comment above the block (and any inline comment
referencing the flag) to state that the feature flag is off (False) and the
handler provider_users_api_handler should NOT publish the event in this case, so
the comment accurately describes the test behavior.
In `@backend/compact-connect/stacks/feature_flag_stack/__init__.py`:
- Around line 130-142: The comment for
home_jurisdiction_change_notification_flag conflicts with its actual config:
auto_enable_envs on FeatureFlagResource includes
FeatureFlagEnvironmentName.TEST, BETA, and PROD which will auto-enable the flag
with passPercentage=100; either update the comment to state the flag is
auto-enabled in those environments or remove those entries from auto_enable_envs
to require manual enablement—locate the FeatureFlagResource instantiation for
home_jurisdiction_change_notification_flag and either change the comment to
reflect auto-enable behavior or delete the FeatureFlagEnvironmentName.* entries
from auto_enable_envs so the flag truly starts disabled.
In `@backend/compact-connect/stacks/notification_stack.py`:
- Around line 211-214: The emailer_event_listener_handler is granted read access
to persistent_stack.compact_configuration_table but its environment dict is
missing COMPACT_CONFIGURATION_TABLE_NAME; update the handler's environment
(where environment={...} for emailer_event_listener_handler) to include
'COMPACT_CONFIGURATION_TABLE_NAME':
persistent_stack.compact_configuration_table.table_name alongside the existing
PROVIDER_TABLE_NAME, EMAIL_NOTIFICATION_SERVICE_LAMBDA_NAME,
EVENT_STATE_TABLE_NAME and **self.common_env_vars so the handler can initialize
the configuration client.
🧹 Nitpick comments (1)
backend/compact-connect/lambdas/nodejs/lib/email/email-notification-service.ts (1)
563-577: Optional: Simplify the else clause.The
elsekeyword is unnecessary since theifblock returns. This is a minor style nit.♻️ Suggested simplification
private formatJurisdictionForEmail(jurisdiction: string): string { if (jurisdiction.toLowerCase() === 'other') { return 'an unlisted jurisdiction'; } - // else we uppercase the jurisdiction - else { - return jurisdiction.toUpperCase(); - } + return jurisdiction.toUpperCase(); }
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/email_service_client.py
Show resolved
Hide resolved
backend/compact-connect/lambdas/python/data-events/handlers/home_jurisdiction_events.py
Show resolved
Hide resolved
backend/compact-connect/lambdas/python/provider-data-v1/handlers/provider_users.py
Outdated
Show resolved
Hide resolved
...rovider-data-v1/tests/function/test_handlers/test_provider_users_home_jurisdiction_change.py
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py`:
- Around line 2596-2605: The code calls .lower() on
top_level_provider_record.currentHomeJurisdiction without guarding for None and
logs a no-op but does not return, so update the no-op handling: in the method
that uses top_level_provider_record.currentHomeJurisdiction and
selected_jurisdiction (the block that logs 'New selected jurisdiction matches
current home state...'), first check for None (e.g., if currentHomeJurisdiction
is None -> skip lower/compare), then perform a case-insensitive comparison only
when both values are non-None; when they match, call logger.info with the
existing context and return early to avoid performing the full update; apply the
same None-guard + early return pattern to the other similar occurrences that use
currentHomeJurisdiction and selected_jurisdiction (the blocks around the other
two occurrences referenced).
In
`@backend/compact-connect/lambdas/python/provider-data-v1/handlers/provider_users.py`:
- Around line 125-148: The current except clause references a non-existent
config.events_client.exceptions.InternalException so publish failures
(ClientError, network, throttling) will propagate; change to catch a broad
exception (e.g., except Exception as e) around the
publish_home_jurisdiction_change_event call in provider_users.py, log the
exception details (include e) along with
compact/previous_home_jurisdiction/selected_jurisdiction/provider_id via
logger.error, and continue execution to preserve best-effort publish semantics
after the DB update.
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/provider_users.py
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@backend/compact-connect/lambdas/python/provider-data-v1/handlers/provider_users.py`:
- Around line 125-149: The comparison between previous_home_jurisdiction and
selected_jurisdiction can produce false positives because selected_jurisdiction
is lowercased while previous_home_jurisdiction may not be; update the logic in
provider_users.py so you normalize both values (e.g., .lower() or .upper())
before the if check that gates the publish_home_jurisdiction_change_event call,
ensuring you only change the comparison behavior (do not alter the stored value)
and keep the existing exception logging and event payloads unchanged (use the
original variables when publishing the event).
backend/compact-connect/lambdas/python/provider-data-v1/handlers/provider_users.py
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@backend/compact-connect/tests/smoke/home_jurisdiction_change_smoke_tests.py`:
- Around line 128-146: The test currently assumes a single privilege by checking
non-empty lists and indexing [0]; update the smoke tests to assert uniqueness by
replacing existence checks with exact-cardinality assertions (e.g., assert
len(ne_privileges) == 1) before using index 0 for all places using
provider_user_records.get_privilege_records (including the blocks around
provider_user_records.get_privilege_records -> ne_privileges and the other
occurrences you flagged at ~294-307 and ~351-364); raise
SmokeTestFailureException with a clear message when len(...) != 1 so duplicates
fail the test, and ensure queries against get_privilege_records are not
paginated when filtering by jurisdiction.
- Around line 26-49: The test_home_jurisdiction_change_to_oh test mutates the
provider user's home jurisdiction and leaves it changed; capture the current
jurisdiction before calling requests.put and restore it in a finally block to
avoid state leaks. Specifically, call the API to read the current home
jurisdiction (using the same auth helper get_provider_user_auth_headers_cached
or existing endpoint) and store it in a variable, perform the PUT to set 'oh' as
now, then in a finally clause issue another PUT to reset the saved original
jurisdiction (and log or raise if the restore fails); reference
test_jurisdiction_configuration, requests.put, and
get_provider_user_auth_headers_cached to locate where to add the capture/restore
logic.
backend/compact-connect/tests/smoke/home_jurisdiction_change_smoke_tests.py
Show resolved
Hide resolved
backend/compact-connect/tests/smoke/home_jurisdiction_change_smoke_tests.py
Show resolved
Hide resolved
landonshumway-ia
force-pushed
the
feat/home-state-change-notifications
branch
from
3e0141c to
4153220
Compare
landonshumway-ia
force-pushed
the
feat/home-state-change-notifications
branch
from
4153220 to
df276f2
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@backend/compact-connect/lambdas/python/data-events/tests/function/test_home_jurisdiction_events.py`:
- Around line 137-175: The test sets up previous jurisdiction as
OTHER_JURISDICTION but calls _generate_home_jurisdiction_change_message(None,
'tx'), so update the message payload to use the actual constant
(OTHER_JURISDICTION) for the previous value; change the call to
_generate_home_jurisdiction_change_message(OTHER_JURISDICTION, 'tx') so
home_jurisdiction_change_notification_listener exercises the "previous is other"
branch and the assertions on
send_home_jurisdiction_change_old_state_notification/send_home_jurisdiction_change_new_state_notification
behave as intended.
🧹 Nitpick comments (1)
backend/compact-connect/lambdas/nodejs/tests/email-notification-service.test.ts (1)
1991-2016: Add a happy‑path test for the new‑state notification wiring.Right now this suite only asserts the missing‑variables error path, so the lambda’s integration (jurisdiction lookup + ops team recipients + SES send) isn’t exercised. A lightweight success test would protect against regressions.
✅ Proposed test addition
describe('Home Jurisdiction Change New State Notification', () => { const SAMPLE_HOME_JURISDICTION_CHANGE_NEW_STATE_NOTIFICATION_EVENT: EmailNotificationEvent = { template: 'homeJurisdictionChangeNewStateNotification', recipientType: 'JURISDICTION_OPERATIONS_TEAM', compact: 'aslp', jurisdiction: 'oh', templateVariables: { providerFirstName: 'John', providerLastName: 'Doe', providerId: 'provider-123', previousJurisdiction: 'TX', newJurisdiction: 'OH' } }; + it('should successfully send home jurisdiction change new state notification email', async () => { + // Ensure ops-team recipients exist for the jurisdiction + mockDynamoDBClient.on(GetItemCommand).callsFake((input) => { + if (input.Key.sk.S === 'aslp#JURISDICTION#oh') { + return Promise.resolve({ + Item: { + ...SAMPLE_JURISDICTION_CONFIGURATION, + jurisdictionOperationsTeamEmails: { L: [{ S: 'oh-ops@example.com' }] } + } + }); + } + return Promise.resolve({ Item: SAMPLE_COMPACT_CONFIGURATION }); + }); + + const response = await lambda.handler( + SAMPLE_HOME_JURISDICTION_CHANGE_NEW_STATE_NOTIFICATION_EVENT, + {} as any + ); + + expect(response).toEqual({ message: 'Email message sent' }); + expect(mockSESClient).toHaveReceivedCommandWith(SendEmailCommand, { + Destination: { ToAddresses: ['oh-ops@example.com'] }, + Content: expect.any(Object), + FromEmailAddress: 'Compact Connect <noreply@example.org>' + }); + }); + it('should throw error when required template variables are missing', async () => { const eventWithMissingVariables: EmailNotificationEvent = { ...SAMPLE_HOME_JURISDICTION_CHANGE_NEW_STATE_NOTIFICATION_EVENT, templateVariables: {} };
...d/compact-connect/lambdas/python/data-events/tests/function/test_home_jurisdiction_events.py
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@backend/compact-connect/lambdas/python/data-events/tests/function/test_home_jurisdiction_events.py`:
- Around line 225-269: The test test_error_handling_records_failure only patches
EmailServiceClient.send_home_jurisdiction_change_old_state_notification, leaving
the new-state send unpatched which can call external code when the handler
continues; update the test to also patch
EmailServiceClient.send_home_jurisdiction_change_new_state_notification (or add
a second `@patch` decorator) and set its return or side_effect appropriately so
the new-state send is stubbed out, ensuring
home_jurisdiction_change_notification_listener runs isolated and only the
old-state failure is exercised.
...d/compact-connect/lambdas/python/data-events/tests/function/test_home_jurisdiction_events.py
Show resolved
Hide resolved
Collaborator
Author
|
@jlkravitz This is ready for your review. Thanks |
jlkravitz
reviewed
Jan 29, 2026
backend/compact-connect/lambdas/nodejs/tests/lib/email/email-notification-service.test.ts
Show resolved
Hide resolved
...rovider-data-v1/tests/function/test_handlers/test_provider_users_home_jurisdiction_change.py
Show resolved
Hide resolved
...d/compact-connect/lambdas/python/data-events/tests/function/test_home_jurisdiction_events.py
Outdated
Show resolved
Hide resolved
…ion/test_home_jurisdiction_events.py Co-authored-by: Joshua Kravitz <jlkravitz@gmail.com>
jlkravitz
approved these changes
Feb 2, 2026
Collaborator
jlkravitz
left a comment
jlkravitz
left a comment
There was a problem hiding this comment.
@isabeleliassen Good to merge!
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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.
When a provider updates their declared home state through the system, the system will now send out notifications to both the previous home state and the new home state.
Requirements List
Description List
Testing List
yarn test:unit:allshould run without errors or warningsyarn serveshould run without errors or warningsyarn buildshould run without errors or warningsCloses #1258
Summary by CodeRabbit
New Features
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.