[Koha-patches] [PATCH] Cleanup opac-search.pl

[Joe Atzberger]

Use GetBranchesLoop.  Use elsif.  Use $format w/ a fallback "" value.

More to come.
---
 opac/opac-search.pl |   62 +++++++++++++++-----------------------------------
 1 files changed, 19 insertions(+), 43 deletions(-)

diff --git a/opac/opac-search.pl b/opac/opac-search.pl
index e1e66a9..9f291ec 100755
--- a/opac/opac-search.pl
+++ b/opac/opac-search.pl
@@ -37,8 +37,9 @@ my $template_name;
 my $template_type = 'basic';
 my @params = $cgi->param("limit");
 
+my $format = $cgi->param("format") || '';
 my $build_grouped_results = C4::Context->preference('OPACGroupResults');
-if ($cgi->param("format") && $cgi->param("format") =~ /(rss|atom|opensearchdescription)/) {
+if ($format =~ /(rss|atom|opensearchdescription)/) {
 	$template_name = 'opac-opensearch.tmpl';
 }
 elsif ($build_grouped_results) {
@@ -60,22 +61,16 @@ else {
     }
 );
 
-if ($cgi->param("format") && $cgi->param("format") eq 'rss2') {
-	$template->param("rss2" => 1);
-}
-elsif ($cgi->param("format") && $cgi->param("format") eq 'atom') {
-	$template->param("atom" => 1);
+if ($format eq 'rss2' or $format eq 'opensearchdescription' or $format eq 'atom') {
+	$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
-    $template->param(timestamp => strftime("%Y-%m-%dT%H:%M:%S-00:00", gmtime));
-}
-elsif ($cgi->param("format") && $cgi->param("format") eq 'opensearchdescription') {
-	$template->param("opensearchdescription" => 1);
 }
 if (C4::Context->preference("marcflavour") eq "UNIMARC" ) {
     $template->param('UNIMARC' => 1);
 }
-if (C4::Context->preference("marcflavour") eq "MARC21" ) {
+elsif (C4::Context->preference("marcflavour") eq "MARC21" ) {
     $template->param('usmarc' => 1);
 }
 
@@ -119,22 +114,12 @@ if (C4::Context->preference('TagsEnabled')) {
 #}
 
 # load the branches
-my $mybranch = ( C4::Context->preference( 'SearchMyLibraryFirst' ) && C4::Context->userenv ) ? C4::Context->userenv->{branch} : '';
-my $branches = GetBranches();
-# FIXME: next line duplicates GetBranchesLoop(0,0);
-my @branch_loop = map {
-                    {
-                        value => $_,
-                        branchname => $branches->{$_}->{branchname},
-                        selected => ( $mybranch eq $_ ) ? 1 : 0
-                    }
-                } sort {
-                    $branches->{$a}->{branchname} cmp $branches->{$b}->{branchname}
-                } keys %$branches;
-
-my $categories = GetBranchCategories(undef,'searchdomain');
-
-$template->param(branchloop => \@branch_loop, searchdomainloop => $categories);
+my $mybranch = ( C4::Context->preference('SearchMyLibraryFirst') && C4::Context->userenv && C4::Context->userenv->{branch} ) ? C4::Context->userenv->{branch} : '';
+my $branches = GetBranches();   # used later in *getRecords, probably should be internalized by those functions after caching in C4::Branch is established
+$template->param(
+    branchloop       => GetBranchesLoop($mybranch, 0),
+    searchdomainloop => GetBranchCategories(undef,'searchdomain'),
+);
 
 # load the Type stuff
 my $itemtypes = GetItemTypes;
@@ -147,7 +132,7 @@ 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++,
+        my %row =(  number=>$cnt++,
 				ccl => $itype_or_itemtype,
                 code => $thisitemtype,
                 selected => $selected,
@@ -155,10 +140,9 @@ if (!$advanced_search_types or $advanced_search_types eq 'itemtypes') {
                 count5 => $cnt % 4,
                 imageurl=> getitemtypeimagelocation( 'opac', $itemtypes->{$thisitemtype}->{'imageurl'} ),
             );
-    	$selected = 0 if ($selected) ;
+    	$selected = 0; # set to zero after first pass through
     	push @itemtypesloop, \%row;
 	}
-	$template->param(itemtypeloop => \@itemtypesloop);
 } else {
     my $advsearchtypes = GetAuthorisedValues($advanced_search_types);
 	for my $thisitemtype (@$advsearchtypes) {
@@ -173,8 +157,8 @@ if (!$advanced_search_types or $advanced_search_types eq 'itemtypes') {
             );
 		push @itemtypesloop, \%row;
 	}
-	$template->param(itemtypeloop => \@itemtypesloop);
 }
+$template->param(itemtypeloop => \@itemtypesloop);
 
 # # load the itypes (Called item types in the template -- just authorized values for searching)
 # my ($itypecount, at itype_loop) = GetCcodes();
@@ -199,7 +183,7 @@ if ( $template_type && $template_type eq 'advsearch' ) {
     # shouldn't appear on the first one, scan indexes should, adding a new
     # box should only appear on the last, etc.
     my @search_boxes_array;
-    my $search_boxes_count = C4::Context->preference("OPACAdvSearchInputCount") || 3; # FIXME: should be a syspref
+    my $search_boxes_count = C4::Context->preference("OPACAdvSearchInputCount") || 3;
     for (my $i=1;$i<=$search_boxes_count;$i++) {
         # if it's the first one, don't display boolean option, but show scan indexes
         if ($i==1) {
@@ -225,7 +209,7 @@ if ( $template_type && $template_type eq 'advsearch' ) {
         }
 
     }
-    $template->param(uc(C4::Context->preference("marcflavour")) => 1,
+    $template->param(uc(C4::Context->preference("marcflavour")) => 1,   # we already did this for UNIMARC
 					  advsearch => 1,
                       search_boxes_loop => \@search_boxes_array);
 
@@ -262,7 +246,7 @@ my $default_sort_by = C4::Context->preference('OPACdefaultSortField')."_".C4::Co
 @sort_by = split("\0",$params->{'sort_by'}) if $params->{'sort_by'};
 $sort_by[0] = $default_sort_by if !$sort_by[0] && defined($default_sort_by);
 foreach my $sort (@sort_by) {
-    $template->param($sort => 1);
+    $template->param($sort => 1);   # FIXME: security hole.  can set any TMPL_VAR here
 }
 $template->param('sort_by' => $sort_by[0]);
 
@@ -630,15 +614,7 @@ if (defined $barshelves) {
 	$template->param( addbarshelvesloop => $barshelves);
 }
 
-my $content_type;
-
-if ($cgi->param('format') && $cgi->param('format') =~ /rss/) {
-    $content_type = 'rss'
-} elsif ($cgi->param('format') && $cgi->param('format') =~ /atom/) {
-    $content_type = 'atom'
-} else {
-    $content_type = 'html'
-}
+my $content_type = ($format eq 'rss' or $format eq 'atom') ? $format : 'html';
 
 # If GoogleIndicTransliteration system preference is On Set paramter to load Google's javascript in OPAC search screens 
 if (C4::Context->preference('GoogleIndicTransliteration')) {
-- 
1.5.6.5