Bugzilla – Attachment 70879 Details for
Bug 20086
AddRenewal is not executed as a transaction and can results in partial success and doubled fines
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 20086 - AddRenewal is not executed as a transaction and can results in partial success and doubled fines
Bug-20086---AddRenewal-is-not-executed-as-a-transa.patch (text/plain), 5.56 KB, created by
Nick Clemens (kidclamp)
on 2018-01-24 14:24:07 UTC
(
hide
)
Description:
Bug 20086 - AddRenewal is not executed as a transaction and can results in partial success and doubled fines
Filename:
MIME Type:
Creator:
Nick Clemens (kidclamp)
Created:
2018-01-24 14:24:07 UTC
Size:
5.56 KB
patch
obsolete
>From c25f2e1c32f412bb89b6e35f6bbf00ba3a7d9503 Mon Sep 17 00:00:00 2001 >From: Nick Clemens <nick@bywatersolutions.com> >Date: Wed, 24 Jan 2018 14:20:34 +0000 >Subject: [PATCH] Bug 20086 - AddRenewal is not executed as a transaction and > can results in partial success and doubled fines > >This patch starts a transaction and only commits if renewal and fines >updates and charges are successful (partial in any cna be problematic) > >There is no feedback (as currently there is none either) but if part >fails, all fails. > >I didn't include stats and notifications in the transaction, but we >could. > >To test: >1 - Apply patch >2 - prove t/db_dependent/Circulation.t >3 - Attempt circs and renewals should be no difference >4 - If possible make part of transaction fail and ensure all fails >--- > C4/Circulation.pm | 77 +++++++++++++++++++++++++++++++++++-------------------- > 1 file changed, 49 insertions(+), 28 deletions(-) > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index 2b7722c..cbcb6bf 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -2808,6 +2808,7 @@ sub AddRenewal { > my $item_object = Koha::Items->find( $itemnumber ); # Should replace $item > my $biblio = $item_object->biblio; > >+ my $schema = Koha::Database->new->schema; > my $dbh = C4::Context->dbh; > > # Find the issues record for this book >@@ -2825,10 +2826,22 @@ sub AddRenewal { > my $patron = Koha::Patrons->find( $borrowernumber ) or return; # FIXME Should do more than just return > my $patron_unblessed = $patron->unblessed; > >+ $schema->storage->txn_begin; >+ > if ( C4::Context->preference('CalculateFinesOnReturn') && $issue->is_overdue ) { >- _CalculateAndUpdateFine( { issue => $issue, item => $item, borrower => $patron_unblessed } ); >+ eval{ >+ _CalculateAndUpdateFine( { issue => $issue, item => $item, borrower => $patron_unblessed } ); >+ }; >+ if ( $@ ) { >+ $schema->txn_rollback; >+ return; >+ } >+ } >+ eval{ _FixOverduesOnReturn( $borrowernumber, $itemnumber ); }; >+ if ( $@ ) { >+ $schema->txn_rollback; >+ return; > } >- _FixOverduesOnReturn( $borrowernumber, $itemnumber ); > > # If the due date wasn't specified, calculate it by adding the > # book's loan length to today's date or the current due date >@@ -2844,34 +2857,42 @@ sub AddRenewal { > > # Update the issues record to have the new due date, and a new count > # of how many times it has been renewed. >- my $renews = $issue->renewals + 1; >- my $sth = $dbh->prepare("UPDATE issues SET date_due = ?, renewals = ?, lastreneweddate = ? >- WHERE borrowernumber=? >- AND itemnumber=?" >- ); >- >- $sth->execute( $datedue->strftime('%Y-%m-%d %H:%M'), $renews, $lastreneweddate, $borrowernumber, $itemnumber ); >- >- # Update the renewal count on the item, and tell zebra to reindex >- $renews = $item->{renewals} + 1; >- ModItem({ renewals => $renews, onloan => $datedue->strftime('%Y-%m-%d %H:%M')}, $item->{biblionumber}, $itemnumber); >- >- # Charge a new rental fee, if applicable? >- my ( $charge, $type ) = GetIssuingCharges( $itemnumber, $borrowernumber ); >- if ( $charge > 0 ) { >- my $accountno = getnextacctno( $borrowernumber ); >- my $manager_id = 0; >- $manager_id = C4::Context->userenv->{'number'} if C4::Context->userenv; >- $sth = $dbh->prepare( >- "INSERT INTO accountlines >- (date, borrowernumber, accountno, amount, manager_id, >- description,accounttype, amountoutstanding, itemnumber) >- VALUES (now(),?,?,?,?,?,?,?,?)" >+ my ( $charge, $type ); >+ eval { >+ my $renews = $issue->renewals + 1; >+ my $sth = $dbh->prepare("UPDATE issues SET date_due = ?, renewals = ?, lastreneweddate = ? >+ WHERE borrowernumber=? >+ AND itemnumber=?" > ); >- $sth->execute( $borrowernumber, $accountno, $charge, $manager_id, >- "Renewal of Rental Item " . $biblio->title . " $item->{'barcode'}", >- 'Rent', $charge, $itemnumber ); >+ >+ $sth->execute( $datedue->strftime('%Y-%m-%d %H:%M'), $renews, $lastreneweddate, $borrowernumber, $itemnumber ); >+ >+ # Update the renewal count on the item, and tell zebra to reindex >+ $renews = $item->{renewals} + 1; >+ ModItem({ renewals => $renews, onloan => $datedue->strftime('%Y-%m-%d %H:%M')}, $item->{biblionumber}, $itemnumber); >+ >+ # Charge a new rental fee, if applicable? >+ ( $charge, $type ) = GetIssuingCharges( $itemnumber, $borrowernumber ); >+ if ( $charge > 0 ) { >+ my $accountno = getnextacctno( $borrowernumber ); >+ my $manager_id = 0; >+ $manager_id = C4::Context->userenv->{'number'} if C4::Context->userenv; >+ $sth = $dbh->prepare( >+ "INSERT INTO accountlines >+ (date, borrowernumber, accountno, amount, manager_id, >+ description,accounttype, amountoutstanding, itemnumber) >+ VALUES (now(),?,?,?,?,?,?,?,?)" >+ ); >+ $sth->execute( $borrowernumber, $accountno, $charge, $manager_id, >+ "Renewal of Rental Item " . $biblio->title . " $item->{'barcode'}", >+ 'Rent', $charge, $itemnumber ); >+ } >+ }; >+ if ( $@ ) { >+ $schema->txn_rollback; >+ return; > } >+ $schema->txn_commit; > > # Send a renewal slip according to checkout alert preferencei > if ( C4::Context->preference('RenewalSendNotice') eq '1' ) { >-- >2.1.4
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 20086
:
70879
|
94374
|
94377
|
94378
|
94421
|
94422