Bug 13084 [Master] - Mixup of columns in deletedborrowers
authorMarcel de Rooy <m.de.rooy@rijksmuseum.nl>
Wed, 15 Oct 2014 14:35:58 +0000 (16:35 +0200)
committerTomas Cohen Arazi <tomascohen@gmail.com>
Thu, 23 Oct 2014 13:40:53 +0000 (10:40 -0300)
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.

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>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

C4/Members.pm

index 3cb0b1e..da8dfaf 100644 (file)
@@ -1808,12 +1808,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;
@@ -1822,13 +1820,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