Bugzilla – Attachment 105551 Details for
Bug 18086
overduerules table is lacking foreign key constraints
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 18086: Add FK constraints for branchcode and categorycode
Bug-18086-Add-FK-constraints-for-branchcode-and-ca.patch (text/plain), 20.12 KB, created by
Kyle M Hall (khall)
on 2020-06-04 13:34:07 UTC
(
hide
)
Description:
Bug 18086: Add FK constraints for branchcode and categorycode
Filename:
MIME Type:
Creator:
Kyle M Hall (khall)
Created:
2020-06-04 13:34:07 UTC
Size:
20.12 KB
patch
obsolete
>From d2fe7f6634faf376dd7844db87e41ddafef2c31a Mon Sep 17 00:00:00 2001 >From: Kyle M Hall <kyle@bywatersolutions.com> >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 <http://www.gnu.org/licenses>. >+ >+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 <http://www.gnu.org/licenses>. >+ >+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 <http://www.gnu.org/licenses>. >+ >+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 <http://www.gnu.org/licenses>. >+ >+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)
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 18086
:
105368
|
105369
|
105380
|
105382
|
105387
|
105388
|
105424
|
105551
|
106201
|
134730
|
134731
|
168810
|
168811
|
168812
|
168813
|
168814
|
168815
|
169287
|
169288
|
169289
|
169290
|
169291
|
169292
|
169293
|
169294
|
169295
|
169296
|
169297
|
169298
|
169299
|
169300
|
169301
|
169302
|
169396
|
169397
|
169398
|
169399
|
169400
|
169401
|
169402
|
169403
|
169404
|
169455
|
169456
|
169457
|
169458
|
169459
|
169460
|
169461
|
169462
|
169463
|
169543
|
169544