-
Notifications
You must be signed in to change notification settings - Fork 56
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
[ENG-6447] Update preprint model for versioning #2419
[ENG-6447] Update preprint model for versioning #2419
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good as we discussed 🎆
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a question and a couple of small things.
// Insert a `versions` relationship to the model | ||
result.data.relationships!.versions = { | ||
links: { | ||
related: resourceHash.links!.preprint_versions!, | ||
}, | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to the API Contract, the API won't be serializing the Preprint.versions
relationship, since it is recursive.
preprintVersion: 1, | ||
isLatestVersion: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is the latest version, shouldn't it be version 3 based on the withVersions
afterCreate?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
withVersions
is a factory trait, so only preprints created with that trait (server.create('preprint', 'withVersions')
) will have 3 versions. Every other preprint will just have only 1 version.
@@ -35,6 +36,8 @@ export function createPreprint(this: HandlerContext, schema: Schema) { | |||
subjects: [], | |||
tags: [] as string[] , | |||
currentUserPermission: [Permission.Admin, Permission.Read, Permission.Write], | |||
versionNumber: 1, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And I would presume this needs to be 3 as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is in the createPreprint
view, so the versionNumber
property for a freshly created preprint should just be 1
in this case. I will have a separate view for createPreprintVersion
that will increment the versionNumber
property
500e4bd
into
CenterForOpenScience:feature/preprints-doi-versioning
Purpose
Summary of Changes
preprintVersion
andisLatestVersion
attrs to Preprintversions
relationship to Preprintpreprint_version
URL that can be used to get versions, hence the chicanery with the preprint-serializerScreenshot(s)
Side Effects
QA Notes