Skip to content
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

CST-12041 add support to set primary bitstream in the submission #242

Merged
merged 3 commits into from
Feb 22, 2024

Conversation

abollini
Copy link
Member

@abollini abollini commented Oct 5, 2023

Related to DSpace/DSpace#8834

The modification allows to expose the primary bitstream of the ORIGINAL bundle in the upload section of workspace/workflow item so that it can be eventually set during the submission process

the files attribute contains the list of user uploaded file in the section. For each file the following attributes exist
the primary attribute contains eventually the uuid of the bitstream set as primary, it will be null if no primary bitstream is set for the ORIGINAL bundle.
The files attribute contains the list of user uploaded file in the section. For each file the following attributes exist
* id: the uuid of the underline bitstream. Useful to set the primary attribute
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we mean "underlying" here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hardyoyo is right. This shouldn't say "underline bitstream". Instead it should either say "underlying bitstream" or just "bitstream" works too.

Copy link
Member

@hardyoyo hardyoyo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than one small quibble about the wording of a comment, this looks fine. Approving as-is, but would love to see the word corrected, if it's indeed a typo.

Copy link
Member

@tdonohue tdonohue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @abollini ! I gave this a look and compared it with the implementation on the backend. Overall this all looks good & mostly correct. Just a few minor updates to make / cleanup and this is ready to merge. See comments below.

workspaceitem-data-upload.md Show resolved Hide resolved
the files attribute contains the list of user uploaded file in the section. For each file the following attributes exist
the primary attribute contains eventually the uuid of the bitstream set as primary, it will be null if no primary bitstream is set for the ORIGINAL bundle.
The files attribute contains the list of user uploaded file in the section. For each file the following attributes exist
* id: the uuid of the underline bitstream. Useful to set the primary attribute
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hardyoyo is right. This shouldn't say "underline bitstream". Instead it should either say "underlying bitstream" or just "bitstream" works too.

{
"files": [
{
"primary": null,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same misalignment occurs here too

"files": [
{
metadata: {
"id": "00001abf-b2e0-477a-99de-104db7cb6469",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the responses, when I test the backend, this is called uuid instead of id. So, I think this example needs to say uuid.

{
metadata: {
"id": "00001abf-b2e0-477a-99de-104db7cb6469",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above, I think this is called uuid.

@tdonohue
Copy link
Member

@abollini : Just a friendly reminder that this REST Contract is nearly ready to merge but has minor feedback to address.

Copy link
Member

@tdonohue tdonohue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Thanks @abollini and @Micheleboychuk ! Looks good now

@tdonohue tdonohue merged commit 9deefd5 into DSpace:main Feb 22, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

4 participants