Bug 17003

Summary: Add API route to get checkout's renewability
Product: Koha Reporter: Jiri Kozlovsky <mail>
Component: REST APIAssignee: Josef Moravec <josef.moravec>
Status: CLOSED FIXED QA Contact:
Severity: new feature    
Priority: P5 - low CC: black23, fridolin.somers, johanna.raisa, jonathan.druart, josef.moravec, lari.taskula, lucas, m.de.rooy, martin.renvoize
Version: Main   
Hardware: All   
OS: All   
URL: https://wiki.koha-community.org/wiki/Checkouts_endpoint_RFC#Checkout_renewability_2
Change sponsored?: Sponsored Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
19.11.00,19.05.03
Bug Depends on: 13895, 22607    
Bug Blocks: 18407, 30983, 16652, 17005    
Attachments: Bug 17003: Add API route to get checkout's renewability
Bug 17003: Add API route to get checkout's renewability
Bug 17003: Add API route to get checkout's renewability
Bug 17003: Add API route to get checkout's renewability
Bug 17003: Add API route to get checkout's renewability
Bug 17003: Add API route to get checkout's renewability
Bug 17003: Add API route to get checkout's renewability
Bug 17003: Adapt to OpenAPI
Bug 17003: Add API route to get checkout's renewability
Bug 17003: Adapt to OpenAPI
Bug 17003: (follow-up) Update definitions according to voted RFC
Bug 17003: Adapt to OpenAPI
Bug 17003: (follow-up) Update definitions according to voted RFC
Bug 17003: Add API route to get checkout's renewability
Bug 17003: Adapt to OpenAPI
Bug 17003: (follow-up) Update definitions according to voted RFC
Bug 17003: Add API route to get checkout's renewability
Bug 17003: Adapt to OpenAPI
Bug 17003: (follow-up) Update definitions according to voted RFC
Bug 17003: Add API route to get checkout's renewability
Bug 17003: Adapt to OpenAPI
Bug 17003: (follow-up) Update definitions according to voted RFC
Bug 17003: (QA follow-up) Fix the number of tests

Description Jiri Kozlovsky 2016-07-31 10:59:55 UTC
Add API route for checking renewability of an checkedout item.
Comment 1 Lari Taskula 2016-08-22 14:43:24 UTC Comment hidden (obsolete)
Comment 2 Lari Taskula 2016-08-22 14:44:42 UTC
Created attachment 54710 [details] [review]
Bug 17003: Add API route to get checkout's renewability
Comment 3 Lari Taskula 2016-10-14 11:08:10 UTC
Created attachment 56507 [details] [review]
Bug 17003: Add API route to get checkout's renewability

GET /checkouts/{checkout_id}/renewability (renewability status)

Response:
- 200: renewable (empty response object {})
- 403: not renewable (or not authorized to make renewals)

To test:
1. Login to Koha
2. Checkout something for yourself
3. Make GET request to http://yourlibrary/api/v1/checkouts/YYY/renewability
   where YYY is checkout_id of your checkout.
4. Observe returned data and HTTP code (200 for renewable, 403 for not renewable)
   If not renewable, an error should also be presented.
5. Run t/db_dependent/api/v1/checkouts.t
Comment 4 Lari Taskula 2016-10-14 11:08:33 UTC
Rebased on top of master.
Comment 5 Lari Taskula 2016-11-15 12:17:02 UTC
Created attachment 57489 [details] [review]
Bug 17003: Add API route to get checkout's renewability

GET /checkouts/{checkout_id}/renewability (renewability status)

Response:
- 200: renewability { renewable: true/false, error: "error message" }

To test:
1. Login to Koha
2. Checkout something for yourself
3. Make GET request to http://yourlibrary/api/v1/checkouts/YYY/renewability
   where YYY is checkout_id of your checkout.
