From 7fb2ab894ee8bfb7eb0aa69375e1d1bed03b5f63 Mon Sep 17 00:00:00 2001 From: Pedro Amorim Date: Mon, 9 Oct 2023 15:29:58 +0000 Subject: [PATCH] Bug 30719: QA follow-up: Rewrite Illbatches list endpoint Update accessors Add +strings embed Add x-koha-embed to batches list andpoint Add embed to API call from the front-end Update table to get data from _strings Add x-koha-embed to tests Add strings_map to Illbatch Add to_api_mapping to Illbatch --- Koha/Illbatch.pm | 63 ++++++++++++++++++- Koha/REST/V1/Illbatches.pm | 62 +++--------------- api/v1/swagger/definitions/ill_batch.yaml | 5 ++ api/v1/swagger/paths/ill_batches.yaml | 22 ++++++- .../intranet-tmpl/prog/js/ill-batch-table.js | 16 +++-- t/db_dependent/api/v1/ill_batches.t | 13 ++-- 6 files changed, 111 insertions(+), 70 deletions(-) diff --git a/Koha/Illbatch.pm b/Koha/Illbatch.pm index e5f65dbab7..f4163a8ed0 100644 --- a/Koha/Illbatch.pm +++ b/Koha/Illbatch.pm @@ -53,7 +53,8 @@ Return the patron object associated with this batch sub patron { my ($self) = @_; - return Koha::Patron->_new_from_dbic( scalar $self->_result->borrowernumber ); + my $patron = return Koha::Patrons->find( { borrowernumber => $self->borrowernumber } ); + return unless $patron; } =head3 branch @@ -66,7 +67,8 @@ Return the branch object associated with this batch sub branch { my ($self) = @_; - return Koha::Library->_new_from_dbic( scalar $self->_result->branchcode ); + my $library = return Koha::Libraries->find( { branchcode => $self->branchcode } ); + return unless $library; } =head3 requests_count @@ -82,6 +84,18 @@ sub requests_count { return Koha::Illrequests->search( { batch_id => $self->id } )->count; } +=head3 requests + +Return the requests for this batch + +=cut + +sub requests { + my ($self) = @_; + my $requests = $self->_result->illrequests; + return Koha::Illrequests->_new_from_dbic($requests); +} + =head3 create_and_log $batch->create_and_log; @@ -175,6 +189,51 @@ sub delete_and_log { =head2 Internal methods + +=head3 strings_map + +=cut + +sub strings_map { + my ( $self, $params ) = @_; + + my $strings = {}; + + if ( defined $self->statuscode ) { + my $ill_batch_status = Koha::IllbatchStatuses->search( { code => $self->statuscode } ); + my $ill_batch_status_str = + $ill_batch_status->count + ? $ill_batch_status->next->name + : $self->statuscode; + + $strings->{status} = { + name => $ill_batch_status_str, + }; + } + + if ( defined $self->branchcode ) { + my $ill_batch_branch = Koha::Libraries->find( $self->branchcode ); + my $ill_batch_branchname_str = $ill_batch_branch ? $ill_batch_branch->branchname : $self->branchcode; + + $strings->{branchname} = $ill_batch_branchname_str; + } + + return $strings; +} + +=head3 to_api_mapping + +=cut + +sub to_api_mapping { + return { + id => 'batch_id', + branchcode => 'library_id', + borrowernumber => 'patron_id', + status => 'batch_status', + }; +} + =head3 _type my $type = Koha::Illbatch->_type; diff --git a/Koha/REST/V1/Illbatches.pm b/Koha/REST/V1/Illbatches.pm index 72efdc3ae8..b8edbc815e 100644 --- a/Koha/REST/V1/Illbatches.pm +++ b/Koha/REST/V1/Illbatches.pm @@ -38,63 +38,15 @@ Return a list of available ILL batches =cut sub list { - my $c = shift->openapi->valid_input; - - #FIXME: This should be $c->objects-search - my @batches = Koha::Illbatches->search()->as_list; + my $c = shift->openapi->valid_input or return; - #FIXME: Below should be coming from $c->objects accessors - # Get all patrons associated with all our batches - # in one go - my $patrons = {}; - foreach my $batch (@batches) { - my $patron_id = $batch->borrowernumber; - $patrons->{$patron_id} = 1; - } - my @patron_ids = keys %{$patrons}; - my $patron_results = Koha::Patrons->search( { borrowernumber => { -in => \@patron_ids } } ); - - # Get all branches associated with all our batches - # in one go - my $branches = {}; - foreach my $batch (@batches) { - my $branch_id = $batch->branchcode; - $branches->{$branch_id} = 1; - } - my @branchcodes = keys %{$branches}; - my $branch_results = Koha::Libraries->search( { branchcode => { -in => \@branchcodes } } ); - - # Get all batch statuses associated with all our batches - # in one go - my $statuses = {}; - foreach my $batch (@batches) { - my $code = $batch->statuscode; - $statuses->{$code} = 1; - } - my @statuscodes = keys %{$statuses}; - my $status_results = Koha::IllbatchStatuses->search( { code => { -in => \@statuscodes } } ); - - # Populate the response - my @to_return = (); - foreach my $it_batch (@batches) { - my $patron = $patron_results->find( { borrowernumber => $it_batch->borrowernumber } ); - my $branch = $branch_results->find( { branchcode => $it_batch->branchcode } ); - my $status = $status_results->find( { code => $it_batch->statuscode } ); - push @to_return, { - batch_id => $it_batch->id, - backend => $it_batch->backend, - library_id => $it_batch->branchcode, - name => $it_batch->name, - statuscode => $it_batch->statuscode, - patron_id => $it_batch->borrowernumber, - patron => $patron, - branch => $branch, - status => $status, - requests_count => $it_batch->requests_count - }; - } + return try { + my $illbatches = $c->objects->search( Koha::Illbatches->new ); + return $c->render( status => 200, openapi => $illbatches ); + } catch { + $c->unhandled_exception($_); + }; - return $c->render( status => 200, openapi => \@to_return ); } =head3 get diff --git a/api/v1/swagger/definitions/ill_batch.yaml b/api/v1/swagger/definitions/ill_batch.yaml index 177c06c35b..eadbe244c5 100644 --- a/api/v1/swagger/definitions/ill_batch.yaml +++ b/api/v1/swagger/definitions/ill_batch.yaml @@ -40,6 +40,11 @@ properties: requests_count: type: string description: The number of requests in this batch + _strings: + type: + - object + - "null" + description: Expanded coded fields (x-koha-embed) additionalProperties: false required: - name diff --git a/api/v1/swagger/paths/ill_batches.yaml b/api/v1/swagger/paths/ill_batches.yaml index c543aaf648..caada10af8 100644 --- a/api/v1/swagger/paths/ill_batches.yaml +++ b/api/v1/swagger/paths/ill_batches.yaml @@ -6,7 +6,27 @@ tags: - ill_batches summary: List ILL batches - parameters: [] + parameters: + - $ref: "../swagger.yaml#/parameters/page" + - $ref: "../swagger.yaml#/parameters/per_page" + - $ref: "../swagger.yaml#/parameters/match" + - $ref: "../swagger.yaml#/parameters/order_by" + - $ref: "../swagger.yaml#/parameters/q_param" + - $ref: "../swagger.yaml#/parameters/q_body" + - $ref: "../swagger.yaml#/parameters/request_id_header" + - name: x-koha-embed + in: header + required: false + description: Embed list sent as a request header + type: array + items: + type: string + enum: + - +strings + - requests+count + - patron + - branch + collectionFormat: csv produces: - application/json responses: diff --git a/koha-tmpl/intranet-tmpl/prog/js/ill-batch-table.js b/koha-tmpl/intranet-tmpl/prog/js/ill-batch-table.js index 011d192841..dd2d400771 100644 --- a/koha-tmpl/intranet-tmpl/prog/js/ill-batch-table.js +++ b/koha-tmpl/intranet-tmpl/prog/js/ill-batch-table.js @@ -27,11 +27,15 @@ table = initTable(); // Do the initial data population - window.doBatchApiRequest() - .then(function (response) { + window.doBatchApiRequest('', { + headers: { + 'x-koha-embed': '+strings,requests+count,patron' + } + }) + .then(function(response) { return response.json(); }) - .then(function (data) { + .then(function(data) { batchesProxy.data = data; }); @@ -87,8 +91,8 @@ } // A render function for branch name - var createBranch = function (data) { - return data.branchname; + var createBranch = function (x, y, data) { + return data._strings.branchname; }; // A render function for batch name @@ -102,7 +106,7 @@ // A render function for batch status var createStatus = function (x, y, data) { - return data.status.name; + return data._strings.status.name; }; // A render function for our patron link diff --git a/t/db_dependent/api/v1/ill_batches.t b/t/db_dependent/api/v1/ill_batches.t index ad0107f33e..b658ee36c1 100755 --- a/t/db_dependent/api/v1/ill_batches.t +++ b/t/db_dependent/api/v1/ill_batches.t @@ -85,11 +85,12 @@ subtest 'list() tests' => sub { ); # One batch created, should get returned - $t->get_ok("//$userid:$password@/api/v1/ill/batches")->status_is(200)->json_has( '/0/batch_id', 'Batch ID' ) - ->json_has( '/0/name', 'Batch name' )->json_has( '/0/backend', 'Backend name' ) - ->json_has( '/0/patron_id', 'Borrowernumber' )->json_has( '/0/library_id', 'Branchcode' ) - ->json_has( '/0/patron', 'patron embedded' )->json_has( '/0/branch', 'branch embedded' ) - ->json_has( '/0/requests_count', 'request count' ); + $t->get_ok( + "//$userid:$password@/api/v1/ill/batches" => { 'x-koha-embed' => '+strings,requests+count,patron,branch' } ) + ->status_is(200)->json_has( '/0/batch_id', 'Batch ID' )->json_has( '/0/name', 'Batch name' ) + ->json_has( '/0/backend', 'Backend name' )->json_has( '/0/patron_id', 'Borrowernumber' ) + ->json_has( '/0/library_id', 'Branchcode' )->json_has( '/0/patron', 'patron embedded' ) + ->json_has( '/0/branch', 'branch embedded' )->json_has( '/0/requests_count', 'request count' ); # Try to create a second batch with the same name, this should fail my $another_batch = $builder->build_object( { class => 'Koha::Illbatches', value => { name => $batch->name } } ); @@ -327,7 +328,7 @@ subtest 'update() tests' => sub { ] ); - my $batch_to_delete = $builder->build_object( { class => 'Koha::Cities' } ); + my $batch_to_delete = $builder->build_object( { class => 'Koha::Illbatches' } ); my $non_existent_id = $batch_to_delete->id; $batch_to_delete->delete; -- 2.30.2