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
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') }, });
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>
Created attachment 144959 [details] [review] Bug 31799: (follow-up) Tidy up and fix duplicate barcode handling
Created attachment 145632 [details] [review] Bug 31799: (follow-up) Add 409 error to spec
I don't think you are supposed to check the uniqueness of the barcode, it's at the DBMS level already.
PUT is expecting a full resource, if you want to partially modify a resource you need PATCH.
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}`.
(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
Also.. should we allow moving an item between biblios on this endpoint.. or should biblio_id really be a readOnly here?
(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.
(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.
QAing
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>
Created attachment 150608 [details] [review] Bug 31799: (follow-up) Tidy up and fix duplicate barcode handling
Created attachment 150609 [details] [review] Bug 31799: (follow-up) Add 409 error to spec
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>
Created attachment 150742 [details] [review] Bug 31799: (follow-up) Tidy up and fix duplicate barcode handling Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 150743 [details] [review] Bug 31799: (follow-up) Add 409 error to spec Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Pushed to master for 23.05. Nice work everyone, thanks!
Enhancement - not backporting to 22.11.x