From 192f8499e9232974cacd1c3dc873f955210ec114 Mon Sep 17 00:00:00 2001 From: Julian Maurice Date: Tue, 24 Mar 2015 11:30:00 +0100 Subject: [PATCH] Bug 13903: 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 bug 13799 --- C4/Reserves.pm | 23 +-- Koha/REST/V1/Borrowers/Reserves.pm | 99 ++++++++++++ Koha/REST/V1/Reserves.pm | 57 +++++++ api/v1/swagger.json | 232 ++++++++++++++++++++++++++++- t/db_dependent/api/v1/borrowers/reserves.t | 99 ++++++++++++ t/db_dependent/api/v1/reserves.t | 91 +++++++++++ 6 files changed, 589 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 f405262..6c54918 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -239,18 +239,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 5e97d03..6a252a7 100644 --- a/api/v1/swagger.json +++ b/api/v1/swagger.json @@ -63,6 +63,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": { @@ -83,9 +197,118 @@ } } }, + "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" }, + "branchcode": { + "description": "Branch code" + }, + "itemnumber": { + "description": "Item internal identifier" + }, "error": { "type": "object", "properties": { @@ -101,7 +324,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" } } 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