-
Notifications
You must be signed in to change notification settings - Fork 1
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
fix: dataType.delete()
should use docId
not versionId
#422
Comments
This could raise the question, "how do I delete a fork", which you do as: const obs = await project.observation.getByDocId(docId)
const merged = await project.observation.update([obs.versionId, ...obs.forks], valueOf(obs)) Could look at creating a helper for this in the API if needed once we introduce a UI for managing forks. |
yeah i think this makes sense. So basically:
|
yeah, although "deleting an older version" and "deleting a fork" doesn't really have a clear meaning. |
gmaclennan
added a commit
that referenced
this issue
Dec 19, 2023
Fixes "`dataType.delete()` should use `docId` not `versionId`" (#422) The deletion document links to all forks, if forks exist.
gmaclennan
added a commit
that referenced
this issue
Dec 19, 2023
* fix!: delete() by docId not versionId(s) Fixes "`dataType.delete()` should use `docId` not `versionId`" (#422) The deletion document links to all forks, if forks exist. * Remove comment
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Description
Reading through #420 made me realize that I think our current
delete()
method does not make sense. Trying to "delete" an arbitrary version of a doc doesn't actually do anything unless it is a "head". It will actually create a fork, which is kind of confusing.e.g. we have a doc with docId
X
, created with versionIdA
. It is then edited so we have versionB
. Now a user tries to delete versionA
. They will create a new versionC
, and we now have two "heads" or "forks":B
andC
.I think the delete method should actually accept the
docId
, since what you actually expect from deletion is that the document is no longer returned fromgetMany()
. The delete function should:docId
, and get the versionIdforks
property, and also get the existing doc(s) for those forks, to get the versionIdsdeleted: true
that haslinks
with the versionIds above.dataType.getByDocId(docId)
.@achou11 @tomasciccola thoughts or comments?
The text was updated successfully, but these errors were encountered: