Bugzilla – Attachment 68485 Details for
Bug 7317
Add an Interlibrary Loan Module to Circulation and OPAC
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 7317: (followup) Migrate endpoint to OpenAPI
Bug-7317-followup-Migrate-endpoint-to-OpenAPI.patch (text/plain), 11.76 KB, created by
Tomás Cohen Arazi (tcohen)
on 2017-10-24 16:54:30 UTC
(
hide
)
Description:
Bug 7317: (followup) Migrate endpoint to OpenAPI
Filename:
MIME Type:
Creator:
Tomás Cohen Arazi (tcohen)
Created:
2017-10-24 16:54:30 UTC
Size:
11.76 KB
patch
obsolete
>From 5420194ffcc355748ddcd1d97a9dc6f8fb7b45c7 Mon Sep 17 00:00:00 2001 >From: Tomas Cohen Arazi <tomascohen@theke.io> >Date: Mon, 23 Oct 2017 15:34:01 -0300 >Subject: [PATCH] Bug 7317: (followup) Migrate endpoint to OpenAPI > >This patch moves the current endpoint implementation from Swagger2 to >the OpenAPI plugin. > >It also takes advantage of the overloaded Koha::Illrequest::TO_JSON method >which has now the option to embed what's needed for the REST api. > >The path spec is adjusted to fit OpenAPI, and some minor fixes are >applied: >- Missing 'metadata' query param >- 'ill' permissions should be required instead of 'borrowers' >- Full test coverage > >To test: >- Apply this patch >- Run: > $ kshell > k$ prove t/db_dependent/api/v1/illrequests.t >=> SUCCESS: Tests pass! >- Sign off :-D > >Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> >--- > Koha/REST/V1/Illrequests.pm | 50 ++++----------- > api/v1/swagger/paths/illrequests.json | 41 ++++++++++-- > t/db_dependent/api/v1/illrequests.t | 113 +++++++++++++++++++--------------- > 3 files changed, 112 insertions(+), 92 deletions(-) > >diff --git a/Koha/REST/V1/Illrequests.pm b/Koha/REST/V1/Illrequests.pm >index 0807eb1bf6..947fb7972f 100644 >--- a/Koha/REST/V1/Illrequests.pm >+++ b/Koha/REST/V1/Illrequests.pm >@@ -20,13 +20,13 @@ use Modern::Perl; > use Mojo::Base 'Mojolicious::Controller'; > > use Koha::Illrequests; >-use Koha::Library; >+use Koha::Libraries; > > sub list { >- my ($c, $args, $cb) = @_; >+ my $c = shift->openapi->valid_input or return; > >+ my $args = $c->req->params->to_hash // {}; > my $filter; >- $args //= {}; > my $output = []; > > # Create a hash where all keys are embedded values >@@ -44,42 +44,16 @@ sub list { > > my $requests = Koha::Illrequests->search($filter); > >- while (my $request = $requests->next) { >- my $unblessed = $request->unblessed; >- # Add the request's id_prefix >- $unblessed->{id_prefix} = $request->id_prefix; >- # Augment the request response with patron details >- # if appropriate >- if (defined $embed{patron}) { >- my $patron = $request->patron; >- $unblessed->{patron} = { >- firstname => $patron->firstname, >- surname => $patron->surname, >- cardnumber => $patron->cardnumber >- }; >- } >- # Augment the request response with metadata details >- # if appropriate >- if (defined $embed{metadata}) { >- $unblessed->{metadata} = $request->metadata; >- } >- # Augment the request response with status details >- # if appropriate >- if (defined $embed{capabilities}) { >- $unblessed->{capabilities} = $request->capabilities; >- } >- # Augment the request response with branch details >- # if appropriate >- if (defined $embed{branch}) { >- $unblessed->{branch} = Koha::Libraries->find( >- $request->branchcode >- )->unblessed; >- } >- push @{$output}, $unblessed >+ if ( scalar (keys %embed) ) >+ { >+ # Need to embed stuff >+ my @results = map { $_->TO_JSON(\%embed) } $requests->as_list; >+ return $c->render( status => 200, openapi => \@results ); >+ } >+ else >+ { >+ return $c->render( status => 200, openapi => $requests ); > } >- >- return $c->$cb( $output, 200 ); >- > } > > 1; >diff --git a/api/v1/swagger/paths/illrequests.json b/api/v1/swagger/paths/illrequests.json >index ddafd80216..5372cb9d09 100644 >--- a/api/v1/swagger/paths/illrequests.json >+++ b/api/v1/swagger/paths/illrequests.json >@@ -1,8 +1,8 @@ > { > "/illrequests": { > "get": { >- "x-mojo-controller": "Koha::REST::V1::Illrequests", >- "operationId": "list", >+ "x-mojo-to": "Illrequests#list", >+ "operationId": "listIllrequests", > "tags": ["illrequests"], > "parameters": [{ > "name": "embed", >@@ -16,7 +16,8 @@ > "enum": [ > "patron", > "branch", >- "capabilities" >+ "capabilities", >+ "metadata" > ] > } > }, { >@@ -85,12 +86,42 @@ > ], > "responses": { > "200": { >- "description": "OK" >+ "description": "A list of ILL requests" >+ }, >+ "401": { >+ "description": "Authentication required", >+ "schema": { >+ "$ref": "../definitions.json#/error" >+ } >+ }, >+ "403": { >+ "description": "Access forbidden", >+ "schema": { >+ "$ref": "../definitions.json#/error" >+ } >+ }, >+ "404": { >+ "description": "ILL requests not found", >+ "schema": { >+ "$ref": "../definitions.json#/error" >+ } >+ }, >+ "500": { >+ "description": "Internal server error", >+ "schema": { >+ "$ref": "../definitions.json#/error" >+ } >+ }, >+ "503": { >+ "description": "Under maintenance", >+ "schema": { >+ "$ref": "../definitions.json#/error" >+ } > } > }, > "x-koha-authorization": { > "permissions": { >- "borrowers": "1" >+ "ill": "1" > } > } > } >diff --git a/t/db_dependent/api/v1/illrequests.t b/t/db_dependent/api/v1/illrequests.t >index fb306ba144..cd1b04a1d6 100644 >--- a/t/db_dependent/api/v1/illrequests.t >+++ b/t/db_dependent/api/v1/illrequests.t >@@ -18,6 +18,7 @@ > use Modern::Perl; > > use Test::More tests => 1; >+use Test::MockModule; > use Test::Mojo; > use Test::Warn; > >@@ -30,8 +31,6 @@ use Koha::Illrequests; > my $schema = Koha::Database->new->schema; > my $builder = t::lib::TestBuilder->new; > >-# FIXME: sessionStorage defaults to mysql, but it seems to break transaction handling >-# this affects the other REST api tests > t::lib::Mocks::mock_preference( 'SessionStorage', 'tmp' ); > > my $remote_address = '127.0.0.1'; >@@ -39,13 +38,19 @@ my $t = Test::Mojo->new('Koha::REST::V1'); > > subtest 'list() tests' => sub { > >- plan tests => 6; >+ plan tests => 15; >+ >+ my $illreqmodule = Test::MockModule->new('Koha::Illrequest'); >+ # Mock ->capabilities >+ $illreqmodule->mock( 'capabilities', sub { return 'capable'; } ); >+ # Mock ->metadata >+ $illreqmodule->mock( 'metadata', sub { return 'metawhat?'; } ); > > $schema->storage->txn_begin; > > Koha::Illrequests->search->delete; >- my ( $borrowernumber, $session_id ) = >- create_user_and_session( { authorized => 1 } ); >+ # ill => 22 (userflags.sql) >+ my ( $borrowernumber, $session_id ) = create_user_and_session({ authorized => 22 }); > > ## Authorized user tests > # No requests, so empty array should be returned >@@ -54,40 +59,56 @@ subtest 'list() tests' => sub { > $tx->req->env( { REMOTE_ADDR => $remote_address } ); > $t->request_ok($tx)->status_is(200)->json_is( [] ); > >-# my $city_country = 'France'; >-# my $city = $builder->build( >-# { source => 'City', value => { city_country => $city_country } } ); >-# >-# # One city created, should get returned >-# $tx = $t->ua->build_tx( GET => '/api/v1/cities' ); >-# $tx->req->cookies( { name => 'CGISESSID', value => $session_id } ); >-# $tx->req->env( { REMOTE_ADDR => $remote_address } ); >-# $t->request_ok($tx)->status_is(200)->json_is( [$city] ); >-# >-# my $another_city = $builder->build( >-# { source => 'City', value => { city_country => $city_country } } ); >-# my $city_with_another_country = $builder->build( { source => 'City' } ); >-# >-# # Two cities created, they should both be returned >-# $tx = $t->ua->build_tx( GET => '/api/v1/cities' ); >-# $tx->req->cookies( { name => 'CGISESSID', value => $session_id } ); >-# $tx->req->env( { REMOTE_ADDR => $remote_address } ); >-# $t->request_ok($tx)->status_is(200) >-# ->json_is( [ $city, $another_city, $city_with_another_country ] ); >-# >-# # Filtering works, two cities sharing city_country >-# $tx = >-# $t->ua->build_tx( GET => "/api/v1/cities?city_country=" . $city_country ); >-# $tx->req->cookies( { name => 'CGISESSID', value => $session_id } ); >-# $tx->req->env( { REMOTE_ADDR => $remote_address } ); >-# my $result = >-# $t->request_ok($tx)->status_is(200)->json_is( [ $city, $another_city ] ); >-# >-# $tx = $t->ua->build_tx( >-# GET => "/api/v1/cities?city_name=" . $city->{city_name} ); >-# $tx->req->cookies( { name => 'CGISESSID', value => $session_id } ); >-# $tx->req->env( { REMOTE_ADDR => $remote_address } ); >-# $t->request_ok($tx)->status_is(200)->json_is( [$city] ); >+ my $library = $builder->build_object( { class => 'Koha::Libraries' } ); >+ my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); >+ >+ # Create an ILL request >+ my $illrequest = $builder->build_object( >+ { >+ class => 'Koha::Illrequests', >+ value => { >+ branchcode => $library->branchcode, >+ borrowernumber => $patron->borrowernumber >+ } >+ } >+ ); >+ >+ # One illrequest created, should get returned >+ $tx = $t->ua->build_tx( GET => '/api/v1/illrequests' ); >+ $tx->req->cookies( { name => 'CGISESSID', value => $session_id } ); >+ $tx->req->env( { REMOTE_ADDR => $remote_address } ); >+ $t->request_ok($tx)->status_is(200)->json_is( [ $illrequest->TO_JSON ] ); >+ >+ # One illrequest created, returned with augmented data >+ $tx = $t->ua->build_tx( GET => >+ '/api/v1/illrequests?embed=patron,branch,capabilities,metadata' ); >+ $tx->req->cookies( { name => 'CGISESSID', value => $session_id } ); >+ $tx->req->env( { REMOTE_ADDR => $remote_address } ); >+ $t->request_ok($tx)->status_is(200)->json_is( >+ [ >+ $illrequest->TO_JSON( >+ { patron => 1, branch => 1, capabilities => 1, metadata => 1 } >+ ) >+ ] >+ ); >+ >+ # Create another ILL request >+ my $illrequest2 = $builder->build_object( >+ { >+ class => 'Koha::Illrequests', >+ value => { >+ branchcode => $library->branchcode, >+ borrowernumber => $patron->borrowernumber >+ } >+ } >+ ); >+ >+ # Two illrequest created, should get returned >+ $tx = $t->ua->build_tx( GET => '/api/v1/illrequests' ); >+ $tx->req->cookies( { name => 'CGISESSID', value => $session_id } ); >+ $tx->req->env( { REMOTE_ADDR => $remote_address } ); >+ $t->request_ok($tx)->status_is(200) >+ ->json_is( [ $illrequest->TO_JSON, $illrequest2->TO_JSON ] ); > > # Warn on unsupported query parameter > $tx = $t->ua->build_tx( GET => '/api/v1/illrequests?request_blah=blah' ); >@@ -102,9 +123,10 @@ subtest 'list() tests' => sub { > > sub create_user_and_session { > >- my $args = shift; >- my $flags = ( $args->{authorized} ) ? $args->{authorized} : 0; >- my $dbh = C4::Context->dbh; >+ my $args = shift; >+ my $dbh = C4::Context->dbh; >+ >+ my $flags = ( $args->{authorized} ) ? 2**$args->{authorized} : 0; > > my $user = $builder->build( > { >@@ -123,13 +145,6 @@ sub create_user_and_session { > $session->param( 'lasttime', time() ); > $session->flush; > >- if ( $args->{authorized} ) { >- $dbh->do( " >- INSERT INTO user_permissions (borrowernumber,module_bit,code) >- VALUES (?,3,'parameters_remaining_permissions')", undef, >- $user->{borrowernumber} ); >- } >- > return ( $user->{borrowernumber}, $session->id ); > } > >-- >2.14.3
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 7317
:
59893
|
59894
|
59895
|
59920
|
60485
|
60486
|
60487
|
60488
|
63081
|
63082
|
63083
|
63084
|
63085
|
63086
|
63087
|
63088
|
63089
|
63090
|
63113
|
63175
|
63176
|
63354
|
63421
|
63472
|
63475
|
65469
|
65470
|
65471
|
65472
|
65473
|
65474
|
65475
|
65476
|
66266
|
67242
|
67243
|
67244
|
67245
|
67246
|
67247
|
67248
|
67249
|
67250
|
67251
|
67252
|
67253
|
67254
|
67255
|
67256
|
67257
|
67258
|
67259
|
67260
|
67261
|
67262
|
67263
|
67264
|
67281
|
67282
|
67283
|
67284
|
67285
|
67286
|
67287
|
67288
|
67289
|
67290
|
67291
|
67292
|
67293
|
67294
|
67295
|
67296
|
67297
|
67298
|
67299
|
67300
|
67301
|
67302
|
67303
|
68185
|
68186
|
68328
|
68391
|
68406
|
68407
|
68408
|
68409
|
68418
|
68444
|
68445
|
68483
|
68484
|
68485
|
68487
|
68516
|
68517
|
68518
|
68519
|
68520
|
68521
|
68522
|
68523
|
68524
|
68525
|
68542
|
68543
|
68544
|
68545
|
68546
|
68547
|
68548
|
68549
|
68550
|
68551
|
68552
|
68558
|
68559
|
68560
|
68561
|
68562
|
68563
|
68564
|
68565
|
68566
|
68567
|
68858
|
68874
|
68877
|
68878
|
68885
|
68886
|
68921
|
68922
|
68926
|
68927
|
68928
|
68929
|
68931
|
68932
|
68933
|
68988
|
68989
|
68999
|
69000
|
69006
|
69008
|
69009
|
69012
|
69014
|
69044
|
69045
|
69046
|
69056
|
69059
|
69060
|
69061
|
69063
|
69064
|
69065
|
69066
|
69069
|
69070
|
69071
|
69074