From f4ec623939a66f34c8f77e76b9b8e6d91cdacbc4 Mon Sep 17 00:00:00 2001
From: Joe Atzberger <joe.atzberger@liblime.com>
Date: Fri, 12 Jun 2009 09:25:47 -0500
Subject: [PATCH] Cleanup - admin scripts

Fixed useless redirect statements that weren't even printed.
Removed unused javascript and script variables;
Removed sth->finish.  Use get instead of post on "cancel" button (no data to post).
Reduce duplicative variables (e.g. scriptname and script_name).

Many other problems are still in the scripts, including use of META-REFRESH instead of
redirect, and the whole idea of redirecting back to the same page for no valid reason.
We should be able to formulate the right response on THIS pass, not ask the browser to start over.

Signed-off-by: Galen Charlton <galen.charlton@liblime.com>
---
 admin/aqbookfund.pl                           | 175 +++++++-----------
 admin/aqbudget.pl                             |  51 ++---
 admin/auth_subfields_structure.pl             |  60 +++---
 admin/auth_tag_structure.pl                   | 130 ++++++-------
 .../prog/en/modules/admin/aqbookfund.tmpl     | 169 +++++------------
 5 files changed, 208 insertions(+), 377 deletions(-)

diff --git a/admin/aqbookfund.pl b/admin/aqbookfund.pl
index 952e068ddc..604ae08c1b 100755
--- a/admin/aqbookfund.pl
+++ b/admin/aqbookfund.pl
@@ -1,6 +1,6 @@
 #!/usr/bin/perl
 
-#written 20/02/2002 by paul.poulain@free.fr
+# written 20/02/2002 by paul.poulain@free.fr
 
 # Copyright 2000-2002 Katipo Communications
 #
@@ -43,14 +43,13 @@ C<op> can be equal to:
 	- builds the add/modify form
 * add_validate, then
 	- the user has just send datas, so we create/modify the record
-* delete_form, then
-	- we show the record having primkey=$primkey and ask for deletion validation form
 * delete_confirm, then
     - we delete the record having primkey=$primkey
 
 =cut
 
 use strict;
+# use warnings; FIXME
 use CGI;
 use List::Util qw/min/;
 use C4::Branch; # GetBranches
@@ -62,38 +61,30 @@ use C4::Output;
 use C4::Dates;
 use C4::Debug;
 
-# use Smart::Comments;
-
-my $dbh = C4::Context->dbh;
-my $input = new CGI;
-my $script_name="/cgi-bin/koha/admin/aqbookfund.pl";
-my $bookfundid=$input->param('bookfundid');
-my $branchcodeid=$input->param('branchcode')|'';
-my $pagesize = 10;
-my $op = $input->param('op') || '';
-
-my ($template, $borrowernumber, $cookie)
-    = get_template_and_user(
-        {template_name => "admin/aqbookfund.tmpl",
-         query => $input,
-         type => "intranet",
-         authnotrequired => 0,
-         flagsrequired => {parameters => 1},
-         debug => 1,
-        }
-    );
+my $input        = new CGI;
+my $script_name  = "/cgi-bin/koha/admin/aqbookfund.pl";
+my $bookfundid   = $input->param('bookfundid');
+my $branchcodeid = $input->param('branchcode') || '';
+my $op           = $input->param('op')         || '';
+my $pagesize     = 10;
+
+$bookfundid = uc $bookfundid if $bookfundid;
+
+my ($template, $borrowernumber, $cookie) = get_template_and_user(
+    {   template_name   => "admin/aqbookfund.tmpl",
+        query           => $input,
+        type            => "intranet",
+        authnotrequired => 0,
+        flagsrequired   => { parameters => 1 },
+        debug           => 1,
+    }
+);
 
-if ($op) {
-    $template->param(
-        script_name => $script_name,
-        $op => 1,
-    ); # we show only the TMPL_VAR names $op
-}
-else {
-    $template->param(script_name => $script_name,
-		else              => 1); # we show only the TMPL_VAR names $op
-}
-$template->param(action => $script_name);
+$template->param(
+    action        => $script_name,
+    script_name   => $script_name,
+    ($op||'else') => 1,
+);
 
 my $branches = GetBranches;
 
@@ -102,73 +93,55 @@ my $branches = GetBranches;
 if ($op eq 'add_form') {
 	#---- if primkey exists, it's a modify action, so read values to modify...
 	my $dataaqbookfund;
-	my $header;
-	if ($bookfundid) {
-    	$dataaqbookfund = GetBookFund($bookfundid,$branchcodeid);
-	}
 	if ($bookfundid) {
-	    $header = "Modify book fund";
+        $dataaqbookfund = GetBookFund($bookfundid, $branchcodeid);
 	    $template->param('header-is-modify-p' => 1);
-	    $template->param('current_branch' =>  $branchcodeid);
+	    $template->param('current_branch' => $branchcodeid);
 	} else {
-	    $header = "Add book fund";
 	    $template->param('header-is-add-p' => 1);
 	}
-	$template->param('use-header-flags-p' => 1);
-	$template->param(header => $header); # NOTE deprecated
-	my $add_or_modify=0;
-	if ($bookfundid) {
-	    $add_or_modify=1;
-	}
-	$template->param(add_or_modify => $add_or_modify);
-	$template->param(bookfundid =>$bookfundid);
-	$template->param(bookfundname =>$dataaqbookfund->{'bookfundname'});
-
-        my @branchloop;
-        foreach my $branchcode (sort keys %{$branches}) {
-            my $row = {
-                branchcode => $branchcode,
-                branchname => $branches->{$branchcode}->{branchname},
-            };
-
-            if (defined $bookfundid
-                and defined $dataaqbookfund->{branchcode}
-                and $dataaqbookfund->{branchcode} eq $branchcode) {
-                $row->{selected} = 1;
-            }
-
-            push @branchloop, $row;
-        }
+	$template->param(
+        'use-header-flags-p' => 1,
+        add_or_modify => $bookfundid ? 1 : 0,
+        bookfundid    => $bookfundid,
+        bookfundname  => $dataaqbookfund->{'bookfundname'}
+    );
+
+    my @branchloop;
+    foreach my $branchcode (sort keys %{$branches}) {
+        push @branchloop, {
+            branchcode => $branchcode,
+            branchname => $branches->{$branchcode}->{branchname},
+            selected   => (defined $bookfundid and defined $dataaqbookfund->{branchcode}
+                            and $dataaqbookfund->{branchcode} eq $branchcode) ? 1 : 0,
+        };
+    }
 
-        $template->param(branches => \@branchloop);
+    $template->param(branches => \@branchloop);
 
 } # END $OP eq ADD_FORM
 
 #-----############# ADD_VALIDATE ##################################
 # called by add_form, used to insert/modify data in DB
 elsif ($op eq 'add_validate') {
-### add
-	my $bookfundid = uc $input->param('bookfundid');
-        my $bookfundname = $input->param('bookfundname');
-        my $branchcode = $input->param('branchcode') || undef;
-
+    my $bookfundname = $input->param('bookfundname');
+    my $branchcode   = $input->param('branchcode') || undef;
     my $number = Countbookfund($bookfundid,$branchcodeid);
     if ($number == 0 ) {
-
         NewBookFund(
             $bookfundid,
             $input->param('bookfundname'),
             $input->param('branchcode')||''
         );
     }
-    $input->redirect('aqbookfund.pl');
+    print $input->redirect('aqbookfund.pl');    # FIXME: unnecessary redirect
+    exit;
 # END $OP eq ADD_VALIDATE
 }
 
 #-----############# MOD_VALIDATE ##################################
 # called by add_form, used to insert/modify data in DB
 elsif ($op eq 'mod_validate') {
-	my $bookfundid  = uc $input->param('bookfundid');
 	my $bookfundname   = $input->param('bookfundname');
 	my $branchcode     = $input->param('branchcode'    ) || undef;
 	my $current_branch = $input->param('current_branch') || undef;
@@ -176,34 +149,27 @@ elsif ($op eq 'mod_validate') {
 
 	my $number = Countbookfund($bookfundid,$branchcodeid);
     if ($number < 2)  {
-         $debug and warn "name :$bookfundname branch:$branchcode";
+        $debug and warn "name :$bookfundname branch:$branchcode";
         ModBookFund($bookfundname,$bookfundid,$current_branch, $branchcode);
     }
-   $input->redirect('aqbookfund.pl');
+   print $input->redirect('aqbookfund.pl'); # FIXME: unnecessary redirect
+   exit;
 }
 
 #-----############# DELETE_CONFIRM ##################################
 # called by default form, used to confirm deletion of data in DB
 elsif ($op eq 'delete_confirm') {
     my $data = GetBookFund($bookfundid,$branchcodeid);
-	$template->param(bookfundid => $bookfundid);
+	$template->param(bookfundid   => $bookfundid);
 	$template->param(bookfundname => $data->{'bookfundname'});
-	$template->param(branchcode => $data->{'branchcode'});
-} # END $OP eq DELETE_CONFIRM
-
-#-----############# DELETE_CONFIRMED ##################################
+	$template->param(branchcode   => $data->{'branchcode'});
+}
 # called by delete_confirm, used to effectively confirm deletion of data in DB
 elsif ($op eq 'delete_confirmed') {
-    DelBookFund(uc($input->param('bookfundid')),$branchcodeid);
-
-}# END $OP eq DELETE_CONFIRMED
-
-#-----############# DEFAULT ##################################
+    DelBookFund($bookfundid, $branchcodeid);
+}
 else { # DEFAULT
-    my ($query, $sth);
-
-    $template->param(scriptname => $script_name);
-
+    my ($sth);
     # filters
     my @branchloop;
     foreach my $branchcode (sort keys %{$branches}) {
@@ -225,12 +191,12 @@ else { # DEFAULT
         if (defined $input->param('filter_bookfundid') and $input->param('filter_bookfundid') eq $row->{bookfundid}){
             $row->{selected} = 1;
         }
-         push @bookfundids_loop, $row;
-     }
+        push @bookfundids_loop, $row;
+    }
 
     $template->param(
-        filter_bookfundids => \@bookfundids_loop,
-        filter_branches => \@branchloop,
+        filter_bookfundids  => \@bookfundids_loop,
+        filter_branches     => \@branchloop,
         filter_bookfundname => $input->param('filter_bookfundname') || undef,
     );
 
@@ -269,15 +235,11 @@ else { # DEFAULT
     );
 
     foreach my $result (@results[$first .. $last]) {
-        push(
-            @loop,
-            {
-                %{$result},
-                branchname =>
-                    $branches->{ $result->{branchcode} }->{branchname},
-                has_budgets => defined $nb_budgets_of{ $result->{bookfundid} },
-            }
-        );
+        push @loop, {
+            %{$result},
+            branchname  => $branches->{ $result->{branchcode} }->{branchname},
+            has_budgets => defined $nb_budgets_of{ $result->{bookfundid} },
+        };
     }
 
     $template->param(
@@ -288,6 +250,7 @@ else { # DEFAULT
                         $page,
                         'page'
             )
-        );
-} #---- END $OP eq DEFAULT
+    );
+}
+
 output_html_with_http_headers $input, $cookie, $template->output;
diff --git a/admin/aqbudget.pl b/admin/aqbudget.pl
index fb4d168dbf..33edc1cd76 100755
--- a/admin/aqbudget.pl
+++ b/admin/aqbudget.pl
@@ -38,6 +38,7 @@
 # Suite 330, Boston, MA  02111-1307 USA
 
 use strict;
+# use warnings; FIXME
 use CGI;
 use C4::Branch; # GetBranches
 use List::Util qw/min/;
@@ -53,8 +54,8 @@ my $script_name="/cgi-bin/koha/admin/aqbudget.pl";
 my $bookfundid   = $input->param('bookfundid');
 my $aqbudgetid   = $input->param('aqbudgetid');
 my $branchcodeid = $input->param('branchcode');
+my $op           = $input->param('op') || '';
 my $pagesize = 20;
