Bugzilla – Attachment 84765 Details for
Bug 13895
Add routes for checkouts retrieval and renewal
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 13895: (follow-up) Adapt checkout endpoint to openapi, update terminology
Bug-13895-follow-up-Adapt-checkout-endpoint-to-ope.patch (text/plain), 17.80 KB, created by
Josef Moravec
on 2019-02-05 12:34:36 UTC
(
hide
)
Description:
Bug 13895: (follow-up) Adapt checkout endpoint to openapi, update terminology
Filename:
MIME Type:
Creator:
Josef Moravec
Created:
2019-02-05 12:34:36 UTC
Size:
17.80 KB
patch
obsolete
>From f7e26fa98e62faf2670cde5fec35c01c49d76dcf Mon Sep 17 00:00:00 2001 >From: Josef Moravec <josef.moravec@gmail.com> >Date: Mon, 4 Feb 2019 14:22:54 +0000 >Subject: [PATCH] Bug 13895: (follow-up) Adapt checkout endpoint to openapi, > update terminology > >--- > Koha/REST/V1/Checkout.pm | 184 ++++++++++++++++++++++++++----- > api/v1/swagger/definitions/checkout.json | 38 ++++--- > api/v1/swagger/paths/checkouts.json | 11 +- > t/db_dependent/api/v1/checkouts.t | 67 +++++------ > 4 files changed, 217 insertions(+), 83 deletions(-) > >diff --git a/Koha/REST/V1/Checkout.pm b/Koha/REST/V1/Checkout.pm >index 8c898ac1d8..14a623b34a 100644 >--- a/Koha/REST/V1/Checkout.pm >+++ b/Koha/REST/V1/Checkout.pm >@@ -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; >diff --git a/api/v1/swagger/definitions/checkout.json b/api/v1/swagger/definitions/checkout.json >index 4267eba24c..e540a55583 100644 >--- a/api/v1/swagger/definitions/checkout.json >+++ b/api/v1/swagger/definitions/checkout.json >@@ -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" > } > } >diff --git a/api/v1/swagger/paths/checkouts.json b/api/v1/swagger/paths/checkouts.json >index e49a33ef80..54aadd9fc5 100644 >--- a/api/v1/swagger/paths/checkouts.json >+++ b/api/v1/swagger/paths/checkouts.json >@@ -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": [{ >@@ -57,14 +57,13 @@ > } > }, > "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" > } >diff --git a/t/db_dependent/api/v1/checkouts.t b/t/db_dependent/api/v1/checkouts.t >index 5298d7fcd4..141beb4b05 100644 >--- a/t/db_dependent/api/v1/checkouts.t >+++ b/t/db_dependent/api/v1/checkouts.t >@@ -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}); >-- >2.11.0
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 13895
:
37146
|
40104
|
40143
|
40588
|
41163
|
41548
|
45817
|
50021
|
51303
|
53816
|
53817
|
53878
|
53889
|
53890
|
53891
|
53901
|
53924
|
54196
|
54685
|
54951
|
56167
|
56506
|
58387
|
69123
|
84765
|
85458
|
85459
|
85460
|
85461
|
85462
|
85463
|
85464
|
85676
|
86120
|
86576
|
86579
|
86580
|
87105
|
87106
|
87107
|
87108
|
87109
|
87110
|
87111
|
87112
|
87113
|
87114
|
87115
|
87200