From 1dc92809ba9a5d5d8912998780ea978fe157c63d Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Thu, 9 Mar 2017 16:58:17 -0300 Subject: [PATCH] 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. --- C4/Circulation.pm | 74 ++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 46 insertions(+), 28 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index af5acce..2dcb1e9 100644 --- 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 -- 2.9.3