Bug 31799 - Add PUT endpoint for Items
Summary: Add PUT endpoint for Items
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: Main
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-12-28 20:42 UTC (History)
6 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:
23.05.00


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
Bug 31799: Add REST endpoint to modify a biblio's item (7.74 KB, patch)
2023-05-03 16:39 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31799: (follow-up) Tidy up and fix duplicate barcode handling (2.33 KB, patch)
2023-05-03 16:39 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31799: (follow-up) Add 409 error to spec (862 bytes, patch)
2023-05-03 16:39 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31799: Add REST endpoint to modify a biblio's item (7.79 KB, patch)
2023-05-05 12:18 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 31799: (follow-up) Tidy up and fix duplicate barcode handling (2.39 KB, patch)
2023-05-05 12:18 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 31799: (follow-up) Add 409 error to spec (918 bytes, patch)
2023-05-05 12:18 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.
Comment 13 Marcel de Rooy 2023-04-14 07:46:36 UTC
QAing
Comment 14 Agustín Moyano 2023-05-03 16:39:16 UTC
Created attachment 150607 [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 15 Agustín Moyano 2023-05-03 16:39:19 UTC
Created attachment 150608 [details] [review]
Bug 31799: (follow-up) Tidy up and fix duplicate barcode handling
Comment 16 Agustín Moyano 2023-05-03 16:39:22 UTC
Created attachment 150609 [details] [review]
Bug 31799: (follow-up) Add 409 error to spec
Comment 17 Nick Clemens 2023-05-05 12:18:22 UTC
Created attachment 150741 [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>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 18 Nick Clemens 2023-05-05 12:18:24 UTC
Created attachment 150742 [details] [review]
Bug 31799: (follow-up) Tidy up and fix duplicate barcode handling

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 19 Nick Clemens 2023-05-05 12:18:26 UTC
Created attachment 150743 [details] [review]
Bug 31799: (follow-up) Add 409 error to spec

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 20 Tomás Cohen Arazi 2023-05-09 13:51:26 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 21 Matt Blenkinsop 2023-05-15 09:55:40 UTC
Enhancement - not backporting to 22.11.x