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

Updated item documentation with information on the patch operation. #25

Merged
merged 1 commit into from Jun 8, 2018

Conversation

mspalti
Copy link
Member

@mspalti mspalti commented May 17, 2018

Only describes the replace operation since that is what has been implemented to date. Will (or may) define other operations after consultation with project leaders.

Copy link
Member

@abollini abollini left a comment

Choose a reason for hiding this comment

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

I'm sorry to be so late in the review and provide guidance it really looks as my fault to provide enough information at the start to perform the task

items.md Outdated
### Replace
The replace operation allows to replace *existent* information with new one. Attempt to use the replace operation to set not yet initialized information must return an error. See [general errors on PATCH requests](patch.md)

To withdraw an item, `curl --data '{[ { "op": "replace", "path": "withdraw"}]}' -X PATCH ${dspace7-url}/api/core/items/${item-uuid}`. The withdraw operation also requires an Authorization header.
Copy link
Member

Choose a reason for hiding this comment

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

the replace operation in a json patch requires a value attribute it should be "value": true for a withdrawn request and "value": false for a reinstate request.
The most similar operation already documented is here:
https://github.com/DSpace/Rest7Contract/blob/master/workspaceitem-data-license.md#replace

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the example. I see what you mean. Will update the contract here before I update the code.

Copy link
Member Author

@mspalti mspalti May 24, 2018

Choose a reason for hiding this comment

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

Quick follow up question.

The issue says that the request should return the 422 error code if the client try to withdrawn a NOT archived item. Does this refer to the "inArchive" flag in the item data?

I notice that withdraw (and reinstate) actions toggle the inArchive flag. For example, setting withdraw = true will result in inArchive = false. My initial idea was that requesting to withdraw and already withdrawn item would result in 422 (because the inArchive flag is false). I suspect this idea is incorrect.

Once I understand the requirement better I'll also update the code PR to do the right thing.

Copy link
Member

Choose a reason for hiding this comment

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

You are right withdrawn items have the inArchive flag to false but this is not the only situation where the inArchive flag is false. InProgress submissions (workspaceitem, workflowitem) also have the inArchive flag to false.
Trying to withdrawn an item that is already withdrawn should succeed (it is a no-op)

items.md Outdated
"type": "item"
```

To reinstate an item, `curl --data '{[ { "op": "replace", "path": "reinstate"}]}' -X PATCH ${dspace7-url}/api/core/items/${item-uuid}`. The reinstate operation also requires an Authorization header.
Copy link
Member

Choose a reason for hiding this comment

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

path must point to a valid attribute in the json object that we go to update (the item), so it should be withdrawn with a value of false

items.md Outdated
```json
"inArchive": false,
"discoverable": true,
"withdrawn": true,
Copy link
Member

Choose a reason for hiding this comment

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

this should be "withdrawn": false,

Only describes the replace operation since that is what has been implemented to date.  Will (or may) define other operations after consultation with project leaders.

Corrected documentation for withdrawal operation.
@abollini abollini merged commit 026d377 into DSpace:master Jun 8, 2018
4science-it pushed a commit to 4Science/Rest7Contract that referenced this pull request Dec 2, 2022
Create a find method to return bitstrems belonging to a bundle filtered by dc.type

Approved-by: Giuseppe Digilio
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants