[Koha-patches] [Signed Off] [PATCH] Bug 6181: Remove CGI Scolling lists from C4::Budgets

Chris Cormack chris at bigballofwax.co.nz
Sat Jun 4 12:00:31 CEST 2011


From: Colin Campbell <colin.campbell at ptfs-europe.com>

As noted on bug 766 more cases of usage of
CGI::scrolling_list were imported into C4::Budgets
Even if we were not trying to remove usage of this
the C4 modules are not the place to generate markup
Most of these routines are noise as they are not used in
any current code but cause confusion and increase
maintenance overhead. They are removed

The sort dropboxes on order create are the only
references in current templates to these routines
they have been replaced by a select list generated
by the markup.
They can probably be removed too but their existence
although the option that causes them to be displayed
seems unlikely to be set. I've left them pending
resolution of some of the inconsistencies and
confusions surrounding Budgts

Signed-off-by: Chris Cormack <chris at bigballofwax.co.nz>
---
 C4/Budgets.pm                                      |   93 ++++----------------
 acqui/addorderiso2709.pl                           |   12 ++--
 acqui/fetch_sort_dropbox.pl                        |   65 --------------
 acqui/neworderempty.pl                             |   16 ++--
 admin/aqbudgetperiods.pl                           |    2 -
 admin/aqbudgets.pl                                 |    3 -
 admin/aqplan.pl                                    |    2 -
 .../prog/en/modules/acqui/addorderiso2709.tt       |   29 +++++--
 .../prog/en/modules/acqui/neworderempty.tt         |   24 ++++-
 9 files changed, 75 insertions(+), 171 deletions(-)
 delete mode 100755 acqui/fetch_sort_dropbox.pl

