[Koha-patches] [PATCH] Cleanup selectbranchprinter.pl and .tmpl

Joe Atzberger joe.atzberger at liblime.com
Fri May 29 23:09:34 CEST 2009


Use GetBranchesLoop and warnings.  Remove unused code.  Moved the Get's to after auth.
I begin to show how this script will use referer to redirect back to the sending
page upon successful change of branch or printer (see hidden div and recycle_loop).

Indicator for singleBranchMode added.
Also added another helper function for GetBranchesLoop, namely mybranch.  This
encapsulates the logic for finding an intelligent default selection, which is
almost always desirable.

This does not resolve bug 2426, but is work in that direction.
---
 C4/Branch.pm                                       |   10 ++-
 circ/selectbranchprinter.pl                        |  118 +++++++-------------
 .../prog/en/modules/circ/selectbranchprinter.tmpl  |   67 +++++++----
 3 files changed, 92 insertions(+), 103 deletions(-)

diff --git a/C4/Branch.pm b/C4/Branch.pm
index 29b1a93..35424d7 100644
--- a/C4/Branch.pm
+++ b/C4/Branch.pm
@@ -45,7 +45,7 @@ BEGIN {
 		&DelBranch
 		&DelBranchCategory
 	);
-	@EXPORT_OK = qw( &onlymine );
+	@EXPORT_OK = qw( &onlymine &mybranch );
 }
 
 =head1 NAME
@@ -145,8 +145,14 @@ sub onlymine {
     C4::Context->userenv->{branch}                 ;
 }
 
