Bug 14185

Summary: Noisy warns in opac-readingrecord.pl
Product: Koha Reporter: Aleisha Amohia <aleisha>
Component: OPACAssignee: Aleisha Amohia <aleisha>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: minor    
Priority: P5 - low CC: chris, indradg, mtj, mtompset, tomascohen, wizzyrea
Version: unspecified   
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:
Attachments: Bug 14185: Undefined $limit causes warn in opac/opac-readingrecord.pl
Bug 14185: Undefined $limit causes warn in opac/opac-readingrecord.pl
[PASSED QA] Bug 14185: Undefined $limit causes warn in opac/opac-readingrecord.pl

Description Aleisha Amohia 2015-05-12 05:14:59 UTC
If $limit is left undefined it causes a warn in error log
Comment 1 Aleisha Amohia 2015-05-12 05:20:00 UTC Comment hidden (obsolete)
Comment 2 Indranil Das Gupta 2015-05-13 00:16:02 UTC Comment hidden (obsolete)
Comment 3 Mark Tompsett 2015-05-13 04:44:07 UTC
Comment on attachment 39111 [details] [review]
Bug 14185: Undefined $limit causes warn in opac/opac-readingrecord.pl

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

::: opac/opac-readingrecord.pl
@@ +73,4 @@
>  
>  
>  my $limit = $query->param('limit');
> +$limit //= '';

This works, but why not add "// ''" to line 75?
Comment 4 Aleisha Amohia 2015-05-13 04:56:49 UTC
(In reply to M. Tompsett from comment #3)
> Comment on attachment 39111 [details] [review] [review]
> Bug 14185: Undefined $limit causes warn in opac/opac-readingrecord.pl
> 
> Review of attachment 39111 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> ::: opac/opac-readingrecord.pl
> @@ +73,4 @@
> >  
> >  
> >  my $limit = $query->param('limit');
> > +$limit //= '';
> 
> This works, but why not add "// ''" to line 75?

I'm still very new to perl and had seen this warn solved this way before so just did it again. Of course if you think your way is cleaner then do that!
Comment 5 Mark Tompsett 2015-05-13 17:48:59 UTC
(In reply to Aleisha Amohia from comment #4)
> I'm still very new to perl and had seen this warn solved this way before so
> just did it again.

Ah, okay. :) I didn't know. When I started perl, I was unaware of //. The difference between || and // is an important distinction to know. :)

undef || 'blah'; -- This is 'blah'
0     || 'blah'; -- This is 'blah'
''    || 'blah'; -- This is 'blah'
undef // 'blah'; -- This is 'blah'
0     // 'blah'; -- This is 0
''    // 'blah'; -- This is ''

As you can see // depends on whether the value is actually defined, not just its truth value. So, you actually chose the better operator inadvertently. :)
Comment 6 Kyle M Hall 2015-05-22 12:33:11 UTC
Created attachment 39403 [details] [review]
[PASSED QA] Bug 14185: Undefined $limit causes warn in opac/opac-readingrecord.pl

This patch sets $limit to be an empty string.

Test plan
=========

1/ login into the opac using your user account credentials
2/ in a terminal, run a `tail -f ` on your instance's opac-error.log
3/ go back to the opac, click on 'your reading history' tab to go to
   opac-readingrecord.pl
4/ notice the warning - "opac-readingrecord.pl: Use of uninitialized
   value $limit" appear in the `tail`ed opac-error.log
5/ apply the patch
6/ reload the page (opac-readingrecord.pl)
7/ page works but the warning in step #4 is no longer logged
8/ run qa test (i.e. koha-qa.pl -c 1 -v 2), there should be no error

Remarks: Testing result match expected test plan output. The QA tests
         pass with "OK" for the commit.

Signed-off-by: Indranil Das Gupta (L2C2 Technologies) <indradg@gmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 7 Tomás Cohen Arazi 2015-05-26 14:06:11 UTC
Patch pushed to master.

Thanks Aleisha!
Comment 8 Chris Cormack 2015-05-27 08:03:07 UTC
Pushed to 3.20.x will be in 3.20.1
Comment 9 Liz Rea 2015-06-22 22:35:56 UTC
Patch pushed to 3.18.x will be in 3.18.08
Comment 10 Mason James 2015-07-25 09:34:10 UTC
Pushed to 3.16.x, will be in 3.16.13