From 6d939d6916419ff56eb4d68709a82d1cf4e9ee0f Mon Sep 17 00:00:00 2001 From: MJ Ray Date: Thu, 28 Feb 2013 15:35:55 +0000 Subject: [PATCH] Bug 9296 - restructure overduerules table to allow future extension The overduerules table is rather hardcoded to three notices. This patch restructures it so we could have more in future, but should not change anything noticeable yet. To test: 1) edit Tools: Overdue notices 2) cause some notices to be sent 3) notice how everything behaves the same as before Signed-off-by: Kyle M Hall --- C4/Overdues.pm | 15 +++++---- installer/data/mysql/kohastructure.sql | 16 ++++------ installer/data/mysql/updatedatabase.pl | 22 ++++++++++++++ misc/cronjobs/overdue_notices.pl | 50 +++++++++++++++++++++----------- tools/overduerules.pl | 32 +++++++++++++------- 5 files changed, 89 insertions(+), 46 deletions(-) diff --git a/C4/Overdues.pm b/C4/Overdues.pm index ac66c36..67f3364 100644 --- a/C4/Overdues.pm +++ b/C4/Overdues.pm @@ -1006,12 +1006,13 @@ C<$categorycode> contains the borrower categorycode sub GetOverdueDelays { my ($category) = @_; - my $query = qq|SELECT delay1,delay2,delay3 + my $query = qq|SELECT delay FROM overduerules - WHERE categorycode=?|; + WHERE categorycode=? + ORDER BY typecode ASC|; my $sth = C4::Context->dbh->prepare($query); $sth->execute($category); - my (@delays) = $sth->fetchrow_array; + my (@delays) = ($sth->fetchrow_array,$sth->fetchrow_array,$sth->fetchrow_array); return (@delays); } @@ -1025,7 +1026,7 @@ returns a list of branch codes for branches with overdue rules defined. sub GetBranchcodesWithOverdueRules { my $dbh = C4::Context->dbh; - my $rqoverduebranches = $dbh->prepare("SELECT DISTINCT branchcode FROM overduerules WHERE delay1 IS NOT NULL AND branchcode <> '' ORDER BY branchcode"); + my $rqoverduebranches = $dbh->prepare("SELECT DISTINCT branchcode FROM overduerules WHERE delay IS NOT NULL AND branchcode <> '' ORDER BY branchcode"); $rqoverduebranches->execute; my @branches = map { shift @$_ } @{ $rqoverduebranches->fetchall_arrayref }; if (!$branches[0]) { @@ -1086,11 +1087,11 @@ C<$notify_level> contains the notify level sub GetOverduerules { my ( $category, $notify_level ) = @_; my $dbh = C4::Context->dbh; - my $query = qq|SELECT debarred$notify_level + my $query = qq|SELECT debarred FROM overduerules - WHERE categorycode=?|; + WHERE categorycode=? AND typecode=?|; my $sth = $dbh->prepare($query); - $sth->execute($category); + $sth->execute($category,$notify_level); my ($overduerules) = $sth->fetchrow; return ($overduerules); } diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql index ed673da..c24bef4 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -1524,18 +1524,14 @@ CREATE TABLE `opac_news` ( -- data from the news tool DROP TABLE IF EXISTS `overduerules`; CREATE TABLE `overduerules` ( -- overdue notice status and triggers + `overduerules_id` mediumint NOT NULL AUTO_INCREMENT, + `delay` int(4) default NULL, -- number of days after the item is overdue that this notice is sent + `letter` varchar(20) default NULL, -- foreign key from the letter table to define which notice should be sent + `debarred` varchar(1) default 0, -- is the patron restricted when this notice is sent (1 for yes, 0 for no) + `typecode` varchar(10) NOT NULL default '1', -- 1 2 3 for first second third, may be extended later `branchcode` varchar(10) NOT NULL default '', -- foreign key from the branches table to define which branch this rule is for (if blank it's all libraries) `categorycode` varchar(10) NOT NULL default '', -- foreign key from the categories table to define which patron category this rule is for - `delay1` int(4) default NULL, -- number of days after the item is overdue that the first notice is sent - `letter1` varchar(20) default NULL, -- foreign key from the letter table to define which notice should be sent as the first notice - `debarred1` varchar(1) default 0, -- is the patron restricted when the first notice is sent (1 for yes, 0 for no) - `delay2` int(4) default NULL, -- number of days after the item is overdue that the second notice is sent - `debarred2` varchar(1) default 0, -- is the patron restricted when the second notice is sent (1 for yes, 0 for no) - `letter2` varchar(20) default NULL, -- foreign key from the letter table to define which notice should be sent as the second notice - `delay3` int(4) default NULL, -- number of days after the item is overdue that the third notice is sent - `letter3` varchar(20) default NULL, -- foreign key from the letter table to define which notice should be sent as the third notice - `debarred3` int(1) default 0, -- is the patron restricted when the third notice is sent (1 for yes, 0 for no) - PRIMARY KEY (`branchcode`,`categorycode`) + PRIMARY KEY (`overduerules_id`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8; -- diff --git a/installer/data/mysql/updatedatabase.pl b/installer/data/mysql/updatedatabase.pl index 7e5e243..55fe1f1 100755 --- a/installer/data/mysql/updatedatabase.pl +++ b/installer/data/mysql/updatedatabase.pl @@ -6444,6 +6444,28 @@ if ( CheckVersion($DBversion) ) { SetVersion($DBversion); } +$DBversion = "3.11.00.XXX"; +if ( C4::Context->preference("Version") < TransformToNum($DBversion) ) { + $dbh->do("ALTER TABLE overduerules RENAME TO old_overduerules"); + $dbh->do("CREATE TABLE `overduerules` ( -- overdue notice status and triggers + `overduerules_id` mediumint NOT NULL AUTO_INCREMENT, + `delay` int(4) default NULL, -- number of days after the item is overdue that this notice is sent + `letter` varchar(20) default NULL, -- foreign key from the letter table to define which notice should be sent + `debarred` varchar(1) default 0, -- is the patron restricted when this notice is sent (1 for yes, 0 for no) + `typecode` varchar(10) NOT NULL default '1', -- 1 2 3 for first second third, may be extended later + `branchcode` varchar(10) NOT NULL default '', -- foreign key from the branches table to define which branch this rule is for (if blank it's all libraries) + `categorycode` varchar(10) NOT NULL default '', -- foreign key from the categories table to define which patron category this rule is for + PRIMARY KEY (`overduerules_id`) +) ENGINE=InnoDB DEFAULT CHARSET=utf8; +"); + $dbh->do("INSERT INTO overduerules (delay, letter, debarred, typecode, branchcode, categorycode) SELECT delay1 AS delay, letter1 AS letter, debarred1 as debarred, '1' AS typecode, branchcode, categorycode FROM old_overduerules"); + $dbh->do("INSERT INTO overduerules (delay, letter, debarred, typecode, branchcode, categorycode) SELECT delay2 AS delay, letter2 AS letter, debarred2 as debarred, '2' AS typecode, branchcode, categorycode FROM old_overduerules"); + $dbh->do("INSERT INTO overduerules (delay, letter, debarred, typecode, branchcode, categorycode) SELECT delay3 AS delay, letter3 AS letter, debarred3 as debarred, '3' AS typecode, branchcode, categorycode FROM old_overduerules"); + print "Upgrade to $DBversion done (Restructure table overduerules for future expansion - you should check the conversion and DROP old_overduerules - that may be done automatically in a future version of Koha)\n"; + SetVersion($DBversion); +} + + =head1 FUNCTIONS diff --git a/misc/cronjobs/overdue_notices.pl b/misc/cronjobs/overdue_notices.pl index ba58e95..b144a44 100755 --- a/misc/cronjobs/overdue_notices.pl +++ b/misc/cronjobs/overdue_notices.pl @@ -409,36 +409,52 @@ SELECT biblio.*, items.*, issues.*, biblioitems.itemtype, TO_DAYS($date)-TO_DAYS AND TO_DAYS($date)-TO_DAYS(date_due) BETWEEN ? and ? END_SQL - my $query = "SELECT * FROM overduerules WHERE delay1 IS NOT NULL AND branchcode = ? "; + my $query = "SELECT * FROM overduerules WHERE delay IS NOT NULL AND typecode = ? AND branchcode = ? "; $query .= " AND categorycode IN (".join( ',' , ('?') x @myborcat ).") " if (@myborcat); $query .= " AND categorycode NOT IN (".join( ',' , ('?') x @myborcatout ).") " if (@myborcatout); my $rqoverduerules = $dbh->prepare($query); - $rqoverduerules->execute($branchcode, @myborcat, @myborcatout); + $rqoverduerules->execute(1, $branchcode, @myborcat, @myborcatout); + + my $trqoverduerules = $dbh->prepare($query); + $trqoverduerules->execute(2, $branchcode, @myborcat, @myborcatout); + + my $ttrqoverduerules = $dbh->prepare($query); + $ttrqoverduerules->execute(3, $branchcode, @myborcat, @myborcatout); # We get default rules is there is no rule for this branch if($rqoverduerules->rows == 0){ - $query = "SELECT * FROM overduerules WHERE delay1 IS NOT NULL AND branchcode = '' "; + $query = "SELECT * FROM overduerules WHERE delay IS NOT NULL AND typecode = ? AND branchcode = '' "; $query .= " AND categorycode IN (".join( ',' , ('?') x @myborcat ).") " if (@myborcat); $query .= " AND categorycode NOT IN (".join( ',' , ('?') x @myborcatout ).") " if (@myborcatout); $rqoverduerules = $dbh->prepare($query); - $rqoverduerules->execute(@myborcat, @myborcatout); + $rqoverduerules->execute(1, @myborcat, @myborcatout); + + $trqoverduerules = $dbh->prepare($query); + $trqoverduerules->execute(2, @myborcat, @myborcatout); + + $ttrqoverduerules = $dbh->prepare($query); + $ttrqoverduerules->execute(3, @myborcat, @myborcatout); } # my $outfile = 'overdues_' . ( $mybranch || $branchcode || 'default' ); - while ( my $overdue_rules = $rqoverduerules->fetchrow_hashref ) { + my @overdue_rules; + while ( $overdue_rules[1] = $rqoverduerules->fetchrow_hashref ) { + $overdue_rules[2] = $trqoverduerules->fetchrow_hashref; + $overdue_rules[3] = $ttrqoverduerules->fetchrow_hashref; + PERIOD: foreach my $i ( 1 .. 3 ) { $verbose and warn "branch '$branchcode', pass $i\n"; - my $mindays = $overdue_rules->{"delay$i"}; # the notice will be sent after mindays days (grace period) + my $mindays = $overdue_rules[$i]->{"delay"}; # the notice will be sent after mindays days (grace period) my $maxdays = ( - $overdue_rules->{ "delay" . ( $i + 1 ) } - ? $overdue_rules->{ "delay" . ( $i + 1 ) } - 1 + $overdue_rules[$i+1]->{ "delay" } + ? $overdue_rules[$i+1]->{ "delay" } - 1 : ($MAX) ); # issues being more than maxdays late are managed somewhere else. (borrower probably suspended) - if ( !$overdue_rules->{"letter$i"} ) { + if ( !$overdue_rules[$i]->{"letter"} ) { $verbose and warn "No letter$i code for branch '$branchcode'"; next PERIOD; } @@ -460,9 +476,9 @@ END_SQL $borrower_sql .= ' AND issues.branchcode=? '; push @borrower_parameters, $branchcode; } - if ( $overdue_rules->{categorycode} ) { + if ( $overdue_rules[$i]->{categorycode} ) { $borrower_sql .= ' AND borrowers.categorycode=? '; - push @borrower_parameters, $overdue_rules->{categorycode}; + push @borrower_parameters, $overdue_rules[$i]->{categorycode}; } $borrower_sql .= ' AND categories.overduenoticerequired=1 '; if($triggered) { @@ -476,7 +492,7 @@ END_SQL # $sth gets borrower info iff at least one overdue item has triggered the overdue action. my $sth = $dbh->prepare($borrower_sql); $sth->execute(@borrower_parameters); - $verbose and warn $borrower_sql . "\n $branchcode | " . $overdue_rules->{'categorycode'} . "\n ($mindays, $maxdays)\nreturns " . $sth->rows . " rows"; + $verbose and warn $borrower_sql . "\n $branchcode | " . $overdue_rules[$i]->{'categorycode'} . "\n ($mindays, $maxdays)\nreturns " . $sth->rows . " rows"; while ( my ( $borrowernumber, $firstname, $lastname, $address1, $address2, $city, $postcode, $country, $email @@ -499,17 +515,17 @@ END_SQL } } - my $letter = C4::Letters::getletter( 'circulation', $overdue_rules->{"letter$i"}, $branchcode ); + my $letter = C4::Letters::getletter( 'circulation', $overdue_rules[$i]->{"letter"}, $branchcode ); unless ($letter) { - $verbose and warn "Message '$overdue_rules->{letter$i}' content not found"; + $verbose and warn "Message '$overdue_rules[$i]->{letter}' content not found"; # might as well skip while PERIOD, no other borrowers are going to work. # FIXME : Does this mean a letter must be defined in order to trigger a debar ? next PERIOD; } - if ( $overdue_rules->{"debarred$i"} ) { + if ( $overdue_rules[$i]->{"debarred"} ) { #action taken is debarring C4::Members::DebarMember($borrowernumber, '9999-12-31'); @@ -538,7 +554,7 @@ END_SQL $sth2->finish; $letter = parse_letter( - { letter_code => $overdue_rules->{"letter$i"}, + { letter_code => $overdue_rules[$i]->{"letter"}, borrowernumber => $borrowernumber, branchcode => $branchcode, items => \@items, @@ -550,7 +566,7 @@ END_SQL } ); unless ($letter) { - $verbose and warn "Message '$overdue_rules->{letter$i}' content not found"; + $verbose and warn "Message '$overdue_rules[$i]->{letter}' content not found"; # might as well skip while PERIOD, no other borrowers are going to work. # FIXME : Does this mean a letter must be defined in order to trigger a debar ? diff --git a/tools/overduerules.pl b/tools/overduerules.pl index cc90d0b..b3abe0f 100755 --- a/tools/overduerules.pl +++ b/tools/overduerules.pl @@ -79,10 +79,10 @@ 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=?"); + my $sth_search = $dbh->prepare("SELECT count(*) AS total FROM overduerules WHERE branchcode=? AND categorycode=? AND typecode = '1'"); - 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_insert = $dbh->prepare("INSERT INTO overduerules (branchcode,categorycode, delay,letter,debarred, typecode) VALUES (?,?,?,?,?,'1'),(?,?,?,?,?,'2'),(?,?,?,?,?,'3')"); + my $sth_update=$dbh->prepare("UPDATE overduerules SET delay=?, letter=?, debarred=? WHERE branchcode=? AND categorycode=? AND typecode=?"); my $sth_delete=$dbh->prepare("DELETE FROM overduerules WHERE branchcode=? AND categorycode=?"); foreach my $key (@names){ # ISSUES @@ -145,22 +145,30 @@ if ($op eq 'save') { ($temphash{$bor}->{"delay1"}?$temphash{$bor}->{"delay1"}:undef), ($temphash{$bor}->{"letter1"}?$temphash{$bor}->{"letter1"}:""), ($temphash{$bor}->{"debarred1"}?$temphash{$bor}->{"debarred1"}:0), + $branch ,$bor, '1' + ); + $sth_update->execute( ($temphash{$bor}->{"delay2"}?$temphash{$bor}->{"delay2"}:undef), ($temphash{$bor}->{"letter2"}?$temphash{$bor}->{"letter2"}:""), ($temphash{$bor}->{"debarred2"}?$temphash{$bor}->{"debarred2"}:0), + $branch ,$bor, '2' + ); + $sth_update->execute( ($temphash{$bor}->{"delay3"}?$temphash{$bor}->{"delay3"}:undef), ($temphash{$bor}->{"letter3"}?$temphash{$bor}->{"letter3"}:""), ($temphash{$bor}->{"debarred3"}?$temphash{$bor}->{"debarred3"}:0), - $branch ,$bor + $branch ,$bor, '3' ); } 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), + $branch,$bor, ($temphash{$bor}->{"delay2"}?$temphash{$bor}->{"delay2"}:0), ($temphash{$bor}->{"letter2"}?$temphash{$bor}->{"letter2"}:""), ($temphash{$bor}->{"debarred2"}?$temphash{$bor}->{"debarred2"}:0), + $branch,$bor, ($temphash{$bor}->{"delay3"}?$temphash{$bor}->{"delay3"}:0), ($temphash{$bor}->{"letter3"}?$temphash{$bor}->{"letter3"}:""), ($temphash{$bor}->{"debarred3"}?$temphash{$bor}->{"debarred3"}:0) @@ -220,15 +228,15 @@ for my $data (@categories) { } } else { #getting values from table - my $sth2=$dbh->prepare("SELECT * from overduerules WHERE branchcode=? AND categorycode=?"); - $sth2->execute($branch,$data->{'categorycode'}); - my $dat=$sth2->fetchrow_hashref; - for (my $i=1;$i<=3;$i++){ + my $sth2=$dbh->prepare("SELECT * from overduerules WHERE branchcode=? AND categorycode=? AND typecode=?"); + foreach my $i (1,2,3) { + $sth2->execute($branch,$data->{'categorycode'},$i); + my $dat=$sth2->fetchrow_hashref; if ($countletters){ my @letterloop; foreach my $thisletter (sort { $letters->{$a} cmp $letters->{$b} } keys %$letters) { my $selected; - if ($dat->{"letter$i"} && $thisletter eq $dat->{"letter$i"}) { + if ($dat->{"letter"} && $thisletter eq $dat->{"letter"}) { $selected = 1; } my %letterrow =(value => $thisletter, @@ -240,10 +248,10 @@ for my $data (@categories) { $row{"letterloop$i"}=\@letterloop; } else { $row{"noletter"}=1; - if ($dat->{"letter$i"}){$row{"letter$i"}=$dat->{"letter$i"};} + if ($dat->{"letter"}){$row{"letter$i"}=$dat->{"letter"};} } - if ($dat->{"delay$i"}){$row{"delay$i"}=$dat->{"delay$i"};} - if ($dat->{"debarred$i"}){$row{"debarred$i"}=$dat->{"debarred$i"};} + if ($dat->{"delay"}){$row{"delay$i"}=$dat->{"delay"};} + if ($dat->{"debarred"}){$row{"debarred$i"}=$dat->{"debarred"};} } } push @line_loop,\%row; -- 1.7.2.5