Bug 17003 - REST API: add route to get checkout's renewability
Summary: REST API: add route to get checkout's renewability
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: REST api (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Target Milestone: ---
Assignee: Josef Moravec
QA Contact:
URL: https://wiki.koha-community.org/wiki/...
Whiteboard:
Keywords:
Depends on: 13895 22607
Blocks: 16652 17005 18407
  Show dependency treegraph
 
Reported: 2016-07-31 10:59 UTC by Jiri Kozlovsky
Modified: 2019-04-24 05:53 UTC (History)
6 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 17003: Add API route to get checkout's renewability (5.47 KB, patch)
2016-08-22 14:43 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17003: Add API route to get checkout's renewability (5.24 KB, patch)
2016-08-22 14:44 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17003: Add API route to get checkout's renewability (5.69 KB, patch)
2016-10-14 11:08 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17003: Add API route to get checkout's renewability (6.07 KB, patch)
2016-11-15 12:17 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17003: Add API route to get checkout's renewability (6.75 KB, patch)
2016-11-15 14:38 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17003: Add API route to get checkout's renewability (7.33 KB, patch)
2016-12-11 21:42 UTC, Jiri Kozlovsky
Details | Diff | Splinter Review
Bug 17003: Add API route to get checkout's renewability (7.29 KB, patch)
2019-02-21 18:43 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17003: Adapt to OpenAPI (4.31 KB, patch)
2019-02-21 18:43 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17003: Add API route to get checkout's renewability (6.41 KB, patch)
2019-03-29 08:22 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17003: Adapt to OpenAPI (4.33 KB, patch)
2019-03-29 08:22 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17003: (follow-up) Update definitions according to voted RFC (7.65 KB, patch)
2019-03-29 08:22 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17003: Adapt to OpenAPI (4.43 KB, patch)
2019-04-05 11:45 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 17003: (follow-up) Update definitions according to voted RFC (7.75 KB, patch)
2019-04-05 11:45 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 17003: Add API route to get checkout's renewability (6.37 KB, patch)
2019-04-18 07:30 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17003: Adapt to OpenAPI (4.43 KB, patch)
2019-04-18 07:30 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17003: (follow-up) Update definitions according to voted RFC (7.75 KB, patch)
2019-04-18 07:30 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17003: Add API route to get checkout's renewability (6.40 KB, patch)
2019-04-24 05:49 UTC, Johanna Räisä
Details | Diff | Splinter Review
Bug 17003: Adapt to OpenAPI (4.48 KB, patch)
2019-04-24 05:51 UTC, Johanna Räisä
Details | Diff | Splinter Review
Bug 17003: (follow-up) Update definitions according to voted RFC (7.78 KB, patch)
2019-04-24 05:53 UTC, Johanna Räisä
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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>