Bug 24401 - REST API: Check-in
Summary: REST API: Check-in
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: Main
Hardware: All All
: P5 - low new feature (vote)
Assignee: Aleisha Amohia
QA Contact:
URL:
Keywords:
: 19406 (view as bug list)
Depends on: 23336
Blocks:
  Show dependency treegraph
 
Reported: 2020-01-10 22:33 UTC by Lari Taskula
Modified: 2023-10-17 22:48 UTC (History)
10 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
This adds a POST /checkouts/{checkout_id}/checkin/{library_id} endpoint for returning checkouts
Version(s) released in:


Attachments
Bug 24401: REST API: Check-in WIP (7.34 KB, patch)
2023-05-12 04:07 UTC, Danyon Sewell
Details | Diff | Splinter Review
Bug 24401: REST API: Checkin unit test fixes (7.57 KB, patch)
2023-05-15 05:49 UTC, Alex Buckley
Details | Diff | Splinter Review
bug 24401 - REST API: Check-in WIP (808 bytes, patch)
2023-05-15 05:55 UTC, Danyon Sewell
Details | Diff | Splinter Review
Bug 24401: REST API: Checkin unit test fixes (7.57 KB, patch)
2023-05-15 05:55 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 24401: REST API: Check-in (8.33 KB, patch)
2023-05-15 06:21 UTC, Danyon Sewell
Details | Diff | Splinter Review
Bug 24401: REST API: Check-in (7.96 KB, patch)
2023-06-27 10:10 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24401: REST API: Check-in (8.04 KB, patch)
2023-08-15 12:17 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24401: (QA follow-up) Tidy Checkouts.pm (11.73 KB, patch)
2023-08-15 12:17 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24401: (QA follow-up) Remove $c->validation (1.04 KB, patch)
2023-08-15 12:17 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24401: (QA follow-up) Fix failing test (3.70 KB, patch)
2023-08-15 12:17 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24401: Unit tests (2.62 KB, patch)
2023-10-12 01:40 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 24401: Add API endpoint to check in items (4.63 KB, patch)
2023-10-12 01:40 UTC, Aleisha Amohia
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Lari Taskula 2020-01-10 22:33:37 UTC

    
Comment 1 Katrin Fischer 2020-01-11 20:32:00 UTC
*** Bug 19406 has been marked as a duplicate of this bug. ***
Comment 2 David Cook 2020-01-13 05:39:56 UTC
Would love to see work done on this one :).

Not really a priority for me, so I'm not planning to develop a solution, but would be interested in testing it.
Comment 3 Danyon Sewell 2023-05-12 04:07:02 UTC
Created attachment 151109 [details] [review]
Bug 24401: REST API: Check-in WIP

 Updating files to include API endpoints for check-ins

 Test plan:

 1) Apply bug 23336 as there are dependencies in there required for this to work

 2) Apply this patch

 3) Run unit tests in t/db_dependent/api/v1/checkouts.t

 4) Make POST request to http://yourlibrary/api/v1/checkouts with request body
containing 'item_id' & 'library_id' properties.
- Observe returned data and HTTP code
  - 201 for item checked in
  - 403 for check in not allowed
  - 409 for Item not found
  - 400 for item not checked out

Sponsored-by: Auckland University of Technology
Comment 4 Alex Buckley 2023-05-15 05:49:36 UTC
Created attachment 151181 [details] [review]
Bug 24401: REST API: Checkin unit test fixes

Sponsored-by: Auckland University of Technology
Comment 5 Danyon Sewell 2023-05-15 05:55:40 UTC
Created attachment 151182 [details] [review]
bug 24401 - REST API: Check-in WIP

Couple minor fixes to swagger.yaml

Sponsored by: Auckland University of Technology
Comment 6 Alex Buckley 2023-05-15 05:55:53 UTC
Created attachment 151183 [details] [review]
Bug 24401: REST API: Checkin unit test fixes

