From 0b9bbfc8092baabfdab0b410d6d9406aa9f884bb Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Tue, 26 May 2020 15:30:31 -0400 Subject: [PATCH] Bug 18086: Add FK constraints for branchcode and categorycode The overduerules table has no foreign key constraints to any for the related tables. The columns for branchcode, categorycode, letter1, letter2, and letter3 should all be foreign keys. Test Plan: 0) Create some overdue rules with invalid branchcodes and categorycodes 1) Apply this patch 2) Restart all the things! 3) Run updatedatabase.pl 4) Note the invalid rules are deleted 5) Note Default codes are NULL now instead of empty strings 6) Editor should behave as it did before --- Koha/OverdueRule.pm | 42 +++++++++++ Koha/OverdueRules.pm | 48 +++++++++++++ .../data/mysql/atomicupdate/bug_18086.perl | 4 ++ installer/data/mysql/kohastructure.sql | 2 +- tools/overduerules.pl | 69 +++++++++---------- 5 files changed, 127 insertions(+), 38 deletions(-) create mode 100644 Koha/OverdueRule.pm create mode 100644 Koha/OverdueRules.pm diff --git a/Koha/OverdueRule.pm b/Koha/OverdueRule.pm new file mode 100644 index 0000000000..59bba4239a --- /dev/null +++ b/Koha/OverdueRule.pm @@ -0,0 +1,42 @@ +package Koha::OverdueRule; + +# This file is part of Koha. +# +# Koha is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# Koha is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Koha; if not, see . + +use Modern::Perl; + +use Carp; + +use Koha::Database; + +use base qw(Koha::Object); + +=head1 NAME + +Koha::OverdueRule - Koha OverdueRule Object class + +=head1 API + +=head2 Class Methods + +=head3 type + +=cut + +sub _type { + return 'Overduerule'; +} + +1; diff --git a/Koha/OverdueRules.pm b/Koha/OverdueRules.pm new file mode 100644 index 0000000000..e602a69800 --- /dev/null +++ b/Koha/OverdueRules.pm @@ -0,0 +1,48 @@ +package Koha::OverdueRules; + +# This file is part of Koha. +# +# Koha is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# Koha is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Koha; if not, see . + +use Modern::Perl; + +use Carp; + +use Koha::Database; + +use Koha::OverdueRule; + +use base qw(Koha::Objects); + +=head1 NAME + +Koha::OverdueRules - Koha OverdueRule Object set class + +=head1 API + +=head2 Class Methods + +=head3 _type + +=cut + +sub _type { + return 'Overduerule'; +} + +sub object_class { + return 'Koha::OverdueRule'; +} + +1; diff --git a/installer/data/mysql/atomicupdate/bug_18086.perl b/installer/data/mysql/atomicupdate/bug_18086.perl index 448cb24378..8f5117c4d9 100644 --- a/installer/data/mysql/atomicupdate/bug_18086.perl +++ b/installer/data/mysql/atomicupdate/bug_18086.perl @@ -1,5 +1,8 @@ $DBversion = 'XXX'; # will be replaced by the RM if( CheckVersion( $DBversion ) ) { + $dbh->do(q{ + + }); $dbh->do(q{ DELETE overduerules FROM overduerules LEFT JOIN categories USING ( categorycode ) where categories.categorycode IS NULL; }); @@ -10,6 +13,7 @@ if( CheckVersion( $DBversion ) ) { $dbh->do(q{ ALTER TABLE overduerules + MODIFY `branchcode` varchar(10) NULL default NULL, ADD CONSTRAINT `fk_branchcode` FOREIGN KEY (`branchcode`) REFERENCES `branches` (`branchcode`) ON DELETE CASCADE ON UPDATE CASCADE, ADD CONSTRAINT `fk_categorycode` FOREIGN KEY (`categorycode`) REFERENCES `categories` (`categorycode`) ON DELETE CASCADE ON UPDATE CASCADE; }); diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql index 93ee28ef2f..dfb2456933 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -1224,7 +1224,7 @@ CREATE TABLE `oai_sets_biblios` ( DROP TABLE IF EXISTS `overduerules`; CREATE TABLE `overduerules` ( -- overdue notice status and triggers `overduerules_id` int(11) NOT NULL AUTO_INCREMENT, -- unique identifier for the overduerules - `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) + `branchcode` varchar(10) NULL default NULL, -- 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 diff --git a/tools/overduerules.pl b/tools/overduerules.pl index da028152c2..80c0cdc773 100755 --- a/tools/overduerules.pl +++ b/tools/overduerules.pl @@ -27,7 +27,7 @@ use C4::Letters; use C4::Members; use C4::Overdues; use Koha::Libraries; - +use Koha::OverdueRules; use Koha::Patron::Categories; our $input = new CGI; @@ -76,7 +76,7 @@ $branch = : C4::Context->preference('DefaultToLoggedInLibraryOverdueTriggers') ? C4::Context::mybranch() : Koha::Libraries->search->count() == 1 ? undef : undef; -$branch ||= q{}; +$branch ||= undef; my $op = $input->param('op'); $op ||= q{}; @@ -88,11 +88,6 @@ my %temphash; my $input_saved = 0; if ($op eq 'save') { my @names=$input->multi_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_insert_mtt = $dbh->prepare(" INSERT INTO overduerules_transport_types( overduerules_id, letternumber, message_transport_type @@ -159,33 +154,33 @@ if ($op eq 'save') { 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"}))) { - $sth_search->execute($branch,$bor); - my $res = $sth_search->fetchrow_hashref(); - if ($res->{'total'}>0) { - $sth_update->execute( - ($temphash{$bor}->{"delay1"}?$temphash{$bor}->{"delay1"}:undef), - ($temphash{$bor}->{"letter1"}?$temphash{$bor}->{"letter1"}:""), - ($temphash{$bor}->{"debarred1"}?$temphash{$bor}->{"debarred1"}:0), - ($temphash{$bor}->{"delay2"}?$temphash{$bor}->{"delay2"}:undef), - ($temphash{$bor}->{"letter2"}?$temphash{$bor}->{"letter2"}:""), - ($temphash{$bor}->{"debarred2"}?$temphash{$bor}->{"debarred2"}:0), - ($temphash{$bor}->{"delay3"}?$temphash{$bor}->{"delay3"}:undef), - ($temphash{$bor}->{"letter3"}?$temphash{$bor}->{"letter3"}:""), - ($temphash{$bor}->{"debarred3"}?$temphash{$bor}->{"debarred3"}:0), - $branch ,$bor - ); + my $overdue_rule = Koha::OverdueRules->find({ branchcode => $branch, categorycode => $bor }); + if ( $overdue_rule ) { + $overdue_rule->update({ + delay1 => ( $temphash{$bor}->{"delay1"} ? $temphash{$bor}->{"delay1"} : undef ), + letter1 => ( $temphash{$bor}->{"letter1"} ? $temphash{$bor}->{"letter1"} : "" ), + debarred1 => ( $temphash{$bor}->{"debarred1"} ? $temphash{$bor}->{"debarred1"} : 0 ), + delay2 => ( $temphash{$bor}->{"delay2"} ? $temphash{$bor}->{"delay2"} : undef ), + letter2 => ( $temphash{$bor}->{"letter2"} ? $temphash{$bor}->{"letter2"} : "" ), + debarred2 => ( $temphash{$bor}->{"debarred2"} ? $temphash{$bor}->{"debarred2"} : 0 ), + delay3 => ( $temphash{$bor}->{"delay3"} ? $temphash{$bor}->{"delay3"} : undef ), + letter3 => ( $temphash{$bor}->{"letter3"} ? $temphash{$bor}->{"letter3"} : "" ), + debarred3 => ( $temphash{$bor}->{"debarred3"} ? $temphash{$bor}->{"debarred3"} : 0 ), + }); } 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) - ); + $overdue_rule = Koha::OverdueRule->new({ + branchcode => $branch, + categorycode => $bor, + delay1 => ( $temphash{$bor}->{"delay1"} ? $temphash{$bor}->{"delay1"} : undef ), + letter1 => ( $temphash{$bor}->{"letter1"} ? $temphash{$bor}->{"letter1"} : "" ), + debarred1 => ( $temphash{$bor}->{"debarred1"} ? $temphash{$bor}->{"debarred1"} : 0 ), + delay2 => ( $temphash{$bor}->{"delay2"} ? $temphash{$bor}->{"delay2"} : undef ), + letter2 => ( $temphash{$bor}->{"letter2"} ? $temphash{$bor}->{"letter2"} : "" ), + debarred2 => ( $temphash{$bor}->{"debarred2"} ? $temphash{$bor}->{"debarred2"} : 0 ), + delay3 => ( $temphash{$bor}->{"delay3"} ? $temphash{$bor}->{"delay3"} : undef ), + letter3 => ( $temphash{$bor}->{"letter3"} ? $temphash{$bor}->{"letter3"} : "" ), + debarred3 => ( $temphash{$bor}->{"debarred3"} ? $temphash{$bor}->{"debarred3"} : 0 ), + })->store(); } $sth_delete_mtt->execute( $branch, $bor ); @@ -201,7 +196,8 @@ if ($op eq 'save') { } unless ($err) { for my $category_code (@rows_to_delete) { - $sth_delete->execute($branch, $category_code); + my $overdue_rule = Koha::OverdueRules->find({ branchcode => $branch, categorycode => $category_code }); + $overdue_rule->delete if $overdue_rule; } $template->param(datasaved => 1); $input_saved = 1; @@ -252,9 +248,8 @@ for my $patron_category (@patron_categories) { } } else { #getting values from table - my $sth2=$dbh->prepare("SELECT * from overduerules WHERE branchcode=? AND categorycode=?"); - $sth2->execute($branch,$patron_category->categorycode); - my $dat=$sth2->fetchrow_hashref; + my $overdue_rule = Koha::OverdueRules->find({ branchcode => $branch, categorycode => $patron_category->categorycode }); + my $dat = $overdue_rule ? $overdue_rule->unblessed : {}; for my $i ( 1..3 ){ my %row = ( overduename => $patron_category->categorycode, -- 2.24.1 (Apple Git-126)