Return full list of adverse actions as top-level field#1512
Merged
isabeleliassen merged 14 commits intocsg-org:mainfrom May 4, 2026
Merged
Return full list of adverse actions as top-level field#1512isabeleliassen merged 14 commits intocsg-org:mainfrom
isabeleliassen merged 14 commits intocsg-org:mainfrom
Conversation
Contributor
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (5)
📒 Files selected for processing (41)
📝 WalkthroughWalkthroughAdds a provider-level top-level adverseActions array to provider API responses by aggregating adverse-action records across the provider; updates response generation, marshmallow schemas, API model wiring, fixtures, snapshots, and unit tests. Also includes unrelated Node.js import/dep/tsconfig changes and CI/requirements bumps. ChangesProvider API: response generation, schema, model, tests
Node.js lambdas: imports, wiring, dev deps, tsconfig
CI / requirements bumps
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant API_Generator as API Generator
participant Records as ProviderRecords
participant AA_Store as AdverseActionStore
participant Serializer as ResponseSerializer
Client->>API_Generator: Request provider details
API_Generator->>Records: Fetch provider + related records
Records->>AA_Store: Query adverse-action records for provider
AA_Store-->>Records: Return adverse-action records
Records->>API_Generator: Return records + adverseActions
API_Generator->>Serializer: Build response (licenses, privileges, provider-level adverseActions)
Serializer-->>Client: Return serialized provider response with top-level `adverseActions`
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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. Review rate limit: 0/1 reviews remaining, refill in 52 minutes and 36 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
backend/cosmetology-app/lambdas/python/common/cc_common/data_model/schema/provider/api.py (1)
117-117: Minor: inconsistentNestedinstantiation style.Throughout this file, nested schemas are passed as instances (e.g.,
Nested(LicenseReadPrivateResponseSchema(), ...)on line 114 andNested(PrivilegeReadPrivateResponseSchema(), ...)on line 115), but the new lines pass the class itself (Nested(AdverseActionGeneralResponseSchema, ...)). Both forms are accepted by marshmallow, but matching the surrounding style improves consistency.♻️ Proposed style alignment
- adverseActions = List(Nested(AdverseActionGeneralResponseSchema, required=False, allow_none=False)) + adverseActions = List(Nested(AdverseActionGeneralResponseSchema(), required=False, allow_none=False))(applies on both line 117 and line 166)
Also applies to: 166-166
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/cosmetology-app/lambdas/python/common/cc_common/data_model/schema/provider/api.py` at line 117, The nested schema instantiation for adverseActions uses the class name instead of an instance, which is inconsistent with surrounding usages; change Nested(AdverseActionGeneralResponseSchema, ...) to Nested(AdverseActionGeneralResponseSchema(), ...) (and do the same for the other occurrence noted) so it matches patterns like Nested(LicenseReadPrivateResponseSchema(), ...) and Nested(PrivilegeReadPrivateResponseSchema(), ...); update the Nested calls referencing AdverseActionGeneralResponseSchema to pass an instance.backend/cosmetology-app/lambdas/python/common/tests/unit/test_provider_record_util.py (2)
573-596: Optional: deduplicate the test helpers.
_make_provider_recordsand_patch_config_for_privilege_generationare now defined identically (modulo theextra_recordsparameter) in three test classes in this file. A small shared mixin or module-level helpers would cut the duplication. Not blocking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/cosmetology-app/lambdas/python/common/tests/unit/test_provider_record_util.py` around lines 573 - 596, Duplicate helper methods _make_provider_records and _patch_config_for_privilege_generation are repeated across multiple test classes; refactor by extracting them into a shared test helper (either a module-level function or a mixin class) and have the test classes import/use that shared helper instead of redefining the methods. Move the TestDataGenerator-based logic from _make_provider_records and the MagicMock/patch logic from _patch_config_for_privilege_generation into the new shared helper (referencing TestDataGenerator and the patch target 'cc_common.data_model.provider_record_util.config') and update each test class to call the centralized functions or inherit the mixin to remove duplication.
598-637: Consider adding assertions that nested per-license/per-privilegeadverseActionsare still populated.The PR explicitly states the existing nested
adverseActionsunderlicensesandprivilegesare retained for backward compatibility. This test only asserts the new top-level field. Adding a couple of extra assertions guards against a future refactor inadvertently dropping the nested lists.♻️ Suggested additions
self.assertEqual( [license_adverse_action.to_dict(), privilege_adverse_action.to_dict()], api_response['adverseActions'], ) + # Backward compatibility: nested adverseActions on the matching license and privilege + # should still be populated. + oh_license = next(lic for lic in api_response['licenses'] if lic['jurisdiction'] == 'oh') + self.assertEqual([license_adverse_action.to_dict()], oh_license['adverseActions']) + al_privilege = next(p for p in api_response['privileges'] if p['jurisdiction'] == 'al') + self.assertEqual([privilege_adverse_action.to_dict()], al_privilege['adverseActions'])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/cosmetology-app/lambdas/python/common/tests/unit/test_provider_record_util.py` around lines 598 - 637, Update the test method test_generate_api_response_object_returns_adverse_actions_as_a_top_level_field_for_all_adverse_actions to also assert that nested per-license and per-privilege adverseActions remain populated: after calling ProviderUserRecords(records).generate_api_response_object(), assert that api_response['licenses'][0]['adverseActions'] contains the license_adverse_action.to_dict() and that the appropriate privilege entry in api_response['privileges'] contains privilege_adverse_action.to_dict(); use ProviderUserRecords, generate_api_response_object, and the api_response dict keys to locate where to add these extra assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@backend/cosmetology-app/lambdas/python/common/cc_common/data_model/schema/provider/api.py`:
- Line 117: The nested schema instantiation for adverseActions uses the class
name instead of an instance, which is inconsistent with surrounding usages;
change Nested(AdverseActionGeneralResponseSchema, ...) to
Nested(AdverseActionGeneralResponseSchema(), ...) (and do the same for the other
occurrence noted) so it matches patterns like
Nested(LicenseReadPrivateResponseSchema(), ...) and
Nested(PrivilegeReadPrivateResponseSchema(), ...); update the Nested calls
referencing AdverseActionGeneralResponseSchema to pass an instance.
In
`@backend/cosmetology-app/lambdas/python/common/tests/unit/test_provider_record_util.py`:
- Around line 573-596: Duplicate helper methods _make_provider_records and
_patch_config_for_privilege_generation are repeated across multiple test
classes; refactor by extracting them into a shared test helper (either a
module-level function or a mixin class) and have the test classes import/use
that shared helper instead of redefining the methods. Move the
TestDataGenerator-based logic from _make_provider_records and the
MagicMock/patch logic from _patch_config_for_privilege_generation into the new
shared helper (referencing TestDataGenerator and the patch target
'cc_common.data_model.provider_record_util.config') and update each test class
to call the centralized functions or inherit the mixin to remove duplication.
- Around line 598-637: Update the test method
test_generate_api_response_object_returns_adverse_actions_as_a_top_level_field_for_all_adverse_actions
to also assert that nested per-license and per-privilege adverseActions remain
populated: after calling
ProviderUserRecords(records).generate_api_response_object(), assert that
api_response['licenses'][0]['adverseActions'] contains the
license_adverse_action.to_dict() and that the appropriate privilege entry in
api_response['privileges'] contains privilege_adverse_action.to_dict(); use
ProviderUserRecords, generate_api_response_object, and the api_response dict
keys to locate where to add these extra assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4b43b50d-6034-477d-8aa3-1cdeedc4278c
📒 Files selected for processing (4)
backend/cosmetology-app/lambdas/python/common/cc_common/data_model/provider_record_util.pybackend/cosmetology-app/lambdas/python/common/cc_common/data_model/schema/provider/api.pybackend/cosmetology-app/lambdas/python/common/tests/resources/api/provider-detail-response.jsonbackend/cosmetology-app/lambdas/python/common/tests/unit/test_provider_record_util.py
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
backend/cosmetology-app/stacks/api_stack/v1_api/api_model.py (1)
707-758: Reformat_adverse_action_schemato use consistent indentation.The body of
_adverse_action_schema(lines 710–757) preserves the deep indentation it had at its original inline location, which is now visually inconsistent with the rest of the file (4-space increments). This is syntactically valid because everything is inside parentheses, but it's likely to be flagged by a formatter and hurts readability.♻️ Proposed fix to normalize indentation
`@property` def _adverse_action_schema(self): return JsonSchema( - type=JsonSchemaType.ARRAY, - items=JsonSchema( - type=JsonSchemaType.OBJECT, - required=[ - 'type', - 'compact', - 'providerId', - 'jurisdiction', - 'licenseTypeAbbreviation', - 'licenseType', - 'actionAgainst', - 'effectiveStartDate', - 'creationDate', - 'adverseActionId', - 'dateOfUpdate', - 'encumbranceType', - ], - properties={ - 'type': JsonSchema(type=JsonSchemaType.STRING, enum=['adverseAction']), - 'compact': JsonSchema( - type=JsonSchemaType.STRING, enum=self.stack.node.get_context('compacts') - ), - 'providerId': JsonSchema( - type=JsonSchemaType.STRING, pattern=cc_api.UUID4_FORMAT - ), - 'jurisdiction': JsonSchema( - type=JsonSchemaType.STRING, - enum=self.stack.node.get_context('jurisdictions'), - ), - 'licenseTypeAbbreviation': JsonSchema(type=JsonSchemaType.STRING), - 'licenseType': JsonSchema(type=JsonSchemaType.STRING), - 'actionAgainst': JsonSchema(type=JsonSchemaType.STRING), - 'effectiveStartDate': JsonSchema( - type=JsonSchemaType.STRING, format='date', pattern=cc_api.YMD_FORMAT - ), - 'creationDate': JsonSchema( - type=JsonSchemaType.STRING, format='date', pattern=cc_api.YMD_FORMAT - ), - 'adverseActionId': JsonSchema(type=JsonSchemaType.STRING), - 'effectiveLiftDate': JsonSchema( - type=JsonSchemaType.STRING, format='date', pattern=cc_api.YMD_FORMAT - ), - 'dateOfUpdate': JsonSchema(type=JsonSchemaType.STRING, format='date-time'), - 'encumbranceType': JsonSchema(type=JsonSchemaType.STRING), - 'clinicalPrivilegeActionCategories': self._clinical_privilege_action_categories_schema, # noqa: E501 - 'liftingUser': JsonSchema(type=JsonSchemaType.STRING), - }, - ), - ) + type=JsonSchemaType.ARRAY, + items=JsonSchema( + type=JsonSchemaType.OBJECT, + required=[ + 'type', + 'compact', + 'providerId', + 'jurisdiction', + 'licenseTypeAbbreviation', + 'licenseType', + 'actionAgainst', + 'effectiveStartDate', + 'creationDate', + 'adverseActionId', + 'dateOfUpdate', + 'encumbranceType', + ], + properties={ + 'type': JsonSchema(type=JsonSchemaType.STRING, enum=['adverseAction']), + 'compact': JsonSchema( + type=JsonSchemaType.STRING, enum=self.stack.node.get_context('compacts') + ), + 'providerId': JsonSchema(type=JsonSchemaType.STRING, pattern=cc_api.UUID4_FORMAT), + 'jurisdiction': JsonSchema( + type=JsonSchemaType.STRING, + enum=self.stack.node.get_context('jurisdictions'), + ), + 'licenseTypeAbbreviation': JsonSchema(type=JsonSchemaType.STRING), + 'licenseType': JsonSchema(type=JsonSchemaType.STRING), + 'actionAgainst': JsonSchema(type=JsonSchemaType.STRING), + 'effectiveStartDate': JsonSchema( + type=JsonSchemaType.STRING, format='date', pattern=cc_api.YMD_FORMAT + ), + 'creationDate': JsonSchema( + type=JsonSchemaType.STRING, format='date', pattern=cc_api.YMD_FORMAT + ), + 'adverseActionId': JsonSchema(type=JsonSchemaType.STRING), + 'effectiveLiftDate': JsonSchema( + type=JsonSchemaType.STRING, format='date', pattern=cc_api.YMD_FORMAT + ), + 'dateOfUpdate': JsonSchema(type=JsonSchemaType.STRING, format='date-time'), + 'encumbranceType': JsonSchema(type=JsonSchemaType.STRING), + 'clinicalPrivilegeActionCategories': self._clinical_privilege_action_categories_schema, + 'liftingUser': JsonSchema(type=JsonSchemaType.STRING), + }, + ), + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/cosmetology-app/stacks/api_stack/v1_api/api_model.py` around lines 707 - 758, The _adverse_action_schema property uses inconsistent/deep indentation that breaks the file's 4-space style; fix it by reformatting the return block for JsonSchema(...) so nested arguments (type=..., items=..., inner JsonSchema(...), required=..., properties={...}) and all property entries (e.g. 'type', 'compact', 'providerId', 'effectiveStartDate', 'dateOfUpdate', 'encumbranceType', 'clinicalPrivilegeActionCategories' referencing self._clinical_privilege_action_categories_schema, and formats using cc_api.YMD_FORMAT) follow the file's 4-space indentation levels and align parentheses consistently; keep the same tokens and structure (JsonSchema, JsonSchemaType, enum calls to self.stack.node.get_context, patterns using cc_api.UUID4_FORMAT/YMD_FORMAT) — only adjust whitespace and indentation to match surrounding code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@backend/cosmetology-app/stacks/api_stack/v1_api/api_model.py`:
- Around line 707-758: The _adverse_action_schema property uses
inconsistent/deep indentation that breaks the file's 4-space style; fix it by
reformatting the return block for JsonSchema(...) so nested arguments (type=...,
items=..., inner JsonSchema(...), required=..., properties={...}) and all
property entries (e.g. 'type', 'compact', 'providerId', 'effectiveStartDate',
'dateOfUpdate', 'encumbranceType', 'clinicalPrivilegeActionCategories'
referencing self._clinical_privilege_action_categories_schema, and formats using
cc_api.YMD_FORMAT) follow the file's 4-space indentation levels and align
parentheses consistently; keep the same tokens and structure (JsonSchema,
JsonSchemaType, enum calls to self.stack.node.get_context, patterns using
cc_api.UUID4_FORMAT/YMD_FORMAT) — only adjust whitespace and indentation to
match surrounding code.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 067f32c1-6c16-4da3-8602-b7606b13b0f0
📒 Files selected for processing (4)
backend/cosmetology-app/lambdas/python/common/cc_common/data_model/schema/provider/api.pybackend/cosmetology-app/stacks/api_stack/v1_api/api_model.pybackend/cosmetology-app/tests/app/test_provider_users/__init__.pybackend/cosmetology-app/tests/resources/snapshots/GET_PROVIDER_RESPONSE_SCHEMA.json
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
backend/cosmetology-app/lambdas/python/common/tests/unit/test_provider_record_util.py (2)
598-642: Strengthen this test to avoid order-coupling and cover backward compatibility.The current assertion at Line 639 is order-sensitive and only validates the top-level array. Consider making the top-level check order-insensitive and asserting nested
licenses[].adverseActions/privileges[].adverseActionsin the same scenario.Suggested test update
- self.assertEqual( + self.assertCountEqual( [license_adverse_action.to_dict(), privilege_adverse_action.to_dict()], api_response['adverseActions'], ) + + self.assertEqual( + [license_adverse_action.to_dict()], + api_response['licenses'][0]['adverseActions'], + ) + privilege_al = next(p for p in api_response['privileges'] if p['jurisdiction'] == 'al') + self.assertEqual( + [privilege_adverse_action.to_dict()], + privilege_al['adverseActions'], + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/cosmetology-app/lambdas/python/common/tests/unit/test_provider_record_util.py` around lines 598 - 642, The test is order-sensitive and only asserts the top-level api_response['adverseActions']; update the assertion to be order-insensitive (e.g., compare sets or sort by a unique key such as the adverse action id or serialize-to-dict tuple) when comparing [license_adverse_action.to_dict(), privilege_adverse_action.to_dict()] against api_response['adverseActions'], and also add assertions that the same adverse actions appear in the nested structures produced by ProviderUserRecords.generate_api_response_object — specifically verify that license_adverse_action appears under api_response['licenses'][...]['adverseActions'] for the matching license and privilege_adverse_action appears under api_response['privileges'][...]['adverseActions'] to cover backward compatibility; use the existing license_adverse_action and privilege_adverse_action test objects to locate matches.
573-597: Reduce helper duplication across test classes.
_make_provider_recordsand_patch_config_for_privilege_generationare now repeated in multiple classes in this file. Extracting them into a shared mixin/helper will reduce drift and future maintenance cost.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/cosmetology-app/lambdas/python/common/tests/unit/test_provider_record_util.py` around lines 573 - 597, Extract the duplicated helpers _make_provider_records and _patch_config_for_privilege_generation into a single test helper (e.g., a TestHelpers mixin or module-level helper) and have the existing test classes import/use it rather than redefining them; move the implementations (including use of TestDataGenerator, serialize_to_database_record, and the MagicMock/patch behavior for config.live_compact_jurisdictions and license_type_abbreviations) into that shared helper, keep the same function names/signatures so callers need only inherit or call the helper, update tests to inherit the mixin or call the helper functions, and remove the duplicate definitions from the individual test classes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@backend/cosmetology-app/lambdas/python/common/tests/unit/test_provider_record_util.py`:
- Around line 598-642: The test is order-sensitive and only asserts the
top-level api_response['adverseActions']; update the assertion to be
order-insensitive (e.g., compare sets or sort by a unique key such as the
adverse action id or serialize-to-dict tuple) when comparing
[license_adverse_action.to_dict(), privilege_adverse_action.to_dict()] against
api_response['adverseActions'], and also add assertions that the same adverse
actions appear in the nested structures produced by
ProviderUserRecords.generate_api_response_object — specifically verify that
license_adverse_action appears under
api_response['licenses'][...]['adverseActions'] for the matching license and
privilege_adverse_action appears under
api_response['privileges'][...]['adverseActions'] to cover backward
compatibility; use the existing license_adverse_action and
privilege_adverse_action test objects to locate matches.
- Around line 573-597: Extract the duplicated helpers _make_provider_records and
_patch_config_for_privilege_generation into a single test helper (e.g., a
TestHelpers mixin or module-level helper) and have the existing test classes
import/use it rather than redefining them; move the implementations (including
use of TestDataGenerator, serialize_to_database_record, and the MagicMock/patch
behavior for config.live_compact_jurisdictions and license_type_abbreviations)
into that shared helper, keep the same function names/signatures so callers need
only inherit or call the helper, update tests to inherit the mixin or call the
helper functions, and remove the duplicate definitions from the individual test
classes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 65f2e012-8f62-4a88-9469-fde46e7a9705
⛔ Files ignored due to path filters (1)
backend/cosmetology-app/lambdas/nodejs/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (3)
backend/cosmetology-app/lambdas/nodejs/package.jsonbackend/cosmetology-app/lambdas/python/common/tests/unit/test_provider_record_util.pybackend/cosmetology-app/stacks/api_stack/v1_api/api_model.py
✅ Files skipped from review due to trivial changes (1)
- backend/cosmetology-app/lambdas/nodejs/package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/cosmetology-app/stacks/api_stack/v1_api/api_model.py
ChiefStief
reviewed
Apr 27, 2026
landonshumway-ia
force-pushed
the
feat/disciplinary-table-schema
branch
from
ffce989 to
56fc467
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
backend/cosmetology-app/lambdas/python/common/tests/unit/test_provider_record_util.py (2)
573-597: Consider extracting shared test helpers to avoid drift.
_make_provider_recordsand_patch_config_for_privilege_generationare duplicated across test classes in this file. Pulling them into a shared base/mixin would reduce maintenance overhead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/cosmetology-app/lambdas/python/common/tests/unit/test_provider_record_util.py` around lines 573 - 597, Duplicate test helper functions _make_provider_records and _patch_config_for_privilege_generation should be extracted into a shared test base or mixin class (e.g., ProviderTestMixin or ProviderTestBase) and referenced by test classes; move the implementations of _make_provider_records and _patch_config_for_privilege_generation into that shared class, keep the same function names and behavior (including use of TestDataGenerator and the patch target 'cc_common.data_model.provider_record_util.config'), then update existing test classes to inherit from or import the mixin/base instead of redefining these helpers and remove the redundant copies to avoid drift.
639-642: Prefer order-insensitive assertion for aggregated adverse actions.This check is stricter than the contract and can fail on harmless ordering changes.
Suggested test tweak
- self.assertEqual( + self.assertCountEqual( [license_adverse_action.to_dict(), privilege_adverse_action.to_dict()], api_response['adverseActions'], )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/cosmetology-app/lambdas/python/common/tests/unit/test_provider_record_util.py` around lines 639 - 642, The test currently asserts exact list order for adverse actions using self.assertEqual([license_adverse_action.to_dict(), privilege_adverse_action.to_dict()], api_response['adverseActions']), which is order-sensitive; change it to an order-insensitive assertion (e.g., replace with self.assertCountEqual(api_response['adverseActions'], [license_adverse_action.to_dict(), privilege_adverse_action.to_dict()]) or otherwise compare as sets/sorted lists) so the test passes regardless of the aggregated adverseActions ordering.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@backend/cosmetology-app/lambdas/python/common/tests/unit/test_provider_record_util.py`:
- Around line 573-597: Duplicate test helper functions _make_provider_records
and _patch_config_for_privilege_generation should be extracted into a shared
test base or mixin class (e.g., ProviderTestMixin or ProviderTestBase) and
referenced by test classes; move the implementations of _make_provider_records
and _patch_config_for_privilege_generation into that shared class, keep the same
function names and behavior (including use of TestDataGenerator and the patch
target 'cc_common.data_model.provider_record_util.config'), then update existing
test classes to inherit from or import the mixin/base instead of redefining
these helpers and remove the redundant copies to avoid drift.
- Around line 639-642: The test currently asserts exact list order for adverse
actions using self.assertEqual([license_adverse_action.to_dict(),
privilege_adverse_action.to_dict()], api_response['adverseActions']), which is
order-sensitive; change it to an order-insensitive assertion (e.g., replace with
self.assertCountEqual(api_response['adverseActions'],
[license_adverse_action.to_dict(), privilege_adverse_action.to_dict()]) or
otherwise compare as sets/sorted lists) so the test passes regardless of the
aggregated adverseActions ordering.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bf7a7e58-b5e7-4dc7-bc1c-239600982f79
⛔ Files ignored due to path filters (1)
backend/cosmetology-app/lambdas/nodejs/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (8)
backend/cosmetology-app/lambdas/nodejs/package.jsonbackend/cosmetology-app/lambdas/python/common/cc_common/data_model/provider_record_util.pybackend/cosmetology-app/lambdas/python/common/cc_common/data_model/schema/provider/api.pybackend/cosmetology-app/lambdas/python/common/tests/resources/api/provider-detail-response.jsonbackend/cosmetology-app/lambdas/python/common/tests/unit/test_provider_record_util.pybackend/cosmetology-app/stacks/api_stack/v1_api/api_model.pybackend/cosmetology-app/tests/app/test_provider_users/__init__.pybackend/cosmetology-app/tests/resources/snapshots/GET_PROVIDER_RESPONSE_SCHEMA.json
✅ Files skipped from review due to trivial changes (2)
- backend/cosmetology-app/lambdas/python/common/tests/resources/api/provider-detail-response.json
- backend/cosmetology-app/lambdas/nodejs/package.json
🚧 Files skipped from review as they are similar to previous changes (3)
- backend/cosmetology-app/lambdas/python/common/cc_common/data_model/provider_record_util.py
- backend/cosmetology-app/tests/resources/snapshots/GET_PROVIDER_RESPONSE_SCHEMA.json
- backend/cosmetology-app/lambdas/python/common/cc_common/data_model/schema/provider/api.py
Collaborator
Author
|
@jlkravitz This is ready for your review. Thanks |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/cosmetology-app/lambdas/nodejs/tsconfig.json`:
- Around line 22-24: The "types" array in tsconfig.json is too restrictive
(currently only "node" and "jest"), which prevents React's ambient types (e.g.,
JSX.IntrinsicElements) from being loaded for TSX files under lib/**/*.tsx;
update the tsconfig.json "types" array to include "react" alongside "node" and
"jest" so `@types/react` is picked up for TSX compilation and JSX types are
available.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e916a02f-4191-455a-aeb0-9399611ee4e4
⛔ Files ignored due to path filters (3)
backend/cosmetology-app/lambdas/nodejs/cognito-emails/handler.js.mapis excluded by!**/*.mapbackend/cosmetology-app/lambdas/nodejs/cognito-emails/lambda.js.mapis excluded by!**/*.mapbackend/cosmetology-app/lambdas/nodejs/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (11)
backend/cosmetology-app/docs/internal/api-specification/latest-oas30.jsonbackend/cosmetology-app/docs/internal/postman/postman-collection.jsonbackend/cosmetology-app/lambdas/nodejs/cognito-emails/handler.jsbackend/cosmetology-app/lambdas/nodejs/cognito-emails/lambda.jsbackend/cosmetology-app/lambdas/nodejs/cognito-emails/lambda.tsbackend/cosmetology-app/lambdas/nodejs/email-notification-service/lambda.tsbackend/cosmetology-app/lambdas/nodejs/ingest-event-reporter/lambda.tsbackend/cosmetology-app/lambdas/nodejs/lib/email/encumbrance-notification-service.tsbackend/cosmetology-app/lambdas/nodejs/lib/email/investigation-notification-service.tsbackend/cosmetology-app/lambdas/nodejs/package.jsonbackend/cosmetology-app/lambdas/nodejs/tsconfig.json
💤 Files with no reviewable changes (2)
- backend/cosmetology-app/lambdas/nodejs/cognito-emails/lambda.js
- backend/cosmetology-app/lambdas/nodejs/cognito-emails/handler.js
✅ Files skipped from review due to trivial changes (5)
- backend/cosmetology-app/lambdas/nodejs/cognito-emails/lambda.ts
- backend/cosmetology-app/lambdas/nodejs/lib/email/investigation-notification-service.ts
- backend/cosmetology-app/lambdas/nodejs/ingest-event-reporter/lambda.ts
- backend/cosmetology-app/lambdas/nodejs/lib/email/encumbrance-notification-service.ts
- backend/cosmetology-app/lambdas/nodejs/email-notification-service/lambda.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/cosmetology-app/lambdas/nodejs/package.json
Contributor
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
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/docs/devops/STAFF_USER_MFA_RECOVERY.md (1)
44-53:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUpdate the procedure description to match the actual steps.
Line 44 still describes the old procedure where changing the
pkvalue causes DynamoDB to delete and recreate the record. However, the actual steps below (lines 48-53) no longer modify thepk—they addarchivedDateandarchivedReasonfields and removefamGivinstead. This inconsistency could confuse operators following the procedure.📝 Proposed fix to align the description with the steps
-In the DynamoDB console, if you change the partition key (pk) value of a existing DynamoDB item, it automatically deletes the old record with the old pk and creates a new one with the new pk, effectively archiving the user record. Perform the following steps: +Archive the existing staff user record by marking it with archive metadata and removing it from the active user index. Perform the following steps:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/compact-connect/docs/devops/STAFF_USER_MFA_RECOVERY.md` around lines 44 - 53, The opening paragraph incorrectly states that changing the partition key (pk) will archive the record; update the description to reflect the actual procedure: do not modify pk, instead in the DynamoDB console open the staff user item, add fields archivedDate and archivedReason, remove the famGiv field, save to archive the user, then verify and note permissions. Make sure to reference archivedDate, archivedReason, famGiv and pk in the text so readers understand that pk is unchanged and archiving is performed by adding/removing fields.
🧹 Nitpick comments (1)
backend/compact-connect/docs/devops/STAFF_USER_MFA_RECOVERY.md (1)
50-50: 💤 Low valueConsider fixing grammar and improving clarity.
"lookup" should be "look up" (verb phrase). The parenthetical explanation is also quite long and could be reformatted for better readability.
✍️ Proposed grammar and clarity improvements
- - **Remove field**: `famGiv`. If this field is not removed from the record, the user will continue to show up in the list of staff users in the UI, since we use this field in the index used to lookup the list of staff users (the first and last name of the staff user are still recorded under the `attributes` field, so no data is lost by removing the `famGiv` index field). + - **Remove field**: `famGiv`. If this field is not removed from the record, the user will continue to show up in the list of staff users in the UI, since we use this field in the index used to look up the list of staff users. (Note: The first and last name of the staff user are still recorded under the `attributes` field, so no data is lost by removing the `famGiv` index field.)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/compact-connect/docs/devops/STAFF_USER_MFA_RECOVERY.md` at line 50, Edit the sentence describing the `famGiv` field to correct grammar and improve clarity: change "lookup" to "look up", remove the long parenthetical and split into two sentences — first state that `famGiv` should be removed to prevent the user appearing in the staff list, then add a second sentence clarifying that the first/last name are preserved under `attributes` so no data is lost. Keep references to the `famGiv` index field and the `attributes` field verbatim for clarity.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/check-compact-connect.yml:
- Around line 144-146: The CI workflow line that suppresses CVE-2026-41066
("pip-audit --ignore-vuln GHSA-4xh5-x5gv-qwph --ignore-vuln CVE-2026-41066") is
masking a real lxml XXE vulnerability but authorizenet 1.1.6 pins lxml==4.* so
you cannot upgrade in-tree; update the workflow comment and suppression
rationale to record that authorizenet 1.1.6's install_requires pins lxml==4.*,
that lxml 6.1.0+ fixes CVE-2026-41066, and link the upstream tracker (issue
`#164`, PR `#167`) so future maintainers know this is an upstream blocker and the
acceptable temporary suppression is intentional until authorizenet is updated or
replaced.
In `@backend/compact-connect/lambdas/nodejs/package.json`:
- Line 23: The package.json currently pins "@types/node": "25.6.0" which is
mismatched with the Lambda Node.js 22.x runtime; update the dependency to the
project’s chosen Node-22-compatible `@types` version (replace "25.6.0" with the
same `@types/node` version used in the other lambda package.json that targets Node
22.x) so the TypeScript compiler cannot accidentally allow Node-25-only APIs.
In `@backend/cosmetology-app/lambdas/nodejs/package.json`:
- Line 23: The `@types/node` entry in package.json is set to 25.6.0 which targets
Node 25 and is incompatible with the Lambda runtime (NODEJS_24_X); update the
package.json dependency "@types/node" to a Node-24-compatible types release (use
a stable 20.x release such as a latest 20.*) and run your package manager to
reinstall devDependencies, then scan for and remove any Node-25-specific APIs
referenced in the code (search for usages in the lambda handlers and utility
functions) to ensure runtime compatibility.
---
Outside diff comments:
In `@backend/compact-connect/docs/devops/STAFF_USER_MFA_RECOVERY.md`:
- Around line 44-53: The opening paragraph incorrectly states that changing the
partition key (pk) will archive the record; update the description to reflect
the actual procedure: do not modify pk, instead in the DynamoDB console open the
staff user item, add fields archivedDate and archivedReason, remove the famGiv
field, save to archive the user, then verify and note permissions. Make sure to
reference archivedDate, archivedReason, famGiv and pk in the text so readers
understand that pk is unchanged and archiving is performed by adding/removing
fields.
---
Nitpick comments:
In `@backend/compact-connect/docs/devops/STAFF_USER_MFA_RECOVERY.md`:
- Line 50: Edit the sentence describing the `famGiv` field to correct grammar
and improve clarity: change "lookup" to "look up", remove the long parenthetical
and split into two sentences — first state that `famGiv` should be removed to
prevent the user appearing in the staff list, then add a second sentence
clarifying that the first/last name are preserved under `attributes` so no data
is lost. Keep references to the `famGiv` index field and the `attributes` field
verbatim for clarity.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d9776193-2e04-4dd1-93f9-d05ca37c9377
⛔ Files ignored due to path filters (5)
backend/compact-connect-ui-app/lambdas/nodejs/yarn.lockis excluded by!**/yarn.lock,!**/*.lockbackend/compact-connect/lambdas/nodejs/yarn.lockis excluded by!**/yarn.lock,!**/*.lockbackend/cosmetology-app/lambdas/nodejs/cognito-emails/handler.js.mapis excluded by!**/*.mapbackend/cosmetology-app/lambdas/nodejs/cognito-emails/lambda.js.mapis excluded by!**/*.mapbackend/cosmetology-app/lambdas/nodejs/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (34)
.github/workflows/check-common-cdk.yml.github/workflows/check-compact-connect-ui-app.yml.github/workflows/check-compact-connect.yml.github/workflows/check-cosmetology-app.yml.github/workflows/check-multi-account.ymlbackend/common-cdk/requirements.inbackend/compact-connect-ui-app/lambdas/nodejs/package.jsonbackend/compact-connect-ui-app/requirements.inbackend/compact-connect-ui-app/requirements.txtbackend/compact-connect/docs/devops/STAFF_USER_MFA_RECOVERY.mdbackend/compact-connect/lambdas/nodejs/cognito-emails/lambda.tsbackend/compact-connect/lambdas/nodejs/email-notification-service/lambda.tsbackend/compact-connect/lambdas/nodejs/ingest-event-reporter/lambda.tsbackend/compact-connect/lambdas/nodejs/lib/email/encumbrance-notification-service.tsbackend/compact-connect/lambdas/nodejs/lib/email/investigation-notification-service.tsbackend/compact-connect/lambdas/nodejs/package.jsonbackend/compact-connect/lambdas/nodejs/tests/ingest-event-reporter.test.tsbackend/compact-connect/lambdas/nodejs/tsconfig.jsonbackend/compact-connect/requirements.inbackend/cosmetology-app/docs/internal/api-specification/latest-oas30.jsonbackend/cosmetology-app/docs/internal/postman/postman-collection.jsonbackend/cosmetology-app/lambdas/nodejs/cognito-emails/handler.jsbackend/cosmetology-app/lambdas/nodejs/cognito-emails/lambda.jsbackend/cosmetology-app/lambdas/nodejs/cognito-emails/lambda.tsbackend/cosmetology-app/lambdas/nodejs/email-notification-service/lambda.tsbackend/cosmetology-app/lambdas/nodejs/ingest-event-reporter/lambda.tsbackend/cosmetology-app/lambdas/nodejs/lib/email/encumbrance-notification-service.tsbackend/cosmetology-app/lambdas/nodejs/lib/email/investigation-notification-service.tsbackend/cosmetology-app/lambdas/nodejs/package.jsonbackend/cosmetology-app/lambdas/nodejs/tsconfig.jsonbackend/cosmetology-app/requirements.inbackend/multi-account/backups/requirements.inbackend/multi-account/control-tower/requirements.inbackend/multi-account/log-aggregation/requirements.in
💤 Files with no reviewable changes (2)
- backend/cosmetology-app/lambdas/nodejs/cognito-emails/lambda.js
- backend/cosmetology-app/lambdas/nodejs/cognito-emails/handler.js
✅ Files skipped from review due to trivial changes (15)
- backend/compact-connect/lambdas/nodejs/cognito-emails/lambda.ts
- backend/compact-connect/lambdas/nodejs/lib/email/investigation-notification-service.ts
- backend/multi-account/control-tower/requirements.in
- backend/multi-account/log-aggregation/requirements.in
- backend/compact-connect-ui-app/requirements.txt
- backend/multi-account/backups/requirements.in
- backend/cosmetology-app/lambdas/nodejs/cognito-emails/lambda.ts
- backend/cosmetology-app/lambdas/nodejs/lib/email/investigation-notification-service.ts
- backend/cosmetology-app/lambdas/nodejs/email-notification-service/lambda.ts
- backend/compact-connect/lambdas/nodejs/lib/email/encumbrance-notification-service.ts
- backend/cosmetology-app/lambdas/nodejs/ingest-event-reporter/lambda.ts
- backend/compact-connect/lambdas/nodejs/email-notification-service/lambda.ts
- backend/cosmetology-app/lambdas/nodejs/tsconfig.json
- backend/common-cdk/requirements.in
- backend/cosmetology-app/lambdas/nodejs/lib/email/encumbrance-notification-service.ts
jlkravitz
reviewed
May 4, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/compact-connect/docs/devops/STAFF_USER_MFA_RECOVERY.md (1)
44-44:⚠️ Potential issue | 🟠 Major | ⚡ Quick winStep 2 intro conflicts with the actual archive procedure.
Line 44 still says archiving happens by changing
pk, but Lines 47-53 now archive via in-place field edits/removingfamGiv. This contradiction can cause incorrect manual actions. Please align the intro with the new workflow.Suggested doc fix
-In the DynamoDB console, if you change the partition key (pk) value of a existing DynamoDB item, it automatically deletes the old record with the old pk and creates a new one with the new pk, effectively archiving the user record. Perform the following steps: +In the DynamoDB console, archive the existing staff-user record in place by adding archive metadata and removing the `famGiv` index field so it no longer appears in active staff-user UI listings. Perform the following steps:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/compact-connect/docs/devops/STAFF_USER_MFA_RECOVERY.md` at line 44, Update the Step 2 intro to match the current archive procedure: change the statement that archiving happens by changing the partition key (`pk`) to explain that archiving is performed via in-place field edits (specifically removing or clearing the `famGiv` field) in DynamoDB rather than changing the `pk`; ensure the Step 2 narrative and any preceding/preceding references consistently describe the new workflow so the lines describing the actual edits (the steps that modify/remove `famGiv`) and the intro no longer contradict each other.
🧹 Nitpick comments (1)
backend/compact-connect/lambdas/nodejs/tests/ingest-event-reporter.test.ts (1)
186-186: ⚡ Quick winRedundant local
mockS3Clientshadows the newly-initialized outer mock — consider consolidating.Tests 2–4 in "Frequent runs" each redeclare
const mockS3Client = mockClient(S3Client), shadowing the outerlet mockS3ClientthatbeforeAllnow properly initializes. This means the suite accumulates multiplemockClient(S3Client)intercept registrations per test run. BecauseIngestEventEmailServiceis fully mocked and S3 is never actually called, this is harmless today, but it adds noise and creates ordering sensitivity if that mock boundary ever changes.The simpler approach: remove the local redeclarations in tests 2–4 so they reuse the outer
mockS3Client, and add amockS3Client.reset()call inbeforeEachto ensure clean state between tests (mirroring how "Weekly runs" reinitializesmockSESClientperbeforeEach).♻️ Suggested consolidation
beforeEach(() => { // Clear all instances and calls to constructor and all methods: jest.clearAllMocks(); + mockS3Client.reset(); mockSendReportEmail.mockClear(); mockSendAllsWellEmail.mockClear(); });it('should not send an email if there were no ingest events', async () => { const mockDynamoDBClient = mockClient(DynamoDBClient); - const mockS3Client = mockClient(S3Client); // ...it('should let DynamoDB errors escape', async () => { const mockDynamoDBClient = mockClient(DynamoDBClient); - const mockS3Client = mockClient(S3Client); // ...it('should skip compact and continue processing when compact configuration is not found', async () => { const mockDynamoDBClient = mockClient(DynamoDBClient); - const mockS3Client = mockClient(S3Client); // ...Also applies to: 246-246, 271-271
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/compact-connect/lambdas/nodejs/tests/ingest-event-reporter.test.ts` at line 186, Remove the redundant local redeclarations of mockS3Client inside the "Frequent runs" tests so they reuse the outer let mockS3Client initialized in beforeAll; instead add mockS3Client.reset() in the beforeEach that runs before those tests (mirroring how mockSESClient is reset for "Weekly runs") to clear handlers between tests, and apply the same removal of local redeclarations at the other occurrences that shadow the outer mock (the other test blocks where const mockS3Client = mockClient(S3Client) appears).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/compact-connect/docs/devops/STAFF_USER_MFA_RECOVERY.md`:
- Line 50: Change the verb "lookup" to the correct two-word verb "look up" in
the sentence describing the index removal; update the line that reads "we use
this field in the index used to lookup the list of staff users" so it reads "we
use this field in the index used to look up the list of staff users" (refer to
the "Remove field: `famGiv`" sentence and the token `famGiv` to locate the exact
phrase).
In
`@backend/cosmetology-app/lambdas/python/common/tests/unit/test_provider_record_util.py`:
- Around line 598-642: Replace the order-sensitive equality check in the test
method
test_generate_api_response_object_returns_adverse_actions_as_a_top_level_field_for_all_adverse_actions
with an unordered comparison: change the final assertion that compares
api_response['adverseActions'] to use unittest's assertCountEqual instead of
assertEqual so the presence of ProviderUserRecords.generate_api_response_object
returned adverse actions (license_adverse_action.to_dict() and
privilege_adverse_action.to_dict()) is validated regardless of order.
---
Outside diff comments:
In `@backend/compact-connect/docs/devops/STAFF_USER_MFA_RECOVERY.md`:
- Line 44: Update the Step 2 intro to match the current archive procedure:
change the statement that archiving happens by changing the partition key (`pk`)
to explain that archiving is performed via in-place field edits (specifically
removing or clearing the `famGiv` field) in DynamoDB rather than changing the
`pk`; ensure the Step 2 narrative and any preceding/preceding references
consistently describe the new workflow so the lines describing the actual edits
(the steps that modify/remove `famGiv`) and the intro no longer contradict each
other.
---
Nitpick comments:
In `@backend/compact-connect/lambdas/nodejs/tests/ingest-event-reporter.test.ts`:
- Line 186: Remove the redundant local redeclarations of mockS3Client inside the
"Frequent runs" tests so they reuse the outer let mockS3Client initialized in
beforeAll; instead add mockS3Client.reset() in the beforeEach that runs before
those tests (mirroring how mockSESClient is reset for "Weekly runs") to clear
handlers between tests, and apply the same removal of local redeclarations at
the other occurrences that shadow the outer mock (the other test blocks where
const mockS3Client = mockClient(S3Client) appears).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8c6ab871-2d0b-4264-adb6-af9ef48093a5
⛔ Files ignored due to path filters (5)
backend/compact-connect-ui-app/lambdas/nodejs/yarn.lockis excluded by!**/yarn.lock,!**/*.lockbackend/compact-connect/lambdas/nodejs/yarn.lockis excluded by!**/yarn.lock,!**/*.lockbackend/cosmetology-app/lambdas/nodejs/cognito-emails/handler.js.mapis excluded by!**/*.mapbackend/cosmetology-app/lambdas/nodejs/cognito-emails/lambda.js.mapis excluded by!**/*.mapbackend/cosmetology-app/lambdas/nodejs/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (35)
.github/workflows/check-common-cdk.yml.github/workflows/check-compact-connect-ui-app.yml.github/workflows/check-compact-connect.yml.github/workflows/check-cosmetology-app.yml.github/workflows/check-multi-account.ymlbackend/common-cdk/requirements.inbackend/compact-connect-ui-app/lambdas/nodejs/package.jsonbackend/compact-connect-ui-app/requirements.inbackend/compact-connect-ui-app/requirements.txtbackend/compact-connect/docs/devops/STAFF_USER_MFA_RECOVERY.mdbackend/compact-connect/lambdas/nodejs/cognito-emails/lambda.tsbackend/compact-connect/lambdas/nodejs/email-notification-service/lambda.tsbackend/compact-connect/lambdas/nodejs/ingest-event-reporter/lambda.tsbackend/compact-connect/lambdas/nodejs/lib/email/encumbrance-notification-service.tsbackend/compact-connect/lambdas/nodejs/lib/email/investigation-notification-service.tsbackend/compact-connect/lambdas/nodejs/package.jsonbackend/compact-connect/lambdas/nodejs/tests/ingest-event-reporter.test.tsbackend/compact-connect/lambdas/nodejs/tsconfig.jsonbackend/compact-connect/requirements.inbackend/cosmetology-app/docs/internal/api-specification/latest-oas30.jsonbackend/cosmetology-app/docs/internal/postman/postman-collection.jsonbackend/cosmetology-app/lambdas/nodejs/cognito-emails/handler.jsbackend/cosmetology-app/lambdas/nodejs/cognito-emails/lambda.jsbackend/cosmetology-app/lambdas/nodejs/cognito-emails/lambda.tsbackend/cosmetology-app/lambdas/nodejs/email-notification-service/lambda.tsbackend/cosmetology-app/lambdas/nodejs/ingest-event-reporter/lambda.tsbackend/cosmetology-app/lambdas/nodejs/lib/email/encumbrance-notification-service.tsbackend/cosmetology-app/lambdas/nodejs/lib/email/investigation-notification-service.tsbackend/cosmetology-app/lambdas/nodejs/package.jsonbackend/cosmetology-app/lambdas/nodejs/tsconfig.jsonbackend/cosmetology-app/lambdas/python/common/tests/unit/test_provider_record_util.pybackend/cosmetology-app/requirements.inbackend/multi-account/backups/requirements.inbackend/multi-account/control-tower/requirements.inbackend/multi-account/log-aggregation/requirements.in
💤 Files with no reviewable changes (2)
- backend/cosmetology-app/lambdas/nodejs/cognito-emails/handler.js
- backend/cosmetology-app/lambdas/nodejs/cognito-emails/lambda.js
✅ Files skipped from review due to trivial changes (21)
- backend/cosmetology-app/lambdas/nodejs/lib/email/encumbrance-notification-service.ts
- backend/cosmetology-app/lambdas/nodejs/cognito-emails/lambda.ts
- backend/multi-account/control-tower/requirements.in
- backend/compact-connect-ui-app/lambdas/nodejs/package.json
- backend/multi-account/log-aggregation/requirements.in
- backend/compact-connect-ui-app/requirements.txt
- backend/multi-account/backups/requirements.in
- backend/compact-connect/requirements.in
- .github/workflows/check-multi-account.yml
- backend/cosmetology-app/lambdas/nodejs/package.json
- .github/workflows/check-common-cdk.yml
- backend/compact-connect/lambdas/nodejs/email-notification-service/lambda.ts
- backend/compact-connect/lambdas/nodejs/lib/email/encumbrance-notification-service.ts
- backend/cosmetology-app/lambdas/nodejs/lib/email/investigation-notification-service.ts
- backend/cosmetology-app/lambdas/nodejs/ingest-event-reporter/lambda.ts
- backend/cosmetology-app/lambdas/nodejs/email-notification-service/lambda.ts
- backend/compact-connect/lambdas/nodejs/lib/email/investigation-notification-service.ts
- backend/compact-connect/lambdas/nodejs/ingest-event-reporter/lambda.ts
- .github/workflows/check-compact-connect-ui-app.yml
- .github/workflows/check-compact-connect.yml
- backend/compact-connect/lambdas/nodejs/cognito-emails/lambda.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- .github/workflows/check-cosmetology-app.yml
- backend/compact-connect-ui-app/requirements.in
- backend/common-cdk/requirements.in
- backend/compact-connect/lambdas/nodejs/package.json
- backend/cosmetology-app/lambdas/nodejs/tsconfig.json
- backend/cosmetology-app/requirements.in
landonshumway-ia
force-pushed
the
feat/disciplinary-table-schema
branch
from
3467943 to
408e60b
Compare
jlkravitz
approved these changes
May 4, 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
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.
Cosmetology would like to display a table of disciplinary information on the provider detail screen, separate from the privilege and license cards. To support this, we are adding a top-level 'adverseActions' field to the provider details API response object, which will include the full list of adverse actions associated with that provider.
We still keep the nested adverseActions fields under the licenses and privileges objects, which list the adverse actions associated with the specific license/privilege, this keeps us backwards compatible with how the license and privilege cards are accessing the data.
Testing List
yarn test:unit:allshould run without errors or warningsyarn serveshould run without errors or warningsyarn buildshould run without errors or warningsbackend/compact-connect/tests/unit/test_api.pyrun compact-connect/bin/download_oas30.pyCloses #1478
Summary by CodeRabbit
New Features
Tests
Chores
Documentation