Bug 31799 - REST API: PUT endpoint for Items
Summary: REST API: PUT endpoint for Items
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: master
Hardware: All All
: P5 - low new feature (vote)
Assignee: Agustín Moyano
QA Contact:
URL:
Keywords:
Depends on: 31798
Blocks: 31789
  Show dependency treegraph
 
Reported: 2022-10-14 14:39 UTC by Nick Clemens
Modified: 2023-02-23 16:04 UTC (History)
4 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 31799: Add REST endpoint to modify a biblio's item (7.65 KB, patch)
2023-01-02 14:52 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31799: Add REST endpoint to modify a biblio's item (7.71 KB, patch)
2023-01-03 17:11 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 31799: (follow-up) Tidy up and fix duplicate barcode handling (2.33 KB, patch)
2023-01-03 19:10 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31799: (follow-up) Add 409 error to spec (862 bytes, patch)
2023-01-24 19:11 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2022-10-14 14:39:32 UTC

    
Comment 1 Agustín Moyano 2023-01-02 14:52:37 UTC
Created attachment 144902 [details] [review]
Bug 31799: Add REST endpoint to modify a biblio's item

To test:
1. Apply patch
2. Set RESTBasicAuth preference to true
3. Get a biblio with an item
4. Make a PUT request to /api/v1/biblios/:biblio_id/items/:item_id with a json body that represents an item to replace
5. Check that the item was modified
6. Sign off
Comment 2 Lucas Gass 2023-01-03 17:10:05 UTC
Tested with different iterations of this:

$.ajax({
    url: '/api/v1/biblios/55/items/117',
    method: 'PUT',
    data: JSON.stringify({
          "collection_code": "COLLECTION1",
          "external_id": "123test",
  }),
    contentType: 'application/json',
    success: function(result) {
        console.log('altered')
    },
});
Comment 3 Lucas Gass 2023-01-03 17:11:24 UTC
Created attachment 144949 [details] [review]
Bug 31799: Add REST endpoint to modify a biblio's item

To test:
1. Apply patch
2. Set RESTBasicAuth preference to true
3. Get a biblio with an item
4. Make a PUT request to /api/v1/biblios/:biblio_id/items/:item_id with a json body that represents an item to replace
5. Check that the item was modified
6. Sign off

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 4 Agustín Moyano 2023-01-03 19:10:27 UTC
Created attachment 144959 [details] [review]
Bug 31799: (follow-up) Tidy up and fix duplicate barcode handling
Comment 5 Nick Clemens 2023-01-24 19:11:30 UTC
Created attachment 145632 [details] [review]
Bug 31799: (follow-up) Add 409 error to spec
Comment 6 Jonathan Druart 2023-02-07 08:08:19 UTC
I don't think you are supposed to check the uniqueness of the barcode, it's at the DBMS level already.
Comment 7 Jonathan Druart 2023-02-07 08:14:42 UTC
PUT is expecting a full resource, if you want to partially modify a resource you need PATCH.
Comment 8 Martin Renvoize 2023-02-07 14:33:40 UTC
I'd also argue that this is perhaps misplaced endpoint wise.. we already have a few `/items` endpoints.. whilst I understand the logic being `POST /biblios/{biblio_id}/items` I'm not so sure that logic holds here..  Shouldn't this be `PUT /items/{item_id}` or indeed if we want partial update `PATCH /items/{item_id}`.
Comment 9 Martin Renvoize 2023-02-07 14:36:06 UTC
(In reply to Jonathan Druart from comment #6)
> I don't think you are supposed to check the uniqueness of the barcode, it's
> at the DBMS level already.

He already fixed this in a followup above ;P
Comment 10 Martin Renvoize 2023-02-07 14:40:05 UTC
Also.. should we allow moving an item between biblios on this endpoint.. or should biblio_id really be a readOnly here?
Comment 11 Tomás Cohen Arazi 2023-02-23 15:45:31 UTC
(In reply to Martin Renvoize from comment #8)
> I'd also argue that this is perhaps misplaced endpoint wise.. we already
> have a few `/items` endpoints.. whilst I understand the logic being `POST
> /biblios/{biblio_id}/items` I'm not so sure that logic holds here.. 
> Shouldn't this be `PUT /items/{item_id}` or indeed if we want partial update
> `PATCH /items/{item_id}`.

This is opinion/taste. Should we vote it? I guess it wouldn't hurt to have both endpoints.
Comment 12 Tomás Cohen Arazi 2023-02-23 16:04:35 UTC
(In reply to Jonathan Druart from comment #7)
> PUT is expecting a full resource, if you want to partially modify a resource
> you need PATCH.

Most of our PUT are actually PATCH. I wouldn't mind asking Agustin to just change the verb to the more correct form.