From 1a014da4c4d1aced8e0f18b53460af32290f0745 Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Wed, 15 Oct 2014 16:35:58 +0200 Subject: [PATCH] [PASSED QA] Bug 13084 [3.14.x]: Mixup of columns in deletedborrowers I could reproduce this problem in 3.14.4 where I wrote the patch. Fields shifted like branchcode moved to B_phone. Could not reproduce it for master. But this code change seems to improve things on master too. NOTE: The callers of this routine should use the return value to check, and not simply assume that it will be fine ;) Changing those places is out of the scope of this report. TEST PLAN --------- 1) Alter the order of your deletedborrowers table. e.g. alter table deletedborrowers change column privacy privacy int(11) after city; 2) Apply the 3.14.x test patch. 3) prove -v t/db_dependent/Members.t -- Will fail on the new test. 4) Apply the 3.14.x code change patch. 5) prove -v t/db_dependent/Members.t -- Will succeed on the new test. 6) run koha qa test tools. Signed-off-by: Mark Tompsett Signed-off-by: Kyle M Hall --- C4/Members.pm | 27 +++++++++++++++++---------- 1 files changed, 17 insertions(+), 10 deletions(-) diff --git a/C4/Members.pm b/C4/Members.pm index a0769ca..2aa77a9 100644 --- a/C4/Members.pm +++ b/C4/Members.pm @@ -1772,12 +1772,10 @@ sub GetSortDetails { $result = &MoveMemberToDeleted($borrowernumber); Copy the record from borrowers to deletedborrowers table. +The routine returns 1 for success, undef for failure. =cut -# FIXME: should do it in one SQL statement w/ subquery -# Otherwise, we should return the @data on success - sub MoveMemberToDeleted { my ($member) = shift or return; my $dbh = C4::Context->dbh; @@ -1786,13 +1784,22 @@ sub MoveMemberToDeleted { WHERE borrowernumber=?|; my $sth = $dbh->prepare($query); $sth->execute($member); - my @data = $sth->fetchrow_array; - (@data) or return; # if we got a bad borrowernumber, there's nothing to insert - $sth = - $dbh->prepare( "INSERT INTO deletedborrowers VALUES (" - . ( "?," x ( scalar(@data) - 1 ) ) - . "?)" ); - $sth->execute(@data); + my $data = $sth->fetchrow_hashref; + return if !$data; # probably bad borrowernumber + + #now construct a insert query that does not depend on the same order of + #columns in borrowers and deletedborrowers (see BZ 13084) + my $insertq = "INSERT INTO deletedborrowers ("; + my @values; + foreach my $key ( keys %$data ) { + $insertq.= $key.","; + push @values, $data->{$key}; + } + $insertq =~ s/,$//; #remove last comma + $insertq .= ") VALUES (" . ( "?," x ( scalar(@values) - 1 ) ) . "?)"; + $sth = $dbh->prepare( $insertq ); + $sth->execute(@values); + return $sth->err? undef: 1; } =head2 DelMember -- 1.7.2.5