Add support for Cosmetology public search#1317
Add support for Cosmetology public search#1317landonshumway-ia wants to merge 28 commits intocsg-org:mainfrom
Conversation
Also added custom cursor logic for tracking accurate page size of licenses.
We are updating our OpenSearch indexing design to index one document per license, rather than per provider. This means that we can use native OpenSearch pagination and return the exact number of results the client requests if there are enough matches found. It also reduces the complexity of only returning licenses that match, rather than having to map inner hits.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new unauthenticated public license search Lambda and tests; surfaces license objects (including licenseNumber) in public provider responses and request schemas; extends OpenSearch client with index/alias helpers and provider index mapping; adds index reset options to populate flow; rethreads stacks so public query is served from the search persistent stack. Changes
Sequence DiagramsequenceDiagram
participant Client as API Consumer
participant APIGW as API Gateway
participant Handler as public_search_api_handler
participant Validator as PublicQueryProvidersRequestSchema
participant OpenSearch as OpenSearch Client
participant Mapper as PublicLicenseSearchResponseSchema
Client->>APIGW: POST /v1/public/compacts/{compact}/providers/query
APIGW->>Handler: invoke(event)
Handler->>Validator: validate body
alt validation error
Validator-->>Handler: ValidationError
Handler-->>APIGW: 400 Bad Request
else valid
Handler->>Handler: build nested licenses query, sort, pagination
Handler->>OpenSearch: search(index, body, size, search_after)
OpenSearch-->>Handler: hits with _source & sort
Handler->>Handler: filter by compact, sanitize each license
Handler->>Mapper: validate license object
Mapper-->>Handler: sanitized provider/license
Handler-->>APIGW: 200 OK (providers, pagination, sorting)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (1 warning, 2 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
backend/cosmetology-app/stacks/api_stack/v1_api/api_model.py (1)
113-118: InconsistentlicenseNumbermax length between request and response schemas.The request model allows
licenseNumberup to 500 characters (line 116), but the response model'squery.licenseNumberecho (line 1349) and_public_license_search_response_schema(line 1664) both specify max length of 100. If a client sends a license number between 101-500 characters, the echoed query in the response would exceed the response schema's constraint.Consider aligning these values for consistency:
🔧 Suggested alignment
'licenseNumber': JsonSchema( type=JsonSchemaType.STRING, min_length=1, - max_length=500, + max_length=100, description='Filter for licenses with a specific license number', ),🤖 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 113 - 118, The request schema for 'licenseNumber' allows up to 500 chars but the response schemas (the echoed query under 'query.licenseNumber' and the '_public_license_search_response_schema') are capped at 100, causing an inconsistent contract; update the response-side JsonSchema definitions for 'licenseNumber' to use the same max_length as the request (or vice versa if you prefer the smaller limit) so both request and response use the identical max_length value, adjusting the JsonSchema() instances that reference 'licenseNumber' (including the query echo and _public_license_search_response_schema) to the chosen consistent max_length and keep the type/description intact.backend/cosmetology-app/lambdas/python/search/handlers/public_search.py (1)
15-17: Minor: Comment mentions 20 seconds but timeout is 25 seconds.📝 Fix comment
# Instantiate the OpenSearch client outside the handler to cache the connection between invocations -# Set timeout to 20 seconds to give API gateway time to respond with response +# Set timeout to 25 seconds to give API Gateway time to respond opensearch_client = OpenSearchClient(timeout=25)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/cosmetology-app/lambdas/python/search/handlers/public_search.py` around lines 15 - 17, The inline comment above the OpenSearch client instantiation is inconsistent: it says "Set timeout to 20 seconds" while the actual instantiation uses OpenSearchClient(timeout=25). Update the comment to match the code (or change the timeout value to 20 if that was intended) so the comment and the OpenSearchClient(timeout=...) call for opensearch_client are consistent.
🤖 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/python/common/cc_common/data_model/schema/provider/api.py`:
- Line 251: The schema field definition for licenseNumber in provider API schema
(the variable/field named licenseNumber in provider api.py) incorrectly uses
Length(max=100); update its validation to Length(min=1, max=500) to match the
published request schema so requests with up to 500 chars are accepted, leaving
required=False and allow_none=False as-is. Ensure you run schema validation
tests after the change.
- Around line 220-223: The schema's licenseJurisdiction field is currently
optional/untyped which allows contract-critical public-search results to lack a
valid jurisdiction; update the field definition(s) for licenseJurisdiction (and
the other occurrence referenced) to require a non-empty, validated jurisdiction
— e.g., set required=True and allow_none=False and add a validation rule
(Length(min=1) or a OneOf(enum_of_allowed_jurisdictions) if you have a canonical
list) matching the project's jurisdiction values; ensure the corresponding
deserialization/validation paths (the schema class that defines jurisdiction,
licenseJurisdiction, and compact) are adjusted so invalid or missing
jurisdictions raise validation errors for public responses.
---
Nitpick comments:
In `@backend/cosmetology-app/lambdas/python/search/handlers/public_search.py`:
- Around line 15-17: The inline comment above the OpenSearch client
instantiation is inconsistent: it says "Set timeout to 20 seconds" while the
actual instantiation uses OpenSearchClient(timeout=25). Update the comment to
match the code (or change the timeout value to 20 if that was intended) so the
comment and the OpenSearchClient(timeout=...) call for opensearch_client are
consistent.
In `@backend/cosmetology-app/stacks/api_stack/v1_api/api_model.py`:
- Around line 113-118: The request schema for 'licenseNumber' allows up to 500
chars but the response schemas (the echoed query under 'query.licenseNumber' and
the '_public_license_search_response_schema') are capped at 100, causing an
inconsistent contract; update the response-side JsonSchema definitions for
'licenseNumber' to use the same max_length as the request (or vice versa if you
prefer the smaller limit) so both request and response use the identical
max_length value, adjusting the JsonSchema() instances that reference
'licenseNumber' (including the query echo and
_public_license_search_response_schema) to the chosen consistent max_length and
keep the type/description intact.
🪄 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: b326cb5c-a0b0-4303-b278-f36d62f85b44
📒 Files selected for processing (17)
backend/cosmetology-app/bin/run_python_tests.pybackend/cosmetology-app/lambdas/python/common/cc_common/data_model/schema/provider/api.pybackend/cosmetology-app/lambdas/python/search/handlers/public_search.pybackend/cosmetology-app/lambdas/python/search/tests/function/test_public_search_providers.pybackend/cosmetology-app/pipeline/backend_stage.pybackend/cosmetology-app/stacks/api_lambda_stack/public_lookup_api.pybackend/cosmetology-app/stacks/api_stack/__init__.pybackend/cosmetology-app/stacks/api_stack/api.pybackend/cosmetology-app/stacks/api_stack/v1_api/api.pybackend/cosmetology-app/stacks/api_stack/v1_api/api_model.pybackend/cosmetology-app/stacks/api_stack/v1_api/public_lookup_api.pybackend/cosmetology-app/stacks/search_persistent_stack/search_handler.pybackend/cosmetology-app/tests/app/test_api/test_public_lookup_api.pybackend/cosmetology-app/tests/resources/snapshots/PUBLIC_QUERY_PROVIDERS_REQUEST_SCHEMA.jsonbackend/cosmetology-app/tests/resources/snapshots/PUBLIC_QUERY_PROVIDERS_RESPONSE_SCHEMA.jsonbackend/cosmetology-app/tests/resources/snapshots/QUERY_PROVIDERS_REQUEST_SCHEMA.jsonbackend/cosmetology-app/tests/smoke/rollback_license_upload_smoke_tests.py
💤 Files with no reviewable changes (1)
- backend/cosmetology-app/tests/smoke/rollback_license_upload_smoke_tests.py
backend/cosmetology-app/lambdas/python/common/cc_common/data_model/schema/provider/api.py
Outdated
Show resolved
Hide resolved
backend/cosmetology-app/lambdas/python/common/cc_common/data_model/schema/provider/api.py
Show resolved
Hide resolved
There was a problem hiding this comment.
🧹 Nitpick comments (1)
backend/cosmetology-app/lambdas/python/search/handlers/public_search.py (1)
15-17: Minor: Comment says 20 seconds but timeout is 25.The comment states "Set timeout to 20 seconds" but the actual timeout is 25 seconds. Consider updating the comment to match.
📝 Suggested fix
# Instantiate the OpenSearch client outside the handler to cache the connection between invocations -# Set timeout to 20 seconds to give API gateway time to respond with response +# Set timeout to 25 seconds to give API gateway time to respond with response opensearch_client = OpenSearchClient(timeout=25)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/cosmetology-app/lambdas/python/search/handlers/public_search.py` around lines 15 - 17, The inline comment above the OpenSearch client initialization is inconsistent: it says "Set timeout to 20 seconds" while the code instantiates OpenSearchClient(timeout=25). Update the comment to reflect the actual timeout (25 seconds) or change the timeout value to 20 to match the comment; locate the OpenSearchClient instantiation (opensearch_client = OpenSearchClient(timeout=25)) and make the comment and the timeout value consistent.
🤖 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/search/handlers/public_search.py`:
- Around line 15-17: The inline comment above the OpenSearch client
initialization is inconsistent: it says "Set timeout to 20 seconds" while the
code instantiates OpenSearchClient(timeout=25). Update the comment to reflect
the actual timeout (25 seconds) or change the timeout value to 20 to match the
comment; locate the OpenSearchClient instantiation (opensearch_client =
OpenSearchClient(timeout=25)) and make the comment and the timeout value
consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 736471cd-b7e4-460a-a6b8-5a45c274e4c3
📒 Files selected for processing (7)
backend/cosmetology-app/lambdas/python/common/cc_common/data_model/schema/provider/api.pybackend/cosmetology-app/lambdas/python/common/tests/unit/test_data_model/test_schema/test_provider.pybackend/cosmetology-app/lambdas/python/provider-data-v1/handlers/public_lookup.pybackend/cosmetology-app/lambdas/python/search/handlers/public_search.pybackend/cosmetology-app/stacks/api_stack/v1_api/api_model.pybackend/cosmetology-app/tests/resources/snapshots/PUBLIC_QUERY_PROVIDERS_REQUEST_SCHEMA.jsonbackend/cosmetology-app/tests/resources/snapshots/QUERY_PROVIDERS_REQUEST_SCHEMA.json
💤 Files with no reviewable changes (1)
- backend/cosmetology-app/lambdas/python/provider-data-v1/handlers/public_lookup.py
✅ Files skipped from review due to trivial changes (2)
- backend/cosmetology-app/tests/resources/snapshots/PUBLIC_QUERY_PROVIDERS_REQUEST_SCHEMA.json
- backend/cosmetology-app/tests/resources/snapshots/QUERY_PROVIDERS_REQUEST_SCHEMA.json
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
backend/cosmetology-app/lambdas/python/common/cc_common/data_model/schema/provider/api.py (1)
218-224:⚠️ Potential issue | 🟠 MajorValidate
licenseJurisdictionwithJurisdiction, not a bareString.Making the field required fixed the missing-value case, but this schema still accepts any string.
public_search.pyrelies on this schema’sload()to sanitize OpenSearch hits, while the CDK model already constrains the field to known jurisdictions, so malformed documents can still pass through here.Suggested fix
- licenseJurisdiction = String(required=True, allow_none=False) + licenseJurisdiction = Jurisdiction(required=True, allow_none=False)🤖 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` around lines 218 - 224, The schema currently defines licenseJurisdiction as a plain String which allows any value; change it to use the Jurisdiction field type so only known jurisdictions are accepted: import or reference the Jurisdiction field/class and replace the line defining licenseJurisdiction (the symbol licenseJurisdiction in provider/api.py) with licenseJurisdiction = Jurisdiction(required=True, allow_none=False), preserving other validators/flags; ensure the Jurisdiction symbol is imported at the top of the module if not already.
🤖 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/python/common/cc_common/data_model/schema/provider/api.py`:
- Around line 206-208: The public provider API model
V1PublicProviderResponseModel must be updated to expose the new sanitized
licenses array that ProviderPublicResponseSchema now produces; locate
V1PublicProviderResponseModel and add a licenses field matching the schema
(array of LicensePublicResponse / appropriate license DTO type,
optional/nullable as ProviderPublicResponseSchema defines) alongside the
existing privileges field, update serialization/validation and any API docs or
OpenAPI model generation for V1PublicProviderResponseModel so the public
contract reflects licenses as returned by ProviderPublicResponseSchema.
---
Duplicate comments:
In
`@backend/cosmetology-app/lambdas/python/common/cc_common/data_model/schema/provider/api.py`:
- Around line 218-224: The schema currently defines licenseJurisdiction as a
plain String which allows any value; change it to use the Jurisdiction field
type so only known jurisdictions are accepted: import or reference the
Jurisdiction field/class and replace the line defining licenseJurisdiction (the
symbol licenseJurisdiction in provider/api.py) with licenseJurisdiction =
Jurisdiction(required=True, allow_none=False), preserving other
validators/flags; ensure the Jurisdiction symbol is imported at the top of the
module if not already.
🪄 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: 9967c659-1463-4807-9109-aea5185d3080
📒 Files selected for processing (5)
backend/cosmetology-app/lambdas/python/common/cc_common/data_model/schema/provider/api.pybackend/cosmetology-app/lambdas/python/search/tests/function/test_public_search_providers.pybackend/cosmetology-app/stacks/api_lambda_stack/public_lookup_api.pybackend/cosmetology-app/stacks/api_stack/v1_api/api_model.pybackend/cosmetology-app/tests/resources/snapshots/PUBLIC_QUERY_PROVIDERS_RESPONSE_SCHEMA.json
✅ Files skipped from review due to trivial changes (1)
- backend/cosmetology-app/lambdas/python/search/tests/function/test_public_search_providers.py
🚧 Files skipped from review as they are similar to previous changes (2)
- backend/cosmetology-app/stacks/api_lambda_stack/public_lookup_api.py
- backend/cosmetology-app/tests/resources/snapshots/PUBLIC_QUERY_PROVIDERS_RESPONSE_SCHEMA.json
backend/cosmetology-app/lambdas/python/common/cc_common/data_model/schema/provider/api.py
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
backend/cosmetology-app/docs/postman/postman-collection.json (1)
427-427: Consider using valid placeholder dates in sample data.The sample request body contains invalid calendar dates (e.g.,
"2047-09-31"- September has 30 days,"1822-04-31"- April has 30 days,"2511-11-31"- November has 30 days). While these are clearly auto-generated placeholders that testers will replace, using valid dates like"2000-01-15"would improve documentation quality and make the examples more realistic for API consumers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/cosmetology-app/docs/postman/postman-collection.json` at line 427, The sample JSON request body contains invalid dates (e.g., in the "raw" payload for fields dateOfBirth, dateOfExpiration, dateOfIssuance, dateOfRenewal) such as "2047-09-31" and "1822-04-31"; update those placeholder values to valid ISO dates (for example use "2000-01-15" for dateOfBirth, "2025-12-31" for dateOfExpiration, "1990-06-20" for dateOfIssuance, and "2024-11-30" for dateOfRenewal) so the example in postman-collection.json is realistic and parsable.backend/cosmetology-app/lambdas/python/common/cc_common/data_model/schema/provider/api.py (1)
212-224: Consider using theJurisdictionfield type for stronger validation.
licenseJurisdictionis defined asString(required=True)which accepts any non-empty string. Since this is a contract-critical field for public search results, using theJurisdictionfield type (already imported) would provide enum validation against allowed jurisdiction values, matching the API Gateway model's enum constraint.Proposed enhancement
class PublicLicenseSearchResponseSchema(ForgivingSchema): """ License object fields returned by the public query providers endpoint (OpenSearch-backed). Jurisdiction is renamed to licenseJurisdiction for parity with JCC implementation. """ providerId = Raw(required=True, allow_none=False) givenName = String(required=True, allow_none=False, validate=Length(1, 100)) familyName = String(required=True, allow_none=False, validate=Length(1, 100)) - licenseJurisdiction = String(required=True, allow_none=False) + licenseJurisdiction = Jurisdiction(required=True, allow_none=False) compact = Compact(required=True, allow_none=False) licenseType = String(required=True, allow_none=False) licenseNumber = String(required=True, allow_none=False, validate=Length(1, 100))🤖 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` around lines 212 - 224, PublicLicenseSearchResponseSchema currently defines licenseJurisdiction as a plain String which permits any value; change it to use the existing Jurisdiction field type to enforce enum validation. In the class PublicLicenseSearchResponseSchema replace the licenseJurisdiction = String(...) declaration with licenseJurisdiction = Jurisdiction(required=True, allow_none=False) so the schema uses the imported Jurisdiction field and matches the API Gateway enum contract. Ensure required and allow_none flags mirror other fields for consistency.
🤖 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/docs/search-internal/api-specification/latest-oas30.json`:
- Around line 843-867: The OpenAPI schema currently allows requests with
"search_after" but no "sort"; update the API model in the CDK/CloudFormation
source (not the generated latest-oas30.json) to express the dependency that when
"search_after" is present "sort" is required—for example add a conditional
schema (oneOf / required-set or a JSON Schema dependency) around the request
object that enforces either (a) no "search_after" or (b) both "search_after" and
"sort" present; target the schema that defines the properties "search_after" and
"sort" so the generator will produce the updated latest-oas30.json, then
regenerate the API spec.
In
`@backend/cosmetology-app/docs/search-internal/postman/postman-collection.json`:
- Line 468: The example payload uses string placeholders for numeric/array
pagination fields—replace quoted strings with proper JSON types: change
"total.value" to a numeric (integer) not a quoted string, change "size", "from",
and any "search_after" to numbers/arrays as appropriate, and change "lastSort"
from a quoted string to an actual JSON array (e.g., [] or array of values).
Update the request body in the Postman collection (the JSON "body" value
containing lastSort/total.value/size/from/search_after) so those fields are
unquoted and match their expected types.
---
Nitpick comments:
In `@backend/cosmetology-app/docs/postman/postman-collection.json`:
- Line 427: The sample JSON request body contains invalid dates (e.g., in the
"raw" payload for fields dateOfBirth, dateOfExpiration, dateOfIssuance,
dateOfRenewal) such as "2047-09-31" and "1822-04-31"; update those placeholder
values to valid ISO dates (for example use "2000-01-15" for dateOfBirth,
"2025-12-31" for dateOfExpiration, "1990-06-20" for dateOfIssuance, and
"2024-11-30" for dateOfRenewal) so the example in postman-collection.json is
realistic and parsable.
In
`@backend/cosmetology-app/lambdas/python/common/cc_common/data_model/schema/provider/api.py`:
- Around line 212-224: PublicLicenseSearchResponseSchema currently defines
licenseJurisdiction as a plain String which permits any value; change it to use
the existing Jurisdiction field type to enforce enum validation. In the class
PublicLicenseSearchResponseSchema replace the licenseJurisdiction = String(...)
declaration with licenseJurisdiction = Jurisdiction(required=True,
allow_none=False) so the schema uses the imported Jurisdiction field and matches
the API Gateway enum contract. Ensure required and allow_none flags mirror other
fields for consistency.
🪄 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: b3bee02f-fc05-4b44-8505-bcd5e52f01c3
📒 Files selected for processing (11)
backend/cosmetology-app/docs/api-specification/latest-oas30.jsonbackend/cosmetology-app/docs/internal/api-specification/latest-oas30.jsonbackend/cosmetology-app/docs/internal/postman/postman-collection.jsonbackend/cosmetology-app/docs/postman/postman-collection.jsonbackend/cosmetology-app/docs/search-internal/api-specification/latest-oas30.jsonbackend/cosmetology-app/docs/search-internal/postman/postman-collection.jsonbackend/cosmetology-app/lambdas/python/common/cc_common/data_model/schema/provider/api.pybackend/cosmetology-app/lambdas/python/search/handlers/public_search.pybackend/cosmetology-app/lambdas/python/search/tests/function/test_public_search_providers.pybackend/cosmetology-app/stacks/api_stack/v1_api/api_model.pybackend/cosmetology-app/tests/resources/snapshots/PUBLIC_GET_PROVIDER_RESPONSE_SCHEMA.json
✅ Files skipped from review due to trivial changes (1)
- backend/cosmetology-app/lambdas/python/search/tests/function/test_public_search_providers.py
backend/cosmetology-app/docs/search-internal/api-specification/latest-oas30.json
Show resolved
Hide resolved
backend/cosmetology-app/docs/search-internal/postman/postman-collection.json
Show resolved
Hide resolved
38c51bb to
2cd2840
Compare
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
backend/cosmetology-app/stacks/search_api_stack/v1_api/api_model.py (1)
359-362: Keep response category items unconstrainedOn Line 359, constraining response values with an enum makes the response model brittle; enum validation belongs on request schemas, not response schemas.
Suggested change
'clinicalPrivilegeActionCategories': JsonSchema( type=JsonSchemaType.ARRAY, items=JsonSchema( - type=JsonSchemaType.STRING, - enum=['Fraud', 'Consumer Harm', 'Other'], + type=JsonSchemaType.STRING, ), ),Based on learnings: In this codebase, request schemas should enforce enums, while response schemas should remain unconstrained for flexibility.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/cosmetology-app/stacks/search_api_stack/v1_api/api_model.py` around lines 359 - 362, The response schema currently constrains the array items using items=JsonSchema(type=JsonSchemaType.STRING, enum=['Fraud', 'Consumer Harm', 'Other']) which makes responses brittle; remove the enum from the response model (leave items as an unconstrained string JsonSchema or remove the items enum field entirely) so that enum validation remains only in request schemas; update the code around the JsonSchema/item definition referenced by items and JsonSchemaType.STRING to drop the enum constraint.
🤖 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/docs/postman/postman-collection.json`:
- Line 691: The bulk-upload example in the Postman collection is inconsistent
with the later "Upload Document" request: update the example JSON under "body"
so upload.fields uses the same keys consumed by the uploader (docField-key,
docField-policy, docField-x-amz-*), or alternatively update the "Upload
Document" request to read the arbitrary example keys (ut_9c, non_8f); locate and
change the upload example (the "body" JSON containing upload.fields) and the
"Upload Document" request variables/parsing logic so both use the identical
contract (match names like upload.fields →
docField-key/docField-policy/docField-x-amz-* and ensure the postman variables
in the Upload Document request refer to those same field names).
- Line 427: The Postman example JSON uses impossible dates (e.g., "2047-09-31",
"1822-04-31", "2511-11-31") for the license fields "dateOfBirth",
"dateOfExpiration", "dateOfIssuance", and "dateOfRenewal", causing schema parse
errors; update each invalid date occurrence (the shown block and the other two
copies referenced) to real calendar dates (for example "2047-09-30",
"1822-04-30", "2511-11-30" or other valid YYYY-MM-DD values) so the POST
examples pass validation, and ensure you replace the same invalid strings
wherever they appear in the collection.
In
`@backend/cosmetology-app/docs/search-internal/api-specification/latest-oas30.json`:
- Around line 857-860: The generated OAS lost the "maximum" constraint for the
pagination "from" property causing contract drift with
SearchProvidersRequestSchema which enforces a 9900 cap; update the source API
model/schema used by the generation pipeline (the SearchProvidersRequestSchema
definition) to include "maximum": 9900 for the "from" integer property, then
re-run the API Gateway/OpenAPI generation to produce a new latest-oas30.json
instead of editing
backend/cosmetology-app/docs/search-internal/api-specification/latest-oas30.json
directly.
In
`@backend/cosmetology-app/lambdas/python/search/handlers/populate_provider_documents.py`:
- Around line 95-103: The code always recreates the concrete index as ..._v1
which can roll an alias backwards; change the logic so you first resolve the
alias's currently active concrete index (use the opensearch_client method that
returns the index name for an alias or add one), parse its version suffix (e.g.
_vN) and compute the next concrete index name (or use the existing highest
version if you intend to preserve it) instead of hardcoding
INITIAL_INDEX_VERSION, then create the new concrete index with that computed
name and only delete or swap indices via the alias after the new index exists;
update the block that uses alias_name, index_name, INITIAL_INDEX_VERSION and the
calls to delete_provider_index_with_alias/create_provider_index_with_alias
accordingly.
- Around line 84-108: The handler currently allows resetIndexes together with
resumption params, which can leave earlier compacts unpopulated; add a guard
after reading reset_indexes, starting_compact, and starting_last_key that fails
fast (raise an exception or return a 400-style error) if reset_indexes is True
and either starting_compact or starting_last_key is provided. Update the logic
around the reset branch (referencing reset_indexes, starting_compact,
starting_last_key, config.compacts, INITIAL_INDEX_VERSION, and
opensearch_client.delete_provider_index_with_alias /
create_provider_index_with_alias) so you check and abort before
deleting/recreating indexes to prevent partial repopulation.
In `@backend/cosmetology-app/lambdas/python/search/opensearch_client.py`:
- Around line 140-172: The create_provider_index_with_alias function uses
check-then-act with alias_exists/index_exists then create_index/create_alias and
can race; wrap the create_index and create_alias calls in try/except that
catches OpenSearch RequestError (or the client’s equivalent) and treat "resource
already exists" (status 400 or the specific error code/message) as a benign
outcome: call create_index and create_alias inside a single try block (or
separate try blocks), on RequestError inspect the status/code/message and log
that the index/alias already exists and return/continue, re-raise for other
errors; keep references to create_provider_index_with_alias, alias_exists,
index_exists, create_index, and create_alias to locate and change the code.
In `@backend/cosmetology-app/stacks/api_stack/v1_api/api_model.py`:
- Around line 816-843: The CDK model currently hard-codes the category enum in
_clinical_privilege_action_categories_schema and permits empty arrays, creating
a second source of truth and a looser contract than the runtime
AdverseActionPostRequestSchema; update
_clinical_privilege_action_categories_schema to remove the enum (do not
duplicate allowed values) and instead return an ARRAY of STRINGs with minItems=1
(use JsonSchemaType.ARRAY, items type JsonSchemaType.STRING, and set minItems=1)
so the API model requires a non-empty list but leaves permitted values
enforcement to AdverseActionPostRequestSchema; adjust
_encumbrance_request_schema as needed to reference the updated
_clinical_privilege_action_categories_schema (keep date format
cc_api.YMD_FORMAT).
---
Nitpick comments:
In `@backend/cosmetology-app/stacks/search_api_stack/v1_api/api_model.py`:
- Around line 359-362: The response schema currently constrains the array items
using items=JsonSchema(type=JsonSchemaType.STRING, enum=['Fraud', 'Consumer
Harm', 'Other']) which makes responses brittle; remove the enum from the
response model (leave items as an unconstrained string JsonSchema or remove the
items enum field entirely) so that enum validation remains only in request
schemas; update the code around the JsonSchema/item definition referenced by
items and JsonSchemaType.STRING to drop the enum constraint.
🪄 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: 38c31be0-4dc8-4615-85ec-fd0b7530a3ec
📒 Files selected for processing (37)
backend/cosmetology-app/docs/api-specification/latest-oas30.jsonbackend/cosmetology-app/docs/internal/api-specification/latest-oas30.jsonbackend/cosmetology-app/docs/internal/postman/postman-collection.jsonbackend/cosmetology-app/docs/postman/postman-collection.jsonbackend/cosmetology-app/docs/search-internal/api-specification/latest-oas30.jsonbackend/cosmetology-app/docs/search-internal/postman/postman-collection.jsonbackend/cosmetology-app/lambdas/python/common/cc_common/data_model/schema/adverse_action/__init__.pybackend/cosmetology-app/lambdas/python/common/cc_common/data_model/schema/adverse_action/api.pybackend/cosmetology-app/lambdas/python/common/cc_common/data_model/schema/adverse_action/record.pybackend/cosmetology-app/lambdas/python/common/cc_common/data_model/schema/common.pybackend/cosmetology-app/lambdas/python/common/cc_common/data_model/schema/fields.pybackend/cosmetology-app/lambdas/python/common/cc_common/data_model/schema/provider/api.pybackend/cosmetology-app/lambdas/python/common/common_test/test_constants.pybackend/cosmetology-app/lambdas/python/common/common_test/test_data_generator.pybackend/cosmetology-app/lambdas/python/common/tests/resources/api/adverse-action-post.jsonbackend/cosmetology-app/lambdas/python/common/tests/unit/test_data_model/test_schema/test_adverse_action.pybackend/cosmetology-app/lambdas/python/common/tests/unit/test_data_model/test_schema/test_investigation.pybackend/cosmetology-app/lambdas/python/provider-data-v1/handlers/encumbrance.pybackend/cosmetology-app/lambdas/python/provider-data-v1/tests/function/test_handlers/test_encumbrance.pybackend/cosmetology-app/lambdas/python/provider-data-v1/tests/function/test_handlers/test_investigation.pybackend/cosmetology-app/lambdas/python/search/handlers/manage_opensearch_indices.pybackend/cosmetology-app/lambdas/python/search/handlers/populate_provider_documents.pybackend/cosmetology-app/lambdas/python/search/handlers/public_search.pybackend/cosmetology-app/lambdas/python/search/opensearch_client.pybackend/cosmetology-app/lambdas/python/search/tests/function/test_manage_opensearch_indices.pybackend/cosmetology-app/lambdas/python/search/tests/function/test_public_search_providers.pybackend/cosmetology-app/stacks/api_stack/v1_api/api_model.pybackend/cosmetology-app/stacks/search_api_stack/v1_api/api_model.pybackend/cosmetology-app/tests/resources/snapshots/GET_PROVIDER_RESPONSE_SCHEMA.jsonbackend/cosmetology-app/tests/resources/snapshots/LICENSE_ENCUMBRANCE_REQUEST_SCHEMA.jsonbackend/cosmetology-app/tests/resources/snapshots/PATCH_LICENSE_INVESTIGATION_REQUEST_SCHEMA.jsonbackend/cosmetology-app/tests/resources/snapshots/PATCH_PRIVILEGE_INVESTIGATION_REQUEST_SCHEMA.jsonbackend/cosmetology-app/tests/resources/snapshots/PRIVILEGE_ENCUMBRANCE_REQUEST_SCHEMA.jsonbackend/cosmetology-app/tests/resources/snapshots/PROVIDER_USER_RESPONSE_SCHEMA.jsonbackend/cosmetology-app/tests/resources/snapshots/PUBLIC_GET_PROVIDER_RESPONSE_SCHEMA.jsonbackend/cosmetology-app/tests/smoke/encumbrance_smoke_tests.pybackend/cosmetology-app/tests/smoke/investigation_smoke_tests.py
💤 Files with no reviewable changes (2)
- backend/cosmetology-app/lambdas/python/common/common_test/test_data_generator.py
- backend/cosmetology-app/lambdas/python/common/cc_common/data_model/schema/fields.py
✅ Files skipped from review due to trivial changes (2)
- backend/cosmetology-app/lambdas/python/common/tests/resources/api/adverse-action-post.json
- backend/cosmetology-app/lambdas/python/search/tests/function/test_public_search_providers.py
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/cosmetology-app/lambdas/python/search/handlers/public_search.py
backend/cosmetology-app/docs/search-internal/api-specification/latest-oas30.json
Show resolved
Hide resolved
backend/cosmetology-app/lambdas/python/search/handlers/populate_provider_documents.py
Show resolved
Hide resolved
backend/cosmetology-app/lambdas/python/search/handlers/populate_provider_documents.py
Show resolved
Hide resolved
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/python/search/opensearch_client.py`:
- Around line 208-225: The adverse_action_properties mapping in
opensearch_client.py is missing the encumbranceType field which causes tests to
fail; update the adverse_action_properties dict (the variable named
adverse_action_properties) to include 'encumbranceType': {'type': 'keyword'}
alongside the other keyword fields so the mapping contains the expected field
used in tests and index creation. Ensure you add it in the same style as other
keyword entries (e.g., near related fields like 'actionAgainst' or 'compact') to
keep the mapping consistent.
🪄 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: 324f01e1-9c54-46b7-bfec-05756d72017c
📒 Files selected for processing (4)
backend/cosmetology-app/lambdas/python/search/handlers/manage_opensearch_indices.pybackend/cosmetology-app/lambdas/python/search/handlers/populate_provider_documents.pybackend/cosmetology-app/lambdas/python/search/opensearch_client.pybackend/cosmetology-app/lambdas/python/search/tests/function/test_manage_opensearch_indices.py
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/stacks/search_persistent_stack/provider_search_domain.py`:
- Around line 214-216: The shared ingest role self._ingest_lambda_role is being
granted read/write/delete via self.domain.grant_read_write which gives DELETE
scope (e.g., on /compact*); split destructive permissions by creating a
dedicated populate/reset role (e.g., self._populate_lambda_role) and only grant
that new role the extra GET/HEAD/DELETE permissions used by
resetIndexes/populate flows, while keeping self._ingest_lambda_role limited to
write-only and minimal read permissions required for stream ingest (adjust calls
that currently use self.domain.grant_read_write to instead call a
non-destructive grant for the stream role and a separate destructive grant for
the populate/reset role, and update any references to
ingest_access_policy/resetIndexes to use the new populate role).
🪄 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: 08c76fc1-6b35-4879-abf6-93353f9aac4a
📒 Files selected for processing (2)
backend/cosmetology-app/stacks/search_persistent_stack/populate_provider_documents_handler.pybackend/cosmetology-app/stacks/search_persistent_stack/provider_search_domain.py
backend/cosmetology-app/stacks/search_persistent_stack/provider_search_domain.py
Show resolved
Hide resolved
| # privilege data for a provider, we only return privilege data for a provider from the public GET provider endpoint | ||
| # Unlike the JCC public provider search, which only returns privilege data for a provider, Cosmetology returns both | ||
| # licenses and privileges and does not return any adverse action data. | ||
| licenses = List(Nested(LicensePublicResponseSchema(), required=False, allow_none=False)) |
There was a problem hiding this comment.
So I assume the diff looks like this because we copy and pasted compact-connect into here previously and then are simply changing what needs to be changed?
So the diffs for this PR will specifically be the differences in functionality between cosmetology and compact-connect?
There was a problem hiding this comment.
That's one way to think about it. We previously had another PR which updated the logic for how we are indexing documents into OpenSearch specifically to support Cosmetology's needs: #1311
This PR focuses specifically on the changes for public search for Cosmetology compared to the solution we have in place for JCC.
| licenseJurisdiction = String(required=True, allow_none=False) | ||
| compact = Compact(required=True, allow_none=False) | ||
| licenseType = String(required=True, allow_none=False) | ||
| licenseNumber = String(required=True, allow_none=False, validate=Length(1, 100)) |
There was a problem hiding this comment.
So they will always provide the license number?
There was a problem hiding this comment.
Yes, Cosmetology requires license numbers be included for every license upload.
| number_of_shards = int(event.get('numberOfShards', 1)) | ||
| number_of_replicas = int(event.get('numberOfReplicas', 0)) | ||
|
|
||
| if reset_indexes: |
There was a problem hiding this comment.
What is this for? Do you need to reset when you add data in order for it to be accurate?
Do we have some mechanism to only pass in true during low traffic times?
There was a problem hiding this comment.
No, this is specifically for development environments due to a limitation with how OpenSearch will randomly drop your data nodes if you only have 1 in your cluster. If the OpenSearch Domain drops that node due to network failures, aliases and indices will be lost and if the ingest pipeline inserts records before the aliases are recreated, OpenSearch will automatically create those indices under the alias name, but without the proper index mapping which will break our search endpoints. This reset functionality allows devs in test environments to reset those aliases/indices into a clean state before populating all the provider records.
| operation_name=f'delete_index({index_name})', | ||
| ) | ||
|
|
||
| def create_provider_index_with_alias( |
There was a problem hiding this comment.
Why are these methods being added in conjunction with the licenses being returned in search?
There was a problem hiding this comment.
see comment above
These constraints don't apply to OpenSearch queries, as we can search for all matches within the system without any search parameters
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/python/search/handlers/public_search.py`:
- Around line 15-17: The comment above the OpenSearch client instantiation is
inaccurate: it says "20 seconds" while the actual timeout passed to
OpenSearchClient is 25; update either the comment to reflect 25 seconds or
change the timeout argument to 20 to match the comment. Locate the
opensearch_client = OpenSearchClient(timeout=25) line and either change the
numeric timeout to 20 or edit the preceding comment to say "Set timeout to 25
seconds" so the comment and the OpenSearchClient(timeout=...) parameter are
consistent.
- Around line 111-120: The _parse_and_validate_public_query_body function
currently only catches ValidationError but json.loads inside schema.loads can
raise json.JSONDecodeError for malformed JSON, causing 500s; update the
try/except to also catch json.JSONDecodeError (import already present) and raise
CCInvalidRequestException with a clear 400-level message including the decode
error details, alongside the existing ValidationError handling for
PublicQueryProvidersRequestSchema.loads.
🪄 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: d44e7866-5a15-4c6c-b2f7-c1fc64da8a7d
📒 Files selected for processing (2)
backend/cosmetology-app/lambdas/python/search/handlers/public_search.pybackend/cosmetology-app/lambdas/python/search/tests/function/test_public_search_providers.py
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/cosmetology-app/lambdas/python/search/tests/function/test_public_search_providers.py
Cosmetology has slightly different requirements for public searching given the expected volume of practitioners with similar names. We need to support additional search parameters, such as license number. In order to accommodate these various patterns, we have determined to leverage the OpenSearch Domain to support public search, while keeping the request schema similar in shape to the public search request body used by JCC.
Closes #1295
Summary by CodeRabbit
New Features
Updates
Tests