Attachment 105387 Details for Bug 18086 – Bug 18086: Add FK constraints for branchcode and categorycode

[patch] Bug 18086: Add FK constraints for branchcode and categorycode

Bug-18086-Add-FK-constraints-for-branchcode-and-ca.patch (text/plain), 14.48 KB, created by Kyle M Hall (khall) on 2020-05-27 10:44:58 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Kyle M Hall (khall)

Created: 2020-05-27 10:44:58 UTC

Size: 14.48 KB

patch

obsolete

>From b70e5550040efc471328a184e5018ccc0c4fc183 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
>---
> Koha/OverdueRule.pm                           | 42 +++++++++++
> Koha/OverdueRules.pm                          | 48 +++++++++++++
> .../data/mysql/atomicupdate/bug_18086.perl    | 20 ++++++
> installer/data/mysql/kohastructure.sql        |  8 ++-
> tools/overduerules.pl                         | 69 +++++++++----------
> 5 files changed, 147 insertions(+), 40 deletions(-)
> create mode 100644 Koha/OverdueRule.pm
> create mode 100644 Koha/OverdueRules.pm
> create mode 100644 installer/data/mysql/atomicupdate/bug_18086.perl
>
>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 <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 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/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..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)

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