Bug 28148 - JavaScript error when printing transfer slip for existing transfer
Summary: JavaScript error when printing transfer slip for existing transfer
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Nick Clemens
QA Contact: Testopia
URL:
Keywords:
Depends on: 28064
Blocks:
  Show dependency treegraph
 
Reported: 2021-04-14 15:17 UTC by Nick Clemens
Modified: 2021-12-13 21:10 UTC (History)
6 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:
21.05.00,20.11.06,20.05.12


Attachments
Bug 28148: Remove 'print' class from 'Print transfer slip' when transfer exists (2.97 KB, patch)
2021-04-14 15:20 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 28148: Remove 'print' class from 'Print transfer slip' when transfer exists (3.03 KB, patch)
2021-04-14 15:43 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28148: Remove 'print' class from 'Print transfer slip' when transfer exists (3.02 KB, patch)
2021-04-14 15:58 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 28148: Remove 'print' class from 'Print transfer slip' when transfer exists (3.08 KB, patch)
2021-04-27 13:56 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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.