Description
Nick Clemens (kidclamp)
2022-10-14 14:38:53 UTC
Created attachment 144893 [details] [review] Bug 31798: Add REST endpoint to add a biblio's item To test: 1. Apply patch 2. Set RESTBasicAuth preference to true 3. Get a biblio 4. Make a POST request to /api/v1/biblios/:biblio_id/items with a json body that represents the new item 5. Check that the item is created 6. Sign off Created attachment 144894 [details] [review] Bug 31798: Add REST endpoint to add a biblio's item To test: 1. Apply patch 2. Set RESTBasicAuth preference to true 3. Get a biblio 4. Make a POST request to /api/v1/biblios/:biblio_id/items with a json body that represents the new item 5. Check that the item is created 6. Sign off Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Created attachment 144952 [details] [review] Bug 31798: (follow-up) Tidy up camel cased variables and unnecessary checks Agustin, I think this is missing tests for the duplicate barcode use case. And in those tests it should be returning a 409 (conflict?) instead of a 500 because of the unhandled exception. Thanks. Created attachment 144953 [details] [review] Bug 31798: (follow-up) Add duplicate barcode test (In reply to Tomás Cohen Arazi from comment #4) > Agustin, I think this is missing tests for the duplicate barcode use case. > And in those tests it should be returning a 409 (conflict?) instead of a 500 > because of the unhandled exception. > > Thanks. Should we return 409 conflict or 400 bad request? Thanks Created attachment 144956 [details] [review] Bug 31798: (follow-up) Tidy up and add duplicate barcode test Aren't we being a bit nieve here in terms of autoBarcode? I'm wondering: 1) Can we move the autoBarcode handling into Koha::Item->store() instead? 2) Even if we don't move the autoBarcode handling, don't we need to account for the other options it supports? (In reply to Martin Renvoize from comment #8) > Aren't we being a bit nieve here in terms of autoBarcode? > > I'm wondering: > 1) Can we move the autoBarcode handling into Koha::Item->store() instead? > 2) Even if we don't move the autoBarcode handling, don't we need to account > for the other options it supports? Hi Martin, If I remove all about autoBarcode and link this to bug 32896, would it pass QA? Thanks Hmm, it would help move this bug.. but in my opinion it's moving towards blocked rather than PQA. It's hard... I don't really want to block this one and asking for the autoBarcode move is a bit out of direct scope of this piece. Created attachment 147463 [details] [review] Bug 31798: (follow-up) Handle all values of autoBarcode Created attachment 147464 [details] [review] Bug 31798: (follow-up) Handle all values of autoBarcode (In reply to Martin Renvoize from comment #10) > Hmm, it would help move this bug.. but in my opinion it's moving towards > blocked rather than PQA. > > It's hard... I don't really want to block this one and asking for the > autoBarcode move is a bit out of direct scope of this piece. Follow-up attached, crude, but lets us handle the move more elegantly on 32896 Created attachment 147655 [details] [review] Bug 31798: Add REST endpoint to add a biblio's item To test: 1. Apply patch 2. Set RESTBasicAuth preference to true 3. Get a biblio 4. Make a POST request to /api/v1/biblios/:biblio_id/items with a json body that represents the new item 5. Check that the item is created 6. Sign off Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 147656 [details] [review] Bug 31798: (follow-up) Tidy up and add duplicate barcode test Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 147657 [details] [review] Bug 31798: (follow-up) Handle all values of autoBarcode Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Applying: Bug 31798: (follow-up) Handle all values of autoBarcode error: sha1 information is lacking or useless (Koha/REST/V1/Biblios.pm). error: could not build fake ancestor Patch failed at 0001 Bug 31798: (follow-up) Handle all values of autoBarcode Created attachment 150598 [details] [review] Bug 31798: Add REST endpoint to add a biblio's item To test: 1. Apply patch 2. Set RESTBasicAuth preference to true 3. Get a biblio 4. Make a POST request to /api/v1/biblios/:biblio_id/items with a json body that represents the new item 5. Check that the item is created 6. Sign off Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 150599 [details] [review] Bug 31798: (follow-up) Tidy up and add duplicate barcode test Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 150600 [details] [review] Bug 31798: (follow-up) Handle all values of autoBarcode Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 150771 [details] [review] Bug 31798: Add REST endpoint to add a biblio's item To test: 1. Apply patch 2. Set RESTBasicAuth preference to true 3. Get a biblio 4. Make a POST request to /api/v1/biblios/:biblio_id/items with a json body that represents the new item 5. Check that the item is created 6. Sign off Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 150772 [details] [review] Bug 31798: Add REST endpoint to add a biblio's item To test: 1. Apply patch 2. Set RESTBasicAuth preference to true 3. Get a biblio 4. Make a POST request to /api/v1/biblios/:biblio_id/items with a json body that represents the new item 5. Check that the item is created 6. Sign off Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 150773 [details] [review] Bug 31798: (follow-up) Tidy up and add duplicate barcode test Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 150774 [details] [review] Bug 31798: (follow-up) Handle all values of autoBarcode Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 150775 [details] [review] Bug 31798: (QA follow-up) Remove use Koha::Item, we already use Koha::Items Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Pushed to master for 23.05. Nice work everyone, thanks! Another API addition.. for now I'm choosing not to backport, but let me know if this is important to you and I'll look at backporting the whole tree. |