Add missing id, name fields to json schema docs

[hellosmithy]

Added the optional id and name fields which are documented on the FINOS site but currently missing from OrderList, InstrumentList, ContactList and Contact (name only) json schema docs.

See:

• OrderList
• InstrumentList
• ContactList
• Contact

[linux-foundation-easycla]

• ✅ login: kriswest / name: Kris West (5e3eee3, bdd65f9)
• ❌ - login: @hellosmithy / name: Ben Smith . The commit (64c5e78, 8ef6005) is not authorized under a signed CLA. Please click here to be authorized. For further assistance with EasyCLA, please submit a support request ticket.

[netlify]

✅ Deploy Preview for fdc3 ready!

Name	Link
🔨 Latest commit	5e3eee3
🔍 Latest deploy log	https://app.netlify.com/sites/fdc3/deploys/6682d7851a11cb00088bc286
😎 Deploy Preview	https://deploy-preview-1233--fdc3.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[kriswest]

Hi @hellosmithy, technically the id and name properties are 'inherited' from the base Context schema, which is composed with each of these via allOf: https://github.com/finos/FDC3/blob/13ccdd2b7b57aa81721ee136e54f42e27e65cf8c/schemas/context/contact.schema.json#L41C28-L41C28

which end up in the TypeScript types generated from the resulting composed schema:

FDC3/src/context/ContextTypes.ts

Lines 1247 to 1266 in dcd98c2

	/**
	* A collection of instruments. Use this type for use cases that require not just a single
	* instrument, but multiple (e.g. to populate a watchlist). However, when holding
	* information for each instrument is required, it is recommended to use the
	* [Portfolio](Portfolio) type.
	*
	* The instrument list schema does not explicitly include identifiers in the `id` section,
	* as there is not a common standard for such identifiers. Applications can, however,
	* populate this part of the contract with custom identifiers if so desired.
	*/
	export interface InstrumentList {
	/**
	* An array of instrument contexts that forms the list.
	*/
	instruments: InstrumentElement[];
	type: "fdc3.instrumentList";
	id?: { [key: string]: any };
	name?: string;
	[property: string]: any;
	}

However, it's not really inheritance (composition of schemas is more like layering filters over one another) and there is no problem defining them in multiple schemas that get composed together. You'll note we use the BaseContext without docs, allowing the downstream schemas to add their own without doubling up - which your PR does.

Finally, a note for the maintainers, the preview for PR #1151 (generate Context docs from schemas) shows that the docs generation script is not picking up the composed schema refs and rendering the Context schema properties: https://issue-1068--exquisite-otter-12d363.netlify.app/docs/context/schemas/Contact
We can either fix that (not trivial) or continue down this path and document the id and name properties where they make sense or are more tightly defined (e.g. specific id subfields). This latter option is simpler and adds more value to the docs / avoids rendering name and id in context docs where they are not so relevant.

[kriswest]

LGTM - although we should review if there are more schemas that need a similar change

[kriswest]

@hellosmithy Once you've got a CLA manger setup, please click on the Please click here to be authorized link above to have EasyCLA check it and add you to its database of approved contributors, which will allow us to merge this once reviewed.

[hughtroeger]

As @kriswest mentioned there are a few other other context types missing id and/or name (see my comment here #1151 (review) )

• Country is missing name
• Organization is missing name
• Portfolio is missing name and id
• Position is missing name and id
• TradeList is missing name and id

[hellosmithy]

technically the id and name properties are 'inherited' from the base Context schema, which is composed with each of these via allOf

Ah yes, I knew about the composition but didn't realise it included id and name. For some reason the json-schema-to-typescript npm package I'm using to generate some types also doesn't appear to pick these up which is what brought my attention to it. (nvm, I was wrong about that)

[hellosmithy]

Slight tangent, but are type fields supposed to be case sensitive or insensitive?
TimeRange has camel case timeRange in the example, but lowercase timerange in the schema type field.

timeRange as per the examples seems more semantically correct, but I guess that might be a breaking change so it might be easier to change the example.

[kriswest]

Ah yes, I knew about the composition but didn't realise it included id and name. For some reason the json-schema-to-typescript npm package I'm using to generate some types also doesn't appear to pick these up which is what brought my attention to it.

If I remember rightly, json-schema-to-typescript doesn't retrieve references ($ref). You can can do that yourself by just replacing the $ref element with the referenced schema. https://www.npmjs.com/package/json-schema-resolver I think can do that for you as well. In the FDC3 repo we use quicktype (which possibly combines these two libs under-the-hood) to generate typescript types from the schemas.

[kriswest]

Slight tangent, but are type fields supposed to be case sensitive or insensitive? TimeRange<https://fdc3.finos.org/schemas/2.1/context/timerange.schema.json> has camel case in the example, but lowercase in the schema

They are case-sensitive, so the example there is wrong. I'll get that fixed. Many thanks for pointing it out!

[kriswest]

@hellosmithy could you run npm run typegen or npm run build for me and check in the changes it makes? This will regenerate the typescript files produced from the schema and ensure they make it into the diff.

I should probably have this added to the pre-commit hook so that it's automated!

[hellosmithy]

Here again

[kriswest]

@hellosmithy I was able to improve the naming of the generated object, but not stop it from doubling up on the comments. What's happening is that quicktype is combining two schemas which a field could be oneOf or anyOf to produce a type that would match any of the allowed types. Hence, its got two comments as it could be either.

In ChatSearchCriteria each criteria could be an instrument, a contact, an organisation or a free text string. I was at least able to get the object named properly (to SearchCriteria) and added a description to fdc3.instrument's name property so at least we're consistent:

FDC3/src/context/ContextTypes.ts

Lines 791 to 798 in 5e3eee3

	/**
	* An optional human-readable name for the instrument
	*
	* An optional human-readable name for the contact
	*
	* An optional human-readable name for the organization
	*/
	name?: string;

[kriswest]

/easycla

[kriswest]

@hellosmithy any joy on the CLA front? If there's anything we or FINOS can do to help let us know!

[hellosmithy]

@hellosmithy any joy on the CLA front? If there's anything we or FINOS can do to help let us know!

@kriswest not yet other than that it's going through the various channels - should I close this PR until we have an update?

[kriswest]

No problem leaving it open. If its likely to take more than a few weeks we'll recreate the changes in a new PR to close out the issue.

[kriswest]

@hellosmithy any update on that approval/CLA? We're talking about prereleases for review and it would be good to get this in...

[hellosmithy]

@hellosmithy any update on that approval/CLA? We're talking about prereleases for review and it would be good to get this in...

@kriswest it's been raised to all the people it needs to be, but we don't yet have an ETA I'm afraid.
Since these are fairly trivial changes it might be best for someone else to re-submit the PR and I can close this one. Wdyt?

[kriswest]

superseded by #1360