feat!(core): add identifier models to core library and website#1000
Conversation
Add the Identifier and SystemId field models, the IdentifierT template they instantiate, and the Identifiers collection. Tighten the employerTaxId, samUEI, and duns scalars to their registry-specific formats.
Remove the ein, uei, and duns fields from OrganizationBase and add an identifiers collection (OrgIds) built from the OrgIdEin, OrgIdUei, and OrgIdDuns registry-specific models. BREAKING CHANGE: OrganizationBase.ein, uei, and duns are removed in favor of the identifiers collection.
The per-version schema reconstruction now strips fields whose @removed version is at or before the target version, not just fields added after it. Without this, v0.4 OrganizationBase kept the removed ein/uei/duns fields.
Rename IdentifierCollection to Identifiers and OrgIdentifierCollection to OrgIds in ADR-0023 to match the implemented models.
Add the Identifiers field page (Identifier, SystemId, Identifiers) and per-identifier sections (OrgIds, OrgIdEin, OrgIdUei, OrgIdDuns) on the Organization model page.
Repoint the question-bank and SF-424 form mappings from the removed org ein/uei/duns fields to the identifiers collection (organizations.primary.identifiers.org:us:ein.id, etc.).
Regenerate the published JSON schemas for the identifier models and the updated OrganizationBase.
|
🚀 Website Preview Deployed! Preview your changes at: https://cg-pr-1000.billy-daly.workers.dev This preview will be automatically deleted when the PR is closed. |
Removes a few fields like `verifiedAt`, `uri`, `registry.schema` etc. to prevent us from prematurely adding props that we wind up removing later. Also makes all of the props in this model optional.
SnowboardTechie
left a comment
There was a problem hiding this comment.
Looked through this, really nice work on the IdentifierT template shape, the registry-pinned org IDs and the version-generator fix all read clean, and the removal tests pin the semantics down well.
On the ADR-0023 deviations: I'd land them the same way. Dropping kind/scope/schema/uri/verifiedAt/verifiedBy is the right conservative call, this PR itself shows how much cheaper adding later is than removing.
One tiny wording note inline, otherwise good to go.
Refers to `IdentifierCollection` as a "field" instead of a "collection" to avoid repetition.
…ions A model or property defined via `is`/`extends` on a versioned base inherits the base's `@added`/`@removed` decorators in addition to its own, so `getAddedOnVersions`/`getRemovedOnVersions` can report the same version more than once. This produced duplicate "Added X model" changelog entries for the identifier template instantiations (Identifier, SystemId, OrgIdEin, OrgIdUei, OrgIdDuns). Collapse duplicate versions before logging model/property additions and removals, since an entity can only be added or removed once per version.
The `refactor(core): simplify Identifier model` change and the org-identifier additions shifted line numbers in identifier.tsp, organization.tsp, types.tsp, and filters/numeric.tsp, but the doc frontmatter ranges were not updated, so several TypeSpec tabs rendered the wrong model or truncated code. Realign every affected range to its declaration (doc comment through closing line): identifier/SystemId/IdentifierCollection, OrgSocialLinks, the string, date, and numeric scalars, and the number range/array filters.
|
@SnowboardTechie FYI - I incorporated your suggested fix in ADR-0023 and then fixed a few incorrect ranges on the website docs in this commit: docs(website): fix stale TypeSpec line ranges |
|
🗑️ Preview Cleaned Up The preview for this PR has been automatically deleted. |
Summary
Adds
IdentifierandIdentifierCollectionmodels; replacesein,duns, andueionOrganizationBasewith new identifier entries; and updates the website docs for these models.Changes proposed
Identifier,IdentifierCollection, andSystemIdmodels to@common-grants/corelibrary following the pattern defined in ADR-0023identifiersprop toOrganizationBaseand moves ID-related props into that collection, matching the org-related IDs added in feat(website): add ID registry catalog #979OrganizationBase.ein->OrganizationBase.identifiers[org:us:ein]OrganizationBase.uei->OrganizationBase.identifiers[org:us:uei]OrganizationBase.duns->OrganizationBase.identifiers[org:xi:duns]/protocol/fields/identifiers/page for the new identifier fields and updates/protocol/models/organizationto include the new org ID entriesversion-generator.tsto remove props marked with@Versioning.removed()in the JSON schemas that are auto-generated from TypeSpec since@typespec/versioningdoesn't support this natively, and commits these auto-generated schemas.Identifierschema proposed in ADR-0023 by removing:kind,scope, andschemafromIdentifer.registryuri,verifiedAt, andverifiedByfrom the rootContext for reviewers
Instructions to review
/protocol/fields/should include Identifiers[/protocol/fields/idenfitiers/](https://cg-pr-1000.billy-daly.workers.dev/protocol/fields/identifiers/) should include the newly added identifier TypeSpec models/protcol/models/organization/should have the new org ID models, an updatedOrganizationBaseand a new entry in the changelogNotes
identifiersprop toOpportunityBasebecause it would break the property-based testing that we have set up in the TypeScript SDK unless we also implement in in the SDK.@Versioning.removed()in the JSON schema output should probably be moved either intolib/changelog-emitteror in a new custom TypeSpec emitter in ourlib/directory because multiple sections of this codebase use@typespec/json-schemaand the resulting JSON schemas will includeein,duns, andueiproperties unless they adopt a similar fix to the one this PR adds toversion-generator.tsto remove them.Identifiermodel a bit by removing some extra fields that we may want to add in the future. But given the complexity of removing properties (as surfaced in this PR) I'd rather be more conservative with the fields we add in the initial release. The fields I removed are:kind,scope, andschemasince these can be looked up in the ID registry entry usingregistry.urluri,verifiedAt, andverifiedBybecause it's a lot to add a verification layer or external lookups to identifiers at this step.Additional information
Fields page
New identifiers field page
Updated organization model page
Update question bank and form mapping