Bug 13482 - A race condition prevents item info from being available for ReserveSlip (hold notice) printing.
Summary: A race condition prevents item info from being available for ReserveSlip (hol...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Olli-Antti Kivilahti
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
: 7255 12514 (view as bug list)
Depends on:
Blocks: 14099
  Show dependency treegraph
 
Reported: 2014-12-19 11:09 UTC by Olli-Antti Kivilahti
Modified: 2016-11-07 08:40 UTC (History)
13 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 13482 - A race condition prevents item info from being available for ReserveSlip (hold notice) printing. (9.30 KB, patch)
2014-12-19 11:15 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 13482 - A race condition prevents item info from being available for ReserveSlip (hold notice) printing. (9.02 KB, patch)
2014-12-19 11:22 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 13482 - A race condition prevents item info from being available for ReserveSlip (hold notice) printing. (9.09 KB, patch)
2015-02-04 19:22 UTC, Kyle M Hall
Details | Diff | Splinter Review
[ALTERNATIVE PATCH] Bug 13482: Prevent race condition on printing a ReserveSlip (8.60 KB, patch)
2015-04-16 12:21 UTC, Jonathan Druart
Details | Diff | Splinter Review
[ALTERNATIVE PATCH] Bug 13482: Prevent race condition on printing a ReserveSlip (8.63 KB, patch)
2015-04-21 08:59 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13482: Prevent race condition on printing a ReserveSlip (8.69 KB, patch)
2015-04-21 15:24 UTC, Mirko Tietgen
Details | Diff | Splinter Review
Bug 13482: Sent biblionumber to the template (961 bytes, patch)
2015-04-22 07:58 UTC, Jonathan Druart
Details | Diff | Splinter Review
[PASSED QA] Bug 13482: Prevent race condition on printing a ReserveSlip (8.75 KB, patch)
2015-04-22 19:29 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 13482: Sent biblionumber to the template (1.01 KB, patch)
2015-04-22 19:30 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Olli-Antti Kivilahti 2014-12-19 11:09:40 UTC
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.

Can be reproduced by entering the following to a javascript console.
Dopop('hold-transfer-slip.pl?transfer=1&borrowernumber=10904891&biblionumber=469263&op=slip');

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.
Comment 1 Olli-Antti Kivilahti 2014-12-19 11:15:10 UTC Comment hidden (obsolete)
Comment 2 Olli-Antti Kivilahti 2014-12-19 11:22:08 UTC Comment hidden (obsolete)
Comment 3 Katrin Fischer 2014-12-20 11:00:32 UTC
Could this be a duplicate to bug 12514 and bug 7255?
Comment 4 Kyle M Hall 2015-02-04 17:28:31 UTC
(In reply to Katrin Fischer from comment #3)
> Could this be a duplicate to bug 12514 and bug 7255?

Yes, I believe those should be marked as duplicates of this bug. The one has no patches, and the other has a patch that will never pass qa.
Comment 5 Kyle M Hall 2015-02-04 19:22:48 UTC Comment hidden (obsolete)
Comment 6 Jonathan Druart 2015-02-11 11:31:04 UTC
Olli, could you please provide a couple of tests for this change?
Comment 7 Katrin Fischer 2015-04-15 13:47:25 UTC
Would be nice to see this moving - I could reproduce the issue whenever a hold is confirmed for the first time in Koha 3.18.5.1.
Comment 8 Jonathan Druart 2015-04-16 12:21:42 UTC Comment hidden (obsolete)
Comment 9 Katrin Fischer 2015-04-20 15:13:42 UTC
*** Bug 12514 has been marked as a duplicate of this bug. ***
Comment 10 Katrin Fischer 2015-04-20 15:16:40 UTC
*** Bug 7255 has been marked as a duplicate of this bug. ***
Comment 11 Kyle M Hall 2015-04-20 16:18:18 UTC
Tested alternate patch, I receive no popup print notice on the initial checkin, print & confirm hold, but I do on the second time around:

http://screencast.com/t/s7lnVcBwox
Comment 12 Jonathan Druart 2015-04-21 08:59:19 UTC Comment hidden (obsolete)
Comment 13 Jonathan Druart 2015-04-21 09:00:15 UTC
(In reply to Kyle M Hall from comment #11)
> Tested alternate patch, I receive no popup print notice on the initial
> checkin, print & confirm hold, but I do on the second time around:
> 
> http://screencast.com/t/s7lnVcBwox

The print_slip hidden input was not set to 1 for 1 button, the one you tested :)
Please try again.
Comment 14 Mirko Tietgen 2015-04-21 15:24:41 UTC Comment hidden (obsolete)
Comment 15 Katrin Fischer 2015-04-21 20:37:08 UTC
I found a problem, I hope this will be enough to reproduce:

It's a transfer slip with a hold.

What I did:
- record with 1 item
- staff user is in Midway
- place record level hold with pickup location Fairfield
- check item in in Midway - transfer slip is triggered (status is transfer now)
- check the item in a second time  in Midway - transfer slip is triggered
  BUT: Slip reads: no hold found
  Without the patch the slip is printed correctly.
Comment 16 Jonathan Druart 2015-04-22 07:58:29 UTC Comment hidden (obsolete)
Comment 17 Jonathan Druart 2015-04-22 07:59:52 UTC
(In reply to Katrin Fischer from comment #15)
> I found a problem, I hope this will be enough to reproduce:
> 
> It's a transfer slip with a hold.
> 
> What I did:
> - record with 1 item
> - staff user is in Midway
> - place record level hold with pickup location Fairfield
> - check item in in Midway - transfer slip is triggered (status is transfer
> now)
> - check the item in a second time  in Midway - transfer slip is triggered
>   BUT: Slip reads: no hold found
>   Without the patch the slip is printed correctly.

Good catch Katrin.
This template is a mess, I tried to fix the problem easily.
Comment 18 Katrin Fischer 2015-04-22 19:29:56 UTC
Created attachment 38359 [details] [review]
[PASSED QA] Bug 13482: Prevent race condition on printing a ReserveSlip

The problem has been well described by Kyle and Nicole on bug 7255
description and comment 1.

With this counter patch, I suggest to popup AFTER the return has been
confirmed. This will remove the race condition and avoid to pass
itemnumber to the pl file as in the patch submitted by Olli.

Only the borrowernumber and biblionumber are needed (transfer and op are
never used by circ/hold-transfer-slip.pl).

Test plan:
It is very hard to know all cases (at least 6) where the popup is
displayed. Try at least to get the hold and the transfer confirmation
messages.
You should get the popup if you click on "print slip" button.

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 19 Katrin Fischer 2015-04-22 19:30:32 UTC
Created attachment 38360 [details] [review]
[PASSED QA] Bug 13482: Sent biblionumber to the template

The biblionumber is sent to the template as 'itembiblionumber' at 1 place, the template get the 'biblionumber' variable.
I don't understand why but the easy way is to sent the variable twice.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 20 Tomás Cohen Arazi 2015-04-23 15:19:18 UTC
Patches pushed to master.

Thanks Jonathan!
Comment 21 Christopher Brannon 2015-04-30 14:56:17 UTC
Found an issue with this patch.  Although most content now appears on first printing, it appears that the items.holdingbranch has incorrect data on first scan of item.  It seems it prints the library code of the library receiving the item on the first scan.  If you scan it again, it then shows the correct library code (the library that is scanning the item).

Before the patch, this field was blank on first scan.  Now it has the incorrect information on first scan.

Christopher
Comment 22 Katrin Fischer 2015-04-30 15:19:23 UTC
Hi Christopher, this patch is already pushed - so it would be better to have the problem description on a separate bug depending on this one and linkng them.

Can you tell which slip is the one that prints the wrong information? With a step by step instruction maybe?
Comment 23 Katrin Fischer 2015-05-04 07:22:24 UTC
Bug 14099 - thx Christopher!
Comment 24 Chris Cormack 2015-05-11 22:19:57 UTC
Pushed to 3.18.x will be in 3.18.7
Comment 25 Pongtawat 2015-11-20 18:09:20 UTC
As a note, this change cause print window pop-up to be blocked by default by both Firefox and Chrome.

In previous behavior, a print window pop-up is created when the print button is clicked so most browsers will allow it. In the new behavior, a print pop-up is created when the page reload, hence most browsers will block it by default.

So now we have to explicitly set the browser to allow pop-up from Koha staff interface.