Sponsored-by: Auckland University of Technology
Comment 7 Danyon Sewell 2023-05-15 06:21:19 UTC
Created attachment 151184 [details] [review]
Bug 24401: REST API: Check-in

 Updating files to include API endpoints for check-ins

 Test plan:

 1) Apply bug 23336 as there are dependencies in there required for this to work

 2) Apply this patch

 3) Run unit tests in t/db_dependent/api/v1/checkouts.t

 4) Make POST request to http://yourlibrary/api/v1/checkouts with request body
containing 'item_id' & 'library_id' properties.
- Observe returned data and HTTP code
  - 201 for item checked in
  - 403 for check in not allowed
  - 409 for Item not found
  - 400 for item not checked out

Sponsored-by: Auckland University of Technology
Comment 8 David Nind 2023-05-22 00:20:10 UTC
The patches for bug 23336 no longer apply - I will retest when that is fixed (I have changed the status for that bug).
Comment 9 Martin Renvoize 2023-06-27 10:10:36 UTC
Created attachment 152717 [details] [review]
Bug 24401: REST API: Check-in

 Updating files to include API endpoints for check-ins

 Test plan:

 1) Apply bug 23336 as there are dependencies in there required for this to work

 2) Apply this patch

 3) Run unit tests in t/db_dependent/api/v1/checkouts.t

 4) Make POST request to http://yourlibrary/api/v1/checkouts with request body
containing 'item_id' & 'library_id' properties.
- Observe returned data and HTTP code
  - 201 for item checked in
  - 403 for check in not allowed
  - 409 for Item not found
  - 400 for item not checked out

Sponsored-by: Auckland University of Technology
Comment 10 David Nind 2023-08-15 11:50:05 UTC
The patch no longer applies 8-(...

Applying: Bug 24401: REST API: Check-in
Using index info to reconstruct a base tree...
M	Koha/REST/V1/Checkouts.pm
M	api/v1/swagger/paths/checkouts.yaml
M	api/v1/swagger/swagger.yaml
M	t/db_dependent/api/v1/checkouts.t
Falling back to patching base and 3-way merge...
Auto-merging t/db_dependent/api/v1/checkouts.t
CONFLICT (content): Merge conflict in t/db_dependent/api/v1/checkouts.t
Auto-merging api/v1/swagger/swagger.yaml
Auto-merging api/v1/swagger/paths/checkouts.yaml
Auto-merging Koha/REST/V1/Checkouts.pm
error: Failed to merge in the changes.
Patch failed at 0001 Bug 24401: REST API: Check-in
Comment 11 Martin Renvoize 2023-08-15 12:17:21 UTC
Created attachment 154428 [details] [review]
Bug 24401: REST API: Check-in

 Updating files to include API endpoints for check-ins

 Test plan:

 1) Apply bug 23336 as there are dependencies in there required for this to work

 2) Apply this patch

 3) Run unit tests in t/db_dependent/api/v1/checkouts.t

 4) Make POST request to http://yourlibrary/api/v1/checkouts with request body
containing 'item_id' & 'library_id' properties.
- Observe returned data and HTTP code
  - 201 for item checked in
  - 403 for check in not allowed
  - 409 for Item not found
  - 400 for item not checked out

Sponsored-by: Auckland University of Technology
Comment 12 Martin Renvoize 2023-08-15 12:17:23 UTC
Created attachment 154429 [details] [review]
Bug 24401: (QA follow-up) Tidy Checkouts.pm

