From 75e73fff45b35aa7ac0d4d8bbe9e86b42bf1e738 Mon Sep 17 00:00:00 2001 From: Srdjan Date: Fri, 8 Nov 2013 15:45:13 +1300 Subject: [PATCH] bug_11213: Changed XSLTParse4Display() interface 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 11ff4ca..62af8dd 100644 --- a/C4/Items.pm +++ b/C4/Items.pm @@ -69,7 +69,6 @@ BEGIN { GetItemInfosOf GetItemsByBiblioitemnumber GetItemsInfo - GetItemsLocationInfo GetHostItemsInfo GetItemnumbersForBiblio get_itemnumbers_of @@ -1232,10 +1231,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 returns a list of references-to-hash. Each element contains a number of keys. Most of them are attributes from the C, C, C, and C tables in the @@ -1273,7 +1276,8 @@ If this is set, it is set to C. =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 = " @@ -1319,7 +1323,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; @@ -1352,6 +1367,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 = ? @@ -1376,71 +1394,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 returns a list of references-to-hash. Data returned: - -=over 2 - -=item C<$data-E{homebranch}> - -Branch Name of the item's homebranch - -=item C<$data-E{holdingbranch}> - -Branch Name of the item's holdingbranch - -=item C<$data-E{location}> - -Item's shelving location code - -=item C<$data-E{location_intranet}> - -The intranet description for the Shelving Location as set in authorised_values 'LOC' - -=item C<$data-E{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{itemcallnumber}> - -Item's itemcallnumber - -=item C<$data-E{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 8d110c1..0eef1ac 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,18 +2094,16 @@ 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} && !(C4::Members::GetHideLostItemsPreference($userenv->{'number'}) && $item->{itemlost}) ) { $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} ); # if something's checked out and lost, mark it as 'long overdue' if ( $item->{itemlost} ) { @@ -2187,25 +2183,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} ); } @@ -2234,7 +2225,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 5a498cd..506e9d4 100644 --- a/C4/VirtualShelves/Page.pm +++ b/C4/VirtualShelves/Page.pm @@ -261,11 +261,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 @@ -281,8 +283,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 a255b33..f8d228a 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 = "\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 d3b34eb..a18fa8f 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,@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 8e9a23f..6666164 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