[Koha-patches] [PATCH] bug_11213: Changed XSLTParse4Display() interface

Srdjan srdjan at catalyst.net.nz
Fri Apr 1 01:45:13 CEST 2016


The list of biblio items is passed on now, instead of GetItemsInfo() being
called. This is because the callers already have the list ready, so the
GetItemsInfo() call is being duplicated unnecessarily.
Search::searchResults() builds items list from XML, and that one is
passed instead.

* XSLT::XSLTParse4Display()
- supply the items list as input param
- removed hidden items list param - hidden should not be in the items
  list
- changed buildKohaItemsNamespace() accordingly

* Items
- removed GetItemsLocationInfo()
- added sort_by input param to GetItemsInfo()
- VirtualShelves::Page::shelfpage() - replaced GetItemsLocationInfo() call
  with GetItemsInfo() call, passing order_by "cn_sort"

* catalogue/detail.pl, opac/opac-detail.pl, shelfpage()
- added items list to the XSLTParse4Display() call

* Search::searchResults()
- include all available info when building items lists
- added combined items list (available, on loan, other) to the
  XSLTParse4Display() call

To test:
This change is a noop, so following screens need to be checked against
any changes:
* Intranet:
- catalogue/search.pl (results)
- catalogue/detail.pl
- virtualshelves/shelves.pl
* Opac
- opac-search.pl (results, hidelostitems syspref on and off)
- opac-detail.pl
- opac-shelves.pl

The display should stay the same before and after patch. The speed
should increase though.
---
 C4/Items.pm         | 91 +++++++++++++----------------------------------------
 C4/Search.pm        | 19 +++--------
 C4/XSLT.pm          | 23 ++++++++------
 catalogue/detail.pl | 12 +++----
 opac/opac-detail.pl | 10 +++---
 5 files changed, 51 insertions(+), 104 deletions(-)

