Bug 23220

Summary: Cancelling transfer on returns.pl is subject to a race condition
Product: Koha Reporter: Nick Clemens (kidclamp) <nick>
Component: CirculationAssignee: Nick Clemens (kidclamp) <nick>
Status: CLOSED FIXED QA Contact: Josef Moravec <josef.moravec>
Severity: normal    
Priority: P5 - low CC: fridolin.somers, gmcharlt, josef.moravec, kyle.m.hall, lucas
Version: Main   
Hardware: All   
OS: All   
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
Bug 23220: Prevent form submission before redirect
Bug 23220: Prevent form submission before redirect

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 %]&amp;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 %]&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
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 %]&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>
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