From 613efc063135121d1ac821257fbc5656477ac161 Mon Sep 17 00:00:00 2001
From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Date: Wed, 15 Oct 2014 16:35:58 +0200
Subject: [PATCH] Bug 13084: Mixup of columns in deletedborrowers (3.14
 version)
Content-Type: text/plain; charset=utf-8

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 <mtompset@hotmail.com>
---
 C4/Members.pm |   27 +++++++++++++++++----------
 1 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/C4/Members.pm b/C4/Members.pm
index b04cc64..26d510b 100644
--- a/C4/Members.pm
+++ b/C4/Members.pm
@@ -1774,12 +1774,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;
@@ -1788,13 +1786,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.7.6