From feef2edba8a166ec890eaea96c75889e8161a5ca Mon Sep 17 00:00:00 2001 From: Pedro Amorim Date: Thu, 27 Apr 2023 16:08:56 +0000 Subject: [PATCH] Bug 22440: Move backend statuses api endpoint Signed-off-by: Martin Renvoize Signed-off-by: Tomas Cohen Arazi (cherry picked from commit 50909d22668486685343df6b3e7b17f68cf7f243) Signed-off-by: Martin Renvoize --- 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 . + +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 + +=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 163888ce0a..e7347f889b 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: @@ -243,8 +241,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( '' ); 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.39.5