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

Srdjan srdjan at catalyst.net.nz
Mon May 11 02:07:23 CEST 2015


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/VirtualShelves/Page.pm |  8 ++---
 C4/XSLT.pm                | 23 ++++++------
 catalogue/detail.pl       | 12 +++----
 opac/opac-detail.pl       | 10 +++---
 6 files changed, 55 insertions(+), 108 deletions(-)

diff --git a/C4/Items.pm b/C4/Items.pm
index d1a866b..ed753f6 100644
--- a/C4/Items.pm
+++ b/C4/Items.pm
@@ -69,7 +69,6 @@ BEGIN {
         GetItemInfosOf
         GetItemsByBiblioitemnumber
         GetItemsInfo
-	GetItemsLocationInfo
 	GetHostItemsInfo
         GetItemnumbersForBiblio
         get_itemnumbers_of
@@ -1257,10 +1256,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
@@ -1298,7 +1301,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.
     my $query = "
@@ -1344,7 +1348,18 @@ sub GetItemsInfo {
      LEFT JOIN serial USING (serialid)
      LEFT JOIN itemtypes   ON   itemtypes.itemtype         = "
      . (C4::Context->preference('item-level_itypes') ? 'items.itype' : 'biblioitems.itemtype');
-    $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($biblionumber);
     my $i = 0;
@@ -1377,6 +1392,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 = ?
@@ -1401,71 +1419,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 990ed39..b5ab049 100644
--- a/C4/Search.pm
+++ b/C4/Search.pm
@@ -2057,7 +2057,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) {
@@ -2079,7 +2078,6 @@ sub searchResults {
                 # hidden based on OpacHiddenItems syspref
                 my @hi = C4::Items::GetHiddenItemnumbers($item);
                 if (scalar @hi) {
-                    push @hiddenitems, @hi;
                     $hideatopac_count++;
                     next;
                 }
@@ -2096,7 +2094,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}
@@ -2104,12 +2102,10 @@ sub searchResults {
             {
                 $onloan_count++;
                 my $key = $prefix . $item->{onloan} . $item->{barcode};
+                $onloan_items->{$key} = { %$item };
                 $onloan_items->{$key}->{due_date} = format_date( $item->{onloan} );
                 $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} );
 
@@ -2192,25 +2188,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} );
                 }
@@ -2239,7 +2230,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/VirtualShelves/Page.pm b/C4/VirtualShelves/Page.pm
index beed3e2..b0c7df0 100644
--- a/C4/VirtualShelves/Page.pm
+++ b/C4/VirtualShelves/Page.pm
@@ -263,11 +263,13 @@ sub shelfpage {
 
                 for my $this_item (@$items) {
                     my $biblionumber = $this_item->{'biblionumber'};
+                    # Getting items infos for location display
+                    my @items_infos = &GetItemsInfo( $this_item->{'biblionumber'}, "cn_sort" );
                     my $record = GetMarcBiblio($biblionumber);
                     if (C4::Context->preference("OPACXSLTResultsDisplay") && $type eq 'opac') {
-                        $this_item->{XSLTBloc} = XSLTParse4Display($biblionumber, $record, "OPACXSLTResultsDisplay");
+                        $this_item->{XSLTBloc} = XSLTParse4Display($biblionumber, $record, "OPACXSLTResultsDisplay", \@items_infos);
                     } elsif (C4::Context->preference("XSLTResultsDisplay") && $type eq 'intranet') {
-                        $this_item->{XSLTBloc} = XSLTParse4Display($biblionumber, $record, "XSLTResultsDisplay");
+                        $this_item->{XSLTBloc} = XSLTParse4Display($biblionumber, $record, "XSLTResultsDisplay", \@items_infos);
                     }
 
                     # the virtualshelfcontents table does not store these columns nor are they retrieved from the items
@@ -283,8 +285,6 @@ sub shelfpage {
                     $this_item->{'normalized_oclc'} = GetNormalizedOCLCNumber($record,$marcflavour);
                     $this_item->{'normalized_isbn'} = GetNormalizedISBN(undef,$record,$marcflavour);
                     if(!defined($this_item->{'size'})) { $this_item->{'size'} = "" }; #TT has problems with size
-                    # Getting items infos for location display
-                    my @items_infos = &GetItemsLocationInfo( $this_item->{'biblionumber'});
                     $this_item->{'itemsissued'} = CountItemsIssued( $this_item->{'biblionumber'} );
                     $this_item->{'ITEM_RESULTS'} = \@items_infos;
                     if ( grep {$_ eq $biblionumber} @cart_list) {
diff --git a/C4/XSLT.pm b/C4/XSLT.pm
index ea9c7f5..db32d6f 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
@@ -236,13 +245,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');
@@ -252,7 +255,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 74665b1..93f8721 100755
--- a/catalogue/detail.pl
+++ b/catalogue/detail.pl
@@ -84,12 +84,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) );
 
@@ -137,6 +131,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 ca882ea..b9ac3e3 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){
-- 
1.9.1


More information about the Koha-patches mailing list