Skip to content

Feat/state data pull api#962

Merged
isabeleliassen merged 9 commits intocsg-org:developmentfrom
InspiringApps:feat/state-data-pull-api
Aug 1, 2025
Merged

Feat/state data pull api#962
isabeleliassen merged 9 commits intocsg-org:developmentfrom
InspiringApps:feat/state-data-pull-api

Conversation

@jusdino
Copy link
Copy Markdown
Contributor

@jusdino jusdino commented Jul 29, 2025

Requirements List

  • Implemented a new simplified API for states to integrate with for data upload / download
  • Added a redirect http -> https UI behavior to our cloudfront distribution

Description List

  • Created a new Stack with a new 'state-api'
  • Copied over bulk-upload and POST license endpoints
  • Implemented a new query providers endpoint for a provider list-view
  • Implemented a new GET provider endpoint with a simplified/flattened structure

Testing List

  • Exercise new state-api, upload a license and query a privilege
  • Code review

Closes #697
Closes #902

Summary by CodeRabbit

  • New Features

    • Introduced a new State API for state-level provider data access, including endpoints for querying providers by jurisdiction and retrieving detailed provider information.
    • Added support for bulk license uploads and new API endpoints for license submissions.
    • Enhanced provider filtering and query capabilities with advanced date range and jurisdiction-based parameters.
    • Updated API documentation tools and scripts, including automated OpenAPI spec downloads and Postman environment updates.
  • Bug Fixes

    • Corrected schema requirements and improved validation for provider and license data.
  • Documentation

    • Added internal API documentation, updated Postman environments, and included new JSON schema definitions for API requests and responses.
  • Tests

    • Added comprehensive tests for the new State API endpoints, provider filtering logic, and schema validation.
  • Style/Refactor

    • Improved import organization and parameter naming consistency across modules.
  • Chores

    • Updated CloudFront distribution to redirect HTTP to HTTPS.
    • Added scripts to automate API documentation updates.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jul 29, 2025

Walkthrough

This update introduces a new State API stack and associated Lambda handlers, schemas, and infrastructure for jurisdiction-based provider data queries and license uploads. It refactors the API stack architecture, adds new request/response schemas, updates environment and deployment scripts, and implements comprehensive tests for the new endpoints and stack configuration. Viewer protocol policies are updated to redirect HTTP to HTTPS.

Changes

Cohort / File(s) Change Summary
State API Stack: Infrastructure & Models
stacks/state_api_stack/__init__.py, stacks/state_api_stack/v1_api/api.py, stacks/state_api_stack/v1_api/api_model.py, stacks/state_api_stack/v1_api/bulk_upload_url.py, stacks/state_api_stack/v1_api/post_licenses.py, stacks/state_api_stack/v1_api/provider_management.py, stacks/state_api_stack/v1_api/__init__.py
Introduces a new State API stack with versioned API resources, Lambda integrations for provider and license endpoints, and detailed JSON schema models for requests and responses.
State API Lambda Handlers & Schemas
lambdas/python/provider-data-v1/handlers/state_api.py, lambdas/python/common/cc_common/data_model/schema/provider/api.py, lambdas/python/common/cc_common/data_model/data_client.py, lambdas/python/common/cc_common/config.py
Implements Lambda handlers for jurisdiction provider queries and provider detail retrieval, adds new Marshmallow schemas for requests/responses, and enhances data client filtering logic and config properties.
State API: Tests & Test Resources
lambdas/python/provider-data-v1/tests/function/test_handlers/test_state_api.py, lambdas/python/provider-data-v1/tests/function/__init__.py, lambdas/python/provider-data-v1/tests/function/test_data_model/test_client.py, lambdas/python/common/tests/unit/test_data_model/test_schema/test_provider.py, lambdas/python/common/tests/resources/api/state-provider-detail-response.json
Adds comprehensive functional and unit tests for State API handlers and filtering logic, and introduces a new test resource for provider detail responses.
State API: Stack Tests & Snapshots
tests/app/test_api/test_state_api.py, tests/resources/snapshots/STATE_API_*, tests/resources/snapshots/QUERY_PROVIDERS_RESPONSE_SCHEMA.json
Adds stack-level tests for State API infrastructure and new JSON schema snapshots for request/response validation.
API Stack Refactor & Domain Handling
common_constructs/cc_api.py, common_constructs/stack.py, stacks/api_stack/__init__.py, stacks/api_stack/v1_api/api.py, stacks/api_stack/v1_api/api_model.py, stacks/api_stack/v1_api/attestations.py, stacks/api_stack/v1_api/bulk_upload_url.py, stacks/api_stack/v1_api/compact_configuration_api.py, stacks/api_stack/v1_api/credentials.py, stacks/api_stack/v1_api/post_licenses.py, stacks/api_stack/v1_api/provider_management.py, stacks/api_stack/v1_api/provider_users.py, stacks/api_stack/v1_api/public_lookup_api.py, stacks/api_stack/v1_api/purchases.py, stacks/api_stack/v1_api/staff_users.py
Refactors API stack to decouple V1Api instantiation from CCApi, adds domain name parameterization, updates imports, and aligns type annotations.
Pipeline & Deployment
pipeline/backend_stage.py, stacks/frontend_deployment_stack/distribution.py
Adds new StateApiStack to backend stage and updates CloudFront viewer protocol policy to redirect HTTP to HTTPS.
Environment & Postman Config
docs/postman/postman-environment.json, docs/internal/postman/postman-environment.json
Updates and adds Postman environment files for beta/internal environments, including new m2m auth variables.
API Spec & Postman Automation
bin/download_oas30.py, bin/trim_oas30.py, bin/update_postman_collection.py, bin/update_api_docs.sh, docs/internal/api-specification/swagger.html
Adds scripts for downloading, trimming, and updating API specs and Postman collections, and a new Swagger UI HTML for internal docs.
Other Test & Minor Changes
lambdas/python/common/common_test/test_data_generator.py, lambdas/python/provider-data-v1/handlers/public_lookup.py, lambdas/python/cognito-backup/requirements.in, lambdas/python/cognito-backup/requirements-dev.in, tests/resources/snapshots/BetaFrontend-FrontendDeploymentStack-UI_DISTRIBUTION.json, tests/resources/snapshots/ProdFrontend-FrontendDeploymentStack-UI_DISTRIBUTION.json, tests/resources/snapshots/TestFrontend-FrontendDeploymentStack-UI_DISTRIBUTION.json
Minor updates to test data generation, handler parameter usage, requirements files, and CloudFront distribution test snapshots.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant APIGateway
    participant LambdaQuery
    participant LambdaGet
    participant DynamoDB

    Client->>APIGateway: POST /v1/compacts/{compact}/jurisdictions/{jurisdiction}/providers/query
    APIGateway->>LambdaQuery: Invoke query_jurisdiction_providers
    LambdaQuery->>DynamoDB: Query providers with filters
    DynamoDB-->>LambdaQuery: Provider records
    LambdaQuery-->>APIGateway: Filtered provider list (general schema)
    APIGateway-->>Client: Response

    Client->>APIGateway: GET /v1/compacts/{compact}/jurisdictions/{jurisdiction}/providers/{providerId}
    APIGateway->>LambdaGet: Invoke get_provider
    LambdaGet->>DynamoDB: Get provider and license records
    DynamoDB-->>LambdaGet: Provider and license data
    LambdaGet-->>APIGateway: Provider detail (private/general schema)
    APIGateway-->>Client: Response
Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~90+ minutes