-my $op = $input->param('op');
 
 my ($template, $borrowernumber, $cookie)
     = get_template_and_user(
@@ -83,8 +84,6 @@ my ($flags, $homebranch)=$sthtemp->fetchrow;
 # called by default. Used to create form to add or  modify a record
 if ($op eq 'add_form') {
     my ($query, $dataaqbudget, $dataaqbookfund, $sth);
-    my $dbh = C4::Context->dbh;
-
     #---- if primkey exists, it's a modify action, so read values to modify...
     if ($aqbudgetid) {
         $query = '
@@ -104,7 +103,6 @@ SELECT aqbudgetid,
         $sth=$dbh->prepare($query);
         $sth->execute($aqbudgetid);
         $dataaqbudget=$sth->fetchrow_hashref;
-        $sth->finish;
     }
 
     $query = '
@@ -121,7 +119,6 @@ SELECT aqbookfund.branchcode,
         $branchcodeid
     );
     $dataaqbookfund=$sth->fetchrow_hashref;
-    $sth->finish;
 
     if (defined $aqbudgetid) {
         $template->param(
@@ -160,10 +157,8 @@ SELECT branchcode,
             $branch->{selected} =
                 $dataaqbudget->{branchcode} eq $row->{branchcode} ? 1 : 0;
         }
-
         push @branches, $branch;
     }
-    $sth->finish;
 
     $template->param(
         dateformat => C4::Dates->new()->visual(),
@@ -205,7 +200,6 @@ UPDATE aqbudget
             $input->param('branch') || '',
             $aqbudgetid,
         );
-        $sth->finish;
     }
     else {
         $query = '
@@ -223,36 +217,31 @@ INSERT
             $input->param('budgetamount'),
             $input->param('branch') || '',
         );
-        $sth->finish;
     }
 
-    $input->redirect("aqbudget.pl");
-
+    print $input->redirect("aqbudget.pl");  # FIXME: unnecessary redirect
+    exit;
 # END $OP eq ADD_VALIDATE
 ################## DELETE_CONFIRM ##################################
 # called by default form, used to confirm deletion of data in DB
 } elsif ($op eq 'delete_confirm') {
-	my $dbh = C4::Context->dbh;
 	my $sth=$dbh->prepare("select aqbudgetid,bookfundid,startdate,enddate,budgetamount,branchcode from aqbudget where aqbudgetid=?");
 	$sth->execute($aqbudgetid);
 	my $data=$sth->fetchrow_hashref;
-	$sth->finish;
 	$template->param(bookfundid => $bookfundid);
 	$template->param(aqbudgetid => $data->{'aqbudgetid'});
-	$template->param(startdate => format_date($data->{'startdate'}));
-	$template->param(enddate => format_date($data->{'enddate'}));
+	$template->param(startdate  => format_date($data->{'startdate'}));
+	$template->param(enddate    => format_date($data->{'enddate'}));
 	$template->param(budgetamount => $data->{'budgetamount'});
 													# END $OP eq DELETE_CONFIRM
 ################## DELETE_CONFIRMED ##################################
 # called by delete_confirm, used to effectively confirm deletion of data in DB
 } elsif ($op eq 'delete_confirmed') {
-	my $dbh = C4::Context->dbh;
 	my $aqbudgetid=uc($input->param('aqbudgetid'));
 	my $sth=$dbh->prepare("delete from aqbudget where aqbudgetid=?");
 	$sth->execute($aqbudgetid);
-	$sth->finish;
-	 print $input->redirect("aqbookfund.pl");
-	 return;
+	print $input->redirect("aqbookfund.pl");
+	exit;
 													# END $OP eq DELETE_CONFIRMED
 ################## DEFAULT ##################################
 } else { # DEFAULT
@@ -270,7 +259,6 @@ SELECT bookfundid, bookfundname
     while (my $row = $sth->fetchrow_hashref) {
         $bookfundname_of{ $row->{bookfundid} } = $row->{bookfundname};
     }
-    $sth->finish;
 
     # filters
     my $branches = GetBranches();
@@ -285,7 +273,6 @@ SELECT bookfundid, bookfundname
             and $input->param('filter_branchcode') eq $branchcode) {
             $row->{selected} = 1;
         }
-
         push @branchloop, $row;
     }
 
@@ -301,10 +288,8 @@ SELECT bookfundid
             and $input->param('filter_bookfundid') eq $row->{bookfundid}) {
             $row->{selected} = 1;
         }
-
         push @bookfundids_loop, $row;
     }
