Update docs to reflect licenseNumber field as required#1231
Update docs to reflect licenseNumber field as required#1231isabeleliassen merged 5 commits intocsg-org:mainfrom
Conversation
|
Warning Rate limit exceeded@landonshumway-ia has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 8 minutes and 51 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughAdded OTCC-mandated Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/compact-connect/docs/README.md (1)
38-40: Clarify the distinction between*and**notation in the field descriptions table.The table header explains that asterisk marks required fields, but doesn't distinguish between API-enforced required fields (
*) and OTCC-mandated fields (**) that aren't API-enforced. This distinction is critical for state integration and compliance understanding.Suggest updating the explanation to:
The following table describes all available fields for the license CSV file. Required fields are marked with an asterisk -(*). Note that our API does not accept `NULL` values - if some of your licenses have no value for an optional field, -leave the field entirely empty. If some of your licenses are missing a required field, those licenses will be rejected. +(*) or double asterisk (**). Fields marked with * are enforced by the API; fields marked with ** are required by OTCC +compact rule but not API-enforced—states are responsible for enforcing these. Note that our API does not accept `NULL` +values - if some of your licenses have no value for an optional field, leave the field entirely empty. If some of your +licenses are missing an API-required field (*), those licenses will be rejected.
🧹 Nitpick comments (1)
backend/compact-connect/docs/README.md (1)
94-102: Consider adding clarity to the "missing optional fields" example.The example at line 101 shows
licenseNumberwith data while optional fields are empty. SincelicenseNumberis now required (marked with**), readers might benefit from explicit guidance in this section clarifying that required fields must always be present even when optional fields are missing.Consider updating the section introduction (line 94-96) to clarify:
If data is not available for an optional field, it must be left empty in the case of CSV data (or omit the CSV column entirely if none of the license records contain that field). In the case of JSON uploads, the key must be excluded from the object. Please do not include NULL or similar N/A type values for missing data. +Note that required fields (marked with * or **) must always include data—they cannot be left empty. The example below shows optional fields omitted while all required fields are present. + **CSV Example with missing optional fields:**
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/compact-connect/docs/README.md(2 hunks)
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 1019
File: webroot/src/network/licenseApi/data.api.ts:256-276
Timestamp: 2025-08-19T19:39:47.790Z
Learning: The backend API for CompactConnect will accept and require the new `encumbranceType` field in encumbrance requests, even though this field may not appear in the current (incomplete) API documentation.
📚 Learning: 2025-08-21T19:17:21.038Z
Learnt from: rmolinares
Repo: csg-org/CompactConnect PR: 1025
File: webroot/src/models/Licensee/Licensee.model.ts:276-312
Timestamp: 2025-08-21T19:17:21.038Z
Learning: In CompactConnect, address information should only come from License objects, not from top-level Licensee fields. The API no longer returns top-level licensee address fields, so all address data flows through individual license objects that contain fields like homeAddressStreet1, homeAddressCity, etc. The bestLicense() and bestLicenseMailingAddress() methods are used to access the most appropriate address from a licensee's collection of licenses.
Applied to files:
backend/compact-connect/docs/README.md
📚 Learning: 2025-08-19T19:39:47.790Z
Learnt from: jsandoval81
Repo: csg-org/CompactConnect PR: 1019
File: webroot/src/network/licenseApi/data.api.ts:256-276
Timestamp: 2025-08-19T19:39:47.790Z
Learning: The backend API for CompactConnect will accept and require the new `encumbranceType` field in encumbrance requests, even though this field may not appear in the current (incomplete) API documentation.
Applied to files:
backend/compact-connect/docs/README.md
📚 Learning: 2025-10-15T19:56:58.050Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1135
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/record.py:60-0
Timestamp: 2025-10-15T19:56:58.050Z
Learning: In the CompactConnect codebase, when migrating from a single field to a list field (e.g., clinicalPrivilegeActionCategory to clinicalPrivilegeActionCategories), both fields are intentionally kept as optional during the migration period to support backwards compatibility. Mutual exclusivity validation is not enforced during this phase, as the deprecated field will be removed in a follow-up PR with migration scripts.
Applied to files:
backend/compact-connect/docs/README.md
📚 Learning: 2025-08-21T19:17:21.038Z
Learnt from: rmolinares
Repo: csg-org/CompactConnect PR: 1025
File: webroot/src/models/Licensee/Licensee.model.ts:276-312
Timestamp: 2025-08-21T19:17:21.038Z
Learning: In CompactConnect's License model, address data is correctly mapped from server fields like homeAddressStreet1, homeAddressCity, homeAddressState, homeAddressPostalCode to a structured mailingAddress object via LicenseSerializer.fromServer(). The top-level Licensee address fields were removed because the API no longer returns them, but License objects retain their address field mappings. Address information should be accessed through bestLicense() and bestLicenseMailingAddress() helper methods rather than top-level licensee properties.
Applied to files:
backend/compact-connect/docs/README.md
📚 Learning: 2025-11-13T22:13:48.430Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1188
File: backend/compact-connect/tests/smoke/rollback_license_upload_smoke_tests.py:354-372
Timestamp: 2025-11-13T22:13:48.430Z
Learning: In the CompactConnect codebase, privilege records use datetime fields (not date fields) for dateOfIssuance, dateOfRenewal, and dateOfExpiration, which is intentionally different from license records that use date fields for these same attributes.
Applied to files:
backend/compact-connect/docs/README.md
📚 Learning: 2025-09-11T20:06:40.709Z
Learnt from: ChiefStief
Repo: csg-org/CompactConnect PR: 1036
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py:370-383
Timestamp: 2025-09-11T20:06:40.709Z
Learning: The EncumbranceDetailsSchema in backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/privilege/record.py does not contain a 'note' field. It only has clinicalPrivilegeActionCategory (String, optional), adverseActionId (UUID, required), and licenseJurisdiction (Jurisdiction, optional).
Applied to files:
backend/compact-connect/docs/README.md
📚 Learning: 2025-06-26T16:42:00.781Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 882
File: backend/compact-connect/lambdas/python/migration/compact_configured_states_871/main.py:127-130
Timestamp: 2025-06-26T16:42:00.781Z
Learning: In the compact_configured_states_871 migration, existing jurisdiction configurations that have licenseeRegistrationEnabled: true are migrated with isLive: true for backwards compatibility. This prevents breaking existing live functionality since those states are already operational. The migration preserves the current live state of jurisdictions to maintain service continuity, while new states added after migration can start with isLive: false and be controlled by compact admins.
Applied to files:
backend/compact-connect/docs/README.md
📚 Learning: 2025-06-18T21:57:02.978Z
Learnt from: jusdino
Repo: csg-org/CompactConnect 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.
Applied to files:
backend/compact-connect/docs/README.md
📚 Learning: 2025-08-29T19:41:38.201Z
Learnt from: jusdino
Repo: csg-org/CompactConnect PR: 1005
File: backend/compact-connect/app_clients/README.md:178-187
Timestamp: 2025-08-29T19:41:38.201Z
Learning: In the CompactConnect system, "state" and "jurisdiction" are functionally synonymous and can be used interchangeably in documentation and code. Users consider consistency between these terms to be a non-issue.
Applied to files:
backend/compact-connect/docs/README.md
📚 Learning: 2025-04-29T02:52:40.532Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 769
File: backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_encumbrance.py:138-147
Timestamp: 2025-04-29T02:52:40.532Z
Learning: In CompactConnect tests, hardcoded values (like license type abbreviations 'slp') in test queries are sometimes used intentionally rather than using dynamic lookups. This is a deliberate design decision to make tests fail if default test data changes, requiring developers to consciously update related tests.
Applied to files:
backend/compact-connect/docs/README.md
⏰ 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: TestApp
🔇 Additional comments (1)
backend/compact-connect/docs/README.md (1)
54-54: Field description is clear and comprehensive.The explanation correctly addresses the OTCC requirement, API non-enforcement, and state responsibility. This aligns with the PR objectives and past feedback.
|
@jlkravitz This is ready for your review. Thanks |
jlkravitz
left a comment
jlkravitz
left a comment
There was a problem hiding this comment.
This seems like something that will come back to bite us in the long-term: what's stopping us from enforcing licenseNumber on upload, even if the rest of the codebase treats it as optional? Or do existing 'live' states not always upload these numbers?
|
@jlkravitz We have multiple states already uploading license data into production, and they are not guaranteed to be uploading license numbers, so to force this requirement anywhere would be a backwards incompatible change that could break existing integrations. This project initially had the requirement to set the license number as optional (before I even joined the project), and we launched with it still as optional. It wasn't until the last month or so that anyone said anything about it needing to be required. The unfortunate reality is once you release a field as optional in an external facing API, you can't make it required unless you either break your existing clients or you create a new version of the API. Neither of those are desirable at this point, so we are at least updating the documentation to reflect this. |
|
@landonshumway-ia That makes sense– I guess I still wonder why we can't prod/encourage inclusion of that field in the API, even if we don't fully require it across-the-board. e.g., in CSV uploads. Or, we could support warnings in validation emails – warnings don't stop the upload, but they do generate an email notification. I'm not saying we should do this or that the time would be worth the outcome, but just throwing ideas out there. Regardless, this shouldn't slow down the merging of this PR, so @isabeleliassen feel free to merge whenever! |
4 participants
Previously, we were asked to allow the
licenseNumberfield to be optional. The system was launched with that field set as optional. We have been asked to document that this field is required by compact rule, and the license number field should be included in every license upload. Although we cannot change the API spec to force this field to be required as this would be a breaking change to the states that are already live, we have been asked to update the documentation to reflect this compact rule requirement so that new onboarding states will be aware the compacts expect license numbers to always be present with every license uploaded into the system.Closes #1182
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.