Assessment against linked issues

Objective Addressed Explanation
HTTP to HTTPS redirection for frontend ( #902 )
Daily transaction report opt-in for states/compacts ( #697 ) No code related to opt-in or daily transaction report is present; changes focus on state API for provider data, not reporting features.

Assessment against linked issues: Out-of-scope changes

Code Change Explanation
Addition of State API stack, Lambda handlers, schemas, and tests (e.g., stacks/state_api_stack/*, lambdas/python/provider-data-v1/handlers/state_api.py, etc.) These changes implement a new State API for provider data, which is unrelated to the objectives of HTTP redirection or daily transaction reporting.
Refactoring of API stack domain handling and V1Api instantiation (e.g., common_constructs/cc_api.py, stacks/api_stack/__init__.py) These architectural changes are not mentioned in the linked issues.
Addition of API spec download/trim/update scripts and Swagger UI (e.g., bin/download_oas30.py, docs/internal/api-specification/swagger.html) These documentation automation and UI changes are not related to the linked issues.
Updates to Postman environments and m2m variables (e.g., docs/postman/postman-environment.json) These configuration changes are not part of the stated objectives.

Possibly related PRs

Suggested reviewers

  • jlkravitz

Poem

🐇✨

A new API stack hops in with pride,
With schemas and Lambdas side by side.
State queries bloom, provider details grow,
HTTP now redirects where HTTPS flows!
Tests abound, docs update too—
This rabbit’s work brings something new!

🥕

Note

⚡️ Unit Test Generation is now available in beta!

Learn more here, or try it out under "Finishing Touches" below.


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 32c91aa and 9e69198.

📒 Files selected for processing (10)
  • backend/compact-connect/lambdas/python/cognito-backup/requirements-dev.in (1 hunks)
  • backend/compact-connect/lambdas/python/cognito-backup/requirements.in (1 hunks)
  • backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py (1 hunks)
  • backend/compact-connect/lambdas/python/common/common_test/test_data_generator.py (3 hunks)
  • backend/compact-connect/pipeline/backend_stage.py (2 hunks)
  • backend/compact-connect/stacks/api_stack/v1_api/api.py (2 hunks)
  • backend/compact-connect/stacks/api_stack/v1_api/api_model.py (4 hunks)
  • backend/compact-connect/stacks/api_stack/v1_api/provider_management.py (2 hunks)
  • backend/compact-connect/stacks/api_stack/v1_api/provider_users.py (2 hunks)
  • backend/compact-connect/stacks/api_stack/v1_api/public_lookup_api.py (2 hunks)
✅ Files skipped from review due to trivial changes (2)
  • backend/compact-connect/lambdas/python/cognito-backup/requirements-dev.in
  • backend/compact-connect/lambdas/python/cognito-backup/requirements.in
🚧 Files skipped from review as they are similar to previous changes (8)
  • backend/compact-connect/pipeline/backend_stage.py
  • backend/compact-connect/stacks/api_stack/v1_api/api.py
  • backend/compact-connect/stacks/api_stack/v1_api/provider_users.py
  • backend/compact-connect/stacks/api_stack/v1_api/public_lookup_api.py
  • backend/compact-connect/stacks/api_stack/v1_api/provider_management.py
  • backend/compact-connect/stacks/api_stack/v1_api/api_model.py
  • backend/compact-connect/lambdas/python/common/common_test/test_data_generator.py
  • backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: TestPython
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@jusdino
Copy link
Copy Markdown
Contributor Author

jusdino commented Jul 29, 2025

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jul 29, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

🔭 Outside diff range comments (1)
backend/compact-connect/upload.sh (1)

1-12: Critical: Remove hardcoded AWS credentials from source code.

This script contains hardcoded AWS credentials, signatures, and security tokens that pose a significant security risk. These credentials should never be committed to source control.

Immediate Actions Required:

  1. Remove this file from the repository or replace credentials with placeholder values
  2. Rotate the exposed AWS credentials if they are still active
  3. Use environment variables or AWS credential profiles for authentication

Suggested Implementation:

-curl --location 'https://sandbox-persistentstack-bulkuploadsbucketda4bdcd0-pi5pskm7prtp.s3.amazonaws.com/' \
---form 'key="aslp/ky/b3b4f3c0c66e450f9bc16f674760bebe"' \
---form 'x-amz-algorithm="AWS4-HMAC-SHA256"' \
---form 'x-amz-credential="ASIA6ODU4SFJWZVTKILF/20250728/us-east-1/s3/aws4_request"' \
---form 'x-amz-date="20250728T220700Z"' \
---form 'x-amz-signature="26fa8ef82a19d40adc2a9e7ef4172d513391884da2f02ecdabf1345ecb7ec2a6"' \
---form 'x-amz-security-token="IQoJb3JpZ2luX2VjEG8aCXVzLWVhc3QtMSJIMEYCIQDhXM7sT2pdJZI/iUNiQ7dIoBGnwWfpeSH4680KRJO0nAIhAJkgmxyJv6i7PNW+5nQ+gjgxHWbDidrWfE8L+wp3UgjWKtwDCJf//////////wEQABoMOTkyMzgyNTg3MjE5IgyVlQ1A77zhOI9OwbwqsAPlzm+gCji2RMpjYbN91X/OiOFvt3vcjz4tgrr0Iox8OLa1oBn8y2L1Tu798zzxQsa0Lv/bj5zptVbtVn3r0pmWjM2oHKNRz7H2pMn6Ol4dI4NrFx1qnz7qUDQsusXMxm5+HfdEtbBxIhPUIJ4O/zOoIi6JEJ+wsK7v1EVyLlga9GFgMnsreBrHQeR5g0f8iNaRcvk8ft4mWZusjmz9z6X0pfw1bkFAJ/rR+jGsQma/tXGxGyGm6X9rqXfgt2RZIpK9LEzw6b0E0rmp8d1uIGUTR4uGsO/RyLrqQ3lzc7jRjutdQYR/GtjM59sxW5M6BV5i8EJdvJoADe2Hatnt4hFO5gYYx4UbfjaL7Wu0ox3NA4BQEcjjUtdbPEy9UWR/07dEudzeGCgL/AqPr43Rd/3W8l0CBIWpROnF00jpg9okgKvEK31tkdPbvAUCczqmq4/FrTDtHoc7gSzQWx/+vuAklOScHRDbG6u6Mal1VMLuv3kk9MXmg9TD8H07zIW9ITENeaKWuuAlM2F6pZIa77RMjV8jFYNn5IvRaqOo1orpsUcGX080zAxDMKgyntQyK+ow9OifxAY6nQEFInEz4+41iNi36xxsximDM6rVsgyav+F9GM9OMiCRoMkqtUKa+ANPtDZlgMIc0eiU01HH7CI4xIHL61BIzT2wE3sWdNF0BhwHsW5yx2sO8bmAz6wTk2TjEnU1dqAI4MGcgvfAjk/IjTplLHAFJrguAdTsFmYkQgom5NEg7kjRjBQ3MIsE2MzCqd+Bwk3HCOmbHak9x2Mjdwe3+yTE"' \
---form 'policy="eyJleHBpcmF0aW9uIjogIjIwMjUtMDctMjhUMjM6MDc6MDBaIiwgImNvbmRpdGlvbnMiOiBbWyJjb250ZW50LWxlbmd0aC1yYW5nZSIsIDEsIDMwMDAwMDAwXSwgeyJidWNrZXQiOiAic2FuZGJveC1wZXJzaXN0ZW50c3RhY2stYnVsa3VwbG9hZHNidWNrZXRkYTRiZGNkMC1waTVwc2ttN3BydHAifSwgeyJrZXkiOiAiYXNscC9reS9iM2I0ZjNjMGM2NmU0NTBmOWJjMTZmNjc0NzYwYmViZSJ9LCB7IngtYW16LWFsZ29yaXRobSI6ICJBV1M0LUhNQUMtU0hBMjU2In0sIHsieC1hbXotY3JlZGVudGlhbCI6ICJBU0lBNk9EVTRTRkpXWlZUS0lMRi8yMDI1MDcyOC91cy1lYXN0LTEvczMvYXdzNF9yZXF1ZXN0In0sIHsieC1hbXotZGF0ZSI6ICIyMDI1MDcyOFQyMjA3MDBaIn0sIHsieC1hbXotc2VjdXJpdHktdG9rZW4iOiAiSVFvSmIzSnBaMmx1WDJWakVHOGFDWFZ6TFdWaGMzUXRNU0pJTUVZQ0lRRGhYTTdzVDJwZEpaSS9pVU5pUTdkSW9CR253V2ZwZVNINDY4MEtSSk8wbkFJaEFKa2dteHlKdjZpN1BOVys1blErZ2pneEhXYkRpZHJXZkU4TCt3cDNVZ2pXS3R3RENKZi8vLy8vLy8vLy93RVFBQm9NT1RreU16Z3lOVGczTWpFNUlneVZsUTFBNzd6aE9JOU93Yndxc0FQbHptK2dDamkyUk1walliTjkxWC9PaU9GdnQzdmNqejR0Z3JyMElveDhPTGExb0JuOHkyTDFUdTc5OHp6eFFzYTBMdi9iajV6cHRWYnRWbjNyMHBtV2pNMm9IS05SejdIMnBNbjZPbDRkSTROckZ4MXFuejdxVURRc3VzWE14bTUrSGZkRXRiQnhJaFBVSUo0Ty96T29JaTZKRUord3NLN3YxRVZ5TGxnYTlHRmdNbnNyZUJySFFlUjVnMGY4aU5hUmN2azhmdDRtV1p1c2ptejl6NlgwcGZ3MWJrRkFKL3JSK2pHc1FtYS90WEd4R3lHbTZYOXJxWGZndDJSWklwSzlMRXp3NmIwRTBybXA4ZDF1SUdVVFI0dUdzTy9SeUxycVEzbHpjN2pSanV0ZFFZUi9HdGpNNTlzeFc1TTZCVjVpOEVKZHZKb0FEZTJIYXRudDRoRk81Z1lZeDRVYmZqYUw3V3Uwb3gzTkE0QlFFY2pqVXRkYlBFeTlVV1IvMDdkRXVkemVHQ2dML0FxUHI0M1JkLzNXOGwwQ0JJV3BST25GMDBqcGc5b2tnS3ZFSzMxdGtkUGJ2QVVDY3pxbXE0L0ZyVER0SG9jN2dTelFXeC8rdnVBa2xPU2NIUkRiRzZ1Nk1hbDFWTUx1djNrazlNWG1nOVREOEgwN3pJVzlJVEVOZWFLV3V1QWxNMkY2cFpJYTc3Uk1qVjhqRllObjVJdlJhcU9vMW9ycHNVY0dYMDgwekF4RE1LZ3ludFF5SytvdzlPaWZ4QVk2blFFRkluRXo0KzQxaU5pMzZ4eHN4aW1ETTZyVnNneWF2K0Y5R005T01pQ1JvTWtxdFVLYStBTlB0RFpsZ01JYzBlaVUwMUhIN0NJNHhJSEw2MUJJelQyd0Uzc1dkTkYwQmh3SHNXNXl4MnNPOGJtQXo2d1RrMlRqRW5VMWRxQUk0TUdjZ3ZmQWprL0lqVHBsTEhBRkpyZ3VBZFRzRm1Za1Fnb201TkVnN2tqUmpCUTNNSXNFMk16Q3FkK0J3azNIQ09tYkhhazl4Mk1qZHdlMyt5VEUifV19"' \
+# Use environment variables or AWS CLI profiles instead
+# Example: aws s3 presign s3://your-bucket/path --expires-in 3600
+curl --location "${S3_UPLOAD_URL}" \
+$(cat upload_fields.txt) \
 --form 'file=@"lambdas/python/common/tests/resources/licenses.csv"'
🧹 Nitpick comments (16)
backend/compact-connect/stacks/api_stack/v1_api/staff_users.py (1)

46-46: Type narrowing may upset static checkers

Resource.api is typed as IRestApi; narrowing it directly to CCApi can trigger mypy/pyright errors.
If you run type-checking, consider adding an explicit cast:

-from common_constructs.cc_api import CCApi
+from typing import cast
+from common_constructs.cc_api import CCApi-        self.api: CCApi = admin_resource.api
+        self.api: CCApi = cast(CCApi, admin_resource.api)

Optional, but keeps static analysis silent.

backend/compact-connect/stacks/api_stack/v1_api/purchases.py (1)

34-35: Same static-typing caveat as in staff_users.py

Directly annotating resource.api as CCApi narrows the type.
If your CI runs strict type-checking, wrap in cast(CCApi, …) to avoid complaints.

backend/compact-connect/stacks/api_stack/v1_api/provider_management.py (1)

50-51: Consider explicit cast for clarity

Same reasoning as previous files: Resource.api returns IRestApi.
Using cast() keeps the type-checker quiet.

backend/compact-connect/stacks/api_stack/v1_api/provider_users.py (1)

36-37: Optional: add cast() for type safety

See earlier comments on narrowing IRestApiCCApi.

backend/compact-connect/tests/resources/snapshots/STATE_API_POST_LICENSES_RESPONSE_SCHEMA.json (1)

1-13: Schema is minimal – add description?

The response contract only enforces a message string.
If clients depend on a fixed literal (e.g., "message": "success"), consider using enum or adding a short "description" for maintainability.
Optional enhancement; no functional issue.

backend/compact-connect/docs/internal-api-specification/swagger.html (2)

8-8: Consider security implications of external CDN dependency.

Loading Swagger UI assets from unpkg.com introduces external dependencies that could impact availability and security. Consider hosting these assets locally or using a more secure CDN with SRI (Subresource Integrity) checks.

-    <link rel="stylesheet" href="https://unpkg.com/swagger-ui-dist@5.11.0/swagger-ui.css" />
+    <link rel="stylesheet" href="https://unpkg.com/swagger-ui-dist@5.11.0/swagger-ui.css" integrity="sha384-..." crossorigin="anonymous" />

12-12: Add SRI integrity check for external script.

The external JavaScript bundle should include integrity verification for security best practices.

-    <script src="https://unpkg.com/swagger-ui-dist@5.11.0/swagger-ui-bundle.js" crossorigin></script>
+    <script src="https://unpkg.com/swagger-ui-dist@5.11.0/swagger-ui-bundle.js" integrity="sha384-..." crossorigin="anonymous"></script>
backend/compact-connect/lambdas/python/common/tests/resources/api/state-provider-detail-response.json (1)

2-33: Consider data consistency between privilege records.

The first privilege record includes emailAddress, npi, and licenseNumber fields that are missing from the second record (lines 34-59). For the same provider, this inconsistency might not reflect realistic data patterns.

Consider whether both records should have consistent field coverage for better test reliability:

         {
             "type": "statePrivilege",
             "providerId": "123e4567-e89b-12d3-a456-426614174000",
             "compact": "aslp",
             "jurisdiction": "ne",
             "ssnLastFour": "1234",
             "licenseType": "audiologist",
             "privilegeId": "AUD-NE-1",
+            "licenseNumber": "A0608337261", 
+            "npi": "0608337261",
             "status": "active",
             "compactEligibility": "eligible",
+            "emailAddress": "björk@example.com",
backend/compact-connect/tests/resources/snapshots/STATE_API_GET_PROVIDER_RESPONSE_SCHEMA.json (1)

1-313: LGTM! Comprehensive schema with robust validation.

The schema provides thorough validation for the state API provider response with appropriate patterns, constraints, and enumerations. The structure aligns well with the flattened data format intended for state IT systems.

Consider extracting the jurisdiction and compact enumerations to shared constants or generating them dynamically to improve maintainability as new jurisdictions/compacts are added.

backend/compact-connect/stacks/state_api_stack/v1_api/api.py (2)

17-23: Validate the super().init() call

The super().__init__() call on line 18 appears unnecessary since V1Api doesn't explicitly inherit from any class. Consider removing it.

-    def __init__(self, root: IResource, persistent_stack: ps.PersistentStack):
-        super().__init__()
+    def __init__(self, root: IResource, persistent_stack: ps.PersistentStack):

23-43: Consider error handling for empty compact/jurisdiction lists

The authorization scope generation assumes that _active_compacts and jurisdictions will always contain data. Consider adding validation to handle edge cases where these lists might be empty.

_active_compacts = persistent_stack.get_list_of_compact_abbreviations()
if not _active_compacts:
    raise ValueError("No active compacts found - cannot configure authorization scopes")
backend/compact-connect/stacks/state_api_stack/v1_api/bulk_upload_url.py (1)

16-27: Remove unnecessary super().init() call

The super().__init__() call on line 26 is unnecessary since BulkUploadUrl doesn't inherit from any class.

-    def __init__(
-        self,
-        *,
-        resource: Resource,
-        method_options: MethodOptions,
-        bulk_uploads_bucket: IBucket,
-        license_upload_role: IRole,
-        api_model: ApiModel,
-    ):
-        super().__init__()
+    def __init__(
+        self,
+        *,
+        resource: Resource,
+        method_options: MethodOptions,
+        bulk_uploads_bucket: IBucket,
+        license_upload_role: IRole,
+        api_model: ApiModel,
+    ):
backend/compact-connect/stacks/state_api_stack/v1_api/post_licenses.py (1)

18-28: Remove unnecessary super().init() call

The super().__init__() call on line 27 is unnecessary since PostLicenses doesn't inherit from any class.

-    def __init__(
-        self,
-        *,
-        resource: Resource,
-        method_options: MethodOptions,
-        persistent_stack: ps.PersistentStack,
-        api_model: ApiModel,
-    ):
-        super().__init__()
+    def __init__(
+        self,
+        *,
+        resource: Resource,
+        method_options: MethodOptions,
+        persistent_stack: ps.PersistentStack,
+        api_model: ApiModel,
+    ):
backend/compact-connect/stacks/state_api_stack/v1_api/provider_management.py (1)

45-47: Consider making the fallback API base URL configurable.

The hardcoded test environment URL might not be appropriate for all deployment scenarios.

Consider using a stack parameter or context value for the fallback URL instead of hardcoding:

-            'API_BASE_URL': f'https://{stack.ui_domain_name}'
-            if stack.ui_domain_name is not None
-            else 'https://app.test.compactconnect.org',
+            'API_BASE_URL': f'https://{stack.ui_domain_name}'
+            if stack.ui_domain_name is not None
+            else stack.node.try_get_context('default_api_base_url') or 'https://app.test.compactconnect.org',
backend/compact-connect/tests/app/test_api/test_state_api.py (1)

378-385: Use logical ID instead of searching for Lambda by handler name.

The current approach of searching through all Lambda functions is less efficient and reliable than using the logical ID directly.

-        # Find the bulk upload handler by looking for the lambda function with the correct handler
-        lambda_functions = state_api_stack_template.find_resources(CfnFunction.CFN_RESOURCE_TYPE_NAME)
-        bulk_upload_handler = None
-        for logical_id, function_props in lambda_functions.items():
-            if function_props['Properties']['Handler'] == 'handlers.bulk_upload.bulk_upload_url_handler':
-                bulk_upload_handler = logical_id
-                break
-
-        self.assertIsNotNone(bulk_upload_handler, 'Bulk upload handler not found')
+        # Get the bulk upload handler logical ID directly from the stack
+        bulk_upload_handler = state_api_stack.get_logical_id(
+            state_api_stack.api.v1_api.bulk_upload_url.bulk_upload_url_handler.node.default_child
+        )
backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_state_api.py (1)

72-127: Consider adding more comprehensive test scenarios.

The test only queries a single point in time (start and end datetime are the same). Consider testing:

  • A proper date range instead of a single point
  • Pagination scenarios with lastKey
  • Different page sizes
  • Edge cases around the date boundaries

Comment thread backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py Outdated
Comment thread backend/compact-connect/stacks/state_api_stack/v1_api/api_model.py Outdated
Comment thread backend/compact-connect/stacks/state_api_stack/v1_api/api_model.py Outdated
Comment thread backend/compact-connect/stacks/state_api_stack/v1_api/api_model.py Outdated
@jusdino jusdino force-pushed the feat/state-data-pull-api branch from f4983d3 to 706eebf Compare July 29, 2025 16:43
@jusdino jusdino marked this pull request as ready for review July 29, 2025 21:44
@jusdino jusdino requested a review from landonshumway-ia July 29, 2025 21:51
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
backend/compact-connect/bin/download_oas30.py (1)

18-33: Fix docstring inconsistency.

The docstring states the function returns "API ID if found, None otherwise", but the implementation raises a RuntimeError when the API is not found instead of returning None.

Update the docstring to match the actual behavior:

-    :return: API ID if found, None otherwise
+    :return: API ID if found, raises RuntimeError otherwise
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 10daeba and abe596c.

📒 Files selected for processing (21)
  • backend/compact-connect/bin/download_oas30.py (1 hunks)
  • backend/compact-connect/bin/trim_oas30.py (2 hunks)
  • backend/compact-connect/bin/update_api_docs.sh (1 hunks)
  • backend/compact-connect/bin/update_postman_collection.py (2 hunks)
  • backend/compact-connect/common_constructs/cc_api.py (5 hunks)
  • backend/compact-connect/docs/internal/api-specification/swagger.html (1 hunks)
  • backend/compact-connect/docs/internal/postman/postman-environment.json (1 hunks)
  • backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py (1 hunks)
  • backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py (1 hunks)
  • backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/provider/api.py (3 hunks)
  • backend/compact-connect/lambdas/python/provider-data-v1/handlers/__init__.py (1 hunks)
  • backend/compact-connect/lambdas/python/provider-data-v1/handlers/state_api.py (1 hunks)
  • backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_data_model/test_client.py (1 hunks)
  • backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_provider_users.py (2 hunks)
  • backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_state_api.py (1 hunks)
  • backend/compact-connect/stacks/api_stack/v1_api/api_model.py (4 hunks)
  • backend/compact-connect/stacks/state_api_stack/v1_api/api_model.py (1 hunks)
  • backend/compact-connect/tests/resources/snapshots/QUERY_PROVIDERS_RESPONSE_SCHEMA.json (1 hunks)
  • backend/compact-connect/tests/resources/snapshots/STATE_API_BULK_UPLOAD_RESPONSE_SCHEMA.json (1 hunks)
  • backend/compact-connect/tests/resources/snapshots/STATE_API_QUERY_PROVIDERS_REQUEST_SCHEMA.json (1 hunks)
  • backend/compact-connect/tests/resources/snapshots/STATE_API_QUERY_PROVIDERS_RESPONSE_SCHEMA.json (1 hunks)
✅ Files skipped from review due to trivial changes (3)
  • backend/compact-connect/docs/internal/postman/postman-environment.json
  • backend/compact-connect/docs/internal/api-specification/swagger.html
  • backend/compact-connect/tests/resources/snapshots/STATE_API_QUERY_PROVIDERS_RESPONSE_SCHEMA.json
🚧 Files skipped from review as they are similar to previous changes (7)
  • backend/compact-connect/lambdas/python/provider-data-v1/handlers/init.py
  • backend/compact-connect/tests/resources/snapshots/STATE_API_BULK_UPLOAD_RESPONSE_SCHEMA.json
  • backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_provider_users.py
  • backend/compact-connect/tests/resources/snapshots/STATE_API_QUERY_PROVIDERS_REQUEST_SCHEMA.json
  • backend/compact-connect/stacks/api_stack/v1_api/api_model.py
  • backend/compact-connect/common_constructs/cc_api.py
  • backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py
🧰 Additional context used
🧠 Learnings (7)
backend/compact-connect/tests/resources/snapshots/QUERY_PROVIDERS_RESPONSE_SCHEMA.json (1)

Learnt from: landonshumway-ia
PR: #769
File: backend/compact-connect/lambdas/python/common/tests/resources/dynamo/provider.json:5-5
Timestamp: 2025-04-29T01:57:43.684Z
Learning: The provider.json test data contains both "providerDateOfUpdate" and "dateOfUpdate" fields which serve different purposes, and both should be maintained in the test files.

backend/compact-connect/stacks/state_api_stack/v1_api/api_model.py (2)

Learnt from: landonshumway-ia
PR: #769
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/record.py:40-43
Timestamp: 2025-04-29T02:18:11.099Z
Learning: In the CompactConnect codebase, API design follows a pattern where fields are either provided with valid values or omitted entirely. Null values are not allowed, even for optional fields. This applies to Marshmallow schemas where fields are configured with required=False, allow_none=False.

Learnt from: landonshumway-ia
PR: #769
File: backend/compact-connect/lambdas/python/common/tests/resources/dynamo/provider.json:5-5
Timestamp: 2025-04-29T01:57:43.684Z
Learning: The provider.json test data contains both "providerDateOfUpdate" and "dateOfUpdate" fields which serve different purposes, and both should be maintained in the test files.

backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/provider/api.py (2)

Learnt from: landonshumway-ia
PR: #769
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/common.py:87-90
Timestamp: 2025-04-29T21:09:04.842Z
Learning: In the CCDataClass implementation, validation is performed during both the dump() and load() processes in Marshmallow, so calling dump() first still results in ValidationError if the provided data is invalid.

Learnt from: landonshumway-ia
PR: #798
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/fields.py:86-90
Timestamp: 2025-05-13T21:20:51.133Z
Learning: The implementation of PositiveDecimal in cc_common/data_model/schema/fields.py is intentionally kept simple without support for stacked validators. If more complex validation is needed, the preferred approach is to create another custom field class.

backend/compact-connect/lambdas/python/provider-data-v1/handlers/state_api.py (3)

Learnt from: landonshumway-ia
PR: #769
File: backend/compact-connect/lambdas/python/provider-data-v1/handlers/encumbrance.py:53-60
Timestamp: 2025-04-29T02:10:38.400Z
Learning: Lambda functions in the provider-data-v1 module that are exposed via API Gateway endpoints (like encumbrance_handler) should only be called through the API, not directly. Therefore, they don't need to handle various body formats as the API Gateway will consistently provide event['body'] as a JSON string.

Learnt from: jusdino
PR: #864
File: backend/compact-connect/lambdas/python/data-events/tests/function/test_encumbrance_events.py:866-866
Timestamp: 2025-06-30T17:30:31.252Z
Learning: In the CompactConnect project, the @sqs_handler decorator transforms Lambda functions to accept (event, context) parameters and automatically handles iteration over event['Records'], parsing JSON from each record's body, and managing SQS batch item failures. Functions decorated with @sqs_handler have their underlying implementation accept a single message: dict parameter, but the decorator makes them callable with standard Lambda (event, context) signature. Tests should call these decorated functions with (event, context) parameters.

Learnt from: jusdino
PR: #864
File: backend/compact-connect/tests/smoke/encumbrance_smoke_tests.py:256-262
Timestamp: 2025-06-18T21:57:02.978Z
Learning: The licenseJurisdiction field is a required field in the provider data API response from the /v1/providers/users/me endpoint, so it can be accessed directly without defensive programming checks.

backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_state_api.py (6)

Learnt from: landonshumway-ia
PR: #882
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/compact_configuration_client.py:287-359
Timestamp: 2025-07-08T18:40:24.408Z
Learning: In the CompactConnect codebase, landonshumway-ia prefers to avoid extraneous unit tests when existing test coverage is already sufficient to catch bugs. For the get_privilege_purchase_options method's live-jurisdiction filtering logic, the existing tests in the purchases test suite provide adequate coverage without needing additional edge case tests.

Learnt from: landonshumway-ia
PR: #769
File: backend/compact-connect/lambdas/python/common/tests/resources/dynamo/provider.json:5-5
Timestamp: 2025-04-29T01:57:43.684Z
Learning: The provider.json test data contains both "providerDateOfUpdate" and "dateOfUpdate" fields which serve different purposes, and both should be maintained in the test files.

Learnt from: landonshumway-ia
PR: #848
File: backend/compact-connect/lambdas/python/migration/provider_user_pool_migration_551/main.py:35-39
Timestamp: 2025-06-10T03:16:16.896Z
Learning: In the provider user pool migration (provider_user_pool_migration_551), the FilterExpression intentionally only checks for the existence of compactConnectRegisteredEmailAddress. The migration should only remove currentHomeJurisdiction if compactConnectRegisteredEmailAddress is also present, targeting records that went through the old registration process. Records with only currentHomeJurisdiction but no compactConnectRegisteredEmailAddress should be left untouched.

Learnt from: landonshumway-ia
PR: #907
File: backend/compact-connect/lambdas/python/cognito-backup/handlers/cognito_backup.py:148-149
Timestamp: 2025-07-21T20:39:47.625Z
Learning: In the CompactConnect Cognito backup system, UserCreateDate and UserLastModifiedDate fields from the AWS Cognito ListUsers API are accessed using direct dictionary access (user_data['UserCreateDate']) rather than .get() method. This is intentional per landonshumway-ia's decision to fail fast if AWS breaks their documented API contract that guarantees these fields are always present. The KeyError that would result from missing fields should trigger alerts to detect API contract violations rather than being silently handled.

Learnt from: landonshumway-ia
PR: #877
File: backend/compact-connect/tests/smoke/practitioner_email_update_smoke_tests.py:22-192
Timestamp: 2025-06-23T20:19:18.952Z
Learning: In the CompactConnect project, smoke test functions (like test_practitioner_email_update in backend/compact-connect/tests/smoke/practitioner_email_update_smoke_tests.py) should remain as single large functions rather than being broken down into smaller helper functions, based on the project maintainer's preference and reasoning.

Learnt from: jusdino
PR: #864
File: backend/compact-connect/tests/smoke/encumbrance_smoke_tests.py:256-262
Timestamp: 2025-06-18T21:57:02.978Z
Learning: The licenseJurisdiction field is a required field in the provider data API response from the /v1/providers/users/me endpoint, so it can be accessed directly without defensive programming checks.

backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py (1)

Learnt from: landonshumway-ia
PR: #796
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py:58-64
Timestamp: 2025-06-03T16:41:07.757Z
Learning: The static get_provider_record method in ProviderRecordUtility is legacy code being phased out in favor of the ProviderUserRecords class. During this migration, they are keeping the static method backwards compatible (returning None) while the new ProviderUserRecords class implements the better pattern of raising exceptions.

backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_data_model/test_client.py (3)

Learnt from: landonshumway-ia
PR: #769
File: backend/compact-connect/lambdas/python/common/tests/resources/dynamo/provider.json:5-5
Timestamp: 2025-04-29T01:57:43.684Z
Learning: The provider.json test data contains both "providerDateOfUpdate" and "dateOfUpdate" fields which serve different purposes, and both should be maintained in the test files.

Learnt from: landonshumway-ia
PR: #882
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/compact_configuration_client.py:287-359
Timestamp: 2025-07-08T18:40:24.408Z
Learning: In the CompactConnect codebase, landonshumway-ia prefers to avoid extraneous unit tests when existing test coverage is already sufficient to catch bugs. For the get_privilege_purchase_options method's live-jurisdiction filtering logic, the existing tests in the purchases test suite provide adequate coverage without needing additional edge case tests.

Learnt from: jusdino
PR: #864
File: backend/compact-connect/tests/smoke/encumbrance_smoke_tests.py:256-262
Timestamp: 2025-06-18T21:57:02.978Z
Learning: The licenseJurisdiction field is a required field in the provider data API response from the /v1/providers/users/me endpoint, so it can be accessed directly without defensive programming checks.

🧬 Code Graph Analysis (4)
backend/compact-connect/stacks/state_api_stack/v1_api/api_model.py (3)
backend/compact-connect/common_constructs/stack.py (1)
  • license_type_names (62-64)
backend/compact-connect/stacks/api_stack/v1_api/api_model.py (11)
  • ApiModel (13-2420)
  • message_response_model (21-39)
  • query_providers_request_model (69-114)
  • _pagination_request_schema (1480-1488)
  • _sorting_schema (1460-1477)
  • query_providers_response_model (117-138)
  • _providers_response_schema (943-963)
  • _pagination_response_schema (1491-1503)
  • provider_response_model (141-150)
  • _provider_detail_response_schema (966-1349)
  • _common_license_properties (1352-1374)
backend/compact-connect/common_constructs/cc_api.py (2)
  • CCApi (68-365)
  • message_response_model (272-282)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/provider/api.py (6)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/common.py (1)
  • CCRequestSchema (15-43)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/jurisdiction/__init__.py (3)
  • compact (58-59)
  • compact (108-109)
  • Jurisdiction (40-83)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/base_record.py (1)
  • ForgivingSchema (21-25)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/__init__.py (14)
  • providerId (24-25)
  • providerId (104-105)
  • compact (28-29)
  • compact (108-109)
  • jurisdiction (32-33)
  • jurisdiction (112-113)
  • licenseType (40-41)
  • licenseType (116-117)
  • privilegeId (64-65)
  • status (84-88)
  • dateOfExpiration (52-53)
  • dateOfIssuance (44-45)
  • dateOfRenewal (48-49)
  • licenseJurisdiction (36-37)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/provider/__init__.py (16)
  • providerId (25-26)
  • providerId (157-158)
  • compact (29-30)
  • compact (161-162)
  • compactEligibility (125-126)
  • dateOfExpiration (69-70)
  • familyName (61-62)
  • givenName (53-54)
  • licenseJurisdiction (33-34)
  • licenseStatus (129-130)
  • middleName (57-58)
  • suffix (65-66)
  • npi (49-50)
  • ssnLastFour (45-46)
  • compactConnectRegisteredEmailAddress (77-83)
  • dateOfBirth (73-74)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/fields.py (5)
  • Compact (47-49)
  • Jurisdiction (52-54)
  • ActiveInactive (57-59)
  • CompactEligibility (62-64)
  • NationalProviderIdentifier (42-44)
backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py (4)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/__init__.py (5)
  • jurisdiction (32-33)
  • jurisdiction (112-113)
  • licenseType (40-41)
  • licenseType (116-117)
  • PrivilegeUpdateData (91-155)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/license/__init__.py (5)
  • jurisdiction (36-37)
  • jurisdiction (168-169)
  • LicenseUpdateData (140-185)
  • licenseType (40-41)
  • licenseType (172-173)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/__init__.py (6)
  • jurisdiction (38-39)
  • jurisdiction (42-43)
  • licenseType (54-55)
  • licenseType (58-59)
  • licenseTypeAbbreviation (46-47)
  • licenseTypeAbbreviation (50-51)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/common.py (2)
  • to_dict (169-180)
  • licenseTypeAbbreviation (156-167)
backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_data_model/test_client.py (2)
backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py (2)
  • DataClient (50-2954)
  • get_providers_sorted_by_updated (261-314)
backend/compact-connect/lambdas/python/provider-data-v1/tests/function/__init__.py (1)
  • _generate_providers (246-387)
🔇 Additional comments (50)
backend/compact-connect/bin/trim_oas30.py (4)

4-6: LGTM! Appropriate imports added.

The addition of argparse and os modules is correct for implementing command-line argument parsing and dynamic path resolution.


34-39: LGTM! Well-implemented argument parsing.

The argument parser setup follows best practices with clear description and help text for the internal flag.


41-51: LGTM! Robust path resolution implementation.

The dynamic path computation using absolute paths and proper directory traversal is well-implemented and cross-platform compatible.


53-53: LGTM! Consistent file operations.

The use of the computed file_path for both reading and writing operations maintains the expected in-place trimming behavior.

Also applies to: 59-59

backend/compact-connect/bin/update_postman_collection.py (3)

10-10: LGTM! Consistent import addition.

The argparse import follows the same pattern as the trim_oas30.py script for consistent CLI argument handling.


195-200: LGTM! Consistent argument parsing implementation.

The argument parser setup matches the pattern used in trim_oas30.py, providing a consistent CLI experience across scripts.


207-212: LGTM! Well-structured path resolution.

The conditional path selection for both API specifications and Postman collections is clearly implemented and handles the internal vs regular directory structure appropriately.

backend/compact-connect/bin/download_oas30.py (5)

9-15: LGTM! Appropriate imports for AWS API operations.

The imports include all necessary modules for AWS API Gateway operations, with specific exception handling imports showing good practice.


36-48: LGTM! Well-implemented stages retrieval.

The function properly extracts stage names from the API Gateway response with appropriate error handling.


51-75: LGTM! Robust OpenAPI specification download.

The function properly handles the AWS API Gateway export with correct parameters and comprehensive error handling for both API calls and JSON parsing.


78-122: LGTM! Well-implemented file operations and URL updates.

Both functions are properly implemented with appropriate error handling. The URL mapping for beta domains appears correct for the development workflow.


124-189: LGTM! Well-structured main execution flow.

The main function and download orchestration are properly implemented with clear argument handling and appropriate output path management for both internal and external API specifications.

backend/compact-connect/bin/update_api_docs.sh (6)

1-35: LGTM! Solid script foundation with proper error handling.

The script setup includes appropriate error handling with set -e and well-organized utility functions for colored output and command validation.


37-69: LGTM! Comprehensive requirements validation.

The function properly checks for all required tools and AWS credentials, providing clear error messages and good user experience by reporting all missing requirements at once.


71-102: LGTM! Well-structured download and trimming workflow.

Both functions properly orchestrate the download and trimming processes with appropriate error handling and clear status reporting for both StateApi and LicenseApi.


104-123: LGTM! Consistent Postman collection update workflow.

The function properly handles updating both StateApi and LicenseApi Postman collections with appropriate flag usage and error handling.


125-144: LGTM! Thorough output file verification.

The function properly verifies all expected output files with clear error reporting, ensuring the workflow completed successfully.


146-185: LGTM! Well-orchestrated main execution flow.

The main function provides excellent user experience with clear progress indication, proper error handling, and a comprehensive summary of the workflow results.

backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py (3)

373-392: LGTM: Clean implementation for license update filtering.

The method correctly filters license update records by jurisdiction and license type, with proper typing and optional filtering support.


394-413: LGTM: Consistent implementation pattern.

The method follows the same filtering pattern as get_update_records_for_license, ensuring consistency across the codebase.


415-461: LGTM: Well-structured API response generation.

The method correctly assembles provider records into a comprehensive API response object using the typed data classes. The use of to_dict() and the new filtering methods ensures consistent data serialization.

backend/compact-connect/tests/resources/snapshots/QUERY_PROVIDERS_RESPONSE_SCHEMA.json (1)

361-361: LGTM: Schema property name update aligns with API response structure.

The change from "items" to "providers" makes the schema more descriptive and consistent with the actual API response format.

backend/compact-connect/stacks/state_api_stack/v1_api/api_model.py (14)

1-19: LGTM!

The imports and class structure are well-organized. The comment about protected access is appropriate for CDK usage patterns.


20-40: LGTM!

The message response model is correctly implemented with proper caching and schema definition.


41-74: LGTM!

The query providers request model is well-structured for time-based queries, appropriately different from the main API's provider query model.


75-98: Good - schema field name mismatch has been fixed.

The response model correctly defines 'providers' in both the required fields and properties, resolving the previous mismatch issue.


99-110: LGTM!

The provider response model correctly delegates to the detail response schema with proper caching.


111-139: Good - naming consistency has been fixed.

The bulk upload response model now follows the consistent 'v1' prefix pattern for cached attributes.


140-183: LGTM!

The post license model is well-structured with appropriate required fields and validation patterns. Good use of the stack's license type names for enum validation.


184-208: Good - phone number pattern uses centralized constant.

The common license properties are well-defined with appropriate validation patterns and constraints. The phone number field correctly uses cc_api.PHONE_NUMBER_FORMAT.


209-266: LGTM!

The providers response schema is comprehensive with appropriate required fields and validation patterns. Good use of context-based enums for compacts and jurisdictions.


267-284: LGTM!

The provider detail response schema is well-structured with appropriate required fields and URI format validation for the provider UI URL.


285-352: LGTM!

The state privilege schema effectively combines privilege and license data into a flattened structure. All required fields and validation patterns are appropriate for the state API use case.


353-366: LGTM!

The sorting schema is appropriately simplified for the state API, which always sorts by date of update. The direction-only structure makes sense for this fixed sorting behavior.


367-377: LGTM!

The pagination request schema has appropriate constraints for page size and key length.


378-392: LGTM!

The pagination response schema correctly allows null values for pagination keys and maintains consistent constraints.

backend/compact-connect/lambdas/python/provider-data-v1/handlers/state_api.py (4)

1-23: LGTM!

All imports are properly organized and used within the module.


25-83: LGTM!

The query endpoint correctly implements jurisdiction-specific filtering with proper authorization, input validation, and data sanitization. Good use of the general schema to ensure only public data is returned.


85-124: LGTM!

The flattening logic correctly handles field conflicts and uses dictionary unpacking to catch any unexpected duplicates. Good defensive programming with the None check for the email field.


126-201: LGTM!

The get provider endpoint is well-implemented with proper authorization, comprehensive error handling, and correct schema selection based on access levels. The flattening logic and license matching are robust.

backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_state_api.py (3)

1-12: LGTM!

Test setup with AWS mocking and datetime patching is appropriate for consistent test execution.


13-283: Excellent test coverage!

The query providers tests comprehensively cover success cases, validation errors, date filtering edge cases, and the 7-day limit enforcement. The helper method effectively generates test data with various privilege/license jurisdiction combinations.


284-544: Good - explicit assertions for private field absence have been added.

The get provider tests provide excellent coverage of permission-based data visibility, jurisdiction filtering, and error cases. The tests now explicitly verify that private fields are excluded when only general permissions are granted (lines 342-349).

backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_data_model/test_client.py (2)

174-191: LGTM! Comprehensive error validation test.

The test correctly validates that a RuntimeError is raised when jurisdiction is None but only_providers_with_privileges_in_jurisdiction is True, matching the validation logic in the DataClient.get_providers_sorted_by_updated method.


193-211: LGTM! Good validation of jurisdiction-specific privilege filtering.

The test properly validates that the method correctly filters providers with privileges in the specified jurisdiction. The assertion checking that all returned providers have 'ne' in their privilegeJurisdictions effectively verifies the filtering behavior.

backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/provider/api.py (5)

2-6: LGTM! Appropriate imports for new validation functionality.

The new imports support the custom validation logic in the QueryJurisdictionProvidersRequestSchema class.


174-218: Well-designed schema with appropriate validation.

The QueryJurisdictionProvidersRequestSchema effectively supports time-windowed queries with sensible constraints:

  • Required date-time range prevents unbounded queries
  • 7-day maximum window prevents performance issues
  • Nested schema structure follows established patterns

The omission of the key field in SortingSchema (compared to the regular QueryProvidersRequestSchema) suggests this API uses predefined sort keys, which is appropriate for state API consumption.


247-281: Excellent flattened schema design for state API consumption.

The StatePrivilegeGeneralResponseSchema provides a well-structured, flattened view that combines privilege and license data appropriately for external state IT systems:

  • Consistent field types and validation with existing schemas
  • Appropriate separation of required vs optional fields
  • Good use of custom field types (Jurisdiction, ActiveInactive, etc.)
  • The type field constraint to 'statePrivilege' ensures response consistency

283-304: Good extension pattern for private field handling.

The StatePrivilegePrivateResponseSchema appropriately extends the general schema with private fields:

  • Clean inheritance pattern maintains all general fields
  • Private field validations are appropriate (SSN format, international phone regex, address lengths)
  • Email field types ensure proper validation
  • The extension approach allows permission-based field visibility

306-333: Well-structured container schemas for state provider details.

The StateProviderDetailPrivateResponseSchema and StateProviderDetailGeneralResponseSchema provide clean top-level response structures:

  • Consistent separation pattern matching their nested privilege schemas
  • Required providerUIUrl field enables external systems to link back to the provider UI
  • Clear documentation explains their purpose for state API consumption
  • The container approach allows different permission levels while maintaining response structure consistency

Copy link
Copy Markdown
Collaborator

@landonshumway-ia landonshumway-ia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome. I just noticed a couple items that need clarification

Comment thread backend/compact-connect/common_constructs/cc_api.py
Comment thread backend/compact-connect/common_constructs/stack.py
Comment thread backend/compact-connect/upload.sh Outdated
Comment thread backend/compact-connect/lambdas/python/common/cc_common/data_model/data_client.py Outdated
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
backend/compact-connect/lambdas/python/common/tests/unit/test_data_model/test_schema/test_provider.py (1)

91-121: Excellent test for timestamp synchronization edge case.

This test effectively validates the correct ordering of dateOfUpdate and providerDateOfUpdate field updates during serialization, preventing time-based query failures. The implementation correctly uses mocking and follows good testing practices.

Consider a minor optimization to avoid redundant load operations:

         with patch('cc_common.config._Config.current_standard_datetime', new_date_of_update):
             loaded_record = schema.load(expected_provider_record.copy())
             # Verify we have the expected _old_ dateOfUpdate on load
             self.assertEqual(loaded_record['dateOfUpdate'], old_date_of_update)
 
-            dumped_record = schema.dump(schema.load(expected_provider_record.copy()))
+            dumped_record = schema.dump(loaded_record)
 
             self.assertEqual(new_date_of_update.isoformat(), dumped_record['dateOfUpdate'])
             # If 1 and 2 happened out of order, `providerDateOfUpdate` will be incorrect
             self.assertEqual(new_date_of_update.isoformat(), dumped_record['providerDateOfUpdate'])
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between abe596c and d6fd466.

📒 Files selected for processing (2)
  • backend/compact-connect/common_constructs/cc_api.py (5 hunks)
  • backend/compact-connect/lambdas/python/common/tests/unit/test_data_model/test_schema/test_provider.py (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • backend/compact-connect/common_constructs/cc_api.py
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: landonshumway-ia
PR: csg-org/CompactConnect#769
File: backend/compact-connect/lambdas/python/provider-data-v1/handlers/encumbrance.py:53-60
Timestamp: 2025-04-29T02:10:38.400Z
Learning: Lambda functions in the provider-data-v1 module that are exposed via API Gateway endpoints (like encumbrance_handler) should only be called through the API, not directly. Therefore, they don't need to handle various body formats as the API Gateway will consistently provide event['body'] as a JSON string.
backend/compact-connect/lambdas/python/common/tests/unit/test_data_model/test_schema/test_provider.py (6)

Learnt from: landonshumway-ia
PR: #769
File: backend/compact-connect/lambdas/python/common/tests/resources/dynamo/provider.json:5-5
Timestamp: 2025-04-29T01:57:43.684Z
Learning: The provider.json test data contains both "providerDateOfUpdate" and "dateOfUpdate" fields which serve different purposes, and both should be maintained in the test files.

Learnt from: landonshumway-ia
PR: #769
File: backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_encumbrance.py:38-40
Timestamp: 2025-04-29T21:14:36.205Z
Learning: In the CompactConnect codebase, cc_common.config._Config.current_standard_datetime is a property rather than a method, and should be patched in tests by directly providing a datetime value: @patch('cc_common.config._Config.current_standard_datetime', datetime.fromisoformat(DEFAULT_DATE_OF_UPDATE_TIMESTAMP)).

Learnt from: landonshumway-ia
PR: #769
File: backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_encumbrance.py:38-40
Timestamp: 2025-04-29T21:14:36.205Z
Learning: In the CompactConnect codebase, cc_common.config._Config.current_standard_datetime is implemented as a property that returns datetime.now(tz=UTC).replace(microsecond=0). The correct way to patch it in tests is by directly providing the datetime value, not wrapping it in a lambda: @patch('cc_common.config._Config.current_standard_datetime', datetime.fromisoformat(DEFAULT_DATE_OF_UPDATE_TIMESTAMP)).

Learnt from: landonshumway-ia
PR: #769
File: backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_encumbrance.py:38-40
Timestamp: 2025-04-29T21:14:36.205Z
Learning: In the CompactConnect codebase, cc_common.config._Config.current_standard_datetime is a property (not a method), and can be properly patched in tests by directly supplying a datetime value rather than wrapping it in a lambda: @patch('cc_common.config._Config.current_standard_datetime', datetime.fromisoformat(DEFAULT_DATE_OF_UPDATE_TIMESTAMP)).

Learnt from: landonshumway-ia
PR: #796
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py:58-64
Timestamp: 2025-06-03T16:41:07.757Z
Learning: The static get_provider_record method in ProviderRecordUtility is legacy code being phased out in favor of the ProviderUserRecords class. During this migration, they are keeping the static method backwards compatible (returning None) while the new ProviderUserRecords class implements the better pattern of raising exceptions.

Learnt from: landonshumway-ia
PR: #769
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/common.py:87-90
Timestamp: 2025-04-29T21:09:04.842Z
Learning: In the CCDataClass implementation, validation is performed during both the dump() and load() processes in Marshmallow, so calling dump() first still results in ValidationError if the provided data is invalid.

🧬 Code Graph Analysis (1)
backend/compact-connect/lambdas/python/common/tests/unit/test_data_model/test_schema/test_provider.py (1)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/provider/record.py (1)
  • ProviderRecordSchema (32-178)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: TestPython
🔇 Additional comments (1)
backend/compact-connect/lambdas/python/common/tests/unit/test_data_model/test_schema/test_provider.py (1)

2-3: LGTM!

The new imports are appropriate and necessary for the datetime manipulation and mocking functionality in the new test.

Copy link
Copy Markdown
Collaborator

@landonshumway-ia landonshumway-ia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks for the test

@jusdino jusdino requested a review from jlkravitz July 31, 2025 16:31
@jusdino
Copy link
Copy Markdown
Contributor Author

jusdino commented Jul 31, 2025

@jlkravitz this one is ready for you!

Copy link
Copy Markdown
Collaborator

@jlkravitz jlkravitz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

few questions!

Comment thread backend/compact-connect/bin/download_oas30.py
Comment thread backend/compact-connect/stacks/api_stack/v1_api/attestations.py Outdated
Comment thread backend/compact-connect/stacks/state_api_stack/v1_api/api.py
Copy link
Copy Markdown
Collaborator

@jlkravitz jlkravitz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@isabeleliassen good to merge!

@isabeleliassen isabeleliassen merged commit a9d5b20 into csg-org:development Aug 1, 2025
3 checks passed
@jlkravitz jlkravitz mentioned this pull request Aug 25, 2025
@jlkravitz jlkravitz mentioned this pull request Aug 25, 2025
15 tasks
isabeleliassen pushed a commit that referenced this pull request Aug 28, 2025
#962 moved the API spec, this fixes it and attempts to use a new job
introduced in ZAP's automation framework that supports updating the exit
codes: zaproxy/action-af#12

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- New Features
- Expanded automated security scanning to cover additional APIs and
environments.
- Tests
- Updated OpenAPI-based scans with correct targets and contexts for more
accurate coverage.
- Introduced explicit exit handling for scan jobs to standardize CI
outcomes.
- Chores
- Simplified manual security scan script by removing an unnecessary
pre-scan copy step.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

“http://…” automatically redirects to “https://…” States and compacts can opt in to daily transaction report

4 participants