If $limit is left undefined it causes a warn in error log
Created attachment 39056 [details] [review] Bug 14185: Undefined $limit causes warn in opac/opac-readingrecord.pl This patch sets $limit to be an empty string. To test: 1) On the OPAC, click on your account in the top right (Welcome, 'yourname') 2) Go to 'your reading history' tab 3) Notice warns in error log 4) Apply patch 5) Reload page 6) Notice page still works but no warns
Created attachment 39111 [details] [review] 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>
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?
(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!
(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. :)
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>
Patch pushed to master. Thanks Aleisha!
Pushed to 3.20.x will be in 3.20.1
Patch pushed to 3.18.x will be in 3.18.08
Pushed to 3.16.x, will be in 3.16.13