4. Observe returned data. If not renewable, an error should also be presented.
5. Run t/db_dependent/api/v1/checkouts.t
Comment 6 Lari Taskula 2016-11-15 12:18:16 UTC
I changed the return JSON. It was confusing in previous patch. Now we return HTTP 200 with:

{
  "renewable": true,
  "error": null
}

...or...

{
  "renewable": false,
  "error": "too_many"
}
Comment 7 Lari Taskula 2016-11-15 14:38:32 UTC
Created attachment 57493 [details] [review]
Bug 17003: Add API route to get checkout's renewability

GET /checkouts/{checkout_id}/renewability (renewability status)

Response:
- 200: renewability { renewable: true/false, error: "error message" }

To test:
1. Login to Koha
2. Checkout something for yourself
3. Make GET request to http://yourlibrary/api/v1/checkouts/YYY/renewability
   where YYY is checkout_id of your checkout.
4. Observe returned data. If not renewable, an error should also be presented.
5. Run t/db_dependent/api/v1/checkouts.t
Comment 8 Jiri Kozlovsky 2016-12-11 19:51:00 UTC
Comment on attachment 57493 [details] [review]
Bug 17003: Add API route to get checkout's renewability

Review of attachment 57493 [details] [review]:
-----------------------------------------------------------------

