Attachment 84785 Details for Bug 20581 – Bug 20581: (follow-up) Do not use PK for AV FK

[patch] Bug 20581: (follow-up) Do not use PK for AV FK

Bug-20581-follow-up-Do-not-use-PK-for-AV-FK.patch (text/plain), 7.53 KB, created by Andrew Isherwood on 2019-02-05 14:36:40 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Andrew Isherwood

Created: 2019-02-05 14:36:40 UTC

Size: 7.53 KB

patch

obsolete

>From d43bb259562758ce7fe2556bd8b4f7ccab017a1c Mon Sep 17 00:00:00 2001
>From: Andrew Isherwood <andrew.isherwood@ptfs-europe.com>
>Date: Tue, 5 Feb 2019 14:10:36 +0000
>Subject: [PATCH] Bug 20581: (follow-up) Do not use PK for AV FK
>
>As per:
>https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c133
>
>We switch from using authorised_values.id when creating the foreign key
>illrequests.status_alias, we now use authorised_values.authorised_value
>
>I have added a migration from using id -> authorised_value, so when
>existing users of this bug get this version, their DB migrates nicely
>instead of just breaking.
>---
> ill/ill-requests.pl                                |  2 +-
> ...81-add_new_illrequests_status_alias_column.perl | 31 ++++++++++++++++++++--
> installer/data/mysql/kohastructure.sql             |  4 +--
> .../prog/en/modules/ill/ill-requests.tt            |  8 +++---
> 4 files changed, 36 insertions(+), 9 deletions(-)
>
>diff --git a/ill/ill-requests.pl b/ill/ill-requests.pl
>index bf951058e1..acbdda29c8 100755
>--- a/ill/ill-requests.pl
>+++ b/ill/ill-requests.pl
>@@ -174,7 +174,7 @@ if ( $backends_available ) {
>             $request->price_paid($params->{price_paid});
>             $request->notesopac($params->{notesopac});
>             $request->notesstaff($params->{notesstaff});
>-            my $alias = ($params->{status_alias} =~ /\d/) ?
>+            my $alias = ($params->{status_alias}) ?
>                 $params->{status_alias} :
>                 undef;
>             $request->status_alias($alias);
>diff --git a/installer/data/mysql/atomicupdate/bug_20581-add_new_illrequests_status_alias_column.perl b/installer/data/mysql/atomicupdate/bug_20581-add_new_illrequests_status_alias_column.perl
>index 881d2cd13e..442cddbe07 100644
>--- a/installer/data/mysql/atomicupdate/bug_20581-add_new_illrequests_status_alias_column.perl
>+++ b/installer/data/mysql/atomicupdate/bug_20581-add_new_illrequests_status_alias_column.perl
>@@ -2,10 +2,37 @@ $DBversion = 'XXX';  # will be replaced by the RM
> if( CheckVersion( $DBversion ) ) {
> 
>     if ( !column_exists( 'illrequests', 'status_alias' ) ) {
>-        $dbh->do( "ALTER TABLE illrequests ADD COLUMN status_alias integer DEFAULT NULL AFTER status" );
>+        # Fresh upgrade, just add the column and constraint
>+        $dbh->do( "ALTER TABLE illrequests ADD COLUMN status_alias varchar(80) DEFAULT NULL AFTER status" );
>+    } else {
>+        # Migrate all existing foreign keys from referencing authorised_values.id
>+        # to referencing authorised_values.authorised_value
>+        # First remove the foreign key constraint and index
>+        if ( foreign_key_exists( 'illrequests', 'illrequests_safk' ) ) {
>+            $dbh->do( "ALTER TABLE illrequests DROP FOREIGN KEY illrequests_safk");
>+        }
>+        if ( index_exists( 'illrequests', 'illrequests_safk' ) ) {
>+            $dbh->do( "DROP INDEX illrequests_safk IN illrequests" );
>+        }
>+        # Now change the illrequests.status_alias column definition from int to varchar
>+        $dbh->do( "ALTER TABLE illrequests MODIFY COLUMN status_alias varchar(80)" );
>+        # Now replace all references to authorised_values.id with their
>+        # corresponding authorised_values.authorised_value
>+        my $sth = $dbh->prepare( "SELECT illrequest_id, status_alias FROM illrequests WHERE status_alias IS NOT NULL" );
>+        $sth->execute();
>+        while (my @row = $sth->fetchrow_array()) {
>+            my $r_id = $row[0];
>+            my $av_id = $row[1];
>+            # Get the authorised value's authorised_value value
>+            my ($av_val) = $dbh->selectrow_array( "SELECT authorised_value FROM authorised_values WHERE id = ?", {}, $av_id );
>+            # Now update illrequests.status_alias
>+            if ($av_val) {
>+                $dbh->do( "UPDATE illrequests SET status_alias = ? WHERE illrequest_id = ?", {}, ($av_val, $r_id) );
>+            }
>+        }
>     }
>     if ( !foreign_key_exists( 'illrequests', 'illrequests_safk' ) ) {
>-        $dbh->do( "ALTER TABLE illrequests ADD CONSTRAINT illrequests_safk FOREIGN KEY (status_alias) REFERENCES authorised_values(id) ON UPDATE CASCADE ON DELETE SET NULL" );
>+        $dbh->do( "ALTER TABLE illrequests ADD CONSTRAINT illrequests_safk FOREIGN KEY (status_alias) REFERENCES authorised_values(authorised_value) ON UPDATE CASCADE ON DELETE SET NULL" );
>     }
>     $dbh->do( "INSERT IGNORE INTO authorised_value_categories SET category_name = 'ILLSTATUS'");
> 
>diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql
>index 325ec2f421..9b245e0c33 100644
>--- a/installer/data/mysql/kohastructure.sql
>+++ b/installer/data/mysql/kohastructure.sql
>@@ -4171,7 +4171,7 @@ CREATE TABLE illrequests (
>     biblio_id integer DEFAULT NULL,             -- Potential bib linked to request
>     branchcode varchar(50) NOT NULL,            -- The branch associated with the request
>     status varchar(50) DEFAULT NULL,            -- Current Koha status of request
>-    status_alias integer DEFAULT NULL,          -- Foreign key to relevant authorised_values.id
>+    status_alias varchar(80) DEFAULT NULL,      -- Foreign key to relevant authorised_values.authorised_value
>     placed date DEFAULT NULL,                   -- Date the request was placed
>     replied date DEFAULT NULL,                  -- Last API response
>     updated timestamp DEFAULT CURRENT_TIMESTAMP -- Last modification to request
>@@ -4195,7 +4195,7 @@ CREATE TABLE illrequests (
>       ON UPDATE CASCADE ON DELETE CASCADE,
>     CONSTRAINT `illrequests_safk`
>       FOREIGN KEY (`status_alias`)
>-      REFERENCES `authorised_values` (`id`)
>+      REFERENCES `authorised_values` (`authorised_value`)
>       ON UPDATE CASCADE ON DELETE SET NULL
> ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci;
> 
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/ill/ill-requests.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/ill/ill-requests.tt
>index b124e8bda9..4adffa4382 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/ill/ill-requests.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/ill/ill-requests.tt
>@@ -204,10 +204,10 @@
>                                             [% request.capabilities.$stat.name | html %]
>                                         </option>
>                                         [% FOREACH alias IN AuthorisedValues.Get('ILLSTATUS') %]
>-                                            [% IF alias.id == current_alias %]
>-                                            <option value="[% alias.id | html %]" selected>
>+                                            [% IF alias.authorised_value == current_alias %]
>+                                            <option value="[% alias.authorised_value | html %]" selected>
>                                             [% ELSE %]
>-                                            <option value="[% alias.id | html %]">
>+                                            <option value="[% alias.authorised_value | html %]">
>                                             [% END %]
>                                                 [% alias.lib | html %]
>                                             </option>
>@@ -874,7 +874,7 @@
>             // Get our data from the API and process it prior to passing
>             // it to datatables
>             var ajax = $.ajax(
>-                '/api/v1/illrequests?embed=metadata,patron,capabilities,library'
>+                '/api/v1/illrequests?embed=metadata,patron,capabilities,library,status_alias'
>                 ).done(function() {
>                     var data = JSON.parse(ajax.responseText);
>                     // Make a copy, we'll be removing columns next and need
>-- 
>2.11.0

Actions: View | Diff | Splinter Review

Attachments on bug 20581: 74323 | 74324 | 74325 | 74326 | 74327 | 74362 | 75333 | 75334 | 75810 | 77819 | 77820 | 77821 | 77822 | 77823 | 77824 | 77825 | 77826 | 78376 | 78377 | 78378 | 78379 | 78380 | 78381 | 78382 | 78383 | 78384 | 78431 | 78515 | 78516 | 78517 | 78518 | 78519 | 78520 | 78521 | 78522 | 78523 | 80279 | 80280 | 80281 | 80282 | 80283 | 80284 | 80285 | 80286 | 80474 | 80475 | 80501 | 80503 | 80505 | 80598 | 80599 | 80600 | 82063 | 82064 | 82065 | 82066 | 82067 | 82068 | 82069 | 82070 | 82071 | 82072 | 82073 | 82074 | 82075 | 82076 | 82443 | 82444 | 82445 | 82446 | 82447 | 82448 | 82449 | 82450 | 82451 | 82452 | 82453 | 82454 | 82455 | 82456 | 82457 | 82458 | 82459 | 82460 | 82461 | 82462 | 82463 | 82464 | 82465 | 82466 | 82467 | 82468 | 82472 | 82483 | 82484 | 82485 | 82486 | 82487 | 82488 | 82489 | 82490 | 82491 | 82492 | 82493 | 82494 | 82495 | 82496 | 82733 | 84202 | 84404 | 84768 | 84770 | 84771 | 84772 | 84773 | 84774 | 84775 | 84776 | 84777 | 84778 | 84779 | 84780 | 84781 | 84782 | 84783 | 84784 | 84785 | 84791 | 84814 | 84818 | 84819 | 84844 | 84911 | 84912 | 84913 | 84914 | 84915 | 84916 | 84917 | 84918 | 84919 | 84920 | 84921 | 84922 | 84923 | 84924 | 84925 | 84926 | 84927 | 84928 | 84929 | 84930 | 84931 | 84964 | 85075 | 85076 | 85077 | 85078 | 85079 | 85080 | 85081 | 85082 | 85083 | 85084 | 85085 | 85086 | 85087 | 85088 | 85089 | 85090 | 85091 | 85092 | 85093 | 85094 | 85095 | 85096