Attachment 156783 Details for Bug 30719 – Bug 30719: QA follow-up: Rewrite remaining ill batches endpoints

[patch] Bug 30719: QA follow-up: Rewrite remaining ill batches endpoints

Bug-30719-QA-follow-up-Rewrite-remaining-ill-batch.patch (text/plain), 9.75 KB, created by Pedro Amorim on 2023-10-10 10:18:27 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Pedro Amorim

Created: 2023-10-10 10:18:27 UTC

Size: 9.75 KB

patch

obsolete

>From 4543a1fc1efd3f50bec465dce81ded8df36f5b07 Mon Sep 17 00:00:00 2001
>From: Pedro Amorim <pedro.amorim@ptfs-europe.com>
>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
>---
> 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.30.2

Actions: View | Diff | Splinter Review

Attachments on bug 30719: 144281 | 144285 | 144286 | 144287 | 144288 | 144289 | 144290 | 144291 | 144292 | 144293 | 144294 | 144295 | 144296 | 144297 | 144298 | 144299 | 144300 | 144301 | 144302 | 145760 | 145869 | 145870 | 145876 | 145934 | 146152 | 147658 | 147929 | 147930 | 147931 | 147932 | 147933 | 147934 | 147952 | 147953 | 147954 | 147955 | 147956 | 151152 | 151153 | 151154 | 151155 | 151156 | 151284 | 151285 | 151286 | 151287 | 151288 | 151303 | 151304 | 151305 | 151306 | 151307 | 151308 | 151309 | 151310 | 151311 | 151312 | 151513 | 151514 | 151515 | 151516 | 151517 | 151519 | 151520 | 151521 | 151522 | 151523 | 151571 | 151572 | 151573 | 151574 | 151575 | 151611 | 151612 | 151613 | 151614 | 151615 | 151804 | 151805 | 151806 | 151807 | 151808 | 151809 | 154046 | 154047 | 154048 | 154049 | 154050 | 154051 | 154052 | 155421 | 155422 | 155423 | 155424 | 155425 | 155426 | 155427 | 155428 | 155429 | 155430 | 155431 | 155432 | 155433 | 155434 | 155435 | 156556 | 156748 | 156749 | 156752 | 156783 | 156785 | 156811 | 156812 | 156827 | 156828