Attachment 41164 Details for Bug 13903 – Bug 13903: 6. Add API routes to list, create, update, delete reserves

[patch] Bug 13903: 6. Add API routes to list, create, update, delete reserves

Bug-13903-6-Add-API-routes-to-list-create-update-d.patch (text/plain), 19.69 KB, created by Olli-Antti Kivilahti on 2015-07-24 14:06:51 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Olli-Antti Kivilahti

Created: 2015-07-24 14:06:51 UTC

Size: 19.69 KB

patch

obsolete

>From 0476c0477b920914d56d9e9cf88c76037e09f3bb Mon Sep 17 00:00:00 2001
>From: Julian Maurice <julian.maurice@biblibre.com>
>Date: Tue, 24 Mar 2015 11:30:00 +0100
>Subject: [PATCH] Bug 13903: 6. Add API routes to list, create, update, delete
> reserves
>
>GET    /borrowers/reserves    (list)
>POST   /borrowers/reserves    (create)
>PUT    /reserves/{reserve_id} (update)
>DELETE /reserves/{reserve_id} (delete)
>
>Unit tests in t/db_dependent/api/v1/borrowers/reserves.t and
>t/db_dependent/api/v1/reserves.t
>
>Test plan:
>1/ Go to http://api.yourlibrary/v1/doc and test the above routes with
>different values for each parameter
>2/ Try to make reserves until the maximum number of reserves for a user
>is reached (you should have a 403 error then)
>3/ Run unit tests
>
>Depends on buugg 13799
>---
> C4/Reserves.pm                             |  23 +--
> Koha/REST/V1/Borrowers/Reserves.pm         |  99 +++++++++++++
> Koha/REST/V1/Reserves.pm                   |  57 ++++++++
> api/v1/swagger.json                        | 226 ++++++++++++++++++++++++++++-
> t/db_dependent/api/v1/borrowers/reserves.t |  99 +++++++++++++
> t/db_dependent/api/v1/reserves.t           |  91 ++++++++++++
> 6 files changed, 583 insertions(+), 12 deletions(-)
> create mode 100644 Koha/REST/V1/Borrowers/Reserves.pm
> create mode 100644 Koha/REST/V1/Reserves.pm
> create mode 100644 t/db_dependent/api/v1/borrowers/reserves.t
> create mode 100644 t/db_dependent/api/v1/reserves.t
>
>diff --git a/C4/Reserves.pm b/C4/Reserves.pm
>index 6252b86..3ab5d0a 100644
>--- a/C4/Reserves.pm
>+++ b/C4/Reserves.pm
>@@ -241,18 +241,19 @@ sub AddReserve {
>     }
> 
>     #}
>-    ($const eq "o" || $const eq "e") or return;   # FIXME: why not have a useful return value?
>-    $query = qq{
>-        INSERT INTO reserveconstraints
>-            (borrowernumber,biblionumber,reservedate,biblioitemnumber)
>-        VALUES
>-            (?,?,?,?)
>-    };
>-    $sth = $dbh->prepare($query);    # keep prepare outside the loop!
>-    foreach (@$bibitems) {
>-        $sth->execute($borrowernumber, $biblionumber, $resdate, $_);
>+    if ($const eq 'o' || $const eq 'e') {
>+        $query = qq{
>+            INSERT INTO reserveconstraints
>+                (borrowernumber,biblionumber,reservedate,biblioitemnumber)
>+            VALUES
>+                (?,?,?,?)
>+        };
>+        $sth = $dbh->prepare($query);    # keep prepare outside the loop!
>+        foreach (@$bibitems) {
>+            $sth->execute($borrowernumber, $biblionumber, $resdate, $_);
>+        }
>     }
>-        
>+
>     return $reserve_id;
> }
> 
>diff --git a/Koha/REST/V1/Borrowers/Reserves.pm b/Koha/REST/V1/Borrowers/Reserves.pm
>new file mode 100644
>index 0000000..4dca9b7
>--- /dev/null
>+++ b/Koha/REST/V1/Borrowers/Reserves.pm
>@@ -0,0 +1,99 @@
>+package Koha::REST::V1::Borrowers::Reserves;
>+
>+use Modern::Perl;
>+
>+use Mojo::Base 'Mojolicious::Controller';
>+
>+use Koha::Borrowers;
>+use Koha::Borrower;
>+
>+use C4::Biblio;
>+use C4::Dates;
>+use C4::Reserves;
>+
>+sub list_borrower_reserves {
>+    my ($c, $args, $cb) = @_;
>+
>+    my $borrowernumber = $args->{borrowernumber};
>+    my $borrower = Koha::Borrowers->find($borrowernumber);
>+    unless ($borrower) {
>+        return $c->$cb({error => "Borrower not found"}, 404);
>+    }
>+
>+    my @reserves = C4::Reserves::GetReservesFromBorrowernumber($borrowernumber);
>+
>+    return $c->$cb(\@reserves, 200);
>+}
>+
>+sub add_borrower_reserve {
>+    my ($c, $args, $cb) = @_;
>+
>+    my $borrowernumber = $args->{borrowernumber};
>+    my $borrower = Koha::Borrowers->find($borrowernumber);
>+    unless ($borrower) {
>+        return $c->$cb({error => "Borrower not found"}, 404);
>+    }
>+
>+    my $body = $c->req->json;
>+
>+    my $biblionumber = $body->{biblionumber};
>+    my $itemnumber = $body->{itemnumber};
>+    my $branchcode = $body->{branchcode};
>+    my $expirationdate = $body->{expirationdate};
>+
>+    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);
>+    }
>+
>+    my $priority = C4::Reserves::CalculatePriority($biblionumber);
>+    $itemnumber ||= undef;
>+
>+    # AddReserve expects date to be in syspref format
>+    if ($expirationdate) {
>+        $expirationdate = C4::Dates->new($expirationdate, 'iso')->output;
>+    }
>+
>+    my $reserve_id = C4::Reserves::AddReserve($branchcode, $borrowernumber,
>+        $biblionumber, 'a', 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);
>+}
>+
>+1;
>diff --git a/Koha/REST/V1/Reserves.pm b/Koha/REST/V1/Reserves.pm
>new file mode 100644
>index 0000000..e1c32bc
>--- /dev/null
>+++ b/Koha/REST/V1/Reserves.pm
>@@ -0,0 +1,57 @@
>+package Koha::REST::V1::Reserves;
>+
>+use Modern::Perl;
>+
>+use Mojo::Base 'Mojolicious::Controller';
>+
>+use C4::Dates;
>+use C4::Reserves;
>+
>+sub edit_reserve {
>+    my ($c, $args, $cb) = @_;
>+
>+    my $reserve_id = $args->{reserve_id};
>+    my $reserve = C4::Reserves::GetReserve($reserve_id);
>+
>+    unless ($reserve) {
>+        return $c->$cb({error => "Reserve not found"}, 404);
>+    }
>+
>+    my $body = $c->req->json;
>+
>+    my $branchcode = $body->{branchcode};
>+    my $priority = $body->{priority};
>+    my $suspend_until = $body->{suspend_until};
>+
>+    if ($suspend_until) {
>+        $suspend_until = C4::Dates->new($suspend_until, 'iso')->output;
>+    }
>+
>+    my $params = {
>+        reserve_id => $reserve_id,
>+        branchcode => $branchcode,
>+        rank => $priority,
>+        suspend_until => $suspend_until,
>+    };
>+    C4::Reserves::ModReserve($params);
>+    $reserve = C4::Reserves::GetReserve($reserve_id);
>+
>+    return $c->$cb($reserve, 200);
>+}
>+
>+sub delete_reserve {
>+    my ($c, $args, $cb) = @_;
>+
>+    my $reserve_id = $args->{reserve_id};
>+    my $reserve = C4::Reserves::GetReserve($reserve_id);
>+
>+    unless ($reserve) {
>+        return $c->$cb({error => "Reserve not found"}, 404);
>+    }
>+
>+    C4::Reserves::CancelReserve({ reserve_id => $reserve_id });
>+
>+    return $c->$cb(undef, 204);
>+}
>+
>+1;
>diff --git a/api/v1/swagger.json b/api/v1/swagger.json
>index a8f7bf7..afec513 100644
>--- a/api/v1/swagger.json
>+++ b/api/v1/swagger.json
>@@ -138,6 +138,120 @@
>           }
>         }
>       }
>+    },
>+    "/borrowers/{borrowernumber}/reserves": {
>+      "get": {
>+        "x-mojo-controller": "Koha::REST::V1::Borrowers::Reserves",
>+        "operationId": "listBorrowerReserves",
>+        "tags": ["borrowers", "reserves"],
>+        "parameters": [
>+          { "$ref": "#/parameters/borrowernumberPathParam" }
>+        ],
>+        "produces": ["application/json"],
>+        "responses": {
>+          "200": {
>+            "description": "A list of reserves",
>+            "schema": { "$ref": "#/definitions/reserves" }
>+          },
>+          "404": {
>+            "description": "Borrower not found",
>+            "schema": { "$ref": "#/definitions/error" }
>+          }
>+        }
>+      },
>+      "post": {
>+        "x-mojo-controller": "Koha::REST::V1::Borrowers::Reserves",
>+        "operationId": "addBorrowerReserve",
>+        "tags": ["borrowers", "reserves"],
>+        "parameters": [
>+          { "$ref": "#/parameters/borrowernumberPathParam" },
>+          {
>+            "name": "body",
>+            "in": "body",
>+            "description": "A JSON object containing informations about the new reserve",
>+            "required": true,
>+            "schema": { "$ref": "#/definitions/newBorrowerReserveBody" }
>+          }
>+        ],
>+        "consumes": ["application/json"],
>+        "produces": ["application/json"],
>+        "responses": {
>+          "201": {
>+            "description": "Created reserve",
>+            "schema": { "$ref": "#/definitions/reserve" }
>+          },
>+          "400": {
>+            "description": "Missing or wrong parameters",
>+            "schema": { "$ref": "#/definitions/error" }
>+          },
>+          "403": {
>+            "description": "Reserve not allowed",
>+            "schema": { "$ref": "#/definitions/error" }
>+          },
>+          "404": {
>+            "description": "Borrower not found",
>+            "schema": { "$ref": "#/definitions/error" }
>+          },
>+          "500": {
>+            "description": "Internal error",
>+            "schema": { "$ref": "#/definitions/error" }
>+          }
>+        }
>+      }
>+    },
>+    "/reserves/{reserve_id}": {
>+      "put": {
>+        "x-mojo-controller": "Koha::REST::V1::Reserves",
>+        "operationId": "editReserve",
>+        "tags": ["reserves"],
>+        "parameters": [
>+          { "$ref": "#/parameters/reserveIdPathParam" },
>+          {
>+            "name": "body",
>+            "in": "body",
>+            "description": "A JSON object containing fields to modify",
>+            "required": true,
>+            "schema": { "$ref": "#/definitions/editReserveBody" }
>+          }
>+        ],
>+        "consumes": ["application/json"],
>+        "produces": ["application/json"],
>+        "responses": {
>+          "200": {
>+            "description": "Updated reserve",
>+            "schema": { "$ref": "#/definitions/reserve" }
>+          },
>+          "400": {
>+            "description": "Missing or wrong parameters",
>+            "schema": { "$ref": "#/definitions/error" }
>+          },
>+          "404": {
>+            "description": "Reserve not found",
>+            "schema": { "$ref": "#/definitions/error" }
>+          }
>+        }
>+      },
>+      "delete": {
>+        "x-mojo-controller": "Koha::REST::V1::Reserves",
>+        "operationId": "deleteReserve",
>+        "tags": ["reserves"],
>+        "parameters": [
>+          { "$ref": "#/parameters/reserveIdPathParam" }
>+        ],
>+        "produces": ["application/json"],
>+        "responses": {
>+          "204": {
>+            "description": "No content",
>+            "schema": {
>+              "type": "null"
>+            }
>+          },
>+          "404": {
>+            "description": "Reserve not found",
>+            "schema": { "$ref": "#/definitions/error" }
>+          }
>+        }
>+      }
>     }
>   },
>   "definitions": {
>@@ -208,6 +322,109 @@
>         }
>       }
>     },
>+    "reserves": {
>+      "type": "array",
>+      "items": { "$ref": "#/definitions/reserve" }
>+    },
>+    "reserve": {
>+      "type": "object",
>+      "properties": {
>+        "reserve_id": {
>+          "description": "Internal reserve identifier"
>+        },
>+        "borrowernumber": { "$ref" : "#/definitions/borrowernumber" },
>+        "reservedate": {
>+          "description": "the date the reserve was placed"
>+        },
>+        "biblionumber": { "$ref": "#/definitions/biblionumber" },
>+        "constrainttype": {
>+          "description": "type of constraint"
>+        },
>+        "branchcode": { "$ref": "#/definitions/branchcode" },
>+        "notificationdate": {
>+          "description": "currently unused"
>+        },
>+        "reminderdate": {
>+          "description": "currently unused"
>+        },
>+        "cancellationdate": {
>+          "description": "the date the reserve was cancelled"
>+        },
>+        "reservenotes": {
>+          "description": "notes related to this reserve"
>+        },
>+        "priority": {
>+          "description": "where in the queue the patron sits"
>+        },
>+        "found": {
>+          "description": "a one letter code defining what the status of the reserve is after it has been confirmed"
>+        },
>+        "timestamp": {
>+          "description": "date and time the reserve was last updated"
>+        },
>+        "itemnumber": { "$ref": "#/definitions/itemnumber" },
>+        "waitingdate": {
>+          "description": "the date the item was marked as waiting for the patron at the library"
>+        },
>+        "expirationdate": {
>+          "description": "the date the reserve expires"
>+        },
>+        "lowestPriority": {
>+          "description": ""
>+        },
>+        "suspend": {
>+          "description": ""
>+        },
>+        "suspend_until": {
>+          "description": ""
>+        }
>+      }
>+    },
>+    "newBorrowerReserveBody": {
>+      "type": "object",
>+      "properties": {
>+        "biblionumber": {
>+          "description": "Biblio internal identifier",
>+          "type": "integer"
>+        },
>+        "itemnumber": {
>+          "description": "Item internal identifier",
>+          "type": "integer"
>+        },
>+        "branchcode": {
>+          "description": "Pickup location",
>+          "type": "string",
>+          "required": true
>+        },
>+        "expirationdate": {
>+          "description": "Reserve end date",
>+          "type": "string",
>+          "format": "date"
>+        }
>+      }
>+    },
>+    "editReserveBody": {
>+      "type": "object",
>+      "properties": {
>+        "priority": {
>+          "description": "Position in waiting queue",
>+          "type": "integer",
>+          "minimum": 1
>+        },
>+        "branchcode": {
>+          "description": "Pickup location",
>+          "type": "string"
>+        },
>+        "suspend_until": {
>+          "description": "Suspend until",
>+          "type": "string",
>+          "format": "date"
>+        }
>+      }
>+    },
>+    "biblionumber": {
>+      "description": "Biblio internal identifier"
>+    },
>     "borrowernumber": {
>       "description": "Borrower internal identifier"
>     },
>@@ -232,7 +449,14 @@
>       "name": "borrowernumber",
>       "in": "path",
>       "description": "Internal borrower identifier",
>-      "required": "true",
>+      "required": true,
>+      "type": "integer"
>+    },
>+    "reserveIdPathParam": {
>+      "name": "reserve_id",
>+      "in": "path",
>+      "description": "Internal reserve identifier",
>+      "required": true,
>       "type": "integer"
>     },
>     "itemnumberPathParam": {
>diff --git a/t/db_dependent/api/v1/borrowers/reserves.t b/t/db_dependent/api/v1/borrowers/reserves.t
>new file mode 100644
>index 0000000..bec26e4
>--- /dev/null
>+++ b/t/db_dependent/api/v1/borrowers/reserves.t
>@@ -0,0 +1,99 @@
>+#!/usr/bin/env perl
>+
>+use Modern::Perl;
>+
>+use Test::More tests => 17;
>+use Test::Mojo;
>+
>+use DateTime;
>+
>+use C4::Context;
>+use C4::Biblio;
>+use C4::Items;
>+
>+use Koha::Database;
>+use Koha::Borrower;
>+
>+my $dbh = C4::Context->dbh;
>+$dbh->{AutoCommit} = 0;
>+$dbh->{RaiseError} = 1;
>+
>+my $t = Test::Mojo->new('Koha::REST::V1');
>+
>+my $categorycode = Koha::Database->new()->schema()->resultset('Category')->first()->categorycode();
>+my $branchcode = Koha::Database->new()->schema()->resultset('Branch')->first()->branchcode();
>+
>+my $borrower = Koha::Borrower->new;
>+$borrower->categorycode( $categorycode );
>+$borrower->branchcode( $branchcode );
>+$borrower->surname("Test Surname");
>+$borrower->store;
>+my $borrowernumber = $borrower->borrowernumber;
>+
>+$t->get_ok("/v1/borrowers/$borrowernumber/reserves")
>+  ->status_is(200)
>+  ->json_is([]);
>+
>+my $inexisting_borrowernumber = $borrowernumber + 1;
>+$t->get_ok("/v1/borrowers/$inexisting_borrowernumber/reserves")
>+  ->status_is(404)
>+  ->json_has('/error');
>+
>+my $biblionumber = create_biblio('RESTful Web APIs');
>+my $itemnumber = create_item($biblionumber, 'TEST000001');
>+
>+$dbh->do('DELETE FROM issuingrules');
>+$dbh->do(q{
>+    INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed)
>+    VALUES (?, ?, ?, ?)
>+}, {}, '*', '*', '*', 1);
>+
>+my $expirationdate = DateTime->now->add(days => 10)->ymd;
>+my $post_data = {
>+    biblionumber => int($biblionumber),
>+    itemnumber => int($itemnumber),
>+    branchcode => $branchcode,
>+    expirationdate => $expirationdate,
>+};
>+$t->post_ok("/v1/borrowers/$borrowernumber/reserves" => json => $post_data)
>+  ->status_is(201)
>+  ->json_has('/reserve_id');
>+
>+my $reserve_id = $t->tx->res->json->{reserve_id};
>+
>+$t->get_ok("/v1/borrowers/$borrowernumber/reserves")
>+  ->status_is(200)
>+  ->json_is('/0/reserve_id', $reserve_id)
>+  ->json_is('/0/expirationdate', $expirationdate)
>+  ->json_is('/0/branchcode', $branchcode);
>+
>+$t->post_ok("/v1/borrowers/$borrowernumber/reserves" => json => $post_data)
>+  ->status_is(403)
>+  ->json_like('/error', qr/tooManyReserves/);
>+
>+$dbh->rollback;
>+
>+sub create_biblio {
>+    my ($title) = @_;
>+
>+    my $record = new MARC::Record;
>+    $record->append_fields(
>+        new MARC::Field('200', ' ', ' ', a => $title),
>+    );
>+
>+    my ($biblionumber) = C4::Biblio::AddBiblio($record, '');
>+
>+    return $biblionumber;
>+}
>+
>+sub create_item {
>+    my ($biblionumber, $barcode) = @_;
>+
>+    my $item = {
>+        barcode => $barcode,
>+    };
>+
>+    my $itemnumber = C4::Items::AddItem($item, $biblionumber);
>+
>+    return $itemnumber;
>+}
>diff --git a/t/db_dependent/api/v1/reserves.t b/t/db_dependent/api/v1/reserves.t
>new file mode 100644
>index 0000000..bff5d63
>--- /dev/null
>+++ b/t/db_dependent/api/v1/reserves.t
>@@ -0,0 +1,91 @@
>+#!/usr/bin/env perl
>+
>+use Modern::Perl;
>+
>+use Test::More tests => 13;
>+use Test::Mojo;
>+
>+use DateTime;
>+
>+use C4::Context;
>+use C4::Biblio;
>+use C4::Items;
>+use C4::Reserves;
>+
>+use Koha::Database;
>+use Koha::Borrower;
>+
>+my $dbh = C4::Context->dbh;
>+$dbh->{AutoCommit} = 0;
>+$dbh->{RaiseError} = 1;
>+
>+my $t = Test::Mojo->new('Koha::REST::V1');
>+
>+my $categorycode = Koha::Database->new()->schema()->resultset('Category')->first()->categorycode();
>+my $branchcode = Koha::Database->new()->schema()->resultset('Branch')->first()->branchcode();
>+
>+my $borrower = Koha::Borrower->new;
>+$borrower->categorycode( $categorycode );
>+$borrower->branchcode( $branchcode );
>+$borrower->surname("Test Surname");
>+$borrower->store;
>+my $borrowernumber = $borrower->borrowernumber;
>+
>+my $biblionumber = create_biblio('RESTful Web APIs');
>+my $itemnumber = create_item($biblionumber, 'TEST000001');
>+
>+my $reserve_id = C4::Reserves::AddReserve($branchcode, $borrowernumber,
>+    $biblionumber, 'a', undef, 1, undef, undef, undef, '', $itemnumber);
>+
>+# Add another reserve to be able to change first reserve's rank
>+C4::Reserves::AddReserve($branchcode, $borrowernumber,
>+    $biblionumber, 'a', undef, 2, undef, undef, undef, '', $itemnumber);
>+
>+my $suspend_until = DateTime->now->add(days => 10)->ymd;
>+my $put_data = {
>+    priority => 2,
>+    suspend_until => $suspend_until,
>+};
>+$t->put_ok("/v1/reserves/$reserve_id" => json => $put_data)
>+  ->status_is(200)
>+  ->json_is('/reserve_id', $reserve_id)
>+  ->json_is('/suspend_until', $suspend_until . ' 00:00:00')
>+  ->json_is('/priority', 2);
>+
>+$t->delete_ok("/v1/reserves/$reserve_id")
>+  ->status_is(204);
>+
>+$t->put_ok("/v1/reserves/$reserve_id" => json => $put_data)
>+  ->status_is(404)
>+  ->json_has('/error');
>+
>+$t->delete_ok("/v1/reserves/$reserve_id")
>+  ->status_is(404)
>+  ->json_has('/error');
>+
>+$dbh->rollback;
>+
>+sub create_biblio {
>+    my ($title) = @_;
>+
>+    my $record = new MARC::Record;
>+    $record->append_fields(
>+        new MARC::Field('200', ' ', ' ', a => $title),
>+    );
>+
>+    my ($biblionumber) = C4::Biblio::AddBiblio($record, '');
>+
>+    return $biblionumber;
>+}
>+
>+sub create_item {
>+    my ($biblionumber, $barcode) = @_;
>+
>+    my $item = {
>+        barcode => $barcode,
>+    };
>+
>+    my $itemnumber = C4::Items::AddItem($item, $biblionumber);
>+
>+    return $itemnumber;
>+}
>-- 
>1.9.1

Actions: View | Diff | Splinter Review

Attachments on bug 13903: 37210 | 40589 | 41164 | 41549 | 45842 | 49843 | 50852 | 50853 | 50959