From 812d26fb394010cfa91bf0d9f152bb0e6895f451 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 Content-Type: text/plain; charset=utf-8 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 | 21 +++++++++++++ misc/cronjobs/overdue_notices.pl | 50 +++++++++++++++++++++----------- tools/overduerules.pl | 32 +++++++++++++------- 5 files changed, 88 insertions(+), 46 deletions(-) diff --git a/C4/Overdues.pm b/C4/Overdues.pm index 9c05668..0a4c815 100644 --- a/C4/Overdues.pm +++ b/C4/Overdues.pm @@ -971,12 +971,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); } @@ -990,7 +991,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]) { @@ -1051,11 +1052,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 ca2f2b2..f51843d 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -1625,18 +1625,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 f3a90d8..52cf98e 100755 --- a/installer/data/mysql/updatedatabase.pl +++ b/installer/data/mysql/updatedatabase.pl @@ -6983,6 +6983,27 @@ INSERT INTO systempreferences (variable,value,explanation,options,type) VALUES ( SetVersion($DBversion); } +$DBversion = "3.13.00.XXX"; +if ( CheckVersion($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 5f285c0..826c07a 100755 --- a/misc/cronjobs/overdue_notices.pl +++ b/misc/cronjobs/overdue_notices.pl @@ -410,36 +410,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; } @@ -461,9 +477,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) { @@ -477,7 +493,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 $data = $sth->fetchrow_hashref ) { my $borrowernumber = $data->{'borrowernumber'}; @@ -502,17 +518,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'); @@ -541,7 +557,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, @@ -553,7 +569,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.7.6