::: Koha/REST/V1/Checkout.pm
@@ +109,5 @@
> +        $OpacRenewalAllowed = C4::Context->preference('OpacRenewalAllowed');
> +    }
> +
> +    unless ($user && ($OpacRenewalAllowed
> +            || haspermission($user->userid, { circulate => "circulate_remaining_permissions" }))) {

This "haspermission" check should be moved to the path definition using "x-koha-authorization".

Also when OpacRenewalAllowed syspref is disabled, the error should be returned sooner then possible "Checkout doesn't exist" error (don't give users hope to renew it when not found, but tell them renewal is not allowed at all instead).

::: api/v1/swagger/paths/checkouts.json
@@ +98,5 @@
> +    "get": {
> +      "operationId": "renewabilityCheckout",
> +      "tags": ["patrons", "checkouts"],
> +      "parameters": [{
> +          "name": "checkout_id",

I think this parameter should be exported in dependent bug 13895 and only referenced here instead of defined again.
Comment 9 Jiri Kozlovsky 2016-12-11 21:42:43 UTC
Created attachment 58099 [details] [review]
Bug 17003: Add API route to get checkout's renewability

GET /checkouts/{checkout_id}/renewability (renewability status)

Response:
- 200: renewability { renewable: true/false, error: "error message" }

To test:
1. Login to Koha
2. Checkout something for yourself
3. Make GET request to http://yourlibrary/api/v1/checkouts/YYY/renewability
   where YYY is checkout_id of your checkout.
4. Observe returned data. If not renewable, an error should also be presented.
5. Run t/db_dependent/api/v1/checkouts.t

Signed-off-by: Jiri Kozlovsky <mail@jkozlovsky.cz>

I've signed-off this because I've made no major change and the
funcionality remains the same.

Changes made:
1. Added x-koha-authorization to the paths definition
2. Changed checkout_id parameter definition to the reference
3. Moved error raised when syspref OpacRenewalAllowed disabled in front of
 possible error "Checkout doesn't exist" (don't give users hope to renew it
 when not found, but tell them renewal is not allowed at all instead)
Comment 10 Jiri Kozlovsky 2016-12-11 21:59:08 UTC
(In reply to Jiri Kozlovsky from comment #8)
> Comment on attachment 57493 [details] [review] [review]
> Bug 17003: Add API route to get checkout's renewability
> 
> Review of attachment 57493 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> ::: Koha/REST/V1/Checkout.pm
> @@ +109,5 @@
> > +        $OpacRenewalAllowed = C4::Context->preference('OpacRenewalAllowed');
> > +    }
> > +
> > +    unless ($user && ($OpacRenewalAllowed
> > +            || haspermission($user->userid, { circulate => "circulate_remaining_permissions" }))) {
> 
> This "haspermission" check should be moved to the path definition using
> "x-koha-authorization".
> 
Sorry about that inconvenience - I haven't realized that ownership check is required.

It would be nice to have bug 17479 implmemented soon so that we avoid such misunderstandings :) .. btw how about setting that bug as a dependency and rewriting this code?
Comment 11 Marcel de Rooy 2017-06-16 09:46:02 UTC
Depends on a FQA
Comment 12 Josef Moravec 2019-02-21 18:43:25 UTC
Created attachment 85475 [details] [review]
Bug 17003: Add API route to get checkout's renewability

GET /checkouts/{checkout_id}/renewability (renewability status)

Response:
- 200: renewability { renewable: true/false, error: "error message" }

To test:
1. Login to Koha
2. Checkout something for yourself
3. Make GET request to http://yourlibrary/api/v1/checkouts/YYY/renewability
   where YYY is checkout_id of your checkout.
4. Observe returned data. If not renewable, an error should also be presented.
5. Run t/db_dependent/api/v1/checkouts.t

Signed-off-by: Jiri Kozlovsky <mail@jkozlovsky.cz>

I've signed-off this because I've made no major change and the
funcionality remains the same.

Changes made:
1. Added x-koha-authorization to the paths definition
2. Changed checkout_id parameter definition to the reference
3. Moved error raised when syspref OpacRenewalAllowed disabled in front of
 possible error "Checkout doesn't exist" (don't give users hope to renew it
 when not found, but tell them renewal is not allowed at all instead)
Comment 13 Josef Moravec 2019-02-21 18:43:35 UTC
Created attachment 85476 [details] [review]
Bug 17003: Adapt to OpenAPI

Test plan:
prove t/db_dependent/api/v1/checkouts.t
Comment 14 Josef Moravec 2019-03-29 08:22:00 UTC
Created attachment 87155 [details] [review]
Bug 17003: Add API route to get checkout's renewability
Comment 15 Josef Moravec 2019-03-29 08:22:04 UTC
Created attachment 87156 [details] [review]
Bug 17003: Adapt to OpenAPI

Test plan:
prove t/db_dependent/api/v1/checkouts.t
Comment 16 Josef Moravec 2019-03-29 08:22:07 UTC
Created attachment 87157 [details] [review]
Bug 17003: (follow-up) Update definitions according to voted RFC

Test plan:
1) Have some patrons with checkouts, some renewable and some not
renewable
2) Use your favorite API tester and access GET
http://koha.url/api/v1/checkouts/{checkout_id}/allows_renewal
3) Check the response is OK according to voted RFC:
https://wiki.koha-community.org/wiki/Checkouts_endpoint_RFC#Checkout_renewability_2
4) prove t/db_dependent/api/v1/checkouts.t
Comment 17 Michal Denar 2019-04-05 11:45:18 UTC
Created attachment 87445 [details] [review]
Bug 17003: Adapt to OpenAPI

Test plan:
prove t/db_dependent/api/v1/checkouts.t

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 18 Michal Denar 2019-04-05 11:45:22 UTC
Created attachment 87446 [details] [review]
Bug 17003: (follow-up) Update definitions according to voted RFC

Test plan:
1) Have some patrons with checkouts, some renewable and some not
renewable
2) Use your favorite API tester and access GET
http://koha.url/api/v1/checkouts/{checkout_id}/allows_renewal
3) Check the response is OK according to voted RFC:
https://wiki.koha-community.org/wiki/Checkouts_endpoint_RFC#Checkout_renewability_2
4) prove t/db_dependent/api/v1/checkouts.t

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 19 Josef Moravec 2019-04-18 07:30:23 UTC
Created attachment 88260 [details] [review]
Bug 17003: Add API route to get checkout's renewability
Comment 20 Josef Moravec 2019-04-18 07:30:27 UTC
Created attachment 88261 [details] [review]
Bug 17003: Adapt to OpenAPI

