Bug 18242: [SOLUTION 2]Handle correctly move to old_issues

The table old_issues has a primary key defined on the issue_id column.
This issue_id comes from the issues table when an item is checked in.

In some case the value of issue_id already exists in the table

Basically this happens when an item is returned and mysqld is restarted:
The auto increment value for issues.issue_id will be reset to
MAX(issue_id)+1 (which is the value of the last entry of old_issues).
See also the description of bug 18003 for more informations.

In this solution the change is done at code level instead of DB
structure: If old_issues.issue_id already exists before moving from
the issues table, the issue_id is updated (not on cascade for
accountlines.issue_id, should it?) before the move.

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>

diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index af5acce..2dcb1e9 100644 (file)
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -2145,7 +2145,15 @@ sub MarkIssueReturned {
         die "Fatal error: the patron ($borrowernumber) has requested their circulation history be anonymized on check-in, but the AnonymousPatron system preference is empty or not set correctly."
             unless C4::Members::GetMember( borrowernumber => $anonymouspatron );
     }
+    my $database = Koha::Database->new();
+    my $schema   = $database->schema;
     my $dbh   = C4::Context->dbh;
+
+    my $issue_id = $dbh->selectrow_array(
+        q|SELECT issue_id FROM issues WHERE itemnumber = ?|,
+        undef, $itemnumber
+    );
+
     my $query = 'UPDATE issues SET returndate=';
     my @bind;
     if ($dropbox_branch) {
@@ -2159,34 +2167,44 @@ sub MarkIssueReturned {
     } else {
         $query .= ' now() ';
     }
-    $query .= ' WHERE  borrowernumber = ?  AND itemnumber = ?';
-    push @bind, $borrowernumber, $itemnumber;
-    # FIXME transaction
-    my $sth_upd  = $dbh->prepare($query);
-    $sth_upd->execute(@bind);
-    my $sth_copy = $dbh->prepare('INSERT INTO old_issues SELECT * FROM issues
-                                  WHERE borrowernumber = ?
-                                  AND itemnumber = ?');
-    $sth_copy->execute($borrowernumber, $itemnumber);
-    # anonymise patron checkout immediately if $privacy set to 2 and AnonymousPatron is set to a valid borrowernumber
-    if ( $privacy == 2) {
-        my $sth_ano = $dbh->prepare("UPDATE old_issues SET borrowernumber=?
-                                  WHERE borrowernumber = ?
-                                  AND itemnumber = ?");
-       $sth_ano->execute($anonymouspatron, $borrowernumber, $itemnumber);
-    }
-    my $sth_del  = $dbh->prepare("DELETE FROM issues
-                                  WHERE borrowernumber = ?
-                                  AND itemnumber = ?");
-    $sth_del->execute($borrowernumber, $itemnumber);
-
-    ModItem( { 'onloan' => undef }, undef, $itemnumber );
-
-    if ( C4::Context->preference('StoreLastBorrower') ) {
-        my $item = Koha::Items->find( $itemnumber );
-        my $patron = Koha::Patrons->find( $borrowernumber );
-        $item->last_returned_by( $patron );
-    }
+    $query .= ' WHERE issue_id = ?';
+    push @bind, $issue_id;
+
+    # FIXME Improve the return value and handle it from callers
+    $schema->txn_do(sub {
+        $dbh->do( $query, undef, @bind );
+
+        my $id_already_exists = $dbh->selectrow_array(
+            q|SELECT COUNT(*) FROM old_issues WHERE issue_id = ?|,
+            undef, $issue_id
+        );
+
+        if ( $id_already_exists ) {
+            my $new_issue_id = $dbh->selectrow_array(q|SELECT MAX(issue_id)+1 FROM old_issues|);
+            $dbh->do(
+                q|UPDATE issues SET issue_id = ? WHERE issue_id = ?|,
+                undef, $new_issue_id, $issue_id
+            );
+            $issue_id = $new_issue_id;
+        }
+
+        $dbh->do(q|INSERT INTO old_issues SELECT * FROM issues WHERE issue_id = ?|, undef, $issue_id);
+
+        # anonymise patron checkout immediately if $privacy set to 2 and AnonymousPatron is set to a valid borrowernumber
+        if ( $privacy == 2) {
+            $dbh->do(q|UPDATE old_issues SET borrowernumber=? WHERE issue_id = ?|, undef, $anonymouspatron, $issue_id);
+        }
+
+        $dbh->do(q|DELETE FROM issues WHERE issue_id = ?|, undef, $issue_id);
+
+        ModItem( { 'onloan' => undef }, undef, $itemnumber );
+
+        if ( C4::Context->preference('StoreLastBorrower') ) {
+            my $item = Koha::Items->find( $itemnumber );
+            my $patron = Koha::Patrons->find( $borrowernumber );
+            $item->last_returned_by( $patron );
+        }
+    });
 }
 
 =head2 _debar_user_on_return