Bugzilla – Attachment 34528 Details for
Bug 13482
A race condition prevents item info from being available for ReserveSlip (hold notice) printing.
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 13482 - A race condition prevents item info from being available for ReserveSlip (hold notice) printing.
Bug-13482---A-race-condition-prevents-item-info-fr.patch (text/plain), 9.30 KB, created by
Olli-Antti Kivilahti
on 2014-12-19 11:15:10 UTC
(
hide
)
Description:
Bug 13482 - A race condition prevents item info from being available for ReserveSlip (hold notice) printing.
Filename:
MIME Type:
Creator:
Olli-Antti Kivilahti
Created:
2014-12-19 11:15:10 UTC
Size:
9.30 KB
patch
obsolete
>From 2897ecbca127a6b7cb88f35f9840af4ff4e326b8 Mon Sep 17 00:00:00 2001 >From: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> >Date: Fri, 19 Dec 2014 13:11:43 +0200 >Subject: [PATCH] Bug 13482 - A race condition prevents item info from being > available for ReserveSlip (hold notice) printing. > >Occasionally the ReserveSlip doesn't have any values from the koha.items-table. >There is no apparent reason why. The issue seems to be more prevalent when >fulfilling holds (checkin-in) to be transferred to another branch. > >Traced the issues to a race condition between printing a just-fulfilled hold >and writing the itemnumber to the reserves-table. > >TEST PLAN: > >1. Find a borrower, make a hold for him, and check-in the book to trigger the > "Hold found:"-prompt. Do not Confirm it! >2. Reproduce the print error by entering the following to a javascript console > at the same page. > Dopop('hold-transfer-slip.pl?transfer=1&borrowernumber=10904891&biblionumber=469263&op=slip'); >3. Notice the missing item fields. > >After this patch, one can confirm that it works by using this command: >Dopop('hold-transfer-slip.pl?transfer=1&borrowernumber=10904891&biblionumber=469263&itemnumber=10563811&op=slip'); > >MAKE SURE TO REPLACE PARAMETERS WITH CORRECT VALUES. >--- > C4/Reserves.pm | 11 +++++++++-- > circ/hold-transfer-slip.pl | 3 ++- > circ/returns.pl | 2 ++ > .../intranet-tmpl/prog/en/modules/circ/returns.tt | 10 +++++----- > 4 files changed, 18 insertions(+), 8 deletions(-) > >diff --git a/C4/Reserves.pm b/C4/Reserves.pm >index 5058c51..42dd625 100644 >--- a/C4/Reserves.pm >+++ b/C4/Reserves.pm >@@ -2314,14 +2314,19 @@ sub GetReserveId { > > =head2 ReserveSlip > >- ReserveSlip($branchcode, $borrowernumber, $biblionumber) >+ ReserveSlip($branchcode, $borrowernumber, $biblionumber, $itemnumber) > > Returns letter hash ( see C4::Letters::GetPreparedLetter ) or undef >+ >+$itemnumber is needed to make sure that the ReserveSlip printing has access to the item in question. >+There is a race condition where the slip is printed before the reservation is confirmed to the DB, >+thus the reserve target needs to be explicitly passed from the calling module if such a race condition is encoutered. >+Otherwise $itemnumber is pulled properly from the reserves-table. > > =cut > > sub ReserveSlip { >- my ($branch, $borrowernumber, $biblionumber) = @_; >+ my ($branch, $borrowernumber, $biblionumber, $itemnumber) = @_; > > # return unless ( C4::Context->boolean_preference('printreserveslips') ); > >@@ -2331,6 +2336,8 @@ sub ReserveSlip { > }) or return; > my $reserve = GetReserveInfo($reserve_id) or return; > >+ $reserve->{itemnumber} = $itemnumber if (not($reserve->{itemnumber}) && $itemnumber); >+ > return C4::Letters::GetPreparedLetter ( > module => 'circulation', > letter_code => 'RESERVESLIP', >diff --git a/circ/hold-transfer-slip.pl b/circ/hold-transfer-slip.pl >index 525e25a..2f062fb 100755 >--- a/circ/hold-transfer-slip.pl >+++ b/circ/hold-transfer-slip.pl >@@ -38,6 +38,7 @@ my $session = get_session($sessionID); > > my $biblionumber = $input->param('biblionumber'); > my $borrowernumber = $input->param('borrowernumber'); >+my $itemnumber = $input->param('itemnumber'); > my $transfer = $input->param('transfer'); > > my ( $template, $loggedinuser, $cookie ) = get_template_and_user( >@@ -53,7 +54,7 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user( > > my $userenv = C4::Context->userenv; > my ($slip, $is_html); >-if ( my $letter = ReserveSlip ($session->param('branch') || $userenv->{branch}, $borrowernumber, $biblionumber) ) { >+if ( my $letter = ReserveSlip ($session->param('branch') || $userenv->{branch}, $borrowernumber, $biblionumber, $itemnumber) ) { > $slip = $letter->{content}; > $is_html = $letter->{is_html}; > } >diff --git a/circ/returns.pl b/circ/returns.pl >index adc2f1e..ce16724 100755 >--- a/circ/returns.pl >+++ b/circ/returns.pl >@@ -236,6 +236,7 @@ if ($barcode) { > $barcode =~ s/^\s*|\s*$//g; # remove leading/trailing whitespace > $barcode = barcodedecode($barcode) if C4::Context->preference('itemBarcodeInputFilter'); > $itemnumber = GetItemnumberFromBarcode($barcode); >+ > > # > # save the return >@@ -267,6 +268,7 @@ if ($barcode) { > author => $biblio->{'author'}, > itembarcode => $biblio->{'barcode'}, > itemtype => $biblio->{'itemtype'}, >+ itemnumber => $itemnumber, > ccode => $biblio->{'ccode'}, > itembiblionumber => $biblio->{'biblionumber'}, > borrower => $borrower, >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt >index 3fe2e53..8b6cc73 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt >@@ -161,7 +161,7 @@ $(document).ready(function () { > <input type="hidden" name="WT-waitingAt" value="[% TransferWaitingAt %]" /> > <input type="hidden" name="WT-From" value="[% wtransfertFrom %]" /> > <input type="submit" class="approve" value="Confirm" /> >- <input type="submit" value="Print slip and confirm" class="print" onclick="Dopop('hold-transfer-slip.pl?transfer=1&borrowernumber=[% borrowernumber %]&biblionumber=[% itembiblionumber %]&op=slip'); this.form.submit();" /> >+ <input type="submit" value="Print slip and confirm" class="print" onclick="Dopop('hold-transfer-slip.pl?transfer=1&borrowernumber=[% borrowernumber %]&biblionumber=[% itembiblionumber %]&itemnumber=[% itemnumber %]&op=slip'); this.form.submit();" /> > <input type="hidden" name="return_date_override" value="[% return_date_override %]" /> > <input type="hidden" name="return_date_override_remember" value="[% return_date_override_remember %]" /> > </form> >@@ -199,7 +199,7 @@ $(document).ready(function () { > <form method="post" action="returns.pl" class="confirm"> > <input type="submit" class="approve" value="Confirm" /> > >- <input type="submit" value="Print and confirm" class="print" onclick="Dopop('hold-transfer-slip.pl?borrowernumber=[% borrowernumber %]&biblionumber=[% itembiblionumber %]&op=slip'); this.form.submit();" /> >+ <input type="submit" value="Print and confirm" class="print" onclick="Dopop('hold-transfer-slip.pl?borrowernumber=[% borrowernumber %]&biblionumber=[% itembiblionumber %]&itemnumber=[% itemnumber %]&op=slip'); this.form.submit();" /> > [% FOREACH inputloo IN inputloop %] > <input type="hidden" name="ri-[% inputloo.counter %]" value="[% inputloo.barcode %]" /> > <input type="hidden" name="dd-[% inputloo.counter %]" value="[% inputloo.duedate %]" /> >@@ -244,7 +244,7 @@ $(document).ready(function () { > > <form method="post" action="returns.pl" class="confirm"> > <input type="submit" class="approve" value="Confirm" /> >- <input type="submit" value="Print slip and confirm" class="print" onclick="Dopop('hold-transfer-slip.pl?transfer=1&borrowernumber=[% borrowernumber %]&biblionumber=[% itembiblionumber %]&op=slip'); this.form.submit();" /> >+ <input type="submit" value="Print slip and confirm" class="print" onclick="Dopop('hold-transfer-slip.pl?transfer=1&borrowernumber=[% borrowernumber %]&biblionumber=[% itembiblionumber %]&itemnumber=[% itemnumber %]&op=slip'); this.form.submit();" /> > [% FOREACH inputloo IN inputloop %] > <input type="hidden" name="ri-[% inputloo.counter %]" value="[% inputloo.barcode %]" /> > <input type="hidden" name="dd-[% inputloo.counter %]" value="[% inputloo.duedate %]" /> >@@ -361,10 +361,10 @@ $(document).ready(function () { > <form method="post" action="returns.pl" class="confirm"> > [% IF ( transfertodo ) %] > <input type="submit" class="approve" value="Confirm hold and transfer" /> >- <input type="submit" value="Print slip, transfer, and confirm" class="print" onclick="Dopop('hold-transfer-slip.pl?transfer=1&borrowernumber=[% borrowernumber %]&biblionumber=[% itembiblionumber %]&op=slip'); this.form.submit();" /> >+ <input type="submit" value="Print slip, transfer, and confirm" class="print" onclick="Dopop('hold-transfer-slip.pl?transfer=1&borrowernumber=[% borrowernumber %]&biblionumber=[% itembiblionumber %]&itemnumber=[% itemnumber %]&op=slip'); this.form.submit();" /> > [% ELSE %] > <input type="submit" class="approve" value="Confirm hold" /> >- <input type="submit" value="Print slip and confirm" class="print" onclick="Dopop('hold-transfer-slip.pl?borrowernumber=[% borrowernumber %]&biblionumber=[% itembiblionumber %]&op=slip'); this.form.submit();" /> >+ <input type="submit" value="Print slip and confirm" class="print" onclick="Dopop('hold-transfer-slip.pl?borrowernumber=[% borrowernumber %]&biblionumber=[% itembiblionumber %]&itemnumber=[% itemnumber %]&op=slip'); this.form.submit();" /> > [% END %] > <input type="submit" class="deny" value="Ignore" onclick="$('.dialog:visible').hide('slow'); $('#barcode').focus(); return false;" /> > [% FOREACH inputloo IN inputloop %] >-- >1.7.9.5
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 13482
:
34528
|
34529
|
35664
|
37956
|
38234
|
38282
|
38304
|
38359
|
38360