Test plan:
prove t/db_dependent/api/v1/checkouts.t

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 21 Josef Moravec 2019-04-18 07:30:31 UTC
Created attachment 88262 [details] [review]
Bug 17003: (follow-up) Update definitions according to voted RFC

Test plan:
1) Have some patrons with checkouts, some renewable and some not
renewable
2) Use your favorite API tester and access GET
http://koha.url/api/v1/checkouts/{checkout_id}/allows_renewal
3) Check the response is OK according to voted RFC:
https://wiki.koha-community.org/wiki/Checkouts_endpoint_RFC#Checkout_renewability_2
4) prove t/db_dependent/api/v1/checkouts.t

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 22 Josef Moravec 2019-04-18 07:30:45 UTC
Just rebased
Comment 23 Johanna Räisä 2019-04-24 05:49:46 UTC
Created attachment 88589 [details] [review]
Bug 17003: Add API route to get checkout's renewability
Comment 24 Johanna Räisä 2019-04-24 05:51:32 UTC
Created attachment 88590 [details] [review]
Bug 17003: Adapt to OpenAPI

Test plan:
prove t/db_dependent/api/v1/checkouts.t

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Johanna Raisa <johanna.raisa@gmail.com>
Comment 25 Johanna Räisä 2019-04-24 05:53:55 UTC
Created attachment 88591 [details] [review]
Bug 17003: (follow-up) Update definitions according to  voted RFC

Test plan:
1) Have some patrons with checkouts, some renewable and some not
renewable
2) Use your favorite API tester and access GET
http://koha.url/api/v1/checkouts/{checkout_id}/allows_renewal
3) Check the response is OK according to voted RFC:
https://wiki.koha-community.org/wiki/Checkouts_endpoint_RFC#Checkout_renewability_2
4) prove t/db_dependent/api/v1/checkouts.t

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Johanna Raisa <johanna.raisa@gmail.com>
Comment 26 Kyle M Hall 2019-06-28 15:29:01 UTC
Created attachment 91102 [details] [review]
Bug 17003: Add API route to get checkout's renewability

Signed-off-by: Johanna Raisa <johanna.raisa@gmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 27 Kyle M Hall 2019-06-28 15:29:11 UTC
Created attachment 91103 [details] [review]
Bug 17003: Adapt to OpenAPI

Test plan:
prove t/db_dependent/api/v1/checkouts.t

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Johanna Raisa <johanna.raisa@gmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 28 Kyle M Hall 2019-06-28 15:29:14 UTC
Created attachment 91104 [details] [review]
Bug 17003: (follow-up) Update definitions according to voted RFC

Test plan:
1) Have some patrons with checkouts, some renewable and some not
renewable
2) Use your favorite API tester and access GET
http://koha.url/api/v1/checkouts/{checkout_id}/allows_renewal
3) Check the response is OK according to voted RFC:
https://wiki.koha-community.org/wiki/Checkouts_endpoint_RFC#Checkout_renewability_2
4) prove t/db_dependent/api/v1/checkouts.t

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Johanna Raisa <johanna.raisa@gmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 29 Kyle M Hall 2019-06-28 15:29:17 UTC
Created attachment 91105 [details] [review]
Bug 17003: (QA follow-up) Fix the number of tests

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 30 Martin Renvoize 2019-07-01 15:19:23 UTC
Nice work!

Pushed to master for 19.11.00
Comment 31 Josef Moravec 2019-07-10 11:00:26 UTC
Fridolin, are you going to backport this to 19.05?
Comment 32 Fridolin Somers 2019-07-25 12:15:33 UTC
Pushed to 19.05.x for 19.05.03

API is good ;)
Comment 33 Lucas Gass 2019-07-26 15:17:48 UTC
not backporting this enhancement to 18.11.x
Comment 34 Martin Renvoize 2019-11-14 11:05:53 UTC
Just working through bugs for the release notes.. Who sponsored this one?