This just tidies the API controller to fit current best practice and
make the QA script happy.
Comment 13 Martin Renvoize 2023-08-15 12:17:26 UTC
Created attachment 154430 [details] [review]
Bug 24401: (QA follow-up) Remove $c->validation
Comment 14 Martin Renvoize 2023-08-15 12:17:29 UTC
Created attachment 154431 [details] [review]
Bug 24401: (QA follow-up) Fix failing test
Comment 15 Martin Renvoize 2023-08-15 12:19:04 UTC
Rebased and resolved QA script issues highlighted.. I've not actually tested the code as yet however, so leaving as NSO.
Comment 16 Lucas Gass 2023-08-15 16:45:15 UTC
Is delete the best name for this subroutine? Since perl has a delete() would it not be better to call this delete_checkout?
Comment 17 Katrin Fischer 2023-08-23 13:46:14 UTC
(In reply to Lucas Gass from comment #16)
> Is delete the best name for this subroutine? Since perl has a delete() would
> it not be better to call this delete_checkout?

Delete seems right from a developer point of view, but in terms of Koha terminlogy... why not use checkin?
Comment 18 David Nind 2023-08-27 20:54:57 UTC
I had a go at testing (using Postman), but I couldn't get things to work correctly to get the right responses back (reflecting my lack of knowledge about APIs!)

I've listed below what I tried and the results I got. I'm sure I am doing something fundamentally wrong here!

I'll leave the testing to someone who knows what they are doing! 8-)

Postman setup
=============

Add a new request, change to POST with the URL http://127.0.0.1:8081/api/v1/checkouts

For authorisation, set system preference RESTBasicAuth = Enable, then add koha and koha for type Basic Auth in the Authorization tab in Postman.

For the Body tab in Postman, select the raw option and then JSON as the type of input.

201 for item checked in
=======================

I couldn't work out the right request to send to check an item in.

Added the patron_id for the user checking item in (koha = 51), otherwise got an error ( {"error":"Patron not found","error_code":"PATRON_NOT_FOUND"} )

{
    "patron_id": "51",
    "item_id": "578",
    "library_id": "CPL"
}

Same as 400 for item not checked out:

- If item is checked out, get '412 Precondition Failed' and {"error":"Confirmation error"}.

- If the item is not checked out, it returns '201 Created', and the item is checked out.

403 for check in not allowed
============================

Made password invalid, returns '403 Forbidden' (as expected) with message: {"error":"Invalid password","required_permissions":null}

{
    "item_id": "578",
    "library_id": "CPL"
}


409 for Item not found
=======================

Used incorrect value for item_id, returns '409 Conflict' with error: {"error":"Item not found","error_code":"ITEM_NOT_FOUND"}

{
    "item_id": "9000",
    "library_id": "CPL"
}

400 for item not checked out
============================

I couldn't work out the right request to send to get this response.

I used patron = koha (patron_id = 51) and item = 578.

{
    "patron_id": "51",
    "item_id": "578",
    "library_id": "CPL"
}

Same as 201 for item checked in:

- If item is checked out, get '412 Precondition Failed' and {"error":"Confirmation error"}.

- If the item is not checked out, it returns '201 Created', and the item is checked out.
Comment 19 Aleisha Amohia 2023-10-12 01:40:01 UTC
Created attachment 156884 [details] [review]
Bug 24401: Unit tests
Comment 20 Aleisha Amohia 2023-10-12 01:40:05 UTC
Created attachment 156885 [details] [review]
Bug 24401: Add API endpoint to check in items

This adds a POST /checkouts/{checkout_id}/checkin/{library_id} endpoint for returning checkouts

To test:

- Run unit tests in t/db_dependent/api/v1/checkouts.t
- Make POST request to http://yourlibrary/api/v1/checkouts/{checkout_id}/checkin/{library_id} where checkout_id is the ID of the checkout you're returning and library_id is the branchcode of the return library
- Observe returned data
  - 400 incorrect params
  - 403 cannot be returned
  - 404 missing params
  - 201 item checked in

Sponsored-by: Auckland University of Technology
Comment 21 David Nind 2023-10-17 22:48:15 UTC
Getting sha1 fake ancestor error when trying to apply the patch:

git bz apply 24401

Bug 24401 - REST API: Check-in

154428 - Bug 24401: REST API: Check-in
154429 - Bug 24401: (QA follow-up) Tidy Checkouts.pm
154430 - Bug 24401: (QA follow-up) Remove $c->validation
154431 - Bug 24401: (QA follow-up) Fix failing test
156884 - Bug 24401: Unit tests
156885 - Bug 24401: Add API endpoint to check in items

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 24401: REST API: Check-in
Applying: Bug 24401: (QA follow-up) Tidy Checkouts.pm
error: sha1 information is lacking or useless (Koha/REST/V1/Checkouts.pm).
error: could not build fake ancestor
Patch failed at 0001 Bug 24401: (QA follow-up) Tidy Checkouts.pm
hint: Use 'git am --show-current-patch=diff' to see the failed patch