Attachment 156749 Details for Bug 30719 – Bug 30719: QA follow-up: Rewrite Illbatches list endpoint

[patch] Bug 30719: QA follow-up: Rewrite Illbatches list endpoint

Bug-30719-QA-follow-up-Rewrite-Illbatches-list-end.patch (text/plain), 10.73 KB, created by Pedro Amorim on 2023-10-09 15:37:23 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Pedro Amorim

Created: 2023-10-09 15:37:23 UTC

Size: 10.73 KB

patch

obsolete

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

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