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

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

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;

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;
+    my $c = shift->openapi->valid_input or return;
 
-    #FIXME: This should be $c->objects-search
+    return try {
-    my @batches = Koha::Illbatches->search()->as_list;
+        my $illbatches = $c->objects->search( Koha::Illbatches->new );
+        return $c->render( status => 200, openapi => $illbatches );
+    } catch {
+        $c->unhandled_exception($_);
+    };
 
-    #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 $c->render( status => 200, openapi => \@to_return );
 }
 
 =head3 get

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

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:

koha-tmpl/intranet-tmpl/prog/js/ill-batch-table.js

@@ -27,11 +27,15 @@
         table = initTable();
 
         // Do the initial data population
-        window.doBatchApiRequest()
+        window.doBatchApiRequest('', {
-            .then(function (response) {
+                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) {
+    var createBranch = function (x, y, data) {
-        return data.branchname;
+        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

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' )
+    $t->get_ok(
-        ->json_has( '/0/name',           'Batch name' )->json_has( '/0/backend', 'Backend name' )
+        "//$userid:$password@/api/v1/ill/batches" => { 'x-koha-embed' => '+strings,requests+count,patron,branch' } )
-        ->json_has( '/0/patron_id',      'Borrowernumber' )->json_has( '/0/library_id', 'Branchcode' )
+        ->status_is(200)->json_has( '/0/batch_id', 'Batch ID' )->json_has( '/0/name', 'Batch name' )
-        ->json_has( '/0/patron',         'patron embedded' )->json_has( '/0/branch', 'branch embedded' )
+        ->json_has( '/0/backend',    'Backend name' )->json_has( '/0/patron_id', 'Borrowernumber' )
-        ->json_has( '/0/requests_count', 'request count' );
+        ->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;