Implemented fixes for bug 170

1. Circ2::returnbook will throw an exception if $branch is undef
2. branch/printer-getting code is modularized into getbranch and getprinter
   with correct logic from circulation.pl (temporarily put into Circ2.pm)
3. circulation.pl and returns.pl modified to use above functions

Noted correct tab sizes for the files; returns.pl partially reformatted to
conform to the dominant correct tab size

Handle non-latin1 charsets for returns.pl

C4/Circulation/Circ2.pm

@@ -1,3 +1,6 @@
+# -*- tab-width: 8 -*-
+# Please use 8-character tabs for this file (indents are every 4 characters)
+
 package C4::Circulation::Circ2;
 
 # $Id$
@@ -64,6 +67,7 @@ returns, as well as general information about the library.
 
 @ISA = qw(Exporter);
 @EXPORT = qw(&getbranches &getprinters &getpatroninformation
+	&getbranch &getprinter
 	&currentissues &getissues &getiteminformation &findborrower
 	&issuebook &returnbook &find_reserves &transferbook &decode
 	&calc_charges);
@@ -148,6 +152,25 @@ sub getprinters {
     return (\%printers);
 }
 
+# FIXME - This function doesn't feel as if it belongs here. It should
+# go in some generic or administrative module, not in circulation.
+sub getbranch ($$) {
+    my($query, $branches) = @_; # get branch for this query from branches
+    my $branch = $query->param('branch');
+    ($branch) || ($branch = $query->cookie('branch'));
+    ($branches->{$branch}) || ($branch=(keys %$branches)[0]);
+    return $branch;
+}
+
+# FIXME - Perhaps this really belongs in C4::Print?
+sub getprinter ($$) {
+    my($query, $printers) = @_; # get printer for this query from printers
+    my $printer = $query->param('printer');
+    ($printer) || ($printer = $query->cookie('printer'));
+    ($printers->{$printer}) || ($printer = (keys %$printers)[0]);
+    return $printer;
+}
+
 =item getpatroninformation
 
   ($borrower, $flags) = &getpatroninformation($env, $borrowernumber,
@@ -977,6 +1000,7 @@ sub returnbook {
     my %env;
     my $messages;
     my $doreturn = 1;
+    die '$branch not defined' unless defined $branch; # just in case (bug 170)
 # get information on item
     my ($iteminformation) = getiteminformation(\%env, 0, $barcode);
     if (not $iteminformation) {
@@ -1042,7 +1066,7 @@ sub returnbook {
 # Updates items.datelastseen for the item.
 # Not exported
 # FIXME - This is only used in &returnbook. Why make it into a
-# separate function?
+# separate function? (is this a recognizable step in the return process? - acli)
 sub doreturn {
     my ($brn, $itm) = @_;
     my $dbh = C4::Context->dbh;

circ/circulation.pl

@@ -1,4 +1,5 @@
 #!/usr/bin/perl
+# Please use 8-character tabs for this file (indents are every 4 characters)
 
 #written 8/5/2002 by Finlay
 #script to execute issuing of books
@@ -47,15 +48,8 @@ my $printers = getprinters(\%env);
 
 my $query = new CGI;
 
-my $branch = $query->param("branch");
-my $printer = $query->param("printer");
-
-($branch) || ($branch=$query->cookie('branch')) ;
-($printer) || ($printer=$query->cookie('printer')) ;
-
-($branches->{$branch}) || ($branch=(keys %$branches)[0]);
-($printers->{$printer}) || ($printer=(keys %$printers)[0]);
-
+my $branch = getbranch($query, $branches);
+my $printer = getprinter($query, $printers);
 
 
 #set up cookie.....
@@ -689,3 +683,7 @@ sub printslip {
     }
     remoteprint($env,\@issues,$borrower);
 }
+
+# Local Variables:
+# tab-width: 8
+# End:

circ/returns.pl

@@ -1,4 +1,5 @@
 #!/usr/bin/perl
+# Please use 8-character tabs for this file (indents are 4 spaces)
 
 #written 11/3/2002 by Finlay
 #script to execute returns of books
@@ -29,6 +30,7 @@ use C4::Output;
 use C4::Print;
 use C4::Reserves2;
 use C4::Auth;
+use C4::Interface::CGI::Output;
 use HTML::Template;
 
 
@@ -55,14 +57,8 @@ my $backgroundimage="/images/background-mem.gif";
 my $branches = getbranches();
 my $printers = getprinters(\%env);
 
-my $branch = $query->param("branch");
-my $printer = $query->param("printer");
-
-($branch) || ($branch=$query->cookie('branch')) ;
-($printer) || ($printer=$query->cookie('printer')) ;
-
-my $genbrname=$branches->{$branch}->{'branchname'};
-my $genprname=$printers->{$printer}->{'printer'};
+my $branch = getbranch($query, $branches);
+my $printer = getprinter($query, $printers);
 
 #
 # Some code to handle the error if there is no branch or printer setting.....
@@ -364,17 +360,17 @@ foreach (sort {$a <=> $b} keys %returneditems) {
 }
 $template->param(riloop => \@riloop);
 
-$template->param(	genbrname => $genbrname,
-								genprname => $genprname,
-								branch => $branch,
-								printer => $printer,
-								hdrbckgdcolor => $headerbackgroundcolor,
-								bckgdimg => $backgroundimage,
-								errmsgloop => \@errmsgloop
-							);
+$template->param(	genbrname => $branches->{$branch}->{'branchname'},
+			genprname => $printers->{$printer}->{'printername'},
+			branch => $branch,
+			printer => $printer,
+			hdrbckgdcolor => $headerbackgroundcolor,
+			bckgdimg => $backgroundimage,
+			errmsgloop => \@errmsgloop
+		);
 
 # actually print the page!
-print $query->header(), $template->output;
+output_html_with_http_headers $query, $cookie, $template->output;
 
 sub cuecatbarcodedecode {
     my ($barcode) = @_;
@@ -386,3 +382,7 @@ sub cuecatbarcodedecode {
     } else {
 	return $barcode;
     }}
+
+# Local Variables:
+# tab-width: 8
+# End: