Bug 14445 - Noisy warns in Notices (letter.tt) module
Summary: Noisy warns in Notices (letter.tt) module
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Aleisha Amohia
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-06-24 03:51 UTC by Aleisha Amohia
Modified: 2016-12-05 21:22 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 14445: Silences warn in letter.pl (7.84 KB, patch)
2015-06-24 04:01 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 14445: Silences warns in letter.tt (2.36 KB, patch)
2015-07-01 04:23 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 14445: Silences warn in letter.pl (1.30 KB, patch)
2015-07-09 07:42 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14445: Silences warns in letter.tt (2.87 KB, patch)
2015-07-09 07:42 UTC, Jonathan Druart
Details | Diff | Splinter Review
[PASSED QA] Bug 14445: Silences warn in letter.pl (1.38 KB, patch)
2015-07-10 14:30 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 14445: Silences warns in letter.tt (2.93 KB, patch)
2015-07-10 14:30 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Aleisha Amohia 2015-06-24 03:51:35 UTC
A few noisy warns in koha-tmpl/intranet-tmpl/prog/en/modules/tools/letter.tt:

In staff interface, go to Tools, then Notices & Slips
Error: Click 'new notice' --> Argument "" isn't numeric in numeric gt (>) at line 400
Error: Change Koha module any other module --> same error
Error: Change Koha module to 'Circulation' --> same error, and use of uninitialized value $code in string eq at line 230
Comment 1 Aleisha Amohia 2015-06-24 03:54:21 UTC
(In reply to Aleisha Amohia from comment #0)

> Error: Change Koha module to 'Circulation' --> same error, and use of
> uninitialized value $code in string eq at line 230

my mistake, this error is actually in tools/letter.pl
Comment 2 Aleisha Amohia 2015-06-24 04:01:41 UTC Comment hidden (obsolete)
Comment 3 Aleisha Amohia 2015-07-01 04:23:36 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2015-07-02 14:52:21 UTC
  $code //= '';
should be put at the beginning of the subroutine, not right in the middle :)
Comment 5 Jonathan Druart 2015-07-02 14:53:25 UTC
Comment on attachment 40743 [details] [review]
Bug 14445: Silences warns in letter.tt

Review of attachment 40743 [details] [review]:
-----------------------------------------------------------------

::: koha-tmpl/intranet-tmpl/prog/en/modules/tools/letter.tt
@@ +397,4 @@
>                  <li>
>                    <label for="SQLfieldname_[% letter.message_transport_type %]">Message body:</label>
>                    [% IF letter.message_transport_type == 'sms' %]
> +                    <span id="sms_counter">[% IF letter.length && letter.length > 0 %][% letter.length %][% ELSE %]0[% END %]/160 characters</span>

This is wrong, we want to display the length of the content.
With your patch, the string displayed is always
   "Message body: 0/160 characters"
Comment 6 Aleisha Amohia 2015-07-03 02:47:04 UTC
(In reply to Jonathan Druart from comment #5)
>
> This is wrong, we want to display the length of the content.
> With your patch, the string displayed is always
>    "Message body: 0/160 characters"

Hi Jonathan, this tests correctly for me, but do you know a better way to silence this warn?
Comment 7 Jonathan Druart 2015-07-09 07:42:31 UTC Comment hidden (obsolete)
Comment 8 Jonathan Druart 2015-07-09 07:42:36 UTC Comment hidden (obsolete)
Comment 9 Jonathan Druart 2015-07-09 07:46:57 UTC
(In reply to Aleisha Amohia from comment #6)
> (In reply to Jonathan Druart from comment #5)
> >
> > This is wrong, we want to display the length of the content.
> > With your patch, the string displayed is always
> >    "Message body: 0/160 characters"
> 
> Hi Jonathan, this tests correctly for me, but do you know a better way to
> silence this warn?

You should have used letter.content.length.
I have amended your 2 patches, please have a look.
Comment 10 Kyle M Hall 2015-07-10 14:30:07 UTC
Created attachment 40922 [details] [review]
[PASSED QA] Bug 14445: Silences warn in letter.pl

When changing Koha module to 'Circulation', there is a warn saying that $code is uninitialized. This patch sets $code to an empty string to silence the warn.

To test:
1) Go to Tools, the Notices & Slips
2) Click 'new notice' (This will trigger warns, but ignore these as they will be corrected in the next patch)
3) Change Koha module to 'Circulation'
4) Notice warn about uninitialized $code variable
5) Apply patch and reload page, change Koha module to 'Circulation'
6) Notice page still works and warns are gone

Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 11 Kyle M Hall 2015-07-10 14:30:16 UTC
Created attachment 40923 [details] [review]
[PASSED QA] Bug 14445: Silences warns in letter.tt

When creating a new notice, warn is triggered "Argument "" isn't numeric in numeric gt (>) at line 400". Same warn is triggered when changing Koha module option to any other module.

To test:
1) Go to Tools, then Notices & Slips
2) Click 'new notice'. Notice warn in intranet-error.log
3) Change Koha module to another module. Notice warn is triggered for every change
4) Apply patch and reload page
5) Change Koha module to another module. Notice there are no longer warns
6) Go back to Notices & Slips and click 'new notice' again. Notice there are no warns

Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 12 Tomás Cohen Arazi 2015-07-20 14:04:51 UTC
Patches pushed to master.

Thanks Aleisha!
Comment 13 Chris Cormack 2015-08-04 21:13:30 UTC
Pushed to 3.20.x will be in 3.20.3
Comment 14 Liz Rea 2015-08-11 02:00:08 UTC
Pushed to 3.18.x, will be in 3.18.10.
Comment 15 Mason James 2015-08-28 05:43:58 UTC
Pushed to 3.16.x, will be in 3.16.14