Bug 28148

Summary: JavaScript error when printing transfer slip for existing transfer
Product: Koha Reporter: Nick Clemens <nick>
Component: CirculationAssignee: Nick Clemens <nick>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: andrewfh, fridolin.somers, gmcharlt, kyle.m.hall, martin.renvoize, victor
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:
21.05.00,20.11.06,20.05.12
Bug Depends on: 28064    
Bug Blocks:    
Attachments: Bug 28148: Remove 'print' class from 'Print transfer slip' when transfer exists
Bug 28148: Remove 'print' class from 'Print transfer slip' when transfer exists
Bug 28148: Remove 'print' class from 'Print transfer slip' when transfer exists
Bug 28148: Remove 'print' class from 'Print transfer slip' when transfer exists

Description Nick Clemens 2021-04-14 15:17:41 UTC
When there is a transfer in progress the modal 'Print transfer slip' button has both 'print' and 'openWin' classes

The latter takes care of launching the popup - the former is not needed because there is no form to submit because the transfer exists

JS error:
Uncaught TypeError: this.form is null
    <anonymous> http://localhost:8081/cgi-bin/koha/circ/returns.pl:1093
    jQuery 9
    <anonymous> http://localhost:8081/cgi-bin/koha/circ/returns.pl:1088
    jQuery 8
Comment 1 Nick Clemens 2021-04-14 15:20:59 UTC
Created attachment 119588 [details] [review]
Bug 28148: Remove 'print' class from 'Print transfer slip' when transfer exists

The 'print' class triggers JS to create the transfer and update a form

When a transfer exists we have no form and use 'openWin' to trigger the print

Also, I remove a stray '1' in the template

To test:
1 - Set AutomaticItemReturn to "Don't"
2 - Check in an item from another branch
3 - Confirm the transfer
4 - Check it in again
5 - Click 'Print slip'
6 - Note error in console:
     Uncaught TypeError: this.form is null
7 - apply patch
8 - Repeat
9 - No error!
Comment 2 Martin Renvoize 2021-04-14 15:43:28 UTC
Created attachment 119589 [details] [review]
Bug 28148: Remove 'print' class from 'Print transfer slip' when transfer exists

The 'print' class triggers JS to create the transfer and update a form

When a transfer exists we have no form and use 'openWin' to trigger the print

Also, I remove a stray '1' in the template

To test:
1 - Set AutomaticItemReturn to "Don't"
2 - Check in an item from another branch
3 - Confirm the transfer
4 - Check it in again
5 - Click 'Print slip'
6 - Note error in console:
     Uncaught TypeError: this.form is null
7 - apply patch
8 - Repeat
9 - No error!

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 3 Martin Renvoize 2021-04-14 15:44:42 UTC
This does resolve the JS error and allows slips to still be printed.

There looks to be other cases in the code, but I can't for the life of me work out how to trigger them..

Signing off for now.. other stuff can be handled in other bugs... this code really needs a concerted cleanup.. it's incredibly difficult to follow.
Comment 4 Owen Leonard 2021-04-14 15:58:11 UTC
Created attachment 119592 [details] [review]
Bug 28148: Remove 'print' class from 'Print transfer slip' when transfer exists

The 'print' class triggers JS to create the transfer and update a form

When a transfer exists we have no form and use 'openWin' to trigger the
print

Also, I remove a stray '1' in the template

To test:
1 - Set AutomaticItemReturn to "Don't"
2 - Check in an item from another branch
3 - Confirm the transfer
4 - Check it in again
5 - Click 'Print slip'
6 - Note error in console:
    Uncaught TypeError: this.form is null
7 - apply patch
8 - Repeat
9 - No error!

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 5 Katrin Fischer 2021-04-17 14:28:57 UTC
It looks like we have a sign-off from Owen now too - Martin, do you want to add your line again counting for QA?
Comment 6 Martin Renvoize 2021-04-27 13:56:49 UTC
Created attachment 120234 [details] [review]
Bug 28148: Remove 'print' class from 'Print transfer slip' when transfer exists

The 'print' class triggers JS to create the transfer and update a form

When a transfer exists we have no form and use 'openWin' to trigger the
print

Also, I remove a stray '1' in the template

To test:
1 - Set AutomaticItemReturn to "Don't"
2 - Check in an item from another branch
3 - Confirm the transfer
4 - Check it in again
5 - Click 'Print slip'
6 - Note error in console:
    Uncaught TypeError: this.form is null
7 - apply patch
8 - Repeat
9 - No error!

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 7 Martin Renvoize 2021-04-27 13:57:15 UTC
Restored SO line and converted to QA :)

Passed
Comment 8 Jonathan Druart 2021-04-28 09:17:01 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 9 Fridolin Somers 2021-04-30 09:24:39 UTC
Pushed to 20.11.x for 20.11.06
Comment 10 Andrew Fuerste-Henry 2021-05-25 11:01:06 UTC
Pushed to 20.05.x for 20.05.12
Comment 11 Victor Grousset/tuxayo 2021-05-25 13:56:12 UTC
Not backported to oldoldstable (19.11.x). Feel free to ask if it's needed.