-
Notifications
You must be signed in to change notification settings - Fork 5
doc/IT Staff machine to machine instruction document #777
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
doc/IT Staff machine to machine instruction document #777
Conversation
WalkthroughThe changes introduce comprehensive onboarding documentation for state IT staff integrating with the CompactConnect license upload API, including new instructions and clarifications on credential handling, authentication, and API usage. The API schema for license uploads is updated to require a provider's SSN, and internal code is refactored to expose new attributes for better testability and maintainability. Extensive tests are added to verify the API Gateway configuration, and new JSON schema snapshots are introduced to validate request and response structures. Documentation for app client setup is also clarified, especially regarding secure credential transmission and onboarding instructions. Changes
Sequence Diagram(s)sequenceDiagram
participant IT_Staff as State IT Staff
participant App as CompactConnect API Client
participant Auth as Cognito Auth Server
participant API as CompactConnect API
IT_Staff->>App: Obtain client ID and secret (one-time, secure channel)
App->>Auth: Request access token (client credentials grant)
Auth-->>App: Return access token (valid 15 min)
App->>API: POST /v1/compacts/{compact}/jurisdictions/{jurisdiction}/licenses (with token)
API-->>App: Return response (success or error)
Assessment against linked issues
Suggested reviewers
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed 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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this 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 (5)
backend/compact-connect/app_clients/README.md (2)
77-84
: Improved secure credential transmission instructions.The updated instructions provide a clearer, more secure method for transmitting credentials. A minor grammar improvement is needed.
-These credentials should be securely transmitted to the consuming team via an encrypted channel (ie a one time use link) in the following format: +These credentials should be securely transmitted to the consuming team via an encrypted channel (i.e., a one-time use link) in the following format:🧰 Tools
🪛 LanguageTool
[uncategorized] ~77-~77: When ‘one-time’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...ing team via an encrypted channel (ie a one time use link) in the following format: ...(ONE_TIME_HYPHEN)
87-89
: Helpful reference to onboarding instructions.Including the onboarding instructions document provides valuable context for the consuming team. The markdown link format needs to be corrected.
- As part of the email message sent to the consuming team, be sure to attach [the onboarding instructions to the email]("./Compact Connect Automated License Upload Instructions.txt"). + As part of the email message sent to the consuming team, be sure to attach the onboarding instructions document ("Compact Connect Automated License Upload Instructions.txt").Also, this heading should start at the beginning of the line to follow proper Markdown formatting:
- #### Email Instructions for consuming team +#### Email Instructions for consuming team🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
87-87: Headings must start at the beginning of the line
null(MD023, heading-start-left)
backend/compact-connect/tests/app/test_api/test_post_licenses.py (1)
64-84
: Consider disabling automatic snapshot overwriting for tests.While snapshot testing is excellent for verifying schema consistency, setting
overwrite_snapshot=True
will automatically update the snapshots if they don't match. In a production environment, this could mask potential issues.- overwrite_snapshot=True, + overwrite_snapshot=False,backend/compact-connect/app_clients/Compact Connect Automated License Upload Instructions.txt (2)
42-43
: Minor grammatical improvements needed.There are a few grammatical issues in this paragraph.
- Likewise, if these credentials are ever accidentally shared or compromised please inform the CompactConnect team - as soon as possible so the credentials can be deactivated and regenerated to prevent abuse to the system. + Likewise, if these credentials are ever accidentally shared or compromised, please inform the CompactConnect team + as soon as possible, so the credentials can be deactivated and regenerated to prevent abuse of the system.🧰 Tools
🪛 LanguageTool
[typographical] ~42-~42: It seems that a comma is missing.
Context: ... ever accidentally shared or compromised please inform the CompactConnect team
as soo...(IF_PLEASE_COMMA)
[uncategorized] ~43-~43: Use a comma before ‘so’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...ompactConnect team
as soon as possible so the credentials can be deactivated and ...(COMMA_COMPOUND_SENTENCE_2)
[uncategorized] ~43-~43: The preposition “of” seems more likely in this position.
Context: ...ivated and regenerated to prevent abuse to the system.The credentials should...
(AI_EN_LECTOR_REPLACEMENT_PREPOSITION)
[uncategorized] ~43-~43: A period might be missing here.
Context: ...and regenerated to prevent abuse to the system.The credentials should be sent to...
(AI_EN_LECTOR_MISSING_PUNCTUATION_PERIOD)
79-84
: Inconsistency between instructions and example.The instructions mention replacing
<authUrl>
but there's no such parameter in the curl example on lines 68-74.Either remove the reference to
<authUrl>
in the replacement instructions or add it to the curl command if needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
backend/compact-connect/app_clients/Compact Connect Automated License Upload Instructions.txt
(1 hunks)backend/compact-connect/app_clients/README.md
(2 hunks)backend/compact-connect/stacks/api_stack/v1_api/api.py
(1 hunks)backend/compact-connect/stacks/api_stack/v1_api/api_model.py
(2 hunks)backend/compact-connect/stacks/api_stack/v1_api/post_licenses.py
(2 hunks)backend/compact-connect/tests/app/test_api/test_post_licenses.py
(1 hunks)backend/compact-connect/tests/resources/snapshots/POST_LICENSES_REQUEST_SCHEMA.json
(1 hunks)backend/compact-connect/tests/resources/snapshots/POST_LICENSES_RESPONSE_SCHEMA.json
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
backend/compact-connect/stacks/api_stack/v1_api/api.py (2)
backend/compact-connect/lambdas/python/provider-data-v1/handlers/licenses.py (1)
post_licenses
(15-50)backend/compact-connect/stacks/api_stack/v1_api/post_licenses.py (1)
PostLicenses
(19-90)
backend/compact-connect/tests/app/test_api/test_post_licenses.py (3)
backend/compact-connect/tests/app/base.py (2)
get_resource_properties_by_logical_id
(65-72)compare_snapshot
(393-420)backend/compact-connect/lambdas/python/provider-data-v1/handlers/licenses.py (1)
post_licenses
(15-50)backend/compact-connect/stacks/api_stack/cc_api.py (1)
staff_users_authorizer
(247-253)
🪛 LanguageTool
backend/compact-connect/app_clients/README.md
[uncategorized] ~77-~77: When ‘one-time’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...ing team via an encrypted channel (ie a one time use link) in the following format: ...
(ONE_TIME_HYPHEN)
backend/compact-connect/app_clients/Compact Connect Automated License Upload Instructions.txt
[typographical] ~42-~42: It seems that a comma is missing.
Context: ... ever accidentally shared or compromised please inform the CompactConnect team
as soo...
(IF_PLEASE_COMMA)
[uncategorized] ~43-~43: Use a comma before ‘so’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...ompactConnect team
as soon as possible so the credentials can be deactivated and ...
(COMMA_COMPOUND_SENTENCE_2)
[uncategorized] ~43-~43: The preposition “of” seems more likely in this position.
Context: ...ivated and regenerated to prevent abuse to the system.
The credentials should...
(AI_EN_LECTOR_REPLACEMENT_PREPOSITION)
[uncategorized] ~43-~43: A period might be missing here.
Context: ...and regenerated to prevent abuse to the system.
The credentials should be sent to...
(AI_EN_LECTOR_MISSING_PUNCTUATION_PERIOD)
[uncategorized] ~101-~101: This verb does not appear to agree with the subject. Consider using a different form.
Context: ...uest a new token before the current one expires
- Store the
access_token
value for ...
(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
[uncategorized] ~179-~179: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...d strings use double quotes, not single quotes
* Verify that your payload is a va...
(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
[uncategorized] ~183-~183: The preposition ‘to’ seems more likely in this position.
Context: ...ken might have expired - generate a new one
* Make sure you're including the "...
(AI_HYDRA_LEO_REPLACE_ON_TO)
[uncategorized] ~194-~194: This verb does not appear to agree with the subject. Consider using a different form.
Context: ... get a new token before the current one expires
2. Implement error handling for API r...
(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
🪛 markdownlint-cli2 (0.17.2)
backend/compact-connect/app_clients/README.md
87-87: Headings must start at the beginning of the line
null
(MD023, heading-start-left)
🔇 Additional comments (12)
backend/compact-connect/tests/resources/snapshots/POST_LICENSES_RESPONSE_SCHEMA.json (1)
1-13
: Well-structured schema definition with clear constraints.This new schema file properly defines the expected structure for the license upload API response, requiring only a "message" string property and disallowing additional properties.
backend/compact-connect/stacks/api_stack/v1_api/api.py (1)
167-167
: Good refactoring to expose PostLicenses as a class attribute.Making the PostLicenses instance accessible as an attribute of the V1Api object improves maintainability and makes the handling of POST licenses more consistent with other endpoints. This enables proper integration with the API and provides clearer resource management.
backend/compact-connect/app_clients/README.md (1)
23-23
: Clearer guidance on scope provisioning.Adding conditional language here helps prevent unnecessary scope assignment, following the principle of least privilege.
backend/compact-connect/stacks/api_stack/v1_api/api_model.py (2)
158-162
: Critical fix: Restored required 'ssn' field to POST license validation schema.This change correctly restores the 'ssn' field to the license request model schema, fixing the issue described in the PR where it was accidentally removed during updates related to the license status field. The field is properly defined with an appropriate description and pattern validation using the SSN_FORMAT pattern.
852-852
: Minor style improvement with trailing comma.Adding the trailing comma after the last item in the properties object follows good practice for maintaining consistent code style and makes future additions cleaner.
backend/compact-connect/tests/resources/snapshots/POST_LICENSES_REQUEST_SCHEMA.json (1)
1-145
: LGTM! Schema structure is well-defined and includes the required SSN field.The JSON schema appropriately defines the validation rules for license request objects with:
- Required fields including SSN (which was restored as mentioned in the PR objectives)
- Proper patterns for validation (SSN, dates, NPI, phone, etc.)
- Appropriate field constraints and enumerated values
This schema will ensure data integrity for license submissions.
backend/compact-connect/stacks/api_stack/v1_api/post_licenses.py (2)
45-48
: Good refactoring to improve testability and access.Extracting the handler creation to a separate variable improves the code structure and makes the handler accessible for testing.
50-61
: Maintaining endpoint reference improves API traceability.Storing the endpoint in an instance variable makes it accessible for testing and reference in other parts of the codebase, which aligns well with the test improvements mentioned in the PR.
backend/compact-connect/tests/app/test_api/test_post_licenses.py (2)
22-34
: Effective test for verifying the Lambda handler configuration.This test ensures the POST licenses endpoint correctly uses the expected Lambda handler.
35-62
: Comprehensive validation of API method configuration.This test properly verifies all critical aspects of the API configuration including authorization, integration, and response models.
backend/compact-connect/app_clients/Compact Connect Automated License Upload Instructions.txt (2)
1-203
: Comprehensive documentation for state IT staff integration.The instructions provide clear guidance on authentication, license data uploads, and troubleshooting. This aligns perfectly with the PR objective to provide instructions for State IT staff.
🧰 Tools
🪛 LanguageTool
[typographical] ~42-~42: It seems that a comma is missing.
Context: ... ever accidentally shared or compromised please inform the CompactConnect team
as soo...(IF_PLEASE_COMMA)
[uncategorized] ~43-~43: Use a comma before ‘so’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...ompactConnect team
as soon as possible so the credentials can be deactivated and ...(COMMA_COMPOUND_SENTENCE_2)
[uncategorized] ~43-~43: The preposition “of” seems more likely in this position.
Context: ...ivated and regenerated to prevent abuse to the system.The credentials should...
(AI_EN_LECTOR_REPLACEMENT_PREPOSITION)
[uncategorized] ~43-~43: A period might be missing here.
Context: ...and regenerated to prevent abuse to the system.The credentials should be sent to...
(AI_EN_LECTOR_MISSING_PUNCTUATION_PERIOD)
[uncategorized] ~101-~101: This verb does not appear to agree with the subject. Consider using a different form.
Context: ...uest a new token before the current one expires
- Store the
access_token
value for ...(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
[uncategorized] ~179-~179: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...d strings use double quotes, not single quotes
* Verify that your payload is a va...(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
[uncategorized] ~183-~183: The preposition ‘to’ seems more likely in this position.
Context: ...ken might have expired - generate a new one
* Make sure you're including the "...(AI_HYDRA_LEO_REPLACE_ON_TO)
[uncategorized] ~194-~194: This verb does not appear to agree with the subject. Consider using a different form.
Context: ... get a new token before the current one expires
2. Implement error handling for API r...(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
114-124
:✅ Verification successful
Verify curl example payload aligns with schema requirements.
The example payload includes the SSN field that was restored in this PR, which is good. However, ensure that the example data fully aligns with any constraints like regex patterns defined in the schema.
🏁 Script executed:
#!/bin/bash # Validate if the example payload in the documentation matches the schema requirements # Extract the example JSON payload from the documentation EXAMPLE_JSON=$(grep -A10 "\-\-data '\[{" backend/compact-connect/app_clients/Compact\ Connect\ Automated\ License\ Upload\ Instructions.txt | grep -v "\-\-data" | head -n 1) # Print the extracted payload for inspection echo "Extracted example payload: $EXAMPLE_JSON" # Check if all required fields from the schema are present in the example REQUIRED_FIELDS=("ssn" "givenName" "familyName" "dateOfBirth" "homeAddressStreet1" "homeAddressCity" "homeAddressState" "homeAddressPostalCode" "licenseType" "dateOfIssuance" "dateOfRenewal" "dateOfExpiration" "licenseStatus" "compactEligibility") for field in "${REQUIRED_FIELDS[@]}"; do if ! echo "$EXAMPLE_JSON" | grep -q "\"$field\":"; then echo "MISSING REQUIRED FIELD: $field is not present in the example" fi done # Check SSN format in the example against the schema pattern SSN_PATTERN=$(grep -A4 "\"ssn\":" backend/compact-connect/tests/resources/snapshots/POST_LICENSES_REQUEST_SCHEMA.json | grep "pattern" | sed 's/.*"pattern": "\(.*\)",/\1/') echo "SSN pattern from schema: $SSN_PATTERN" SSN_EXAMPLE=$(echo "$EXAMPLE_JSON" | grep -o "\"ssn\":\"[^\"]*\"" | sed 's/"ssn":"//;s/"//') echo "SSN from example: $SSN_EXAMPLE" if [[ "$SSN_EXAMPLE" =~ ^[0-9]{3}-[0-9]{2}-[0-9]{4}$ ]]; then echo "SSN format in example matches schema pattern" else echo "SSN format mismatch: Example SSN doesn't conform to schema pattern" fiLength of output: 4322
I’ll pull the exact payload from the docs to verify it against the schema.
🏁 Script executed:
#!/bin/bash set -e # 1. Locate the instructions file FILE=$(fd 'Compact Connect Automated License Upload Instructions.txt$' backend/compact-connect/app_clients) echo "Found instructions file at: $FILE" # 2. Find the line number containing the --data flag DATA_LINE=$(grep -n -- "--data" "$FILE" | cut -d: -f1) echo "‘--data’ occurs on line: $DATA_LINE" # 3. Extract that line plus the following 10 lines to capture the full JSON payload echo "---- Payload snippet ----" sed -n "${DATA_LINE},$((DATA_LINE+10))p" "$FILE" echo "-------------------------"Length of output: 988
Let’s grab the JSON at the final
--data
block (line 117) by picking only the last match:
🏁 Script executed:
#!/bin/bash set -e # 1. Locate the instructions file FILE=$(fd -t f 'Compact Connect Automated License Upload Instructions.txt$' backend/compact-connect/app_clients) echo "Instructions file: $FILE" # 2. Get the last occurrence of --data LAST_LINE=$(grep -n -- "--data" "$FILE" | tail -n1 | cut -d: -f1) echo "Last --data at line: $LAST_LINE" # 3. Extract that line plus the next 10 lines, stripping CR echo "---- Payload snippet ----" sed -n "${LAST_LINE},$((LAST_LINE+10))p" "$FILE" | sed 's/\r$//' echo "-------------------------"Length of output: 1969
Example payload complies with schema requirements.
I’ve verified the JSON snippet against the POST /licenses schema:
- All required properties are present.
- SSN, dates, state code, postal code, phone number, and NPI all match their regex patterns.
- Enumerated fields (licenseStatus, compactEligibility, licenseType) use valid values.
No changes needed.
There was a problem hiding this 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 (4)
backend/compact-connect/app_clients/Compact Connect Automated License Upload Instructions.txt (4)
98-102
: Improve formatting of bullet points in the "Important Notes" sectionThe bullet points in this section are all on the same line which makes the content harder to read. Consider formatting each point on a new line for better readability.
-Important Notes: -* For security reasons, the access token is valid for 15 minutes from the time it is generated (900 seconds) -* Your application should request a new token before the current one expires -* Store the `access_token` value for use in API requests +Important Notes: +* For security reasons, the access token is valid for 15 minutes from the time it is generated (900 seconds) + +* Your application should request a new token before the current one expires + +* Store the `access_token` value for use in API requests🧰 Tools
🪛 LanguageTool
[uncategorized] ~100-~100: This verb does not appear to agree with the subject. Consider using a different form.
Context: ...uest a new token before the current one expires
- Store the
access_token
value for ...(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
174-190
: Improve formatting consistency in the "Troubleshooting Common Issues" sectionThe formatting in this section is inconsistent. Sub-bullets are on the same line as the main bullet point, which affects readability. Also, in item #2, consider using "to" instead of a hyphen for better grammar.
Troubleshooting Common Issues 1. "Unknown error parsing request body" - * Ensure your JSON data is properly formatted with no trailing commas - * Check that all quoted strings use double quotes, not single quotes - * Verify that your payload is a valid JSON array, even for a single license record + * Ensure your JSON data is properly formatted with no trailing commas + * Check that all quoted strings use double quotes, not single quotes + * Verify that your payload is a valid JSON array, even for a single license record 2. Authentication errors (401) - * Your access token might have expired - generate a new one - * Make sure you're including the "Bearer" prefix before the token in the Authorization header + * Your access token might have expired to generate a new one + * Make sure you're including the "Bearer" prefix before the token in the Authorization header 3. Validation errors (400) - * Check the error response for specific validation issues - * Ensure all required fields are present and formatted correctly - * Verify that licenseType matches exactly one of the valid types for the compact + * Check the error response for specific validation issues + * Ensure all required fields are present and formatted correctly + * Verify that licenseType matches exactly one of the valid types for the compact🧰 Tools
🪛 LanguageTool
[uncategorized] ~178-~178: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...d strings use double quotes, not single quotes
* Verify that your payload is a va...(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
[uncategorized] ~182-~182: The preposition ‘to’ seems more likely in this position.
Context: ...ken might have expired - generate a new one
* Make sure you're including the "...(AI_HYDRA_LEO_REPLACE_ON_TO)
191-197
: Use consistent formatting for implementation recommendationsThe formatting style in this section changes from bullet points to numbered items. Consider using consistent formatting throughout the document.
Implementation Recommendations: -1. Implement token refresh logic to get a new token before the current one expires -2. Implement error handling for API responses -3. Configure your application to securely store and access the client credentials, do not store the credentials in your -application code. +* Implement token refresh logic to get a new token before the current one expires + +* Implement error handling for API responses + +* Configure your application to securely store and access the client credentials, do not store the credentials in your +application code.🧰 Tools
🪛 LanguageTool
[uncategorized] ~193-~193: This verb does not appear to agree with the subject. Consider using a different form.
Context: ... get a new token before the current one expires
2. Implement error handling for API r...(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
113-117
: Improve curl command formatting for readabilityThe curl command example is currently on a single line with multiple parameters. Consider formatting it with line breaks for better readability.
-curl --location --request POST 'https://api.beta.compactconnect.org/v1/compacts/<compact>/jurisdictions/<jurisdiction>/licenses' \ ---header 'Authorization: Bearer <access_token>' \ ---header 'Content-Type: application/json' \ ---data '[{"ssn":"123-45-6789","licenseNumber":"LIC123456","licenseStatusName":"Active","licenseStatus":"active","compactEligibility":"eligible","licenseType":"audiologist","givenName":"Jane","middleName":"Marie","familyName":"Smith","dateOfIssuance":"2023-01-15","dateOfRenewal":"2023-01-15","dateOfExpiration":"2025-01-14","dateOfBirth":"1980-05-20","homeAddressStreet1":"123 Main Street","homeAddressStreet2":"Apt 4B","homeAddressCity":"Louisville","homeAddressState":"KY","homeAddressPostalCode":"40202","emailAddress":"[email protected]","phoneNumber":"+15555551234","npi":"1234567890"}]' +curl --location --request POST 'https://api.beta.compactconnect.org/v1/compacts/<compact>/jurisdictions/<jurisdiction>/licenses' \ +--header 'Authorization: Bearer <access_token>' \ +--header 'Content-Type: application/json' \ +--data '[ + { + "ssn":"123-45-6789", + "licenseNumber":"LIC123456", + "licenseStatusName":"Active", + "licenseStatus":"active", + "compactEligibility":"eligible", + "licenseType":"audiologist", + "givenName":"Jane", + "middleName":"Marie", + "familyName":"Smith", + "dateOfIssuance":"2023-01-15", + "dateOfRenewal":"2023-01-15", + "dateOfExpiration":"2025-01-14", + "dateOfBirth":"1980-05-20", + "homeAddressStreet1":"123 Main Street", + "homeAddressStreet2":"Apt 4B", + "homeAddressCity":"Louisville", + "homeAddressState":"KY", + "homeAddressPostalCode":"40202", + "emailAddress":"[email protected]", + "phoneNumber":"+15555551234", + "npi":"1234567890" + } +]'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
backend/compact-connect/app_clients/Compact Connect Automated License Upload Instructions.txt
(1 hunks)backend/compact-connect/app_clients/README.md
(2 hunks)backend/compact-connect/tests/app/test_api/test_post_licenses.py
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/compact-connect/app_clients/README.md
🧰 Additional context used
🪛 LanguageTool
backend/compact-connect/app_clients/Compact Connect Automated License Upload Instructions.txt
[uncategorized] ~100-~100: This verb does not appear to agree with the subject. Consider using a different form.
Context: ...uest a new token before the current one expires
- Store the
access_token
value for ...
(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
[uncategorized] ~178-~178: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...d strings use double quotes, not single quotes
* Verify that your payload is a va...
(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
[uncategorized] ~182-~182: The preposition ‘to’ seems more likely in this position.
Context: ...ken might have expired - generate a new one
* Make sure you're including the "...
(AI_HYDRA_LEO_REPLACE_ON_TO)
[uncategorized] ~193-~193: This verb does not appear to agree with the subject. Consider using a different form.
Context: ... get a new token before the current one expires
2. Implement error handling for API r...
(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: TestPython
🔇 Additional comments (2)
backend/compact-connect/tests/app/test_api/test_post_licenses.py (1)
1-85
: Well-structured test implementation for the POST licenses endpointThis test class effectively validates the API configuration for the POST licenses endpoint, ensuring that the Lambda handler, authorizer, integration, and request/response models are correctly configured. The snapshot comparison approach for schema validation provides good protection against unintentional schema changes.
backend/compact-connect/app_clients/Compact Connect Automated License Upload Instructions.txt (1)
1-202
: Comprehensive documentation for IT staff integrationThe documentation provides clear instructions for state IT departments to integrate with the CompactConnect platform. It covers all necessary aspects including authentication, data uploads, schema requirements, verification, and troubleshooting. The document effectively supports the PR objective of providing machine-to-machine authentication instructions and examples.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~100-~100: This verb does not appear to agree with the subject. Consider using a different form.
Context: ...uest a new token before the current one expires
- Store the
access_token
value for ...(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
[uncategorized] ~178-~178: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...d strings use double quotes, not single quotes
* Verify that your payload is a va...(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
[uncategorized] ~182-~182: The preposition ‘to’ seems more likely in this position.
Context: ...ken might have expired - generate a new one
* Make sure you're including the "...(AI_HYDRA_LEO_REPLACE_ON_TO)
[uncategorized] ~193-~193: This verb does not appear to agree with the subject. Consider using a different form.
Context: ... get a new token before the current one expires
2. Implement error handling for API r...(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! One minor suggestion.
backend/compact-connect/app_clients/Compact Connect Automated License Upload Instructions.txt
Outdated
Show resolved
Hide resolved
@jlkravitz This is ready for your review. Thanks |
There was a problem hiding this 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 (2)
backend/compact-connect/app_clients/README.md (2)
62-63
: Fix grammar and plurality for AWS account reference
Add a comma after "app clients" and change "AWS accounts" to singular "AWS account" to improve readability and correctness.Apply this diff:
- If the consuming team plans to use both the beta and production environments, you will need to create two separate app clients one in each respective AWS accounts for those environments. + If the consuming team plans to use both the beta and production environments, you will need to create two separate app clients, one in each respective AWS account for those environments.🧰 Tools
🪛 LanguageTool
[uncategorized] ~62-~62: Possible missing comma found.
Context: ...ou will need to create two separate app clients one in each respective AWS accounts for...(AI_HYDRA_LEO_MISSING_COMMA)
80-86
: Improve JSON snippet formatting
Indent the JSON payload for better readability in documentation.Apply this diff:
- ```json - { - "clientId": "<client id>", - "clientSecret": "<client secret>" - } - ``` + ```json + { + "clientId": "<client id>", + "clientSecret": "<client secret>" + } + ```
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/compact-connect/app_clients/README.md
(3 hunks)
🧰 Additional context used
🪛 LanguageTool
backend/compact-connect/app_clients/README.md
[uncategorized] ~62-~62: Possible missing comma found.
Context: ...ou will need to create two separate app clients one in each respective AWS accounts for...
(AI_HYDRA_LEO_MISSING_COMMA)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: TestPython
🔇 Additional comments (3)
backend/compact-connect/app_clients/README.md (3)
23-23
: Clarify compact-level scope recommendation
The updated guidance accurately emphasizes that{compact}/readGeneral
should be the primary compact-level scope when needed, which improves security posture by minimizing overly broad permissions.
64-65
: Environment-specific token URLs are clear
The explicit inclusion of separate Cognito token URLs for beta and prod enhances clarity for deployment.
89-91
: Reinforce attachment instruction
The new section clearly instructs attaching the onboarding document to the email. This ensures consuming teams receive all necessary context.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all small nits but generally looks great and is well-written!
backend/compact-connect/app_clients/Compact Connect Automated License Upload Instructions.txt
Outdated
Show resolved
Hide resolved
backend/compact-connect/app_clients/Compact Connect Automated License Upload Instructions.txt
Outdated
Show resolved
Hide resolved
backend/compact-connect/app_clients/Compact Connect Automated License Upload Instructions.txt
Outdated
Show resolved
Hide resolved
backend/compact-connect/app_clients/Compact Connect Automated License Upload Instructions.txt
Outdated
Show resolved
Hide resolved
backend/compact-connect/app_clients/Compact Connect Automated License Upload Instructions.txt
Outdated
Show resolved
Hide resolved
There was a problem hiding this 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 (4)
backend/compact-connect/app_clients/it_staff_onboarding_instructions/README.md (4)
70-76
: Unify unordered list style across the documentThe file currently mixes asterisks (
*
) and dashes (-
) for unordered lists, triggering markdownlint errors. To maintain consistency and adhere to our style guide, please use dashes (-
) for all bullets. For example, in the “Replace:” section:- * `<clientId>` with your client ID - * `<clientSecret>` with your client secret + - `<clientId>` with your client ID + - `<clientSecret>` with your client secretApply the same change to all other asterisk-prefixed lists.
Also applies to: 90-92, 110-113, 129-131, 149-152, 164-166, 169-170, 173-175
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
71-71: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
72-72: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
73-73: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
74-74: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
87-87
: Convert bare URLs into markdown linksMarkdownlint flagged bare URLs at lines 87, 121, and 126 (MD034). Please replace them with link syntax for better readability and to avoid lint errors. For example:
For more information about this authentication process, please see the following [AWS Cognito Token Endpoint documentation](https://docs.aws.amazon.com/cognito/latest/developerguide/token-endpoint.html).Also applies to: 121-121, 126-126
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
87-87: Bare URL used
null(MD034, no-bare-urls)
80-80
: Clarify that the example access token is a placeholderThe
"access_token"
value shown resembles a real token, which could confuse readers or trigger security scans. Please replace it with a clearly fictitious placeholder such as:"access_token": "<REDACTED_ACCESS_TOKEN>"Or add a note indicating “(example only—not a real token)”.
🧰 Tools
🪛 Gitleaks (8.21.2)
80-80: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
181-183
: Split the lengthy recommendation into two distinct stepsPoint 3 under “Implementation Recommendations” combines two ideas in one item. For clarity and consistency with the other numbered steps, consider splitting it:
- 3. Configure your application to securely store and access the client credentials, do not store the credentials in your application code. + 3. Configure your application to securely store and access the client credentials. + 4. Ensure that credentials are never hardcoded or committed to source control.This makes each recommendation concise and actionable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/compact-connect/app_clients/it_staff_onboarding_instructions/README.md
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
backend/compact-connect/app_clients/it_staff_onboarding_instructions/README.md
[style] ~47-~47: Did you mean “that” or “date”?
Context: ...ntication Process for Uploading License Data Follow these steps to obtain an acces...
(DAT)
[uncategorized] ~96-~96: Possible missing comma found.
Context: ...e API can be called through a POST REST endpoint which takes in a list of license record...
(AI_HYDRA_LEO_MISSING_COMMA)
🪛 Gitleaks (8.21.2)
backend/compact-connect/app_clients/it_staff_onboarding_instructions/README.md
80-80: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 markdownlint-cli2 (0.17.2)
backend/compact-connect/app_clients/it_staff_onboarding_instructions/README.md
71-71: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
72-72: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
73-73: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
74-74: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
87-87: Bare URL used
null
(MD034, no-bare-urls)
90-90: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
91-91: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
92-92: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
110-110: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
111-111: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
112-112: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
113-113: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
121-121: Bare URL used
null
(MD034, no-bare-urls)
126-126: Bare URL used
null
(MD034, no-bare-urls)
129-129: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
130-130: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
131-131: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
149-149: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
150-150: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
151-151: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
152-152: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
164-164: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
165-165: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
166-166: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
169-169: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
170-170: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
173-173: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
174-174: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
175-175: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: TestPython
🔇 Additional comments (1)
backend/compact-connect/app_clients/it_staff_onboarding_instructions/README.md (1)
1-188
: Comprehensive and clear documentation—excellent work!The onboarding guide thoroughly covers authentication, API usage, schema requirements, error handling, and troubleshooting. It provides actionable curl examples, detailed field explanations, and implementation recommendations, which will greatly streamline the integration process for state IT staff.
🧰 Tools
🪛 LanguageTool
[style] ~47-~47: Did you mean “that” or “date”?
Context: ...ntication Process for Uploading License Data Follow these steps to obtain an acces...(DAT)
[uncategorized] ~96-~96: Possible missing comma found.
Context: ...e API can be called through a POST REST endpoint which takes in a list of license record...(AI_HYDRA_LEO_MISSING_COMMA)
🪛 Gitleaks (8.21.2)
80-80: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 markdownlint-cli2 (0.17.2)
71-71: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
72-72: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
73-73: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
74-74: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
87-87: Bare URL used
null(MD034, no-bare-urls)
90-90: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
91-91: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
92-92: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
110-110: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
111-111: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
112-112: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
113-113: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
121-121: Bare URL used
null(MD034, no-bare-urls)
126-126: Bare URL used
null(MD034, no-bare-urls)
129-129: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
130-130: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
131-131: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
149-149: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
150-150: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
151-151: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
152-152: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
164-164: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
165-165: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
166-166: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
169-169: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
170-170: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
173-173: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
174-174: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
175-175: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
There was a problem hiding this 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 (8)
backend/compact-connect/app_clients/it_staff_onboarding_instructions/README.md (8)
1-2
: Remove BOM/hidden character at top of file
The first line begins with a zero-width no-break space (BOM) before the “#” marker, which can cause rendering or encoding issues in some editors. Please remove the hidden character so the file starts cleanly with#
.
32-35
: Merge blockquote paragraphs and remove blank line
There’s an unintended blank line between two blockquote lines, which breaks the visual grouping. Combine these into one continuous blockquote, e.g.:- > **Important**: If the link provided has already been used when you attempt to access the credentials, please contact the individual who sent the link to you as the credentials will need to be regenerated and sent using another link. - - > Likewise, if these credentials are ever accidentally shared or compromised, please inform the CompactConnect team as soon as possible, so the credentials can be deactivated and regenerated to prevent abuse of the system. + > **Important**: If the link provided has already been used when you attempt to access the credentials, please contact the individual who sent the link to you as the credentials will need to be regenerated and sent using another link. Likewise, if these credentials are ever accidentally shared or compromised, please inform the CompactConnect team as soon as possible, so the credentials can be deactivated and regenerated to prevent abuse of the system.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
33-33: Blank line inside blockquote
null(MD028, no-blanks-blockquote)
95-95
: Add missing comma before non-restrictive clause
In the sentence “The API can be called through a POST REST endpoint which takes in a list of license record objects,” add a comma before “which” for correct punctuation:- The API can be called through a POST REST endpoint which takes in a list of license record objects. + The API can be called through a POST REST endpoint, which takes in a list of license record objects.🧰 Tools
🪛 LanguageTool
[uncategorized] ~95-~95: Possible missing comma found.
Context: ...e API can be called through a POST REST endpoint which takes in a list of license record...(AI_HYDRA_LEO_MISSING_COMMA)
85-87
: Convert bare AWS documentation URL to Markdown link
Bare URLs reduce readability and may not render with link text in all viewers. Convert the AWS doc link to a named link, for example:- For more information about this authentication process, please see the following - AWS documentation: https://docs.aws.amazon.com/cognito/latest/developerguide/token-endpoint.html + For more information about this authentication process, please see the following AWS documentation: + [Cognito Token Endpoint Guide](https://docs.aws.amazon.com/cognito/latest/developerguide/token-endpoint.html)🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
86-86: Bare URL used
null(MD034, no-bare-urls)
102-106
: Clarify SSN field casing consistency
The text refers to “SSN” (uppercase) but the JSON example uses"ssn"
(lowercase). To avoid confusion, explicitly call out that the field key in the payload is lowercase:- --data '[{"ssn":"123-45-6789", ...}]' + --data '[{"ssn":"123-45-6789", ...}]' # note: the field key is `"ssn"` in lowercaseAlternatively, update the prose to reference
ssn
in lowercase.
119-121
: Convert bare GitHub docs URL to Markdown link
The link to CSV upload docs is presented as a bare URL. Update to a named link for clarity:- For detailed documentation on CSV file uploads, including required fields, formatting requirements, and the upload process, please refer to: - https://github.com/csg-org/CompactConnect/tree/main/backend/compact-connect/docs#machine-to-machine-automated-uploads + For detailed documentation on CSV file uploads, including required fields, formatting requirements, and the upload process, please refer to the CSV guides: + [Machine-to-Machine Automated Uploads](https://github.com/csg-org/CompactConnect/tree/main/backend/compact-connect/docs#machine-to-machine-automated-uploads)🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
120-120: Bare URL used
null(MD034, no-bare-urls)
124-126
: Convert bare GitHub docs URL to Markdown link
Similarly, the field descriptions link should be a named link:- For the latest information about the license data field requirements, along with descriptions of each field, please see: - https://github.com/csg-org/CompactConnect/tree/main/backend/compact-connect/docs#field-descriptions + For the latest information about the license data field requirements, along with descriptions of each field, please see: + [Field Descriptions](https://github.com/csg-org/CompactConnect/tree/main/backend/compact-connect/docs#field-descriptions)🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
125-125: Bare URL used
null(MD034, no-bare-urls)
79-79
: Redact example access_token to a placeholder
Static analysis flagged a generic API key in the example response. Since this is documentation, replace the long base64 string with a short placeholder to reinforce it’s not a real token:- "access_token": "eyJraWQiOiJleGFtcGxlS2V5SWQiLCJhbGciOiJSUzI1NiJ9..." + "access_token": "<your access_token>"This reduces confusion and avoids accidental exposure of live credentials.
🧰 Tools
🪛 Gitleaks (8.21.2)
79-79: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
backend/compact-connect/app_clients/README.md
(3 hunks)backend/compact-connect/app_clients/it_staff_onboarding_instructions/README.md
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/compact-connect/app_clients/README.md
🧰 Additional context used
🪛 LanguageTool
backend/compact-connect/app_clients/it_staff_onboarding_instructions/README.md
[style] ~45-~45: Did you mean “that” or “date”?
Context: ...ntication Process for Uploading License Data Follow these steps to obtain an acces...
(DAT)
[uncategorized] ~95-~95: Possible missing comma found.
Context: ...e API can be called through a POST REST endpoint which takes in a list of license record...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~164-~164: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...d strings use double quotes, not single quotes - Verify that your payload is a valid ...
(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
[uncategorized] ~168-~168: The preposition ‘to’ seems more likely in this position.
Context: ...ken might have expired - generate a new one - Make sure you're including the "Bear...
(AI_HYDRA_LEO_REPLACE_ON_TO)
[uncategorized] ~178-~178: This verb does not appear to agree with the subject. Consider using a different form.
Context: ... get a new token before the current one expires 2. Implement error handling for API re...
(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
🪛 Gitleaks (8.21.2)
backend/compact-connect/app_clients/it_staff_onboarding_instructions/README.md
79-79: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 markdownlint-cli2 (0.17.2)
backend/compact-connect/app_clients/it_staff_onboarding_instructions/README.md
33-33: Blank line inside blockquote
null
(MD028, no-blanks-blockquote)
86-86: Bare URL used
null
(MD034, no-bare-urls)
120-120: Bare URL used
null
(MD034, no-bare-urls)
125-125: Bare URL used
null
(MD034, no-bare-urls)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: TestPython
Found while testing that the ssn field had been accidentally removed from the schema. This fixes that and adds tests to catch this issue if it happens again.
We will send a link to this page directly.
8b3d7bb
to
d8f4f07
Compare
@landonshumway-ia looks good but test seems to be failing? |
@jlkravitz there seems to be an issue with one of the PRs that was recently merged changing one of the CDK snapshot tests. I'll see if I can rebase and address the issue. |
There was a problem hiding this 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 (2)
backend/compact-connect/app_clients/it_staff_onboarding_instructions/README.md (2)
45-92
: Clear authentication instructions with exampleThe step-by-step authentication process with example curl commands provides concrete guidance for implementation. This makes it easier for IT staff to understand and implement the authentication flow.
Consider adding a blank line after the "Authentication Process for Uploading License Data" heading for consistent markdown formatting.
## Authentication Process for Uploading License Data + Follow these steps to obtain an access token and make requests to the CompactConnect License API:
🧰 Tools
🪛 LanguageTool
[style] ~45-~45: Did you mean “that” or “date”?
Context: ...ntication Process for Uploading License Data Follow these steps to obtain an acces...(DAT)
[uncategorized] ~90-~90: This verb does not appear to agree with the subject. Consider using a different form.
Context: ...uest a new token before the current one expires - Store theaccess_token
value for u...(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
177-182
: Strong implementation recommendationsThe implementation recommendations emphasize important aspects like token refresh, error handling, and secure credential storage. This guidance will help ensure more robust integrations.
For consistency with the rest of the document, consider adding periods at the end of each recommendation.
-1. Implement token refresh logic to get a new token before the current one expires -2. Implement error handling for API responses -3. Configure your application to securely store and access the client credentials, do not store the credentials in your +1. Implement token refresh logic to get a new token before the current one expires. +2. Implement error handling for API responses. +3. Configure your application to securely store and access the client credentials, do not store the credentials in your application code.🧰 Tools
🪛 LanguageTool
[uncategorized] ~178-~178: This verb does not appear to agree with the subject. Consider using a different form.
Context: ... get a new token before the current one expires 2. Implement error handling for API re...(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
backend/compact-connect/app_clients/README.md
(3 hunks)backend/compact-connect/app_clients/it_staff_onboarding_instructions/README.md
(1 hunks)backend/compact-connect/stacks/api_stack/v1_api/api.py
(1 hunks)backend/compact-connect/stacks/api_stack/v1_api/api_model.py
(2 hunks)backend/compact-connect/stacks/api_stack/v1_api/post_licenses.py
(2 hunks)backend/compact-connect/tests/app/test_api/test_post_licenses.py
(1 hunks)backend/compact-connect/tests/resources/snapshots/COMPACT_CONFIGURATION_UPLOADER_BETA_ENV_INPUT.json
(1 hunks)backend/compact-connect/tests/resources/snapshots/POST_LICENSES_REQUEST_SCHEMA.json
(1 hunks)backend/compact-connect/tests/resources/snapshots/POST_LICENSES_RESPONSE_SCHEMA.json
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- backend/compact-connect/stacks/api_stack/v1_api/api.py
- backend/compact-connect/tests/resources/snapshots/POST_LICENSES_RESPONSE_SCHEMA.json
- backend/compact-connect/tests/resources/snapshots/POST_LICENSES_REQUEST_SCHEMA.json
- backend/compact-connect/app_clients/README.md
- backend/compact-connect/stacks/api_stack/v1_api/api_model.py
- backend/compact-connect/stacks/api_stack/v1_api/post_licenses.py
🧰 Additional context used
🪛 LanguageTool
backend/compact-connect/app_clients/it_staff_onboarding_instructions/README.md
[style] ~45-~45: Did you mean “that” or “date”?
Context: ...ntication Process for Uploading License Data Follow these steps to obtain an acces...
(DAT)
[uncategorized] ~90-~90: This verb does not appear to agree with the subject. Consider using a different form.
Context: ...uest a new token before the current one expires - Store the access_token
value for u...
(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
[uncategorized] ~95-~95: Possible missing comma found.
Context: ...e API can be called through a POST REST endpoint which takes in a list of license record...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~164-~164: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...d strings use double quotes, not single quotes - Verify that your payload is a valid ...
(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
[uncategorized] ~168-~168: The preposition ‘to’ seems more likely in this position.
Context: ...ken might have expired - generate a new one - Make sure you're including the "Bear...
(AI_HYDRA_LEO_REPLACE_ON_TO)
[uncategorized] ~178-~178: This verb does not appear to agree with the subject. Consider using a different form.
Context: ... get a new token before the current one expires 2. Implement error handling for API re...
(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: TestPython
🔇 Additional comments (10)
backend/compact-connect/tests/resources/snapshots/COMPACT_CONFIGURATION_UPLOADER_BETA_ENV_INPUT.json (1)
140-140
: Good consistency update to the scope-of-practice attestation text.The modification expands the consequences of practice act violations to include "revocation of other Compact Privileges or licenses I may hold" instead of just "revocation of other Compact Privileges". This matches the text already used for the ASLP compact on line 41 and ensures consistent messaging across different compacts.
backend/compact-connect/tests/app/test_api/test_post_licenses.py (3)
1-85
: Well-structured test for the POST licenses endpointThis new test class effectively validates the POST licenses endpoint configuration, including:
- Lambda handler integration
- API Gateway method setup with correct authorization
- Request/response model validation against schemas
The test will help ensure the restored 'ssn' field remains in the schema validation as mentioned in the PR objective.
28-33
: Good approach to verify the handler pathTesting that the Lambda handler is correctly set to 'handlers.licenses.post_licenses' is critical to ensure the endpoint functions properly. This validation will catch any accidental changes to the handler path in the future.
69-84
: Comprehensive schema validation with snapshotsThe approach of comparing the request and response model schemas against stored snapshots is excellent for maintaining API contract consistency. Setting
overwrite_snapshot=False
ensures the test will fail if the schema changes unexpectedly, which is important for maintaining backward compatibility.backend/compact-connect/app_clients/it_staff_onboarding_instructions/README.md (6)
1-23
: Excellent overview section for state IT staffThe introduction clearly explains the purpose of CompactConnect and the value of integrating with the API. This context is important for IT departments to understand why they're implementing this integration.
24-43
: Strong emphasis on credential securityThe credential security section effectively communicates the importance of proper credential handling and provides clear instructions for secure storage, which is critical for maintaining system security.
93-113
: Comprehensive license data upload instructionsThe step-by-step upload instructions with example curl commands provide a clear path for implementation. Including the 'ssn' field in the example payload aligns with the PR objective of restoring this field to the schema validation.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~95-~95: Possible missing comma found.
Context: ...e API can be called through a POST REST endpoint which takes in a list of license record...(AI_HYDRA_LEO_MISSING_COMMA)
102-106
: SSN included in example curl commandThe example includes the 'ssn' field which aligns with the PR objective of restoring this field to the schema validation. This ensures IT staff will include this required field in their implementation.
127-131
: Clear license data requirementsThe important note about license status and eligibility relationships helps prevent validation errors during implementation. These business rules are critical for IT staff to understand when preparing data for upload.
160-175
: Helpful troubleshooting guidanceThe troubleshooting section anticipates common issues and provides clear solutions, which will reduce support requests and implementation time for IT staff.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~164-~164: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...d strings use double quotes, not single quotes - Verify that your payload is a valid ...(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
[uncategorized] ~168-~168: The preposition ‘to’ seems more likely in this position.
Context: ...ken might have expired - generate a new one - Make sure you're including the "Bear...(AI_HYDRA_LEO_REPLACE_ON_TO)
@jlkravitz The test snapshot has been fixed now. |
There was a problem hiding this 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!
This adds a document that is to be sent to State IT staff instructing them how to authenticate with the compact connect system. It includes an overview of the compact connect system and some example curl commands they can use to upload license data into the beta system.
While testing the curl commands for this, I discovered that the 'ssn' field was accidentally removed from the POST license schema validation in CDK several commit back when we made the license status field updates. This adds the fix to put the field back in, along with CDK tests to catch this issue in the future should it happen again.
Closes #774
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
Refactor
Style