[Koha-patches] [PATCH] kohabug 2491 Improving error trapping and communication w/user

Chris Nighswonger chris.nighswonger at liblime.com
Sat Aug 23 22:00:12 CEST 2008


This patch adds code to indicate why a rule is not saved when the "Delay" field is left
blank. Form validation in general is improved. It also corrects a condition which caused
the form not to display current rules for the selected branch. It also adds the ability
to delete a rule.

This patch also adds the 'use warnings' pragma.

Developer note: Perhaps form validation should be done with js or ajax.

Documentation note: Docs will need to be updated as the template has changed to add a
"Delete" checkbox column to the rules table when displaying existing rules.
---
 .../prog/en/modules/tools/overduerules.tmpl        |   59 ++--
 tools/overduerules.pl                              |  403 +++++++++++++-------
 2 files changed, 303 insertions(+), 159 deletions(-)

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/overduerules.tmpl b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/overduerules.tmpl
index 690642b..b746171 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/overduerules.tmpl
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/overduerules.tmpl
@@ -29,7 +29,7 @@ $(document).ready(function() {
         <h1>Defining <!-- TMPL_IF NAME="branch" -->overdue actions for <!-- TMPL_VAR NAME="branch" --><!-- TMPL_ELSE -->default overdue actions<!-- /TMPL_IF --></h1>
         <div class="help">
             <p>Delay is the number of days after an issue is due before an action is triggered. </p>
-            <p>If you want Koha to trigger an action (send a letter or debar member), a delay value is required.</p>
+            <p>A delay value is required.</p>
             <p>Columns must be filled left to right: if the first column is blank, other columns will be ignored. </p>
         </div>
             <form method="post" action="/cgi-bin/koha/tools/overduerules.pl" id="selectlibrary">
@@ -46,22 +46,29 @@ $(document).ready(function() {
                 </select>
                 <input type="submit" value="Select" />
             </form>
-            <!--TMPL_IF Name="ERROR"-->
-            <div class="dialog alert">
-                <h3>Data error</h3>
-                <p>The following fields have a forbidden value. Correct them and press OK again :</p>
-                <ul>
-                <!-- TMPL_IF NAME="ERRORDELAY" -->
-                    <li>Delay <!--TMPL_VAR Name="ERRORDELAY"--> for <!--TMPL_VAR Name="BORERR"--> borrower category has some unexpected characters. There should be only numerical characters. </li>
-                <!-- /TMPL_IF -->
-                <!-- TMPL_IF NAME="ERRORUSELESSDELAY" -->
-                    <li>No letter or debar action specified for delay <!--TMPL_VAR Name="ERRORUSELESSDELAY"--> for <!--TMPL_VAR Name="BORERR"--> borrower category.  If a delay is supplied, either a letter, debar action, or both should be specified.</li>
-                <!-- /TMPL_IF -->
-                <!-- TMPL_IF NAME="ERRORORDER" -->
-                    <li>Delay1 should be less than Delay2 which should be less than Delay3 for <!--TMPL_VAR Name="BORERR"--> borrower category </li>
-                <!-- /TMPL_IF -->
-                </ul>
-            </div>
+            <!-- TMPL_IF NAME="error" -->
+                <div class="dialog alert">
+                    <h3>Error</h3>
+                    <p>The following errors have been detected. Please correct them and press <b>Save Changes</b> again :</p>
+                <!-- TMPL_LOOP NAME="error" -->
+                    <ul>
+                    <!-- TMPL_IF NAME="ERRORNODELAY" -->
+                        <li>No delay has been set for the first action of the <!--TMPL_VAR Name="BORERR"--> borrower category. You must define a first action prior to defining subsequent actions. </li>
+                    <!-- TMPL_ELSIF NAME="ERRORDELAY" -->
+                        <li>The <!--TMPL_VAR Name="ERRORDELAY"--> for <!--TMPL_VAR Name="BORERR"--> borrower category has some unexpected characters. There should be only numerical characters. </li>
+                    <!-- TMPL_ELSIF NAME="ERRORUSELESSDELAY" -->
+                        <li>No letter or debar action specified for the <!--TMPL_VAR Name="ERRORUSELESSDELAY"--> for <!--TMPL_VAR Name="BORERR"--> borrower category.  If a delay is supplied, either a letter, debar action, or both should be specified.</li>
+                    <!-- TMPL_ELSIF NAME="ERRORORDER" -->
+                        <li>The first delay should be less than the second delay which should be less than the third delay for <!--TMPL_VAR Name="BORERR"--> borrower category </li>
+                    <!-- TMPL_ELSIF NAME="ERRORNODATA" -->
+                        <li>All fields are blank. Please specify at least one delay and an associated letter and/or debar action.</li>
+                    <!-- /TMPL_IF -->
+                    </ul>
+                <!-- /TMPL_LOOP -->
+                </div>
+            <!--/TMPL_IF -->
+            <!--TMPL_IF Name="datasaved"-->
+                <br /><div class="dialog message"><h3>Rules Successfully Updated/Saved</h3></div>
             <!--/TMPL_IF -->
             <!-- TMPL_IF name="table" -->
             <form method="post" action="/cgi-bin/koha/tools/overduerules.pl">
@@ -69,12 +76,9 @@ $(document).ready(function() {
                 <input type="hidden" name="branch" value="<!-- TMPL_VAR NAME="branch" -->" />
                 <table>
                     <caption>Rules for overdue actions: <!--TMPL_IF Name="branch"--><!-- TMPL_VAR NAME="branch" --><!--TMPL_ELSE--> default library <!--/TMPL_IF-->
-                    <!--TMPL_IF Name="datasaved"-->
-                        <br /><div class="dialog message">INPUT SAVED</div>
-                    <!--/TMPL_IF -->
                     </caption>
                     <tr>
-                        <th rowspan="2">&nbsp;</th><th colspan="3" scope="col">First</th><th colspan="3" scope="col">Second</th><th colspan="3" scope="col">Third</th>
+                        <th rowspan="2">&nbsp;</th><!-- TMPL_IF NAME="displayingrules" --><th rowspan="2">Delete Rule</th><!-- /TMPL_IF --><th colspan="3" scope="col">First</th><th colspan="3" scope="col">Second</th><th colspan="3" scope="col">Third</th>
                     </tr>
                     <tr>
                         <th scope="col">Delay</th><th scope="col">Letter</th><th scope="col">Debar</th><th scope="col">Delay</th><th scope="col">Letter</th><th scope="col">Debar</th><th scope="col">Delay</th><th scope="col">Letter</th><th scope="col">Debar</th>
@@ -86,9 +90,10 @@ $(document).ready(function() {
                                 <tr>
                             <!-- /TMPL_IF -->
                             <th scope="row"><!-- TMPL_VAR NAME="line" --></th>
-<td>
-                                <input name="delay1-<!-- TMPL_VAR NAME="overduename" -->" size="5" value="<!-- TMPL_VAR NAME="delay1" -->" />
-                            </td>
+                            <!-- TMPL_IF NAME="displayingrules" -->
+                            <td align="center"><input type="checkbox" <!-- TMPL_UNLESS NAME="rule" -->disabled="disabled" <!-- /TMPL_UNLESS -->name="delete1-<!-- TMPL_VAR NAME="overduename" -->" value="1" /></td>
+                            <!-- /TMPL_IF -->
+                            <td><input name="delay1-<!-- TMPL_VAR NAME="overduename" -->" size="5" value="<!-- TMPL_VAR NAME="delay1" -->" /></td>
 <td>
                             <!--TMPL_IF Name="noletter" -->
                                 <input type="text" name="letter1-<!-- TMPL_VAR NAME="overduename" -->" value="<!--TMPL_VAR Name="letter1"-->" />
@@ -105,7 +110,7 @@ $(document).ready(function() {
                                 </select>
                             <!--/TMPL_IF -->
                             </td>
-<td>
+<td align="center">
                                 <!-- TMPL_IF NAME="debarred1" -->
                                     <input type="checkbox" name="debarred1-<!-- TMPL_VAR NAME="overduename" -->" checked="checked" value="1" />
                                 <!-- TMPL_ELSE -->
@@ -131,7 +136,7 @@ $(document).ready(function() {
                                 </select>
                             <!--/TMPL_IF -->
                             </td>
-<td>
+<td align="center">
                                 <!-- TMPL_IF NAME="debarred2" -->
                                     <input type="checkbox" name="debarred2-<!-- TMPL_VAR NAME="overduename" -->" checked="checked" value="1" />
                                 <!-- TMPL_ELSE -->
@@ -157,7 +162,7 @@ $(document).ready(function() {
                                 </select>
                             <!--/TMPL_IF -->
                             </td>
-<td>
+<td align="center">
                                 <!-- TMPL_IF NAME="debarred3" -->
                                     <input type="checkbox" name="debarred3-<!-- TMPL_VAR NAME="overduename" -->" checked="checked" value="1" />
                                 <!-- TMPL_ELSE -->
diff --git a/tools/overduerules.pl b/tools/overduerules.pl
index 03fee7b..5a56e55 100755
--- a/tools/overduerules.pl
+++ b/tools/overduerules.pl
@@ -18,6 +18,7 @@
 # Suite 330, Boston, MA  02111-1307 USA
 
 use strict;
+use warnings;  #aka "fix bugs" :-)
 use CGI;
 use C4::Context;
 use C4::Output;
@@ -29,182 +30,316 @@ use C4::Members;
 
 my $input = new CGI;
 my $dbh = C4::Context->dbh;
-
-my $type=$input->param('type');
-my $branch = $input->param('branch');
-$branch="" unless $branch;
+my $type = $input->param('type');
+my $branch = $input->param('branch') ? $input->param('branch') : '';
 my $op = $input->param('op');
-
-# my $flagsrequired;
-# $flagsrequired->{circulation}=1;
-my ($template, $loggedinuser, $cookie)
-    = get_template_and_user({template_name => "tools/overduerules.tmpl",
-                            query => $input,
-                            type => "intranet",
-                            authnotrequired => 0,
-                            flagsrequired => {parameters => 1, tools => 'edit_notice_status_triggers'},
-                            debug => 1,
-                            });
-my $err=0;
+my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
+    {
+        template_name   => "tools/overduerules.tmpl",
+        query           => $input,
+        type            => "intranet",
+        authnotrequired => 0,
+        flagsrequired   =>
+          { parameters => 1, tools => 'edit_notice_status_triggers' },
+        debug           => 1,
+    }
+);
 
 # save the values entered into tables
-my %temphash;
+my $formdata = {} if $op eq 'save';
 my $input_saved = 0;
-if ($op eq 'save') {
+my $err = 0;
+my @errors = ();
+if ( $op eq 'save' ) {
     my @names=$input->param();
     my $sth_search = $dbh->prepare("SELECT count(*) AS total FROM overduerules WHERE branchcode=? AND categorycode=?");
-
     my $sth_insert = $dbh->prepare("INSERT INTO overduerules (branchcode,categorycode, delay1,letter1,debarred1, delay2,letter2,debarred2, delay3,letter3,debarred3) VALUES (?,?,?,?,?,?,?,?,?,?,?)");
-    my $sth_update=$dbh->prepare("UPDATE overduerules SET delay1=?, letter1=?, debarred1=?, delay2=?, letter2=?, debarred2=?, delay3=?, letter3=?, debarred3=? WHERE branchcode=? AND categorycode=?");
-    my $sth_delete=$dbh->prepare("DELETE FROM overduerules WHERE branchcode=? AND categorycode=?");
+    my $sth_update = $dbh->prepare("UPDATE overduerules SET delay1=?, letter1=?, debarred1=?, delay2=?, letter2=?, debarred2=?, delay3=?, letter3=?, debarred3=? WHERE branchcode=? AND categorycode=?");
+    my $sth_delete = $dbh->prepare("DELETE FROM overduerules WHERE branchcode=? AND categorycode=?");
     foreach my $key (@names){
-            # ISSUES
-            if ($key =~ /(.*)([1-3])-(.*)/) {
-                    my $type = $1; # data type
-                    my $num = $2; # From 1 to 3
-                    my $bor = $3; # borrower category
-                    $temphash{$bor}->{"$type$num"}=$input->param("$key") if (($input->param("$key") ne "") or ($input->param("$key")>0));
+        if ( $key =~ /(.*)([1-3])-(.*)/ ) {
+            my $type = $1; # data type
+            my $num = $2; # From 1 to 3
+            my $bor = $3; # borrower category
+            $formdata->{$bor}->{"$type$num"}=$input->param("$key") if (($input->param("$key") ne "") or ($input->param("$key") gt "0"));
+        }
+    }
+#   FIXME Would form validation be better done w/js or ajax?
+#   If $formdata is empty, the form was submitted empty...
+    if ( not keys %$formdata ) {
+        push(
+            @errors,
+            {
+                ERROR       => 1,
+                ERRORNODATA => 1,
             }
+        );
     }
-    foreach my $bor (keys %temphash){
-        # get category name if we need it for an error message
-        my $bor_category = GetBorrowercategory($bor);
-        my $bor_category_name = defined($bor_category) ? $bor_category->{description} : $bor;
-
-        # Do some Checking here : delay1 < delay2 <delay3 all of them being numbers
-        # Raise error if not true
-        if ($temphash{$bor}->{delay1}=~/[^0-9]/ and $temphash{$bor}->{delay1} ne ""){
-            $template->param("ERROR"=>1,"ERRORDELAY"=>"delay1","BORERR"=>$bor_category_name);
-            $err=1;
-        } elsif ($temphash{$bor}->{delay2}=~/[^0-9]/ and $temphash{$bor}->{delay2} ne ""){
-            $template->param("ERROR"=>1,"ERRORDELAY"=>"delay2","BORERR"=>$bor_category_name);
-            $err=1;
-        } elsif ($temphash{$bor}->{delay3}=~/[^0-9]/ and $temphash{$bor}->{delay3} ne ""){
-            $template->param("ERROR"=>1,"ERRORDELAY"=>"delay3","BORERR"=>$bor_category_name);
-            $err=1;
-        } elsif ($temphash{$bor}->{delay1} and not ($temphash{$bor}->{"letter1"} or $temphash{$bor}->{"debarred1"})) {
-            $template->param("ERROR"=>1,"ERRORUSELESSDELAY"=>"delay1","BORERR"=>$bor_category_name);
-            $err=1;
-        } elsif ($temphash{$bor}->{delay2} and not ($temphash{$bor}->{"letter2"} or $temphash{$bor}->{"debarred2"})) {
-            $template->param("ERROR"=>1,"ERRORUSELESSDELAY"=>"delay2","BORERR"=>$bor_category_name);
-            $err=1;
-        } elsif ($temphash{$bor}->{delay3} and not ($temphash{$bor}->{"letter3"} or $temphash{$bor}->{"debarred3"})) {
-            $template->param("ERROR"=>1,"ERRORUSELESSDELAY"=>"delay3","BORERR"=>$bor_category_name);
-            $err=1;
-        }elsif ($temphash{$bor}->{delay3} and
-                ($temphash{$bor}->{delay3}<=$temphash{$bor}->{delay2} or $temphash{$bor}->{delay3}<=$temphash{$bor}->{delay1})
-                or $temphash{$bor}->{delay2} and ($temphash{$bor}->{delay2}<=$temphash{$bor}->{delay1})){
-                    $template->param("ERROR"=>1,"ERRORORDER"=>1,"BORERR"=>$bor_category_name);
-                        $err=1;
+    else {
+        VALIDATEFIELDS:
+        foreach my $bor ( keys %$formdata ) {
+            my $bor_category = GetBorrowercategory($bor);
+            my $bor_category_name = $bor_category->{description};
+#       Is there a value for delay1...
+            if ( not defined($formdata->{$bor}->{delay1}) ) {
+                unshift(
+                    @errors,
+                    {
+                        ERROR      => 1,
+                        ERRORNODELAY => 1,
+                        BORERR     => $bor_category_name,
+                    }
+                );
+                $err = 1;
+                next VALIDATEFIELDS;
+            }
+#       Are all delays numbers...
+            elsif ( $formdata->{$bor}->{delay1} !~ m/[0-9]+?/ ) {
+                unshift(
+                    @errors,
+                    {
+                        ERROR      => 1,
+                        ERRORDELAY => "first delay",
+                        BORERR     => $bor_category_name,
+                    }
+                );
+                $err = 1;
+                next VALIDATEFIELDS;
+            }
+            elsif ( defined($formdata->{$bor}->{delay2}) and $formdata->{$bor}->{delay2} !~ m/[0-9]+?/ ) {
+                unshift(
+                    @errors,
+                    {
+                        ERROR      => 1,
+                        ERRORDELAY => "second delay",
+                        BORERR     => $bor_category_name,
+                    }
+                );
+                $err = 1;
+                next VALIDATEFIELDS;
+            }
+            elsif ( defined($formdata->{$bor}->{delay3}) and $formdata->{$bor}->{delay3} !~ m/[0-9]+?/ ) {
+                unshift(
+                    @errors,
+                    {
+                        ERROR      => 1,
+                        ERRORDELAY => "third delay",
+                        BORERR     => $bor_category_name,
+                    }
+                );
+                $err = 1;
+                next VALIDATEFIELDS;
+            }
+#       Is delay1 < delay2 <delay3...
+            elsif (
+                ( $formdata->{$bor}->{delay3}
+                and (  $formdata->{$bor}->{delay3} <= $formdata->{$bor}->{delay2}
+                    or $formdata->{$bor}->{delay3} <= $formdata->{$bor}->{delay1} ) )
+                or ( $formdata->{$bor}->{delay2}
+                and ( $formdata->{$bor}->{delay2} <= $formdata->{$bor}->{delay1} ) )
+            )
+            {
+                unshift(
+                    @errors,
+                    {
+                        ERROR      => 1,
+                        ERRORORDER => 1,
+                        BORERR     => $bor_category_name,
+                    }
+                );
+                $err = 1;
+                next VALIDATEFIELDS;
+            }
+#       Check to see if delays have actions assigned
+            elsif (
+                $formdata->{$bor}->{delay1}
+                and not( $formdata->{$bor}->{"letter1"}
+                    or $formdata->{$bor}->{"debarred1"} )
+            )
+            {
+                unshift(
+                    @errors,
+                    {
+                        ERROR             => 1,
+                        ERRORUSELESSDELAY => "first delay",
+                        BORERR            => $bor_category_name,
+                    }
+                );
+                $err = 1;
+                next VALIDATEFIELDS;
+            }
+            elsif (
+                $formdata->{$bor}->{delay2}
+                and not( $formdata->{$bor}->{"letter2"}
+                    or $formdata->{$bor}->{"debarred2"} )
+            )
+            {
+                unshift(
+                    @errors,
+                    {
+                        ERROR             => 1,
+                        ERRORUSELESSDELAY => "second delay",
+                        BORERR            => $bor_category_name,
+                    }
+                );
+                $err = 1;
+                next VALIDATEFIELDS;
+            }
+            elsif (
+                $formdata->{$bor}->{delay3}
+                and not( $formdata->{$bor}->{"letter3"}
+                    or $formdata->{$bor}->{"debarred3"} )
+            )
+            {
+                unshift(
+                    @errors,
+                    {
+                        ERROR             => 1,
+                        ERRORUSELESSDELAY => "third delay",
+                        BORERR            => $bor_category_name,
+                    }
+                );
+                $err = 1;
+                next VALIDATEFIELDS;
+            }
         }
-        unless ($err){
-            if (($temphash{$bor}->{delay1} and ($temphash{$bor}->{"letter1"} or $temphash{$bor}->{"debarred1"}))
-                or ($temphash{$bor}->{delay2} and ($temphash{$bor}->{"letter2"} or $temphash{$bor}->{"debarred2"}))
-                or ($temphash{$bor}->{delay3} and ($temphash{$bor}->{"letter3"} or $temphash{$bor}->{"debarred3"}))) {
+#       If no errors, save/update this rule...
+        if ( !$err ){
+            foreach my $bor ( keys %$formdata ) {
+                if ($formdata->{$bor}->{delete1}) {
+                    $sth_delete->execute($branch,$bor);
+                }
+                else {
                     $sth_search->execute($branch,$bor);
                     my $res = $sth_search->fetchrow_hashref();
-                    if ($res->{'total'}>0) {
+                    if ($res->{'total'} > 0) {
                         $sth_update->execute(
-                            ($temphash{$bor}->{"delay1"}?$temphash{$bor}->{"delay1"}:0),
-                            ($temphash{$bor}->{"letter1"}?$temphash{$bor}->{"letter1"}:""),
-                            ($temphash{$bor}->{"debarred1"}?$temphash{$bor}->{"debarred1"}:0),
-                            ($temphash{$bor}->{"delay2"}?$temphash{$bor}->{"delay2"}:0),
-                            ($temphash{$bor}->{"letter2"}?$temphash{$bor}->{"letter2"}:""),
-                            ($temphash{$bor}->{"debarred2"}?$temphash{$bor}->{"debarred2"}:0),
-                            ($temphash{$bor}->{"delay3"}?$temphash{$bor}->{"delay3"}:0),
-                            ($temphash{$bor}->{"letter3"}?$temphash{$bor}->{"letter3"}:""),
-                            ($temphash{$bor}->{"debarred3"}?$temphash{$bor}->{"debarred3"}:0),
-                            $branch ,$bor
-                            );
-                    } else {
+                            $formdata->{$bor}->{"delay1"},
+                            ($formdata->{$bor}->{"letter1"}?$formdata->{$bor}->{"letter1"}:""),
+                            ($formdata->{$bor}->{"debarred1"}?$formdata->{$bor}->{"debarred1"}:0),
+                            ($formdata->{$bor}->{"delay2"}?$formdata->{$bor}->{"delay2"}:0),
+                            ($formdata->{$bor}->{"letter2"}?$formdata->{$bor}->{"letter2"}:""),
+                            ($formdata->{$bor}->{"debarred2"}?$formdata->{$bor}->{"debarred2"}:0),
+                            ($formdata->{$bor}->{"delay3"}?$formdata->{$bor}->{"delay3"}:0),
+                            ($formdata->{$bor}->{"letter3"}?$formdata->{$bor}->{"letter3"}:""),
+                            ($formdata->{$bor}->{"debarred3"}?$formdata->{$bor}->{"debarred3"}:0),
+                            $branch,
+                            $bor,
+                        );
+                    }
+                    else {
                         $sth_insert->execute($branch,$bor,
-                            ($temphash{$bor}->{"delay1"}?$temphash{$bor}->{"delay1"}:0),
-                            ($temphash{$bor}->{"letter1"}?$temphash{$bor}->{"letter1"}:""),
-                            ($temphash{$bor}->{"debarred1"}?$temphash{$bor}->{"debarred1"}:0),
-                            ($temphash{$bor}->{"delay2"}?$temphash{$bor}->{"delay2"}:0),
-                            ($temphash{$bor}->{"letter2"}?$temphash{$bor}->{"letter2"}:""),
-                            ($temphash{$bor}->{"debarred2"}?$temphash{$bor}->{"debarred2"}:0),
-                            ($temphash{$bor}->{"delay3"}?$temphash{$bor}->{"delay3"}:0),
-                            ($temphash{$bor}->{"letter3"}?$temphash{$bor}->{"letter3"}:""),
-                            ($temphash{$bor}->{"debarred3"}?$temphash{$bor}->{"debarred3"}:0)
-                            );
+                            $formdata->{$bor}->{"delay1"},
+                            ($formdata->{$bor}->{"letter1"}?$formdata->{$bor}->{"letter1"}:""),
+                            ($formdata->{$bor}->{"debarred1"}?$formdata->{$bor}->{"debarred1"}:0),
+                            ($formdata->{$bor}->{"delay2"}?$formdata->{$bor}->{"delay2"}:0),
+                            ($formdata->{$bor}->{"letter2"}?$formdata->{$bor}->{"letter2"}:""),
+                            ($formdata->{$bor}->{"debarred2"}?$formdata->{$bor}->{"debarred2"}:0),
+                            ($formdata->{$bor}->{"delay3"}?$formdata->{$bor}->{"delay3"}:0),
+                            ($formdata->{$bor}->{"letter3"}?$formdata->{$bor}->{"letter3"}:""),
+                            ($formdata->{$bor}->{"debarred3"}?$formdata->{$bor}->{"debarred3"}:0)
+                        );
                     }
                 }
+            }
+            $template->param( datasaved => 1 );
+            $input_saved = 1;
         }
     }
-    unless ($err) {
-        $template->param(datasaved=>1);
-        $input_saved = 1;
-    }
 }
+
 my $branches = GetBranches();
-my @branchloop;
-foreach my $thisbranch (sort { $branches->{$a}->{branchname} cmp $branches->{$b}->{branchname} } keys %$branches) {
-        my $selected = 1 if $thisbranch eq $branch;
-        my %row =(value => $thisbranch,
-                                selected => $selected,
-                                branchname => $branches->{$thisbranch}->{'branchname'},
-                        );
-        push @branchloop, \%row;
+my @branchloop = ();
+foreach my $thisbranch (
+    sort { $branches->{$a}->{branchname} cmp $branches->{$b}->{branchname} }
+    keys %$branches )
+{
+    my $selected = 1 if $thisbranch eq $branch;
+    my %row = (
+        value      => $thisbranch,
+        selected   => $selected,
+        branchname => $branches->{$thisbranch}->{'branchname'},
+    );
+    push @branchloop, \%row;
 }
 
-my $letters = GetLetters("circulation");
+my $sth=$dbh->prepare("SELECT description,categorycode FROM categories WHERE overduenoticerequired > 0 ORDER BY description");
+$sth->execute;
 
+my $letters = GetLetters("circulation");
 my $countletters = scalar $letters;
-
-my $sth=$dbh->prepare("SELECT description,categorycode FROM categories WHERE overduenoticerequired>0 ORDER BY description");
-$sth->execute;
-my @line_loop;
-my $toggle= 1;
-# my $i=0;
-while (my $data=$sth->fetchrow_hashref){
+my @line_loop = ();
+my $toggle = 1;
+my $displaying_rules = 0;
+while (my $data = $sth->fetchrow_hashref){
     if ( $toggle eq 1 ) {
         $toggle = 0;
     } else {
         $toggle = 1;
     }
-    my %row = ( overduename => $data->{'categorycode'},
-                toggle => $toggle,
-                line => $data->{'description'}
-                );
-    if (%temphash and not $input_saved){
+    my %row = (
+        overduename => $data->{'categorycode'},
+        toggle      => $toggle,
+        line        => $data->{'description'},
+    );
+    if ( not $input_saved ) {
         # if we managed to save the form submission, don't
         # reuse %temphash, but take the values from the
         # database - this makes it easier to identify
         # bugs where the form submission was not correctly saved
         for (my $i=1;$i<=3;$i++){
-            $row{"delay$i"}=$temphash{$data->{'categorycode'}}->{"delay$i"};
-            $row{"debarred$i"}=$temphash{$data->{'categorycode'}}->{"debarred$i"};
+            $row{"delay$i"}=$formdata->{$data->{'categorycode'}}->{"delay$i"};
+            $row{"debarred$i"}=$formdata->{$data->{'categorycode'}}->{"debarred$i"};
             if ($countletters){
-                my @letterloop;
-                foreach my $thisletter (sort { $letters->{$a} cmp $letters->{$b} } keys %$letters) {
-                    my $selected = 1 if $thisletter eq $temphash{$data->{'categorycode'}}->{"letter$i"};
-                    my %letterrow =(value => $thisletter,
-                                    selected => $selected,
-                                    lettername => $letters->{$thisletter},
-                                    );
+                my @letterloop = ();
+                foreach
+                  my $thisletter ( sort { $letters->{$a} cmp $letters->{$b} }
+                    keys %$letters )
+                {
+                    my $selected = 1
+                      if (
+                        defined(
+                            $formdata->{ $data->{'categorycode'} }->{"letter$i"}
+                        )
+                        and $thisletter eq
+                        $formdata->{ $data->{'categorycode'} }->{"letter$i"}
+                      );
+                    my %letterrow = (
+                        value      => $thisletter,
+                        selected   => $selected,
+                        lettername => $letters->{$thisletter},
+                    );
                     push @letterloop, \%letterrow;
                 }
                 $row{"letterloop$i"}=\@letterloop;
             } else {
                 $row{"noletter"}=1;
-                $row{"letter$i"}=$temphash{$data->{'categorycode'}}->{"letter$i"};
+                $row{"letter$i"}=$formdata->{$data->{'categorycode'}}->{"letter$i"};
             }
         }
-    } else {
+    }
+    else {
     #getting values from table
-        my $sth2=$dbh->prepare("SELECT * from overduerules WHERE branchcode=? AND categorycode=?");
+        my $sth2 = $dbh->prepare("SELECT * from overduerules WHERE branchcode=? AND categorycode=?");
         $sth2->execute($branch,$data->{'categorycode'});
-        my $dat=$sth2->fetchrow_hashref;
+        my $dat = $sth2->fetchrow_hashref;
+        if (defined($dat)) {
+            $displaying_rules = 1;
+            $row{"rule"} = 1;
+        }
         for (my $i=1;$i<=3;$i++){
             if ($countletters){
-                my @letterloop;
-                foreach my $thisletter (sort { $letters->{$a} cmp $letters->{$b} } keys %$letters) {
-                    my $selected = 1 if $thisletter eq $dat->{"letter$i"};
-                    my %letterrow =(value => $thisletter,
-                                    selected => $selected,
-                                    lettername => $letters->{$thisletter},
-                                    );
+                my @letterloop = ();
+                foreach
+                  my $thisletter ( sort { $letters->{$a} cmp $letters->{$b} }
+                    keys %$letters )
+                {
+                    my $selected = 1 if ( defined $dat->{"letter$i"} and $thisletter eq $dat->{"letter$i"} );
+                    my %letterrow = (
+                        value      => $thisletter,
+                        selected   => $selected,
+                        lettername => $letters->{$thisletter},
+                    );
                     push @letterloop, \%letterrow;
                 }
                 $row{"letterloop$i"}=\@letterloop;
@@ -221,7 +356,11 @@ while (my $data=$sth->fetchrow_hashref){
 }
 $sth->finish;
 
-$template->param(table=> \@line_loop,
-                branchloop => \@branchloop,
-                branch => $branch);
+$template->param(
+    table               => \@line_loop,
+    branchloop          => \@branchloop,
+    branch              => $branch,
+    displayingrules     => $displaying_rules,
+    error               => \@errors,
+);
 output_html_with_http_headers $input, $cookie, $template->output;
-- 
1.5.5.GIT




More information about the Koha-patches mailing list