Bugzilla – Attachment 7014 Details for
Bug 6374
Use "size" as names/hash keys leads to an unexpected results when using Template::Toolkit (name of a virtual method there)
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
proposed patch #2
0001-Bug-6374-default-value-for-Size-in-results.patch (text/plain), 21.02 KB, created by
Adrien SAURAT
on 2012-01-02 16:01:49 UTC
(
hide
)
Description:
proposed patch #2
Filename:
MIME Type:
Creator:
Adrien SAURAT
Created:
2012-01-02 16:01:49 UTC
Size:
21.02 KB
patch
obsolete
>From 199d7ce2ed343a3e6e36c46d40cc201c21d97146 Mon Sep 17 00:00:00 2001 >From: Adrien Saurat <adrien.saurat@biblibre.com> >Date: Mon, 2 Jan 2012 16:58:57 +0100 >Subject: [PATCH] Bug 6374: default value for Size in results > >In order to avoid the TT to display random numbers when >the size information is not provided, a default empty string >is sent if nothing else is available. >--- > catalogue/search.pl | 40 +++++---- > opac/opac-search.pl | 262 ++++++++++++++++++++++++++------------------------- > 2 files changed, 155 insertions(+), 147 deletions(-) > >diff --git a/catalogue/search.pl b/catalogue/search.pl >index 3013320..86ad2b7 100755 >--- a/catalogue/search.pl >+++ b/catalogue/search.pl >@@ -440,18 +440,18 @@ if ($params->{'limit-yr'}) { > # $ %z3950p will be a hash ref if the indexes are present (advacned search), otherwise undef > my $z3950par; > my $indexes2z3950 = { >- kw=>'title', au=>'author', 'au,phr'=>'author', nb=>'isbn', ns=>'issn', >- 'lcn,phr'=>'dewey', su=>'subject', 'su,phr'=>'subject', >- ti=>'title', 'ti,phr'=>'title', se=>'title' >+ kw=>'title', au=>'author', 'au,phr'=>'author', nb=>'isbn', ns=>'issn', >+ 'lcn,phr'=>'dewey', su=>'subject', 'su,phr'=>'subject', >+ ti=>'title', 'ti,phr'=>'title', se=>'title' > }; > for (my $ii = 0; $ii < @operands; ++$ii) > { >- my $name = $indexes2z3950->{$indexes[$ii]}; >- if (defined $name && defined $operands[$ii]) >- { >- $z3950par ||= {}; >- $z3950par->{$name} = $operands[$ii] if !exists $z3950par->{$name}; >- } >+ my $name = $indexes2z3950->{$indexes[$ii]}; >+ if (defined $name && defined $operands[$ii]) >+ { >+ $z3950par ||= {}; >+ $z3950par->{$name} = $operands[$ii] if !exists $z3950par->{$name}; >+ } > } > > >@@ -484,9 +484,9 @@ for my $this_cgi ( split('&',$query_cgi) ) { > my $input_name = $1; > my $input_value = $2; > push @query_inputs, { input_name => $input_name, input_value => $input_value }; >- if ($input_name eq 'idx') { >- $scan_index_to_use = $input_value; # unless $scan_index_to_use; >- } >+ if ($input_name eq 'idx') { >+ $scan_index_to_use = $input_value; # unless $scan_index_to_use; >+ } > } > $template->param ( QUERY_INPUTS => \@query_inputs, > scan_index_to_use => $scan_index_to_use ); >@@ -552,8 +552,8 @@ for (my $i=0;$i<@servers;$i++) { > ## If there's just one result, redirect to the detail page > if ($total == 1) { > my $biblionumber = $newresults[0]->{biblionumber}; >- my $defaultview = C4::Context->preference('IntranetBiblioDefaultView'); >- my $views = { C4::Search::enabled_staff_search_views }; >+ my $defaultview = C4::Context->preference('IntranetBiblioDefaultView'); >+ my $views = { C4::Search::enabled_staff_search_views }; > if ($defaultview eq 'isbd' && $views->{can_view_ISBD}) { > print $cgi->redirect("/cgi-bin/koha/catalogue/ISBDdetail.pl?biblionumber=$biblionumber"); > } elsif ($defaultview eq 'marc' && $views->{can_view_MARC}) { >@@ -578,14 +578,18 @@ for (my $i=0;$i<@servers;$i++) { > $template->param(limit_desc => $limit_desc); > $template->param(offset => $offset); > $template->param(DisplayMultiPlaceHold => $DisplayMultiPlaceHold); >- $template->param (z3950_search_params => C4::Search::z3950_search_args($query_desc)); >+ $template->param (z3950_search_params => C4::Search::z3950_search_args($query_desc)); > if ($query_desc || $limit_desc) { > $template->param(searchdesc => 1); > } > $template->param(stopwords_removed => "@$stopwords_removed") if $stopwords_removed; > $template->param(results_per_page => $results_per_page); >+ # must define a value for size if not present in DB >+ # in order to avoid problems generated by the default size value in TT >+ foreach my $line (@newresults) { >+ if ( not exists $line->{'size'} ) { $line->{'size'} = "" } >+ } > $template->param(SEARCH_RESULTS => \@newresults); >- > ## FIXME: add a global function for this, it's better than the current global one > ## Build the page numbers on the bottom of the page > my @page_numbers; >@@ -649,7 +653,7 @@ for (my $i=0;$i<@servers;$i++) { > # no hits > else { > $template->param(searchdesc => 1,query_desc => $query_desc,limit_desc => $limit_desc); >- $template->param (z3950_search_params => C4::Search::z3950_search_args($z3950par || $query_desc)); >+ $template->param (z3950_search_params => C4::Search::z3950_search_args($z3950par || $query_desc)); > } > > } # end of the if local >@@ -679,7 +683,7 @@ $template->param( > total => $total, > opacfacets => 1, > facets_loop => $facets, >- displayFacetCount=> C4::Context->preference('displayFacetCount')||0, >+ displayFacetCount=> C4::Context->preference('displayFacetCount')||0, > scan => $scan, > search_error => $error, > ); >diff --git a/opac/opac-search.pl b/opac/opac-search.pl >index aba23a8..4a2a493 100755 >--- a/opac/opac-search.pl >+++ b/opac/opac-search.pl >@@ -48,10 +48,10 @@ use CGI qw('-no_undef_params'); > my $cgi = new CGI; > > BEGIN { >- if (C4::Context->preference('BakerTaylorEnabled')) { >- require C4::External::BakerTaylor; >- import C4::External::BakerTaylor qw(&image_url &link_url); >- } >+ if (C4::Context->preference('BakerTaylorEnabled')) { >+ require C4::External::BakerTaylor; >+ import C4::External::BakerTaylor qw(&image_url &link_url); >+ } > } > > my ($template,$borrowernumber,$cookie); >@@ -64,13 +64,13 @@ my @params = $cgi->param("limit"); > my $format = $cgi->param("format") || ''; > my $build_grouped_results = C4::Context->preference('OPACGroupResults'); > if ($format =~ /(rss|atom|opensearchdescription)/) { >- $template_name = 'opac-opensearch.tmpl'; >+ $template_name = 'opac-opensearch.tmpl'; > } > elsif (@params && $build_grouped_results) { > $template_name = 'opac-results-grouped.tmpl'; > } > elsif ((@params>=1) || ($cgi->param("q")) || ($cgi->param('multibranchlimit')) || ($cgi->param('limit-yr')) ) { >- $template_name = 'opac-results.tmpl'; >+ $template_name = 'opac-results.tmpl'; > } > else { > $template_name = 'opac-advsearch.tmpl'; >@@ -89,7 +89,7 @@ if ($template_name eq 'opac-results.tmpl') { > } > > if ($format eq 'rss2' or $format eq 'opensearchdescription' or $format eq 'atom') { >- $template->param($format => 1); >+ $template->param($format => 1); > $template->param(timestamp => strftime("%Y-%m-%dT%H:%M:%S-00:00", gmtime)) if ($format eq 'atom'); > # FIXME - the timestamp is a hack - the biblio update timestamp should be used for each > # entry, but not sure if that's worth an extra database query for each bib >@@ -104,18 +104,18 @@ $template->param( 'AllowOnShelfHolds' => C4::Context->preference('AllowOnShelfHo > $template->param( 'OPACNoResultsFound' => C4::Context->preference('OPACNoResultsFound') ); > > if (C4::Context->preference('BakerTaylorEnabled')) { >- $template->param( >- BakerTaylorEnabled => 1, >- BakerTaylorImageURL => &image_url(), >- BakerTaylorLinkURL => &link_url(), >- BakerTaylorBookstoreURL => C4::Context->preference('BakerTaylorBookstoreURL'), >- ); >+ $template->param( >+ BakerTaylorEnabled => 1, >+ BakerTaylorImageURL => &image_url(), >+ BakerTaylorLinkURL => &link_url(), >+ BakerTaylorBookstoreURL => C4::Context->preference('BakerTaylorBookstoreURL'), >+ ); > } > if (C4::Context->preference('TagsEnabled')) { >- $template->param(TagsEnabled => 1); >- foreach (qw(TagsShowOnList TagsInputOnList)) { >- C4::Context->preference($_) and $template->param($_ => 1); >- } >+ $template->param(TagsEnabled => 1); >+ foreach (qw(TagsShowOnList TagsInputOnList)) { >+ C4::Context->preference($_) and $template->param($_ => 1); >+ } > } > > ## URI Re-Writing >@@ -163,32 +163,32 @@ my $cnt; > my $advanced_search_types = C4::Context->preference("AdvancedSearchTypes"); > > if (!$advanced_search_types or $advanced_search_types eq 'itemtypes') { >- foreach my $thisitemtype ( sort {$itemtypes->{$a}->{'description'} cmp $itemtypes->{$b}->{'description'} } keys %$itemtypes ) { >- my %row =( number=>$cnt++, >- ccl => "$itype_or_itemtype,phr", >+ foreach my $thisitemtype ( sort {$itemtypes->{$a}->{'description'} cmp $itemtypes->{$b}->{'description'} } keys %$itemtypes ) { >+ my %row =( number=>$cnt++, >+ ccl => "$itype_or_itemtype,phr", > code => $thisitemtype, > selected => $selected, > description => $itemtypes->{$thisitemtype}->{'description'}, > count5 => $cnt % 4, > imageurl=> getitemtypeimagelocation( 'opac', $itemtypes->{$thisitemtype}->{'imageurl'} ), > ); >- $selected = 0; # set to zero after first pass through >- push @itemtypesloop, \%row; >- } >+ $selected = 0; # set to zero after first pass through >+ push @itemtypesloop, \%row; >+ } > } else { > my $advsearchtypes = GetAuthorisedValues($advanced_search_types, '', 'opac'); >- for my $thisitemtype (@$advsearchtypes) { >- my %row =( >- number=>$cnt++, >- ccl => $advanced_search_types, >+ for my $thisitemtype (@$advsearchtypes) { >+ my %row =( >+ number=>$cnt++, >+ ccl => $advanced_search_types, > code => $thisitemtype->{authorised_value}, > selected => $selected, > description => $thisitemtype->{'lib'}, > count5 => $cnt % 4, > imageurl=> getitemtypeimagelocation( 'opac', $thisitemtype->{'imageurl'} ), > ); >- push @itemtypesloop, \%row; >- } >+ push @itemtypesloop, \%row; >+ } > } > $template->param(itemtypeloop => \@itemtypesloop); > >@@ -244,20 +244,20 @@ if ( $template_type && $template_type eq 'advsearch' ) { > } > > } >- $template->param(uc(C4::Context->preference("marcflavour")) => 1, # we already did this for UNIMARC >- advsearch => 1, >- search_boxes_loop => \@search_boxes_array); >- >-# use the global setting by default >- if ( C4::Context->preference("expandedSearchOption") == 1 ) { >- $template->param( expanded_options => C4::Context->preference("expandedSearchOption") ); >- } >- # but let the user override it >- if (defined $cgi->param('expanded_options')) { >- if ( ($cgi->param('expanded_options') == 0) || ($cgi->param('expanded_options') == 1 ) ) { >- $template->param( expanded_options => $cgi->param('expanded_options')); >- } >+ $template->param(uc( C4::Context->preference("marcflavour")) => 1, # we already did this for UNIMARC >+ advsearch => 1, >+ search_boxes_loop => \@search_boxes_array); >+ >+ # use the global setting by default >+ if ( C4::Context->preference("expandedSearchOption") == 1 ) { >+ $template->param( expanded_options => C4::Context->preference("expandedSearchOption") ); >+ } >+ # but let the user override it >+ if (defined $cgi->param('expanded_options')) { >+ if ( ($cgi->param('expanded_options') == 0) || ($cgi->param('expanded_options') == 1 ) ) { >+ $template->param( expanded_options => $cgi->param('expanded_options')); > } >+ } > output_html_with_http_headers $cgi, $cookie, $template->output; > exit; > } >@@ -284,8 +284,6 @@ for (keys %$params) { > $j++; > } > } >-# >- > > # Params that can have more than one value > # sort by is used to sort the query >@@ -430,15 +428,15 @@ my $results_hashref; > my @coins; > > if ($tag) { >- $query_cgi = "tag=" .$tag . "&" . $query_cgi; >- my $taglist = get_tags({term=>$tag, approved=>1}); >- $results_hashref->{biblioserver}->{hits} = scalar (@$taglist); >- my @biblist = (map {GetBiblioData($_->{biblionumber})} @$taglist); >- my @marclist = (map {$_->{marc}} @biblist ); >- $DEBUG and printf STDERR "taglist (%s biblionumber)\nmarclist (%s records)\n", scalar(@$taglist), scalar(@marclist); >- $results_hashref->{biblioserver}->{RECORDS} = \@marclist; >- # FIXME: tag search and standard search should work together, not exclusively >- # FIXME: No facets for tags search. >+ $query_cgi = "tag=" .$tag . "&" . $query_cgi; >+ my $taglist = get_tags({term=>$tag, approved=>1}); >+ $results_hashref->{biblioserver}->{hits} = scalar (@$taglist); >+ my @biblist = (map {GetBiblioData($_->{biblionumber})} @$taglist); >+ my @marclist = (map {$_->{marc}} @biblist ); >+ $DEBUG and printf STDERR "taglist (%s biblionumber)\nmarclist (%s records)\n", scalar(@$taglist), scalar(@marclist); >+ $results_hashref->{biblioserver}->{RECORDS} = \@marclist; >+ # FIXME: tag search and standard search should work together, not exclusively >+ # FIXME: No facets for tags search. > } > elsif (C4::Context->preference('NoZebra')) { > eval { >@@ -494,76 +492,82 @@ for (my $i=0;$i<@servers;$i++) { > @newresults = searchResults('opac', $query_desc, $hits, $results_per_page, $offset, $scan, > @{$results_hashref->{$server}->{"RECORDS"}},, C4::Context->preference('hidelostitems')); > } >- my $tag_quantity; >- if (C4::Context->preference('TagsEnabled') and >- $tag_quantity = C4::Context->preference('TagsShowOnList')) { >- foreach (@newresults) { >- my $bibnum = $_->{biblionumber} or next; >- $_->{itemsissued} = CountItemsIssued( $bibnum ); >- $_ ->{'TagLoop'} = get_tags({biblionumber=>$bibnum, approved=>1, 'sort'=>'-weight', >- limit=>$tag_quantity }); >- } >- } >- if (C4::Context->preference('COinSinOPACResults')) { >- foreach (@newresults) { >- my $record = GetMarcBiblio($_->{'biblionumber'}); >- $_->{coins} = GetCOinSBiblio($record); >- } >- } >+ >+ # must define a value for size if not present in DB >+ # in order to avoid problems generated by the default size value in TT >+ foreach my $line (@newresults) { >+ if ( not exists $line->{'size'} ) { $line->{'size'} = "" } >+ } >+ >+ my $tag_quantity; >+ if (C4::Context->preference('TagsEnabled') and >+ $tag_quantity = C4::Context->preference('TagsShowOnList')) { >+ foreach (@newresults) { >+ my $bibnum = $_->{biblionumber} or next; >+ $_->{itemsissued} = CountItemsIssued( $bibnum ); >+ $_ ->{'TagLoop'} = get_tags({biblionumber=>$bibnum, approved=>1, 'sort'=>'-weight', >+ limit=>$tag_quantity }); >+ } >+ } >+ if (C4::Context->preference('COinSinOPACResults')) { >+ foreach (@newresults) { >+ my $record = GetMarcBiblio($_->{'biblionumber'}); >+ $_->{coins} = GetCOinSBiblio($record); >+ } >+ } > >- if ($results_hashref->{$server}->{"hits"}){ >- $total = $total + $results_hashref->{$server}->{"hits"}; >- } >- # Opac search history >- my $newsearchcookie; >- if (C4::Context->preference('EnableOpacSearchHistory')) { >- my @recentSearches; >- >- # Getting the (maybe) already sent cookie >- my $searchcookie = $cgi->cookie('KohaOpacRecentSearches'); >- if ($searchcookie){ >- $searchcookie = uri_unescape($searchcookie); >- if (thaw($searchcookie)) { >- @recentSearches = @{thaw($searchcookie)}; >- } >- } >- >- # Adding the new search if needed >- if (!$borrowernumber || $borrowernumber eq '') { >- # To a cookie (the user is not logged in) >- >- if (($params->{'offset'}||'') eq '') { >- >- push @recentSearches, { >- "query_desc" => $query_desc || "unknown", >- "query_cgi" => $query_cgi || "unknown", >- "time" => time(), >- "total" => $total >- }; >- $template->param(ShowOpacRecentSearchLink => 1); >- } >- >- shift @recentSearches if (@recentSearches > 15); >- # Pushing the cookie back >- $newsearchcookie = $cgi->cookie( >- -name => 'KohaOpacRecentSearches', >- # We uri_escape the whole freezed structure so we're sure we won't have any encoding problems >- -value => uri_escape(freeze(\@recentSearches)), >- -expires => '' >- ); >- $cookie = [$cookie, $newsearchcookie]; >- } >- else { >- # To the session (the user is logged in) >- if (($params->{'offset'}||'') eq '') { >- AddSearchHistory($borrowernumber, $cgi->cookie("CGISESSID"), $query_desc, $query_cgi, $total); >- $template->param(ShowOpacRecentSearchLink => 1); >- } >- } >- } >- ## If there's just one result, redirect to the detail page >+ if ($results_hashref->{$server}->{"hits"}){ >+ $total = $total + $results_hashref->{$server}->{"hits"}; >+ } >+ >+ # Opac search history >+ my $newsearchcookie; >+ if (C4::Context->preference('EnableOpacSearchHistory')) { >+ my @recentSearches; >+ >+ # Getting the (maybe) already sent cookie >+ my $searchcookie = $cgi->cookie('KohaOpacRecentSearches'); >+ if ($searchcookie){ >+ $searchcookie = uri_unescape($searchcookie); >+ if (thaw($searchcookie)) { >+ @recentSearches = @{thaw($searchcookie)}; >+ } >+ } >+ >+ # Adding the new search if needed >+ if (!$borrowernumber || $borrowernumber eq '') { >+ # To a cookie (the user is not logged in) >+ if (($params->{'offset'}||'') eq '') { >+ push @recentSearches, { >+ "query_desc" => $query_desc || "unknown", >+ "query_cgi" => $query_cgi || "unknown", >+ "time" => time(), >+ "total" => $total >+ }; >+ $template->param(ShowOpacRecentSearchLink => 1); >+ } >+ >+ shift @recentSearches if (@recentSearches > 15); >+ # Pushing the cookie back >+ $newsearchcookie = $cgi->cookie( >+ -name => 'KohaOpacRecentSearches', >+ # We uri_escape the whole freezed structure so we're sure we won't have any encoding problems >+ -value => uri_escape(freeze(\@recentSearches)), >+ -expires => '' >+ ); >+ $cookie = [$cookie, $newsearchcookie]; >+ } >+ else { >+ # To the session (the user is logged in) >+ if (($params->{'offset'}||'') eq '') { >+ AddSearchHistory($borrowernumber, $cgi->cookie("CGISESSID"), $query_desc, $query_cgi, $total); >+ $template->param(ShowOpacRecentSearchLink => 1); >+ } >+ } >+ } >+ ## If there's just one result, redirect to the detail page > if ($total == 1 && $format ne 'rss2' >- && $format ne 'opensearchdescription' && $format ne 'atom') { >+ && $format ne 'opensearchdescription' && $format ne 'atom') { > my $biblionumber=$newresults[0]->{biblionumber}; > if (C4::Context->preference('BiblioDefaultView') eq 'isbd') { > print $cgi->redirect("/cgi-bin/koha/opac-ISBDdetail.pl?biblionumber=$biblionumber"); >@@ -668,7 +672,7 @@ for (my $i=0;$i<@servers;$i++) { > $template->param( PAGE_NUMBERS => \@page_numbers, > previous_page_offset => $previous_page_offset) unless $pages < 2; > $template->param(next_page_offset => $next_page_offset) unless $pages eq $current_page_number; >- } >+ } > # no hits > else { > $template->param(searchdesc => 1,query_desc => $query_desc,limit_desc => $limit_desc); >@@ -699,7 +703,7 @@ $template->param( > total => $total, > opacfacets => 1, > facets_loop => $facets, >- displayFacetCount=> C4::Context->preference('displayFacetCount')||0, >+ displayFacetCount=> C4::Context->preference('displayFacetCount')||0, > scan => $scan, > search_error => $error, > ); >@@ -715,18 +719,18 @@ my @addpubshelves; > my $pubshelves = $session->param('pubshelves'); > my $barshelves = $session->param('barshelves'); > foreach my $shelf (@$pubshelves) { >- next if ( ($shelf->{'owner'} != ($borrowernumber ? $borrowernumber : -1)) && ($shelf->{'category'} < 3) ); >- push (@addpubshelves, $shelf); >+ next if ( ($shelf->{'owner'} != ($borrowernumber ? $borrowernumber : -1)) && ($shelf->{'category'} < 3) ); >+ push (@addpubshelves, $shelf); > } > > if (@addpubshelves) { >- $template->param( addpubshelves => scalar (@addpubshelves)); >- $template->param( addpubshelvesloop => \@addpubshelves); >+ $template->param( addpubshelves => scalar (@addpubshelves)); >+ $template->param( addpubshelvesloop => \@addpubshelves); > } > > if (defined $barshelves) { >- $template->param( addbarshelves => scalar (@$barshelves)); >- $template->param( addbarshelvesloop => $barshelves); >+ $template->param( addbarshelves => scalar (@$barshelves)); >+ $template->param( addbarshelvesloop => $barshelves); > } > > my $content_type = ($format eq 'rss' or $format eq 'atom') ? $format : 'html'; >-- >1.7.4.1 >
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=7014">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 6374
:
4191
|
6721
|
7014
|
7016
|
7027
|
7028