-    $sth->finish;
 
     $template->param(
         filter_bookfundids => \@bookfundids_loop,
@@ -341,7 +326,7 @@ SELECT aqbudgetid,
        budgetamount,
        branchcode
   FROM aqbudget
-  WHERE 1 = 1';			# What's the point?
+  WHERE 1 = 1';
 
     my @bindings;
 
@@ -389,7 +374,6 @@ SELECT aqbudgetid,
     while (my $row = $sth->fetchrow_hashref){
         push @results, $row;
     }
-    $sth->finish;
 
     # filter budgets depending on the pagination
     my $page = $input->param('page') || 1;
@@ -404,16 +388,13 @@ SELECT aqbudgetid,
 
     my @loop;
     foreach my $result (@results[$first .. $last]) {
-        push(
-            @loop,
-            {
-                %{$result},
-                bookfundname => $bookfundname_of{ $result->{'bookfundid'} },
-                branchname => $branches->{ $result->{branchcode} }->{branchname},
-                startdate => format_date($result->{startdate}),
-                enddate => format_date($result->{enddate}),
-            }
-        );
+        push @loop, {
+            %{$result},
+            bookfundname => $bookfundname_of{ $result->{'bookfundid'} },
+              branchname => $branches->{ $result->{branchcode} }->{branchname},
+               startdate => format_date($result->{startdate}),
+                 enddate => format_date($result->{enddate}),
+        };
     }
 
     $template->param(
diff --git a/admin/auth_subfields_structure.pl b/admin/auth_subfields_structure.pl
index b166777b05..6eb3667783 100755
--- a/admin/auth_subfields_structure.pl
+++ b/admin/auth_subfields_structure.pl
@@ -29,19 +29,10 @@ sub string_search  {
 	my $dbh = C4::Context->dbh;
 	$searchstring=~ s/\'/\\\'/g;
 	my @data=split(' ',$searchstring);
-	my $count=@data;
 	my $sth=$dbh->prepare("Select * from auth_subfield_structure where (tagfield like ? and authtypecode=?) order by tagfield");
 	$sth->execute("$searchstring%",$authtypecode);
-	my @results;
-	my $cnt=0;
-	my $u=1;
-	while (my $data=$sth->fetchrow_hashref){
-		push(@results,$data);
-		$cnt ++;
-		$u++;
-	}
-	$sth->finish;
-	return ($cnt,\@results);
+	my $results = $sth->fetchall_arrayref({});
+	return (scalar(@$results), $results);
 }
 
 sub auth_subfield_structure_exists {
@@ -52,25 +43,25 @@ sub auth_subfield_structure_exists {
 	return @$rows > 0;
 }
 
-my $input = new CGI;
-my $tagfield=$input->param('tagfield');
-my $tagsubfield=$input->param('tagsubfield');
-my $authtypecode=$input->param('authtypecode');
-my $pkfield="tagfield";
-my $offset=$input->param('offset');
-my $script_name="/cgi-bin/koha/admin/auth_subfields_structure.pl";
+my $input        = new CGI;
+my $tagfield     = $input->param('tagfield');
+my $tagsubfield  = $input->param('tagsubfield');
+my $authtypecode = $input->param('authtypecode');
+my $offset       = $input->param('offset');
+my $op           = $input->param('op') || '';
+my $script_name  = "/cgi-bin/koha/admin/auth_subfields_structure.pl";
 
-my ($template, $borrowernumber, $cookie)
-    = get_template_and_user({template_name => "admin/auth_subfields_structure.tmpl",
-			     query => $input,
-			     type => "intranet",
-			     authnotrequired => 0,
-			     flagsrequired => {parameters => 1},
-			     debug => 1,
-			     });
-my $pagesize=30;
-my $op = $input->param('op');
-$tagfield=~ s/\,//g;
+my ($template, $borrowernumber, $cookie) = get_template_and_user(
+    {   template_name   => "admin/auth_subfields_structure.tmpl",
+        query           => $input,
+        type            => "intranet",
+        authnotrequired => 0,
+        flagsrequired   => { parameters => 1 },
+        debug           => 1,
+    }
+);
+my $pagesize = 30;
+$tagfield =~ s/\,//g;
 
 if ($op) {
 $template->param(script_name => $script_name,
@@ -84,11 +75,11 @@ $template->param(script_name => $script_name,
 						else              => 1); # we show only the TMPL_VAR names $op
 }
 
+my $dbh = C4::Context->dbh;
 ################## ADD_FORM ##################################
 # called by default. Used to create form to add or  modify a record
 if ($op eq 'add_form') {
 	my $data;
-	my $dbh = C4::Context->dbh;
 	my $more_subfields = $input->param("more_subfields")+1;
 	# builds kohafield tables
 	my @kohafields;
@@ -100,7 +91,6 @@ if ($op eq 'add_form') {
 	}
 	
 	# build authorised value list
-	$sth2->finish;
 	$sth2 = $dbh->prepare("select distinct category from authorised_values");
 	$sth2->execute;
 	my @authorised_values;
@@ -112,7 +102,6 @@ if ($op eq 'add_form') {
 	push (@authorised_values,"itemtypes");
     
     # build thesaurus categories list
-    $sth2->finish;
     $sth2 = $dbh->prepare("select authtypecode from auth_types");
     $sth2->execute;
     my @authtypes;
@@ -393,7 +382,6 @@ if ($op eq 'add_form') {
 ################## ADD_VALIDATE ##################################
 # called by add_form, used to insert/modify data in DB
 } elsif ($op eq 'add_validate') {
-	my $dbh = C4::Context->dbh;
 	$template->param(tagfield => "$input->param('tagfield')");
 #	my $sth=$dbh->prepare("replace auth_subfield_structure (authtypecode,tagfield,tagsubfield,liblibrarian,libopac,repeatable,mandatory,kohafield,tab,seealso,authorised_value,frameworkcode,value_builder,hidden,isurl)
 #									values (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)");
@@ -478,8 +466,6 @@ if ($op eq 'add_form') {
 			}
 		}
 	}
-	$sth_insert->finish;
-	$sth_update->finish;
 	print "Content-Type: text/html\n\n<META HTTP-EQUIV=Refresh CONTENT=\"0; URL=auth_subfields_structure.pl?tagfield=$tagfield&authtypecode=$authtypecode\"></html>";
 	exit;
 
@@ -487,11 +473,9 @@ if ($op eq 'add_form') {
 ################## DELETE_CONFIRM ##################################
 # called by default form, used to confirm deletion of data in DB
 } elsif ($op eq 'delete_confirm') {
-	my $dbh = C4::Context->dbh;
 	my $sth=$dbh->prepare("select * from auth_subfield_structure where tagfield=? and tagsubfield=? and authtypecode=?");
 	$sth->execute($tagfield,$tagsubfield,$authtypecode);
 	my $data=$sth->fetchrow_hashref;
-	$sth->finish;
 	$template->param(liblibrarian => $data->{'liblibrarian'},
 							tagsubfield => $data->{'tagsubfield'},
 							delete_link => $script_name,
@@ -503,11 +487,9 @@ if ($op eq 'add_form') {
 ################## DELETE_CONFIRMED ##################################
 # called by delete_confirm, used to effectively confirm deletion of data in DB
 } elsif ($op eq 'delete_confirmed') {
-	my $dbh = C4::Context->dbh;
 	unless (C4::Context->config('demo') eq 1) {
 		my $sth=$dbh->prepare("delete from auth_subfield_structure where tagfield=? and tagsubfield=? and authtypecode=?");
 		$sth->execute($tagfield,$tagsubfield,$authtypecode);
-		$sth->finish;
 	}
 	print "Content-Type: text/html\n\n<META HTTP-EQUIV=Refresh CONTENT=\"0; URL=auth_subfields_structure.pl?tagfield=$tagfield&authtypecode=$authtypecode\"></html>";
 	exit;
diff --git a/admin/auth_tag_structure.pl b/admin/auth_tag_structure.pl
index 279a10ca55..4cd9b75508 100755
--- a/admin/auth_tag_structure.pl
+++ b/admin/auth_tag_structure.pl
@@ -29,20 +29,17 @@ use C4::Context;
 
 # retrieve parameters
 my $input = new CGI;
-my $authtypecode = $input->param('authtypecode'); # set to select framework
-$authtypecode="" unless $authtypecode;
-my $existingauthtypecode = $input->param('existingauthtypecode'); # set when we have to create a new framework (in authtype) by copying an old one (in existingauthtype)
-$existingauthtypecode = "" unless $existingauthtypecode;
-# my $authtypeinfo = getauthtypeinfo($authtype);
-my $searchfield=$input->param('searchfield');
-$searchfield=0 unless $searchfield;
-$searchfield=~ s/\,//g;
+my $authtypecode         = $input->param('authtypecode')         || '';    # set to select framework
+my $existingauthtypecode = $input->param('existingauthtypecode') || '';    # set when we have to create a new framework (in authtype) by copying an old one (in existingauthtype)
 
-my $offset=$input->param('offset');
-my $op = $input->param('op');
-my $pagesize=20;
+# my $authtypeinfo = getauthtypeinfo($authtype);
+my $searchfield = $input->param('searchfield') || 0;
+my $offset      = $input->param('offset') || 0;
+my $op          = $input->param('op')     || '';
+$searchfield =~ s/\,//g;
 
-my $script_name="/cgi-bin/koha/admin/auth_tag_structure.pl";
+my $pagesize    = 20;
+my $script_name = "/cgi-bin/koha/admin/auth_tag_structure.pl";
 
 my $dbh = C4::Context->dbh;
 
@@ -71,7 +68,6 @@ foreach my $thisauthtype (keys %$authtypes) {
 my $sth;
 # check that authtype framework is defined in auth_tag_structure if we are on a default action
 if (!$op or $op eq 'authtype_create_confirm') {
-#warn "IN";
 	$sth=$dbh->prepare("select count(*) from auth_tag_structure where authtypecode=?");
 	$sth->execute($authtypecode);
 	my ($authtypeexist) = $sth->fetchrow;
@@ -86,13 +82,12 @@ if (!$op or $op eq 'authtype_create_confirm') {
 		}
 	}
 }
+$template->param(script_name  => $script_name);
 $template->param(authtypeloop => \@authtypesloop);
-if ($op && $op ne 'authtype_create_confirm') {
-$template->param(script_name => $script_name,
-						$op              => 1); # we show only the TMPL_VAR names $op
+if ($op ne 'authtype_create_confirm') {
+    $template->param($op  => 1);
 } else {
-$template->param(script_name => $script_name,
-						else              => 1); # we show only the TMPL_VAR names $op
+    $template->param(else => 1);
 }
  
 ################## ADD_FORM ##################################
@@ -104,7 +99,6 @@ if ($op eq 'add_form') {
 		$sth=$dbh->prepare("select tagfield,liblibrarian,libopac,repeatable,mandatory,authorised_value from auth_tag_structure where tagfield=? and authtypecode=?");
 		$sth->execute($searchfield,$authtypecode);
 		$data=$sth->fetchrow_hashref;
-		$sth->finish;
 	}
 	my $sth = $dbh->prepare("select distinct category from authorised_values");
 	$sth->execute;
@@ -143,46 +137,37 @@ if ($op eq 'add_form') {
 ################## ADD_VALIDATE ##################################
 # called by add_form, used to insert/modify data in DB
 } elsif ($op eq 'add_validate') {
-    if ($input->param('modif')) {
-        $sth=$dbh->prepare("UPDATE auth_tag_structure SET tagfield=?, liblibrarian=?, libopac=?, repeatable=?, mandatory=?, authorised_value=? WHERE authtypecode=? AND tagfield=?");
-        my $tagfield       =$input->param('tagfield');
-        my $liblibrarian  = $input->param('liblibrarian');
-        my $libopac       =$input->param('libopac');
-        my $repeatable =$input->param('repeatable');
-        my $mandatory =$input->param('mandatory');
-        my $authorised_value =$input->param('authorised_value');
-        unless (C4::Context->config('demo') eq 1) {
+    my $tagfield         = $input->param('tagfield');
+    my $liblibrarian     = $input->param('liblibrarian');
+    my $libopac          = $input->param('libopac');
+    my $repeatable       = $input->param('repeatable') ? 1 : 0;
+    my $mandatory        = $input->param('mandatory')  ? 1 : 0;
+    my $authorised_value = $input->param('authorised_value');
+    unless (C4::Context->config('demo') eq 1) {
+        if ($input->param('modif')) {
+            $sth=$dbh->prepare("UPDATE auth_tag_structure SET tagfield=?, liblibrarian=?, libopac=?, repeatable=?, mandatory=?, authorised_value=? WHERE authtypecode=? AND tagfield=?");
             $sth->execute(
-                            $tagfield,
-                            $liblibrarian,
-                            $libopac,
-                            $repeatable?1:0,
-                            $mandatory?1:0,
-                            $authorised_value,
-                            $authtypecode,
-                            $tagfield,
-                            );
-        }
-        $sth->finish;
-    } else {
-        $sth=$dbh->prepare("INSERT INTO auth_tag_structure (tagfield,liblibrarian,libopac,repeatable,mandatory,authorised_value,authtypecode) VALUES (?,?,?,?,?,?,?)");
-        my $tagfield       =$input->param('tagfield');
-        my $liblibrarian  = $input->param('liblibrarian');
-        my $libopac       =$input->param('libopac');
-        my $repeatable =$input->param('repeatable');
-        my $mandatory =$input->param('mandatory');
-        my $authorised_value =$input->param('authorised_value');
-        unless (C4::Context->config('demo') eq 1) {
-            $sth->execute($tagfield,
-                            $liblibrarian,
-                            $libopac,
-                            $repeatable?1:0,
-                            $mandatory?1:0,
-                            $authorised_value,
-                            $authtypecode
-                            );
+                $tagfield,
+                $liblibrarian,
+                $libopac,
+                $repeatable,
+                $mandatory,
+                $authorised_value,
+                $authtypecode,
+                $tagfield,
+            );
+        } else {
+            $sth=$dbh->prepare("INSERT INTO auth_tag_structure (tagfield,liblibrarian,libopac,repeatable,mandatory,authorised_value,authtypecode) VALUES (?,?,?,?,?,?,?)");
+            $sth->execute(
+                $tagfield,
+                $liblibrarian,
+                $libopac,
+                $repeatable,
+                $mandatory,
+                $authorised_value,
+                $authtypecode
+           );
         }
-        $sth->finish;
     }
 	print "Content-Type: text/html\n\n<META HTTP-EQUIV=Refresh CONTENT=\"0; URL=auth_tag_structure.pl?searchfield=".$input->param('tagfield')."&authtypecode=$authtypecode\">";
 	exit;
@@ -193,7 +178,6 @@ if ($op eq 'add_form') {
 	$sth=$dbh->prepare("select tagfield,liblibrarian,libopac,repeatable,mandatory,authorised_value from auth_tag_structure where tagfield=?");
 	$sth->execute($searchfield);
 	my $data=$sth->fetchrow_hashref;
-	$sth->finish;
 	$template->param(liblibrarian => $data->{'liblibrarian'},
 							searchfield => $searchfield,
 							authtypecode => $authtypecode,
@@ -205,6 +189,7 @@ if ($op eq 'add_form') {
 	unless (C4::Context->config('demo') eq 1) {
 		$dbh->do("delete from auth_tag_structure where tagfield='$searchfield' and authtypecode='$authtypecode'");
 		$dbh->do("delete from auth_subfield_structure where tagfield='$searchfield' and authtypecode='$authtypecode'");
+        # FIXME: Secuity vulnerability -- use placeholders, prepare and execute!
 	}
     print "Content-Type: text/html\n\n<META HTTP-EQUIV=Refresh CONTENT=\"0; URL=auth_tag_structure.pl?searchfield=".$input->param('tagfield')."&authtypecode=$authtypecode\">";
     exit;
@@ -243,15 +228,15 @@ if ($op eq 'add_form') {
 			$toggle=1;
 	  	}
 		my %row_data;  # get a fresh hash for the row data
-		$row_data{tagfield} = $results->[$i]{'tagfield'};
-		$row_data{liblibrarian} = $results->[$i]{'liblibrarian'};
-		$row_data{repeatable} = $results->[$i]{'repeatable'};
-		$row_data{mandatory} = $results->[$i]{'mandatory'};
-		$row_data{authorised_value} = $results->[$i]{'authorised_value'};
-		$row_data{subfield_link} ="auth_subfields_structure.pl?tagfield=".$results->[$i]{'tagfield'}."&amp;authtypecode=".$authtypecode;
-		$row_data{edit} = "$script_name?op=add_form&amp;searchfield=".$results->[$i]{'tagfield'}."&amp;authtypecode=".$authtypecode;
-		$row_data{delete} = "$script_name?op=delete_confirm&amp;searchfield=".$results->[$i]{'tagfield'}."&amp;authtypecode=".$authtypecode;
-		$row_data{toggle} = $toggle;
+        $row_data{tagfield}         = $results->[$i]{'tagfield'};
+        $row_data{liblibrarian}     = $results->[$i]{'liblibrarian'};
+        $row_data{repeatable}       = $results->[$i]{'repeatable'};
+        $row_data{mandatory}        = $results->[$i]{'mandatory'};
+        $row_data{authorised_value} = $results->[$i]{'authorised_value'};
+        $row_data{subfield_link}    = "auth_subfields_structure.pl?tagfield=" . $results->[$i]{'tagfield'} . "&amp;authtypecode=" . $authtypecode;
+        $row_data{edit}             = "$script_name?op=add_form&amp;searchfield=" . $results->[$i]{'tagfield'} . "&amp;authtypecode=" . $authtypecode;
+        $row_data{delete}           = "$script_name?op=delete_confirm&amp;searchfield=" . $results->[$i]{'tagfield'} . "&amp;authtypecode=" . $authtypecode;
+        $row_data{toggle}           = $toggle;
 		push(@loop_data, \%row_data);
 	}
 	$template->param(loop => \@loop_data,
@@ -262,24 +247,18 @@ if ($op eq 'add_form') {
 		$template->param(isprevpage => $offset,
 						prevpage=> $prevpage,
 						searchfield => $searchfield,
-						script_name => $script_name,
 		 );
 	}
 	if ($offset+$pagesize<$count) {
 		my $nextpage =$offset+$pagesize;
 		$template->param(nextpage =>$nextpage,
 						searchfield => $searchfield,
-						script_name => $script_name,
 		);
 	}
 } #---- END $OP eq DEFAULT
 
-$template->param(loggeninuser => $loggedinuser,
-		);
-
 output_html_with_http_headers $input, $cookie, $template->output;
 
-
 #
 # the sub used for searches
 #
@@ -288,15 +267,12 @@ sub StringSearch  {
 	my $dbh = C4::Context->dbh;
 	$searchstring=~ s/\'/\\\'/g;
 	my @data=split(' ',$searchstring);
-	my $count=@data;
 	my $sth=$dbh->prepare("Select tagfield,liblibrarian,libopac,repeatable,mandatory,authorised_value from auth_tag_structure where (tagfield >= ? and authtypecode=?) order by tagfield");
 	$sth->execute($data[0], $authtypecode);
 	my @results;
 	while (my $data=$sth->fetchrow_hashref){
-	push(@results,$data);
+        push(@results,$data);
 	}
-	#  $sth->execute;
-	$sth->finish;
 	return (scalar(@results),\@results);
 }
 
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbookfund.tmpl b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbookfund.tmpl
index 3403c690ca..eb464b9d46 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbookfund.tmpl
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbookfund.tmpl
@@ -1,43 +1,15 @@
 <!-- TMPL_INCLUDE NAME="doc-head-open.inc" -->
 <title>Koha &rsaquo; Administration &rsaquo; Funds and Budgets</title>
 <!-- TMPL_INCLUDE NAME="doc-head-close.inc" -->
-<!-- TMPL_IF name="add_form" -->
 <script type="text/javascript">
 //<![CDATA[
-
-	function isNotNull(f,noalert) {
-		if (f.value.length ==0) {
-   return false;
-		}
-		return true;
-	}
-
+<!-- TMPL_IF name="add_form" -->
 	function toUC(f) {
-		var x=f.value.toUpperCase();
-		f.value=x;
+		f.value=f.value.toUpperCase();
 		return true;
 	}
 
-	function isNum(v,maybenull) {
-	var n = new Number(v.value);
-	if (isNaN(n)) {
-		return false;
-		}
-	if (maybenull==0 && v.value=="") {
-		return false;
-	}
-	return true;
-	}
-
-	function isDate(f) {
-		var t = Date.parse(f.value);
-		if (isNaN(t)) {
-			return false;
-		}
-	}
-
 	function Check(f) {
-		var ok=1;
 		var _alertString="";
 		var alertString2;
 		if (f.bookfundid.value.length==0) {
@@ -55,47 +27,34 @@
 			alert(alertString2);
 		}
 	}
-	//]]>
-</script>
 <!-- /TMPL_IF -->
+	 $(document).ready(function() {
+	    new YAHOO.widget.Button("newfund");
+	 });
+//]]>
+</script>
 </head>
 <body>
 <!-- TMPL_INCLUDE NAME="header.inc" -->
 <!-- TMPL_INCLUDE NAME="bookfund-admin-search.inc" -->
 
-<div id="breadcrumbs"><a href="/cgi-bin/koha/mainpage.pl">Home</a> &rsaquo; <a href="/cgi-bin/koha/admin/admin-home.pl">Administration</a> &rsaquo; <!-- TMPL_IF name="else" -->Funds and Budgets  <!-- /TMPL_IF --> <!-- TMPL_IF name="add_form" --><a href="/cgi-bin/koha/admin/aqbookfund.pl">Funds and Budgets</a> &rsaquo; Add fund<!-- /TMPL_IF --></div>
+<div id="breadcrumbs">
+    <a href="/cgi-bin/koha/mainpage.pl">Home</a>
+&rsaquo; <a href="/cgi-bin/koha/admin/admin-home.pl">Administration</a>
+&rsaquo; <a href="/cgi-bin/koha/admin/aqbookfund.pl">Funds and Budgets</a>
+<!-- TMPL_IF name="add_form" -->&rsaquo; Add fund<!-- /TMPL_IF -->
+</div>
 
 <div id="doc3" class="yui-t2">
-   
-   <div id="bd">
-	<div id="yui-main">
-	<div class="yui-b">
+  <div id="bd">
+    <div id="yui-main">
+      <div class="yui-b">
 	
 <!-- TMPL_IF name="else" -->
 <div id="toolbar">
-	<script type="text/javascript">
-	//<![CDATA[
-
-	// prepare DOM for YUI Toolbar
-
-	 $(document).ready(function() {
-	    yuiToolbar();
-	 });
-
-	// YUI Toolbar Functions
-
-	function yuiToolbar() {
-	    new YAHOO.widget.Button("newfund");
-	}
-
-	//]]>
-	</script>
 	<ul class="toolbar">
 	<li><a id="newfund" href="/cgi-bin/koha/admin/aqbookfund.pl?op=add_form">New Fund</a></li>
 </ul></div>
-<!-- /TMPL_IF -->
-
-<!-- TMPL_IF name="else" -->
 <h1>Funds and budgets administration</h1>
   <!-- TMPL_IF NAME="bookfund" -->
 <form action="aqbudget.pl" method="post">
@@ -113,28 +72,18 @@
       <!-- TMPL_ELSE -->
   <tr>
       <!-- /TMPL_UNLESS -->
+    <td><!-- TMPL_VAR name="bookfundid" --></td>
+    <td><!-- TMPL_VAR name="bookfundname" --></td>
+    <td><!-- TMPL_VAR name="branchname" DEFAULT="" --></td>
     <td>
-      <!-- TMPL_VAR name="bookfundid" -->
-    </td>
-    <td>
-      <!-- TMPL_VAR name="bookfundname" -->
-    </td>
-    <td>
-      <!-- TMPL_IF NAME="branchname" -->
-      <!-- TMPL_VAR name="branchname" -->
-      <!-- TMPL_ELSE -->
-      <!-- /TMPL_IF -->
-    </td>
-    <td>
-      <a href="<!-- TMPL_VAR name="scriptname" -->?op=add_form&amp;bookfundid=<!-- TMPL_VAR name="bookfundid" -->&amp;branchcode=<!-- TMPL_VAR name="branchcode" -->">Edit</a></td><td>
-      <a href="<!-- TMPL_VAR name="scriptname" -->?op=delete_confirm&amp;bookfundid=<!-- TMPL_VAR name="bookfundid" -->&amp;branchcode=<!-- TMPL_VAR name="branchcode" -->">Delete</a></td><td>
+      <a href="<!-- TMPL_VAR name="script_name" -->?op=add_form&amp;bookfundid=<!-- TMPL_VAR name="bookfundid" -->&amp;branchcode=<!-- TMPL_VAR name="branchcode" -->">Edit</a></td><td>
+      <a href="<!-- TMPL_VAR name="script_name" -->?op=delete_confirm&amp;bookfundid=<!-- TMPL_VAR name="bookfundid" -->&amp;branchcode=<!-- TMPL_VAR name="branchcode" -->">Delete</a></td><td>
       <a href="aqbudget.pl?op=add_form&amp;bookfundid=<!-- TMPL_VAR name="bookfundid" -->&amp;branchcode=<!-- TMPL_VAR name="branchcode" -->">Add budget</a>
       <!-- TMPL_IF NAME="has_budgets" -->
       <a href="aqbudget.pl?filter_bookfundid=<!-- TMPL_VAR name="bookfundid" -->">Show budgets</a>
       <!-- /TMPL_IF -->
     </td>
   </tr>
-
     <!-- /TMPL_LOOP --> <!-- bookfund -->
 </table>
 </form>
@@ -150,44 +99,37 @@
 
 <!-- TMPL_IF name="add_form" -->
 <form action="<!-- TMPL_VAR name="action" -->" name="Aform" method="post">
-
+  <input type="hidden" name="checked" value="0" />
 
 <!-- TMPL_IF name="header-is-modify-p" -->
   <input type="hidden" name="op" value="mod_validate" />
   <input type="hidden" name="current_branch" value="<!-- TMPL_VAR name="current_branch" -->"/>
-  <!-- /TMPL_IF -->
-
-<!-- TMPL_IF name="header-is-add-p" -->
+<!-- TMPL_ELSIF name="header-is-add-p" -->
   <input type="hidden" name="op" value="add_validate" />
-  <!-- /TMPL_IF -->
-
-
-  <input type="hidden" name="checked" value="0" />
+<!-- /TMPL_IF -->
 
   <fieldset class="rows">
-    <legend><!-- TMPL_VAR name="header" --></legend>
+    <legend><!-- TMPL_IF name="header-is-modify-p" -->Modify book fund<!-- TMPL_ELSIF name="header-is-add-p" -->Add book fund<!-- /TMPL_IF -->
+    </legend>
     
-  <ol><!-- TMPL_IF name="add_or_modify" -->
-    <li>
-      	<span class="label">Fund: </span>
+  <ol>
+  <!-- TMPL_IF name="add_or_modify" -->
+    <li><span class="label">Fund: </span>
         <input type="hidden" name="bookfundid" id="bookfundid" value="<!-- TMPL_VAR name="bookfundid" -->" />
         <!-- TMPL_VAR name="bookfundid" -->
     </li>
   <!-- TMPL_ELSE -->
-    <li>
-      <label for="bookfundid">Fund: </label>
-      <input type="text" name="bookfundid" id="bookfundid" size="5" maxlength="5" onblur="toUC(this)" />
+    <li><label for="bookfundid">Fund: </label>
+        <input type="text" name="bookfundid" id="bookfundid" size="5" maxlength="5" onblur="toUC(this)" />
     </li>
   <!-- /TMPL_IF --> <!-- add_or_modify -->
     
-    <li>
-      <label for="bookfundname">Name: </label>
-	  <input type="text" name="bookfundname" id="bookfundname" size="40" maxlength="80" value="<!-- TMPL_VAR name="bookfundname" escape="HTML" -->" />
-      </li>
+    <li><label for="bookfundname">Name: </label>
+	    <input type="text" name="bookfundname" id="bookfundname" size="40" maxlength="80" value="<!-- TMPL_VAR name="bookfundname" escape="HTML" -->" />
+    </li>
 
-    <li>
-      <label for="branchcode">Library: </label>
-      <select name="branchcode" id="branchcode">
+    <li><label for="branchcode">Library: </label>
+        <select name="branchcode" id="branchcode">
           <option value="">----</option>
   <!-- TMPL_LOOP NAME="branches" -->
     <!-- TMPL_IF NAME="selected" -->
@@ -197,38 +139,29 @@
     <!-- /TMPL_IF -->
   <!-- /TMPL_LOOP -->
   	</select>
-    </li></ol>
+    </li>
+  </ol>
     
   </fieldset>
 
   <fieldset class="action">
-    <input type="button" value="Submit" onclick="Check(this.form); return false;" /> <a class="cancel" href="/cgi-bin/koha/admin/aqbookfund.pl">Cancel</a>
+    <input type="button" value="Submit" onclick="Check(this.form); return false;" />
+    <a class="cancel" href="/cgi-bin/koha/admin/aqbookfund.pl">Cancel</a>
   </fieldset>
 </form>
 <!-- /TMPL_IF --> <!-- add_form -->
 
-
 <!-- TMPL_IF name="add_validate" -->
 <h3>Fund added</h3>
-
 <p>
   <a href="<!-- TMPL_VAR name="action" -->">Return to fund list</a>
 </p>
-<!-- /TMPL_IF -->
-
-
-<!-- TMPL_IF name="mod_validate" -->
+<!-- TMPL_ELSIF name="mod_validate" -->
 <h3>Fund modified</h3>
-
 <p>
   <a href="<!-- TMPL_VAR name="action" -->">Return to fund list</a>
 </p>
-<!-- /TMPL_IF -->
-
-
-
-<!-- TMPL_IF name="delete_confirm" -->
-
+<!-- TMPL_ELSIF name="delete_confirm" -->
 <div class="dialog alert">
 <h3>Confirm Deletion of Fund <em><!-- TMPL_VAR name="bookfundid" --></em></h3>
 <form action="<!-- TMPL_VAR name="action" -->" method="post">
@@ -236,31 +169,27 @@
   <input type="hidden" name="bookfundid" value="<!-- TMPL_VAR name="bookfundid" -->" />
   <input type="hidden" name="branchcode" value="<!-- TMPL_VAR name="branchcode" -->" />
 	<table>
-		  <tr>
+  <tr>
     <th scope="row">Fund: </th><td>
     <!-- TMPL_VAR name="bookfundid" --></td></tr>
-  
   <tr>
     <th scope="row">Library: </th><td>
     <!-- TMPL_VAR name="branchcode" --></td></tr>
-  
   <tr>
     <th scope="row">Name: </th><td>
     <!-- TMPL_VAR name="bookfundname" --></td></tr>
-  
   <tr>
     <th scope="row">Group: </th><td>
     <!-- TMPL_VAR name="bookfundgroup" --></td></tr>
   	</table>
-   <input type="submit" class="approve" value="Yes, Delete this Fund" /></form>
-   <form action="/cgi-bin/koha/admin/aqbookfund.pl" method="get"><input type="submit" class="deny" value="No, Do not Delete" /></form>
-   </div>
-
-<!-- /TMPL_IF --> <!-- delete_confirm -->
+  <input type="submit" class="approve" value="Yes, Delete this Fund" />
+</form>
+<form action="/cgi-bin/koha/admin/aqbookfund.pl" method="get"><input type="submit" class="deny" value="No, Do not Delete" /></form>
+</div>
 
-<!-- TMPL_IF name="delete_confirmed" -->
+<!-- TMPL_ELSIF name="delete_confirmed" -->
 <div class="dialog message"><h3>Data Deleted</h3>
-<form action="<!-- TMPL_VAR name="action" -->" method="post">
+<form action="<!-- TMPL_VAR name="action" -->" method="get">
   <input type="submit" class="approve" value="OK" />
 </form></div>
 <!-- /TMPL_IF --> <!-- delete_confirmed -->
-- 
2.39.2