Attachment 48447 Details for Bug 15566 – Bug 15566: API - allow to reserve first available from a group of titles

[patch] Bug 15566: API - allow to reserve first available from a group of titles

Bug-15566-API---allow-to-reserve-first-available-f.patch (text/plain), 12.08 KB, created by Julian Maurice on 2016-02-29 12:42:08 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Julian Maurice

Created: 2016-02-29 12:42:08 UTC

Size: 12.08 KB

patch

obsolete

>From 6b29095a375e29142fcf63486a2524cf858764db Mon Sep 17 00:00:00 2001
>From: Julian Maurice <julian.maurice@biblibre.com>
>Date: Mon, 29 Feb 2016 10:07:06 +0100
>Subject: [PATCH] Bug 15566: API - allow to reserve first available from a
> group of titles
>
>Test plan:
>0. Find 1 borrower and 2 biblios they can place a hold on. Let's call
>   them $borrowernumber, $biblionumber1 and $biblionumber2
>1. Make a POST request to http://your.library.org/api/v1/reserves with
>   the following body:
>     {
>       "onlyfirst": true,
>       "reserves": [
>         {
>           "borrowernumber": $borrowernumber,
>           "biblionumber": $biblionumber1
>         },
>         {
>           "borrowernumber": $borrowernumber,
>           "biblionumber": $biblionumber2
>         },
>       ]
>     }
>2. Check that you get a 201 response with the reserves informations
>3. Check in staff interface that the reserves were correctly made
>---
> Koha/REST/V1/Reserve.pm          | 137 ++++++++++++++++++++++-----------------
> api/v1/definitions/reserve.json  |   4 ++
> api/v1/swagger.json              |  56 ++++++++++------
> t/db_dependent/api/v1/reserves.t |  53 ++++++++++++---
> 4 files changed, 164 insertions(+), 86 deletions(-)
>
>diff --git a/Koha/REST/V1/Reserve.pm b/Koha/REST/V1/Reserve.pm
>index beb0abb..5339798 100644
>--- a/Koha/REST/V1/Reserve.pm
>+++ b/Koha/REST/V1/Reserve.pm
>@@ -44,70 +44,91 @@ sub add {
> 
>     my $body = $c->req->json;
> 
>-    my $borrowernumber = $body->{borrowernumber};
>-    my $biblionumber = $body->{biblionumber};
>-    my $itemnumber = $body->{itemnumber};
>-    my $branchcode = $body->{branchcode};
>-    my $expirationdate = $body->{expirationdate};
>-
>-    my $borrower = Koha::Borrowers->find($borrowernumber);
>-    unless ($borrower) {
>-        return $c->$cb({error => "Borrower not found"}, 404);
>-    }
>-
>-    unless ($biblionumber or $itemnumber) {
>-        return $c->$cb({
>-            error => "At least one of biblionumber, itemnumber should be given"
>-        }, 400);
>-    }
>-    unless ($branchcode) {
>-        return $c->$cb({
>-            error => "Branchcode is required"
>-        }, 400);
>-    }
>-
>-    my $item_biblionumber = C4::Biblio::GetBiblionumberFromItemnumber($itemnumber);
>-    if ($biblionumber and $biblionumber != $item_biblionumber) {
>-        return $c->$cb({
>-            error => "Item $itemnumber doesn't belong to biblio $biblionumber"
>-        }, 400);
>+    my $onlyfirst = $body->{onlyfirst};
>+    my $reserve_group_id;
>+    if ($onlyfirst) {
>+        $reserve_group_id = AddReserveGroup();
>     }
> 
>-    $biblionumber ||= $item_biblionumber;
>-    my $biblio = C4::Biblio::GetBiblio($biblionumber);
>-
>-    my $can_reserve =
>-      $itemnumber
>-      ? CanItemBeReserved( $borrowernumber, $itemnumber )
>-      : CanBookBeReserved( $borrowernumber, $biblionumber );
>-
>-    unless ($can_reserve eq 'OK') {
>-        return $c->$cb({
>-            error => "Reserve cannot be placed. Reason: $can_reserve"
>-        }, 403);
>+    my @possible_reserves;
>+    foreach my $reserve (@{ $body->{reserves} }) {
>+        my $borrowernumber = $reserve->{borrowernumber};
>+        my $biblionumber = $reserve->{biblionumber};
>+        my $itemnumber = $reserve->{itemnumber};
>+        my $branchcode = $reserve->{branchcode};
>+        my $expirationdate = $reserve->{expirationdate};
>+
>+        my $borrower = Koha::Borrowers->find($borrowernumber);
>+        unless ($borrower) {
>+            return $c->$cb({error => "Borrower not found"}, 404);
>+        }
>+
>+        unless ($biblionumber or $itemnumber) {
>+            return $c->$cb({
>+                error => "At least one of biblionumber, itemnumber should be given"
>+            }, 400);
>+        }
>+        unless ($branchcode) {
>+            return $c->$cb({
>+                error => "Branchcode is required"
>+            }, 400);
>+        }
>+
>+        my $item_biblionumber = C4::Biblio::GetBiblionumberFromItemnumber($itemnumber);
>+        if ($biblionumber and $biblionumber != $item_biblionumber) {
>+            return $c->$cb({
>+                error => "Item $itemnumber doesn't belong to biblio $biblionumber"
>+            }, 400);
>+        }
>+
>+        $biblionumber ||= $item_biblionumber;
>+        my $biblio = C4::Biblio::GetBiblio($biblionumber);
>+
>+        my $can_reserve =
>+          $itemnumber
>+          ? CanItemBeReserved( $borrowernumber, $itemnumber )
>+          : CanBookBeReserved( $borrowernumber, $biblionumber );
>+
>+        unless ($can_reserve eq 'OK') {
>+            return $c->$cb({
>+                error => "Reserve cannot be placed. Reason: $can_reserve"
>+            }, 403);
>+        }
>+
>+        $reserve->{title} = $biblio->{title};
>+        push @possible_reserves, $reserve;
>     }
> 
>-    my $priority = C4::Reserves::CalculatePriority($biblionumber);
>-    $itemnumber ||= undef;
>-
>-    # AddReserve expects date to be in syspref format
>-    if ($expirationdate) {
>-        $expirationdate = output_pref(dt_from_string($expirationdate, 'iso'));
>+    my @reserves;
>+    foreach my $reserve (@possible_reserves) {
>+        my $borrowernumber = $reserve->{borrowernumber};
>+        my $biblionumber = $reserve->{biblionumber};
>+        my $itemnumber = $reserve->{itemnumber};
>+        my $branchcode = $reserve->{branchcode};
>+        my $expirationdate = $reserve->{expirationdate};
>+
>+        my $priority = C4::Reserves::CalculatePriority($biblionumber);
>+        $itemnumber ||= undef;
>+
>+        # AddReserve expects date to be in syspref format
>+        if ($expirationdate) {
>+            $expirationdate = output_pref(dt_from_string($expirationdate, 'iso'));
>+        }
>+
>+        my $reserve_id = C4::Reserves::AddReserve($branchcode, $borrowernumber,
>+            $biblionumber, undef, $priority, undef, $expirationdate, undef,
>+            $reserve->{title}, $itemnumber, undef, $reserve_group_id);
>+
>+        unless ($reserve_id) {
>+            return $c->$cb({
>+                error => "Error while placing reserve. See Koha logs for details."
>+            }, 500);
>+        }
>+
>+        push @reserves, C4::Reserves::GetReserve($reserve_id);
>     }
> 
>-    my $reserve_id = C4::Reserves::AddReserve($branchcode, $borrowernumber,
>-        $biblionumber, undef, $priority, undef, $expirationdate, undef,
>-        $biblio->{title}, $itemnumber);
>-
>-    unless ($reserve_id) {
>-        return $c->$cb({
>-            error => "Error while placing reserve. See Koha logs for details."
>-        }, 500);
>-    }
>-
>-    my $reserve = C4::Reserves::GetReserve($reserve_id);
>-
>-    return $c->$cb($reserve, 201);
>+    return $c->$cb(\@reserves, 201);
> }
> 
> sub edit {
>diff --git a/api/v1/definitions/reserve.json b/api/v1/definitions/reserve.json
>index 74370fe..6db6f51 100644
>--- a/api/v1/definitions/reserve.json
>+++ b/api/v1/definitions/reserve.json
>@@ -58,6 +58,10 @@
>         },
>         "suspend_until": {
>             "description": ""
>+        },
>+        "reserve_group_id": {
>+            "description": "Identifier of the group the reserve belongs to",
>+            "type": ["string", "null"]
>         }
>     }
> }
>diff --git a/api/v1/swagger.json b/api/v1/swagger.json
>index b5c6d0a..35d1b2a 100644
>--- a/api/v1/swagger.json
>+++ b/api/v1/swagger.json
>@@ -111,26 +111,39 @@
>             "schema": {
>               "type": "object",
>               "properties": {
>-                "borrowernumber": {
>-                  "description": "Borrower internal identifier",
>-                  "type": "integer"
>+                "reserves": {
>+                  "description": "Reserves informations",
>+                  "type": "array",
>+                  "items": {
>+                    "type": "object",
>+                    "properties": {
>+                      "borrowernumber": {
>+                        "description": "Borrower internal identifier",
>+                        "type": "integer"
>+                      },
>+                      "biblionumber": {
>+                        "description": "Biblio internal identifier",
>+                        "type": "integer"
>+                      },
>+                      "itemnumber": {
>+                        "description": "Item internal identifier",
>+                        "type": "integer"
>+                      },
>+                      "branchcode": {
>+                        "description": "Pickup location",
>+                        "type": "string"
>+                      },
>+                      "expirationdate": {
>+                        "description": "Reserve end date",
>+                        "type": "string",
>+                        "format": "date"
>+                      }
>+                    }
>+                  }
>                 },
>-                "biblionumber": {
>-                  "description": "Biblio internal identifier",
>-                  "type": "integer"
>-                },
>-                "itemnumber": {
>-                  "description": "Item internal identifier",
>-                  "type": "integer"
>-                },
>-                "branchcode": {
>-                  "description": "Pickup location",
>-                  "type": "string"
>-                },
>-                "expirationdate": {
>-                  "description": "Reserve end date",
>-                  "type": "string",
>-                  "format": "date"
>+                "onlyfirst": {
>+                  "description": "Reserve only first available item",
>+                  "type": "boolean"
>                 }
>               }
>             }
>@@ -141,7 +154,10 @@
>         "responses": {
>           "201": {
>             "description": "Created reserve",
>-            "schema": { "$ref": "#/definitions/reserve" }
>+            "schema": {
>+              "type": "array",
>+              "items": { "$ref": "#/definitions/reserve" }
>+            }
>           },
>           "400": {
>             "description": "Missing or wrong parameters",
>diff --git a/t/db_dependent/api/v1/reserves.t b/t/db_dependent/api/v1/reserves.t
>index 9741542..c8d6815 100644
>--- a/t/db_dependent/api/v1/reserves.t
>+++ b/t/db_dependent/api/v1/reserves.t
>@@ -17,7 +17,7 @@
> 
> use Modern::Perl;
> 
>-use Test::More tests => 30;
>+use Test::More tests => 37;
> use Test::Mojo;
> 
> use DateTime;
>@@ -103,17 +103,21 @@ $dbh->do(q{
> 
> my $expirationdate = DateTime->now->add(days => 10)->ymd;
> my $post_data = {
>-    borrowernumber => int($borrowernumber),
>-    biblionumber => int($biblionumber),
>-    itemnumber => int($itemnumber),
>-    branchcode => $branchcode,
>-    expirationdate => $expirationdate,
>+    reserves => [
>+        {
>+            borrowernumber => int($borrowernumber),
>+            biblionumber => int($biblionumber),
>+            itemnumber => int($itemnumber),
>+            branchcode => $branchcode,
>+            expirationdate => $expirationdate,
>+        },
>+    ],
> };
> $t->post_ok("/api/v1/reserves" => json => $post_data)
>   ->status_is(201)
>-  ->json_has('/reserve_id');
>+  ->json_has('/0/reserve_id');
> 
>-$reserve_id = $t->tx->res->json->{reserve_id};
>+$reserve_id = $t->tx->res->json->[0]->{reserve_id};
> 
> $t->get_ok("/api/v1/reserves?borrowernumber=$borrowernumber")
>   ->status_is(200)
>@@ -125,6 +129,39 @@ $t->post_ok("/api/v1/reserves" => json => $post_data)
>   ->status_is(403)
>   ->json_like('/error', qr/tooManyReserves/);
> 
>+$t->delete_ok("/api/v1/reserves/$reserve_id")
>+  ->status_is(200);
>+
>+my $biblionumber2 = create_biblio('RESTful Web APIs 2');
>+my $itemnumber2 = create_item($biblionumber2, 'TEST000002');
>+
>+$post_data = {
>+    onlyfirst => \1,
>+    reserves => [
>+        {
>+            borrowernumber => int($borrowernumber),
>+            biblionumber => int($biblionumber),
>+            itemnumber => int($itemnumber),
>+            branchcode => $branchcode,
>+            expirationdate => $expirationdate,
>+        },
>+        {
>+            borrowernumber => int($borrowernumber),
>+            biblionumber => int($biblionumber2),
>+            itemnumber => int($itemnumber2),
>+            branchcode => $branchcode,
>+            expirationdate => $expirationdate,
>+        },
>+    ],
>+};
>+$t->post_ok("/api/v1/reserves" => json => $post_data)
>+  ->status_is(201);
>+
>+$t->get_ok("/api/v1/reserves?borrowernumber=$borrowernumber")
>+  ->status_is(200);
>+
>+my $json = $t->tx->res->json;
>+is($json->[0]->{reserve_group_id}, $json->[1]->{reserve_group_id}, "Both reserves belong to the same reserve group");
> 
> $dbh->rollback;
> 
>-- 
>1.9.1

Actions: View | Diff | Splinter Review

Attachments on bug 15566: 48447 | 50097