diff --git a/C4/Budgets.pm b/C4/Budgets.pm
index d2010d0..4b41b12 100644
--- a/C4/Budgets.pm
+++ b/C4/Budgets.pm
@@ -50,10 +50,7 @@ BEGIN {
         &AddBudgetPeriod
 	    &DelBudgetPeriod
 
-	    &GetBudgetPeriodsDropbox
-        &GetBudgetSortDropbox
         &GetAuthvalueDropbox
-        &GetBudgetPermDropbox
 
         &ModBudgetPlan
 
@@ -335,24 +332,6 @@ sub GetBudgetOrdered {
 }
 
 # -------------------------------------------------------------------
-sub GetBudgetPermDropbox {
-	my ($perm) = @_;
-	my %labels;
-	$labels{'0'} = 'None';
-	$labels{'1'} = 'Owner';
-	$labels{'2'} = 'Library';
-	my $radio = CGI::scrolling_list(
-		-id       => 'budget_permission',
-		-name      => 'budget_permission',
-		-values    => [ '0', '1', '2' ],
-		-default   => $perm,
-		-labels    => \%labels,
-		-size    => 1,
-	);
-	return $radio;
-}
-
-# -------------------------------------------------------------------
 sub GetBudgetAuthCats  {
     my ($budget_period_id) = shift;
     # now, populate the auth_cats_loop used in the budget planning button
@@ -374,61 +353,27 @@ sub GetBudgetAuthCats  {
 
 # -------------------------------------------------------------------
 sub GetAuthvalueDropbox {
-	my ( $name, $authcat, $default ) = @_;
-	my @authorised_values;
-	my %authorised_lib;
-	my $value;
-	my $dbh = C4::Context->dbh;
-	my $sth = $dbh->prepare(
-		"SELECT authorised_value,lib
-            FROM authorised_values
-            WHERE category = ?
-            ORDER BY  lib"
-	);
-	$sth->execute( $authcat );
-
-	push @authorised_values, '';
-	while (my ($value, $lib) = $sth->fetchrow_array) {
-		push @authorised_values, $value;
-		$authorised_lib{$value} = $lib;
-	}
-
-    return 0 if keys(%authorised_lib) == 0;
-
-    my $budget_authvalue_dropbox = CGI::scrolling_list(
-        -values   => \@authorised_values,
-        -labels   => \%authorised_lib,
-        -default  => $default,
-        -override => 1,
-        -size     => 1,
-        -multiple => 0,
-        -name     => $name,
-        -id       => $name,
+    my ( $authcat, $default ) = @_;
+    my $dbh = C4::Context->dbh;
+    my $sth = $dbh->prepare(
+        'SELECT authorised_value,lib FROM authorised_values
+        WHERE category = ? ORDER BY lib'
     );
+    $sth->execute( $authcat );
+    my $option_list = [];
+    my @authorised_values = ( q{} );
+    while (my ($value, $lib) = $sth->fetchrow_array) {
+        push @{$option_list}, {
+            value => $value,
+            label => $lib,
+            default => ($default eq $value),
+        };
+    }
 
-    return $budget_authvalue_dropbox
-}
-
-# -------------------------------------------------------------------
-sub GetBudgetPeriodsDropbox {
-    my ($budget_period_id) = @_;
-	my %labels;
-	my @values;
-	my ($active, $periods) = GetBudgetPeriods();
-	foreach my $r (@$periods) {
-		$labels{"$r->{budget_period_id}"} = $r->{budget_period_description};
-		push @values, $r->{budget_period_id};
-	}
-
-	# if no buget_id is passed then its an add
-	my $budget_period_dropbox = CGI::scrolling_list(
-		-name    => 'budget_period_id',
-		-values  => \@values,
-		-default => $budget_period_id ? $budget_period_id :  $active,
-		-size    => 1,
-		-labels  => \%labels,
-	);
-	return $budget_period_dropbox;
+    if ( @{$option_list} ) {
+        return $option_list;
+    }
+    return;
 }
 
 # -------------------------------------------------------------------
diff --git a/acqui/addorderiso2709.pl b/acqui/addorderiso2709.pl
index c46263a..0b9d860 100755
--- a/acqui/addorderiso2709.pl
+++ b/acqui/addorderiso2709.pl
@@ -281,12 +281,12 @@ $template->param( budget_loop    => $budget_loop,);
 my $CGIsort1;
 if ($budget) {    # its a mod ..
     if ( defined $budget->{'sort1_authcat'} ) {    # with custom  Asort* planning values
-        $CGIsort1 = GetAuthvalueDropbox( 'sort1', $budget->{'sort1_authcat'}, $data->{'sort1'} );
+        $CGIsort1 = GetAuthvalueDropbox(  $budget->{'sort1_authcat'}, $data->{'sort1'} );
     }
 } elsif ( scalar(@$budgets) ) {
-    $CGIsort1 = GetAuthvalueDropbox( 'sort1', @$budgets[0]->{'sort1_authcat'}, '' );
+    $CGIsort1 = GetAuthvalueDropbox(  @$budgets[0]->{'sort1_authcat'}, '' );
 } else {
-    $CGIsort1 = GetAuthvalueDropbox( 'sort1', '', '' );
+    $CGIsort1 = GetAuthvalueDropbox(  '', '' );
 }
 
 # if CGIsort is successfully fetched, the use it
@@ -300,12 +300,12 @@ if ($CGIsort1) {
 my $CGIsort2;
 if ($budget) {
     if ( defined $budget->{'sort2_authcat'} ) {
-        $CGIsort2 = GetAuthvalueDropbox( 'sort2', $budget->{'sort2_authcat'}, $data->{'sort2'} );
+        $CGIsort2 = GetAuthvalueDropbox(  $budget->{'sort2_authcat'}, $data->{'sort2'} );
     }
 } elsif ( scalar(@$budgets) ) {
-    $CGIsort2 = GetAuthvalueDropbox( 'sort2', @$budgets[0]->{sort2_authcat}, '' );
+    $CGIsort2 = GetAuthvalueDropbox(  @$budgets[0]->{sort2_authcat}, '' );
 } else {
-    $CGIsort2 = GetAuthvalueDropbox( 'sort2', '', '' );
+    $CGIsort2 = GetAuthvalueDropbox( '', '' );
 }
 
 if ($CGIsort2) {
diff --git a/acqui/fetch_sort_dropbox.pl b/acqui/fetch_sort_dropbox.pl
deleted file mode 100755
index 21d3c61..0000000
--- a/acqui/fetch_sort_dropbox.pl
+++ /dev/null
@@ -1,65 +0,0 @@
-#!/usr/bin/perl
-
-# Copyright 2008-2009 BibLibre SARL
-#
-# This file is part of Koha.
-#
-# Koha is free software; you can redistribute it and/or modify it under the
-# terms of the GNU General Public License as published by the Free Software
-# Foundation; either version 2 of the License, or (at your option) any later
-# version.
-#
-# Koha is distributed in the hope that it will be useful, but WITHOUT ANY
-# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
-# A PARTICULAR PURPOSE.  See the GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License along
-# with Koha; if not, write to the Free Software Foundation, Inc.,
-# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
-
-use strict;
-#use warnings; FIXME - Bug 2505
-use CGI;
-use C4::Context;
-use C4::Output;
-use C4::Auth;
-use C4::Budgets;
-
-=head1 NAME
-
-fetch_sort_dropbox
-
-=cut
-
-my $input = new CGI;
-
-my $budget_id = $input->param('budget_id');
-my $sort_id   = $input->param('sort');
-
-my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
-    {   template_name   => "acqui/ajax.tmpl", # FIXME: REMOVE TMPL DEP?
-        query           => $input,
-        type            => "intranet",
-        authnotrequired => 0,
-        flagsrequired => {editcatalogue => 'edit_catalogue'},
-        debug => 0,
-    }
-);
-
-my $sort_dropbox;
-my $budget = GetBudget($budget_id);
-
-if ( $sort_id == 1 ) {
-    $sort_dropbox = GetAuthvalueDropbox( 'sort1', $budget->{'sort1_authcat'}, '' );
-} elsif ( $sort_id == 2 ) {
-    $sort_dropbox = GetAuthvalueDropbox( 'sort2', $budget->{'sort2_authcat'}, '' );
-}
-
-#strip off select tags ;/
-$sort_dropbox =~ s/^\<select.*?\"\>//;
-$sort_dropbox =~ s/\<\/select\>$//;
-chomp $sort_dropbox;
-
-$template->param( return => $sort_dropbox );
-output_html_with_http_headers $input, $cookie, $template->output;
-1;
diff --git a/acqui/neworderempty.pl b/acqui/neworderempty.pl
index 386dda7..eb92ba2 100755
--- a/acqui/neworderempty.pl
+++ b/acqui/neworderempty.pl
@@ -278,12 +278,12 @@ if ($close) {
 my $CGIsort1;
 if ($budget) {    # its a mod ..
     if ( defined $budget->{'sort1_authcat'} ) {    # with custom  Asort* planning values
-        $CGIsort1 = GetAuthvalueDropbox( 'sort1', $budget->{'sort1_authcat'}, $data->{'sort1'} );
+        $CGIsort1 = GetAuthvalueDropbox( $budget->{'sort1_authcat'}, $data->{'sort1'} );
     }
-} elsif(scalar(@$budgets)){
-    $CGIsort1 = GetAuthvalueDropbox( 'sort1', @$budgets[0]->{'sort1_authcat'}, '' );
+} elsif(@{$budgets}){
+    $CGIsort1 = GetAuthvalueDropbox(  @$budgets[0]->{'sort1_authcat'}, '' );
 }else{
-    $CGIsort1 = GetAuthvalueDropbox( 'sort1','', '' );
+    $CGIsort1 = GetAuthvalueDropbox( '', '' );
 }
 
 # if CGIsort is successfully fetched, the use it
@@ -297,12 +297,12 @@ if ($CGIsort1) {
 my $CGIsort2;
 if ($budget) {
     if ( defined $budget->{'sort2_authcat'} ) {
-        $CGIsort2 = GetAuthvalueDropbox( 'sort2', $budget->{'sort2_authcat'}, $data->{'sort2'} );
+        $CGIsort2 = GetAuthvalueDropbox(  $budget->{'sort2_authcat'}, $data->{'sort2'} );
     }
-} elsif(scalar(@$budgets)) {
-    $CGIsort2 = GetAuthvalueDropbox( 'sort2', @$budgets[0]->{sort2_authcat}, '' );
+} elsif(@{$budgets}) {
+    $CGIsort2 = GetAuthvalueDropbox(  @$budgets[0]->{sort2_authcat}, '' );
 }else{
-    $CGIsort2 = GetAuthvalueDropbox( 'sort2','', '' );
+    $CGIsort2 = GetAuthvalueDropbox( '', '' );
 }
 
 if ($CGIsort2) {
diff --git a/admin/aqbudgetperiods.pl b/admin/aqbudgetperiods.pl
index 724c7e5..e74935e 100755
--- a/admin/aqbudgetperiods.pl
+++ b/admin/aqbudgetperiods.pl
@@ -188,10 +188,8 @@ elsif ( $op eq 'delete_confirmed' ) {
         $budgetperiod->{budget_active} = 1;
         push( @period_loop, $budgetperiod );
     }
-    my $budget_period_dropbox = GetBudgetPeriodsDropbox();
 
     $template->param(
-        budget_period_dropbox => $budget_period_dropbox,
         period_loop           => \@period_loop,
 		pagination_bar		  => pagination_bar("aqbudgetperiods.pl",getnbpages(scalar(@$results),$pagesize),$page),
     );
diff --git a/admin/aqbudgets.pl b/admin/aqbudgets.pl
index e34f2cc..79c7b13 100755
--- a/admin/aqbudgets.pl
+++ b/admin/aqbudgets.pl
@@ -73,7 +73,6 @@ my $script_name               = "/cgi-bin/koha/admin/aqbudgets.pl";
 my $budget_hash               = $input->Vars;
 my $budget_id                 = $$budget_hash{budget_id};
 my $budget_permission         = $input->param('budget_permission');
-my $budget_period_dropbox     = $input->param('budget_period_dropbox');
 my $filter_budgetbranch       = $input->param('filter_budgetbranch');
 #filtering non budget keys
 delete $$budget_hash{$_} foreach grep {/filter|^op$|show/} keys %$budget_hash;
@@ -225,9 +224,7 @@ if ($op eq 'add_form') {
         }
     }
     my $branches = GetBranches();
-    my $budget_period_dropbox = GetBudgetPeriodsDropbox($$period{budget_period_id} );
     $template->param(
-        budget_period_dropbox     => $budget_period_dropbox,
         budget_id                 => $budget_id,
         %$period,
     );
diff --git a/admin/aqplan.pl b/admin/aqplan.pl
index 8b68d51..81418ff 100755
--- a/admin/aqplan.pl
+++ b/admin/aqplan.pl
@@ -74,14 +74,12 @@ my $budget_period_startdate   = $period->{'budget_period_startdate'};
 my $budget_period_enddate     = $period->{'budget_period_enddate'};
 my $budget_period_locked      = $period->{'budget_period_locked'};
 my $budget_period_description = $period->{'budget_period_description'};
-my $budget_period_dropbox     = GetBudgetPeriodsDropbox($budget_period_id );
 
 
 $template->param(
     budget_period_id          => $budget_period_id,
     budget_period_locked      => $budget_period_locked,
     budget_period_description => $budget_period_description,
-    budget_period_dropbox     => $budget_period_dropbox,
     auth_cats_loop            => $auth_cats_loop,
 );
 
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/addorderiso2709.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/addorderiso2709.tt
index 04d0c58..dd864cf 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/addorderiso2709.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/addorderiso2709.tt
@@ -184,21 +184,36 @@
                         <li><div class="hint">The 2 following fields are available for your own usage. They can be useful for statistical purposes</div>
                             <label for="sort1">Planning value1: </label>
 
-                            [% IF ( CGIsort1 ) %]
-                                [% CGIsort1 %]
+                            [% IF CGIsort1 %]
+                                <select id="sort1" size="1" name="sort1">
+                                [% FOREACH sort_opt IN CGIsort1 %]
+                                    [% IF sort_opt.default %]
+                                        <option value="[% sort_opt.id %]" selected="selected">[% sort_opt.label %]</option>
+                                    [% ELSE %]
+                                        <option value="[% sort_opt.id %]">[% sort_opt.label %]</option>
+                                    [% END %]
+                                [% END %]
+                                </select>
                             [% ELSE %]
-
                                 <input type="text" id="sort1" size="20" name="sort1" value="[% sort1 %]" />
                             [% END %]
                         </li>
                         <li>
                             <label for="sort2">Planning value2: </label>
 
-                            [% IF ( CGIsort2 ) %]
-                                [% CGIsort2 %]
-                            [% ELSE %]
-                                <input type="text" id="sort2" size="20" name="sort2" value="[% sort2 %]" />
+                        [% IF CGIsort2 %]
+                            <select id="sort2" size="1" name="sort1">
+                            [% FOREACH sort_opt IN CGIsort2 %]
+                                [% IF sort_opt.default %]
+                                    <option value="[% sort_opt.id %]" selected="selected">[% sort_opt.label %]</option>
+                                [% ELSE %]
+                                    <option value="[% sort_opt.id %]">[% sort_opt.label %]</option>
+                                [% END %]
                             [% END %]
+                            </select>
+                        [% ELSE %]
+                             <input type="text" id="sort2" size="20" name="sort2" value="[% sort2 %]" />
+                        [% END %]
                         </li>
                         <li>
                             
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/neworderempty.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/neworderempty.tt
index d6c8459..03c8863 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/neworderempty.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/neworderempty.tt
@@ -412,8 +412,16 @@ $(document).ready(function()
             <li><div class="hint">The 2 following fields are available for your own usage. They can be useful for statistical purposes</div>
                 <label for="sort1">Statistic 1: </label>
 
-                [% IF ( CGIsort1 ) %]
-                    [% CGIsort1 %]
+                [% IF CGIsort1 %]
+                    <select id="sort1" size="1" name="sort1">
+                    [% FOREACH sort_opt IN CGIsort1 %]
+                       [% IF sort_opt.default %]
+                          <option value="[% sort_opt.id %]" selected="selected">[% sort_opt.label %]</option>
+                        [% ELSE %]
+                          <option value="[% sort_opt.id %]">[% sort_opt.label %]</option>
+                        [% END %]
+                    [% END %]
+                    </select>
                 [% ELSE %]
 
                     <input type="text" id="sort1" size="20" name="sort1" value="[% sort1 %]" />
@@ -422,8 +430,16 @@ $(document).ready(function()
             <li>
                 <label for="sort2">Statistic 2: </label>
 
-                [% IF ( CGIsort2 ) %]
-                    [% CGIsort2 %]
+                [% IF CGIsort2 %]
+                    <select id="sort2" size="1" name="sort1">
+                    [% FOREACH sort_opt IN CGIsort2 %]
+                       [% IF sort_opt.default %]
+                          <option value="[% sort_opt.id %]" selected="selected">[% sort_opt.label %]</option>
+                        [% ELSE %]
+                          <option value="[% sort_opt.id %]">[% sort_opt.label %]</option>
+                        [% END %]
+                    [% END %]
+                    </select>
                 [% ELSE %]
                     <input type="text" id="sort2" size="20" name="sort2" value="[% sort2 %]" />
                 [% END %]
-- 
1.7.2.2



More information about the Koha-patches mailing list