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 7f0b1ff269..d72e5df829 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 4d560069ac..eb5d07fa8a 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 5f4131860d..8f91aed964 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.39.5