Attachment 169455 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), 18.67 KB, created by Baptiste Wojtkowski (bwoj) on 2024-07-24 08:31:12 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Baptiste Wojtkowski (bwoj)

Created: 2024-07-24 08:31:12 UTC

Size: 18.67 KB

patch

obsolete

>From 3ef488e4dfcbd322b94259fb7b44a9a1f6b27b5d 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) Update DBIx classes
>5) Note the invalid rules are deleted
>6) Note Default codes are NULL now instead of empty strings
>7) Editor should behave as it did before
>---
> C4/Overdues.pm                                | 27 +++---
> Koha/OverdueRule.pm                           | 28 ++++++
> Koha/OverdueRules/TransportType.pm            | 42 +++++++++
> Koha/OverdueRules/TransportTypes.pm           | 52 +++++++++++
> .../data/mysql/atomicupdate/bug_18086.perl    | 38 ++++++++
> installer/data/mysql/kohastructure.sql        |  8 +-
> tools/overduerules.pl                         | 89 ++++++++-----------
> 7 files changed, 220 insertions(+), 64 deletions(-)
> 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 a2494f7..febca41 100644
>--- a/C4/Overdues.pm
>+++ b/C4/Overdues.pm
>@@ -765,18 +765,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 $current_rule = $rule->next ) {
>+        push @mtts, $current_rule->get_column('message_transport_type');
>     }
> 
>     # Put 'print' in first if exists
>diff --git a/Koha/OverdueRule.pm b/Koha/OverdueRule.pm
>index 16152a5..d60c66e 100644
>--- a/Koha/OverdueRule.pm
>+++ b/Koha/OverdueRule.pm
>@@ -21,6 +21,8 @@ use Modern::Perl;
> 
> use base qw(Koha::Object);
> 
>+use Koha::Database;
>+
> =head1 NAME
> 
> Koha::OverdueRule - Koha Overdue rule object class
>@@ -37,4 +39,30 @@ sub _type {
>     return 'Overduerule';
> }
> 
>+=head3 object_class
>+
>+=cut
>+
>+sub object_class {
>+    return 'Koha::OverdueRule';
>+}
>+
>+
>+
>+=head3 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);
>+}
>+
>+
> 1;
>diff --git a/Koha/OverdueRules/TransportType.pm b/Koha/OverdueRules/TransportType.pm
>new file mode 100644
>index 0000000..fd35ee1
>--- /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 0000000..9f1ef7f
>--- /dev/null
>+++ b/Koha/OverdueRules/TransportTypes.pm
>@@ -0,0 +1,52 @@
>+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';
>+}
>+
>+=head3 object_class
>+
>+=cut
>+
>+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 0000000..123dbd0
>--- /dev/null
>+++ b/installer/data/mysql/atomicupdate/bug_18086.perl
>@@ -0,0 +1,38 @@
>+$DBversion = 'XXX'; # will be replaced by the RM
>+if( CheckVersion( $DBversion ) ) {
>+    $dbh->do(q{
>+        UPDATE overduerules SET categorycode = NULL WHERE categorycode = "";
>+    });
>+
>+    $dbh->do(q{
>+        DELETE overduerules FROM overduerules
>+        LEFT JOIN categories USING ( categorycode )
>+        WHERE
>+            overduerules.categorycode IS NOT NULL
>+          AND
>+            categories.categorycode IS NULL;
>+    });
>+
>+    $dbh->do(q{
>+        UPDATE overduerules SET branchcode = NULL WHERE branchcode = "";
>+    });
>+
>+    $dbh->do(q{
>+        DELETE overduerules FROM overduerules
>+        LEFT JOIN branches USING ( branchcode )
>+        WHERE
>+            branches.branchcode IS NULL
>+          AND
>+            overduerules.branchcode IS NOT 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 14cef90..bbd72d4 100644
>--- a/installer/data/mysql/kohastructure.sql
>+++ b/installer/data/mysql/kohastructure.sql
>@@ -4962,8 +4962,8 @@ DROP TABLE IF EXISTS `overduerules`;
> /*!40101 SET character_set_client = utf8 */;
> CREATE TABLE `overduerules` (
>   `overduerules_id` int(11) NOT NULL AUTO_INCREMENT COMMENT 'unique identifier for the overduerules',
>-  `branchcode` varchar(10) NOT NULL DEFAULT '' COMMENT '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 '' COMMENT 'foreign key from the categories table to define which patron category this rule is for',
>+  `branchcode` varchar(10) COLLATE utf8mb4_unicode_ci NULL DEFAULT NULL COMMENT 'foreign key from the branches table to define which branch this rule is for (if blank it''s all libraries)',
>+  `categorycode` varchar(10) COLLATE utf8mb4_unicode_ci NOT NULL COMMENT 'foreign key from the categories table to define which patron category this rule is for',
>   `delay1` int(4) DEFAULT NULL COMMENT 'number of days after the item is overdue that the first notice is sent',
>   `letter1` varchar(20) DEFAULT NULL COMMENT 'foreign key from the letter table to define which notice should be sent as the first notice',
>   `debarred1` varchar(1) DEFAULT '0' COMMENT 'is the patron restricted when the first notice is sent (1 for yes, 0 for no)',
>@@ -4974,7 +4974,9 @@ CREATE TABLE `overduerules` (
>   `letter3` varchar(20) DEFAULT NULL COMMENT 'foreign key from the letter table to define which notice should be sent as the third notice',
>   `debarred3` int(1) DEFAULT 0 COMMENT '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;
> /*!40101 SET character_set_client = @saved_cs_client */;
> 
>diff --git a/tools/overduerules.pl b/tools/overduerules.pl
>index 4ebc6d4..30d75f2 100755
>--- a/tools/overduerules.pl
>+++ b/tools/overduerules.pl
>@@ -26,7 +26,8 @@ use C4::Letters;
> use C4::Members;
> use C4::Overdues qw( GetOverdueMessageTransportTypes );
> use Koha::Libraries;
>-
>+use Koha::OverdueRules;
>+use Koha::OverdueRules::TransportTypes;
> use Koha::Patron::Categories;
> 
> our $input = CGI->new;
>@@ -73,7 +74,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{};
>@@ -85,22 +86,6 @@ my %temphash;
> my $input_saved = 0;
> if ($op eq 'cud-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
>@@ -156,41 +141,45 @@ if ($op eq 'cud-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();
>                         }
>                     }
>                 }
>@@ -198,7 +187,8 @@ if ($op eq 'cud-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;
>@@ -247,9 +237,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.43.0

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