+# always returns a string for OK comparison via "eq" or "ne"
+sub mybranch {
+    C4::Context->userenv           or return '';
+    return C4::Context->userenv->{branch} || '';
+}
+
 sub GetBranchesLoop (;$$) {  # since this is what most pages want anyway
-    my $branch   = @_ ? shift : '';     # optional first argument is branchcode of "my branch", if preselection is wanted.
+    my $branch   = @_ ? shift : mybranch();     # optional first argument is branchcode of "my branch", if preselection is wanted.
     my $onlymine = @_ ? shift : onlymine();
     my $branches = GetBranches($onlymine);
     my @loop;
diff --git a/circ/selectbranchprinter.pl b/circ/selectbranchprinter.pl
index cfb5831..c878742 100755
--- a/circ/selectbranchprinter.pl
+++ b/circ/selectbranchprinter.pl
@@ -18,101 +18,67 @@
 # Suite 330, Boston, MA  02111-1307 USA
 
 use strict;
-use CGI qw/:standard/;
+use warnings;
+use CGI;
 
 use C4::Context;
-use C4::Circulation;
 use C4::Output;
 use C4::Auth;
-use C4::Print;
+use C4::Print;  # GetPrinters
 use C4::Koha;
-use C4::Branch; # GetBranches
+use C4::Branch; # GetBranches GetBranchesLoop
 
-# this is a reorganisation of circulationold.pl
-# dividing it up into three scripts......
-# this will be the first one that chooses branch and printer settings....
+# this will be the script that chooses branch and printer settings....
 
-#general design stuff...
+my $query = CGI->new();
+my ( $template, $borrowernumber, $cookie ) = get_template_and_user({
+    template_name   => "circ/selectbranchprinter.tmpl",
+    query           => $query,
+    type            => "intranet",
+    debug           => 1,
+    authnotrequired => 0,
+    flagsrequired   => { circulate => "circulate_remaining_permissions" },
+});
 
-# try to get the branch and printer settings from the http....
-my $query    = new CGI;
+# try to get the branch and printer settings from http, fallback to userenv
 my $branches = GetBranches();
 my $printers = GetPrinters();
-my $branch   = $query->param('branch');
-my $printer  = $query->param('printer');
+my $branch   = $query->param('branch' ) || C4::Context->userenv->{'branch'}; 
+my $printer  = $query->param('printer') || C4::Context->userenv->{'branchprinter'};
 
-# set header with cookie....
-
-my ( $template, $borrowernumber, $cookie ) = get_template_and_user(
-    {
-        template_name   => "circ/selectbranchprinter.tmpl",
-        query           => $query,
-        type            => "intranet",
-        authnotrequired => 0,
-        flagsrequired   => { circulate => "circulate_remaining_permissions" },
-    }
-);
-
-
-($branch)  || ( $branch  = C4::Context->userenv->{'branch'} );
-($printer) || ( $printer = C4::Context->userenv->{'branchprinter'} );
-( $branches->{$branch} )  || ( $branch  = ( keys %$branches )[0] );
-( $printers->{$printer} ) || ( $printer = ( keys %$printers )[0] );
-
-# if you force a selection....
-my $oldbranch  = $branch;
-my $oldprinter = $printer;
-
-# set up select options....
-my $branchcount  = 0;
-my $printercount = 0;
-my @branchloop;
-for my $br (sort { $branches->{$a}->{branchname} cmp $branches->{$b}->{branchname} } keys %$branches) {
-    next unless $br =~ /\S/; # next unless $br is not blank.
+unless ($branches->{$branch}) {
+    $branch = (keys %$branches)[0];  # if branch didn't really exist, then replace it w/ one that does
+}
 
-    $branchcount++;
-    my %branch;
-    $branch{selected} = ( $br eq $oldbranch );
-    $branch{name}     = $branches->{$br}->{'branchname'};
-    $branch{value}    = $br;
-    push( @branchloop, \%branch );
+my @printkeys = sort keys %$printers;
+if (scalar(@printkeys) == 1 or not $printers->{$printer}) {
+    $printer = $printkeys[0];
 }
+
 my @printerloop;
-foreach ( keys %$printers ) {
-    (next) unless ($_); # next unless if this printer is blank.
-    $printercount++;
-    my %printer;
-    $printer{selected} = ( $_ eq $oldprinter );
-    $printer{name}     = $printers->{$_}->{'printername'};
-    $printer{value}    = $_;
-    push( @printerloop, \%printer );
+foreach ( @printkeys ) {
+    next unless ($_); # skip printer if blank.
+    push @printerloop, {
+        selected => ( $_ eq $printer ),
+        name     => $printers->{$_}->{'printername'},
+        value    => $_,
+    };
 }
 
-# if there is only one....
-my $printername;
-my $branchname;
-
-my $oneprinter = ( $printercount == 1 );
-my $onebranch  = ( $branchcount == 1 );
-if ( $printercount == 1 ) {
-    my ($tmpprinter) = keys %$printers;
-    $printername = $printers->{$tmpprinter}->{printername};
+my @recycle_loop;
+foreach ($query->param()) {
+    /^branch(printer)?$/ and next;  # disclude branch and branchprinter
+    push @recycle_loop, {
+        param => $_,
+        value => $query->param($_),
+    };
 }
-if ( $branchcount == 1 ) {
-    my ($tmpbranch) = keys %$branches;
-    $branchname = $branches->{$tmpbranch}->{branchname};
-}
-
-################################################################################
-# Start writing page....
 
 $template->param(
-    oneprinter              => $oneprinter,
-    onebranch               => $onebranch,
-    printername             => $printername,
-    branchname              => $branchname,
-    printerloop             => \@printerloop,
-    branchloop              => \@branchloop,
+    referer     => $ENV{HTTP_REFERER},
+    printerloop => \@printerloop,
+    branchloop  => GetBranchesLoop($branch),
+    recycle_loop=> \@recycle_loop,
 );
 
 output_html_with_http_headers $query, $cookie, $template->output;
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/selectbranchprinter.tmpl b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/selectbranchprinter.tmpl
index bc3326a..2298ba2 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/selectbranchprinter.tmpl
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/selectbranchprinter.tmpl
@@ -1,55 +1,72 @@
 <!-- TMPL_INCLUDE NAME="doc-head-open.inc" -->
 <title>Koha &rsaquo; Circulation &rsaquo; Set Library</title>
 <!-- TMPL_INCLUDE NAME="doc-head-close.inc" -->
+<style type="text/css">
+    .noshow {display: none;}
+</style>
 </head>
 <body>
 <!-- TMPL_INCLUDE NAME="header.inc" -->
 <!-- TMPL_INCLUDE NAME="circ-search.inc" -->
 
-<div id="breadcrumbs"><a href="/cgi-bin/koha/mainpage.pl">Home</a> &rsaquo; <a href="/cgi-bin/koha/circ/circulation-home.pl">Circulation</a> &rsaquo; Set Library</div>
+<div id="breadcrumbs">
+         <a href="/cgi-bin/koha/mainpage.pl">Home</a>
+&rsaquo; <a href="/cgi-bin/koha/circ/circulation-home.pl">Circulation</a>
+&rsaquo; <a href="/cgi-bin/koha/circ/selectbranchprinter.pl">Set Library</a>
+</div>
 
 <div id="doc" class="yui-t7">
-   
-   <div id="bd">
-	<div id="yui-main">
-	<div class="yui-b">
-
-
+  <div id="bd">
+    <div id="yui-main">
+      <div class="yui-b">
+<!-- NOTE this posts back to circulation to enact changes.  Should stay here. -->
 <form method="post" action="/cgi-bin/koha/circ/circulation.pl">
-<!-- TMPL_VAR NAME="branch" -->
-
+<!-- TMPL_IF NAME="singleBranchMode" -->
+    <br />Single Branch mode is ON.
+<!-- TMPL_ELSE -->
 <fieldset class="rows">
 	<legend>Set Library</legend>
 	<ol><li><label for="branch">Choose library:</label>
-<!--branchselection-->
 	<select name="branch" id="branch">
 	<!-- TMPL_LOOP Name="branchloop" -->
-		<!-- TMPL_IF NAME="selected" --><option value="<!-- TMPL_VAR NAME="value" -->" selected="selected"><!-- TMPL_VAR NAME="name" --></option>
-				<!-- TMPL_ELSE -->
-				<option value="<!-- TMPL_VAR NAME="value" -->"><!-- TMPL_VAR NAME="name" --></option>
-				<!-- /TMPL_IF -->
+		<!-- TMPL_IF NAME="selected" -->
+        <option value="<!-- TMPL_VAR NAME="value" -->" selected="selected"><!-- TMPL_VAR NAME="branchname" --></option>
+        <!-- TMPL_ELSE -->
+        <option value="<!-- TMPL_VAR NAME="value" -->"><!-- TMPL_VAR NAME="branchname" --></option>
+        <!-- /TMPL_IF -->
 	<!-- /TMPL_LOOP -->
 	</select></li>
-
+<!-- /TMPL_IF -->
 
 <!-- TMPL_IF Name="printerloop" -->
     <li><label for="printer">Choose a network printer:</label>
     <input type="hidden" name="setcookies" value="1" />
-    <!--printerselection-->
     <select name="printer" id="printer">
         <!-- TMPL_LOOP Name="printerloop" -->
-            <!-- TMPL_IF NAME="selected" --><option value="<!-- TMPL_VAR NAME="value" -->" selected="selected"><!-- TMPL_VAR NAME="name" --></option>
-				<!-- TMPL_ELSE -->
+            <!-- TMPL_IF NAME="selected" -->
+                <option value="<!-- TMPL_VAR NAME="value" -->" selected="selected"><!-- TMPL_VAR NAME="name" --></option>
+            <!-- TMPL_ELSE -->
 				<option value="<!-- TMPL_VAR NAME="value" -->"><!-- TMPL_VAR NAME="name" --></option>
-				<!-- /TMPL_IF -->
+            <!-- /TMPL_IF -->
         <!-- /TMPL_LOOP -->
         </select></li>
 <!-- /TMPL_IF --></ol>
-</fieldset>    <fieldset class="action"><input type="submit" value="Submit" name="changesettings" /></fieldset>
+</fieldset>
+<fieldset class="action"><input type="submit" value="Submit" name="changesettings" /></fieldset>
+<div class="noshow">
+    <!-- TMPL_LOOP Name="recycle_loop" -->
+    <input name="<!-- TMPL_VAR NAME="param" -->" value="<!-- TMPL_VAR NAME="value" ESCAPE='HTML' -->" />
+    <!-- /TMPL_LOOP -->
+    <input type="hidden" name="oldreferer" value="<!-- TMPL_VAR NAME='referer' ESCAPE='HTML' DEFAULT='/cgi-bin/koha/circ/circulation.pl' -->" />
+    <div>referer is:
+        <a href="<!-- TMPL_VAR NAME='referer' ESCAPE='HTML' DEFAULT='/cgi-bin/koha/circ/circulation.pl' -->">
+                 <!-- TMPL_VAR NAME='referer' ESCAPE='HTML' DEFAULT='/cgi-bin/koha/circ/circulation.pl' -->
+        </a>
+    </div>
+</div>
 </form>
 
-
-</div>
-</div>
-</div>
-<!-- TMPL_INCLUDE NAME="intranet-bottom.inc" -->
\ No newline at end of file
+      </div>
+    </div>
+  </div>
+<!-- TMPL_INCLUDE NAME="intranet-bottom.inc" -->
-- 
1.5.6.5




More information about the Koha-patches mailing list