Bugzilla – Attachment 150722 Details for
Bug 22440
Improve ILL page performance by moving to server side filtering
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 22440: Move backend statuses api endpoint
Bug-22440-Move-backend-statuses-api-endpoint.patch (text/plain), 14.09 KB, created by
Martin Renvoize (ashimema)
on 2023-05-05 11:05:04 UTC
(
hide
)
Description:
Bug 22440: Move backend statuses api endpoint
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2023-05-05 11:05:04 UTC
Size:
14.09 KB
patch
obsolete
>From 2cc5cf847bfa53869526e54de849e0cbe9344f6c Mon Sep 17 00:00:00 2001 >From: Pedro Amorim <pedro.amorim@ptfs-europe.com> >Date: Thu, 27 Apr 2023 16:08:56 +0000 >Subject: [PATCH] Bug 22440: Move backend statuses api endpoint > >Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> >--- > Koha/Illbackend.pm | 124 ++++++++++++++++++ > Koha/REST/V1/Illbackends.pm | 87 +++++------- > api/v1/swagger/definitions/ill_backend.yaml | 5 + > api/v1/swagger/definitions/ill_statuses.yaml | 5 - > api/v1/swagger/paths/ill_backends.yaml | 56 ++------ > api/v1/swagger/swagger.yaml | 4 - > .../intranet-tmpl/prog/js/ill-list-table.js | 8 +- > t/db_dependent/api/v1/ill_backends.t | 8 +- > 8 files changed, 179 insertions(+), 118 deletions(-) > create mode 100644 Koha/Illbackend.pm > delete mode 100644 api/v1/swagger/definitions/ill_statuses.yaml > >diff --git a/Koha/Illbackend.pm b/Koha/Illbackend.pm >new file mode 100644 >index 0000000000..8a6be8a7a6 >--- /dev/null >+++ b/Koha/Illbackend.pm >@@ -0,0 +1,124 @@ >+package Koha::Illbackend; >+ >+# Copyright PTFS Europe 2023 >+# >+# This file is part of Koha. >+# >+# Koha is free software; you can redistribute it and/or modify it >+# under the terms of the GNU General Public License as published by >+# the Free Software Foundation; either version 3 of the License, or >+# (at your option) any later version. >+# >+# Koha is distributed in the hope that it will be useful, but >+# WITHOUT ANY WARRANTY; without even the implied warranty of >+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >+# GNU General Public License for more details. >+# >+# You should have received a copy of the GNU General Public License >+# along with Koha; if not, see <http://www.gnu.org/licenses>. >+ >+use Modern::Perl; >+ >+use base qw(Koha::Object); >+ >+=head1 NAME >+ >+Koha::Illbackend - Koha Illbackend Object class >+ >+=head2 Class methods >+ >+=head3 new >+ >+New illbackend >+ >+=cut >+ >+sub new { >+ my $class = shift; >+ my $self = {}; >+ return bless $self, $class; >+} >+ >+=head3 existing_statuses >+ >+Return a list of existing ILL statuses >+ >+=cut >+ >+sub existing_statuses { >+ my ( $self, $backend_id ) = @_; >+ >+ #FIXME: Currently fetching all requests, it'd be great if we could fetch distinct(status). >+ # Even doing it with distinct status, we need the ILL request object, so that strings_map works and >+ # the ILL request returns the correct status and info respective to its backend. >+ my $ill_requests = Koha::Illrequests->search( >+ {backend => $backend_id}, >+ # { >+ # columns => [ qw/status/ ], >+ # group_by => [ qw/status/ ], >+ # } >+ ); >+ >+ my @data; >+ while (my $request = $ill_requests->next) { >+ my $status_data = $request->strings_map; >+ >+ foreach my $status_class ( qw(status_alias status) ){ >+ if ($status_data->{$status_class}){ >+ push @data, { >+ $status_data->{$status_class}->{str} ? (str => $status_data->{$status_class}->{str}) : >+ $status_data->{$status_class}->{code} ? (str => $status_data->{$status_class}->{code}) : (), >+ $status_data->{$status_class}->{code} ? (code => $status_data->{$status_class}->{code}) : (), >+ } >+ } >+ } >+ } >+ >+ # Remove duplicate statuses >+ my %seen; >+ @data = grep { my $e = $_; my $key = join '___', map { $e->{$_}; } sort keys %$_;!$seen{$key}++ } @data; >+ >+ return \@data; >+} >+ >+=head3 embed >+ >+ Embed info in backend for API response >+ >+=cut >+ >+sub embed { >+ my ( $self, $backend_id, $embed_header ) = @_; >+ $embed_header ||= q{}; >+ >+ my $return_embed; >+ >+ foreach my $embed_req ( split /\s*,\s*/, $embed_header ) { >+ if ( $embed_req eq 'statuses+strings' ) { >+ $return_embed->{statuses} = $self->existing_statuses( $backend_id ); >+ } >+ } >+ return $return_embed; >+} >+ >+=head2 Internal methods >+ >+=head3 _type >+ >+ my $type = Koha::Illbackend->_type; >+ >+Return this object's type >+ >+=cut >+ >+sub _type { >+ return 'Illbackend'; >+} >+ >+=head1 AUTHOR >+ >+Pedro Amorim <pedro.amorim@ptfs-europe.com> >+ >+=cut >+ >+1; >diff --git a/Koha/REST/V1/Illbackends.pm b/Koha/REST/V1/Illbackends.pm >index c3c917ef70..02096db565 100644 >--- a/Koha/REST/V1/Illbackends.pm >+++ b/Koha/REST/V1/Illbackends.pm >@@ -21,6 +21,7 @@ use Mojo::Base 'Mojolicious::Controller'; > > use Koha::Illrequest::Config; > use Koha::Illrequests; >+use Koha::Illbackend; > > =head1 NAME > >@@ -37,87 +38,59 @@ Return a list of available ILL backends and its capabilities > sub list { > my $c = shift->openapi->valid_input; > >- my $config = Koha::Illrequest::Config->new; >+ my $config = Koha::Illrequest::Config->new; > my $backends = $config->available_backends; > > my @data; >- foreach my $b ( @$backends ) { >- my $backend = Koha::Illrequest->new->load_backend( $b ); >- push @data, { >+ foreach my $b (@$backends) { >+ my $backend = Koha::Illrequest->new->load_backend($b); >+ push @data, >+ { > ill_backend_id => $b, >- capabilities => $backend->capabilities, >- }; >+ capabilities => $backend->capabilities, >+ }; > } > return $c->render( status => 200, openapi => \@data ); > } > >-=head3 list_statuses >+=head3 get > >-Return a list of existing ILL statuses >+Get one backend > > =cut > >-sub list_statuses { >+sub get { > my $c = shift->openapi->valid_input; > > my $backend_id = $c->validation->param('ill_backend_id'); > >- #FIXME: Currently fetching all requests, it'd be great if we could fetch distinct(status). >- # Even doing it with distinct status, we need the ILL request object, so that strings_map works and >- # the ILL request returns the correct status and info respective to its backend. >- my $ill_requests = Koha::Illrequests->search( >- {backend => $backend_id}, >- # { >- # columns => [ qw/status/ ], >- # group_by => [ qw/status/ ], >- # } >- ); >- >- my @data; >- while (my $request = $ill_requests->next) { >- my $status_data = $request->strings_map; >- >- foreach my $status_class ( qw(status_alias status) ){ >- if ($status_data->{$status_class}){ >- push @data, { >- $status_data->{$status_class}->{str} ? (str => $status_data->{$status_class}->{str}) : >- $status_data->{$status_class}->{code} ? (str => $status_data->{$status_class}->{code}) : (), >- $status_data->{$status_class}->{code} ? (code => $status_data->{$status_class}->{code}) : (), >- } >- } >- } >- } >- >- # Remove duplicate statuses >- my %seen; >- @data = grep { my $e = $_; my $key = join '___', map { $e->{$_}; } sort keys %$_;!$seen{$key}++ } @data; >- >- return $c->render( status => 200, openapi => \@data ); >-} >+ return try { > >-=head3 get >+ #FIXME: Should we move load_backend into Koha::Illbackend... >+ # or maybe make Koha::Ill::Backend a base class for all >+ # backends? >+ my $backend = Koha::Illrequest->new->load_backend($backend_id); > >-Get one backend >+ my $backend_module = Koha::Illbackend->new; > >-=cut >+ my $embed = >+ $backend_module->embed( $backend_id, >+ $c->req->headers->header('x-koha-embed') ); > >-sub get { >- my $c = shift->openapi->valid_input; >- >- my $backend_id = $c->validation->param('ill_backend_id'); >+ #TODO: We need a to_api method in Koha::Illbackend >+ my $return = { >+ ill_backend_id => $backend_id, >+ capabilities => $backend->capabilities, >+ }; > >- return try { >- my $backend = Koha::Illrequest->new->load_backend( $backend_id ); > return $c->render( >- status => 200, >- openapi => { >- ill_backend_id => $backend_id, >- capabilities => $backend->capabilities >- } >+ status => 200, >+ openapi => $embed ? { %$return, %$embed } : $return, > ); >- } catch { >+ } >+ catch { > return $c->render( >- status => 404, >+ status => 404, > openapi => { error => "ILL backend does not exist" } > ); > }; >diff --git a/api/v1/swagger/definitions/ill_backend.yaml b/api/v1/swagger/definitions/ill_backend.yaml >index 9d742041b6..d0087457c3 100644 >--- a/api/v1/swagger/definitions/ill_backend.yaml >+++ b/api/v1/swagger/definitions/ill_backend.yaml >@@ -7,4 +7,9 @@ properties: > capabilities: > type: object > description: List of capabilities >+ statuses: >+ type: array >+ description: existing statuses >+ items: >+ $ref: ill_status.yaml > additionalProperties: false >diff --git a/api/v1/swagger/definitions/ill_statuses.yaml b/api/v1/swagger/definitions/ill_statuses.yaml >deleted file mode 100644 >index ba2daf9447..0000000000 >--- a/api/v1/swagger/definitions/ill_statuses.yaml >+++ /dev/null >@@ -1,5 +0,0 @@ >---- >-type: array >-items: >- $ref: "ill_status.yaml" >-additionalProperties: false >diff --git a/api/v1/swagger/paths/ill_backends.yaml b/api/v1/swagger/paths/ill_backends.yaml >index 1a74f8b622..874481a5ec 100644 >--- a/api/v1/swagger/paths/ill_backends.yaml >+++ b/api/v1/swagger/paths/ill_backends.yaml >@@ -53,6 +53,16 @@ > description: ILL backend id/name > required: true > type: string >+ - name: x-koha-embed >+ in: header >+ required: false >+ description: Embed list sent as a request header >+ type: array >+ items: >+ type: string >+ enum: >+ - statuses+strings >+ collectionFormat: csv > produces: > - application/json > responses: >@@ -83,52 +93,6 @@ > description: Under maintenance > schema: > $ref: "../swagger.yaml#/definitions/error" >- x-koha-authorization: >- permissions: >- ill: "1" >-"/ill/backends/{ill_backend_id}/statuses": >- get: >- x-mojo-to: Illbackends#list_statuses >- operationId: getIllbackendsStatuses >- tags: >- - ill_backends >- summary: Get existing ILL statuses >- parameters: >- - name: ill_backend_id >- in: path >- description: ILL backend id/name >- required: true >- type: string >- produces: >- - application/json >- responses: >- "200": >- description: A list of existing ILL statuses >- schema: >- $ref: "../swagger.yaml#/definitions/ill_statuses" >- "401": >- description: Authentication required >- schema: >- $ref: "../swagger.yaml#/definitions/error" >- "403": >- description: Access forbidden >- schema: >- $ref: "../swagger.yaml#/definitions/error" >- "404": >- description: ILL backends not found >- schema: >- $ref: "../swagger.yaml#/definitions/error" >- "500": >- description: | >- Internal server error. Possible `error_code` attribute values: >- >- * `internal_server_error` >- schema: >- $ref: "../swagger.yaml#/definitions/error" >- "503": >- description: Under maintenance >- schema: >- $ref: "../swagger.yaml#/definitions/error" > x-koha-authorization: > permissions: > ill: "1" >\ No newline at end of file >diff --git a/api/v1/swagger/swagger.yaml b/api/v1/swagger/swagger.yaml >index 1cb992b23b..020f1284df 100644 >--- a/api/v1/swagger/swagger.yaml >+++ b/api/v1/swagger/swagger.yaml >@@ -54,8 +54,6 @@ definitions: > $ref: ./definitions/ill_backends.yaml > ill_status: > $ref: ./definitions/ill_status.yaml >- ill_statuses: >- $ref: ./definitions/ill_statuses.yaml > ill_request: > $ref: ./definitions/ill_request.yaml > import_batch_profile: >@@ -249,8 +247,6 @@ paths: > $ref: ./paths/ill_backends.yaml#/~1ill~1backends > "/ill/backends/{ill_backend_id}": > $ref: "./paths/ill_backends.yaml#/~1ill~1backends~1{ill_backend_id}" >- "/ill/backends/{ill_backend_id}/statuses": >- $ref: "./paths/ill_backends.yaml#/~1ill~1backends~1{ill_backend_id}~1statuses" > /ill/requests: > $ref: ./paths/ill_requests.yaml#/~1ill~1requests > "/import_batches/{import_batch_id}/records/{import_record_id}/matches/chosen": >diff --git a/koha-tmpl/intranet-tmpl/prog/js/ill-list-table.js b/koha-tmpl/intranet-tmpl/prog/js/ill-list-table.js >index 174622ad50..8a358015ac 100644 >--- a/koha-tmpl/intranet-tmpl/prog/js/ill-list-table.js >+++ b/koha-tmpl/intranet-tmpl/prog/js/ill-list-table.js >@@ -430,8 +430,12 @@ $(document).ready(function() { > function populateStatusFilter(backend) { > $.ajax({ > type: "GET", >- url: "/api/v1/ill/backends/"+backend+"/statuses", >- success: function(statuses){ >+ url: "/api/v1/ill/backends/"+backend, >+ headers: { >+ 'x-koha-embed': 'statuses+strings' >+ }, >+ success: function(response){ >+ let statuses = response.statuses > $('#illfilter_status').append( > '<option value="">'+ill_all_statuses+'</option>' > ); >diff --git a/t/db_dependent/api/v1/ill_backends.t b/t/db_dependent/api/v1/ill_backends.t >index 704fca4432..44270cf83d 100755 >--- a/t/db_dependent/api/v1/ill_backends.t >+++ b/t/db_dependent/api/v1/ill_backends.t >@@ -193,14 +193,14 @@ subtest 'list() tests' => sub { > ); > > #Check for backend existing statuses >- $t->get_ok("//$userid:$password@/api/v1/ill/backends/Mock/statuses") >+ $t->get_ok("//$userid:$password@/api/v1/ill/backends/Mock" => {'x-koha-embed' => 'statuses+strings'} ) > ->status_is(200) >- ->json_is( [ $backend_status, $core_status, $alias_status ] ); >+ ->json_has( '/statuses', [ $backend_status, $core_status, $alias_status ] ); > > #Check for backend existing statuses of a backend that doesn't exist >- $t->get_ok("//$userid:$password@/api/v1/ill/backends/GhostBackend/statuses") >+ $t->get_ok("//$userid:$password@/api/v1/ill/backends/GhostBackend" => {'x-koha-embed' => 'statuses+strings'} ) > ->status_is(200) >- ->json_is( [] ); >+ ->json_hasnt( 'statuses' ); > > # Unauthorized attempt to list > $t->get_ok("//$unauth_userid:$password@/api/v1/ill/backends") >-- >2.40.1
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 22440
:
150715
|
150716
|
150717
|
150718
|
150719
|
150720
|
150721
| 150722 |
150723
|
150788
|
150789