diff --git a/C4/Items.pm b/C4/Items.pm
index a0dfb29..fb32cf0 100644
--- a/C4/Items.pm
+++ b/C4/Items.pm
@@ -69,7 +69,6 @@ BEGIN {
         GetItemInfosOf
         GetItemsByBiblioitemnumber
         GetItemsInfo
-	GetItemsLocationInfo
 	GetHostItemsInfo
         GetItemnumbersForBiblio
         get_itemnumbers_of
@@ -1258,10 +1257,14 @@ sub GetItemsByBiblioitemnumber {
 
 =head2 GetItemsInfo
 
-  @results = GetItemsInfo($biblionumber);
+  @results = GetItemsInfo($biblionumber, $order_by);
 
 Returns information about items with the given biblionumber.
 
+The list is ordered by home branch name and some complex criteria
+within it (see the code), unless $order_by is specified.
+Currently only "cn_sort" is supported.
+
 C<GetItemsInfo> returns a list of references-to-hash. Each element
 contains a number of keys. Most of them are attributes from the
 C<biblio>, C<biblioitems>, C<items>, and C<itemtypes> tables in the
@@ -1299,7 +1302,8 @@ If this is set, it is set to C<One Order>.
 =cut
 
 sub GetItemsInfo {
-    my ( $biblionumber ) = @_;
+    my ( $biblionumber, $order_by ) = @_;
+
     my $dbh   = C4::Context->dbh;
     # note biblioitems.* must be avoided to prevent large marc and marcxml fields from killing performance.
     require C4::Languages;
@@ -1354,7 +1358,18 @@ sub GetItemsInfo {
         AND localization.lang = ?
     |;
 
-    $query .= " WHERE items.biblionumber = ? ORDER BY home.branchname, items.enumchron, LPAD( items.copynumber, 8, '0' ), items.dateaccessioned DESC" ;
+    $query .= " WHERE items.biblionumber = ? ORDER BY ";
+    my $order_by_cause = "home.branchname, items.enumchron, LPAD( items.copynumber, 8, '0' ), items.dateaccessioned DESC" ;
+    if ($order_by) {
+        if ($order_by eq 'cn_sort') {
+            $order_by_cause = "cn_sort ASC";
+        }
+        else {
+            warn qq{Unsupported order by "$order_by"};
+        }
+    }
+    $query .= $order_by_cause;
+
     my $sth = $dbh->prepare($query);
     $sth->execute($language, $biblionumber);
     my $i = 0;
@@ -1387,6 +1402,9 @@ sub GetItemsInfo {
             $data->{stack}          = C4::Koha::GetKohaAuthorisedValueLib( $code, $data->{stack} );
         }
 
+        $data->{location_intranet} = GetKohaAuthorisedValueLib('LOC', $data->{location});
+        $data->{location_opac}     = GetKohaAuthorisedValueLib('LOC', $data->{location}, 1);
+
         # Find the last 3 people who borrowed this item.
         my $sth2 = $dbh->prepare("SELECT * FROM old_issues,borrowers
                                     WHERE itemnumber = ?
@@ -1411,71 +1429,6 @@ sub GetItemsInfo {
         : @results;
 }
 
-=head2 GetItemsLocationInfo
-
-  my @itemlocinfo = GetItemsLocationInfo($biblionumber);
-
-Returns the branch names, shelving location and itemcallnumber for each item attached to the biblio in question
-
-C<GetItemsInfo> returns a list of references-to-hash. Data returned:
-
-=over 2
-
-=item C<$data-E<gt>{homebranch}>
-
-Branch Name of the item's homebranch
-
-=item C<$data-E<gt>{holdingbranch}>
-
-Branch Name of the item's holdingbranch
-
-=item C<$data-E<gt>{location}>
-
-Item's shelving location code
-
-=item C<$data-E<gt>{location_intranet}>
-
-The intranet description for the Shelving Location as set in authorised_values 'LOC'
-
-=item C<$data-E<gt>{location_opac}>
-
-The OPAC description for the Shelving Location as set in authorised_values 'LOC'.  Falls back to intranet description if no OPAC 
-description is set.
-
-=item C<$data-E<gt>{itemcallnumber}>
-
-Item's itemcallnumber
-
-=item C<$data-E<gt>{cn_sort}>
-
-Item's call number normalized for sorting
-
-=back
-  
-=cut
-
-sub GetItemsLocationInfo {
-        my $biblionumber = shift;
-        my @results;
-
-	my $dbh = C4::Context->dbh;
-	my $query = "SELECT a.branchname as homebranch, b.branchname as holdingbranch, 
-			    location, itemcallnumber, cn_sort
-		     FROM items, branches as a, branches as b
-		     WHERE homebranch = a.branchcode AND holdingbranch = b.branchcode 
-		     AND biblionumber = ?
-		     ORDER BY cn_sort ASC";
-	my $sth = $dbh->prepare($query);
-        $sth->execute($biblionumber);
-
-        while ( my $data = $sth->fetchrow_hashref ) {
-             $data->{location_intranet} = GetKohaAuthorisedValueLib('LOC', $data->{location});
-             $data->{location_opac}= GetKohaAuthorisedValueLib('LOC', $data->{location}, 1);
-	     push @results, $data;
-	}
-	return @results;
-}
-
 =head2 GetHostItemsInfo
 
 	$hostiteminfo = GetHostItemsInfo($hostfield);
diff --git a/C4/Search.pm b/C4/Search.pm
index 9b7bde3..a1e6b4c 100644
--- a/C4/Search.pm
+++ b/C4/Search.pm
@@ -2007,7 +2007,6 @@ sub searchResults {
         my $items_count           = scalar(@fields);
         my $maxitems_pref = C4::Context->preference('maxItemsinSearchResults');
         my $maxitems = $maxitems_pref ? $maxitems_pref - 1 : 1;
-        my @hiddenitems; # hidden itemnumbers based on OpacHiddenItems syspref
 
         # loop through every item
         foreach my $field (@fields) {
@@ -2029,7 +2028,6 @@ sub searchResults {
                 # hidden based on OpacHiddenItems syspref
                 my @hi = C4::Items::GetHiddenItemnumbers($item);
                 if (scalar @hi) {
-                    push @hiddenitems, @hi;
                     $hideatopac_count++;
                     next;
                 }
@@ -2046,7 +2044,7 @@ sub searchResults {
                 $item->{'branchname'} = $branches{$item->{$otherbranch}};
             }
 
-			my $prefix = $item->{$hbranch} . '--' . $item->{location} . $item->{itype} . $item->{itemcallnumber};
+            my $prefix = $item->{$hbranch} . '--' . $item->{location} . $item->{itype} . $item->{itemcallnumber};
 # For each grouping of items (onloan, available, unavailable), we build a key to store relevant info about that item
             my $userenv = C4::Context->userenv;
             if ( $item->{onloan}
@@ -2054,12 +2052,10 @@ sub searchResults {
             {
                 $onloan_count++;
                 my $key = $prefix . $item->{onloan} . $item->{barcode};
+                $onloan_items->{$key} = { %$item };
                 $onloan_items->{$key}->{due_date} = output_pref( { dt => dt_from_string( $item->{onloan} ), dateonly => 1 } );
                 $onloan_items->{$key}->{count}++ if $item->{$hbranch};
-                $onloan_items->{$key}->{branchname}     = $item->{branchname};
                 $onloan_items->{$key}->{location}       = $shelflocations->{ $item->{location} };
-                $onloan_items->{$key}->{itemcallnumber} = $item->{itemcallnumber};
-                $onloan_items->{$key}->{description}    = $item->{description};
                 $onloan_items->{$key}->{imageurl} =
                   getitemtypeimagelocation( $search_context, $itemtypes{ $item->{itype} }->{imageurl} );
 
@@ -2145,25 +2141,20 @@ sub searchResults {
                     $other_count++;
 
                     my $key = $prefix . $item->{status};
-                    foreach (qw(withdrawn itemlost damaged branchname itemcallnumber)) {
-                        $other_items->{$key}->{$_} = $item->{$_};
-                    }
+                    $other_items->{$key} = { %$item };
                     $other_items->{$key}->{intransit} = ( $transfertwhen ne '' ) ? 1 : 0;
                     $other_items->{$key}->{onhold} = ($reservestatus) ? 1 : 0;
                     $other_items->{$key}->{notforloan} = GetAuthorisedValueDesc('','',$item->{notforloan},'','',$notforloan_authorised_value) if $notforloan_authorised_value and $item->{notforloan};
 					$other_items->{$key}->{count}++ if $item->{$hbranch};
 					$other_items->{$key}->{location} = $shelflocations->{ $item->{location} };
-					$other_items->{$key}->{description} = $item->{description};
 					$other_items->{$key}->{imageurl} = getitemtypeimagelocation( $search_context, $itemtypes{ $item->{itype} }->{imageurl} );
                 }
                 # item is available
                 else {
                     $can_place_holds = 1;
                     $available_count++;
+                    $available_items->{$prefix} = { %$item };
 					$available_items->{$prefix}->{count}++ if $item->{$hbranch};
-					foreach (qw(branchname itemcallnumber description)) {
-                    	$available_items->{$prefix}->{$_} = $item->{$_};
-					}
 					$available_items->{$prefix}->{location} = $shelflocations->{ $item->{location} };
 					$available_items->{$prefix}->{imageurl} = getitemtypeimagelocation( $search_context, $itemtypes{ $item->{itype} }->{imageurl} );
                 }
@@ -2192,7 +2183,7 @@ sub searchResults {
         # XSLT processing of some stuff
         my $interface = $search_context eq 'opac' ? 'OPAC' : '';
         if (!$scan && C4::Context->preference($interface . "XSLTResultsDisplay")) {
-            $oldbiblio->{XSLTResultsRecord} = XSLTParse4Display($oldbiblio->{biblionumber}, $marcrecord, $interface."XSLTResultsDisplay", 1, \@hiddenitems);
+            $oldbiblio->{XSLTResultsRecord} = XSLTParse4Display($oldbiblio->{biblionumber}, $marcrecord, $interface."XSLTResultsDisplay", [@available_items_loop, @onloan_items_loop, @other_items_loop], 1);
         # the last parameter tells Koha to clean up the problematic ampersand entities that Zebra outputs
         }
 
diff --git a/C4/XSLT.pm b/C4/XSLT.pm
index dd13c50..874deee 100644
--- a/C4/XSLT.pm
+++ b/C4/XSLT.pm
@@ -156,8 +156,17 @@ sub _get_best_default_xslt_filename {
     return $xslfilename;
 }
 
+=head2 XSLTParse4Display( $biblionumber, $orig_record, $xslsyspref, $items, $fixamps )
+
+  $items => an array of items rerords, as returned from eg. GetItemsInfo
+
+Returns XSLT block
+
+=cut
+
 sub XSLTParse4Display {
-    my ( $biblionumber, $orig_record, $xslsyspref, $fixamps, $hidden_items ) = @_;
+    my ( $biblionumber, $orig_record, $xslsyspref, $items, $fixamps ) = @_;
+
     my $xslfilename = C4::Context->preference($xslsyspref);
     if ( $xslfilename =~ /^\s*"?default"?\s*$/i ) {
         my $htdocs;
@@ -195,7 +204,7 @@ sub XSLTParse4Display {
 
     # grab the XML, run it through our stylesheet, push it out to the browser
     my $record = transformMARCXML4XSLT($biblionumber, $orig_record);
-    my $itemsxml  = buildKohaItemsNamespace($biblionumber, $hidden_items);
+    my $itemsxml  = $items ? buildKohaItemsNamespace($biblionumber, $items) : "";
     my $xmlrecord = $record->as_xml(C4::Context->preference('marcflavour'));
     my $sysxml = "<sysprefs>\n";
     foreach my $syspref ( qw/ hidelostitems OPACURLOpenInNewWindow
@@ -242,13 +251,7 @@ Is only used in this module currently.
 =cut
 
 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 ($biblionumber, $items) = @_;
 
     my $shelflocations = GetKohaAuthorisedValues('items.location',GetFrameworkCode($biblionumber), 'opac');
     my $ccodes         = GetKohaAuthorisedValues('items.ccode',GetFrameworkCode($biblionumber), 'opac');
@@ -258,7 +261,7 @@ sub buildKohaItemsNamespace {
     my $location = "";
     my $ccode = "";
     my $xml = '';
-    for my $item (@items) {
+    for my $item (@$items) {
         my $status;
 
         my ( $transfertwhen, $transfertfrom, $transfertto ) = C4::Circulation::GetTransfers($item->{itemnumber});
diff --git a/catalogue/detail.pl b/catalogue/detail.pl
index c4afb62..bea9c13 100755
--- a/catalogue/detail.pl
+++ b/catalogue/detail.pl
@@ -83,12 +83,6 @@ my $fw           = GetFrameworkCode($biblionumber);
 my $showallitems = $query->param('showallitems');
 my $marcflavour  = C4::Context->preference("marcflavour");
 
-# XSLT processing of some stuff
-if (C4::Context->preference("XSLTDetailsDisplay") ) {
-    $template->param('XSLTDetailsDisplay' =>'1',
-        'XSLTBloc' => XSLTParse4Display($biblionumber, $record, "XSLTDetailsDisplay") );
-}
-
 $template->param( 'SpineLabelShowPrintOnBibDetails' => C4::Context->preference("SpineLabelShowPrintOnBibDetails") );
 $template->param( ocoins => GetCOinSBiblio($record) );
 
@@ -136,6 +130,12 @@ if (@hostitems){
 	push (@items, at hostitems);
 }
 
+# XSLT processing of some stuff
+if (C4::Context->preference("XSLTDetailsDisplay") ) {
+    $template->param('XSLTDetailsDisplay' =>'1',
+        'XSLTBloc' => XSLTParse4Display($biblionumber, $record, "XSLTDetailsDisplay", \@all_items) );
+}
+
 my $dat = &GetBiblioData($biblionumber);
 
 #coping with subscriptions
diff --git a/opac/opac-detail.pl b/opac/opac-detail.pl
index 6d71bed..d753700 100755
--- a/opac/opac-detail.pl
+++ b/opac/opac-detail.pl
@@ -140,11 +140,6 @@ SetUTF8Flag($record);
 my $marcflavour      = C4::Context->preference("marcflavour");
 my $ean = GetNormalizedEAN( $record, $marcflavour );
 
-# XSLT processing of some stuff
-if (C4::Context->preference("OPACXSLTDetailsDisplay") ) {
-    $template->param( 'XSLTBloc' => XSLTParse4Display($biblionumber, $record, "OPACXSLTDetailsDisplay" ) );
-}
-
 my $OpacBrowseResults = C4::Context->preference("OpacBrowseResults");
 $template->{VARS}->{'OpacBrowseResults'} = $OpacBrowseResults;
 
@@ -480,6 +475,11 @@ if ($hideitems) {
     @items = @all_items;
 }
 
+# XSLT processing of some stuff
+if (C4::Context->preference("OPACXSLTDetailsDisplay") ) {
+    $template->param( 'XSLTBloc' => XSLTParse4Display($biblionumber, $record, "OPACXSLTDetailsDisplay", \@items) );
+}
+
 my $branches = GetBranches();
 my $branch = '';
 if (C4::Context->userenv){
-- 
2.5.0


More information about the Koha-patches mailing list