From 833a808cdf70a63e3a5089210e9624ef5fa59b00 Mon Sep 17 00:00:00 2001 From: Pedro Amorim Date: Mon, 9 Oct 2023 15:54:54 +0000 Subject: [PATCH] Bug 30719: (QA follow-up) Rewrite remaining ill batches endpoints get, add and update rewrite Signed-off-by: Tomas Cohen Arazi --- Koha/REST/V1/Illbatches.pm | 74 +++++-------------- api/v1/swagger/paths/ill_batches.yaml | 33 +++++++++ .../intranet-tmpl/prog/js/ill-batch-modal.js | 12 ++- t/db_dependent/api/v1/ill_batches.t | 9 ++- 4 files changed, 68 insertions(+), 60 deletions(-) diff --git a/Koha/REST/V1/Illbatches.pm b/Koha/REST/V1/Illbatches.pm index b8edbc815e..1b25c7956c 100644 --- a/Koha/REST/V1/Illbatches.pm +++ b/Koha/REST/V1/Illbatches.pm @@ -56,34 +56,25 @@ Get one batch =cut sub get { - my $c = shift->openapi->valid_input; + my $c = shift->openapi->valid_input or return; - my $batchid = $c->param('ill_batch_id'); + return try { + my $ill_batch = $c->objects->find( Koha::Illbatches->search, $c->param('ill_batch_id') ); - my $batch = Koha::Illbatches->find($batchid); + unless ($ill_batch) { + return $c->render( + status => 404, + openapi => { error => "ILL batch not found" } + ); + } - if ( not defined $batch ) { return $c->render( - status => 404, - openapi => { error => "ILL batch not found" } + status => 200, + openapi => $ill_batch ); - } - - return $c->render( - status => 200, - openapi => { - batch_id => $batch->id, - backend => $batch->backend, - library_id => $batch->branchcode, - name => $batch->name, - statuscode => $batch->statuscode, - patron_id => $batch->borrowernumber, - patron => $batch->patron->unblessed, - branch => $batch->branch->unblessed, - status => $batch->status->unblessed, - requests_count => $batch->requests_count - } - ); + } catch { + $c->unhandled_exception($_); + }; } =head3 add @@ -97,8 +88,6 @@ sub add { my $body = $c->req->json; - # We receive cardnumber, so we need to look up the corresponding - # borrowernumber my $patron = Koha::Patrons->find( { cardnumber => $body->{cardnumber} } ); if ( not defined $patron ) { @@ -113,26 +102,16 @@ sub add { $body->{branchcode} = delete $body->{library_id}; return try { - my $batch = Koha::Illbatch->new($body); + my $batch = Koha::Illbatch->new_from_api($body); $batch->create_and_log; + $c->res->headers->location( $c->req->url->to_string . '/' . $batch->id ); - my $ret = { - batch_id => $batch->id, - backend => $batch->backend, - library_id => $batch->branchcode, - name => $batch->name, - statuscode => $batch->statuscode, - patron_id => $batch->borrowernumber, - patron => $batch->patron->unblessed, - branch => $batch->branch->unblessed, - status => $batch->status->unblessed, - requests_count => 0 - }; + my $ill_batch = $c->objects->find( Koha::Illbatches->search, $batch->id ); return $c->render( status => 201, - openapi => $ret + openapi => $ill_batch ); } catch { if ( blessed $_ ) { @@ -158,7 +137,7 @@ sub update { my $batch = Koha::Illbatches->find( $c->param('ill_batch_id') ); - if ( not defined $batch ) { + unless ($batch) { return $c->render( status => 404, openapi => { error => "ILL batch not found" } @@ -173,22 +152,9 @@ sub update { return try { $batch->update_and_log($params); - my $ret = { - batch_id => $batch->id, - backend => $batch->backend, - library_id => $batch->branchcode, - name => $batch->name, - statuscode => $batch->statuscode, - patron_id => $batch->borrowernumber, - patron => $batch->patron->unblessed, - branch => $batch->branch->unblessed, - status => $batch->status->unblessed, - requests_count => $batch->requests_count - }; - return $c->render( status => 200, - openapi => $ret + openapi => $batch->to_api ); } catch { $c->unhandled_exception($_); diff --git a/api/v1/swagger/paths/ill_batches.yaml b/api/v1/swagger/paths/ill_batches.yaml index caada10af8..dd3173cdd6 100644 --- a/api/v1/swagger/paths/ill_batches.yaml +++ b/api/v1/swagger/paths/ill_batches.yaml @@ -73,6 +73,19 @@ required: true schema: $ref: "../swagger.yaml#/definitions/ill_batch" + - 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: @@ -127,6 +140,26 @@ description: ILL batch id/name/contents required: true type: string + - $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-modal.js b/koha-tmpl/intranet-tmpl/prog/js/ill-batch-modal.js index 607e5582f6..1b8a4ab8b0 100644 --- a/koha-tmpl/intranet-tmpl/prog/js/ill-batch-modal.js +++ b/koha-tmpl/intranet-tmpl/prog/js/ill-batch-modal.js @@ -309,7 +309,7 @@ var payload = { batch_id: batchId, ill_backend_id: batch.data.backend, - patron_id: batch.data.patron.borrowernumber, + patron_id: batch.data.patron.patron_id, library_id: batch.data.library_id, extended_attributes: extended_attributes }; @@ -499,7 +499,11 @@ // Get the batch function fetchBatch() { - window.doBatchApiRequest("/" + batchId) + window.doBatchApiRequest("/" + batchId, { + headers: { + 'x-koha-embed': 'patron' + } + }) .then(function (response) { return response.json(); }) @@ -528,7 +532,8 @@ return doBatchApiRequest('', { method: 'POST', headers: { - 'Content-type': 'application/json' + 'Content-type': 'application/json', + 'x-koha-embed': 'patron' }, body: JSON.stringify({ name: nameInput.value, @@ -572,6 +577,7 @@ function updateBatch() { var selectedBranchcode = branchcodeSelect.selectedOptions[0].value; var selectedStatuscode = statusesSelect.selectedOptions[0].value; + return doBatchApiRequest('/' + batch.data.batch_id, { method: 'PUT', headers: { diff --git a/t/db_dependent/api/v1/ill_batches.t b/t/db_dependent/api/v1/ill_batches.t index b658ee36c1..3b2b5c57fa 100755 --- a/t/db_dependent/api/v1/ill_batches.t +++ b/t/db_dependent/api/v1/ill_batches.t @@ -161,7 +161,8 @@ subtest 'get() tests' => sub { $patron->set_password( { password => $password, skip_validation => 1 } ); my $unauth_userid = $patron->userid; - $t->get_ok( "//$userid:$password@/api/v1/ill/batches/" . $batch->id )->status_is(200) + $t->get_ok( "//$userid:$password@/api/v1/ill/batches/" + . $batch->id => { 'x-koha-embed' => '+strings,requests+count,patron,branch' } )->status_is(200) ->json_has( '/batch_id', 'Batch ID' )->json_has( '/name', 'Batch name' ) ->json_has( '/backend', 'Backend name' )->json_has( '/patron_id', 'Borrowernumber' ) ->json_has( '/library_id', 'Branchcode' )->json_has( '/patron', 'patron embedded' ) @@ -238,11 +239,13 @@ subtest 'add() tests' => sub { # Authorized attempt to write my $batch_id = - $t->post_ok( "//$userid:$password@/api/v1/ill/batches" => json => $batch_metadata )->status_is(201) + $t->post_ok( "//$userid:$password@/api/v1/ill/batches" => + { 'x-koha-embed' => '+strings,requests+count,patron,branch' } => json => $batch_metadata ) + ->status_is(201) ->json_is( '/name' => $batch_metadata->{name} )->json_is( '/backend' => $batch_metadata->{backend} ) ->json_is( '/patron_id' => $librarian->borrowernumber ) ->json_is( '/library_id' => $batch_metadata->{library_id} )->json_is( '/statuscode' => $batch_status->code ) - ->json_has('/patron')->json_has('/status')->json_has('/requests_count')->json_has('/branch'); + ->json_has('/patron')->json_has('/_strings/status')->json_has('/requests_count')->json_has('/branch'); # Authorized attempt to create with null id $batch_metadata->{id} = undef; -- 2.39.2