Attachment 62601 Details for Bug 18409 – Bug 18409: Make the controller for holds use Koha::Holds

[patch] Bug 18409: Make the controller for holds use Koha::Holds

Bug-18409-Make-the-controller-for-holds-use-KohaHo.patch (text/plain), 3.76 KB, created by Kyle M Hall (khall) on 2017-04-24 13:44:35 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Kyle M Hall (khall)

Created: 2017-04-24 13:44:35 UTC

Size: 3.76 KB

patch

obsolete

>From 694c09aa873973c4f55b5d90948b1027adc0f1c1 Mon Sep 17 00:00:00 2001
>From: Tomas Cohen Arazi <tomascohen@theke.io>
>Date: Fri, 21 Apr 2017 20:34:00 -0300
>Subject: [PATCH] Bug 18409: Make the controller for holds use Koha::Holds
>
>Recently, there's been a major fix on the REST api swagger spec,
>which involved fixing boolean values so they are actually booleans
>and Koha::Object was extended to handle that.
>While the swagger spec for this endpoint got fixed, such is not the case
>with the implementation (the controller class).
>
>This patch fixes this situation by:
>- Specifying boolean properties as boolean in the schema file
>- Fixes the controller so it returns Koha::Hold objects instead of the
>  hashref returned by GetReserve, which is wrong.
>- Better (than empty) descriptions are added to 'suspend',
>  'suspend_until' and 'lowestPriority' on the spec.
>
>To test:
>- Run:
>  $ sudo koha-shell kohadev
> k$ cd kohaclone
> k$ prove t/db_dependent/api/v1/holds.t
>=> FAIL: Tests fail, mostly due to error 500 results.
>- Apply this patch
>- Run:
> k$ prove t/db_dependent/api/v1/holds.t
>=> SUCCESS: Tests pass!
>- Sign off :-D
>
>This can also be tested using any interface for REST apis.
>
>Note: This endpoint lacks several of the new guidelines and is not
>complete (there's no GET for single holds, etc). It is also missing
>exception handling. There are probably
>other bug reports for that, just thought it was worth mentioning.
>
>Followed test plan and this patch worked as intended
>Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
>
>Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
>---
> Koha/REST/V1/Hold.pm                 | 7 ++++---
> Koha/Schema/Result/Reserve.pm        | 5 +++++
> api/v1/swagger/definitions/hold.json | 6 +++---
> 3 files changed, 12 insertions(+), 6 deletions(-)
>
>diff --git a/Koha/REST/V1/Hold.pm b/Koha/REST/V1/Hold.pm
>index 7f0b1ff..d72e5df 100644
>--- a/Koha/REST/V1/Hold.pm
>+++ b/Koha/REST/V1/Hold.pm
>@@ -34,7 +34,7 @@ sub list {
>     foreach my $key (keys %$params) {
>         delete $params->{$key} unless grep { $key eq $_ } @valid_params;
>     }
>-    my $holds = Koha::Holds->search($params)->unblessed;
>+    my $holds = Koha::Holds->search($params);
> 
>     return $c->$cb($holds, 200);
> }
>@@ -106,7 +106,7 @@ sub add {
>         }, 500);
>     }
> 
>-    my $reserve = C4::Reserves::GetReserve($reserve_id);
>+    my $reserve = Koha::Holds->find($reserve_id);
> 
>     return $c->$cb($reserve, 201);
> }
>@@ -137,8 +137,9 @@ sub edit {
>         rank => $priority,
>         suspend_until => $suspend_until,
>     };
>+
>     C4::Reserves::ModReserve($params);
>-    $reserve = C4::Reserves::GetReserve($reserve_id);
>+    $reserve = Koha::Holds->find($reserve_id);
> 
>     return $c->$cb($reserve, 200);
> }
>diff --git a/Koha/Schema/Result/Reserve.pm b/Koha/Schema/Result/Reserve.pm
>index 4d56006..eb5d07f 100644
>--- a/Koha/Schema/Result/Reserve.pm
>+++ b/Koha/Schema/Result/Reserve.pm
>@@ -334,4 +334,9 @@ __PACKAGE__->belongs_to(
>   },
> );
> 
>+__PACKAGE__->add_columns(
>+    '+lowestPriority' => { is_boolean => 1 },
>+    '+suspend' => { is_boolean => 1 }
>+);
>+
> 1;
>diff --git a/api/v1/swagger/definitions/hold.json b/api/v1/swagger/definitions/hold.json
>index 5f41318..8f91aed 100644
>--- a/api/v1/swagger/definitions/hold.json
>+++ b/api/v1/swagger/definitions/hold.json
>@@ -59,15 +59,15 @@
>     },
>     "lowestPriority": {
>       "type": "boolean",
>-      "description": ""
>+      "description": "Controls if the hold is given the lowest priority on the queue"
>     },
>     "suspend": {
>       "type": "boolean",
>-      "description": ""
>+      "description": "Controls if the hold is suspended"
>     },
>     "suspend_until": {
>       "type": ["string", "null"],
>-      "description": ""
>+      "description": "Date until which the hold has been suspended"
>     },
>     "itemtype": {
>       "type": ["string", "null"],
>-- 
>2.10.2

Actions: View | Diff | Splinter Review

Attachments on bug 18409: 62564 | 62568 | 62601