bug 2293: better validation in overdue rules editor
If a rule should not be saved because a delay is given but no letter or debar action is specified for that delay, an error message is now reported. Also: * use "dialog alert" CSS class for confirmation that form was saved * if form input is saved, retrieve the settings from the database again when displaying the form - this will help turn up any bugs where the script is not saving the overdue rules correctly. * fix display of patron category in error messages Signed-off-by: Joshua Ferraro <jmf@liblime.com>
This commit is contained in:
parent
bf12bc05e1
commit
b8e404700a
|
|
@ -52,10 +52,13 @@ $(document).ready(function() {
|
|||
<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="bor"--> borrower category has some unexpected characters. There should be only numerical characters. </li>
|
||||
<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="bor"--> borrower category </li>
|
||||
<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>
|
||||
|
|
@ -67,7 +70,7 @@ $(document).ready(function() {
|
|||
<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 /><h3 align="center">INPUT SAVED</h3>
|
||||
<br /><div class="dialog message">INPUT SAVED</div>
|
||||
<!--/TMPL_IF -->
|
||||
</caption>
|
||||
<tr>
|
||||
|
|
|
|||
|
|
@ -25,6 +25,7 @@ use C4::Auth;
|
|||
use C4::Koha;
|
||||
use C4::Branch; # GetBranches
|
||||
use C4::Letters;
|
||||
use C4::Members;
|
||||
|
||||
my $input = new CGI;
|
||||
my $dbh = C4::Context->dbh;
|
||||
|
|
@ -48,6 +49,7 @@ my $err=0;
|
|||
|
||||
# save the values entered into tables
|
||||
my %temphash;
|
||||
my $input_saved = 0;
|
||||
if ($op eq 'save') {
|
||||
my @names=$input->param();
|
||||
my $sth_search = $dbh->prepare("SELECT count(*) AS total FROM overduerules WHERE branchcode=? AND categorycode=?");
|
||||
|
|
@ -65,21 +67,34 @@ if ($op eq 'save') {
|
|||
}
|
||||
}
|
||||
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);
|
||||
$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);
|
||||
$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);
|
||||
$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);
|
||||
$template->param("ERROR"=>1,"ERRORORDER"=>1,"BORERR"=>$bor_category_name);
|
||||
$err=1;
|
||||
}
|
||||
unless ($err){
|
||||
|
|
@ -117,7 +132,10 @@ if ($op eq 'save') {
|
|||
}
|
||||
}
|
||||
}
|
||||
unless ($err) {$template->param(datasaved=>1);}
|
||||
unless ($err) {
|
||||
$template->param(datasaved=>1);
|
||||
$input_saved = 1;
|
||||
}
|
||||
}
|
||||
my $branches = GetBranches();
|
||||
my @branchloop;
|
||||
|
|
@ -149,7 +167,11 @@ while (my $data=$sth->fetchrow_hashref){
|
|||
toggle => $toggle,
|
||||
line => $data->{'description'}
|
||||
);
|
||||
if (%temphash){
|
||||
if (%temphash and 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"};
|
||||
|
|
|
|||
Loading…
Reference in New Issue