From c0624575407e977e4d12de2aef3c069d3c41801d Mon Sep 17 00:00:00 2001
From: Tomas Cohen Arazi <tomascohen@theke.io>
Date: Mon, 10 May 2021 10:01:02 +0200
Subject: [PATCH] Bug 28299: Make buildKohaItemsNamespace accept Koha::Items

This patch makes buildKohaItemsNamespace (and its only caller,
XSLTParse4Display) accept a an optional Koha::Items resultset.

This way we don't need to calculate (from the DB) the list of hidden
itemnumbers, but solve it in one query.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: David Nind <david@davidnind.com>
---
 C4/XSLT.pm           | 36 ++++++++++++++++++++++++------------
 opac/opac-shelves.pl | 19 +++++++------------
 2 files changed, 31 insertions(+), 24 deletions(-)

diff --git a/C4/XSLT.pm b/C4/XSLT.pm
index 3cdf451b28..b8ae90e978 100644
--- a/C4/XSLT.pm
+++ b/C4/XSLT.pm
@@ -190,7 +190,7 @@ sub get_xslt_sysprefs {
 }
 
 sub XSLTParse4Display {
-    my ( $biblionumber, $orig_record, $xslsyspref, $fixamps, $hidden_items, $sysxml, $xslfilename, $lang, $variables ) = @_;
+    my ( $biblionumber, $orig_record, $xslsyspref, $fixamps, $hidden_items, $sysxml, $xslfilename, $lang, $variables, $items_rs ) = @_;
 
     $sysxml ||= C4::Context->preference($xslsyspref);
     $xslfilename ||= C4::Context->preference($xslsyspref);
@@ -246,7 +246,7 @@ sub XSLTParse4Display {
     if ( $xslsyspref eq "OPACXSLTDetailsDisplay" || $xslsyspref eq "XSLTDetailsDisplay" || $xslsyspref eq "XSLTResultsDisplay" ) {
         $itemsxml = ""; #We don't use XSLT for items display on these pages
     } else {
-        $itemsxml = buildKohaItemsNamespace($biblionumber, $hidden_items);
+        $itemsxml = buildKohaItemsNamespace($biblionumber, $hidden_items, $items_rs);
     }
     my $xmlrecord = $record->as_xml(C4::Context->preference('marcflavour'));
 
@@ -289,22 +289,34 @@ sub XSLTParse4Display {
 
 =head2 buildKohaItemsNamespace
 
-Returns XML for items.
+    my $items_xml = buildKohaItemsNamespace( $biblionumber, [ $hidden_items, $items ] );
+
+Returns XML for items. It accepts two optional parameters:
+- I<$hidden_items>: An arrayref of itemnumber values, for items that should be hidden
+- I<$items>: A Koha::Items resultset, for the items to be returned
+
+If both parameters are passed, I<$items> is used as the basis resultset, and I<$hidden_items>
+are filtered out of it.
+
 Is only used in this module currently.
 
 =cut
 
 sub buildKohaItemsNamespace {
-    my ($biblionumber, $hidden_items) = @_;
+    my ($biblionumber, $hidden_items, $items_rs) = @_;
 
     $hidden_items ||= [];
-    my @items = Koha::Items->search(
-        {
-            'me.biblionumber' => $biblionumber,
-            'me.itemnumber'   => { not_in => $hidden_items }
-        },
-        { prefetch => [ 'branchtransfers', 'reserves' ] }
-    );
+
+    my $query = {};
+    $query = { 'me.itemnumber' => { not_in => $hidden_items } }
+      if $hidden_items;
+
+    unless ( $items_rs && ref($items_rs) eq 'Koha::Items' ) {
+        $query->{'me.biblionumber'} = $biblionumber;
+        $items_rs = Koha::Items->new;
+    }
+
+    my $items = $items_rs->search( $query, { prefetch => [ 'branchtransfers', 'reserves' ] } );
 
     my $shelflocations =
       { map { $_->{authorised_value} => $_->{opac_description} } Koha::AuthorisedValues->get_descriptions_by_koha_field( { frameworkcode => "", kohafield => 'items.location' } ) };
@@ -318,7 +330,7 @@ sub buildKohaItemsNamespace {
     my %descs = map { $_->{authorised_value} => $_ } Koha::AuthorisedValues->get_descriptions_by_koha_field( { kohafield => 'items.notforloan' } );
     my $ref_status = C4::Context->preference('Reference_NFL_Statuses') || '1|2';
 
-    for my $item (@items) {
+    while ( my $item = $items->next ) {
         my $status;
         my $substatus = '';
 
diff --git a/opac/opac-shelves.pl b/opac/opac-shelves.pl
index 7eb6e20bb8..55d11f6ab4 100755
--- a/opac/opac-shelves.pl
+++ b/opac/opac-shelves.pl
@@ -346,23 +346,18 @@ if ( $op eq 'view' ) {
                     });
                 }
 
-                my @items;
-                my $items = $biblio->items;
+                my $items = $biblio->items->filter_by_visible_in_opac({ patron => $patron });
                 my $allow_onshelf_holds;
-                my @hidden_items;
                 while ( my $item = $items->next ) {
-                    if ( $item->hidden_in_opac({rules => C4::Context->yaml_preference('OpacHiddenItems')} ) ) {
-                        push @hidden_items, $item->itemnumber;
-                        next;
-                    }
 
+                    # This method must take a Koha::Items rs
                     $allow_onshelf_holds ||= Koha::CirculationRules->get_onshelfholds_policy(
                         { item => $item, patron => $patron } );
 
-                    push @items, $item; # This is for non-xslt only
                 }
+
                 $this_item->{allow_onshelf_holds} = $allow_onshelf_holds;
-                $this_item->{'ITEM_RESULTS'} = \@items;
+                $this_item->{'ITEM_RESULTS'} = $items;
 
                 if ($xslfile) {
                     my $variables = {
@@ -371,9 +366,9 @@ if ( $op eq 'view' ) {
                     $this_item->{XSLTBloc} = XSLTParse4Display(
                         $biblionumber,          $record,
                         "OPACXSLTListsDisplay", 1,
-                        \@hidden_items,         $sysxml,
-                        $xslfile,               $lang,
-                        $variables
+                        undef,                 $sysxml,
+                        $xslfile,              $lang,
+                        $variables,            $items->reset
                     );
                 }
 
-- 
2.20.1