Bug 23220 - Cancelling transfer on returns.pl is subject to a race condition
Summary: Cancelling transfer on returns.pl is subject to a race condition
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Nick Clemens (kidclamp)
QA Contact: Josef Moravec
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-06-26 12:47 UTC by Nick Clemens (kidclamp)
Modified: 2020-06-04 20:32 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
19.11.00,19.05.03
Circulation function:


Attachments
Bug 23220: Prevent form submission before redirect (1.55 KB, patch)
2019-06-26 12:54 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 23220: Prevent form submission before redirect (1.59 KB, patch)
2019-06-26 14:04 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 23220: Prevent form submission before redirect (1.65 KB, patch)
2019-06-26 18:58 UTC, Josef Moravec
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens (kidclamp) 2019-06-26 12:47:39 UTC
To recreate:
1 - Check in an item
2 - If AutomaticItemReturn = Do it will generate a transfer, otherwise accept the transfer
3 - Check the item in again - your modal should now have a 'Cancel transfer' button
4 - Click it
5 - It may or may not work, depending on system speed
6 - edit return.tt, find the cancel transfer JS and edit it:
            $('.cancel-transfer').on("click",function(e){
                delay(5000);
                window.location.href='/cgi-bin/koha/circ/returns.pl?itemnumber=[% itemnumber | html %]&canceltransfer=1'
            });
7 - Now check in item and hit cancel transfer
8 - Repeat and note the 'Cancel transfer' button remains
9 - Click it and then view the item, it is in transit
Comment 1 Nick Clemens (kidclamp) 2019-06-26 12:48:17 UTC
The problem here is that the modal form is sill submitted on closing the modal, and if we aren't fast enough the redirect doesn't happen
Comment 2 Nick Clemens (kidclamp) 2019-06-26 12:54:09 UTC
Created attachment 91017 [details] [review]
Bug 23220: Prevent form submission before redirect

To test:
 1 - Check in an item to generate a transfer (form another library, no floating)
 2 - Check it in again, click 'Cancel transfer'
 3 - It may or may not work
 4 - To highlight the issue edit return.tt to add the delay line as below:
	$('.cancel-transfer').on("click",function(e){
		delay(5000);
		window.location.href='/cgi-bin/koha/circ/returns.pl?itemnumber=[% itemnumber | html %]&canceltransfer=1'
	});
 5 - Check in the item repeatedly, click 'Cancel transfer'
 6 - View the item in the staff or opac and verify it remains in transit
 7 - Undo changes above, apply patch
 8 - Test 'Cancel transfer', note it works
 9 - Add the delay back in, confirm it still works
Comment 3 Owen Leonard 2019-06-26 14:04:23 UTC
Created attachment 91021 [details] [review]
Bug 23220: Prevent form submission before redirect

To test:
 1 - Check in an item to generate a transfer (form another library, no floating)
 2 - Check it in again, click 'Cancel transfer'
 3 - It may or may not work
 4 - To highlight the issue edit return.tt to add the delay line as below:
	$('.cancel-transfer').on("click",function(e){
		delay(5000);
		window.location.href='/cgi-bin/koha/circ/returns.pl?itemnumber=[% itemnumber | html %]&canceltransfer=1'
	});
 5 - Check in the item repeatedly, click 'Cancel transfer'
 6 - View the item in the staff or opac and verify it remains in transit
 7 - Undo changes above, apply patch
 8 - Test 'Cancel transfer', note it works
 9 - Add the delay back in, confirm it still works

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 4 Josef Moravec 2019-06-26 18:58:48 UTC
Created attachment 91034 [details] [review]
Bug 23220: Prevent form submission before redirect

To test:
 1 - Check in an item to generate a transfer (form another library, no floating)
 2 - Check it in again, click 'Cancel transfer'
 3 - It may or may not work
 4 - To highlight the issue edit return.tt to add the delay line as below:
	$('.cancel-transfer').on("click",function(e){
		delay(5000);
		window.location.href='/cgi-bin/koha/circ/returns.pl?itemnumber=[% itemnumber | html %]&amp;canceltransfer=1'
	});
 5 - Check in the item repeatedly, click 'Cancel transfer'
 6 - View the item in the staff or opac and verify it remains in transit
 7 - Undo changes above, apply patch
 8 - Test 'Cancel transfer', note it works
 9 - Add the delay back in, confirm it still works

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 5 Martin Renvoize (ashimema) 2019-06-28 13:30:38 UTC
Nice work!

Pushed to master for 19.11.00
Comment 6 Fridolin Somers 2019-07-25 11:42:37 UTC
Pushed to 19.05.x for 19.05.03
Comment 7 Lucas Gass (lukeg) 2019-07-25 21:28:10 UTC
backported to 18.11.x for 18.11.09