From d2fe7f6634faf376dd7844db87e41ddafef2c31a 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 --- C4/Overdues.pm | 27 +++--- Koha/OverdueRule.pm | 56 ++++++++++++ Koha/OverdueRules.pm | 48 ++++++++++ Koha/OverdueRules/TransportType.pm | 42 +++++++++ Koha/OverdueRules/TransportTypes.pm | 48 ++++++++++ .../data/mysql/atomicupdate/bug_18086.perl | 20 +++++ installer/data/mysql/kohastructure.sql | 8 +- tools/overduerules.pl | 89 ++++++++----------- 8 files changed, 274 insertions(+), 64 deletions(-) create mode 100644 Koha/OverdueRule.pm create mode 100644 Koha/OverdueRules.pm create mode 100644 Koha/OverdueRules/TransportType.pm create mode 100644 Koha/OverdueRules/TransportTypes.pm create mode 100644 installer/data/mysql/atomicupdate/bug_18086.perl diff --git a/C4/Overdues.pm b/C4/Overdues.pm index a9de67d1ee..759d3c42b6 100644 --- a/C4/Overdues.pm +++ b/C4/Overdues.pm @@ -742,18 +742,23 @@ sub GetOverduesForBranch { sub GetOverdueMessageTransportTypes { my ( $branchcode, $categorycode, $letternumber ) = @_; return unless $categorycode and $letternumber; - my $dbh = C4::Context->dbh; - my $sth = $dbh->prepare(" - SELECT message_transport_type - FROM overduerules odr LEFT JOIN overduerules_transport_types ott USING (overduerules_id) - WHERE branchcode = ? - AND categorycode = ? - AND letternumber = ? - "); - $sth->execute( $branchcode, $categorycode, $letternumber ); + my $rule = Koha::OverdueRules->search( + { + branchcode => $branchcode, + categorycode => $categorycode, + letternumber => $letternumber, + }, + { + join => 'overduerules_transport_types', + } + )->next; + + return [] unless $rule; + + my $transport_types = $rule->transport_types; my @mtts; - while ( my $mtt = $sth->fetchrow ) { - push @mtts, $mtt; + while ( my $mtt = $transport_types->next ) { + push @mtts, $mtt->message_transport_type; } # Put 'print' in first if exists diff --git a/Koha/OverdueRule.pm b/Koha/OverdueRule.pm new file mode 100644 index 0000000000..e17bf1989c --- /dev/null +++ b/Koha/OverdueRule.pm @@ -0,0 +1,56 @@ +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 message_transport_types + +my $transport_types = $rule->transport_types; + +=cut + +sub transport_types { + my ( $self ) = @_; + + my $rs = $self->_result->overduerules_transport_types; + return unless $rs; + return Koha::OverdueRules::TransportTypes->_new_from_dbic($rs); +} + +=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/Koha/OverdueRules/TransportType.pm b/Koha/OverdueRules/TransportType.pm new file mode 100644 index 0000000000..fd35ee1a8c --- /dev/null +++ b/Koha/OverdueRules/TransportType.pm @@ -0,0 +1,42 @@ +package Koha::OverdueRules::TransportType; + +# 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::OverdueRules::TransportType - Koha OverdueRules::TransportType Object class + +=head1 API + +=head2 Class Methods + +=head3 _type + +=cut + +sub _type { + return 'OverduerulesTransportType'; +} + +1; diff --git a/Koha/OverdueRules/TransportTypes.pm b/Koha/OverdueRules/TransportTypes.pm new file mode 100644 index 0000000000..de0ba8997e --- /dev/null +++ b/Koha/OverdueRules/TransportTypes.pm @@ -0,0 +1,48 @@ +package Koha::OverdueRules::TransportTypes; + +# 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::OverdueRules::TransportType; + +use base qw(Koha::Objects); + +=head1 NAME + +Koha::OverdueRules::TransportTypes - Koha OverdueRules::TransportType Object set class + +=head1 API + +=head2 Class Methods + +=head3 _type + +=cut + +sub _type { + return 'OverduerulesTransportType'; +} + +sub object_class { + return 'Koha::OverdueRules::TransportType'; +} + +1; diff --git a/installer/data/mysql/atomicupdate/bug_18086.perl b/installer/data/mysql/atomicupdate/bug_18086.perl new file mode 100644 index 0000000000..b3e18dbdaa --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug_18086.perl @@ -0,0 +1,20 @@ +$DBversion = 'XXX'; # will be replaced by the RM +if( CheckVersion( $DBversion ) ) { + $dbh->do(q{ + DELETE overduerules FROM overduerules LEFT JOIN categories USING ( categorycode ) where categories.categorycode IS NULL; + }); + + $dbh->do(q{ + DELETE overduerules FROM overduerules LEFT JOIN branches USING ( branchcode ) where branches.branchcode IS NULL; + }); + + $dbh->do(q{ + ALTER TABLE overduerules + MODIFY `branchcode` varchar(10) NULL default NULL, + MODIFY `categorycode` varchar(10) NOT 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; + }); + + NewVersion( $DBversion, 18086, "Add FK constraints for branchcode and categorycode"); +} diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql index c5a3879662..a9b689dcd9 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -1224,8 +1224,8 @@ 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) - `categorycode` varchar(10) NOT NULL default '', -- foreign key from the categories table to define which patron category this rule is for + `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, -- 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) @@ -1236,7 +1236,9 @@ CREATE TABLE `overduerules` ( -- overdue notice status and triggers `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 (`overduerules_id`), - UNIQUE KEY `overduerules_branch_cat` (`branchcode`,`categorycode`) + UNIQUE KEY `overduerules_branch_cat` (`branchcode`,`categorycode`), + CONSTRAINT `fk_branchcode` FOREIGN KEY (`branchcode`) REFERENCES `branches` (`branchcode`) ON DELETE CASCADE ON UPDATE CASCADE, + CONSTRAINT `fk_categorycode` FOREIGN KEY (`categorycode`) REFERENCES `categories` (`categorycode`) ON DELETE CASCADE ON UPDATE CASCADE ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; -- Table structure for table `pending_offline_operations` diff --git a/tools/overduerules.pl b/tools/overduerules.pl index da028152c2..9b01316c8f 100755 --- a/tools/overduerules.pl +++ b/tools/overduerules.pl @@ -27,7 +27,8 @@ use C4::Letters; use C4::Members; use C4::Overdues; use Koha::Libraries; - +use Koha::OverdueRules; +use Koha::OverdueRules::TransportTypes; use Koha::Patron::Categories; our $input = new CGI; @@ -76,7 +77,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,22 +89,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 - ) VALUES ( - (SELECT overduerules_id FROM overduerules WHERE branchcode = ? AND categorycode = ?), ?, ? - ) - "); - my $sth_delete_mtt = $dbh->prepare(" - DELETE FROM overduerules_transport_types - WHERE overduerules_id = (SELECT overduerules_id FROM overduerules WHERE branchcode = ? AND categorycode = ?) - "); foreach my $key (@names){ # ISSUES @@ -159,41 +144,45 @@ 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 ); + Koha::OverdueRules::TransportTypes->search({ overduerules_id => $overdue_rule->id })->delete(); for my $letternumber ( 1..3 ) { my @mtt = $input->multi_param( "mtt${letternumber}-$bor" ); next unless @mtt; for my $mtt ( @mtt ) { - $sth_insert_mtt->execute( $branch, $bor, $letternumber, $mtt); + Koha::OverdueRules::TransportType->new({ + letternumber => $letternumber, + message_transport_type => $mtt, + overduerules_id => $overdue_rule->id, + })->store(); } } } @@ -201,7 +190,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 +242,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)