Bug 13895: (follow-up) Adapt checkout endpoint to openapi, update terminology
authorJosef Moravec <josef.moravec@gmail.com>
Mon, 4 Feb 2019 14:22:54 +0000 (14:22 +0000)
committerNick Clemens <nick@bywatersolutions.com>
Thu, 28 Mar 2019 19:38:41 +0000 (19:38 +0000)
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Koha/REST/V1/Checkout.pm
api/v1/swagger/definitions/checkout.json
api/v1/swagger/paths/checkouts.json
t/db_dependent/api/v1/checkouts.t

index 8c898ac..14a623b 100644 (file)
@@ -15,8 +15,6 @@ package Koha::REST::V1::Checkout;
 # with Koha; if not, write to the Free Software Foundation, Inc.,
 # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
 
-use Modern::Perl;
-
 use Mojo::Base 'Mojolicious::Controller';
 
 use C4::Auth qw( haspermission );
@@ -24,42 +22,84 @@ use C4::Context;
 use C4::Circulation;
 use Koha::Checkouts;
 
-sub list {
-    my ($c, $args, $cb) = @_;
+use Try::Tiny;
+
+=head1 NAME
+
+Koha::REST::V1::Checkout
+
+=head1 API
 
-    my $borrowernumber = $c->param('borrowernumber');
-    my $checkouts = C4::Circulation::GetIssues({
-        borrowernumber => $borrowernumber
-    });
+=head2 Methods
 
-    $c->$cb($checkouts, 200);
+=head3 list
+
+List Koha::Checkout objects
+
+=cut
+
+sub list {
+    my $c = shift->openapi->valid_input or return;
+    try {
+        my $checkouts_set = Koha::Checkouts->new;
+        my $checkouts = $c->objects->search( $checkouts_set, \&_to_model, \&_to_api );
+        return $c->render( status => 200, openapi => $checkouts );
+    } catch {
+        if ( $_->isa('DBIx::Class::Exception') ) {
+            return $c->render(
+                status => 500,
+                openapi => { error => $_->{msg} }
+            );
+        } else {
+            return $c->render(
+                status => 500,
+                openapi => { error => "Something went wrong, check the logs." }
+            );
+        }
+    };
 }
 
+=head3 get
+
+get one checkout
+
+=cut
+
 sub get {
-    my ($c, $args, $cb) = @_;
+    my $c = shift->openapi->valid_input or return;
 
-    my $checkout_id = $args->{checkout_id};
-    my $checkout = Koha::Checkouts->find($checkout_id);
+    my $checkout = Koha::Checkouts->find( $c->validation->param('checkout_id') );
 
-    if (!$checkout) {
-        return $c->$cb({
-            error => "Checkout doesn't exist"
-        }, 404);
+    unless ($checkout) {
+        return $c->render(
+            status => 404,
+            openapi => { error => "Checkout doesn't exist" }
+        );
     }
 
-    return $c->$cb($checkout->unblessed, 200);
+    return $c->render(
+        status => 200,
+        openapi => _to_api($checkout->TO_JSON)
+    );
 }
 
+=head3 renew
+
+Renew a checkout
+
+=cut
+
 sub renew {
-    my ($c, $args, $cb) = @_;
+    my $c = shift->openapi->valid_input or return;
 
-    my $checkout_id = $args->{checkout_id};
-    my $checkout = Koha::Checkouts->find($checkout_id);
+    my $checkout_id = $c->validation->param('checkout_id');
+    my $checkout = Koha::Checkouts->find( $checkout_id );
 
-    if (!$checkout) {
-        return $c->$cb({
-            error => "Checkout doesn't exist"
-        }, 404);
+    unless ($checkout) {
+        return $c->render(
+            status => 404,
+            openapi => { error => "Checkout doesn't exist" }
+        );
     }
 
     my $borrowernumber = $checkout->borrowernumber;
@@ -69,7 +109,10 @@ sub renew {
     unless (C4::Context->preference('OpacRenewalAllowed')) {
         my $user = $c->stash('koha.user');
         unless ($user && haspermission($user->userid, { circulate => "circulate_remaining_permissions" })) {
-            return $c->$cb({error => "Opac Renewal not allowed"}, 403);
+            return $c->render(
+                status => 403,
+                openapi => { error => "Opac Renewal not allowed"}
+            );
         }
     }
 
@@ -77,13 +120,100 @@ sub renew {
         $borrowernumber, $itemnumber);
 
     if (!$can_renew) {
-        return $c->$cb({error => "Renewal not authorized ($error)"}, 403);
+        return $c->render(
+            status => 403,
+            openapi => { error => "Renewal not authorized ($error)" }
+        );
     }
 
     AddRenewal($borrowernumber, $itemnumber, $checkout->branchcode);
     $checkout = Koha::Checkouts->find($checkout_id);
 
-    return $c->$cb($checkout->unblessed, 200);
+    return $c->render(
+        status => 200,
+        openapi => _to_api( $checkout->TO_JSON )
+    );
 }
 
+=head3 _to_api
+
+Helper function that maps a hashref of Koha::Checkout attributes into REST api
+attribute names.
+
+=cut
+
+sub _to_api {
+    my $checkout = shift;
+
+    foreach my $column ( keys %{ $Koha::REST::V1::Checkout::to_api_mapping } ) {
+        my $mapped_column = $Koha::REST::V1::Checkout::to_api_mapping->{$column};
+        if ( exists $checkout->{ $column } && defined $mapped_column )
+        {
+            $checkout->{ $mapped_column } = delete $checkout->{ $column };
+        }
+        elsif ( exists $checkout->{ $column } && !defined $mapped_column ) {
+            delete $checkout->{ $column };
+        }
+    }
+    return $checkout;
+}
+
+=head3 _to_model
+
+Helper function that maps REST api objects into Koha::Checkouts
+attribute names.
+
+=cut
+
+sub _to_model {
+    my $checkout = shift;
+
+    foreach my $attribute ( keys %{ $Koha::REST::V1::Checkout::to_model_mapping } ) {
+        my $mapped_attribute = $Koha::REST::V1::Checkout::to_model_mapping->{$attribute};
+        if ( exists $checkout->{ $attribute } && defined $mapped_attribute )
+        {
+            $checkout->{ $mapped_attribute } = delete $checkout->{ $attribute };
+        }
+        elsif ( exists $checkout->{ $attribute } && !defined $mapped_attribute )
+        {
+            delete $checkout->{ $attribute };
+        }
+    }
+    return $checkout;
+}
+
+=head2 Global variables
+
+=head3 $to_api_mapping
+
+=cut
+
+our $to_api_mapping = {
+    issue_id        => 'checkout_id',
+    borrowernumber  => 'patron_id',
+    itemnumber      => 'item_id',
+    date_due        => 'due_date',
+    branchcode      => 'library_id',
+    returndate      => 'checkin_date',
+    lastreneweddate => 'last_renewed_date',
+    issuedate       => 'checked_out_date',
+    notedate        => 'note_date',
+};
+
+=head3 $to_model_mapping
+
+=cut
+
+our $to_model_mapping = {
+    checkout_id       => 'issue_id',
+    patron_id         => 'borrowernumber',
+    item_id           => 'itemnumber',
+    due_date          => 'date_due',
+    library_id        => 'branchcode',
+    checkin_date      => 'returndate',
+    last_renewed_date => 'lastreneweddate',
+    checked_out_date  => 'issuedate',
+    note_date         => 'notedate',
+};
+
 1;
index 4267eba..e540a55 100644 (file)
@@ -1,34 +1,34 @@
 {
   "type": "object",
   "properties": {
-    "issue_id": {
-      "type": "string",
+    "checkout_id": {
+      "type": "integer",
       "description": "internally assigned checkout identifier"
     },
-    "borrowernumber": {
-      "$ref": "../x-primitives.json#/borrowernumber"
+    "patron_id": {
+      "$ref": "../x-primitives.json#/patron_id"
     },
-    "itemnumber": {
-      "$ref": "../x-primitives.json#/itemnumber"
+    "item_id": {
+      "type": "integer",
+      "description": "internal identifier of checked out item"
     },
-    "date_due": {
+    "due_date": {
       "type": "string",
+      "format": "date-time",
       "description": "Due date"
     },
-    "branchcode": {
-      "type": "string",
+    "library_id": {
+      "type": ["string", "null"],
       "description": "code of the library the item was checked out"
     },
-    "issuingbranch": {
-      "type": "string",
-      "description": "Code of the branch where issue was made"
-    },
-    "returndate": {
+    "checkin_date": {
       "type": ["string", "null"],
+      "format": "date",
       "description": "Date the item was returned"
     },
-    "lastreneweddate": {
+    "last_renewed_date": {
       "type": ["string", "null"],
+      "format": "date-time",
       "description": "Date the item was last renewed"
     },
     "renewals": {
@@ -43,8 +43,9 @@
       "type": "string",
       "description": "Last update time"
     },
-    "issuedate": {
-      "type": ["string", "null"],
+    "checked_out_date": {
+      "type": "string",
+      "format": "date-time",
       "description": "Date the item was issued"
     },
     "onsite_checkout": {
@@ -55,8 +56,9 @@
       "type": ["string", "null"],
       "description": "Issue note text"
     },
-    "notedate": {
+    "note_date": {
       "type": ["string", "null"],
+      "format": "date",
       "description": "Datetime of the issue note"
     }
   }
index e49a33e..54aadd9 100644 (file)
@@ -1,10 +1,11 @@
 {
   "/checkouts": {
     "get": {
+      "x-mojo-to": "Checkout#list",
       "operationId": "listCheckouts",
       "tags": ["patrons", "checkouts"],
       "parameters": [{
-        "$ref": "../parameters.json#/borrowernumberQueryParam"
+        "$ref": "../parameters.json#/patron_id_qp"
       }],
       "produces": [
         "application/json"
@@ -26,8 +27,6 @@
         }
       },
       "x-koha-authorization": {
-        "allow-owner": true,
-        "allow-guarantor": true,
         "permissions": {
           "circulate": "circulate_remaining_permissions"
         }
@@ -36,6 +35,7 @@
   },
   "/checkouts/{checkout_id}": {
     "get": {
+      "x-mojo-to": "Checkout#get",
       "operationId": "getCheckout",
       "tags": ["patrons", "checkouts"],
       "parameters": [{
         }
       },
       "x-koha-authorization": {
-        "allow-owner": true,
-        "allow-guarantor": true,
         "permissions": {
           "circulate": "circulate_remaining_permissions"
         }
       }
     },
     "put": {
+      "x-mojo-to": "Checkout#renew",
       "operationId": "renewCheckout",
       "tags": ["patrons", "checkouts"],
       "parameters": [{
@@ -86,8 +85,6 @@
         }
       },
       "x-koha-authorization": {
-        "allow-owner": true,
-        "allow-guarantor": true,
         "permissions": {
           "circulate": "circulate_remaining_permissions"
         }
index 5298d7f..141beb4 100644 (file)
@@ -32,16 +32,21 @@ use C4::Circulation;
 use C4::Items;
 
 use Koha::Database;
+use Koha::DateUtils;
 use Koha::Patron;
 
 my $schema = Koha::Database->schema;
-$schema->storage->txn_begin;
-my $dbh = C4::Context->dbh;
 my $builder = t::lib::TestBuilder->new;
-$dbh->{RaiseError} = 1;
+
+$schema->storage->txn_begin;
+
+t::lib::Mocks::mock_preference( 'SessionStorage', 'tmp' );
 
 $ENV{REMOTE_ADDR} = '127.0.0.1';
 my $t = Test::Mojo->new('Koha::REST::V1');
+my $tx;
+
+my $dbh = C4::Context->dbh;
 
 $dbh->do('DELETE FROM issues');
 $dbh->do('DELETE FROM items');
@@ -73,14 +78,14 @@ my $branchcode = $builder->build({ source => 'Branch' })->{ branchcode };
 my $module = new Test::MockModule('C4::Context');
 $module->mock('userenv', sub { { branch => $branchcode } });
 
-my $tx = $t->ua->build_tx(GET => "/api/v1/checkouts?borrowernumber=$borrowernumber");
+$tx = $t->ua->build_tx(GET => "/api/v1/checkouts?patron_id=$borrowernumber");
 $tx->req->cookies({name => 'CGISESSID', value => $session->id});
 $t->request_ok($tx)
   ->status_is(200)
   ->json_is([]);
 
 my $notexisting_borrowernumber = $borrowernumber + 1;
-$tx = $t->ua->build_tx(GET => "/api/v1/checkouts?borrowernumber=$notexisting_borrowernumber");
+$tx = $t->ua->build_tx(GET => "/api/v1/checkouts?patron_id=$notexisting_borrowernumber");
 $tx->req->cookies({name => 'CGISESSID', value => $session->id});
 $t->request_ok($tx)
   ->status_is(200)
@@ -99,16 +104,16 @@ my $date_due2 = Koha::DateUtils::dt_from_string( $issue2->date_due );
 my $issue3 = C4::Circulation::AddIssue($loggedinuser, 'TEST000003', $date_due);
 my $date_due3 = Koha::DateUtils::dt_from_string( $issue3->date_due );
 
-$tx = $t->ua->build_tx(GET => "/api/v1/checkouts?borrowernumber=$borrowernumber");
+$tx = $t->ua->build_tx(GET => "/api/v1/checkouts?patron_id=$borrowernumber");
 $tx->req->cookies({name => 'CGISESSID', value => $session->id});
 $t->request_ok($tx)
   ->status_is(200)
-  ->json_is('/0/borrowernumber' => $borrowernumber)
-  ->json_is('/0/itemnumber' => $itemnumber1)
-  ->json_is('/0/date_due' => $date_due1->ymd . ' ' . $date_due1->hms)
-  ->json_is('/1/borrowernumber' => $borrowernumber)
-  ->json_is('/1/itemnumber' => $itemnumber2)
-  ->json_is('/1/date_due' => $date_due2->ymd . ' ' . $date_due2->hms)
+  ->json_is('/0/patron_id' => $borrowernumber)
+  ->json_is('/0/item_id' => $itemnumber1)
+  ->json_is('/0/due_date' => output_pref({ dateformat => "rfc3339", dt => $date_due1 }) )
+  ->json_is('/1/patron_id' => $borrowernumber)
+  ->json_is('/1/item_id' => $itemnumber2)
+  ->json_is('/1/due_date' => output_pref({ dateformat => "rfc3339", dt => $date_due2 }) )
   ->json_hasnt('/2');
 
 $tx = $t->ua->build_tx(GET => "/api/v1/checkouts/".$issue3->issue_id);
@@ -119,7 +124,7 @@ $t->request_ok($tx)
               required_permissions => { circulate => "circulate_remaining_permissions" }
                                                });
 
-$tx = $t->ua->build_tx(GET => "/api/v1/checkouts?borrowernumber=".$loggedinuser->{borrowernumber});
+$tx = $t->ua->build_tx(GET => "/api/v1/checkouts?patron_id=".$loggedinuser->{borrowernumber});
 $tx->req->cookies({name => 'CGISESSID', value => $patron_session->id});
 $t->request_ok($tx)
   ->status_is(403)
@@ -127,38 +132,38 @@ $t->request_ok($tx)
                                                  required_permissions => { circulate => "circulate_remaining_permissions" }
                                          });
 
-$tx = $t->ua->build_tx(GET => "/api/v1/checkouts?borrowernumber=$borrowernumber");
-$tx->req->cookies({name => 'CGISESSID', value => $patron_session->id});
+$tx = $t->ua->build_tx(GET => "/api/v1/checkouts?patron_id=$borrowernumber");
+$tx->req->cookies({name => 'CGISESSID', value => $session->id});
 $t->request_ok($tx)
   ->status_is(200)
-  ->json_is('/0/borrowernumber' => $borrowernumber)
-  ->json_is('/0/itemnumber' => $itemnumber1)
-  ->json_is('/0/date_due' => $date_due1->ymd . ' ' . $date_due1->hms)
-  ->json_is('/1/borrowernumber' => $borrowernumber)
-  ->json_is('/1/itemnumber' => $itemnumber2)
-  ->json_is('/1/date_due' => $date_due2->ymd . ' ' . $date_due2->hms)
+  ->json_is('/0/patron_id' => $borrowernumber)
+  ->json_is('/0/item_id' => $itemnumber1)
+  ->json_is('/0/due_date' => output_pref({ dateformat => "rfc3339", dt => $date_due1 }) )
+  ->json_is('/1/patron_id' => $borrowernumber)
+  ->json_is('/1/item_id' => $itemnumber2)
+  ->json_is('/1/due_date' => output_pref({ dateformat => "rfc3339", dt => $date_due2 }) )
   ->json_hasnt('/2');
 
 $tx = $t->ua->build_tx(GET => "/api/v1/checkouts/" . $issue1->issue_id);
-$tx->req->cookies({name => 'CGISESSID', value => $patron_session->id});
+$tx->req->cookies({name => 'CGISESSID', value => $session->id});
 $t->request_ok($tx)
   ->status_is(200)
-  ->json_is('/borrowernumber' => $borrowernumber)
-  ->json_is('/itemnumber' => $itemnumber1)
-  ->json_is('/date_due' => $date_due1->ymd . ' ' . $date_due1->hms)
+  ->json_is('/patron_id' => $borrowernumber)
+  ->json_is('/item_id' => $itemnumber1)
+  ->json_is('/due_date' => output_pref({ dateformat => "rfc3339", dt => $date_due1 }) )
   ->json_hasnt('/1');
 
 $tx = $t->ua->build_tx(GET => "/api/v1/checkouts/" . $issue1->issue_id);
 $tx->req->cookies({name => 'CGISESSID', value => $session->id});
 $t->request_ok($tx)
   ->status_is(200)
-  ->json_is('/date_due' => $date_due1->ymd . ' ' . $date_due1->hms);
+  ->json_is('/due_date' => output_pref({ dateformat => "rfc3339", dt => $date_due1 }) );
 
 $tx = $t->ua->build_tx(GET => "/api/v1/checkouts/" . $issue2->issue_id);
 $tx->req->cookies({name => 'CGISESSID', value => $session->id});
 $t->request_ok($tx)
   ->status_is(200)
-  ->json_is('/date_due' => $date_due2->ymd . ' ' . $date_due2->hms);
+  ->json_is('/due_date' => output_pref( { dateformat => "rfc3339", dt => $date_due2 }) );
 
 
 $dbh->do('DELETE FROM issuingrules');
@@ -172,7 +177,7 @@ $tx = $t->ua->build_tx(PUT => "/api/v1/checkouts/" . $issue1->issue_id);
 $tx->req->cookies({name => 'CGISESSID', value => $session->id});
 $t->request_ok($tx)
   ->status_is(200)
-  ->json_is('/date_due' => $expected_datedue->ymd . ' ' . $expected_datedue->hms);
+  ->json_is('/due_date' => output_pref( { dateformat => "rfc3339", dt => $expected_datedue }) );
 
 $tx = $t->ua->build_tx(PUT => "/api/v1/checkouts/" . $issue3->issue_id);
 $tx->req->cookies({name => 'CGISESSID', value => $patron_session->id});
@@ -187,14 +192,14 @@ $tx = $t->ua->build_tx(PUT => "/api/v1/checkouts/" . $issue2->issue_id);
 $tx->req->cookies({name => 'CGISESSID', value => $patron_session->id});
 $t->request_ok($tx)
   ->status_is(403)
-  ->json_is({ error => "Opac Renewal not allowed"      });
+  ->json_is({ error => "Opac Renewal not allowed" });
 
 t::lib::Mocks::mock_preference( "OpacRenewalAllowed", 1 );
 $tx = $t->ua->build_tx(PUT => "/api/v1/checkouts/" . $issue2->issue_id);
-$tx->req->cookies({name => 'CGISESSID', value => $patron_session->id});
+$tx->req->cookies({name => 'CGISESSID', value => $session->id});
 $t->request_ok($tx)
   ->status_is(200)
-  ->json_is('/date_due' => $expected_datedue->ymd . ' ' . $expected_datedue->hms);
+  ->json_is('/due_date' => output_pref({ dateformat => "rfc3339", dt => $expected_datedue}) );
 
 $tx = $t->ua->build_tx(PUT => "/api/v1/checkouts/" . $issue1->issue_id);
 $tx->req->cookies({name => 'CGISESSID', value => $session->id});