From 90aff6c57fbe993b84022979228a654ff36cf07a Mon Sep 17 00:00:00 2001
From: Nick Clemens <nick@bywatersolutions.com>
Date: Thu, 1 Aug 2019 15:42:16 +0000
Subject: [PATCH] Bug 23414: Use Koha Item objects in buildKohaItemsNamespace

Removing the use of C4::GetItemsInfo cuts down on the number of DB requests and speeds things up

To test I added 100 items to each of the first 20 records returned by a search for 'a'
I saw a reduction from ~30 seconds to ~26 seconds

This also makes the code a little cleaner and moves us toward the Koha namespace

To test:
1 - Perform a search in the OPAC and STaff Client with the Browser Console opened (F12)
2 - View the netwrok tab and see how long the pages take to load
3 - Apply the patches, restart all the things
4 - Repeat and note if the results are returned faster
---
 C4/XSLT.pm | 43 +++++++++++++++++++++----------------------
 1 file changed, 21 insertions(+), 22 deletions(-)

diff --git a/C4/XSLT.pm b/C4/XSLT.pm
index 648bab28ab..af4cfc6e26 100644
--- a/C4/XSLT.pm
+++ b/C4/XSLT.pm
@@ -292,11 +292,10 @@ Is only used in this module currently.
 sub buildKohaItemsNamespace {
     my ($biblionumber, $hidden_items) = @_;
 
-    my @items = C4::Items::GetItemsInfo($biblionumber);
-    if ($hidden_items && @$hidden_items) {
-        my %hi = map {$_ => 1} @$hidden_items;
-        @items = grep { !$hi{$_->{itemnumber}} } @items;
-    }
+    my $search_params;
+    $search_params->{biblionumber} = $biblionumber;
+    $search_params->{itemnumber} = { not_in => $hidden_items } if $hidden_items;
+    my @items = Koha::Items->search($search_params);
 
     my $shelflocations =
       { map { $_->{authorised_value} => $_->{opac_description} } Koha::AuthorisedValues->get_descriptions_by_koha_field( { frameworkcode => GetFrameworkCode($biblionumber), kohafield => 'items.location' } ) };
@@ -312,28 +311,28 @@ sub buildKohaItemsNamespace {
     for my $item (@items) {
         my $status;
 
-        my ( $transfertwhen, $transfertfrom, $transfertto ) = C4::Circulation::GetTransfers($item->{itemnumber});
+        my ( $transfertwhen, $transfertfrom, $transfertto ) = C4::Circulation::GetTransfers($item->itemnumber);
 
-        my $reservestatus = C4::Reserves::GetReserveStatus( $item->{itemnumber} );
+        my $reservestatus = C4::Reserves::GetReserveStatus( $item->itemnumber );
 
-        if ( ( $item->{itype} && $itemtypes->{ $item->{itype} }->{notforloan} ) || $item->{notforloan} || $item->{onloan} || $item->{withdrawn} || $item->{itemlost} || $item->{damaged} ||
-             (defined $transfertwhen && $transfertwhen ne '') || $item->{itemnotforloan} || (defined $reservestatus && $reservestatus eq "Waiting") || $item->{has_pending_hold} ){
-            if ( $item->{notforloan} < 0) {
+        if ( ( $item->itype && $itemtypes->{ $item->itype }->{notforloan} ) || $item->notforloan || $item->onloan || $item->withdrawn || $item->itemlost || $item->damaged ||
+             (defined $transfertwhen && $transfertwhen ne '') || $item->{itemnotforloan} || (defined $reservestatus && $reservestatus eq "Waiting") || $item->has_pending_hold ){
+            if ( $item->notforloan < 0) {
                 $status = "On order";
             }
-            if ( $item->{itemnotforloan} && $item->{itemnotforloan} > 0 || $item->{notforloan} && $item->{notforloan} > 0 || $item->{itype} && $itemtypes->{ $item->{itype} }->{notforloan} && $itemtypes->{ $item->{itype} }->{notforloan} == 1 ) {
+            if ( $item->notforloan && $item->notforloan > 0 || $item->itype && $itemtypes->{ $item->itype }->{notforloan} && $itemtypes->{ $item->itype }->{notforloan} == 1 ) {
                 $status = "reference";
             }
-            if ($item->{onloan}) {
+            if ($item->onloan) {
                 $status = "Checked out";
             }
-            if ( $item->{withdrawn}) {
+            if ( $item->withdrawn) {
                 $status = "Withdrawn";
             }
-            if ($item->{itemlost}) {
+            if ($item->itemlost) {
                 $status = "Lost";
             }
-            if ($item->{damaged}) {
+            if ($item->damaged) {
                 $status = "Damaged";
             }
             if (defined $transfertwhen && $transfertwhen ne '') {
@@ -342,18 +341,18 @@ sub buildKohaItemsNamespace {
             if (defined $reservestatus && $reservestatus eq "Waiting") {
                 $status = 'Waiting';
             }
-            if ($item->{has_pending_hold}) {
+            if ($item->has_pending_hold) {
                 $status = 'Pending hold';
             }
         } else {
             $status = "available";
         }
-        my $homebranch = $item->{homebranch}? xml_escape($branches{$item->{homebranch}}):'';
-        my $holdingbranch = $item->{holdingbranch}? xml_escape($branches{$item->{holdingbranch}}):'';
-        $location = $item->{location}? xml_escape($shelflocations->{$item->{location}}||$item->{location}):'';
-        $ccode = $item->{ccode}? xml_escape($ccodes->{$item->{ccode}}||$item->{ccode}):'';
-        my $itemcallnumber = xml_escape($item->{itemcallnumber});
-        my $stocknumber = $item->{stocknumber}? xml_escape($item->{stocknumber}):'';
+        my $homebranch = $item->homebranch? xml_escape($branches{$item->homebranch}):'';
+        my $holdingbranch = $item->holdingbranch? xml_escape($branches{$item->holdingbranch}):'';
+        $location = $item->location? xml_escape($shelflocations->{$item->location}||$item->location):'';
+        $ccode = $item->ccode? xml_escape($ccodes->{$item->ccode}||$item->ccode):'';
+        my $itemcallnumber = xml_escape($item->itemcallnumber);
+        my $stocknumber = $item->stocknumber? xml_escape($item->stocknumber):'';
         $xml .=
             "<item>"
           . "<homebranch>$homebranch</homebranch>"
-- 
2.11.0