From 5fe707bdf0066c5c3af20eebbe23ce8bfe3f5599 Mon Sep 17 00:00:00 2001 From: MJ Ray Date: Mon, 17 Dec 2012 02:09:38 +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 --- C4/Overdues.pm | 15 ++++++----- installer/data/mysql/kohastructure.sql | 16 ++++------- installer/data/mysql/updatedatabase.pl | 24 ++++++++++++++++++ misc/cronjobs/overdue_notices.pl | 41 +++++++++++++++++++++---------- tools/overduerules.pl | 32 +++++++++++++++--------- 5 files changed, 86 insertions(+), 42 deletions(-) diff --git a/C4/Overdues.pm b/C4/Overdues.pm index 913e0d3..b9bba09 100644 --- a/C4/Overdues.pm +++ b/C4/Overdues.pm @@ -982,12 +982,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); } @@ -1001,7 +1002,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]) { @@ -1062,11 +1063,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 8648e18..573fa9e 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -1498,18 +1498,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 9dbbada..841ceb6 100755 --- a/installer/data/mysql/updatedatabase.pl +++ b/installer/data/mysql/updatedatabase.pl @@ -5291,6 +5291,30 @@ if ( C4::Context->preference("Version") < TransformToNum($DBversion) ) { } + +$DBversion = "3.09.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 =head2 TableExists($table) diff --git a/misc/cronjobs/overdue_notices.pl b/misc/cronjobs/overdue_notices.pl index c93f59b..bd698ed 100755 --- a/misc/cronjobs/overdue_notices.pl +++ b/misc/cronjobs/overdue_notices.pl @@ -385,36 +385,51 @@ SELECT biblio.*, items.*, issues.*, biblioitems.itemtype, TO_DAYS(NOW())-TO_DAYS AND TO_DAYS(NOW())-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 ) { + while ( my $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; } @@ -438,7 +453,7 @@ END_SQL } if ( $overdue_rules->{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) { @@ -452,7 +467,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 @@ -460,7 +475,7 @@ END_SQL { $verbose and warn "borrower $firstname, $lastname ($borrowernumber) has items triggering level $i."; - if ( $overdue_rules->{"debarred$i"} ) { + if ( $overdue_rules[$i]->{"debarred"} ) { #action taken is debarring C4::Members::DebarMember($borrowernumber, '9999-12-31'); @@ -489,7 +504,7 @@ END_SQL $sth2->finish; my $letter = parse_letter( - { letter_code => $overdue_rules->{"letter$i"}, + { letter_code => $overdue_rules[$i]->{"letter"}, borrowernumber => $borrowernumber, branchcode => $branchcode, items => \@items, diff --git a/tools/overduerules.pl b/tools/overduerules.pl index 478a762..caf1768 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