feat!: replace createdBy with originalVersionId
#191
Conversation
See <digidem/comapeo-core#371> for details.
This patch: - Removes `createdBy` from documents. Removes `MapeoProject.prototype.$createdByToDeviceId` as a side-effect. - Returns `originalVersionId` on all documents. Closes <#371>. Depends on <https://github.com/digidem/mapeo-schema/pull/191> and <digidem/mapeo-mock-data#7>.
| @@ -51,7 +51,7 @@ | |||
| "updatedAt", | |||
| "links", | |||
| "versionId", | |||
| "createdBy", | |||
| "originalVersionId", | |||
There was a problem hiding this comment.
This does get confusing, because is this required or not? It seems from digidem/comapeo-core#715 that we are not storing this on creation, so reads will not include this/
Trying to fully represent the type in JSONSchema might be difficult, it might be easier to do in the TypeScript defs in this repo, maybe something like:
type CommonNew = (Omit<Common, 'links' | 'originalVersionId'> & { links: [] }) | SetRequired<Common, 'originalVersionId'>e.g. you originalVersionId can be missing if link is an empty array, but otherwise it is required.
src/lib/decode-conversions.ts
Outdated
| const versionId = getVersionId(versionObj) | ||
|
|
||
| /** @type {string} */ let originalVersionId | ||
| if (common.originalVersionId) { | ||
| originalVersionId = common.originalVersionId | ||
| } else if (common.links.length === 0) { | ||
| originalVersionId = versionId | ||
| } else { | ||
| throw new Error('Cannot determine original version ID; data is malformed') | ||
| } | ||
|
|
There was a problem hiding this comment.
Ah, I see re. my comment above about it being required in JSON Schema is maybe ok given you are always appending it here.
There was a problem hiding this comment.
But maybe the encode function could benefit from the stricter typing and this same check when writing.
There was a problem hiding this comment.
I think I addressed this in d1b0975.
src/lib/encode-conversions.ts
Outdated
| @@ -28,7 +29,7 @@ import { | |||
| * schema name, and returning the most recent JSONSchema type */ | |||
| type ConvertFunction<TSchemaName extends SchemaName> = ( | |||
| mapeoDoc: Extract< | |||
| OmitUnion<MapeoDocInternal, 'versionId'>, | |||
| OmitUnion<MapeoDocInternal, 'versionId' | 'originalVersionId'>, | |||
There was a problem hiding this comment.
I think we're loosing some type safety where you are doing this, but it's late so maybe I'm missing something?
There was a problem hiding this comment.
I think I did this in d1b0975, but maybe you were imagining something else.
This patch: - Removes `createdBy` from documents. Removes `MapeoProject.prototype.$createdByToDeviceId` as a side-effect. - Returns `originalVersionId` on all documents. - Updates `@mapeo/schema` to the latest version, which requires a bunch of other changes. Closes [#371](#371). See <https://github.com/digidem/mapeo-schema/pull/191> and <digidem/mapeo-mock-data#7>. Co-Authored-By: Tomás Ciccola <[email protected]>
See digidem/comapeo-core#371 for details.